- Devel - libvirt List Archives

[libvirt] [PATCH 0/6] plug memory leaks
by ajia＠redhat.com 01 Dec '11

01 Dec '11

From: Alex Jia <ajia(a)redhat.com> Detected by Coverity. Alex Jia (6): src/conf/domain_conf.c: Plug memory leak on virDomainDefParseXML() error path src/qemu/qemu_process.c: Plug memory leak on qemuProcessWaitForMonitor() error path src/rpc/virnetclient.c: Plug memory leak on virNetClientSendInternal() error path src/uml/uml_driver.c: Plug memory leak on umlStartVMDaemon() error path src/util/virnetdevbridge.c: Plug memory leak on virNetDevBridgeGet() sucessful path src/util/virnetdevmacvlan.c: Plug memory leak on virNetDevMacVLanCreateWithVPortProfile() error path Signed-off-by: Alex Jia <ajia(a)redhat.com> --- src/conf/domain_conf.c | 1 + src/qemu/qemu_process.c | 1 + src/rpc/virnetclient.c | 1 + src/uml/uml_driver.c | 1 + src/util/virnetdevbridge.c | 1 + src/util/virnetdevmacvlan.c | 1 + 6 files changed, 6 insertions(+), 0 deletions(-)

4 20

[libvirt] 'make check' hangs
by Wen Congyang 01 Dec '11

01 Dec '11

When I run 'make check', it hangs sometimes. I use gdb to attach lt-virsh, and the following is the backtrace: (gdb) thread 1 [Switching to thread 1 (Thread 0x7fc6c04d5800 (LWP 24099))]#0 0x0000003bdce0804d in pthread_join () from /lib64/libpthread.so.0 (gdb) bt #0 0x0000003bdce0804d in pthread_join () from /lib64/libpthread.so.0 #1 0x000000000041c0b3 in vshDeinit (ctl=0x7fff015f4570) at virsh.c:17262 #2 0x00000000004248d1 in main (argc=<value optimized out>, argv=0x7fff015f4728) at virsh.c:17608 (gdb) thread 2 [Switching to thread 2 (Thread 0x7fc6c04d4700 (LWP 24138))]#0 0x0000003bdc2dc053 in poll () from /lib64/libc.so.6 (gdb) bt #0 0x0000003bdc2dc053 in poll () from /lib64/libc.so.6 #1 0x00007fc6c075359c in virEventPollRunOnce () at util/event_poll.c:619 #2 0x00007fc6c07527d7 in virEventRunDefaultImpl () at util/event.c:247 #3 0x000000000041bea3 in vshEventLoop (opaque=0x7fff015f4570) at virsh.c:16800 #4 0x00007fc6c0764702 in virThreadHelper (data=<value optimized out>) at util/threads-pthread.c:157 #5 0x0000003bdce077f1 in start_thread () from /lib64/libpthread.so.0 #6 0x0000003bdc2e570d in clone () from /lib64/libc.so.6 (gdb)

6 10

[libvirt] [PATCH] virsh: Fix possible deadlock when virsh is about to exit
by Jiri Denemark 01 Dec '11

01 Dec '11

