May 2011 - Devel - libvirt List Archives

[libvirt] Release of libvirt-0.9.1
by Daniel Veillard 05 May '11

05 May '11

As planned and after most of the clang detected problems got fixed (thanks Eric !) the new release is available at: ftp://libvirt.org/libvirt/ It's a mixed release, it includes a number of improvements as well as many bug fixes and a few new features: Features: - support various persistent domain updates (KAMEZAWA Hiroyuki) - improvements on memory APIs (Taku Izumi) - Add virDomainEventRebootNew (Matthias Bolte) - various improvements to libxl driver (Markus Groß) - Spice: support audio, images and stream compression (Michal Privoznik) Documentation: - fix missing VLAN id for Qbg example (Gerhard Stenzel) - docs: Document filesystem tag device (Cole Robinson) - maint: fix comment typos (Eric Blake) - Fix two out-of-date comments in LVM backend (Richard Laager) - docs: Serial and parallel device target ports actually start from 0 (Matthias Bolte) - maint: fix grammar errors (Eric Blake) - docs: document freecell --all (Eric Blake) - docs: remove "returns" word from beginning of lines (Jean-Baptiste Rouault) - docs: add an IPv6 address to network XML examples (Laine Stump) Portability: - build: fix getcwd portability problems (Eric Blake) - build: avoid test warnings on mingw (Eric Blake) - virsh: avoid compiler warning on mingw (Eric Blake) - build: Use pkg-config for libssh2 check (Jiri Denemark) - build: Ignore old audit library (Jiri Denemark) - build: fix 32-bit test failure (Eric Blake) - build: Fix problem of building Python bindings (Osier Yang) - build: fix 32-bit test failure (Eric Blake) - ppc: Enable starting of Qemu VMs on ppc host (Stefan Berger) - Migrate VMs between different-endianess hosts (Stefan Berger) - build: really fix mingw startup (Eric Blake) - build: fix mingw build (Eric Blake) - do not build libvirt_iohelper when building without libvirtd (Wen Congyang) - virsh: fix mingw startup (Eric Blake) - build: avoid compiler warning on cygwin (Eric Blake) - Fix build for older gcc (Jim Fehlig) - Don't try to enable stack protector on Win32 (Daniel P. Berrange) Bug Fixes: - storage: avoid null deref and leak on failure (Eric Blake) - esx: Avoid null dereference on error in esxDomainGetInfo (Matthias Bolte) - remote: avoid null dereference on error (Eric Blake) - cgroup: avoid leaking a file (Eric Blake) - virsh: avoid null pointer dereference (Eric Blake) - pci: fix null pointer dereference (Eric Blake) - qemu: avoid null pointer dereference (Eric Blake) - tests: avoid null pointer dereference (Eric Blake) - Fix disability to run on systems with no PCI bus (Michal Privoznik) - virsh: fix regression in log to file (Supriya Kannery) - nwfilter: Fix memory leak in the ebtables subdriver (Matthias Bolte) - qemu: Fix qemuDomainModifyDeviceFlags leaking the caps bitmap (Matthias Bolte) - Fix memory leak in __virExec (Matthias Bolte) - hash: fix memory leak regression (Eric Blake) - Xen: Do not generate net ifname if domain is inactive (Jim Fehlig) - xen: check if device is assigned to guest before reattaching (Yufang Zhang) - util: Initialize hooks at daemon shutdown if no hooks defined (Osier Yang) - fix virsh's regression (Wen Congyang) - release PCI address only when we have ensured it successfully (Wen Congyang) - free memory properly in cleanup patch (Hu Tao) - free buf content when vsnprintf() failed (Wen Congyang) - esx: Fix dynamic dispatch for CastFromAnyType functions (Matthias Bolte) - Make crash and live flags mutually exclusive in virDomainCoreDump (Mark Wu) - Fix small memory leaks in config parsing related functions (Matthias Bolte) - daemon: Don't try to free an unsigned int in error paths (Matthias Bolte) - Fix QEMU tunnelled migration FD handling (Daniel P. Berrange) - Remove artificial minimum limit for guest memory (Jiri Denemark) - fix tunnelled migration's regression (Wen Congyang) - qemu: fix the check of virDomainObjUnref()'s return value (Wen Congyang) - Fix checking of return codes in dispatcher (Daniel P. Berrange) - qemu: avoid qemu_driver being unlocked twice when virThreadPoolNew() failed (Wen Congyang) - virsh: fix regression in parsing optional integer (Eric Blake) - phyp: avoid memory leaks in command values (Eric Blake) - phyp: avoid memory leak on failure (Eric Blake) - phyp: avoid a logic bug (Eric Blake) - free cpumask of vcpupinDef (Hu Tao) - network: Fix NULL dereference during error recovery (Michal Privoznik) - Add missing checks for QEMU domain state in tunables APIs (Daniel P. Berrange) - phyp: Fix too small buffer allocation in phypAttachDevice (Matthias Bolte) - phyp: Don't overwrite error from virDomainDeviceDefParse by OOM error (Matthias Bolte) - phyp: Don't try to use a string from a failed virAsprintf (Matthias Bolte) - phyp: Remove stack allocating a 4kb volume key and fix related memory leaks (Matthias Bolte) - qemu: fix a dead-lock problem (Hu Tao) - util: Fix crash when removing entries during hash iteration (Jiri Denemark) - Fix possible infinite loop in remote driver (Michal Privoznik) - qemu: Remove the managed state file only if restoring succeeded (Osier Yang) - reattach pci devices when qemuPrepareHostdevPCIDevices() failed (Wen Congyang) - reattach pci device when pciBindDeviceToStub() failed (Wen Congyang) - remove devices from driver activePciHostdevs when qemuPrepareHostdevPCIDevices() failed (Wen Congyang) - pci: avoid invalid free, init path to NULL (Wen Congyang) - qemu: Support for overriding NPROC limit (Jiri Denemark) - qemu: Always reserves slot 0x02 for primary VGA. (Osier Yang) - Fix typo in systemtap tapset directory name (Daniel P. Berrange) - qemu: Ignore unusable binaries (Jiri Denemark) Improvements: - maint: detect clang 2.9 (Eric Blake) - qemu: update qemuCgroupControllerActive signature (Eric Blake) - lxc: report correct error (Eric Blake) - libxl: avoid compiler warning (Eric Blake) - storage: use virCommand to avoid compiler warning (Eric Blake) - tests: Lower stack usage below 4096 bytes (Matthias Bolte) - tests: Update valgrind suppressions file (Matthias Bolte) - tests: simplify common setup (Eric Blake) - network: fix return value of hostsFileWrite (Laine Stump) - libvirt/qemu - support persistent attach/detach disks (KAMEZAWA Hiroyuki) - libvirt/qemu - support persistent modification of devices (KAMEZAWA Hiroyuki) - util: Simplify hash implementation (Jiri Denemark) - tests: More unit tests for internal hash APIs (Jiri Denemark) - Move call to virReportOOMError into virFileBuildPath (Matthias Bolte) - build: use gnulib passfd for simpler SCM_RIGHTS code (Eric Blake) - qemu: Add flags checking in DomainCoreDump (Jiri Denemark) - Make sure DNSMASQ_STATE_DIR exists (Guido Günther) - build: make VIR_FREE do some type checking (Eric Blake) - threads: add one-time initialization support (Eric Blake) - esx: Add a wrapper for shared CURL handles (Matthias Bolte) - esx: Move CURL handling code to it's own type (Matthias Bolte) - daemon: Honor error variable name change in the generator (Matthias Bolte) - libvirt/qemu - clean up UpdateDevice for consolidation. (KAMEZAWA Hiroyuki) - libvirt/qemu - clean up At(De)tachDeviceFlags() for consolidation. (KAMEZAWA Hiroyuki) - libvirt/qemu - Centralize device modification in the more flexible APIs (KAMEZAWA Hiroyuki) - Add support for s390(x) cpu options parsing (Michal Privoznik) - PHYP: Adding reboot domain function (Eduardo Otubo) - nwfilter: no support for direct type of interface (Stefan Berger) - Add cputune support to libxl driver (Markus Groß) - Update and sort msg_gen_function list and mark unmarked messages (Matthias Bolte) - Write error check conditionals in more compact form for dispatcher (Daniel P. Berrange) - Merge all returns paths from dispatcher into single path (Daniel P. Berrange) - Experimental libvirtd upstart job (Alan Pevec) - enhance processWatchdogEvent() (Wen Congyang) - xen: Replace statsErrorFunc with a macro (Matthias Bolte) - tests: Unit tests for internal hash APIs (Jiri Denemark) - build: include esx_vi.generated.* into dist file (Wen Congyang) - tests: test recent virsh option parsing changes (Eric Blake) - virsh: list required options first (Eric Blake) - phyp: use consistent return string handling (Eric Blake) - maint: use lighter-weight function for straight appends (Eric Blake) - libvirt-guests: implement START_DELAY (Alexander Todorov) - network: truncate bridges' dummy tap device names to IFNAMSIZ (15) chars (Laine Stump) - Introduce virDomainChrDefNew() (Michal Novotny) - esx: Make the parsed URI part of the private connection data (Matthias Bolte) - esx: Mark error message in macros for translation (Matthias Bolte) - esx: Extend VI generator to cover managed object types (Matthias Bolte) - esx: Cleanup and refactor CastFromAnyType macros (Matthias Bolte) - esx: Cleanup VI generator code (Matthias Bolte) - phyp: Reduce code duplication in error and success paths (Matthias Bolte) - Replace REMOTE_DEBUG with VIR_DEBUG in daemon dispatcher (Daniel P. Berrange) - Add missing checks for whether the connection is open in dispatcher (Daniel P. Berrange) - PHYP: Adding network interface (Eduardo Otubo) - xen: Remove PATH_MAX sized stack allocation from block stats code (Matthias Bolte) - setmaxmem: add the new options to "virsh setmaxmem" command (Taku Izumi) - maxmem: implement virDomainSetMaxMemory API of the qemu driver (Taku Izumi) - maxmem: introduces VIR_DOMAIN_MEM_MAXIMUM flag (Taku Izumi) - nwfilters: support for TCP flags evaluation (Stefan Berger) - setmem: add --current option to virsh setmem command (Taku Izumi) - setmem: add VIR_DOMAIN_MEM_CURRENT support to qemu (Taku Izumi) - setmem: introduce VIR_DOMAIN_MEM_CURRENT flag (Taku Izumi) - build: Install libxenlight log dir (Jim Fehlig) - Add domainSet/GetSchedulerParameters to libxl driver (Markus Groß) - rename pciUnBindDeviceFromStub() to pciUnbindDeviceFromStub() and float it up (Wen Congyang) - Add domainIsUpdated to libxl driver (Markus Groß) - Change locking for udev monitor and callbacks (Serge Hallyn) - Improve SCSI volume key generation (Daniel P. Berrange) - qemu: Rewrite LOOKUP_PTYS macro into a function (Jiri Denemark) - Enable use of -Wold-style-definition compiler flag (Daniel P. Berrange) - Enable use of -Wmissing-noreturn (Daniel P. Berrange) - Enable -Wmissing-format-attribute warning (Daniel P. Berrange) - Use gnulib's manywarnings and warnings modules (Daniel P. Berrange) - Use virBufferPtr for sexpr2string instead of manual buffer handling (Matthias Bolte) - xend: Remove 4kb stack allocation (Matthias Bolte) - uml: Remove PATH_MAX sized stack allocation from /proc parsing code (Matthias Bolte) - storage: Remove PATH_MAX sized stack allocation from iSCSI backend (Matthias Bolte) - qemu: Remove PATH_MAX sized stack allocation used in commandline building (Matthias Bolte) - Remove PATH_MAX sized stack allocation from virFileOpenTtyAt (Matthias Bolte) - openvz: Remove several larger stack allocations (Matthias Bolte) - daemon: Remove 4kb stack allocation of security label (Matthias Bolte) - virsh: Remove two 4kb stack allocations (Matthias Bolte) - Use virFileAbsPath instead of manually creating the absolute path (Matthias Bolte) - xenxs: Remove PATH_MAX sized stack allocation in XM script parsing (Matthias Bolte) - sasl: Remove stack allocated 8kb temporary buffers (Matthias Bolte) - qemu: Use heap allocated memory to read the monitor greeting (Matthias Bolte) - phyp: Remove 16kb stack allocation (Matthias Bolte) - virt-aa-helper: Remove PATH_MAX sized stack allocations (Matthias Bolte) - ebtables: Remove PATH_MAX sized stack allocation (Matthias Bolte) - pci: Remove PATH_MAX sized stack allocations (Matthias Bolte) - Remove PATH_MAX sized stack allocations related to virFileBuildPath (Matthias Bolte) - vmx: Use case-insensitive compare functions for all content (Matthias Bolte) - vmx: Support persistent CPU shares (Matthias Bolte) - Add autostart support to libxl driver (Markus Groß) - Allow relative path for qemu backing file (Jesse Cook) - build: detect potentential uninitialized variables (Eric Blake) Cleanups: - esx: Disable performance counter queries in esxDomainGetInfo (Matthias Bolte) - esx: Remove dead store in esxUtil_ParseDatastorePath (Matthias Bolte) - util: remove dead assignment (Eric Blake) - qemu: remove dead assignment (Eric Blake) - qemu: silence clang false positives (Eric Blake) - tests: suppress more valgrind situations (Eric Blake) - tests: avoid compiler warning (Eric Blake) - qemu: fix uninitialized variable warning (Christophe Fergeau) - build: fix syntax-check failure (Eric Blake) - virsh: nuke use of TRUE and FALSE (Eric Blake) - phyp: another simplification (Eric Blake) - maint: ignore built file (Eric Blake) - Change some variable names to follow standard in daemon dispatcher (Daniel P. Berrange) - Remove curly braces on all single-line conditional jumps in dispatcher (Daniel P. Berrange) - Remove virConnectPtr from virRaiseErrorFull (Matthias Bolte) - phyp: prefer memcpy over memmove when legal (Eric Blake) - phyp: use consistent style for labels (Eric Blake) - phyp: more return handling cleanup (Eric Blake) - esx: Fix gcc 4.6 warning about initialized but unused variables (Matthias Bolte) - maint: silence cppi warnings (Eric Blake) - Fix gcc 4.6 warnings in vbox_tmpl.c (Christophe Fergeau) - Fix gcc 4.6 warnings (Christophe Fergeau) - Remove C99 variable declare in PHYP network driver (Daniel P. Berrange) - Standard on error variable name in libvirtd dispatcher (Daniel P. Berrange) - Remove all whitespace before function brackets in daemon dispatcher (Daniel P. Berrange) - docs: tweak virsh restore warning (Eric Blake) - setmaxmem: remove the code to invoke virDomainSetMemory in cmdSetmaxmem (Taku Izumi) - build: fix gitignore sorting (Eric Blake) - tests: fix recent test failures (Eric Blake) - libxl: avoid compiler warning (Eric Blake) - Avoid compiler warnings about int to void * casts (Daniel P. Berrange) - Remove acinclude.m4 file (Daniel P. Berrange) - Remove possible uninitialized variable in openvz driver (Daniel P. Berrange) Thanks to everybody who helped with this release in any way, bug reports, documentation, patches or features ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

1 0

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

04 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