September 2014 - Devel - libvirt List Archives

[libvirt] [PATCH 00/26] Resolve more Coverity issues
by John Ferlan 11 Sep '14

11 Sep '14

Sorry for the large dump, but before I got too involved in other things I figured I'd go through the list of the remaining 68 Coverity issues from the new version in order to reduce the pile. Many are benign, some seemingly false positives, and I think most are error paths. The one non error path that does stick out is the qemu_driver.c changes in the qemuDomainSetBlkioParameters() routine where 'param' and 'params' were used differently between LIVE and CONFIG. In particular, in CONFIG the use of 'params->field' instead of 'param->field'. One that does bear looking at more closely and if someone has a better idea is avoiding a false positive resource_leak in remote_driver.c. I left a healthy comment in the code - you'll know when you see it. These patches get the numbers down to 19 issues. Of the remaining issues - some are related to Coverity thinking that 'mgetgroups' could return a negative value with an allocated groups structure (which I'm still scratching my head over). There is also a few calls to virJSONValueObjectGetNumberUlong() in qemu_monitor_json.c that don't check status, but I'm not sure why - just didn't have the research cycles for that. John Ferlan (26): qemu_driver: Resolve Coverity COPY_PASTE_ERROR remote_driver: Resolve Coverity RESOURCE_LEAK storage: Resolve Coverity UNUSED_VALUE vbox: Resolve Coverity UNUSED_VALUE qemu: Resolve Coverity REVERSE_INULL storage: Resolve Coverity OVERFLOW_BEFORE_WIDEN virsh: Resolve Coverity DEADCODE virfile: Resolve Coverity DEADCODE virsh: Resolve Coverity DEADCODE qemu: Resolve Coverity DEADCODE tests: Resolve Coverity DEADCODE virsh: Resolve Coverity DEADCODE qemu: Resolve Coverity FORWARD_NULL lxc: Resolve Coverity FORWARD_NULL qemu: Resolve Coverity FORWARD_NULL network: Resolve Coverity FORWARD_NULL virstring: Resolve Coverity FORWARD_NULL qemu: Resolve Coverity FORWARD_NULL network_conf: Resolve Coverity FORWARD_NULL qemu: Resolve Coverity NEGATIVE_RETURNS nodeinfo: Resolve Coverity NEGATIVE_RETURNS virsh: Resolve Coverity NEGATIVE_RETURNS xen: Resolve Coverity NEGATIVE_RETURNS qemu: Resolve Coverity NEGATIVE_RETURNS qemu: Resolve Coverity NEGATIVE_RETURNS libxl: Resolve Coverity NULL_RETURNS src/conf/network_conf.c | 4 ++-- src/libxl/libxl_migration.c | 1 - src/lxc/lxc_driver.c | 6 ++++-- src/network/leaseshelper.c | 3 +-- src/nodeinfo.c | 2 +- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_command.c | 1 + src/qemu/qemu_driver.c | 26 +++++++++++++++----------- src/qemu/qemu_migration.c | 3 ++- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_process.c | 5 +++-- src/remote/remote_driver.c | 12 ++++++++++++ src/storage/storage_backend_disk.c | 2 +- src/storage/storage_backend_fs.c | 1 - src/util/virfile.c | 5 ++--- src/util/virstring.c | 3 +++ src/vbox/vbox_common.c | 9 +++++++-- src/xen/xend_internal.c | 3 ++- tests/virstringtest.c | 5 +++++ tools/virsh-domain.c | 22 ++++++++-------------- tools/virsh-edit.c | 9 --------- tools/virsh-interface.c | 3 --- tools/virsh-network.c | 12 +++++------- tools/virsh-nwfilter.c | 3 --- tools/virsh-pool.c | 3 --- tools/virsh-snapshot.c | 3 --- 26 files changed, 76 insertions(+), 74 deletions(-) -- 1.9.3

4 60

[libvirt] [PATCH] util/virprocess.c: fix MinGW build
by Pavel Hrdina 11 Sep '14

11 Sep '14

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/util/virprocess.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 15d8309..3dae1bd 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -28,7 +28,6 @@ #include <stdlib.h> #include <sys/wait.h> #include <unistd.h> -#include <sys/syscall.h> #if HAVE_SETRLIMIT # include <sys/time.h> # include <sys/resource.h> @@ -78,10 +77,21 @@ VIR_LOG_INIT("util.process"); #endif #ifndef HAVE_SETNS +# ifndef WIN32 +# include <sys/syscall.h> + static inline int setns(int fd, int nstype) { return syscall(__NR_setns, fd, nstype); } +# else +static inline int setns(int fd ATTRIBUTE_UNUSED, int nstype ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Namespaces are not supported on windows.")); + return -1; +} +# endif /* WIN32 */ #endif /* HAVE_SETNS */ /** -- 1.8.5.5

2 4

[libvirt] [PATCH] selinux: Properly check TAP FD label
by Michal Privoznik 11 Sep '14

11 Sep '14

After a4431931 the TAP FDs ale labeled with image label instead of the process label. On the other hand, the commit was incomplete as a few lines above, there's still old check for the process label presence while it should be check for the image label instead. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Pushed under trivial rule. After this commit, the function is completely the same as virSecuritySELinuxSetImageFDLabel(). However I'd like to keep them separate because there's an ongoing bug: https://bugzilla.redhat.com/show_bug.cgi?id=1095636 so with fair chance the TapFDLabel() function will be rewritten soon. src/security/security_selinux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 7064158..bf67fb5 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2347,7 +2347,7 @@ virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virSecurityLabelDefPtr secdef; secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); - if (!secdef || !secdef->label) + if (!secdef || !secdef->imagelabel) return 0; return virSecuritySELinuxFSetFilecon(fd, secdef->imagelabel); -- 1.8.5.5

1 0

[libvirt] [PATCH] libxl: support domainReset
by Jim Fehlig 10 Sep '14

10 Sep '14

Currently, libxl_send_trigger() does not implement the LIBXL_TRIGGER_RESET option, but domainReset can be implemented in the libxl driver by forcibly destroying the domain and starting it again. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- src/libxl/libxl_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 67fd7bc6..08018d4 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -979,6 +979,62 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) } static int +libxlDomainReset(virDomainPtr dom, unsigned int flags) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + libxlDomainObjPrivatePtr priv; + + virCheckFlags(0, -1); + + if (!(vm = libxlDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainResetEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is not running")); + goto cleanup; + } + + /* + * The semantics of reset can be achieved by forcibly destroying + * the domain and starting it again. + */ + priv = vm->privateData; + if (libxl_domain_destroy(priv->ctx, vm->def->id, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to reset domain '%d'"), vm->def->id); + goto cleanup; + } + + if (!libxlDomainCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to cleanup domain '%d' after reset"), + vm->def->id); + vm = NULL; + goto cleanup; + } + + if (libxlDomainStart(driver, vm, false, -1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to start domain '%d' after reset"), + vm->def->id); + goto cleanup; + } + + ret = 0; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int libxlDomainDestroyFlags(virDomainPtr dom, unsigned int flags) { @@ -4758,6 +4814,7 @@ static virDriver libxlDriver = { .domainShutdown = libxlDomainShutdown, /* 0.9.0 */ .domainShutdownFlags = libxlDomainShutdownFlags, /* 0.9.10 */ .domainReboot = libxlDomainReboot, /* 0.9.0 */ + .domainReset = libxlDomainReset, /* 1.2.8 */ .domainDestroy = libxlDomainDestroy, /* 0.9.0 */ .domainDestroyFlags = libxlDomainDestroyFlags, /* 0.9.4 */ .domainGetOSType = libxlDomainGetOSType, /* 0.9.0 */ -- 1.8.4.5

5 15

[libvirt] [PATCH] qemu: remove leftover virResetLastError
by Ján Tomko 10 Sep '14

10 Sep '14

As of commit 5d29ca0: qemu: switch PCI address set from hash table to an array There is no error to be reset. --- src/qemu/qemu_command.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5856fe7..665a590 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1770,7 +1770,6 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def, VIR_DEBUG("PCI address 0:0:2.0 in use, future addition of a video" " device will not be possible without manual" " intervention"); - virResetLastError(); } else if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) { goto cleanup; } -- 1.8.5.5

