- Devel - libvirt List Archives

[libvirt] [PATCH] maint: detect clang 2.9
by Eric Blake 05 May '11

05 May '11

In Fedora 15, with clang 2.8, 'scan-build env' shows: CCC_ANALYZER_ANALYSIS=-analyzer-check-objc-mem -analyzer-check-security-syntactic -analyzer-check-dead-stores -analyzer-check-objc-unused-ivars -analyzer-check-objc-methodsigs But in rawhide, with clang 2.9, the same variable is set but empty, implying the default set of analysis. We still want sa_assert defined in that case, to stop clang from hitting false positives. * configure.ac (STATIC_ANALYSIS): Detect clang even when the set of analyses is the default. --- I'm pushing this under the trivial rule. It has no effect on a real build, but makes it easier for me to repeat my clang testing on rawhide. configure.ac | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index 7c68bca..314a594 100644 --- a/configure.ac +++ b/configure.ac @@ -2297,7 +2297,7 @@ cp -f COPYING.LIB COPYING # Detect when running under the clang static analyzer's scan-build driver # or Coverity-prevent's cov-build. Define STATIC_ANALYSIS accordingly. -test -n "$CCC_ANALYZER_ANALYSIS$COVERITY_BUILD_COMMAND" && t=1 || t=0 +test -n "${CCC_ANALYZER_ANALYSIS+set}$COVERITY_BUILD_COMMAND" && t=1 || t=0 AC_DEFINE_UNQUOTED([STATIC_ANALYSIS], [$t], [Define to 1 when performing static analysis.]) -- 1.7.4.4

1 0

[libvirt] [PATCH] storage: avoid null deref and leak on failure
by Eric Blake 04 May '11

04 May '11

Detected by clang. NULL deref added in commit 343a27a (Mar 11), but leak of voldef present since commit 2cd9b2d (Apr 09). * src/storage/storage_driver.c (storageVolumeCreateXML): Don't leak voldef or dereference null volobj. --- src/storage/storage_driver.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 1ea5d12..19c7d63 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1319,8 +1319,10 @@ storageVolumeCreateXML(virStoragePoolPtr obj, pool->volumes.objs[pool->volumes.count++] = voldef; volobj = virGetStorageVol(obj->conn, pool->def->name, voldef->name, voldef->key); + if (!volobj) + goto cleanup; - if (volobj && backend->buildVol) { + if (backend->buildVol) { int buildret; virStorageVolDefPtr buildvoldef = NULL; -- 1.7.4.4

2 5

[libvirt] qemu qxl video memory parameter - windows bsod with spice
by Emre Erenoglu 04 May '11

04 May '11

