- Devel - libvirt List Archives

[libvirt] backingStore info adding late breaks virt-aa-helper
by Christian Ehrhardt 18 Jul '17

18 Jul '17

Hi, there is a behavioral change I try to track down that affects virt-aa-helper. TL;DR: - it seems backingStore info gets added "later" in recent versions which causes issues in virt-aa-helper Details: For a guest containing a qcow2 disk like this: <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow'/> <target dev='vda' bus='virtio'/> </disk> And said qcow disk having a backing file: $ qemu-img info /var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow image: /var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow [...] backing file: /var/lib/uvtool/libvirt/images/x-uvt-b64-Y29tLnVidW50dS5jbG91ZC5kYWlseTpzZXJ2ZXI6MTcuMTA6cHBjNjRlbCAyMDE3MDcxMw== Now when instantiating the guest this gets the backingStore info added like: <backingStore type='file' index='1'> <format type='qcow2'/> <source file='/var/lib/uvtool/libvirt/images/x-uvt-b64-Y29tLnVidW50dS5jbG91ZC5kYWlseTpzZXJ2ZXI6MTcuMTA6cHBjNjRlbCAyMDE3MDcxMw=='/> <backingStore/> </backingStore> But this now seems to come in "too late" for virt-aa-helper. That tool is reading the guest definition to create custom rules for that guest that opens up the apparmor profile. And in relation to the devices the following in src/security/virt-aa-helper.c is the important part: Loops over disks and in those "down" the chain of backing stores: 929 for (i = 0; i < ctl->def->ndisks; i++) { [...] 947 if (virDomainDiskDefForeachPath(disk, true, add_file_path, &buf) < 0) If you pass virt-aa-helper as in libvirt 3.5 a full snippet with backingStore info it behaves the same as back in 2.5 emmitting a rule for the backing store. But when starting a guest on libvirt 3.5 this does no more work, so it seems that on instantiating the guest Past (2.5) 1. add backingStore info to guest representation 2. virt-aa-helper parses guest representation and creates rules 3. guest starts fine changed to now (3.5): 1. virt-aa-helper parses guest representation and creates rules 2. add backingStore info to guest representation 3. guest fails to start as the apparmor rule to allow it access to its backing file is missing. I've verified that recent libvirt properly adds the backingStore eventually (by disabling the apparmor profile and then starting the guest). Once fully started the live xml representation has the backing store info added. But as outlined above, at the point virt-aa-helper runs now the necessary backingStore data seems to be missing. I couldn't find the related change or a way to fix it so far, so any hints are welcome. -- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd

2 5

[libvirt] [PATCH v4 0/4] Host device isolation for pSeries guests
by Andrea Bolognani 17 Jul '17

17 Jul '17

Changes from [v3]: * correctly handle interfaces connected to hostdev-backed networks; * drop patches implementing support for multiple PHBs, as they have been merged already; * some minor cleanups. Changes from [v2]: * support hot(un)plug properly; * add documentation. Changes from [v1]: * address review comments; * implement a much better isolation algorithm that doesn't require parsing and formatting the isolation group and can handle more dynamic scenarios, such as empty PHBs changing their isolation groups to accomodate hotplugged hostdevs; * add more test cases. [v3] https://www.redhat.com/archives/libvir-list/2017-June/msg01018.html [v2] https://www.redhat.com/archives/libvir-list/2017-June/msg00695.html [v1] https://www.redhat.com/archives/libvir-list/2017-June/msg00110.html Andrea Bolognani (4): conf: Introduce isolation groups conf: Implement isolation rules qemu: Isolate hostdevs on pSeries guests news: Update for hostdev isolation docs/news.xml | 10 + src/bhyve/bhyve_device.c | 4 +- src/conf/device_conf.h | 10 + src/conf/domain_addr.c | 86 ++++++- src/conf/domain_addr.h | 12 +- src/conf/domain_conf.c | 2 + src/qemu/qemu_domain_address.c | 276 +++++++++++++++++++-- src/qemu/qemu_domain_address.h | 4 + src/qemu/qemu_hotplug.c | 7 + tests/qemumemlocktest.c | 2 +- .../qemuxml2argv-pseries-hostdevs-1.args | 8 +- .../qemuxml2argv-pseries-hostdevs-2.args | 3 +- .../qemuxml2argv-pseries-hostdevs-3.args | 2 +- .../qemuxml2xmlout-pseries-hostdevs-1.xml | 14 +- .../qemuxml2xmlout-pseries-hostdevs-2.xml | 6 +- .../qemuxml2xmlout-pseries-hostdevs-3.xml | 2 +- 16 files changed, 410 insertions(+), 38 deletions(-) -- 2.7.5

