September 2018 - Devel - libvirt List Archives

[libvirt] [PATCH] qemuProcessBuildDestroyMemoryPathsImpl: Don't overwrite error
by Michal Privoznik 08 Sep '18

08 Sep '18

The qemuSecurityDomainSetPathLabel() function reports perfect error itself. Do not overwrite it to something less meaningful. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_process.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 960c3ed011..eb9904b7ba 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3669,11 +3669,8 @@ qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver, } if (qemuSecurityDomainSetPathLabel(driver->securityManager, - def, path, true) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to label %s"), path); + def, path, true) < 0) return -1; - } } else { if (virFileDeleteTree(path) < 0) return -1; -- 2.16.4

2 1

[libvirt] [PATCH] conf: Fix check for chardev source path
by Andrea Bolognani 08 Sep '18

08 Sep '18

Attempting to use a chardev definition like <serial type='unix'> <target type='isa-serial'/> </serial> correctly results in an error being reported, since the source path - a required piece of information - is missing; however, the very similar <serial type='unix'> <target type='pci-serial'/> </serial> was happily accepted by libvirt, only to result in libvirtd crashing as soon as the guest was started. The issue was caused by checking the chardev's targetType against whitelisted values from virDomainChrChannelTargetType without first checking the chardev's deviceType to make sure it is actually a channel, for which the check makes sense, rather than a different type of chardev. The only reason this wasn't spotted earlier is that the whitelisted values just so happen to correspond to USB and PCI serial devices and Xen and UML consoles respectively, all of which are fairly uncommon. https://bugzilla.redhat.com/show_bug.cgi?id=1609720 Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- I feel compelled to point out that stuff like this wouldn't happen if libvirt was written in Rust O:-) src/conf/domain_conf.c | 8 ++++++-- .../serial-unix-missing-source.xml | 15 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/serial-unix-missing-source.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 86199623cc..6162843028 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5467,10 +5467,14 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *def, break; case VIR_DOMAIN_CHR_TYPE_UNIX: - /* path can be auto generated */ + /* The source path can be auto generated for certain specific + * types of channels, but in most cases we should report an + * error if the user didn't provide it */ if (!def->data.nix.path && (!chr_def || - (chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN && + chr_def->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL || + (chr_def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && + chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN && chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing source path attribute for char device")); diff --git a/tests/qemuxml2argvdata/serial-unix-missing-source.xml b/tests/qemuxml2argvdata/serial-unix-missing-source.xml new file mode 100644 index 0000000000..1e1221f12d --- /dev/null +++ b/tests/qemuxml2argvdata/serial-unix-missing-source.xml @@ -0,0 +1,15 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <serial type='unix'> + <target type='pci-serial'/> + </serial> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 35df63b2ac..949b203998 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1417,6 +1417,7 @@ mymain(void) DO_TEST("serial-unix-chardev", QEMU_CAPS_DEVICE_ISA_SERIAL); DO_TEST_CAPS_LATEST("serial-unix-chardev"); + DO_TEST_PARSE_ERROR("serial-unix-missing-source", NONE); DO_TEST("serial-tcp-chardev", QEMU_CAPS_DEVICE_ISA_SERIAL); DO_TEST("serial-udp-chardev", -- 2.17.1

2 2

[libvirt] [PATCH 0/9] qemu: remove more capabilities
by Ján Tomko 08 Sep '18

08 Sep '18

While reviewing a bootindex-related patch, I found out that we can also drop all the per-device BOOTINDEX capabilities and more. Ján Tomko (9): qemu: always assume QEMU_CAPS_PCI_CONFIGFD qemu: always assume QEMU_CAPS_PCI_BOOTINDEX qemu: drop unused QEMU_CAPS_DEVICE_USB_NET qemu: always assume QEMU_CAPS_USB_REDIR_BOOTINDEX qemu: always assume QEMU_CAPS_USB_HOST_BOOTINDEX qemu: always assume QEMU_CAPS_VFIO_PCI_BOOTINDEX qemu: always assume QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX qemu: remove leftover property probing FIXUP: renumber replies and microcode version Also available on: git://repo.or.cz/libvirt/jtomko.git caps src/qemu/qemu_capabilities.c | 28 -- src/qemu/qemu_capabilities.h | 14 +- src/qemu/qemu_command.c | 48 +-- src/qemu/qemu_hotplug.c | 3 +- .../qemucapabilitiesdata/caps_1.5.3.x86_64.replies | 262 +++-------------- tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 9 +- .../qemucapabilitiesdata/caps_1.6.0.x86_64.replies | 266 +++-------------- tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 9 +- .../qemucapabilitiesdata/caps_1.7.0.x86_64.replies | 266 +++-------------- tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 9 +- .../qemucapabilitiesdata/caps_2.1.1.x86_64.replies | 270 +++-------------- tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 9 +- .../caps_2.10.0.aarch64.replies | 240 +++------------ tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 6 +- .../qemucapabilitiesdata/caps_2.10.0.ppc64.replies | 240 +++------------ tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 6 +- .../qemucapabilitiesdata/caps_2.10.0.s390x.replies | 175 +++-------- tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 4 +- .../caps_2.10.0.x86_64.replies | 326 ++++----------------- tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 9 +- .../qemucapabilitiesdata/caps_2.11.0.s390x.replies | 179 +++-------- tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 4 +- .../caps_2.11.0.x86_64.replies | 272 ++++------------- tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml | 7 +- .../caps_2.12.0.aarch64.replies | 252 ++++------------ tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 6 +- .../qemucapabilitiesdata/caps_2.12.0.ppc64.replies | 252 ++++------------ tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 6 +- .../qemucapabilitiesdata/caps_2.12.0.s390x.replies | 191 ++++-------- tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 4 +- .../caps_2.12.0.x86_64.replies | 288 +++++------------- tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 7 +- .../qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 292 ++++-------------- tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 9 +- .../qemucapabilitiesdata/caps_2.5.0.x86_64.replies | 296 ++++--------------- tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 9 +- .../caps_2.6.0.aarch64.replies | 236 +++------------ tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml | 6 +- .../qemucapabilitiesdata/caps_2.6.0.ppc64.replies | 236 +++------------ tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 6 +- .../qemucapabilitiesdata/caps_2.6.0.x86_64.replies | 296 ++++--------------- tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 9 +- .../qemucapabilitiesdata/caps_2.7.0.s390x.replies | 167 +++-------- tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 4 +- .../qemucapabilitiesdata/caps_2.7.0.x86_64.replies | 305 ++++--------------- tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 9 +- .../qemucapabilitiesdata/caps_2.8.0.s390x.replies | 175 +++-------- tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 4 +- .../qemucapabilitiesdata/caps_2.8.0.x86_64.replies | 305 ++++--------------- tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 9 +- .../qemucapabilitiesdata/caps_2.9.0.ppc64.replies | 179 +++-------- tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 5 +- .../qemucapabilitiesdata/caps_2.9.0.s390x.replies | 175 +++-------- tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 4 +- .../qemucapabilitiesdata/caps_2.9.0.x86_64.replies | 326 ++++----------------- tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 9 +- .../qemucapabilitiesdata/caps_3.0.0.ppc64.replies | 252 ++++------------ tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 6 +- .../caps_3.0.0.riscv32.replies | 167 +++-------- tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml | 1 - .../caps_3.0.0.riscv64.replies | 167 +++-------- tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml | 1 - .../qemucapabilitiesdata/caps_3.0.0.x86_64.replies | 288 +++++------------- tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 7 +- tests/qemuxml2argvtest.c | 9 +- 65 files changed, 1599 insertions(+), 6037 deletions(-) -- 2.16.4

2 17

[libvirt] [PATCH] conf: fix args naming of virDomainChrSourceDefCopy
by Ján Tomko 07 Sep '18

07 Sep '18

Since its introduction in commit 2e37bf42 the naming of the arguments between the prototype and the definition does not match. Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- Pushed as trivial. src/conf/domain_conf.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3ff6eea117..e30a4b2fe7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2889,8 +2889,8 @@ void virDomainNetDefFree(virDomainNetDefPtr def); void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def); void virDomainChrDefFree(virDomainChrDefPtr def); void virDomainChrSourceDefFree(virDomainChrSourceDefPtr def); -int virDomainChrSourceDefCopy(virDomainChrSourceDefPtr src, - virDomainChrSourceDefPtr dest); +int virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest, + virDomainChrSourceDefPtr src); void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def); void virDomainSoundDefFree(virDomainSoundDefPtr def); void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def); -- 2.16.4

