August 2018 - Devel - libvirt List Archives

[libvirt] [PATCH] rpc: fix handling of SSH auth failure code
by Daniel P. Berrangé 31 Aug '18

31 Aug '18

The result of libssh2_userauth_password is being assigned to 'ret' in one branch and 'rc' in the other branch. Checks are all done against the 'ret' variable, so one branch never does the correct check. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/rpc/virnetsshsession.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index 35dc6c5356..54c3a6f7aa 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -706,9 +706,9 @@ virNetSSHAuthenticatePassword(virNetSSHSessionPtr sess, if (priv->password) { /* tunelled password authentication */ - if ((ret = libssh2_userauth_password(sess->session, - priv->username, - priv->password)) == 0) { + if ((rc = libssh2_userauth_password(sess->session, + priv->username, + priv->password)) == 0) { ret = 0; goto cleanup; } @@ -737,7 +737,7 @@ virNetSSHAuthenticatePassword(virNetSSHSessionPtr sess, goto cleanup; } - if (ret != LIBSSH2_ERROR_AUTHENTICATION_FAILED) + if (rc != LIBSSH2_ERROR_AUTHENTICATION_FAILED) break; VIR_FREE(password); @@ -750,7 +750,7 @@ virNetSSHAuthenticatePassword(virNetSSHSessionPtr sess, _("authentication failed: %s"), errmsg); /* determine exist status */ - if (ret == LIBSSH2_ERROR_AUTHENTICATION_FAILED) + if (rc == LIBSSH2_ERROR_AUTHENTICATION_FAILED) return 1; else return -1; -- 2.17.1

2 2

[libvirt] [PATCH] qemu: mdev: Set vfio device property 'display' to off only if mdev is vfio-pci
by Farhan Ali 31 Aug '18

31 Aug '18

S390 is aware of both vfio-pci and vfio-ccw devices, so on S390 the capability QEMU_CAPS_VFIO_PCI_DISPLAY will be available. Add an extra check to make sure we only set the display to off for vfio-pci mediated devices. Otherwise we add display for vfio-ccw device and this breaks vfio-ccw device qemu command line. Fixes: d54e45b6e conf: Introduce new <hostdev> attribute 'display' Signed-off-by: Farhan Ali <alifm(a)linux.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay(a)linux.ibm.com> --- src/qemu/qemu_domain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 22c6d8f..df7e937 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6375,6 +6375,7 @@ qemuDomainHostdevDefMdevPostParse(virDomainHostdevSubsysMediatedDevPtr mdevsrc, /* QEMU 2.12 added support for vfio-pci display type, we default to * 'display=off' to stay safe from future changes */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_PCI_DISPLAY) && + mdevsrc->model == VIR_MDEV_MODEL_TYPE_VFIO_PCI && mdevsrc->display == VIR_TRISTATE_SWITCH_ABSENT) mdevsrc->display = VIR_TRISTATE_SWITCH_OFF; -- 2.7.4

2 1

[libvirt] [PATCH 0/5] JSON test improvements
by Ján Tomko 31 Aug '18

31 Aug '18

{ "BLURB": "HERE" } Ján Tomko (5): virjsontest: store name in testInfo virjsontest: use name instead of doc for deflatten test virjsontest: use the test name in AddRemove test Test parsing of large numbers in JSON tests: pass ULLONG_MAX to qemuMonitorJSONGetBalloonInfo tests/qemumonitorjsontest.c | 4 ++-- tests/virjsontest.c | 22 +++++++++++++--------- 2 files changed, 15 insertions(+), 11 deletions(-) -- 2.16.4

2 10

[libvirt] [PATCH v2 0/2] nwfilter: Fix a couple of session mode issues
by John Ferlan 30 Aug '18

30 Aug '18

v1: https://www.redhat.com/archives/libvir-list/2018-August/msg01464.html Changes in v2 - different approach as review pointed out we should never open the nwfilter driver in session mode (although driver initialization does set up some barebones list infrastructure). First, let's make sure we don't allow creation of the nwfilter filter binding similar to how nwfiler filter creation is not allowed. Second, rather than blindly open the nwfilter during the teardown processing, let's first ensure a filter exists for the network. It's not possible to call instantiation when net->filter == NULL. Rather than alter all the callers, just alter the two teardown API's to check if !net->filter and return prior to opening the nwfilter connection. Since we cannot create a filter nor can we create a binding, this filtering works. Keeps the changes minimal too. John Ferlan (2): nwfilter: Disallow binding creation in session mode nwfilter: Check for filter presence before open connect during teardown src/conf/domain_nwfilter.c | 22 +++++++++++++++------- src/nwfilter/nwfilter_driver.c | 6 ++++++ 2 files changed, 21 insertions(+), 7 deletions(-) -- 2.17.1

2 6

[libvirt] [PATCH] storage: Fix mention of disk pool default
by Eric Blake 30 Aug '18

30 Aug '18

The default disk storage pool type is 'dos', not 'msdos'. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- I ran into this doc bug when trying to figure out why a disk storage pool that I had copied from another machine wouldn't autostart; it turns out that the old disk used BIOS partitioning (dos), and the new one uses UEFI (gpt). I wonder if it would be nicer if a disk pool without a default partition type supplied by the user could auto-detect the right format, rather than defaulting to dos, but that's a bigger question that I won't be tackling, and wouldn't have necessarily helped me (if the <target> element was already present when I copied the file). docs/storage.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/storage.html.in b/docs/storage.html.in index e9e6ec7423..6ec623831c 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -310,7 +310,7 @@ on the size and placement of volumes. The 'free extents' information will detail the regions which are available for creating new volumes. A volume cannot span across 2 different free extents. - It will default to using <code>msdos</code> as the pool source format. + It will default to using <code>dos</code> as the pool source format. </p> <h3>Example pool input</h3> -- 2.17.1

2 3

[libvirt] [PATCH] qemuDomainAttachNetDevice: use only one virErrorPtr variable
by Ján Tomko 30 Aug '18

30 Aug '18

Commit f7b5566 added 'save_error' even though the function already has 'originalError' used in the 'try_remove' section. Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- src/qemu/qemu_hotplug.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4f290b5648..a30cb1f1a0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1205,7 +1205,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, bool charDevPlugged = false; bool netdevPlugged = false; char *netdev_name; - virErrorPtr save_error = NULL; /* preallocate new slot for device */ if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0) @@ -1487,9 +1486,9 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, qemuDomainReleaseDeviceAddress(vm, &net->info, NULL); if (iface_connected) { - virErrorPreserveLast(&save_error); + virErrorPreserveLast(&originalError); virDomainConfNWFilterTeardown(net); - virErrorRestore(&save_error); + virErrorRestore(&originalError); if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) { ignore_value(virNetDevMacVLanDeleteWithVPortProfile( -- 2.16.4

2 1

[libvirt] [PATCH] virDomainNetDefCheckABIStability: Check for MTU change too
by Michal Privoznik 30 Aug '18

30 Aug '18

https://bugzilla.redhat.com/show_bug.cgi?id=1623157 Changing MTU on a running guest is not possible and trying to do so made us face many problems. That's why we forbid it in 5f44d7e357f61f7. However, there is still one possible path where users can sneak in change: migration XML. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/conf/domain_conf.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 38cac07913..59ca9f5888 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21901,6 +21901,13 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; } + if (src->mtu != dst->mtu) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target network card MTU %d does not match source %d"), + dst->mtu, src->mtu); + return false; + } + if (src->virtio && dst->virtio && !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) return false; -- 2.16.4

2 1

[libvirt] [PATCH] qemuDomainSnapshotCreateActiveExternal: Remove memory snapshot on failure less frequently
by Michal Privoznik 30 Aug '18

30 Aug '18

https://bugzilla.redhat.com/show_bug.cgi?id=1589115 When creating a memory snapshot the domain is suspended and qemu is told to dump memory into the desired file. After that we set a flag so that the file is not left behind if a failure occurs at some later point (e.g. when creating disk snapshot fails). However, the way we currently handle the memory snapshot file is way too aggressive. For instance, if resuming domain vCPUs fails after all snapshots were done successfully, we still remove the memory snapshot (even though we leave disk snapshot be). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a0f7c71675..2eae3dd49d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15248,6 +15248,7 @@ qemuDomainSnapshotCreateActiveExternal(virQEMUDriverPtr driver, virObjectEventStateQueue(driver->domainEventState, event); } + memory_unlink = false; ret = 0; cleanup: -- 2.16.4

2 3

[libvirt] Entering freeze for libvirt-4.7.0
by Daniel Veillard 30 Aug '18

30 Aug '18

Sorry I forgot to announce it but we are the last week of the month, so it's about time, I tagged RC1 in git and pushed the signed tarball and rpms to the usual place earlier today: ftp://libvirt.org/libvirt/ this seems to work well in my limited testing but as usual we need more review and check on other platforms or OSes before the release. In general https://ci.centos.org/view/libvirt/ is green except for glib bindings and for the master build (weird!) I will likely push the rc2 Thursday morning, but I mwill be travelling at the end of the week so that may depends on conditions, and hopefully the release during this w.e. thanks for giving it a try ! Daniel -- Daniel Veillard | Red Hat Developers Tools http://developer.redhat.com/ veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

2 3

[libvirt] [PATCH for v4.7.0] virDomainDefCompatibleDevice: Relax alias change check
by Michal Privoznik 29 Aug '18

29 Aug '18

https://bugzilla.redhat.com/show_bug.cgi?id=1621910 When introducing this check back in 4ad54a417a1 my mindset was that if an element is missing in update XML then user is requesting for removal of the corresponding setting. For instance, if <bandwidth/> is not present in update XML any QoS previously set on <interface/> is cleared out. Well this assumption is correct but only to some extent. Turns out, we have some lazy users who when updating path to ISO image construct very minimalistic disk XML and pass it to device update API. Such XML is lacking a lot of information, and alias is one of them. This triggers error in virDomainDefCompatibleDevice() because we think that user is requesting to remove the alias. Well, they are not. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/conf/domain_conf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 38cac07913..603a4ad652 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28785,9 +28785,9 @@ virDomainDefCompatibleDevice(virDomainDefPtr def, if (action == VIR_DOMAIN_DEVICE_ACTION_UPDATE && live && - ((!!data.newInfo != !!data.oldInfo) || - (data.newInfo && data.oldInfo && - STRNEQ_NULLABLE(data.newInfo->alias, data.oldInfo->alias)))) { + (data.newInfo && data.oldInfo && + data.newInfo->alias && data.oldInfo->alias && + STRNEQ(data.newInfo->alias, data.oldInfo->alias))) { virReportError(VIR_ERR_OPERATION_DENIED, "%s", _("changing device alias is not allowed")); return -1; -- 2.16.4

2 1