Not only was ctl->quit accessed without a mutex but unfortunately, virEventAddTimeout only interrupts the poll when event loop is running so the hack needs to add a timeout that will make next poll return immediately without blocking. --- tools/virsh.c | 39 +++++++++++++++++++++++++++++++++------ 1 files changed, 33 insertions(+), 6 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 618b0c1..723ec65 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -251,6 +251,7 @@ typedef struct __vshControl { bool useSnapshotOld; /* cannot use virDomainSnapshotGetParent or virDomainSnapshotNumChildren */ virThread eventLoop; + virMutex lock; bool eventLoopStarted; bool quit; } __vshControl; @@ -16796,10 +16797,17 @@ vshEventLoop(void *opaque) { vshControl *ctl = opaque; - while (!ctl->quit) { - if (virEventRunDefaultImpl() < 0) { + while (1) { + bool quit; + virMutexLock(&ctl->lock); + quit = ctl->quit; + virMutexUnlock(&ctl->lock); + + if (quit) + break; + + if (virEventRunDefaultImpl() < 0) virshReportError(ctl); - } } } @@ -17235,13 +17243,18 @@ vshReadline (vshControl *ctl, const char *prompt) #endif /* !USE_READLINE */ +static void +vshDeinitTimer(int timer ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) +{ + /* nothing to be done here */ +} + /* * Deinitialize virsh */ static bool vshDeinit(vshControl *ctl) { - ctl->quit = true; vshReadlineDeinit(ctl); vshCloseLogFile(ctl); VIR_FREE(ctl->name); @@ -17254,15 +17267,24 @@ vshDeinit(vshControl *ctl) virResetLastError(); if (ctl->eventLoopStarted) { + int timer; + + virMutexLock(&ctl->lock); + ctl->quit = true; /* HACK: Add a dummy timeout to break event loop */ - int timer = virEventAddTimeout(-1, NULL, NULL, NULL); + timer = virEventAddTimeout(0, vshDeinitTimer, NULL, NULL); + virMutexUnlock(&ctl->lock); + + virThreadJoin(&ctl->eventLoop); + if (timer != -1) virEventRemoveTimeout(timer); - virThreadJoin(&ctl->eventLoop); ctl->eventLoopStarted = false; } + virMutexDestroy(&ctl->lock); + return true; } @@ -17543,6 +17565,11 @@ main(int argc, char **argv) return EXIT_FAILURE; } + if (virMutexInit(&ctl->lock) < 0) { + vshError(ctl, "%s", _("Failed to initialize mutex")); + return EXIT_FAILURE; + } + if (virInitialize() < 0) { vshError(ctl, "%s", _("Failed to initialize libvirt")); return EXIT_FAILURE; -- 1.7.8.rc4

4 3

[libvirt] [PATCHv2 2/6] qemu: Plug memory leak onqemuProcessWaitForMonitor() error path
by ajia＠redhat.com 30 Nov '11

30 Nov '11

From: Alex Jia <ajia(a)redhat.com> Detected by Coverity. Leak introduced in commit 109efd7. Signed-off-by: Alex Jia <ajia(a)redhat.com> --- src/qemu/qemu_process.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2563f97..b05c280 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1224,7 +1224,7 @@ qemuProcessWaitForMonitor(struct qemud_driver* driver, if (VIR_ALLOC_N(buf, buf_size) < 0) { virReportOOMError(); - return -1; + goto closelog; } if (qemuProcessReadLogOutput(vm, logfd, buf, buf_size, -- 1.7.1

2 1

[libvirt] [PATCHv4 0/4] implement per-device cgroup blkio weight in qemu
by Eric Blake 30 Nov '11

30 Nov '11

v8: two new bug cleanup commits, move major()/minor() handling out of domain_conf.c into cgroup.c, and fix behavior when more than one device is listed, add a test for xml parsing. v7 was here (1-3 are already applied, 4-5 of that series are now 3-4): https://www.redhat.com/archives/libvir-list/2011-November/msg00317.html Eric Blake (2): API: prevent query of --live and --config at once qemu: fix blkiotune --live --config Hu Tao (2): blkiotune: add interface for blkiotune.device_weight blkiotune: add qemu support for blkiotune.device_weight docs/formatdomain.html.in | 29 +++- docs/schemas/domaincommon.rng | 26 ++- include/libvirt/libvirt.h.in | 10 + src/conf/domain_conf.c | 99 +++++++++- src/conf/domain_conf.h | 14 ++ src/libvirt.c | 36 +++- src/libvirt_private.syms | 2 + src/qemu/qemu_cgroup.c | 20 ++ src/qemu/qemu_driver.c | 223 +++++++++++++++++++- src/util/cgroup.c | 51 +++++ src/util/cgroup.h | 4 + .../qemuxml2argv-blkiotune-device.args | 4 + .../qemuxml2argv-blkiotune-device.xml | 36 +++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + tools/virsh.c | 43 +++- tools/virsh.pod | 8 +- 17 files changed, 576 insertions(+), 31 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml -- 1.7.7.1

3 23

[libvirt] [PATCH 0/2] blkiotune tweaks
by Eric Blake 30 Nov '11

30 Nov '11

As mentioned here: https://www.redhat.com/archives/libvir-list/2011-November/msg01720.html Eric Blake (2): conf: reject duplicate paths in device weights qemu: amend existing table of device weights src/conf/domain_conf.c | 10 ++++++ src/qemu/qemu_driver.c | 80 +++++++++++++++++++++++++++++++++++------------ tools/virsh.pod | 4 ++- 3 files changed, 72 insertions(+), 22 deletions(-) -- 1.7.7.3

2 5

[libvirt] [PATCH] Fix bug on setting both weight and device-weights at the same time
by Hu Tao 30 Nov '11

30 Nov '11

When setting both blkio weight and device-weights at the same time, the weight is lost. Fix it. --- tools/virsh.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 01da8e5..af9f538 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4788,14 +4788,14 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) if (!virStrcpy(temp->field, VIR_DOMAIN_BLKIO_WEIGHT, sizeof(temp->field))) goto cleanup; - } - - if (device_weight) { + weight = 0; + } else if (device_weight) { temp->value.s = vshStrdup(ctl, device_weight); temp->type = VIR_TYPED_PARAM_STRING; if (!virStrcpy(temp->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT, sizeof(temp->field))) goto cleanup; + device_weight = NULL; } } -- 1.7.3.1

2 1

[libvirt] [libvirt-glib] Fix potential race in GVirObjectConnection::domain_event_cb
by Christophe Fergeau 30 Nov '11

30 Nov '11

This method starts by looking up a domain in priv->domains, and later, depending on the kind of event that occurred, it may remove this domain from priv->domains. While the individual operations (lookup, removal) are protected by priv->lock, there is no guarantee that the looked up domain and even priv->domains will stay unchanged when priv->lock isn't held. In particular, gvir_connection_close will destroy priv->domains which will unreference all the domains it contains (potentially destroying them too), and gvir_connection_fetch_domains will change priv->domains value. To avoid these issues, this commit takes a reference on priv->domains so that it doesn't get away behind our back, and it takes a reference on the looked up domain too to ensure it stays alive for the duration of domain_event_cb run. --- libvirt-gobject/libvirt-gobject-connection.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 59b828d..786a026 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -257,6 +257,7 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, void *opaque) { gchar uuid[VIR_UUID_STRING_BUFLEN]; + GHashTable *doms; GVirConnection *gconn = opaque; GVirDomain *gdom; GVirConnectionPrivate *priv = gconn->priv; @@ -269,14 +270,18 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, g_debug("%s: %s event:%d, detail:%d", G_STRFUNC, uuid, event, detail); g_mutex_lock(priv->lock); - gdom = g_hash_table_lookup(priv->domains, uuid); + doms = g_hash_table_ref(priv->domains); + gdom = g_hash_table_lookup(doms, uuid); + if (gdom != NULL) + g_object_ref(G_OBJECT(gdom)); g_mutex_unlock(priv->lock); if (gdom == NULL) { gdom = GVIR_DOMAIN(g_object_new(GVIR_TYPE_DOMAIN, "handle", dom, NULL)); g_mutex_lock(priv->lock); - g_hash_table_insert(priv->domains, (gpointer)gvir_domain_get_uuid(gdom), gdom); + g_hash_table_insert(doms, (gpointer)gvir_domain_get_uuid(gdom), + g_object_ref(G_OBJECT(gdom))); g_mutex_unlock(priv->lock); } @@ -293,11 +298,10 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, case VIR_DOMAIN_EVENT_UNDEFINED: if (detail == VIR_DOMAIN_EVENT_UNDEFINED_REMOVED) { g_mutex_lock(priv->lock); - g_hash_table_steal(priv->domains, uuid); + g_hash_table_remove(doms, uuid); g_mutex_unlock(priv->lock); g_signal_emit(gconn, signals[VIR_DOMAIN_REMOVED], 0, gdom); - g_object_unref(gdom); } else g_warn_if_reached(); break; @@ -365,11 +369,10 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, if (virDomainIsPersistent(dom) != 1) { g_mutex_lock(priv->lock); - g_hash_table_steal(priv->domains, uuid); + g_hash_table_remove(doms, uuid); g_mutex_unlock(priv->lock); g_signal_emit(gconn, signals[VIR_DOMAIN_REMOVED], 0, gdom); - g_object_unref(gdom); } break; @@ -377,6 +380,8 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, g_warn_if_reached(); } + g_object_unref(G_OBJECT(gdom)); + g_hash_table_unref(doms); return 0; } -- 1.7.7.3

