July 2016 - Devel - libvirt List Archives

[libvirt] [PATCH v3 0/6] Test persistent device attachment
by Tomasz Flendrich 21 Sep '16

21 Sep '16

Changes in v3: * fewer patches, because part of v2 was ACK'd * two old commits, 08/10 and 09/10, are now one commit * qemuDomainUpdateDeviceFlags is now also split, creating a new function qemuDomainUpdateDeviceLiveAndConfig(). This function is now used in qemuhotplugtest.c. * qemuDomainAttachDeviceLiveAndConfig, qemuDomainDetachDeviceLiveAndConfig and qemuDomainUpdateDeviceLiveAndConfig are now in a new file: qemu_driverpriv.h * variable "target" is renamed to "impact" and its type is changed * testQemuHotplugCheckResult() now takes "impact" as a parameter to determine whether to test the ->def or ->newDef * def->id is no longer set when handling a persistent domain * split DO_TEST_ATTACH (and other macros) to DO_TEST_ATTACH_LIVE and DO_TEST_ATTACH_CONFIG instead of taking a parameter * fixed indentation * "<source mode='bind'/>" is removed from one xml * a three year old testcase for updating a disk device was modified a little bit to make it finally execute and test something Link to v2: https://www.redhat.com/archives/libvir-list/2016-July/msg00595.html Tomasz Flendrich (6): Split qemuDomainUpdateDeviceFlags in two Make qemu attach/detach functions public Make qemuhotplugtest work with persistent domains qemuhotplug: Use a more generic function to update devices qemuhotplugtest: add testcase to update a disk device qemuhotplugtest: add a persistent attachment testcase src/qemu/qemu_driver.c | 117 +++--- src/qemu/qemu_driverpriv.h | 47 +++ tests/qemuhotplugtest.c | 433 ++++++++++++--------- .../qemuhotplug-disk-cdrom-nochange.xml | 2 +- .../qemuhotplug-qemu-agent.xml | 1 - .../qemuhotplug-base-config+qemu-agent+config.xml | 44 +++ .../qemuhotplug-base-config.xml | 40 ++ .../qemuhotplug-disk-cdrom.xml | 2 +- 8 files changed, 454 insertions(+), 232 deletions(-) create mode 100644 src/qemu/qemu_driverpriv.h create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-config+qemu-agent+config.xml create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml -- 1.9.1

2 8

[libvirt] [PATCH v1 0/4] libxl: host cpu element in capabilities
by Joao Martins 16 Sep '16

16 Sep '16

Hey, This small series implements host cpu description in caps, by getting topology and xen hwcaps parsing done, followed by having cpu-{compare,baseline} APIs implemented. Last thing missing I think it would be to libxl_cpuid_set the features to enable/disable whichever format we choose plus the appropriate XML convertion to/from XM and XL config formats. Note that since RFC[0] I removed the Get CPU model names API since it would make more sense having it exported once we do support guest cpu model/features setting. Changelog included in individual patches (only second patch got changed) Cheers, Joao [0] http://www.redhat.com/archives/libvir-list/2016-July/msg00245.html Joao Martins (4): libxl: describe host topology in capabilities libxl: describe host cpu features based on hwcaps libxl: implement virConnectCompareCPU libxl: implement virConnectBaselineCPU src/libxl/libxl_capabilities.c | 168 ++++++++++++++++++++++++++++++++++++++--- src/libxl/libxl_driver.c | 60 +++++++++++++++ 2 files changed, 218 insertions(+), 10 deletions(-) -- 2.1.4

4 11

[libvirt] qemu: migration: shall we abort migration while the guest is rebooting？
by Zhangbo (Oscar) 13 Sep '16

13 Sep '16

Hi all: Here's the steps we produce the problem: 1 reboot guest with the flag of VIR_DOMAIN_REBOOT_ACPI_POWER_BTN 2 sleep 1 second (so that the guest is still rebooting, although the API already returned.) 3 migrate the guest The problem is that : the guest failed to migrate to the dest, and crashed on source side. We don't bother to dig further into the problem, the root cause we think is that we migrate a guest while it's rebooting. So, shall we 1 make the reboot JOB LOCK longer enough until the guest has already rebooted(got monitor message from qemu)? 2 let openstack do the mutex work ? (has openstack already done this?) (I checked openstack codes, and found that openstack uses shutdown and create APIs rather than REBOOT to do the reboot work. So it seems that openstack doesn't take care of this problem nowadays. Am I right? --- see: nova/virt/libvirt/driver.py: _soft_reboot) Thanks in advance. Zhang Bo (Oscar)

2 4

[libvirt] [PATCH 0/3] add option to keep nvram file on undefine
by Nikolay Shirokovskiy 13 Sep '16

13 Sep '16

There is already a patch [1] on this topic with a different approach - keep nvram file by default. There is also some discussion there. To sum up keeping nvram on undefine could be useful in some usecases so there should be an option to do it. On the other hand there is a danger of leaving domain assets after its undefine and unsing them unintentionally on defining domain with the same name. AFAIU keeping nvram by default was motivated by domain disks behaviour. I think there is a difference as libvirt never create disks for domain as opposed to nvram and managed save and without disks domain will not start so user is quite aware of disks files. On the other hand one can start using nvram file solely putting <nvram> in config and managed save is created on daemon shutdown. So user is much less aware of nvram and managed save existence. Thus one can easily mess up by unaware define $name/using/undefine/define $name again usecase. Thus I vote for keeping said assets only if it is specified explicitly so user knows what he is doing. Adding option to undefine is best solution I come up with. The other options are add checks on define or start and both are impossible. Such a check should be done without any extra flags for it to be useful but this way we break existing users. As this a proof of concept this series does not add extra flag for managed save. [1] https://www.redhat.com/archives/libvir-list/2015-February/msg00915.html Nikolay Shirokovskiy (3): api: add VIR_DOMAIN_UNDEFINE_KEEP_NVRAM flag qemu: add VIR_DOMAIN_UNDEFINE_KEEP_NVRAM support virsh: add --keep-nvram option to undefine command include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 26 +++++++++++++++++--------- tools/virsh-domain.c | 8 ++++++++ tools/virsh.pod | 6 +++--- 4 files changed, 29 insertions(+), 12 deletions(-) -- 1.8.3.1

5 10

[libvirt] [PATCH] usb: allow host devices to be specified by port
by Thomas Hebb 08 Sep '16

08 Sep '16

