- Devel - libvirt List Archives

[libvirt] libvirt tracking of hung/stopped QEMU VMs
by Ryan Harper 04 May '11

04 May '11

I've encountered an interesting scenario: 1. define a guest via virsh define <xml> 2 start this guest via virsh 3. one of the disk elements is a multipath device that is currently misconfigured such that any io to the device hangs the calling process 4. libvirt times out when attemping to communicate via the monitor to the guest (btw, this timeout isn't configurable AFAICT) 5. returns an error from create indicating that we failed to create the VM At this point: 1) libvirt reports that the VM is stopped (and this is true, the qemu process has never been issued the 'cont' command and thus won't ever execute gues tcode) 2) the qemu process for this VM is still running (just blocked on IO) 3) it is possible that if the process becomes unblocked that the QEMU process will be functional again, but won't be started, and the process won't be terminated since libvirt isn't tracking this any more, and is consuming some amount of resources that are allocated on start up. How can we clean up from this failure scenario? Would it make sense for libvirt to send a SIGTERM to a qemu if it failed to create? In the above scenario, this would allow us to reap the process if it ever became unblocked. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ryanh(a)us.ibm.com

1 1

Re: [libvirt] Problem with libvirt on Fedora 14 with virt-preview repo
by Laine Stump 03 May '11

03 May '11

On 05/03/2011 04:16 PM, Scott Dowdle wrote: > Greetings, > > I wasn't sure who to report this to... and I did a bugzilla search and didn't find anything... and I'm not sure where to report this to... since it isn't Fedora 14 exactly since I'm using the virt-preview repo. I *think* the best place to report these bugs is at bugzilla.redhat.com, Classification "Other", Product "Virtualization Tools", component "libvirt". You could first try sending a message to libvirt-users(a)redhat.com or libvir-list(a)redhat.com (the developers' list) to see if it's a known problem though. > I just did a bunch of updates on a Fedora 14 x86_64 box that has the virt-preview repo. I was using all of the new features with SPICE... but after updating I can't get libvirtd to start anymore. I get the following error message in /var/log/messages: > > May 3 14:20:04 virt-desk libvirtd: Could not find keytab file: /etc/libvirt/krb5.tab: No such file or directory > May 3 14:20:04 virt-desk libvirtd: 14:20:04.854: 2746: info : libvirt version: 0.8.8, package: 4.fc14 (Unknown, 2011-04-07-15:51:18, fedora64.linuxtx.org) > May 3 14:20:04 virt-desk libvirtd: 14:20:04.854: 2746: error : remoteCheckCertFile:288 : Cannot access CA certificate '/etc/pki/CA/cacert.pem': No such file or directory > > I look at the config files in /etc/libvirt/ and everything is commented out... so I'm not sure how to fix this via a config. I'm not familiar with that part of the code, but it looks like the cert check happens if listen_tls = 1, which is its default value. You can disable it by uncommenting listen_tls=0 in /etc/libvirt/libvirtd.conf. I'm not sure what undesired effects that will have on your system's operation. > Any help? Can anyone else reproduce this? As a result, I can't get libvirtd to start and my VMs are hosed. :( Shall I report the bug? I'm Cc'ing this response to libvir-list. It's very likely a known problem that I just didn't notice, and someone there will know about it. > Scott Dowdle > [address redacted :-)] > Belgrade, MT 59714 Seriously? I was at MSU from 1981-1985. I'm guessing a lot has changed since then...

2 1

[libvirt] [PATCH] qemu: silence clang false positives
by Eric Blake 03 May '11

03 May '11

