September 2017 - Devel - libvirt List Archives

[libvirt] [PATCH] util: Fix stack smashing in virNetDevGetFamilyId
by Jiri Denemark 22 Sep '17

22 Sep '17

After commit 8708ca01c0d libvirtd consistently aborts with "stack smashing detected" when nodedev driver is initialized. Apparently this is caused by nlmsg_parse trying to write more than CTRL_ATTR_MAX bytes in tb because it is told tb can accommodate CTRL_CMD_MAX bytes. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/util/virnetdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 41a659732b..be8f4b7938 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -3158,7 +3158,7 @@ virNetDevGetFamilyId(const char *family_name) struct nl_msg *nl_msg = NULL; struct nlmsghdr *resp = NULL; struct genlmsghdr* gmsgh = NULL; - struct nlattr *tb[CTRL_ATTR_MAX + 1] = {NULL, }; + struct nlattr *tb[CTRL_CMD_MAX + 1] = {NULL, }; unsigned int recvbuflen; uint32_t family_id = 0; -- 2.14.1

4 3

[libvirt] [PATCH] qemuBuildHostNetStr: Don't leak @addr
by Michal Privoznik 22 Sep '17

22 Sep '17

The virSocketAddrFormat() allocates the string and it's caller responsibility to free it afterwards. ==28857== 11 bytes in 1 blocks are definitely lost in loss record 37 of 168 ==28857== at 0x4C2BEDF: malloc (vg_replace_malloc.c:299) ==28857== by 0x9A81D79: strdup (in /lib64/libc-2.23.so) ==28857== by 0x5DA3BF0: virStrdup (virstring.c:902) ==28857== by 0x5D96182: virSocketAddrFormatFull (virsocketaddr.c:427) ==28857== by 0x5D95E13: virSocketAddrFormat (virsocketaddr.c:352) ==28857== by 0x5706890: qemuBuildHostNetStr (qemu_command.c:3891) ==28857== by 0x57138D3: qemuBuildInterfaceCommandLine (qemu_command.c:8597) ==28857== by 0x5713D6A: qemuBuildNetCommandLine (qemu_command.c:8699) ==28857== by 0x57176F6: qemuBuildCommandLine (qemu_command.c:10027) ==28857== by 0x5769D61: qemuProcessCreatePretendCmd (qemu_process.c:6004) ==28857== by 0x4056EC: testCompareXMLToArgv (qemuxml2argvtest.c:502) ==28857== by 0x41DF40: virTestRun (testutils.c:180) Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_command.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9b3e3fc04..abeb24846 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3900,6 +3900,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, if (ip->prefix) virBufferAsprintf(&buf, "/%u", ip->prefix); virBufferAddChar(&buf, ','); + VIR_FREE(addr); } break; -- 2.13.5

5 7

[libvirt] [PATCH v2] qemu: Introduce a wrapper over virFileWrapperFdClose
by Michal Privoznik 22 Sep '17

22 Sep '17