Hi, Please see the below discussion with the spice community. libvirt is adding a parameter about video ram to qemu process and this parameter might be responsible of a crash in the qxl at higher resolutions. Spice people tell me that their default ram should be 64 Mbytes. I'm using libvirt 0.9.0 and qemu 0.14, virt-manager 0.8.7. When configuring a guest in virt-manager, cirrus driver is added as default, then you can change it to qxl, but the 9 MB videoram parameter can't be adjusted and it does not automatically change to 64 MB. If you add a 2nd qxl video card, then it has 64 MB memory. The parameter that qemu gets with one qxl device is: -vga qxl -global qxl-vga.vram_size=9437184 The parameter that qemu gets with a second qxl device is: -vga qxl -global qxl-vga.vram_size=9437184 -device qxl,id=video1,vram_size=67108864,bus=pci.0,addr=0x8 Is this a bug in libvirt or other related tools? Thanks, Br, Emre ---------- Forwarded message ---------- From: Emre Erenoglu <erenoglu(a)gmail.com> Date: Wed, May 4, 2011 at 7:42 PM Subject: Re: [Spice-devel] windows bsod with spice 0.8.1 To: Emre Erenoglu <erenoglu(a)gmail.com>, spice-devel(a)freedesktop.org On Wed, May 4, 2011 at 6:00 PM, Alon Levy <alevy(a)redhat.com> wrote: > On Wed, May 04, 2011 at 05:10:31PM +0400, Emre Erenoglu wrote: > > On Wed, May 4, 2011 at 2:12 PM, Alon Levy <alevy(a)redhat.com> wrote: > > > > > On Wed, May 04, 2011 at 01:45:39PM +0400, Emre Erenoglu wrote: > > > > On Wed, Apr 27, 2011 at 1:16 AM, Emre Erenoglu <erenoglu(a)gmail.com> > > > wrote: > > > > > > > > > Hi, > > > > > > > > > > I started to see frequent crashes with my guests running windows xp > and > > > > > windows server 2003 x86. The bsod appears on qxldd.dll. I can give > > > further > > > > > info if needed. > > > > > > > > > > The host is Linux x86_64, qemu 0.14, spice 0.8.1. Guest has qxl > 0.6.1 > > > > > drivers loaded. I may revert back to 0.8.0 since it was not > crashing. > > > > > Nothing changed in the guests when I upgraded spice from 0.8.0 to > > > 0.8.1. > > > > > > > > > > > > > > Some updates on the original issue. I checked a lot and I found out > that > > > the > > > > issue continued with any version of spice and guest qxl drivers. > > > > > > > > I finally digged down to the point that, the qemu comment line which > > > states > > > > " -global qxl-vga.vram_size=9xxxxxx " parameter is the problem. > When > > > the > > > > guest is started with 9 MB of video memory given as comment line > > > parameter > > > > to qemu, the guest qxldd driver crashes with BSOD. > > > > > > Why was it started with 9MB? anyway, good catch, we should add a check > for > > > this > > > in the driver (and probably just refuse to load). Could you open a bug > on > > > this > > > in bugzilla.freedesktop.org? > > > > > > > This is added by libvirt (virt-manager) when launching qemu. Do we need > to > > file it against libvirt or against spice? I will post this to the > libvirt > > list also. > > > > Btw, rather than refusing to load, I think we should just limit the > possible > > resolutions. I'm not sure if the resolutions are the issue though. The > crash > > was not happening immediately and it was working OK with that 9 MB > memory. > > When I launched outlook, or IE8, or just in the middle of an application, > it > > could crash and bsod. > > But that number doesn't make any sense - the spice default is 64MB, we > never tested > with less then that. It is probably fixable like you say, but until that's > fixed why > not stick to the spice default? Well, this parameter is not configurable from the virt-manager UI and it configures 9 MB to the command line of qemu by libvirt (even latest 0.9.0) version. When you add a 2nd VGA adapter, it adds 64 MB to that adapter correctly (-device qxl,id=video1,vram_size=67108864,bus=pci.0,addr=0x8) I guess i need to file a bug report to libvirt too. Emre

4 5

[libvirt] [PATCH] esx: avoid null dereference on error
by Eric Blake 04 May '11

04 May '11

Detected by clang. * src/esx/esx_driver.c (esxDomainGetInfo): Fail early on error. --- src/esx/esx_driver.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 1f8f90b..e929208 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2372,8 +2372,9 @@ esxDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) if (perfEntityMetric == NULL) { VIR_ERROR(_("QueryPerf returned object with unexpected type '%s'"), esxVI_Type_ToString(perfEntityMetricBase->_type)); + goto cleanup; } perfMetricIntSeries = esxVI_PerfMetricIntSeries_DynamicCast(perfEntityMetric->value); -- 1.7.4.4

4 4

[libvirt] [PATCH] remote: avoid null dereference on error
by Eric Blake 04 May '11

04 May '11