2 1

[libvirt] [PATCH 0/5] Fix starting of VMs with backing chains containing unknown backing file specification
by Peter Krempa 10 Sep '14

10 Sep '14

When a backing chain element specifies a parent in a format not known to libvirt we'd fail to start the VM as the chain would appear broken. To prevent this happening introduce a new disk type to collect unknow format specs and avoid startup failures with such disk type. Peter Krempa (5): util: storage: Convert disk locality check to switch statement conf: Introduce raw disk string passthrough conf: Mark backing chain ending with "raw" volume as broken tests: Add tests for disk type 'raw' utils: storage: Fall back to "raw" disk on backing store parse failure docs/schemas/domaincommon.rng | 14 ++++ src/conf/domain_conf.c | 12 ++- src/qemu/qemu_command.c | 1 + src/qemu/qemu_driver.c | 4 + src/storage/storage_driver.c | 8 ++ src/util/virstoragefile.c | 35 +++++++- src/util/virstoragefile.h | 1 + .../qemuxml2argv-disk-backing-chains-raw.xml | 94 +++++++++++++++++++++ .../qemuxml2argv-disk-virtio-raw.args | 9 ++ .../qemuxml2argv-disk-virtio-raw.xml | 45 ++++++++++ tests/qemuxml2argvtest.c | 1 + ...muxml2xmlout-disk-backing-chains-raw-active.xml | 95 ++++++++++++++++++++++ ...xml2xmlout-disk-backing-chains-raw-inactive.xml | 59 ++++++++++++++ tests/qemuxml2xmltest.c | 2 + 14 files changed, 375 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-backing-chains-raw.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-raw.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-raw.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-raw-active.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-raw-inactive.xml -- 2.0.2

4 17

[libvirt] [PATCH] virsh: desc command in --title mode mentions description instead of title
by Peter Krempa 10 Sep '14

10 Sep '14