2 8

[libvirt] [v2 RESEND PATCH] qemu: reduce packet loss rate for vm with macvtap passthrough mode in migration
by ZhiPeng Lu 17 Jul '17

17 Jul '17

Before libvirt that calls virNetDevMacVLanCreateWithVPortProfile sets mac address or vlan of a Virtual Function(VF) linked to a macvtap passthrough device of migration destination host in migration start step. If we ping the migrating vm, we get the network does not pass. Because VFs of migration source and destination have the same MAC address. The patch later calling qemuMigrationVPAssociatePortProfiles sets mac address of VF in migration finish step instead of start step. The patch aims to reduce packet loss rate. Signed-off-by: ZhiPeng Lu <lu.zhipeng(a)zte.com.cn> --- src/qemu/qemu_migration.c | 18 ++++++++++++++++-- src/util/virnetdevmacvlan.c | 17 +++++++++++------ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 09adb04..795ed71 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5004,7 +5004,7 @@ qemuMigrationPerform(virQEMUDriverPtr driver, } static int -qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) +qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def, const char *stateDir) { size_t i; int last_good_net = -1; @@ -5013,6 +5013,20 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) for (i = 0; i < def->nnets; i++) { net = def->nets[i]; if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) { + if ((!virDomainNetGetActualVirtPortProfile(net) || (virDomainNetGetActualVirtPortProfile(net) && + virDomainNetGetActualVirtPortProfile(net)->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBG && + virDomainNetGetActualVirtPortProfile(net)->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBH)) && + virDomainNetGetActualDirectMode(net) == + VIR_NETDEV_MACVLAN_MODE_PASSTHRU) { + if (virNetDevSaveNetConfig(virDomainNetGetActualDirectDev(net), + -1, stateDir, false) < 0) { + goto err_exit; + } + if (virNetDevSetNetConfig(virDomainNetGetActualDirectDev(net), + -1, NULL, virDomainNetGetActualVlan(net), &net->mac, false) < 0) { + goto err_exit; + } + } if (virNetDevVPortProfileAssociate(net->ifname, virDomainNetGetActualVirtPortProfile(net), &net->mac, @@ -5187,7 +5201,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, goto endjob; } - if (qemuMigrationVPAssociatePortProfiles(vm->def) < 0) + if (qemuMigrationVPAssociatePortProfiles(vm->def, cfg->stateDir) < 0) goto endjob; if (mig->network && qemuDomainMigrateOPDRelocate(driver, vm, mig) < 0) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 7222b0f..682dcd1 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -1020,12 +1020,17 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested, */ setVlan = false; } - - if (virNetDevSaveNetConfig(linkdev, -1, stateDir, setVlan) < 0) - return -1; - - if (virNetDevSetNetConfig(linkdev, -1, NULL, vlan, macaddress, setVlan) < 0) - return -1; + if (vmOp != VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START && + virtPortProfile && (virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_8021QBH || + virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_8021QBG)) { + if (virNetDevSaveNetConfig(linkdev, -1, stateDir, setVlan) < 0) { + return -1; + } + if (virNetDevSetNetConfig(linkdev, -1, NULL, vlan, macaddress, + setVlan) < 0) { + return -1; + } + } } if (ifnameRequested) { -- 1.8.3.1

3 2

[libvirt] [PATCH] Revert "nwfilter: Move save of config until after successful assign"
by John Ferlan 17 Jul '17

17 Jul '17

This reverts commit b3e71a8830b2683ee88fa10cb048eabb99a446c0. As it turns out this ends up very badly as the @def could be Free'd even though it's owned by @obj as a result of the AssignDef. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/conf/virnwfilterobj.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index b5aaa6b..b36eda1 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -501,14 +501,14 @@ virNWFilterObjListLoadConfig(virNWFilterObjListPtr nwfilters, goto error; } - if (!(obj = virNWFilterObjListAssignDef(nwfilters, def))) - goto error; - /* We generated a UUID, make it permanent by saving the config to disk */ if (!def->uuid_specified && virNWFilterSaveConfig(configDir, def) < 0) goto error; + if (!(obj = virNWFilterObjListAssignDef(nwfilters, def))) + goto error; + VIR_FREE(configFile); return obj; -- 2.9.4

2 4

[libvirt] [PATCH] security: Use VIR_DEBUG instead of VIR_INFO in virSecurityDACSetOwnershipInternal
by xinhua.Cao 17 Jul '17

17 Jul '17

virSecurityDACSetOwnershipInternal was called by libvirt child process, so if we log message by VIR_INFO at normal scene, it would probability occurs dead lock sence, then libvirtd will also by dead lock because libvirtd is waitting for child message. so our suggest is use VIR_DEBUG instead of VIR_INFO to avoid this sence. --- src/security/security_dac.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index ca7a6af..7bfd090 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -552,8 +552,8 @@ virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv, else if (rc > 0) return 0; - VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'", - NULLSTR(src ? src->path : path), (long) uid, (long) gid); + VIR_DEBUG("Setting DAC user and group on '%s' to '%ld:%ld'", + NULLSTR(src ? src->path : path), (long) uid, (long) gid); if (priv && src && priv->chownCallback) { rc = priv->chownCallback(src, uid, gid); @@ -591,17 +591,17 @@ virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv, if (rc < 0) { if (errno == EOPNOTSUPP || errno == EINVAL) { - VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not " - "supported by filesystem", - (long) uid, (long) gid, path); + VIR_DEBUG("Setting user and group to '%ld:%ld' on '%s' not " + "supported by filesystem", + (long) uid, (long) gid, path); } else if (errno == EPERM) { - VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not " - "permitted", - (long) uid, (long) gid, path); + VIR_DEBUG("Setting user and group to '%ld:%ld' on '%s' not " + "permitted", + (long) uid, (long) gid, path); } else if (errno == EROFS) { - VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not " - "possible on readonly filesystem", - (long) uid, (long) gid, path); + VIR_DEBUG("Setting user and group to '%ld:%ld' on '%s' not " + "possible on readonly filesystem", + (long) uid, (long) gid, path); } else { virReportSystemError(errno, _("unable to set user and group to '%ld:%ld' " -- 2.8.3

3 6

[libvirt] [PATCH] qemu: Use the proper string in qemuBlock...JSONSocketAddress()
by Martin Kletzander 17 Jul '17

17 Jul '17

Recent refactors made it so that the function may use uninitialized pointer, but it actually wanted to use a different variable and value at all. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/qemu/qemu_block.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index ccaf3261101d..93124c5ba486 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -482,7 +482,7 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src, case VIR_STORAGE_NET_HOST_TRANS_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("transport protocol '%s' is not yet supported"), - transport); + virStorageNetHostTransportTypeToString(host->transport)); goto cleanup; } -- 2.13.3