1 0

[libvirt] [PATCH] conf: fix bogus error when <boot order='1'/> is in an <interface type='hostdev'>
by Laine Stump 07 Sep '18

07 Sep '18

virDomainDefCollectBootOrder() is called for every item on the list for each type of device. Since an <interface type='hostdev'> is on both the list of hostdev devices and the list of network devices, it will be counted twice, and the code that checks for multiple devices with the same boot order will give a false positive. To remedy this, we make sure to return early for hostdev devices that have a parent.type != NONE. This was introduced in commit 5b75a4, which was first in libvirt-4.4.0. Resolves: https://bugzilla.redhat.com/1601318 Signed-off-by: Laine Stump <laine(a)laine.org> --- src/conf/domain_conf.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 77cc73744f..71a2fb0426 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5062,6 +5062,14 @@ virDomainDefCollectBootOrder(virDomainDefPtr def ATTRIBUTE_UNUSED, if (info->bootIndex == 0) return 0; + if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && + dev->data.hostdev->parent.type != VIR_DOMAIN_DEVICE_NONE) { + /* This hostdev is a child of a higher level device + * (e.g. interface), and thus already being counted on the + * list for the other device type. + */ + return 0; + } if (virAsprintf(&order, "%u", info->bootIndex) < 0) goto cleanup; -- 2.14.4