Previously, only VID/PID and bus/device matching were supported. Neither of these provide a stable and persistent way of assigning a guest a specific host device out of several with the same VID and PID, as device numbers change on every enumeration. Add a third method of matching, bus/port, which allows a specific port on the host to be specified using the dotted notation found in Linux's "devpath" sysfs attribute. --- src/conf/domain_conf.c | 36 ++++- src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 2 - src/util/virhostdev.c | 126 ++++++++-------- src/util/virusb.c | 160 ++++++++------------- src/util/virusb.h | 23 ++- tests/virusbtest.c | 124 ++++++++++------ .../sys_bus_usb/devices/1-1.5.3.1/devpath | 1 + .../sys_bus_usb/devices/1-1.5.3.3/devpath | 1 + .../sys_bus_usb/devices/1-1.5.3/devpath | 1 + .../sys_bus_usb/devices/1-1.5.4/devpath | 1 + .../sys_bus_usb/devices/1-1.5.5/devpath | 1 + .../sys_bus_usb/devices/1-1.5.6/devpath | 1 + .../sys_bus_usb/devices/1-1.5/devpath | 1 + .../sys_bus_usb/devices/1-1.6/devpath | 1 + .../virusbtestdata/sys_bus_usb/devices/1-1/devpath | 1 + .../sys_bus_usb/devices/2-1.2/devpath | 1 + .../virusbtestdata/sys_bus_usb/devices/2-1/devpath | 1 + .../sys_bus_usb/devices/usb1/devpath | 1 + .../sys_bus_usb/devices/usb2/devpath | 1 + .../sys_bus_usb/devices/usb3/devpath | 1 + .../sys_bus_usb/devices/usb4/devpath | 1 + 22 files changed, 266 insertions(+), 222 deletions(-) create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9f7b906..a587cc2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5531,7 +5531,8 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, goto out; } } else if (xmlStrEqual(cur->name, BAD_CAST "address")) { - char *bus, *device; + char *bus, *device, *port; + bool dev_or_port = false; bus = virXMLPropString(cur, "bus"); if (bus) { @@ -5557,10 +5558,24 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, VIR_FREE(device); goto out; } + dev_or_port = true; VIR_FREE(device); - } else { + } + + port = virXMLPropString(cur, "port"); + if (port) { + if (*port) { + usbsrc->port = port; + dev_or_port = true; + } else { + VIR_FREE(port); + } + } + + if (!dev_or_port) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("usb address needs device id")); + _("usb address needs either device id " + "or port")); goto out; } } else { @@ -20326,10 +20341,17 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virBufferAsprintf(buf, "<vendor id='0x%.4x'/>\n", usbsrc->vendor); virBufferAsprintf(buf, "<product id='0x%.4x'/>\n", usbsrc->product); } - if (usbsrc->bus || usbsrc->device) { - virBufferAsprintf(buf, "<address %sbus='%d' device='%d'/>\n", - includeTypeInAddr ? "type='usb' " : "", - usbsrc->bus, usbsrc->device); + if (usbsrc->bus || usbsrc->device || usbsrc->port) { + virBufferAddLit(buf, "<address"); + if (includeTypeInAddr) + virBufferAddLit(buf, " type='usb'"); + if (usbsrc->bus) + virBufferAsprintf(buf, " bus='%u'", usbsrc->bus); + if (usbsrc->device) + virBufferAsprintf(buf, " device='%u'", usbsrc->device); + if (usbsrc->port) + virBufferAsprintf(buf, " port='%s'", usbsrc->port); + virBufferAddLit(buf, "/>\n"); } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ba0ad5f..91bd621 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -317,6 +317,8 @@ VIR_ENUM_DECL(virDomainHostdevSubsysSCSIProtocol) typedef struct _virDomainHostdevSubsysUSB virDomainHostdevSubsysUSB; typedef virDomainHostdevSubsysUSB *virDomainHostdevSubsysUSBPtr; struct _virDomainHostdevSubsysUSB { + char *port; + bool autoAddress; /* bus/device were filled automatically based on vendor/product */ unsigned bus; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ccb4c5e..d412eac 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2511,8 +2511,6 @@ virURIResolveAlias; # util/virusb.h virUSBDeviceFileIterate; virUSBDeviceFind; -virUSBDeviceFindByBus; -virUSBDeviceFindByVendor; virUSBDeviceFree; virUSBDeviceGetBus; virUSBDeviceGetDevno; diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 9b5ca6f..e809c97 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1179,89 +1179,95 @@ virHostdevMarkUSBDevices(virHostdevManagerPtr mgr, static int +virHostdevFindUSBDeviceWithFlags(virDomainHostdevDefPtr hostdev, + bool mandatory, + unsigned int flags, + virUSBDevicePtr *usb) +{ + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + unsigned vendor = usbsrc->vendor; + unsigned product = usbsrc->product; + unsigned bus = usbsrc->bus; + char *port = usbsrc->port; + unsigned device = usbsrc->device; + virUSBDeviceListPtr devs; + int rc; + + rc = virUSBDeviceFind(vendor, product, bus, device, port, NULL, + mandatory, flags, &devs); + if (rc < 0) + return -1; + + if (rc == 1) { + *usb = virUSBDeviceListGet(devs, 0); + virUSBDeviceListSteal(devs, *usb); + } + virObjectUnref(devs); + + if (rc > 1) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Multiple USB devices for %x:%x, " + "use <address> to specify one"), + vendor, product); + return -1; + } + + return rc; +} + +static int virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev, bool mandatory, virUSBDevicePtr *usb) { virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; unsigned vendor = usbsrc->vendor; - unsigned product = usbsrc->product; unsigned bus = usbsrc->bus; unsigned device = usbsrc->device; + char *port = usbsrc->port; bool autoAddress = usbsrc->autoAddress; + unsigned int flags = 0; int rc; *usb = NULL; - if (vendor && bus) { - rc = virUSBDeviceFind(vendor, product, bus, device, - NULL, - autoAddress ? false : mandatory, - usb); - if (rc < 0) { - return -1; - } else if (!autoAddress) { - goto out; - } else { - VIR_INFO("USB device %x:%x could not be found at previous" - " address (bus:%u device:%u)", - vendor, product, bus, device); - } - } + /* First attempt, matching on all known fields. */ + if (vendor) + flags |= USB_DEVICE_FIND_BY_VENDOR; + if (device) + flags |= USB_DEVICE_FIND_BY_DEVICE; + if (port) + flags |= USB_DEVICE_FIND_BY_PORT; + + rc = virHostdevFindUSBDeviceWithFlags(hostdev, + autoAddress ? false : mandatory, + flags, usb); + if (rc < 0) + return -1; - /* When vendor is specified, its USB address is either unspecified or the - * device could not be found at the USB device where it had been - * automatically found before. - */ - if (vendor) { - virUSBDeviceListPtr devs; + if (rc != 1 && autoAddress) { + VIR_INFO("USB device could not be found at previous address " + "(bus:%u device:%u)", bus, device); - rc = virUSBDeviceFindByVendor(vendor, product, NULL, mandatory, &devs); - if (rc < 0) - return -1; + /* Second attempt, for when the device number has changed. */ + flags &= ~((unsigned int) USB_DEVICE_FIND_BY_DEVICE); + usbsrc->device = 0; - if (rc == 1) { - *usb = virUSBDeviceListGet(devs, 0); - virUSBDeviceListSteal(devs, *usb); - } - virObjectUnref(devs); - - if (rc == 0) { - goto out; - } else if (rc > 1) { - if (autoAddress) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Multiple USB devices for %x:%x were found," - " but none of them is at bus:%u device:%u"), - vendor, product, bus, device); - } else { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Multiple USB devices for %x:%x, " - "use <address> to specify one"), - vendor, product); - } + rc = virHostdevFindUSBDeviceWithFlags(hostdev, mandatory, + flags, usb); + + if (rc < 0) return -1; - } + } + if (!*usb) { + hostdev->missing = true; + } else if (!usbsrc->bus || !usbsrc->device) { usbsrc->bus = virUSBDeviceGetBus(*usb); usbsrc->device = virUSBDeviceGetDevno(*usb); usbsrc->autoAddress = true; - - if (autoAddress) { - VIR_INFO("USB device %x:%x found at bus:%u device:%u (moved" - " from bus:%u device:%u)", - vendor, product, - usbsrc->bus, usbsrc->device, - bus, device); - } - } else if (!vendor && bus) { - if (virUSBDeviceFindByBus(bus, device, NULL, mandatory, usb) < 0) - return -1; } - out: - if (!*usb) - hostdev->missing = true; return 0; } diff --git a/src/util/virusb.c b/src/util/virusb.c index 6a001a7..2e15849 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -68,12 +68,6 @@ struct _virUSBDeviceList { virUSBDevicePtr *devs; }; -typedef enum { - USB_DEVICE_ALL = 0, - USB_DEVICE_FIND_BY_VENDOR = 1 << 0, - USB_DEVICE_FIND_BY_BUS = 1 << 1, -} virUSBDeviceFindFlags; - static virClassPtr virUSBDeviceListClass; static void virUSBDeviceListDispose(void *obj); @@ -119,11 +113,33 @@ static int virUSBSysReadFile(const char *f_name, const char *d_name, return ret; } +static int virUSBSysReadFileStr(const char *f_name, const char *d_name, + char **value) +{ + int ret = -1, tmp; + char *buf = NULL; + char *filename = NULL; + + tmp = virAsprintf(&filename, USB_SYSFS "/devices/%s/%s", d_name, f_name); + if (tmp < 0) + goto cleanup; + + if (virFileReadAll(filename, 1024, &buf) < 0) + goto cleanup; + + *value = buf; + ret = 0; + cleanup: + VIR_FREE(filename); + return ret; +} + static virUSBDeviceListPtr virUSBDeviceSearch(unsigned int vendor, unsigned int product, unsigned int bus, unsigned int devno, + const char *port, const char *vroot, unsigned int flags) { @@ -143,6 +159,8 @@ virUSBDeviceSearch(unsigned int vendor, while ((direrr = virDirRead(dir, &de, USB_SYSFS "/devices")) > 0) { unsigned int found_prod, found_vend, found_bus, found_devno; + char *found_port; + bool port_matches; char *tmpstr = de->d_name; if (strchr(de->d_name, ':')) @@ -170,16 +188,31 @@ virUSBDeviceSearch(unsigned int vendor, 10, &found_devno) < 0) goto cleanup; + if (virUSBSysReadFileStr("devpath", de->d_name, + &found_port) < 0) { + goto cleanup; + } else { + found_port[strlen(found_port) - 1] = '\0'; /* remove newline */ + port_matches = STREQ_NULLABLE(found_port, port); + VIR_FREE(found_port); + } + if ((flags & USB_DEVICE_FIND_BY_VENDOR) && (found_prod != product || found_vend != vendor)) continue; - if (flags & USB_DEVICE_FIND_BY_BUS) { + if (flags & USB_DEVICE_FIND_BY_DEVICE) { if (found_bus != bus || found_devno != devno) continue; found = true; } + if (flags & USB_DEVICE_FIND_BY_PORT) { + if (found_bus != bus || !port_matches) + continue; + found = true; + } + usb = virUSBDeviceNew(found_bus, found_devno, vroot); if (!usb) goto cleanup; @@ -204,36 +237,43 @@ virUSBDeviceSearch(unsigned int vendor, } int -virUSBDeviceFindByVendor(unsigned int vendor, - unsigned int product, - const char *vroot, - bool mandatory, - virUSBDeviceListPtr *devices) +virUSBDeviceFind(unsigned int vendor, + unsigned int product, + unsigned int bus, + unsigned int devno, + const char *port, + const char *vroot, + bool mandatory, + unsigned int flags, + virUSBDeviceListPtr *devices) { virUSBDeviceListPtr list; int count; - if (!(list = virUSBDeviceSearch(vendor, product, 0, 0, - vroot, - USB_DEVICE_FIND_BY_VENDOR))) + if (!(list = virUSBDeviceSearch(vendor, product, bus, devno, port, + vroot, flags))) return -1; - if (list->count == 0) { + count = list->count; + if (count == 0) { virObjectUnref(list); if (!mandatory) { - VIR_DEBUG("Did not find USB device %x:%x", - vendor, product); if (devices) *devices = NULL; return 0; } virReportError(VIR_ERR_INTERNAL_ERROR, - _("Did not find USB device %x:%x"), vendor, product); + _("Did not find a matching USB device (matching " + "on%s%s%s all others ignored): vid:%04x, pid:%04x, " + "bus:%u, device:%u, port:%s"), + flags & USB_DEVICE_FIND_BY_VENDOR ? " vid/pid," : "", + flags & USB_DEVICE_FIND_BY_DEVICE ? " bus/device," : "", + flags & USB_DEVICE_FIND_BY_PORT ? " bus/port," : "", + vendor, product, bus, devno, port ? port : "(null)"); return -1; } - count = list->count; if (devices) *devices = list; else @@ -242,86 +282,6 @@ virUSBDeviceFindByVendor(unsigned int vendor, return count; } -int -virUSBDeviceFindByBus(unsigned int bus, - unsigned int devno, - const char *vroot, - bool mandatory, - virUSBDevicePtr *usb) -{ - virUSBDeviceListPtr list; - - if (!(list = virUSBDeviceSearch(0, 0, bus, devno, - vroot, - USB_DEVICE_FIND_BY_BUS))) - return -1; - - if (list->count == 0) { - virObjectUnref(list); - if (!mandatory) { - VIR_DEBUG("Did not find USB device bus:%u device:%u", - bus, devno); - if (usb) - *usb = NULL; - return 0; - } - - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Did not find USB device bus:%u device:%u"), - bus, devno); - return -1; - } - - if (usb) { - *usb = virUSBDeviceListGet(list, 0); - virUSBDeviceListSteal(list, *usb); - } - virObjectUnref(list); - - return 0; -} - -int -virUSBDeviceFind(unsigned int vendor, - unsigned int product, - unsigned int bus, - unsigned int devno, - const char *vroot, - bool mandatory, - virUSBDevicePtr *usb) -{ - virUSBDeviceListPtr list; - - unsigned int flags = USB_DEVICE_FIND_BY_VENDOR|USB_DEVICE_FIND_BY_BUS; - if (!(list = virUSBDeviceSearch(vendor, product, bus, devno, - vroot, flags))) - return -1; - - if (list->count == 0) { - virObjectUnref(list); - if (!mandatory) { - VIR_DEBUG("Did not find USB device %x:%x bus:%u device:%u", - vendor, product, bus, devno); - if (usb) - *usb = NULL; - return 0; - } - - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Did not find USB device %x:%x bus:%u device:%u"), - vendor, product, bus, devno); - return -1; - } - - if (usb) { - *usb = virUSBDeviceListGet(list, 0); - virUSBDeviceListSteal(list, *usb); - } - virObjectUnref(list); - - return 0; -} - virUSBDevicePtr virUSBDeviceNew(unsigned int bus, unsigned int devno, diff --git a/src/util/virusb.h b/src/util/virusb.h index f98ea21..0834cd4 100644 --- a/src/util/virusb.h +++ b/src/util/virusb.h @@ -35,29 +35,26 @@ typedef virUSBDevice *virUSBDevicePtr; typedef struct _virUSBDeviceList virUSBDeviceList; typedef virUSBDeviceList *virUSBDeviceListPtr; +typedef enum { + USB_DEVICE_ALL = 0, + USB_DEVICE_FIND_BY_VENDOR = 1 << 0, + USB_DEVICE_FIND_BY_DEVICE = 1 << 1, + USB_DEVICE_FIND_BY_PORT = 1 << 2, +} virUSBDeviceFindFlags; + virUSBDevicePtr virUSBDeviceNew(unsigned int bus, unsigned int devno, const char *vroot); -int virUSBDeviceFindByBus(unsigned int bus, - unsigned int devno, - const char *vroot, - bool mandatory, - virUSBDevicePtr *usb); - -int virUSBDeviceFindByVendor(unsigned int vendor, - unsigned int product, - const char *vroot, - bool mandatory, - virUSBDeviceListPtr *devices); - int virUSBDeviceFind(unsigned int vendor, unsigned int product, unsigned int bus, unsigned int devno, + const char *port, const char *vroot, bool mandatory, - virUSBDevicePtr *usb); + unsigned int flags, + virUSBDeviceListPtr *devices); void virUSBDeviceFree(virUSBDevicePtr dev); int virUSBDeviceSetUsedBy(virUSBDevicePtr dev, diff --git a/tests/virusbtest.c b/tests/virusbtest.c index 4bbfe4a..0782b73 100644 --- a/tests/virusbtest.c +++ b/tests/virusbtest.c @@ -33,7 +33,8 @@ typedef enum { FIND_BY_ALL, FIND_BY_VENDOR, - FIND_BY_BUS + FIND_BY_DEVICE, + FIND_BY_PORT } testUSBFindFlags; struct findTestInfo { @@ -42,6 +43,7 @@ struct findTestInfo { unsigned int product; unsigned int bus; unsigned int devno; + const char *port; const char *vroot; bool mandatory; int how; @@ -78,25 +80,31 @@ static int testDeviceFind(const void *opaque) virUSBDeviceListPtr devs = NULL; int rv = 0; size_t i, ndevs = 0; + unsigned int flags = 0; switch (info->how) { case FIND_BY_ALL: - rv = virUSBDeviceFind(info->vendor, info->product, - info->bus, info->devno, - info->vroot, info->mandatory, &dev); + flags = USB_DEVICE_FIND_BY_VENDOR | + USB_DEVICE_FIND_BY_DEVICE | + USB_DEVICE_FIND_BY_PORT; break; case FIND_BY_VENDOR: - rv = virUSBDeviceFindByVendor(info->vendor, info->product, - info->vroot, info->mandatory, &devs); + flags = USB_DEVICE_FIND_BY_VENDOR; break; - case FIND_BY_BUS: - rv = virUSBDeviceFindByBus(info->bus, info->devno, - info->vroot, info->mandatory, &dev); + case FIND_BY_DEVICE: + flags = USB_DEVICE_FIND_BY_DEVICE; + break; + case FIND_BY_PORT: + flags = USB_DEVICE_FIND_BY_PORT; break; } + rv = virUSBDeviceFind(info->vendor, info->product, + info->bus, info->devno, info->port, + info->vroot, info->mandatory, flags, &devs); + if (info->expectFailure) { - if (rv == 0) { + if (rv >= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "unexpected success"); } else { @@ -107,9 +115,18 @@ static int testDeviceFind(const void *opaque) goto cleanup; } + if (info->how != FIND_BY_VENDOR) { + if (rv == 1) { + dev = virUSBDeviceListGet(devs, 0); + virUSBDeviceListSteal(devs, dev); + } else { + goto cleanup; + } + } + switch (info->how) { case FIND_BY_ALL: - case FIND_BY_BUS: + case FIND_BY_DEVICE: if (virUSBDeviceFileIterate(dev, testDeviceFileActor, NULL) < 0) goto cleanup; break; @@ -155,14 +172,17 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED) virUSBDeviceListPtr list = NULL; virUSBDeviceListPtr devlist = NULL; virUSBDevicePtr dev = NULL; + virUSBDeviceListPtr devs = NULL; int ret = -1; + int rv; size_t i, ndevs; if (!(list = virUSBDeviceListNew())) goto cleanup; #define EXPECTED_NDEVS_ONE 3 - if (virUSBDeviceFindByVendor(0x1d6b, 0x0002, NULL, true, &devlist) < 0) + if (virUSBDeviceFind(0x1d6b, 0x0002, 0, 0, NULL, NULL, true, + USB_DEVICE_FIND_BY_VENDOR, &devlist) < 0) goto cleanup; ndevs = virUSBDeviceListCount(devlist); @@ -186,7 +206,8 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED) goto cleanup; #define EXPECTED_NDEVS_TWO 3 - if (virUSBDeviceFindByVendor(0x18d1, 0x4e22, NULL, true, &devlist) < 0) + if (virUSBDeviceFind(0x18d1, 0x4e22, 0, 0, NULL, NULL, true, + USB_DEVICE_FIND_BY_VENDOR, &devlist) < 0) goto cleanup; ndevs = virUSBDeviceListCount(devlist); @@ -206,8 +227,17 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED) EXPECTED_NDEVS_ONE + EXPECTED_NDEVS_TWO) < 0) goto cleanup; - if (virUSBDeviceFind(0x18d1, 0x4e22, 1, 20, NULL, true, &dev) < 0) + rv = virUSBDeviceFind(0x18d1, 0x4e22, 1, 20, "1.5.6", NULL, true, + USB_DEVICE_FIND_BY_VENDOR | + USB_DEVICE_FIND_BY_DEVICE | + USB_DEVICE_FIND_BY_PORT, &devs); + if (rv != 1) { goto cleanup; + } else { + dev = virUSBDeviceListGet(devs, 0); + virUSBDeviceListSteal(devs, dev); + } + virObjectUnref(devs); if (!virUSBDeviceListFind(list, dev)) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -239,49 +269,63 @@ mymain(void) { int rv = 0; -#define DO_TEST_FIND_FULL(name, vend, prod, bus, devno, vroot, mand, how, fail) \ - do { \ - struct findTestInfo data = { name, vend, prod, bus, \ - devno, vroot, mand, how, fail \ - }; \ - if (virTestRun("USBDeviceFind " name, testDeviceFind, &data) < 0) \ - rv = -1; \ +#define DO_TEST_FIND_FULL(name, vend, prod, bus, devno, \ + port, vroot, mand, how, fail) \ + do { \ + struct findTestInfo data = { name, vend, prod, bus, \ + devno, port, vroot, mand, how, fail \ + }; \ + if (virTestRun("USBDeviceFind " name, testDeviceFind, &data) < 0) \ + rv = -1; \ } while (0) -#define DO_TEST_FIND(name, vend, prod, bus, devno) \ - DO_TEST_FIND_FULL(name, vend, prod, bus, devno, NULL, true, \ +#define DO_TEST_FIND(name, vend, prod, bus, devno, port) \ + DO_TEST_FIND_FULL(name, vend, prod, bus, devno, port, NULL, true, \ FIND_BY_ALL, false) -#define DO_TEST_FIND_FAIL(name, vend, prod, bus, devno) \ - DO_TEST_FIND_FULL(name, vend, prod, bus, devno, NULL, true, \ +#define DO_TEST_FIND_FAIL(name, vend, prod, bus, devno, port) \ + DO_TEST_FIND_FULL(name, vend, prod, bus, devno, port, NULL, true, \ FIND_BY_ALL, true) -#define DO_TEST_FIND_BY_BUS(name, bus, devno) \ - DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, true, \ - FIND_BY_BUS, false) -#define DO_TEST_FIND_BY_BUS_FAIL(name, bus, devno) \ - DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, true, \ - FIND_BY_BUS, true) +#define DO_TEST_FIND_BY_DEVICE(name, bus, devno) \ + DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, NULL, true, \ + FIND_BY_DEVICE, false) +#define DO_TEST_FIND_BY_DEVICE_FAIL(name, bus, devno) \ + DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, NULL, true, \ + FIND_BY_DEVICE, true) #define DO_TEST_FIND_BY_VENDOR(name, vend, prod) \ - DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, true, \ + DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, NULL, true, \ FIND_BY_VENDOR, false) #define DO_TEST_FIND_BY_VENDOR_FAIL(name, vend, prod) \ - DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, true, \ + DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, NULL, true, \ FIND_BY_VENDOR, true) - DO_TEST_FIND("Nexus", 0x18d1, 0x4e22, 1, 20); - DO_TEST_FIND_FAIL("Nexus wrong devnum", 0x18d1, 0x4e22, 1, 25); - DO_TEST_FIND_FAIL("Bogus", 0xf00d, 0xbeef, 1024, 768); +#define DO_TEST_FIND_BY_PORT(name, bus, port) \ + DO_TEST_FIND_FULL(name, 101, 202, bus, 456, port, NULL, true, \ + FIND_BY_PORT, false) +#define DO_TEST_FIND_BY_PORT_FAIL(name, bus, port) \ + DO_TEST_FIND_FULL(name, 101, 202, bus, 456, port, NULL, true, \ + FIND_BY_PORT, true) + + DO_TEST_FIND("Nexus", 0x18d1, 0x4e22, 1, 20, "1.5.6"); + DO_TEST_FIND_FAIL("Nexus wrong devnum", 0x18d1, 0x4e22, 1, 25, "1.5.6"); + DO_TEST_FIND_FAIL("Nexus wrong port", 0x18d1, 0x4e22, 1, 25, "1.5.4"); + DO_TEST_FIND_FAIL("Bogus", 0xf00d, 0xbeef, 1024, 768, "1.2.3.4"); - DO_TEST_FIND_BY_BUS("integrated camera", 1, 5); - DO_TEST_FIND_BY_BUS_FAIL("wrong bus/devno combination", 2, 20); - DO_TEST_FIND_BY_BUS_FAIL("missing bus", 5, 20); - DO_TEST_FIND_BY_BUS_FAIL("missing devnum", 1, 158); + DO_TEST_FIND_BY_DEVICE("integrated camera", 1, 5); + DO_TEST_FIND_BY_DEVICE_FAIL("wrong bus/devno combination", 2, 20); + DO_TEST_FIND_BY_DEVICE_FAIL("missing bus", 5, 20); + DO_TEST_FIND_BY_DEVICE_FAIL("missing devnum", 1, 158); DO_TEST_FIND_BY_VENDOR("Nexus (multiple results)", 0x18d1, 0x4e22); DO_TEST_FIND_BY_VENDOR_FAIL("Bogus vendor and product", 0xf00d, 0xbeef); DO_TEST_FIND_BY_VENDOR_FAIL("Valid vendor", 0x1d6b, 0xbeef); + DO_TEST_FIND_BY_PORT("Logitech mouse", 1, "1.5.3.3"); + DO_TEST_FIND_BY_PORT_FAIL("wrong bus/port combination", 2, "1.5.3.3"); + DO_TEST_FIND_BY_PORT_FAIL("missing bus", 5, "1.5.3.3"); + DO_TEST_FIND_BY_PORT_FAIL("missing port", 1, "8.2.5"); + if (virTestRun("USB List test", testUSBList, NULL) < 0) rv = -1; diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath new file mode 100644 index 0000000..02a7fbe --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath @@ -0,0 +1 @@ +1.5.3.1 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath new file mode 100644 index 0000000..23ca863 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath @@ -0,0 +1 @@ +1.5.3.3 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath new file mode 100644 index 0000000..8af85be --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath @@ -0,0 +1 @@ +1.5.3 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath new file mode 100644 index 0000000..94fe62c --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath @@ -0,0 +1 @@ +1.5.4 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath new file mode 100644 index 0000000..9075be4 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath @@ -0,0 +1 @@ +1.5.5 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath new file mode 100644 index 0000000..eac1e0a --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath @@ -0,0 +1 @@ +1.5.6 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath new file mode 100644 index 0000000..c239c60 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath @@ -0,0 +1 @@ +1.5 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath new file mode 100644 index 0000000..810ee4e --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath @@ -0,0 +1 @@ +1.6 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath @@ -0,0 +1 @@ +1 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath b/tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath new file mode 100644 index 0000000..5625e59 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath @@ -0,0 +1 @@ +1.2 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath b/tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath @@ -0,0 +1 @@ +1 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath b/tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath @@ -0,0 +1 @@ +0 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath b/tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath @@ -0,0 +1 @@ +0 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath b/tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath @@ -0,0 +1 @@ +0 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath b/tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath @@ -0,0 +1 @@ +0 -- 2.7.4