Clang 2.8 wasn't quite able to follow that persistentDef was assigned earlier if (flags & VIR_DOMAIN_MEM_CONFIG) is true. Silence this false positive, to make clang analysis easier to use. * src/qemu/qemu_driver.c (qemudDomainSetMemoryFlags): Add an annotation to silence clang's claim of a NULL dereference. --- I'm pushing this under the trivial rule - sa_assert is a no-op in normal compilation, so this can't have any effect on the end executable, but makes the rest of my clang analysis easier. src/qemu/qemu_driver.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0919503..7132837 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1640,6 +1640,8 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, } if (flags & VIR_DOMAIN_MEM_CONFIG) { + /* Help clang 2.8 decipher the logic flow. */ + sa_assert(persistentDef); persistentDef->mem.max_balloon = newmem; if (persistentDef->mem.cur_balloon > newmem) persistentDef->mem.cur_balloon = newmem; @@ -1675,6 +1677,7 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, } if (flags & VIR_DOMAIN_MEM_CONFIG) { + sa_assert(persistentDef); persistentDef->mem.cur_balloon = newmem; ret = virDomainSaveConfig(driver->configDir, persistentDef); goto endjob; -- 1.7.4.4

1 0

[libvirt] [PATCH] Add support for YAJL version 2 API/ABI
by Daniel P. Berrange 03 May '11

03 May '11

Version 2.0.0 or yajl changed API. It is fairly trivial for us to cope with both APIs in libvirt, so adapt. * configure.ac: Probe for yajl2 API * src/util/json.c: Conditional support for yajl2 API --- configure.ac | 8 ++++++++ src/util/json.c | 44 +++++++++++++++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index 7c68bca..dc87347 100644 --- a/configure.ac +++ b/configure.ac @@ -878,6 +878,7 @@ AC_ARG_WITH([yajl], YAJL_CFLAGS= YAJL_LIBS= +with_yajl2=no if test "x$with_yajl" != "xno"; then if test "x$with_yajl" != "xyes" && test "x$with_yajl" != "xcheck"; then YAJL_CFLAGS="-I$with_yajl/include" @@ -898,6 +899,9 @@ if test "x$with_yajl" != "xno"; then AC_CHECK_LIB([yajl], [yajl_parse],[ YAJL_LIBS="$YAJL_LIBS -lyajl" with_yajl=yes + AC_CHECK_LIB([yajl], [yajl_tree_parse],[ + with_yajl2=yes + ],[]) ],[ if test "x$with_yajl" = "xcheck" ; then with_yajl=no @@ -914,6 +918,10 @@ if test "x$with_yajl" != "xno"; then AC_DEFINE_UNQUOTED([HAVE_YAJL], 1, [whether YAJL is available for JSON parsing/formatting]) fi + if test "x$with_yajl2" = "xyes" ; then + AC_DEFINE_UNQUOTED([HAVE_YAJL2], 1, + [whether YAJL has API version 2]) + fi fi AM_CONDITIONAL([HAVE_YAJL], [test "x$with_yajl" = "xyes"]) AC_SUBST([YAJL_CFLAGS]) diff --git a/src/util/json.c b/src/util/json.c index 0daeae9..80dbca4 100644 --- a/src/util/json.c +++ b/src/util/json.c @@ -709,7 +709,12 @@ static int virJSONParserHandleBoolean(void * ctx, int boolean_) static int virJSONParserHandleNumber(void * ctx, const char * s, - unsigned int l) +#ifdef HAVE_YAJL2 + size_t l +#else + unsigned int l +#endif + ) { virJSONParserPtr parser = ctx; char *str = strndup(s, l); @@ -735,7 +740,12 @@ static int virJSONParserHandleNumber(void * ctx, static int virJSONParserHandleString(void * ctx, const unsigned char * stringVal, - unsigned int stringLen) +#ifdef HAVE_YAJL2 + size_t stringLen +#else + unsigned int stringLen +#endif + ) { virJSONParserPtr parser = ctx; virJSONValuePtr value = virJSONValueNewStringLen((const char *)stringVal, @@ -756,7 +766,12 @@ static int virJSONParserHandleString(void * ctx, static int virJSONParserHandleMapKey(void * ctx, const unsigned char * stringVal, - unsigned int stringLen) +#ifdef HAVE_YAJL2 + size_t stringLen +#else + unsigned int stringLen +#endif + ) { virJSONParserPtr parser = ctx; virJSONParserStatePtr state; @@ -894,14 +909,22 @@ static const yajl_callbacks parserCallbacks = { /* XXX add an incremental streaming parser - yajl trivially supports it */ virJSONValuePtr virJSONValueFromString(const char *jsonstring) { - yajl_parser_config cfg = { 1, 1 }; yajl_handle hand; virJSONParser parser = { NULL, NULL, 0 }; virJSONValuePtr ret = NULL; +#ifndef HAVE_YAJL2 + yajl_parser_config cfg = { 1, 1 }; +#endif VIR_DEBUG("string=%s", jsonstring); +#ifdef HAVE_YAJL2 + hand = yajl_alloc(&parserCallbacks, NULL, &parser); + yajl_config(hand, yajl_allow_comments, 1); + yajl_config(hand, yajl_dont_validate_strings, 0); +#else hand = yajl_alloc(&parserCallbacks, &cfg, NULL, &parser); +#endif if (yajl_parse(hand, (const unsigned char *)jsonstring, @@ -1002,15 +1025,26 @@ static int virJSONValueToStringOne(virJSONValuePtr object, char *virJSONValueToString(virJSONValuePtr object) { - yajl_gen_config conf = { 0, " " }; /* Turns off pretty printing since QEMU can't cope */ yajl_gen g; const unsigned char *str; char *ret = NULL; +#ifdef HAVE_YAJL2 + size_t len; +#else unsigned int len; + yajl_gen_config conf = { 0, " " }; /* Turns off pretty printing since QEMU can't cope */ +#endif VIR_DEBUG("object=%p", object); +#ifdef HAVE_YAJL2 + g = yajl_gen_alloc(NULL); + yajl_gen_config(g, yajl_gen_beautify, 0); + yajl_gen_config(g, yajl_gen_indent_string, " "); + yajl_gen_config(g, yajl_gen_validate_utf8, 1); +#else g = yajl_gen_alloc(&conf, NULL); +#endif if (virJSONValueToStringOne(object, g) < 0) { virReportOOMError(); -- 1.7.4.4

2 1

[libvirt] [PATCH] virsh: avoid null pointer dereference
by Eric Blake 03 May '11

03 May '11

Clang detected that vol-download will call unlink(NULL) if there is a parse error during option parsing. Also, mingw doesn't like unlinking an open file. * tools/virsh.c (cmdVolDownload): Only unlink file if created. --- tools/virsh.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3d4ed2f..5d8b025 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -7259,10 +7259,10 @@ cmdVolDownload (vshControl *ctl, const vshCmd *cmd) virStreamPtr st = NULL; const char *name = NULL; unsigned long long offset = 0, length = 0; - bool created = true; + bool created = false; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) { vshError(ctl, _("Unable to parse integer")); @@ -7283,12 +7283,13 @@ cmdVolDownload (vshControl *ctl, const vshCmd *cmd) } if ((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0666)) < 0) { - created = false; if (errno != EEXIST || (fd = open(file, O_WRONLY|O_TRUNC, 0666)) < 0) { vshError(ctl, _("cannot create %s"), file); goto cleanup; } + } else { + created = true; } st = virStreamNew(ctl->conn, 0); @@ -7316,13 +7317,13 @@ cmdVolDownload (vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: + VIR_FORCE_CLOSE(fd); if (ret == false && created) unlink(file); if (vol) virStorageVolFree(vol); if (st) virStreamFree(st); - VIR_FORCE_CLOSE(fd); return ret; } -- 1.7.4.4

2 2

[libvirt] [PATCH] pci: fix null pointer dereference
by Eric Blake 03 May '11

03 May '11

Clang detected a null-pointer dereference regression, introduced in commit 4e8969eb. Without this patch, a device with unbind_from_stub set to false would eventually try to call virFileExists on uncomputed drvdir. * src/util/pci.c (pciUnbindDeviceFromStub): Ensure drvdir is set before use. --- src/util/pci.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index 945f32a..d7f74f9 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -879,15 +879,16 @@ pciUnbindDeviceFromStub(pciDevice *dev, const char *driver) char *drvdir = NULL; char *path = NULL; + if (pciDriverDir(&drvdir, driver) < 0) + goto cleanup; + if (!dev->unbind_from_stub) goto remove_slot; /* If the device is bound to stub, unbind it. */ - if (pciDriverDir(&drvdir, driver) < 0 || - pciDeviceFile(&path, dev->name, "driver") < 0) { + if (pciDeviceFile(&path, dev->name, "driver") < 0) goto cleanup; - } if (virFileExists(drvdir) && virFileLinkPointsTo(path, drvdir)) { if (pciDriverFile(&path, driver, "unbind") < 0) { -- 1.7.4.4

2 2

[libvirt] [PATCH] qemu: avoid null pointer dereference
by Eric Blake 03 May '11

03 May '11

This code has had problems historically. As originally written, in commit 6bcf2501 (Jun 08), it could call unlink on a random string, nuking an unrelated file. Then commit 182a80b9 (Sep 09), the code was rewritten to allocate tmp, with both a use-after-free bug and a chance to call unlink(NULL). Commit e206946 (Mar 11) fixed the use-after-free, but not the NULL dereference. Thanks to clang for catching this! * src/qemu/qemu_driver.c (qemudDomainMemoryPeek): Don't call unlink on NULL. --- src/qemu/qemu_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 16d869d..3ee4720 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5355,7 +5355,8 @@ endjob: cleanup: VIR_FORCE_CLOSE(fd); - unlink (tmp); + if (tmp) + unlink (tmp); VIR_FREE(tmp); if (vm) virDomainObjUnlock(vm); -- 1.7.4.4

2 2

[libvirt] [PATCH] tests: avoid null pointer dereference
by Eric Blake 03 May '11

03 May '11

Unlikely to hit in real life, but clang noticed it. * tests/commandtest.c (test4): Avoid unlink(NULL) on OOM. --- Much like a similar patch I sent for qemu_driver, but this time the unlink(NULL) is much harder to trigger. tests/commandtest.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tests/commandtest.c b/tests/commandtest.c index fa0061c..e8d8858 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -240,7 +240,8 @@ static int test4(const void *unused ATTRIBUTE_UNUSED) cleanup: virCommandFree(cmd); - unlink(pidfile); + if (pidfile) + unlink(pidfile); VIR_FREE(pidfile); return ret; } -- 1.7.4.4

2 2

[libvirt] [PATCH] Fix security driver handling of FIFOs with QEMU
by Daniel P. Berrange 03 May '11

03 May '11

When setting up a FIFO for QEMU, it allows either a pair of fifos used unidirectionally, or a single fifo used bidirectionally. Look for the bidirectional fifo first when labelling since that is more useful * src/security/security_dac.c, src/security/security_selinux.c: Fix fifo handling --- src/security/security_dac.c | 19 ++++++++++++------- src/security/security_selinux.c | 19 ++++++++++++------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index fba2d1d..b8642d2 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -406,14 +406,19 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, break; case VIR_DOMAIN_CHR_TYPE_PIPE: - if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || - (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { - virReportOOMError(); - goto done; + if (virFileExists(dev->data.file.path)) { + if (virSecurityDACSetOwnership(dev->data.file.path, priv->user, priv->group) < 0) + goto done; + } else { + if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || + (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { + virReportOOMError(); + goto done; + } + if ((virSecurityDACSetOwnership(in, priv->user, priv->group) < 0) || + (virSecurityDACSetOwnership(out, priv->user, priv->group) < 0)) + goto done; } - if ((virSecurityDACSetOwnership(in, priv->user, priv->group) < 0) || - (virSecurityDACSetOwnership(out, priv->user, priv->group) < 0)) - goto done; ret = 0; break; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 216208e..476bbbf 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -733,14 +733,19 @@ SELinuxSetSecurityChardevLabel(virDomainObjPtr vm, break; case VIR_DOMAIN_CHR_TYPE_PIPE: - if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || - (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { - virReportOOMError(); - goto done; + if (virFileExists(dev->data.file.path)) { + if (SELinuxSetFilecon(dev->data.file.path, secdef->imagelabel) < 0) + goto done; + } else { + if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || + (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { + virReportOOMError(); + goto done; + } + if ((SELinuxSetFilecon(in, secdef->imagelabel) < 0) || + (SELinuxSetFilecon(out, secdef->imagelabel) < 0)) + goto done; } - if ((SELinuxSetFilecon(in, secdef->imagelabel) < 0) || - (SELinuxSetFilecon(out, secdef->imagelabel) < 0)) - goto done; ret = 0; break; -- 1.7.4.4

2 1

[libvirt] [PATCH] tests: Update valgrind suppressions file
by Matthias Bolte 03 May '11

03 May '11

--- tests/.valgrind.supp | 58 +++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 51 insertions(+), 7 deletions(-) diff --git a/tests/.valgrind.supp b/tests/.valgrind.supp index 4af10b1..68cfa0c 100644 --- a/tests/.valgrind.supp +++ b/tests/.valgrind.supp @@ -258,14 +258,58 @@ Memcheck:Param capget(data) fun:capget + fun:* fun:capng_clear + fun:virClearCapabilities fun:__virExec fun:virExecWithHook - fun:virExec - fun:qemudProbeMachineTypes - fun:qemudCapsInitGuest - fun:qemudCapsInit - fun:qemudStartup - fun:virStateInitialize - fun:main +} +{ + libnlMemoryLeak1 + Memcheck:Leak + fun:malloc + fun:strdup + obj:/usr/lib/libnl.so.1.1 +} +{ + libnlMemoryLeak2 + Memcheck:Leak + fun:calloc + obj:/usr/lib/libnl.so.1.1 +} +{ + libselinuxMemoryLeak1 + Memcheck:Leak + fun:malloc + fun:getdelim + obj:/lib/libselinux.so.1 +} +{ + dashMemoryLeak1 + Memcheck:Leak + fun:malloc + obj:/bin/dash +} +{ + dashMemoryLeak2 + Memcheck:Leak + fun:malloc + fun:strdup + obj:/bin/dash +} +{ + vboxMemoryLeak1 + Memcheck:Leak + ... + fun:VBoxNsxpNS_InitXPCOM2 +} +{ + libnetcfMemoryLeak1 + fun:malloc + fun:xmlStrndup + fun:xmlHashUpdateEntry3 + fun:* + fun:xsltRegisterAllExtras + fun:drv_init + fun:interfaceOpenInterface } -- 1.7.0.4

3 5