2 2

[libvirt] [PATCH] qemuDomainNamespaceTeardownHostdev: Drop useless check
by Michal Privoznik 07 Sep '18

07 Sep '18

There is no need to check if @npaths is not zero. Let's qemuDomainNamespaceUnlinkPaths() handle that. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_domain.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 05e90c3615..d43b6978ad 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -12606,8 +12606,7 @@ qemuDomainNamespaceTeardownHostdev(virDomainObjPtr vm, &npaths, &paths, NULL) < 0) goto cleanup; - if (npaths != 0 && - qemuDomainNamespaceUnlinkPaths(vm, (const char **)paths, npaths) < 0) + if (qemuDomainNamespaceUnlinkPaths(vm, (const char **)paths, npaths) < 0) goto cleanup; ret = 0; -- 2.16.4

4 4

[libvirt] [PATCH] nwfilter:fix deadlock when nwfilter reload
by Wang Yechao 07 Sep '18

07 Sep '18

user run "firewalld-cmd --reload" nwfilterStateReload called in main thread step 1. virRWLockWrite(&updateLock) step 2. virNWFilterLoadAllConfigs step 3. virRWLockUnlock(&updateLock); lauch a vm: qemuDomainCreateXML runs in other thread step 1. virRWLockRead(&updateLock); step 2. qemuProcessStart step 3. qemuProcessWaitForMonitor step 4. ... step 5 virRWLockUnlock(&updateLock); if nwfilterStateReload called in the middle of step 1 and step 5 of qemuDomainCreateXML, it can't get the updateLock and then block the event_loop, so event_loop can't handle the qemu-monitor messages, cause deadlock move nwfilterStateReload into thread to fix this problem. Signed-off-by: Wang Yechao <wang.yechao255(a)zte.com.cn> Reviewed-by: Wang Yi <wang.yi59(a)zte.com.cn> --- src/nwfilter/nwfilter_driver.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) mode change 100644 => 100755 src/nwfilter/nwfilter_driver.c diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c old mode 100644 new mode 100755 index ac3a964..2c099e2 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -81,17 +81,28 @@ static void nwfilterDriverUnlock(void) #if HAVE_FIREWALLD +static void nwfilterThread(void *opaque ATTRIBUTE_UNUSED) +{ + nwfilterStateReload(); +} + static DBusHandlerResult nwfilterFirewalldDBusFilter(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, void *user_data ATTRIBUTE_UNUSED) { + virThread thread; + if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, "NameOwnerChanged") || dbus_message_is_signal(message, "org.fedoraproject.FirewallD1", "Reloaded")) { VIR_DEBUG("Reload in nwfilter_driver because of firewalld."); - nwfilterStateReload(); + + /* create thread handle the nwfilter reload, don't block the event_loop.*/ + if (virThreadCreate(&thread, false, nwfilterThread, NULL) < 0) { + VIR_ERROR("create nwfilterThread failed."); + } } return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; -- 1.8.3.1

1 0

[libvirt] [PATCHv2 0/4] Add virNetlinkNewLink for simplifying virNetDev*Create
by Shi Lei 07 Sep '18

07 Sep '18