2 1

[libvirt] NPIV storage pools do not map to same LUN units across hosts.
by Nitesh Konkar 07 Sep '16

07 Sep '16

Link: http://wiki.libvirt.org/page/NPIV_in_libvirt Topic: Virtual machine configuration change to use vHBA LUN There is a NPIV storage pool defined on two hosts and pool contains a total of 8 volumes, allocated from a storage device. Source: # virsh vol-list poolvhba0 Name Path ------------------------------------------------------------------------------ unit:0:0:0 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000366 unit:0:0:1 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000367 unit:0:0:2 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000368 unit:0:0:3 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000369 unit:0:0:4 /dev/disk/by-id/wwn-0x6005076802818bda300000000000036a unit:0:0:5 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000380 unit:0:0:6 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000381 unit:0:0:7 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000382 -------------------------------------------------------------------- Destination: -------------------------------------------------------------------- # virsh vol-list poolvhba0 Name Path ------------------------------------------------------------------------------ unit:0:0:0 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000380 unit:0:0:1 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000381 unit:0:0:2 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000382 unit:0:0:3 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000367 unit:0:0:4 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000368 unit:0:0:5 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000366 unit:0:0:6 /dev/disk/by-id/wwn-0x6005076802818bda300000000000036a unit:0:0:7 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000369 -------------------------------------------------------------------- As you can see in the above output,the same set of eight LUNs from the storage server have been mapped, but the order that the LUNs are probed on each host is different, resulting in different unit names on the two different hosts . If the the guest XMLs is referencing its storage by "unit" number then is it safe to migrate such guests because the "unit number" is assigned by the driver according to the specific way it probes the storage and hence when you migrate these guests , it results in different unit names on the destination hosts. Thus the migrated guest gets mapped to the wrong LUNs and is given the wrong disks. The problem is that the LUN numbers on the destination host and source host do not agree. Example, LUN 0 on source_host, for example, may be LUN 5 on destination_host. When the guest is given the wrong disk, it suffers a fatal I/O error. (This is manifested as fatal I/O errors since the guest has no idea that its disks just changed out under it.)The migration does not take into account that the unit numbers do match on on the source and destination sides. So, should libvirt make sure that the guest domains reference NPIV pool volumes by their globally-unique wwn instead of by "unit" numbers? The guest XML references its storage by "unit" number. Eg:- <disk type='volume' device='lun'> <driver name='qemu' type='raw' cache='none'/> <source pool='poolvhba0' volume='unit:0:0:0'/> <backingStore/> <target dev='vdb' bus='virtio'/> <alias name='virtio-disk1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </disk> I am planning to write a patch for it. Any comments on the above observation/approach would be appreciated. Thanks, Nitesh.