2 1

[libvirt] [PATCH] phyp: Fix memory leak in phypUUIDTable_Push
by ZhiPeng Lu 17 Jul '17

17 Jul '17

@remote_file, allocated by virAsprintf, was not freed and leaked. Signed-off-by: Zhipeng Lu <lu.zhipeng(a)zte.com.cn> --- src/phyp/phyp_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 9121581..662e2f7 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -568,6 +568,7 @@ phypUUIDTable_Push(virConnectPtr conn) ret = 0; cleanup: + VIR_FREE(remote_file); if (channel) { libssh2_channel_send_eof(channel); libssh2_channel_wait_eof(channel); -- 1.8.3.1

1 0

[libvirt] [PATCH 0/3] exposing busy polling support for vhost-net
by sferdjao＠redhat.com 17 Jul '17

17 Jul '17

From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui(a)redhat.com> In version 2.7.0, QEMU introduced support of busy polling for vhost-net [0]. To avoid paraphrasing original authors of that feature and the purpose it I prefer to share a pointer [1]. This patch serie exposes throught the NIC driver-specific element a new option 'poll_us'. That option is only available with the backend driver 'vhost'. The option 'poll_us' takes a positive number. 0 means that the option is not going to be exposed. [0] 69e87b32680a41d9761191443587c595b6f5fc3f [1] https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg04595.html Sahid Orentino Ferdjaoui (3): qemu: add capability for vhost-net busy polling conf: introduce 'poll_us' attribute for domain interface qemu: add busy polling support docs/formatdomain.html.in | 12 ++++++++++- docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 16 +++++++++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 6 ++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 22 +++++++++++++++++++++ src/qemu/qemu_hotplug.c | 12 +++++++++++ tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-net-virtio-netdev-pollus-fail.xml | 23 ++++++++++++++++++++++ ...xml2argv-net-virtio-netdev-pollus-qemu-fail.xml | 23 ++++++++++++++++++++++ .../qemuxml2argv-net-virtio-netdev-pollus.args | 23 ++++++++++++++++++++++ .../qemuxml2argv-net-virtio-netdev-pollus.xml | 23 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 9 +++++++++ 18 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml -- 2.9.4

6 11

[libvirt] [PATCH v4 00/14] Make virNodeDeviceObj and virNodeDeviceObjList private
by John Ferlan 17 Jul '17

17 Jul '17

v3 here: https://www.redhat.com/archives/libvir-list/2017-June/msg00155.html Most patches were ACK'd in v3 (some with conditions). Changes this time around are: -> Patch1: Make the call to virNodeDeviceObjFree in dev_refresh() prior to dev_create Move the virNodeDeviceObjFree in device_removed() to after the Unlock as it avoids the need for { } else { } (besides the *ObjFree checks for !dev anyway). -> Patch2: Change as requested to remove the unnecessary obj = NULL in testDestroyVport Change as requested to goto cleanup in testStoragePoolDestroy -> Patch3: Fixed the comments to have the right variable name -> Patch4: Use "if ((obj = ...))) {" type syntax as suggested. -> Patch5-9: No change -> Patch10: Fix commit description -> Patch11: No change -> Patch 12: Fix the testNodeDeviceDestroy logic -> Patch 13-14 New patches that take the next two logical steps - objectifying the virNodeDeviceObjList and using a HashTable for access (along with all those bells and whistles). Finally remove the need for driver level locks when accessing the @devs ObjList. John Ferlan (14): nodedev: Alter virNodeDeviceObjRemove test: Adjust cleanup/error paths for nodedev test APIs nodedev: Use common naming for virnodedeviceobj nodedev: Use consistent names for driver variables nodedev: Introduce virNodeDeviceObjNew nodedev: Introduce virNodeDeviceObjListNew nodedev: Alter node device obj list function names nodedev: Dereference the obj/def in virNodeDeviceObjListFind* APIs nodedev: Introduce virNodeDeviceGetSCSIHostCaps nodedev: Introduce virNodeDeviceObjListFindSCSIHostByWWNs nodedev: Privatize _virNodeDeviceObj and _virNodeDeviceObjList nodedev: Convert virNodeDeviceObj to use virObjectLockable nodedev: Convert virNodeDeviceObjListPtr to use hash tables nodedev: Remove driver locks around object list mgmt code src/conf/node_device_conf.c | 82 +++ src/conf/node_device_conf.h | 20 +- src/conf/virnodedeviceobj.c | 846 ++++++++++++++++++++---------- src/conf/virnodedeviceobj.h | 67 +-- src/libvirt_private.syms | 20 +- src/node_device/node_device_driver.c | 201 +++---- src/node_device/node_device_hal.c | 51 +- src/node_device/node_device_linux_sysfs.c | 77 +-- src/node_device/node_device_udev.c | 63 +-- src/test/test_driver.c | 133 +++-- 10 files changed, 895 insertions(+), 665 deletions(-) -- 2.9.4

2 28

[libvirt] [PATCH RESEND] virpcimock: Fix memory leak in pci_driver_new
by ZhiPeng Lu 16 Jul '17

16 Jul '17

@driverpath, allocated by virAsprintfQuiet, was not freed and leaked. Signed-off-by: Zhipeng Lu <lu.zhipeng(a)zte.com.cn> --- tests/virpcimock.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/virpcimock.c b/tests/virpcimock.c index e9408aa..dec9e01 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -480,6 +480,7 @@ pci_driver_new(const char *name, int fail, ...) if (VIR_APPEND_ELEMENT_QUIET(pciDrivers, nPCIDrivers, driver) < 0) ABORT_OOM(); + VIR_FREE(driverpath); } static struct pciDriver * -- 1.8.3.1

2 1