| Message ID | 1444648509-29179-3-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 10/12/2015 05:14 AM, Daniel P. Berrange wrote: > The qapi_copy_SocketAddress method is going to be useful > in more places than just qemu-char.c, so move it into > the qemu-sockets.c file to allow its reuse. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > include/qemu/sockets.h | 4 ++++ > qemu-char.c | 25 ------------------------- > util/qemu-sockets.c | 30 ++++++++++++++++++++++++++++++ > 3 files changed, 34 insertions(+), 25 deletions(-) > > +++ b/qemu-char.c > @@ -92,31 +92,6 @@ > > /***********************************************************/ > /* Socket address helpers */ > -static void qapi_copy_SocketAddress(SocketAddress **p_dest, > - SocketAddress *src) > -{ > - QmpOutputVisitor *qov; > - QmpInputVisitor *qiv; > - Visitor *ov, *iv; > - QObject *obj; > - > - *p_dest = NULL; > - > - qov = qmp_output_visitor_new(); > - ov = qmp_output_get_visitor(qov); > - visit_type_SocketAddress(ov, &src, NULL, &error_abort); > - obj = qmp_output_get_qobject(qov); > - qmp_output_visitor_cleanup(qov); > - if (!obj) { > - return; > - } > - > - qiv = qmp_input_visitor_new(obj); > - iv = qmp_input_get_visitor(qiv); > - visit_type_SocketAddress(iv, p_dest, NULL, &error_abort); > - qmp_input_visitor_cleanup(qiv); > - qobject_decref(obj); Interesting approach - it means that this copy will work no matter what further extensions we add into the qapi type. But rather heavyweight compared to just doing a memberwise copy, no? At any rate, this commit is straight code motion, so you did it correctly, but we may want to simplify things in a later commit. Reviewed-by: Eric Blake <eblake@redhat.com>
On 20/10/2015 00:05, Eric Blake wrote: > > - qiv = qmp_input_visitor_new(obj); > > - iv = qmp_input_get_visitor(qiv); > > - visit_type_SocketAddress(iv, p_dest, NULL, &error_abort); > > - qmp_input_visitor_cleanup(qiv); > > - qobject_decref(obj); > Interesting approach - it means that this copy will work no matter what > further extensions we add into the qapi type. But rather heavyweight > compared to just doing a memberwise copy, no? At any rate, this commit > is straight code motion, so you did it correctly, but we may want to > simplify things in a later commit. A memberwise copy requires you to allocate embedded objects (I'm not sure whether your boxing changes can simplify this). Paolo
On Tue, Oct 20, 2015 at 02:08:09PM +0200, Paolo Bonzini wrote: > > > On 20/10/2015 00:05, Eric Blake wrote: > > > - qiv = qmp_input_visitor_new(obj); > > > - iv = qmp_input_get_visitor(qiv); > > > - visit_type_SocketAddress(iv, p_dest, NULL, &error_abort); > > > - qmp_input_visitor_cleanup(qiv); > > > - qobject_decref(obj); > > Interesting approach - it means that this copy will work no matter what > > further extensions we add into the qapi type. But rather heavyweight > > compared to just doing a memberwise copy, no? At any rate, this commit > > is straight code motion, so you did it correctly, but we may want to > > simplify things in a later commit. > > A memberwise copy requires you to allocate embedded objects (I'm not > sure whether your boxing changes can simplify this). It would probably be a nice idea if the QAPI code generator could emit an efficient copy func. So if someone wants an coding exercise.... In the mean time, this socket kaddress copy func is not called in any performance critical code paths, so the inefficiency doesn't particularly matter for now. Regards, Daniel
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h index 3ea7cc9..5a183c5 100644 --- a/include/qemu/sockets.h +++ b/include/qemu/sockets.h @@ -118,4 +118,8 @@ SocketAddress *socket_local_address(int fd, Error **errp); */ SocketAddress *socket_remote_address(int fd, Error **errp); + +void qapi_copy_SocketAddress(SocketAddress **p_dest, + SocketAddress *src); + #endif /* QEMU_SOCKET_H */ diff --git a/qemu-char.c b/qemu-char.c index 653ea10..50ea4f7 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -92,31 +92,6 @@ /***********************************************************/ /* Socket address helpers */ -static void qapi_copy_SocketAddress(SocketAddress **p_dest, - SocketAddress *src) -{ - QmpOutputVisitor *qov; - QmpInputVisitor *qiv; - Visitor *ov, *iv; - QObject *obj; - - *p_dest = NULL; - - qov = qmp_output_visitor_new(); - ov = qmp_output_get_visitor(qov); - visit_type_SocketAddress(ov, &src, NULL, &error_abort); - obj = qmp_output_get_qobject(qov); - qmp_output_visitor_cleanup(qov); - if (!obj) { - return; - } - - qiv = qmp_input_visitor_new(obj); - iv = qmp_input_get_visitor(qiv); - visit_type_SocketAddress(iv, p_dest, NULL, &error_abort); - qmp_input_visitor_cleanup(qiv); - qobject_decref(obj); -} static int SocketAddress_to_str(char *dest, int max_len, const char *prefix, SocketAddress *addr, diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 2bdfacf..9cc5bee 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -25,6 +25,9 @@ #include "monitor/monitor.h" #include "qemu/sockets.h" #include "qemu/main-loop.h" +#include "qapi/qmp-input-visitor.h" +#include "qapi/qmp-output-visitor.h" +#include "qapi-visit.h" #ifndef AI_ADDRCONFIG # define AI_ADDRCONFIG 0 @@ -1125,3 +1128,30 @@ SocketAddress *socket_remote_address(int fd, Error **errp) return socket_sockaddr_to_address(&ss, sslen, errp); } + + +void qapi_copy_SocketAddress(SocketAddress **p_dest, + SocketAddress *src) +{ + QmpOutputVisitor *qov; + QmpInputVisitor *qiv; + Visitor *ov, *iv; + QObject *obj; + + *p_dest = NULL; + + qov = qmp_output_visitor_new(); + ov = qmp_output_get_visitor(qov); + visit_type_SocketAddress(ov, &src, NULL, &error_abort); + obj = qmp_output_get_qobject(qov); + qmp_output_visitor_cleanup(qov); + if (!obj) { + return; + } + + qiv = qmp_input_visitor_new(obj); + iv = qmp_input_get_visitor(qiv); + visit_type_SocketAddress(iv, p_dest, NULL, &error_abort); + qmp_input_visitor_cleanup(qiv); + qobject_decref(obj); +}
The qapi_copy_SocketAddress method is going to be useful in more places than just qemu-char.c, so move it into the qemu-sockets.c file to allow its reuse. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- include/qemu/sockets.h | 4 ++++ qemu-char.c | 25 ------------------------- util/qemu-sockets.c | 30 ++++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 25 deletions(-)