[libvirt] [PATCH 0/4] Various cleanups

*** BLURB HERE *** Martin Kletzander (4): build: Remove unnecessarily repeated rules for syms -> def rpc: Fix possible crash when MDNSAddEntry fails Generate JSON with mDNS entries only when built --with-avahi tests: Use libvirt properly with initialization and error dispatching src/Makefile.am | 20 ++------------------ src/rpc/virnetserver.c | 4 +++- tests/virnetservertest.c | 35 ++++++++++++++++++++--------------- 3 files changed, 25 insertions(+), 34 deletions(-) -- 2.4.3

Suggested-by: Michal Prívozník <mprivozn@redhat.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/Makefile.am | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 0d1f58b98022..a9ebb07b1d32 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2003,29 +2003,13 @@ libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) \ chmod a-w $@-tmp && \ mv $@-tmp libvirt.syms -libvirt.def: libvirt.syms +%.def: %.syms $(AM_V_GEN)rm -f -- $@-tmp $@ ; \ printf 'EXPORTS\n' > $@-tmp && \ sed -e '/^$$/d; /#/d; /:/d; /}/d; /\*/d; /LIBVIRT_/d' \ -e 's/[ ]*\(.*\)\;/ \1/g' $^ >> $@-tmp && \ chmod a-w $@-tmp && \ - mv $@-tmp libvirt.def - -libvirt_qemu.def: $(srcdir)/libvirt_qemu.syms - $(AM_V_GEN)rm -f -- $@-tmp $@ ; \ - printf 'EXPORTS\n' > $@-tmp && \ - sed -e '/^$$/d; /#/d; /:/d; /}/d; /\*/d; /LIBVIRT_/d' \ - -e 's/[ ]*\(.*\)\;/ \1/g' $^ >> $@-tmp && \ - chmod a-w $@-tmp && \ - mv $@-tmp libvirt_qemu.def - -libvirt_lxc.def: $(srcdir)/libvirt_lxc.syms - $(AM_V_GEN)rm -f -- $@-tmp $@ ; \ - printf 'EXPORTS\n' > $@-tmp && \ - sed -e '/^$$/d; /#/d; /:/d; /}/d; /\*/d; /LIBVIRT_/d' \ - -e 's/[ ]*\(.*\)\;/ \1/g' $^ >> $@-tmp && \ - chmod a-w $@-tmp && \ - mv $@-tmp libvirt_lxc.def + mv $@-tmp $@ # Empty source list - it merely links a bunch of convenience libs together libvirt_la_SOURCES = -- 2.4.3

If virNetServerMDNSAddEntry() fails when adding a service to a server, it doesn't decrease the number of services. Hence access to their members segfaults (e.g. when free()-ing the sruct). Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/virnetserver.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 091a1dc0bc8f..2af878977a1b 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -974,8 +974,10 @@ int virNetServerAddService(virNetServerPtr srv, if (!virNetServerMDNSAddEntry(srv->mdnsGroup, mdnsEntryName, - port)) + port)) { + srv->nservices--; goto error; + } } srv->services[srv->nservices-1] = svc; -- 2.4.3

One string was already used only if that condition was true, second one is added now. Both are used in a nicer way. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tests/virnetservertest.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/virnetservertest.c b/tests/virnetservertest.c index 596fabedd38b..e8e91f867e45 100644 --- a/tests/virnetservertest.c +++ b/tests/virnetservertest.c @@ -35,6 +35,13 @@ testCreateServer(const char *host, int family) virNetServerClientPtr cln1 = NULL, cln2 = NULL; virNetSocketPtr sk1 = NULL, sk2 = NULL; int fdclient[2]; + const char *mdns_entry = NULL; + const char *mdns_group = NULL; + +# ifdef WITH_AVAHI + mdns_entry = "libvirt-ro"; + mdns_group = "libvirtTest"; +# endif if (socketpair(PF_UNIX, SOCK_STREAM, 0, fdclient) < 0) { virReportSystemError(errno, "%s", @@ -44,11 +51,7 @@ testCreateServer(const char *host, int family) if (!(srv = virNetServerNew(10, 50, 5, 100, 10, 120, 5, true, -# ifdef WITH_AVAHI - "libvirtTest", -# else - NULL, -# endif + mdns_group, NULL, NULL, NULL, @@ -79,9 +82,9 @@ testCreateServer(const char *host, int family) 5))) goto error; - if (virNetServerAddService(srv, svc1, "libvirt-ro") < 0) + if (virNetServerAddService(srv, svc1, mdns_entry) < 0) goto error; - if (virNetServerAddService(srv, svc2, "libvirt-ro") < 0) + if (virNetServerAddService(srv, svc2, mdns_entry) < 0) goto error; if (virNetSocketNewConnectSockFD(fdclient[0], &sk1) < 0) -- 2.4.3

We were using "complicated" error printing in virnetservertest even though we could've just dispatched the error. Also add some good practices that might come in handy (the code may fail without proper initialization and event loop). Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tests/virnetservertest.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/virnetservertest.c b/tests/virnetservertest.c index e8e91f867e45..d546f87764c1 100644 --- a/tests/virnetservertest.c +++ b/tests/virnetservertest.c @@ -119,6 +119,8 @@ testCreateServer(const char *host, int family) goto error; cleanup: + if (!srv) + virDispatchError(NULL); virObjectUnref(cln1); virObjectUnref(cln2); virObjectUnref(svc1); @@ -235,14 +237,8 @@ static int testExecRestart(const void *opaque) ret = 0; cleanup: - if (ret < 0) { - virErrorPtr err = virGetLastError(); - /* Rather be safe, we have lot of missing errors */ - if (err) - fprintf(stderr, "%s\n", err->message); - else - fprintf(stderr, "%s\n", "Unknown error"); - } + if (ret < 0) + virDispatchError(NULL); fail: VIR_FREE(infile); VIR_FREE(outfile); @@ -264,6 +260,12 @@ mymain(void) { int ret = 0; + if (virInitialize() < 0 || + virEventRegisterDefaultImpl() < 0) { + virDispatchError(NULL); + return EXIT_FAILURE; + } + /* Hack to make it easier to generate new JSON files when * the RPC classes change. Just set this env var, save * the generated JSON, and replace the file descriptor -- 2.4.3

On Mon, Jun 15, 2015 at 03:08:38PM +0200, Martin Kletzander wrote:
*** BLURB HERE ***
Martin Kletzander (4): build: Remove unnecessarily repeated rules for syms -> def rpc: Fix possible crash when MDNSAddEntry fails Generate JSON with mDNS entries only when built --with-avahi tests: Use libvirt properly with initialization and error dispatching
src/Makefile.am | 20 ++------------------ src/rpc/virnetserver.c | 4 +++- tests/virnetservertest.c | 35 ++++++++++++++++++++--------------- 3 files changed, 25 insertions(+), 34 deletions(-)
ACK series Jan
participants (2)
-
Ján Tomko
-
Martin Kletzander