2 1

[libvirt] [libvirt-glib/libvirt-gconfig 00/17] Graphics: Introduce the new Remote and Local classes (and also implement a few missing methods).
by Fabiano Fidêncio 06 Sep '16

06 Sep '16

While trying to use libvirt-gobject and libvirt-gconfig for accessing VMs and looking at their config, instead of using libvirt and parsing XML directly, I found out that a few methods have been missing and that libvirt-gconfig is not exactly thought for the "reading their config" use case (see more explanations on the 10th and 14th commits. This series, unfortunately, introduces an ABI breakage. Fabiano Fidêncio (17): gconfig: Implement gvir_config_domain_graphics_vnc_get_autoport() gconfig: Implement gvir_config_domain_graphics_spice_get_autoport() gconfig: Implement gvir_config_domain_graphics_rdp_get_autoport() gconfig: Implement gvir_config_domain_graphics_sdl_get_display() gconfig: Implement gvir_config_domain_graphics_sdl_get_fullscreen() gconfig: Implement gvir_config_domain_graphics_spice_get_tls_port() gconfig: Implement gvir_config_domain_graphics_spice_{get,set}_host() gconfig: Implement gvir_config_domain_graphics_vnc_{get,set}_host() gconfig: Implement gvir_config_domain_graphics_rdp_{get,set}_host() gconfig: Add GVirCofigDomainGraphicsRemote class gconfig: Adapt GVirConfigDomainGraphicsSpice to GVirConfigDomainGraphicsRemote gconfig: Adapt GVirConfigDomainGraphicsRdp to GVirConfigDomainGraphicsRemote gconfig: Adapt GVirConfigDomainGraphicsVnc to GVirConfigDomainGraphicsRemote gconfig: Add GVirCofigDomainGraphicsLocal class gconfig: Adapt GVirConfigDomainGraphicsSdl to GVirConfigDomainGraphicsLocal gconfig: Adapt GVirConfigDomainGraphicsDesktop to GVirConfigDomainGraphicsLocal gconfig,graphics: Avoid crash when gvir_config_object_new_from_xml() returns NULL libvirt-gconfig/Makefile.am | 4 + .../libvirt-gconfig-domain-graphics-desktop.c | 14 ++- .../libvirt-gconfig-domain-graphics-desktop.h | 4 +- .../libvirt-gconfig-domain-graphics-local.c | 97 +++++++++++++++++++ .../libvirt-gconfig-domain-graphics-local.h | 68 ++++++++++++++ .../libvirt-gconfig-domain-graphics-rdp.c | 32 ++++++- .../libvirt-gconfig-domain-graphics-rdp.h | 9 +- .../libvirt-gconfig-domain-graphics-remote.c | 103 +++++++++++++++++++++ .../libvirt-gconfig-domain-graphics-remote.h | 70 ++++++++++++++ .../libvirt-gconfig-domain-graphics-sdl.c | 19 +++- .../libvirt-gconfig-domain-graphics-sdl.h | 6 +- .../libvirt-gconfig-domain-graphics-spice.c | 40 +++++++- .../libvirt-gconfig-domain-graphics-spice.h | 10 +- .../libvirt-gconfig-domain-graphics-vnc.c | 32 ++++++- .../libvirt-gconfig-domain-graphics-vnc.h | 9 +- libvirt-gconfig/libvirt-gconfig.h | 2 + libvirt-gconfig/libvirt-gconfig.sym | 20 ++++ po/POTFILES.in | 2 + 18 files changed, 513 insertions(+), 28 deletions(-) create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-local.c create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-local.h create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-remote.c create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-remote.h -- 2.5.0

2 30

[libvirt] [PATCH v2] virsh: use virConnectGetDomainCapabilities with maxvcpus
by Shivaprasad G Bhat 01 Sep '16

01 Sep '16

virsh maxvcpus --type kvm output is useless on PPC. Also, in commit e6806d79 we documented not rely on virConnectGetMaxVcpus output. Fix the maxvcpus to use virConnectGetDomainCapabilities now to make it useful. The call is made to use the default emulator binary and to check for the host machine and arch which is what the command intends to show anyway. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com> --- tools/virsh-host.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 57f0c0e..dd6ff4e 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -606,15 +606,37 @@ static bool cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd) { const char *type = NULL; - int vcpus; + int vcpus = -1; + char *caps = NULL; + const unsigned int flags = 0; /* No flags so far */ + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; virshControlPtr priv = ctl->privData; if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) return false; + caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, type, flags); + if (!caps) + goto fallback; + + xml = virXMLParseStringCtxt(caps, _("(domainCapabilities)"), &ctxt); + if (!xml) { + VIR_FREE(caps); + goto fallback; + } + + virXPathInt("string(./vcpu[1]/@max)", ctxt, &vcpus); + + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + if (vcpus > 0) + goto exit; + + fallback: if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0) return false; - + exit: vshPrint(ctl, "%d\n", vcpus); return true;