Clang found three instances of uninitialized use of nparams in the cleanup path. Unfortunately, one is a false positive: clang couldn't see that ret->params.params_val is guaranteed to be NULL unless allocated within a function, and that nparams is guaranteed to be assigned prior to the allocation; hoisting the assignment to nparams to be earlier in the function shuts up that false positive. But two of the reports also happened to highlight a real bug - the error path can dereference NULL. * daemon/remote.c (remoteDispatchDomainGetMemoryParameters) (remoteDispatchDomainGetBlkioParameters): Don't clear fields if array was not allocated. (remoteDispatchDomainGetSchedulerParameters): Initialize nparams earlier. --- daemon/remote.c | 29 ++++++++++++++++------------- 1 files changed, 16 insertions(+), 13 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index eedbc77..214f7a4 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -886,7 +886,8 @@ remoteDispatchDomainGetSchedulerParameters(struct qemud_server *server ATTRIBUTE { virDomainPtr dom = NULL; virSchedParameterPtr params = NULL; - int i, nparams; + int i; + int nparams = args->nparams; int rv = -1; if (!conn) { @@ -894,8 +895,6 @@ remoteDispatchDomainGetSchedulerParameters(struct qemud_server *server ATTRIBUTE goto cleanup; } - nparams = args->nparams; - if (nparams > REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX) { virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); goto cleanup; @@ -2970,7 +2969,8 @@ remoteDispatchDomainGetMemoryParameters(struct qemud_server *server { virDomainPtr dom = NULL; virMemoryParameterPtr params = NULL; - int i, nparams; + int i; + int nparams = args->nparams; unsigned int flags; int rv = -1; @@ -2979,7 +2979,6 @@ remoteDispatchDomainGetMemoryParameters(struct qemud_server *server goto cleanup; } - nparams = args->nparams; flags = args->flags; if (nparams > REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) { @@ -3060,9 +3059,11 @@ success: cleanup: if (rv < 0) { remoteDispatchError(rerr); - for (i = 0; i < nparams; i++) - VIR_FREE(ret->params.params_val[i].field); - VIR_FREE(ret->params.params_val); + if (ret->params.params_val) { + for (i = 0; i < nparams; i++) + VIR_FREE(ret->params.params_val[i].field); + VIR_FREE(ret->params.params_val); + } } if (dom) virDomainFree(dom); @@ -3186,7 +3187,8 @@ remoteDispatchDomainGetBlkioParameters(struct qemud_server *server { virDomainPtr dom = NULL; virBlkioParameterPtr params = NULL; - int i, nparams; + int i; + int nparams = args->nparams; unsigned int flags; int rv = -1; @@ -3195,7 +3197,6 @@ remoteDispatchDomainGetBlkioParameters(struct qemud_server *server goto cleanup; } - nparams = args->nparams; flags = args->flags; if (nparams > REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX) { @@ -3276,9 +3277,11 @@ success: cleanup: if (rv < 0) { remoteDispatchError(rerr); - for (i = 0; i < nparams; i++) - VIR_FREE(ret->params.params_val[i].field); - VIR_FREE(ret->params.params_val); + if (ret->params.params_val) { + for (i = 0; i < nparams; i++) + VIR_FREE(ret->params.params_val[i].field); + VIR_FREE(ret->params.params_val); + } } VIR_FREE(params); if (dom) -- 1.7.4.4

2 3

[libvirt] [PATCH] esx: remove dead store
by Eric Blake 04 May '11

04 May '11

Detected by clang. * src/esx/esx_util.c (esxUtil_ParseDatastorePath): No need to increment. --- src/esx/esx_util.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index 9ef947c..640e984 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -2,7 +2,7 @@ /* * esx_util.c: utility functions for the VMware ESX driver * - * Copyright (C) 2010 Red Hat, Inc. + * Copyright (C) 2010-2011 Red Hat, Inc. * Copyright (C) 2009 Matthias Bolte <matthias.bolte(a)googlemail.com> * Copyright (C) 2009 Maximilian Wilhelm <max(a)rfc2324.org> * @@ -332,7 +332,7 @@ esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName, preliminaryFileName = strrchr(preliminaryDirectoryAndFileName, '/'); if (preliminaryFileName != NULL) { - *preliminaryFileName++ = '\0'; + *preliminaryFileName = '\0'; } if (esxVI_String_DeepCopyValue(directoryName, -- 1.7.4.4

3 4

[libvirt] [PATCH] cgroup: avoid leaking a file
by Eric Blake 04 May '11

04 May '11

Clang detected a dead store to rc. It turns out that in fixing this, I also found a FILE* leak. * src/util/cgroup.c (virCgroupKillInternal): Abort rather than resuming loop on fscanf failure, and cleanup file on error. --- As a bonus, this also fixes a decl-after-statement for fp. src/util/cgroup.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/util/cgroup.c b/src/util/cgroup.c index afe8731..62b371d 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -1351,7 +1351,9 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr int killedAny = 0; char *keypath = NULL; bool done = false; - VIR_DEBUG("group=%p path=%s signum=%d pids=%p", group, group->path, signum, pids); + FILE *fp = NULL; + VIR_DEBUG("group=%p path=%s signum=%d pids=%p", + group, group->path, signum, pids); rc = virCgroupPathOfController(group, -1, "tasks", &keypath); if (rc != 0) { @@ -1364,7 +1366,6 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr */ while (!done) { done = true; - FILE *fp; if (!(fp = fopen(keypath, "r"))) { rc = -errno; VIR_DEBUG("Failed to read %s: %m\n", keypath); @@ -1376,7 +1377,8 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr if (feof(fp)) break; rc = -errno; - break; + VIR_DEBUG("Failed to read %s: %m\n", keypath); + goto cleanup; } if (virHashLookup(pids, (void*)pid)) continue; @@ -1403,6 +1405,7 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr cleanup: VIR_FREE(keypath); + VIR_FORCE_FCLOSE(fp); return rc; } -- 1.7.4.4

3 5

[libvirt] [PATCH] qemu: update qemuCgroupControllerActive signature
by Eric Blake 04 May '11

04 May '11

Clang warned about a dead assignment. In the process, I noticed that we are only using the function for a bool value. I audited all other callers in qemu_{migration,cgroup,driver,hotplug), and all were making the call in a bool context. * src/qemu/qemu_cgroup.c (qemuSetupCgroup): Delete dead assignment. (qemuCgroupControllerActive): Change return type to bool. * src/qemu/qemu_cgroup.h (qemuCgroupControllerActive): Likewise. --- src/qemu/qemu_cgroup.c | 14 +++++++------- src/qemu/qemu_cgroup.h | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 7e88a67..ac1c016 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -43,16 +43,16 @@ static const char *const defaultDeviceACL[] = { #define DEVICE_PTY_MAJOR 136 #define DEVICE_SND_MAJOR 116 -int qemuCgroupControllerActive(struct qemud_driver *driver, - int controller) +bool qemuCgroupControllerActive(struct qemud_driver *driver, + int controller) { if (driver->cgroup == NULL) - return 0; + return false; if (!virCgroupMounted(driver->cgroup, controller)) - return 0; + return false; if (driver->cgroupControllers & (1 << controller)) - return 1; - return 0; + return true; + return false; } static int @@ -312,7 +312,7 @@ int qemuSetupCgroup(struct qemud_driver *driver, if (vm->def->mem.hard_limit != 0 || vm->def->mem.soft_limit != 0 || vm->def->mem.swap_hard_limit != 0) { - if ((rc = qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY))) { + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { if (vm->def->mem.hard_limit != 0) { rc = virCgroupSetMemoryHardLimit(cgroup, vm->def->mem.hard_limit); if (rc != 0) { diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 299bd2d..e8abfb4 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -34,8 +34,8 @@ struct _qemuCgroupData { }; typedef struct _qemuCgroupData qemuCgroupData; -int qemuCgroupControllerActive(struct qemud_driver *driver, - int controller); +bool qemuCgroupControllerActive(struct qemud_driver *driver, + int controller); int qemuSetupDiskCgroup(struct qemud_driver *driver, virDomainObjPtr vm, virCgroupPtr cgroup, -- 1.7.4.4

2 2

[libvirt] [PATCH] lxc: report correct error
by Eric Blake 04 May '11

04 May '11

Clang noticed a dead assignment, which turned out to be the use of the wrong variable. rc starts life as -1, and is only ever assigned to 0 just before a successful cleanup. * src/lxc/lxc_driver.c (lxcSetupInterfaces): Don't call virReportSystemError(-1). --- src/lxc/lxc_driver.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index e905302..b94941d 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1100,10 +1100,9 @@ static int lxcSetupInterfaces(virConnectPtr conn, } if ((ret = brAddInterface(brctl, bridge, parentVeth)) != 0) { - virReportSystemError(rc, + virReportSystemError(ret, _("Failed to add %s device to %s"), parentVeth, bridge); - rc = -1; goto error_exit; } -- 1.7.4.4

2 2

[libvirt] [PATCH] util: remove dead assignment
by Eric Blake 04 May '11

04 May '11

Clang complained about this, and it was easy enough to fix. * src/util/util.c (virFileOpenAs): Drop dead assignment. --- src/util/util.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 37472bb..9041ab6 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1523,8 +1523,7 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, if (ret < 0 && errno != EACCES) { ret = -errno; VIR_FORCE_CLOSE(pair[0]); - while ((waitret = waitpid(pid, NULL, 0) == -1) - && (errno == EINTR)); + while (waitpid(pid, NULL, 0) == -1 && errno == EINTR); goto parenterror; } else { fd = ret; -- 1.7.4.4

2 2

[libvirt] [PATCH] libxl: avoid compiler warning
by Eric Blake 04 May '11

04 May '11

Detected by gcc: libxl/libxl_driver.c: In function 'libxlDomainDestroy': libxl/libxl_drier.c:1351:30: error: variable 'priv' set but not used [-Werror=unused-but-set-variable] * src/libxl/libxl_driver.c (libxlDomainDestroy): Delete unused variable. --- Trivial, so pushing under the build-breaker rule. src/libxl/libxl_driver.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 247d78e..dec4f43 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1348,7 +1348,6 @@ libxlDomainDestroy(virDomainPtr dom) libxlDriverPrivatePtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; - libxlDomainObjPrivatePtr priv; virDomainEventPtr event = NULL; libxlDriverLock(driver); @@ -1370,7 +1369,6 @@ libxlDomainDestroy(virDomainPtr dom) event = virDomainEventNewFromObj(vm,VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); - priv = vm->privateData; if (libxlVmReap(driver, vm, 1) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), dom->id); -- 1.7.4.4

1 0

[libvirt] [PATCH] qemu: remove dead assignment
by Eric Blake 04 May '11

04 May '11

Detected by clang. * src/qemu/qemu_migration.c (qemuMigrationToFile): Nothing later uses is_reg. --- src/qemu/qemu_migration.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7f4b111..6c5bf66 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1311,7 +1311,6 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm, if (virSecurityManagerSetFDLabel(driver->securityManager, vm, compressor ? pipeFD[1] : fd) < 0) goto cleanup; - is_reg = true; bypassSecurityDriver = true; } else { /* Phooey - we have to fall back on exec migration, where qemu -- 1.7.4.4

2 2

[libvirt] [PATCH] storage: use virCommand to avoid compiler warning
by Eric Blake 04 May '11

04 May '11

clang didn't like the last increment to nargs. But why even track nargs ourselves, when virCommand does it for us? * src/storage/storage_backend_iscsi.c (virStorageBackendISCSIConnection): Switch to virCommand to avoid a dead-store warning on nargs. --- src/storage/storage_backend_iscsi.c | 36 ++++++++-------------------------- 1 files changed, 9 insertions(+), 27 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index d86f077..ec52dbc 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -42,6 +42,7 @@ #include "memory.h" #include "logging.h" #include "files.h" +#include "command.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -348,28 +349,11 @@ virStorageBackendISCSIConnection(const char *portal, "--targetname", target, NULL }; - int i; - int nargs = 0; + virCommandPtr cmd; char *ifacename = NULL; - const char **cmdargv; - - for (i = 0 ; baseargv[i] != NULL ; i++) - nargs++; - for (i = 0 ; extraargv[i] != NULL ; i++) - nargs++; - if (initiatoriqn) - nargs += 2; - - if (VIR_ALLOC_N(cmdargv, nargs+1) < 0) { - virReportOOMError(); - return -1; - } - nargs = 0; - for (i = 0 ; baseargv[i] != NULL ; i++) - cmdargv[nargs++] = baseargv[i]; - for (i = 0 ; extraargv[i] != NULL ; i++) - cmdargv[nargs++] = extraargv[i]; + cmd = virCommandNewArgs(baseargv); + virCommandAddArgSet(cmd, extraargv); if (initiatoriqn) { switch (virStorageBackendIQNFound(initiatoriqn, &ifacename)) { @@ -377,7 +361,8 @@ virStorageBackendISCSIConnection(const char *portal, VIR_DEBUG("ifacename: '%s'", ifacename); break; case IQN_MISSING: - if (virStorageBackendCreateIfaceIQN(initiatoriqn, &ifacename) != 0) { + if (virStorageBackendCreateIfaceIQN(initiatoriqn, + &ifacename) != 0) { goto cleanup; } break; @@ -385,19 +370,16 @@ virStorageBackendISCSIConnection(const char *portal, default: goto cleanup; } - - cmdargv[nargs++] = "--interface"; - cmdargv[nargs++] = ifacename; + virCommandAddArgList(cmd, "--interface", ifacename, NULL); } - cmdargv[nargs++] = NULL; - if (virRun(cmdargv, NULL) < 0) + if (virCommandRun(cmd, NULL) < 0) goto cleanup; ret = 0; cleanup: - VIR_FREE(cmdargv); + virCommandFree(cmd); VIR_FREE(ifacename); return ret; -- 1.7.4.4

2 2

[libvirt] pci device passthrough: Failed to assign irq
by guan qin 04 May '11

04 May '11

Hi, I encounter a problem that the eth can't be up in the VM by using VT-d. I boot the VM using "virsh create vt_d.xml". The VM could recongize the NIC which was assigned .then I do in VM as follows: #cat /sys/class/net/eth0/operstate down #ifconfig eth0 up #cat /sys/class/net/eth0/operstate down #ficonfig eth 10.100.0.5 #cat /sys/class/net/eth0/operstate down I look into the qemu log file ,find that : LC_ALL=C PATH=/usr/local/sbin:/usr/ local/bin:/sbin:/bin:/usr/sbin:/usr/bin HOME=/ QEMU_AUDIO_DRV=none /usr/bin/kvm -S -M pc-0.12 -enable-kvm -m 512 -smp 1,sockets=1,cores=1,threads=1 -name vt_d_5 -uuid 05f608a3-f64f-2418-a90a-d611982c8336 -nodefaults -chardev socket,id=monitor,path=/var/lib/libvirt/qemu/vt_d_5.monitor,server,nowait -mon chardev=monitor,mode=readline -rtc base=utc -boot c -drive file=/home/qinguan/exp/kvm_vtd/ubuntu_1.img,if=none,id=drive-virtio-disk0,boot=on,format=raw -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0 -usb -device usb-mouse,id=input0 -vnc 127.0.0.1:0 -vga cirrus -device pci-assign,host=00:19.0,id=hostdev0,bus=pci.0,addr=0x4 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 Failed to assign irq for "hostdev0": Operation not permitted Perhaps you are assigning a device that shares an IRQ with another device? Failed to assign irq for "hostdev0": Operation not permitted Perhaps you are assigning a device that shares an IRQ with another device? Failed to assign irq for "hostdev0": Operation not permitted Perhaps you are assigning a device that shares an IRQ with another device? Failed to assign irq for "hostdev0": Operation not permitted Perhaps you are assigning a device that shares an IRQ with another device? Failed to assign irq for "hostdev0": Operation not permitted maybe the VM can't access to the network because of Failed to assign irg for "hostdev0"? in kern.log : May 1 12:40:07 Optiplex-780 kernel: [ 3172.784803] e1000e 0000:00:19.0: PCI INT A disabled May 1 12:40:07 Optiplex-780 kernel: [ 3172.784863] pci-stub 0000:00:19.0: claimed by stub May 1 12:40:07 Optiplex-780 kernel: [ 3173.103965] pci-stub 0000:00:19.0: PCI INT A -> GSI 21 (level, low) -> IRQ 21 May 1 12:40:07 Optiplex-780 kernel: [ 3173.204030] pci-stub 0000:00:19.0: restoring config space at offset 0x6 (was 0x1, writing 0xece1) May 1 12:40:07 Optiplex-780 kernel: [ 3173.204035] pci-stub 0000:00:19.0: restoring config space at offset 0x5 (was 0x0, writing 0xf7ad9000) May 1 12:40:07 Optiplex-780 kernel: [ 3173.204039] pci-stub 0000:00:19.0: restoring config space at offset 0x4 (was 0x0, writing 0xf7ae0000) May 1 12:40:07 Optiplex-780 kernel: [ 3173.204045] pci-stub 0000:00:19.0: restoring config space at offset 0x1 (was 0x100000, writing 0x100103) May 1 12:40:07 Optiplex-780 kernel: [ 3173.435751] assign device 0:0:19.0 May 1 12:44:24 Optiplex-780 kernel: [ 3430.568526] pci-stub 0000:00:19.0: restoring config space at offset 0x6 (was 0x1, writing 0xece1) May 1 12:44:24 Optiplex-780 kernel: [ 3430.568532] pci-stub 0000:00:19.0: restoring config space at offset 0x5 (was 0x0, writing 0xf7ad9000) May 1 12:44:24 Optiplex-780 kernel: [ 3430.568537] pci-stub 0000:00:19.0: restoring config space at offset 0x4 (was 0x0, writing 0xf7ae0000) May 1 12:44:24 Optiplex-780 kernel: [ 3430.568543] pci-stub 0000:00:19.0: restoring config space at offset 0x1 (was 0x100000, writing 0x100103) May 1 12:44:24 Optiplex-780 kernel: [ 3430.568561] pci-stub 0000:00:19.0: PCI INT A disabled root@Optiplex-780:/var/log/libvirt/qemu# cat /proc/interrupts CPU0 CPU1 CPU2 CPU3 0: 97 1 2 1 IO-APIC-edge timer 1: 2 0 0 0 IO-APIC-edge i8042 7: 0 0 0 0 IO-APIC-edge parport0 8: 1 0 0 0 IO-APIC-edge rtc0 9: 0 0 0 0 IO-APIC-fasteoi acpi 12: 1 2 1 0 IO-APIC-edge i8042 16: 0 0 0 0 IO-APIC-fasteoi uhci_hcd:usb2 17: 5822 1952 3614 2233 IO-APIC-fasteoi uhci_hcd:usb4, uhci_hcd:usb7 18: 0 0 0 0 IO-APIC-fasteoi uhci_hcd:usb8 22: 1 1 1 0 IO-APIC-fasteoi ehci_hcd:usb1, uhci_hcd:usb5 23: 1461 1299 1153 912 IO-APIC-fasteoi ehci_hcd:usb3, uhci_hcd:usb6 24: 268579 0 0 0 HPET_MSI-edge hpet2 25: 0 286310 0 0 HPET_MSI-edge hpet3 26: 0 0 210675 0 HPET_MSI-edge hpet4 27: 0 0 0 256688 HPET_MSI-edge hpet5 29: 1 0 0 0 DMAR_MSI-edge dmar1 30: 0 0 0 0 DMAR_MSI-edge dmar0 31: 0 0 0 0 DMAR_MSI-edge dmar2 35: 0 0 0 1 PCI-MSI-edge eth1 36: 1456 916 1323 1657 PCI-MSI-edge eth1-rx-0 37: 1031 772 1459 895 PCI-MSI-edge eth1-rx-1 38: 958 965 785 470 PCI-MSI-edge eth1-rx-2 39: 1319 557 1007 714 PCI-MSI-edge eth1-rx-3 40: 892 864 417 1134 PCI-MSI-edge eth1-tx-0 41: 414 749 998 606 PCI-MSI-edge eth1-tx-1 42: 307 921 375 332 PCI-MSI-edge eth1-tx-2 43: 966 693 396 502 PCI-MSI-edge eth1-tx-3 54: 7109 5755 5683 5609 PCI-MSI-edge ahci 55: 74 105 73 84 PCI-MSI-edge hda_intel 56: 1446 927 1108 870 PCI-MSI-edge i915 NMI: 0 0 0 0 Non-maskable interrupts LOC: 740 725 703 677 Local timer interrupts SPU: 0 0 0 0 Spurious interrupts PMI: 0 0 0 0 Performance monitoring interrupts PND: 0 0 0 0 Performance pending work RES: 665 679 714 547 Rescheduling interrupts CAL: 5332 408 5619 539 Function call interrupts TLB: 1880 2843 1788 3202 TLB shootdowns TRM: 0 0 0 0 Thermal event interrupts THR: 0 0 0 0 Threshold APIC interrupts MCE: 0 0 0 0 Machine check exceptions MCP: 5 5 5 5 Machine check polls ERR: 3 MIS: 0 root@Optiplex-780:/var/log/libvirt/qemu# I search the similarly problems that sharing the irg but never find a answer , and now I have no idea. How to solve the problem ,can you help me ? thanks. best regards, qinguan vt_d.xml: <domain type='kvm'> <name>test</name> <description>just test using sriov!</description> <os> <type>hvm</type> <boot dev='hd'/> </os> <memory>524288</memory> <currentMemory>524288</ currentMemory> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> <features><pae/><acpi/><apic/></features> <clock offset='utc'/> <devices> <input type='mouse' bus='usb'/> <disk type='file' device='disk'> <source file='/home/qinguan/exp/ubuntu_1.img'/> <target dev='hda' bus='virtio'/> </disk> <emulator>/usr/bin/kvm</emulator> <graphics type='vnc' port='-1'/> <hostdev mode='subsystem' type='pci' managed='yes'> <source> <address bus='0x0' slot='0x19' function='0x0'/> </source> </hostdev> </devices> </domain>

1 1

[libvirt] Support for more than 32 virtio devices
by Badari Pulavarty 04 May '11

04 May '11

Hi, How can I make use of "multifunction" functionality to support more than 32 virtio devices in QEMU through libvirt ? Are there corresponding changes in libvirt ? Please let me know. Thanks, Badari

3 3

[libvirt] failure to build on rawhide
by Eric Blake 04 May '11

04 May '11

I'm getting this when trying to build libvirt on rawhide (using the package gnutls-devel-2.12.3-1.fc16.x86_64): remote/remote_driver.c: In function 'negotiate_gnutls_on_connection': remote/remote_driver.c:1361:9: error: 'gnutls_certificate_type_set_priority' is deprecated (declared at /usr/include/gnutls/compat.h:347) [-Werror=deprecated-declarations] but I can't find anything in the gnutls man pages that says why it is deprecated or what to use in its place. Any ideas? -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2 1

[libvirt] [PATCH] lxc: Do not try to reconnect inactive domain when do lxcStartup
by Osier Yang 04 May '11

04 May '11

Otherwise if there are inactive lxc domains, lxcStartup will try to reconnect to sockets of these domains, which results in errors in libvirtd log. --- src/lxc/lxc_driver.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index e905302..ef7827b 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1992,6 +1992,9 @@ lxcReconnectVM(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) virDomainObjLock(vm); + if (!virDomainObjIsActive(vm)) + goto cleanup; + priv = vm->privateData; if ((priv->monitor = lxcMonitorClient(driver, vm)) < 0) { goto cleanup; -- 1.7.4

3 7

[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