https://bugzilla.redhat.com/show_bug.cgi?id=1448268 When migrating to a file (e.g. when doing 'virsh save file'), couple of things are happening in the thread that is executing the API: 1) the domain obj is locked 2) iohelper is spawned as a separate process to handle all I/O 3) the thread waits for iohelper to finish 4) the domain obj is unlocked Now, the problem is that while the thread waits in step 3 for iohelper to finish this may take ages because iohelper calls fdatasync(). And unfortunately, we are waiting the whole time with the domain locked. So if another thread wants to jump in and say copy the domain name ('virsh list' for instance), they are stuck. The solution is to unlock the domain whenever waiting for I/O and lock it back again when it finished. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- diff to v1: - check after locking the domain obj back if the domain is still alive, since it was unlocked it might have changed its state. src/qemu/qemu_driver.c | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e1a0dd553..6095a5ec5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3216,6 +3216,31 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid, goto cleanup; } + +static int +qemuFileWrapperFDClose(virDomainObjPtr vm, + virFileWrapperFdPtr fd) +{ + int ret; + + /* virFileWrapperFd uses iohelper to write data onto disk. + * However, iohelper calls fdatasync() which may take ages to + * finish. Therefore, we shouldn't be waiting with the domain + * object locked. */ + + virObjectUnlock(vm); + ret = virFileWrapperFdClose(fd); + virObjectLock(vm); + if (!virDomainObjIsActive(vm)) { + if (!virGetLastError()) + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("domain is no longer running")); + ret = -1; + } + return ret; +} + + /* Helper function to execute a migration to file with a correct save header * the caller needs to make sure that the processors are stopped and do all other * actions besides saving memory */ @@ -3276,7 +3301,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, goto cleanup; } - if (virFileWrapperFdClose(wrapperFd) < 0) + if (qemuFileWrapperFDClose(vm, wrapperFd) < 0) goto cleanup; if ((fd = qemuOpenFile(driver, vm, path, O_WRONLY, NULL, NULL)) < 0 || @@ -3827,7 +3852,7 @@ doCoreDump(virQEMUDriverPtr driver, path); goto cleanup; } - if (virFileWrapperFdClose(wrapperFd) < 0) + if (qemuFileWrapperFDClose(vm, wrapperFd) < 0) goto cleanup; ret = 0; @@ -6687,7 +6712,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, data, path, false, QEMU_ASYNC_JOB_START); - if (virFileWrapperFdClose(wrapperFd) < 0) + if (qemuFileWrapperFDClose(vm, wrapperFd) < 0) VIR_WARN("Failed to close %s", path); qemuProcessEndJob(driver, vm); @@ -6962,7 +6987,7 @@ qemuDomainObjRestore(virConnectPtr conn, ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, data, path, start_paused, asyncJob); - if (virFileWrapperFdClose(wrapperFd) < 0) + if (qemuFileWrapperFDClose(vm, wrapperFd) < 0) VIR_WARN("Failed to close %s", path); cleanup: -- 2.13.5

2 2

[libvirt] [PATCH 0/2] qemu: Order PCI controllers on the command line
by Andrea Bolognani 21 Sep '17

21 Sep '17

All the details are in the commit message for patch 2/2. Andrea Bolognani (2): qemu: Clean up qemuBuildControllerDevCommandLine() qemu: Order PCI controllers on the command line src/qemu/qemu_command.c | 91 +++++++++++++++------- .../qemuxml2argv-pci-autoadd-idx.args | 2 +- .../qemuxml2argv-pseries-many-buses-2.args | 4 +- 3 files changed, 68 insertions(+), 29 deletions(-) -- 2.13.5

3 8

[libvirt] [PATCH 0/6] Fix guest CPU updates for inactive domains
by Jiri Denemark 21 Sep '17

21 Sep '17

This series originally started as a cleanup work, but it turned out two real bugs got fixed with this cleanup :-) Since commit v2.2.0-199-g7ce711a30e libvirt stores an updated guest CPU in domain's live definition and there's no need to update it every time we want to format the definition. Thus libvirt should never internally request a guest CPU update when formatting domain XML. But if it is asked by a user to do so, it should use the same host CPU model which is used when starting a domain to provide meaningful results. https://bugzilla.redhat.com/show_bug.cgi?id=1481309 https://bugzilla.redhat.com/show_bug.cgi?id=1485022 Jiri Denemark (6): qemuxml2xmltest: Add tests for Power CPUs cpu_conf: Drop updateCPU from virCPUDefFormat cpu_conf: Simplify formatting of guest CPU attributes conf: Drop unused VIR_DOMAIN_DEF_FORMAT_UPDATE_CPU qemu: Use correct host model for updating guest cpu qemu: Don't update CPU when formatting live def src/bhyve/bhyve_driver.c | 2 +- src/conf/capabilities.c | 2 +- src/conf/cpu_conf.c | 38 +++++++++------------ src/conf/cpu_conf.h | 9 ++--- src/conf/domain_capabilities.c | 2 +- src/conf/domain_conf.c | 6 +--- src/conf/domain_conf.h | 1 - src/conf/snapshot_conf.c | 3 +- src/libxl/libxl_driver.c | 2 +- src/qemu/qemu_domain.c | 17 +++++++--- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_driver.c | 9 ++++- src/qemu/qemu_migration_cookie.c | 2 +- src/test/test_driver.c | 2 +- src/vz/vz_driver.c | 2 +- tests/cputest.c | 15 ++++----- .../ppc64-host+guest-compat-incompatible.xml | 2 +- .../ppc64-host+guest-compat-invalid.xml | 2 +- .../cputestdata/ppc64-host+guest-compat-valid.xml | 2 +- .../qemuxml2xmlout-pseries-cpu-compat-power9.xml | 38 +++++++++++++++++++++ .../qemuxml2xmlout-pseries-cpu-compat.xml | 38 +++++++++++++++++++++ .../qemuxml2xmlout-pseries-cpu-exact.xml | 39 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 4 +++ 23 files changed, 178 insertions(+), 62 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat-power9.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml -- 2.14.1