Tweak the messages so that they mention "title" rather than "description" when operating in title mode. Also fixes one missing "%s" before non-formatted gettext message. Before: $ virsh desc --title dom No description for domain: dom After: $ virsh desc --title dom No title for domain: dom Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140034 --- tools/virsh-domain.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 86deae6..6aa8631 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7219,7 +7219,9 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) /* Compare original XML with edited. Has it changed at all? */ if (STREQ(desc, desc_edited)) { - vshPrint(ctl, _("Domain description not changed.\n")); + vshPrint(ctl, "%s", + title ? _("Domain title not changed\n") : + _("Domain description not changed\n")); ret = true; goto cleanup; } @@ -7231,10 +7233,13 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) if (virDomainSetMetadata(dom, type, desc, NULL, NULL, flags) < 0) { vshError(ctl, "%s", - _("Failed to set new domain description")); + title ? _("Failed to set new domain title") : + _("Failed to set new domain description")); goto cleanup; } - vshPrint(ctl, "%s", _("Domain description updated successfully")); + vshPrint(ctl, "%s", + title ? _("Domain title updated successfully") : + _("Domain description updated successfully")); } else { desc = vshGetDomainDescription(ctl, dom, title, config?VIR_DOMAIN_XML_INACTIVE:0); @@ -7244,7 +7249,9 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) if (strlen(desc) > 0) vshPrint(ctl, "%s", desc); else - vshPrint(ctl, _("No description for domain: %s"), + vshPrint(ctl, + title ? _("No title for domain: %s") : + _("No description for domain: %s"), virDomainGetName(dom)); } -- 2.0.2

2 1

[libvirt] lxc: shutdown $domain broken in libvirt 1.2.3
by Steven Wilson 10 Sep '14

10 Sep '14

We recently ran into libvirt lxc shutdown failing because of mount namespaces not being available: # virsh -c lxc:/// start test01 Domain test01 started # virsh -c lxc:/// list Id Name State ---------------------------------------------------- 9336 test01 running # virsh -c lxc:/// shutdown test01 error: Failed to shutdown domain test01 error: Mount namespaces are not available on this platform: Function not implemented /var/log/libvirt/libvirtd.log: 2014-09-03 15:50:50.682+0000: 3030: error : virProcessRunInMountNamespace:999 : Mount namespaces are not available on this platform: Function not implemented We're running Debian wheezy with stock libc6 2.13-38, a custom 3.12.15 kernel (with namespace support) and libvirt 1.2.3. The patch in this previous thread fixes shutdown for us and we'd like to see it merged upstream: https://www.redhat.com/archives/libvir-list/2014-May/msg00175.html Thanks, Steve

3 2

[libvirt] [PATCH 0/2] [RFC] Use the power of SystemTap to get rid of all* deadlocks forever*
by Martin Kletzander 10 Sep '14

10 Sep '14

Many moons ago, I wanted to make locking more debuggable from logs. An idea appeared that this might be better off with SystemTap. I let it rot for a while and some time back I had a deadlock that I wanted to try it on. So I started scripting it and here's the work (in progress). Well, the idea is simple. For each mutex that is about to get locked, append it's symname and pid that locked it into the list A, when it get's locked move it to list B and when it's unlocked, remove it. Then whenever the user presses ^C, the script prints all the locks that were locked (with their particular backtraces) and locks that are waiting to be locked. This could be enhanced that it would print both pieces of information only for the locks that are waiting to be acquired. But I don't really care what color the shed will have. Here are some problems I'd love to get any help with: - I cannot run it against built git version, two problems with that: - When I'm not root, staprun needs setuid bit set and that is, of course, incompatible with the needed LD_LIBRARY_PATH. - Even when I am root, I'm unable to get any backtrace that goes beyond virLogMutex() no matter how I'm loading the debug symbols. - When running with installed libvirtd, I am not getting as nice backtrace as I would in gdb (but that's probably the only way how systemtap will get any backtrace so it's better than nothing). And I'm unable to get line numbers even with addr2line. I'd be interested in hearing how this works for others, or any hints on how to make the script so good that I could post it for the inclusion upstream. Any best practices about systemtap scripts are welcomed as well. _______________________________________________________ (*) Not quite, I just wanted you to notice this mail ;) Martin Kletzander (2): lock-debug.stp DO NOT APPLY: deadlock test daemon/libvirtd.c | 1 + examples/systemtap/lock-debug.stp | 115 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 examples/systemtap/lock-debug.stp -- 2.1.0

2 5

[libvirt] [PATCH 0/4] Few unrelated fixes in save/dump code
by Peter Krempa 10 Sep '14

10 Sep '14

I spent some time in the save/dump code today and found a few nits to fix. Peter Krempa (4): util: process: Don't report OOM errors in helper virsh: domain: Clean up handling of "dom" in "save" command qemu: dump: Fix formatting of function headers and code inline qemu: dump: Resume CPUs only when the VM is still alive src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++------------------------ src/util/virprocess.c | 10 +++++----- tools/virsh-domain.c | 6 +++--- 3 files changed, 35 insertions(+), 32 deletions(-) -- 2.0.2

2 6