Patch v1 here: https://www.redhat.com/archives/libvir-list/2018-August/msg01413.html Shi Lei (4): 1/4: Introduce virNetlinkNewLink (Since v1) - Remove callback and just use special-case to handle macvlan in virNetlinkNewLink - Add struct virNetlinkNewLinkData to pack some arguments of virNetlinkNewLink 2/4: Add wrapper macros around nla_nest*/nla_put* to make code more readable (New patch) - Add wrapper macros according to the code written by Erik - Apply these macros to virNetlinkNewLink and virNetDevSetVfConfig and virNetDevVPortProfileOpSetLink 3/4: Using virNetlinkNewLink to simplify virNetDev*Create (Since v1) - Remove the callback for macvlan in virnetdevmacvlan.c 4/4: Remove virNetDevPutExtraHeader and replace with nlmsg_append (New patch) src/libvirt_private.syms | 1 + src/util/virnetdev.c | 98 ++++++++--------------- src/util/virnetdevbridge.c | 78 ++++--------------- src/util/virnetdevmacvlan.c | 109 ++++---------------------- src/util/virnetdevvportprofile.c | 117 +++++++++------------------- src/util/virnetlink.c | 130 +++++++++++++++++++++++++------ src/util/virnetlink.h | 65 ++++++++++++++++ 7 files changed, 270 insertions(+), 328 deletions(-) -- 2.17.1

2 12

[libvirt] [PATCH] nwfilter:fix deadlock when nwfilter reload
by Wang Yechao 07 Sep '18

07 Sep '18

user run "firewalld-cmd --reload" nwfilterStateReload called in main thread step 1. virRWLockWrite(&updateLock) step 2. virNWFilterLoadAllConfigs step 3. virRWLockUnlock(&updateLock); lauch a vm: qemuDomainCreateXML runs in other thread step 1. virRWLockRead(&updateLock); step 2. qemuProcessStart step 3. qemuProcessWaitForMonitor step 4. ... step 5 virRWLockUnlock(&updateLock); if nwfilterStateReload called in the middle of step 1 and step 5 of qemuDomainCreateXML, it can't get the updateLock and then block the event_loop, so event_loop can't handle the qemu-monitor messages, cause deadlock move nwfilterStateReload into thread to fix this problem. Signed-off-by: Wang Yechao <wang.yechao255(a)zte.com.cn> Reviewed-by: Wang Yi <wang.yi59(a)zte.com.cn> --- src/nwfilter/nwfilter_driver.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) mode change 100644 => 100755 src/nwfilter/nwfilter_driver.c diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c old mode 100644 new mode 100755 index ac3a964..2c099e2 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -81,17 +81,28 @@ static void nwfilterDriverUnlock(void) #if HAVE_FIREWALLD +static void nwfilterThread(void *opaque ATTRIBUTE_UNUSED) +{ + nwfilterStateReload(); +} + static DBusHandlerResult nwfilterFirewalldDBusFilter(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, void *user_data ATTRIBUTE_UNUSED) { + virThread thread; + if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, "NameOwnerChanged") || dbus_message_is_signal(message, "org.fedoraproject.FirewallD1", "Reloaded")) { VIR_DEBUG("Reload in nwfilter_driver because of firewalld."); - nwfilterStateReload(); + + /* create thread handle the nwfilter reload, don't block the event_loop.*/ + if (virThreadCreate(&thread, false, nwfilterThread, NULL) < 0) { + VIR_ERROR("create nwfilterThread failed."); + } } return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; -- 1.8.3.1

1 0

[libvirt] [PATCH] qemu: Fix indentation in virDomainDefFormatInternal
by Jiri Denemark 07 Sep '18

07 Sep '18

Broken by v3.10.0-68-gaed3d038a6. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/conf/domain_conf.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 86199623cc..6aa5ff51b4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27582,10 +27582,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, * but no leading indentation before the starting element. * Thankfully, libxml maps what looks like globals into * thread-local uses, so we are thread-safe. */ - xmlIndentTreeOutput = 1; - xmlbuf = xmlBufferCreate(); - if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata, - virBufferGetIndent(buf, false) / 2, 1) < 0) { + xmlIndentTreeOutput = 1; + xmlbuf = xmlBufferCreate(); + if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata, + virBufferGetIndent(buf, false) / 2, 1) < 0) { xmlBufferFree(xmlbuf); xmlIndentTreeOutput = oldIndentTreeOutput; goto error; -- 2.18.0

2 1