2 1

[libvirt] [libvirt-glib] Fix gvir_config_domain_clock_set_variable_offset
by Christophe Fergeau 30 Nov '11

30 Nov '11

It was creating an extra <clock> node as a child of the "main" one which is not what was intended --- libvirt-gconfig/libvirt-gconfig-domain-clock.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-clock.c b/libvirt-gconfig/libvirt-gconfig-domain-clock.c index 27d1e67..b28856c 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-clock.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-clock.c @@ -121,7 +121,7 @@ void gvir_config_domain_clock_set_variable_offset(GVirConfigDomainClock *klock, g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_CLOCK(klock)); - node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(klock), "clock"); + node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(klock)); if (node == NULL) return; -- 1.7.7.3

2 1

[libvirt] [libvirt-glib 1/2] Rely on libvirt API to detect transient domains
by Zeeshan Ali (Khattak) 30 Nov '11

30 Nov '11

From: "Zeeshan Ali (Khattak)" <zeeshanak(a)gnome.org> --- libvirt-gobject/libvirt-gobject-connection.c | 6 +----- 1 files changed, 1 insertions(+), 5 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index c9985b2..b647bfa 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -260,7 +260,6 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, GVirConnection *gconn = opaque; GVirDomain *gdom; GVirConnectionPrivate *priv = gconn->priv; - gboolean was_unknown = FALSE; if (virDomainGetUUIDString(dom, uuid) < 0) { g_warning("Failed to get domain UUID on %p", dom); @@ -279,8 +278,6 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, g_mutex_lock(priv->lock); g_hash_table_insert(priv->domains, (gpointer)gvir_domain_get_uuid(gdom), gdom); g_mutex_unlock(priv->lock); - - was_unknown = TRUE; } switch (event) { @@ -307,8 +304,7 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, case VIR_DOMAIN_EVENT_STARTED: if (detail == VIR_DOMAIN_EVENT_STARTED_BOOTED) { - if (was_unknown) - /* Most probably a transient domain */ + if (!virDomainIsPersistent(dom)) g_signal_emit(gconn, signals[VIR_DOMAIN_ADDED], 0, gdom); g_signal_emit_by_name(gdom, "started::booted"); } else if (detail == VIR_DOMAIN_EVENT_STARTED_MIGRATED) -- 1.7.7.3

3 5