2 9

[libvirt] [PATCH 0/5] Reject parallel ports on s390, and pseries
by Pino Toscano 21 Sep '17

21 Sep '17

Hi, this is a simple series to reject parallel ports on s390 architectures, and pseries machines -- both simply do not support them. This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1487499 Pino Toscano (5): tests: qemuxml2argv: fix expected type for usb-bus-missing tests: qemuxml2argv: fail also on unexpected pass qemu: pass the virDomainDef to qemuDomainChrDefValidate qemu: reject parallel ports for s390 archs qemu: reject parallel ports for pseries machines src/qemu/qemu_domain.c | 16 ++++++++++++---- .../qemuxml2argv-pseries-no-parallel.xml | 18 ++++++++++++++++++ .../qemuxml2argv-s390-no-parallel.xml | 22 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 20 +++++++++++++++++--- 4 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml -- 2.13.5

4 10

[libvirt] [PATCH] qemu: capabilities: Remove support for downstream-only QMP monitor backport
by Peter Krempa 21 Sep '17

21 Sep '17

Some distros (see diff) chose to backport QMP support rather than rebase to newer version of qemu. As a hack they added the string 'libvirt' to the qemu -help output. Remove this as downstream-only hacks should be carried by downstream and not litter upstream. This effectively reverts commit ff88cd590572277f10ecee4ebb1174d9b70fc0d7 --- src/qemu/qemu_capabilities.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7bfb71420..085910dd4 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1335,8 +1335,7 @@ virQEMUCapsComputeCmdFlags(const char *help, if ((netdev = strstr(help, "-netdev"))) { /* Disable -netdev on 0.12 since although it exists, * the corresponding netdev_add/remove monitor commands - * do not, and we need them to be able to do hotplug. - * But see below about RHEL build. */ + * do not, and we need them to be able to do hotplug. */ if (version >= 13000) { if (strstr(netdev, "bridge")) virQEMUCapsSet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE); @@ -1369,20 +1368,10 @@ virQEMUCapsComputeCmdFlags(const char *help, /* While JSON mode was available in 0.12.0, it was too * incomplete to contemplate using. The 0.13.0 release * is good enough to use, even though it lacks one or - * two features. This is also true of versions of qemu - * built for RHEL, labeled 0.12.1, but with extra text - * in the help output that mentions that features were - * backported for libvirt. The benefits of JSON mode now - * outweigh the downside. - */ + * two features. */ #if WITH_YAJL - if (version >= 13000) { - virQEMUCapsSet(qemuCaps, QEMU_CAPS_MONITOR_JSON); - } else if (version >= 12000 && - strstr(help, "libvirt")) { + if (version >= 13000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MONITOR_JSON); - virQEMUCapsSet(qemuCaps, QEMU_CAPS_NETDEV); - } #else /* Starting with qemu 0.15 and newer, upstream qemu no longer * promises to keep the human interface stable, but requests that @@ -1392,8 +1381,7 @@ virQEMUCapsComputeCmdFlags(const char *help, * telling them to recompile (the spec file includes the * dependency, so distros won't hit this). This check is * also in m4/virt-yajl.m4 (see $with_yajl). */ - if (version >= 15000 || - (version >= 12000 && strstr(help, "libvirt"))) { + if (version >= 15000) { if (check_yajl) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("this qemu binary requires libvirt to be " -- 2.13.5

2 1

[libvirt] [PATCH] Fix commandhelper build on win32
by Daniel P. Berrange 21 Sep '17

21 Sep '17

For win32 we need EXIT_AM_SKIP which is in testutils.h. We must define NO_LIBVIRT to prevent replacement of fprintf with virFilePrintf as we can't link to libvirt_util.la Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- Pushed as a build fix tests/commandhelper.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/commandhelper.c b/tests/commandhelper.c index e9e6353f3..1da2834aa 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -28,6 +28,8 @@ #include <sys/stat.h> #include "internal.h" +#define NO_LIBVIRT +#include "testutils.h" #ifndef WIN32 -- 2.13.5

1 0

[libvirt] [PATCH libvirt-glib 0/5] Portability fixes
by Andrea Bolognani 21 Sep '17

21 Sep '17

Currently, trying to build libvirt-glib on FreeBSD results in all sorts of interesting failures. After applying this series, it's possible to build libvirt-glib and run the test suite successfully. There's even a couple of clean-ups thrown in for good measure :) Andrea Bolognani (5): maint: Drop autobuild.sh configure: Bump required libvirt version to 1.2.5 configure: Look for Perl interpreter path tests: Don't rely on non-portable paths scripts: Fix sha-bang lines autobuild.sh | 91 ------------------------------------------- build-aux/check-symfile.pl | 2 +- build-aux/check-symsorting.pl | 2 +- configure.ac | 4 +- examples/config-demo.py | 2 +- examples/conn-test.js | 2 +- examples/conn-test.py | 2 +- examples/event-test.py | 1 + tap-driver.sh | 2 +- tap-test | 2 +- tests/test-events.c | 2 +- 11 files changed, 12 insertions(+), 100 deletions(-) delete mode 100755 autobuild.sh -- 2.13.5

2 6

[libvirt] Questions about function virPCIDeviceIsBehindSwitchLackingACS in virpci.c
by Wuzongyong (Euler Dept) 21 Sep '17

21 Sep '17

Hi, In function virPCIDeviceIsBehindSwitchLackingACS, I noticed that(line 8): 1 if (virPCIDeviceGetParent(dev, &parent) < 0) 2 return -1; 3 if (!parent) { 4 /* if we have no parent, and this is the root bus, ACS doesn't come 5 * into play since devices on the root bus can't P2P without going 6 * through the root IOMMU. 7 */ 8 if (dev->address.bus == 0) { 9 return 0; 10 } else { 11 virReportError(VIR_ERR_INTERNAL_ERROR, 12 _("Failed to find parent device for %s"), 13 dev->name); 14 return -1; 15 } 16 } Why we just return 0 only if device's bus is 0? In my server, I can see a root bus which bus number is greater than 0, see the results(just a part) after I run lspci -t: +-[0000:80]-+-02.0-[81-83]--+-00.0 | | \-00.1 | +-05.0 | +-05.1 | +-05.2 | \-05.4 +-[0000:7f]-+-08.0 | +-08.2 | +-08.3 | + . . . | \-1f.2 \-[0000:00]-+-00.0 +-01.0-[01]----00.0 +-02.0-[02]--+-00.0 | +-00.1 | +-00.2 | \-00.3 +-02.2-[03]-- +-03.0-[04-0b]----00.0-[05-0b]--+-08.0-[06-08]----00.0 | \-10.0-[09-0b]----00.0 +-05.0 +-05.1 +-05.2 +-05.4 +-11.0 +-11.4 +-16.0 +-16.1 +-1a.0 If I assign the device 0000:81:00.0 to a VM, I get "Failed to find parent device". I think I should get no error with return value 0 just like bus number is 0, because bus 80 is the root bus as well in my case. In the <<Intel C610 Series Chipset and Intel X99 Chipset Platform Controller Hub(PCH)>> Datasheet, I found that(Chapter 9.1): For some server platforms, it may be desirable to have multiple PCHs in the system Which means some PCH's may reside on a bus greater than 0. So, is this a bug? Thanks, Zongyong Wu

4 6