2 1

[libvirt] [PATCH v2] qemu: neglect cur_balloon in supplied xml
by Nikolay Shirokovskiy 31 Aug '16

31 Aug '16

cur_balloon value can change in between preparing external config and using it in operations like save and migrate. As a resutl operation will fail for ABI inconsistency. cur_balloon changes can not be predicted generally and thus operations will fail from time to time. Skip checking cur_balloon if domain lock can not be hold between preparing external config outside of libvirt and checking it against active config. Instead update cur_balloon value in external config from active config. This way it is protected from forges and is keeped up to date too. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com> --- src/conf/domain_conf.c | 14 +++++++++++--- src/conf/domain_conf.h | 9 +++++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 29 ++++++++++++++++++++--------- src/qemu/qemu_domain.h | 6 +++--- src/qemu/qemu_driver.c | 5 +++-- src/qemu/qemu_migration.c | 4 ++-- 7 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 85f6e31..f1cf87f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18493,8 +18493,9 @@ virDomainDefVcpuCheckAbiStability(virDomainDefPtr src, * validation of custom XML config passed in during migration */ bool -virDomainDefCheckABIStability(virDomainDefPtr src, - virDomainDefPtr dst) +virDomainDefCheckABIStabilityFlags(virDomainDefPtr src, + virDomainDefPtr dst, + unsigned int flags) { size_t i; virErrorPtr err; @@ -18538,7 +18539,8 @@ virDomainDefCheckABIStability(virDomainDefPtr src, virDomainDefGetMemoryInitial(src)); goto error; } - if (src->mem.cur_balloon != dst->mem.cur_balloon) { + if (!(flags & VIR_DOMAIN_DEF_ABI_CHECK_SKIP_VOLATILE) && + src->mem.cur_balloon != dst->mem.cur_balloon) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain current memory %lld does not match source %lld"), dst->mem.cur_balloon, src->mem.cur_balloon); @@ -18963,6 +18965,12 @@ virDomainDefCheckABIStability(virDomainDefPtr src, return false; } +bool +virDomainDefCheckABIStability(virDomainDefPtr src, + virDomainDefPtr dst) +{ + return virDomainDefCheckABIStabilityFlags(src, dst, 0); +} static int virDomainDefAddDiskControllersForType(virDomainDefPtr def, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3792562..cd7b966 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2623,6 +2623,11 @@ typedef enum { VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST = 1 << 9, } virDomainDefFormatFlags; +typedef enum { + /* skip checking values like cur_balloon that can be changed meanwhile */ + VIR_DOMAIN_DEF_ABI_CHECK_SKIP_VOLATILE = 1 << 0, +} virDomainDefABICheckFlags; + virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr, const virDomainDef *def, virCapsPtr caps, @@ -2658,6 +2663,10 @@ virDomainObjPtr virDomainObjParseFile(const char *filename, bool virDomainDefCheckABIStability(virDomainDefPtr src, virDomainDefPtr dst); +bool virDomainDefCheckABIStabilityFlags(virDomainDefPtr src, + virDomainDefPtr dst, + unsigned int flags); + int virDomainDefAddImplicitDevices(virDomainDefPtr def); virDomainIOThreadIDDefPtr virDomainIOThreadIDFind(const virDomainDef *def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 42f664c..4e7840c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -208,6 +208,7 @@ virDomainDefAddController; virDomainDefAddImplicitDevices; virDomainDefAddUSBController; virDomainDefCheckABIStability; +virDomainDefCheckABIStabilityFlags; virDomainDefClearCCWAddresses; virDomainDefClearDeviceAliases; virDomainDefClearPCIAddresses; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d1f8175..4b45caf 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4620,21 +4620,32 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver, } -bool -qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, - virDomainDefPtr src, - virDomainDefPtr dst) +int +qemuDomainDefUpdateVolatile(virQEMUDriverPtr driver, + virDomainDefPtr src, + virDomainDefPtr dst) { virDomainDefPtr migratableDefSrc = NULL; virDomainDefPtr migratableDefDst = NULL; - const int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_UPDATE_CPU | VIR_DOMAIN_XML_MIGRATABLE; - bool ret = false; + const unsigned int copy_flags = VIR_DOMAIN_XML_SECURE | + VIR_DOMAIN_XML_UPDATE_CPU | + VIR_DOMAIN_XML_MIGRATABLE; + const unsigned int check_flags = VIR_DOMAIN_DEF_ABI_CHECK_SKIP_VOLATILE; + int ret = -1; - if (!(migratableDefSrc = qemuDomainDefCopy(driver, src, flags)) || - !(migratableDefDst = qemuDomainDefCopy(driver, dst, flags))) + + if (!(migratableDefSrc = qemuDomainDefCopy(driver, src, copy_flags)) || + !(migratableDefDst = qemuDomainDefCopy(driver, dst, copy_flags))) + goto cleanup; + + if (!virDomainDefCheckABIStabilityFlags(migratableDefSrc, + migratableDefDst, + check_flags)) goto cleanup; - ret = virDomainDefCheckABIStability(migratableDefSrc, migratableDefDst); + dst->mem.cur_balloon = src->mem.cur_balloon; + + ret = 0; cleanup: virDomainDefFree(migratableDefSrc); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2443e97..7581fff 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -578,9 +578,9 @@ int qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob); -bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, - virDomainDefPtr src, - virDomainDefPtr dst); +int qemuDomainDefUpdateVolatile(virQEMUDriverPtr driver, + virDomainDefPtr src, + virDomainDefPtr dst); bool qemuDomainAgentAvailable(virDomainObjPtr vm, bool reportError); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e70d3ce..a819f53 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3244,7 +3244,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) { goto endjob; } - if (!qemuDomainDefCheckABIStability(driver, vm->def, def)) { + if (qemuDomainDefUpdateVolatile(driver, vm->def, def) < 0) { virDomainDefFree(def); goto endjob; } @@ -15103,7 +15103,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, /* Transitions 5, 6, 8, 9 */ /* Check for ABI compatibility. We need to do this check against * the migratable XML or it will always fail otherwise */ - if (config && !qemuDomainDefCheckABIStability(driver, vm->def, config)) { + if (config + && qemuDomainDefUpdateVolatile(driver, vm->def, config) < 0) { virErrorPtr err = virGetLastError(); if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 89350c8..38f471b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3237,7 +3237,7 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto cleanup; - if (!qemuDomainDefCheckABIStability(driver, vm->def, def)) + if (qemuDomainDefUpdateVolatile(driver, vm->def, def) < 0) goto cleanup; rv = qemuDomainDefFormatLive(driver, def, false, true); @@ -3582,7 +3582,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (!newdef) goto cleanup; - if (!qemuDomainDefCheckABIStability(driver, *def, newdef)) { + if (qemuDomainDefUpdateVolatile(driver, *def, newdef) < 0) { virDomainDefFree(newdef); goto cleanup; } -- 1.8.3.1

2 4

[libvirt] [PATCH 0/6] auto-assign addresses when <address type='pci'/> is specified
by Laine Stump 28 Aug '16

28 Aug '16

This is an alternative to Cole's series that permits <address type='pci'/> to force assignment of a PCI address, which is particularly useful on platforms that could connect the same device in different ways (e.g. aarch64/virt). Here is Cole's last iteration of the series: https://www.redhat.com/archives/libvir-list/2016-May/msg01088.html I had expressed a dislike of the "auto_allocate" flag that his series temporarily adds to the address (while simultaneously changing the address type to NONE) and suggested just changing all the necessary places to check for a valid PCI address instead of just checking the address type. He replied that this wasn't as simple as it looked, so I decided to try it; turns out he's right. But I still like this method better because it's not playing tricks with the address type, or adding a temporary private attribute to what should be pure config data. Your opinion may vary though :-) Note that patch 5/6 incorporates the same test case that Cole used in his penultimate patch, and I've added his patch to check the case of aarch64 at the end as well. Cole Robinson (1): tests: qemu: test <address type='pci'/> with aarch64 Laine Stump (5): conf: move virDomainDeviceInfo definition from domain_conf.h to device_conf.h conf: new functions to check if PCI address is wanted/present conf: allow type='pci' addresses with no address attributes specified bhyve: auto-assign addresses when <address type='pci'/> is specified qemu: auto-assign addresses when <address type='pci'/> is specified docs/schemas/basictypes.rng | 8 +- src/bhyve/bhyve_device.c | 10 +- src/conf/device_conf.c | 6 +- src/conf/device_conf.h | 132 ++++++++++++++++++++- src/conf/domain_addr.c | 2 +- src/conf/domain_conf.c | 13 +- src/conf/domain_conf.h | 129 -------------------- src/qemu/qemu_domain_address.c | 64 +++++----- ...l2argv-aarch64-virtio-pci-manual-addresses.args | 4 +- ...ml2argv-aarch64-virtio-pci-manual-addresses.xml | 5 + .../qemuxml2argv-pci-autofill-addr.args | 25 ++++ .../qemuxml2argv-pci-autofill-addr.xml | 35 ++++++ tests/qemuxml2argvtest.c | 1 + ...2xmlout-aarch64-virtio-pci-manual-addresses.xml | 5 + .../qemuxml2xmlout-pci-autofill-addr.xml | 41 +++++++ tests/qemuxml2xmltest.c | 1 + 16 files changed, 298 insertions(+), 183 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-autofill-addr.xml -- 2.5.5

4 21