[libvirt] [PATCH 09/10] qemu: remove pciaddrs caching

[Tomasz Flendrich]

The cached pci address set is not required anymore, because the set
is now being recalculated from the domain definition on demand,
so the cache can be deleted.
---
 src/qemu/qemu_domain.c |  1 -
 src/qemu/qemu_domain.h |  2 --
 src/qemu/qemu_domain_address.c | 16 +++-
 3 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5054ffe..e51584e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1298,7 +1298,6 @@ qemuDomainObjPrivateFree(void *data)
 virObjectUnref(priv->qemuCaps);
 
 virCgroupFree(>cgroup);
-virDomainPCIAddressSetFree(priv->pciaddrs);
 virDomainChrSourceDefFree(priv->monConfig);
 qemuDomainObjFreeJob(priv);
 VIR_FREE(priv->lockState);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index fdabbf9..bf96ad3 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -184,8 +184,6 @@ struct _qemuDomainObjPrivate {
 bool beingDestroyed;
 char *pidfile;
 
-virDomainPCIAddressSetPtr pciaddrs;
-
 virQEMUCapsPtr qemuCaps;
 char *lockState;
 
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index a5add50..df36349 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1466,12 +1466,10 @@ qemuDomainPCIAddrSetCreateFromDomain(virDomainDefPtr 
def,
 
 static int
 qemuDomainAssignPCIAddresses(virDomainDefPtr def,
- virQEMUCapsPtr qemuCaps,
- virDomainObjPtr obj)
+ virQEMUCapsPtr qemuCaps)
 {
 int ret = -1;
 virDomainPCIAddressSetPtr addrs = NULL;
-qemuDomainObjPrivatePtr priv = NULL;
 int max_idx = -1;
 int nbuses = 0;
 size_t i;
@@ -1625,14 +1623,6 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
 }
 }
 
-if (obj && obj->privateData) {
-priv = obj->privateData;
-/* if this is the live domain object, we persist the PCI addresses */
-virDomainPCIAddressSetFree(priv->pciaddrs);
-priv->pciaddrs = addrs;
-addrs = NULL;
-}
-
 ret = 0;
 
  cleanup:
@@ -1645,7 +1635,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
 int
 qemuDomainAssignAddresses(virDomainDefPtr def,
   virQEMUCapsPtr qemuCaps,
-  virDomainObjPtr obj,
+  virDomainObjPtr obj ATTRIBUTE_UNUSED,
   bool newDomain ATTRIBUTE_UNUSED)
 {
 if (qemuDomainAssignVirtioSerialAddresses(def) < 0)
@@ -1659,7 +1649,7 @@ qemuDomainAssignAddresses(virDomainDefPtr def,
 
 qemuDomainAssignARMVirtioMMIOAddresses(def, qemuCaps);
 
-if (qemuDomainAssignPCIAddresses(def, qemuCaps, obj) < 0)
+if (qemuDomainAssignPCIAddresses(def, qemuCaps) < 0)
 return -1;
 
 return 0;
-- 
2.7.4 (Apple Git-66)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 05/10] qemu_hotplug: generate ccw address list on demand

[Tomasz Flendrich]

Dropping the caching of ccw address set.
Instead of using the cached address set, functions in qemu_hotplug.c
now recalculate it on demand.
---
 src/qemu/qemu_hotplug.c | 26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5101ae1..ee77e15 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -308,6 +308,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 char *drivestr = NULL;
 char *drivealias = NULL;
 bool releaseaddr = false;
+virDomainCCWAddressSetPtr ccwaddrs = NULL;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 const char *src = virDomainDiskGetSource(disk);
 
@@ -327,7 +328,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 goto cleanup;
 
 if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
-if (virDomainCCWAddressAssign(>info, priv->ccwaddrs,
+if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
+goto error;
+if (virDomainCCWAddressAssign(>info, ccwaddrs,
   !disk->info.addr.ccw.assigned) < 0)
 goto error;
 } else if (!disk->info.type ||
@@ -375,6 +378,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 
  cleanup:
 qemuDomainSecretDiskDestroy(disk);
+virDomainCCWAddressSetFree(ccwaddrs);
 VIR_FREE(devstr);
 VIR_FREE(drivestr);
 VIR_FREE(drivealias);
@@ -416,6 +420,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr 
driver,
 const char* type = virDomainControllerTypeToString(controller->type);
 char *devstr = NULL;
 qemuDomainObjPrivatePtr priv = vm->privateData;
+virDomainCCWAddressSetPtr ccwaddrs = NULL;
 bool releaseaddr = false;
 
 if (controller->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
@@ -457,7 +462,9 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr 
driver,
 if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, >info) < 
0)
 goto cleanup;
 } else if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
-if (virDomainCCWAddressAssign(>info, priv->ccwaddrs,
+if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
+goto cleanup;
+if (virDomainCCWAddressAssign(>info, ccwaddrs,
   !controller->info.addr.ccw.assigned) < 0)
 goto cleanup;
 }
@@ -490,6 +497,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr 
driver,
 qemuDomainReleaseDeviceAddress(vm, >info, NULL);
 
 VIR_FREE(devstr);
+virDomainCCWAddressSetFree(ccwaddrs);
 return ret;
 }
 
@@ -820,6 +828,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 int actualType;
 virNetDevBandwidthPtr actualBandwidth;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+virDomainCCWAddressSetPtr ccwaddrs = NULL;
 size_t i;
 
 /* preallocate new slot for device */
@@ -957,7 +966,9 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 if (qemuDomainMachineIsS390CCW(vm->def) &&
 virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
 net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
-if (virDomainCCWAddressAssign(>info, priv->ccwaddrs,
+if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
+goto cleanup;
+if (virDomainCCWAddressAssign(>info, ccwaddrs,
   !net->info.addr.ccw.assigned) < 0)
 goto cleanup;
 } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
@@ -1131,6 +1142,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 VIR_FREE(vhostfd);
 VIR_FREE(vhostfdName);
 virObjectUnref(cfg);
+virDomainCCWAddressSetFree(ccwaddrs);
 
 return ret;
 
@@ -1592,6 +1604,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
 char *charAlias = NULL;
 char *objAlias = NULL;
 virJSONValuePtr props = NULL;
+virDomainCCWAddressSetPtr ccwaddrs = NULL;
 const char *type;
 int ret = -1;
 
@@ -1620,9 +1633,11 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
 if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, >info) < 0)
 return -1;
 } else if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
-if (virDomainCCWAddressAssign(>info, priv->ccwaddrs,
+if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
+goto cleanup;
+if (virDomainCCWAddressAssign(>info, ccwaddrs,
   !rng->info.addr.ccw.assigned) < 0)
-return -1;
+goto cleanup;
 }
 
 /* build required metadata */
@@ -1671,6 +1686,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
 VIR_FREE(charAlias);
 VIR_FREE(objAlias);
 VIR_FREE(devstr);
+virDomainCCWAddressSetFree(ccwaddrs);
 return ret;
 
 /* rollback */
-- 
2.7.4 

[libvirt] [PATCH 10/10] Remove unused functions that release addresses

[Tomasz Flendrich]

Since address sets are now recalculated on demand instead of being
cached, there's no need for functions that release addresses.
---
 src/conf/domain_addr.c   | 92 
 src/conf/domain_addr.h   | 12 ---
 src/libvirt_private.syms |  3 --
 3 files changed, 107 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index fc3b9be..67c12a4 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -505,30 +505,6 @@ virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr 
addrs,
 return 0;
 }
 
-int
-virDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs,
-   virPCIDeviceAddressPtr addr)
-{
-/* permit any kind of connection type in validation, since we
- * already had it, and are giving it back.
- */
-virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPES_MASK;
-int ret = -1;
-char *addrStr = NULL;
-
-if (!(addrStr = virDomainPCIAddressAsString(addr)))
-goto cleanup;
-
-if (!virDomainPCIAddressValidate(addrs, addr, addrStr, flags, false))
-goto cleanup;
-
-addrs->buses[addr->bus].slots[addr->slot] = 0;
-ret = 0;
- cleanup:
-VIR_FREE(addrStr);
-return ret;
-}
-
 
 virDomainPCIAddressSetPtr
 virDomainPCIAddressSetAlloc(unsigned int nbuses)
@@ -781,30 +757,6 @@ virDomainCCWAddressValidate(virDomainDefPtr def 
ATTRIBUTE_UNUSED,
 return virDomainCCWAddressAssign(info, data, false);
 }
 
-int
-virDomainCCWAddressReleaseAddr(virDomainCCWAddressSetPtr addrs,
-   virDomainDeviceInfoPtr dev)
-{
-char *addr;
-int ret;
-
-addr = virDomainCCWAddressAsString(&(dev->addr.ccw));
-if (!addr)
-return -1;
-
-if ((ret = virHashRemoveEntry(addrs->defined, addr)) == 0 &&
-dev->addr.ccw.cssid == addrs->next.cssid &&
-dev->addr.ccw.ssid == addrs->next.ssid &&
-dev->addr.ccw.devno < addrs->next.devno) {
-addrs->next.devno = dev->addr.ccw.devno;
-addrs->next.assigned = false;
-}
-
-VIR_FREE(addr);
-
-return ret;
-}
-
 void virDomainCCWAddressSetFree(virDomainCCWAddressSetPtr addrs)
 {
 if (!addrs)
@@ -1238,47 +1190,3 @@ virDomainVirtioSerialAddrReserve(virDomainDefPtr def 
ATTRIBUTE_UNUSED,
 VIR_FREE(str);
 return ret;
 }
-
-/* virDomainVirtioSerialAddrRelease
- *
- * Release the virtio serial address of the device
- */
-int
-virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr addrs,
- virDomainDeviceInfoPtr info)
-{
-virBitmapPtr map;
-char *str = NULL;
-int ret = -1;
-ssize_t i;
-
-if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL ||
-info->addr.vioserial.port == 0)
-return 0;
-
-VIR_DEBUG("Releasing virtio serial %u %u", info->addr.vioserial.controller,
-  info->addr.vioserial.port);
-
-i = virDomainVirtioSerialAddrFindController(addrs, 
info->addr.vioserial.controller);
-if (i < 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("virtio serial controller %u is missing"),
-   info->addr.vioserial.controller);
-goto cleanup;
-}
-
-map = addrs->controllers[i]->ports;
-if (virBitmapClearBit(map, info->addr.vioserial.port) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("virtio serial controller %u does not have port %u"),
-   info->addr.vioserial.controller,
-   info->addr.vioserial.port);
-goto cleanup;
-}
-
-ret = 0;
-
- cleanup:
-VIR_FREE(str);
-return ret;
-}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 73d7474..a060683 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -143,10 +143,6 @@ int 
virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs,
virPCIDeviceAddressPtr addr)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
-int virDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs,
-   virPCIDeviceAddressPtr addr)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-
 int virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs,
virPCIDeviceAddressPtr next_addr,
virDomainPCIConnectFlags flags)
@@ -180,9 +176,6 @@ int virDomainCCWAddressValidate(virDomainDefPtr def,
 void *data)
 ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
 
-int virDomainCCWAddressReleaseAddr(virDomainCCWAddressSetPtr addrs,
-   virDomainDeviceInfoPtr dev)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 virDomainCCWAddressSetPtr virDomainCCWAddressSetCreate(void);
 
 struct _virDomainVirtioSerialController {
@@ -235,9 +228,4 @@ virDomainVirtioSerialAddrReserve(virDomainDefPtr def,
  void *data)
 

[libvirt] [PATCH 07/10] add qemuDomainPCIAddrSetCreateFromDomain

[Tomasz Flendrich]

The address sets (pci, ccw, virtio serial) are currently cached
in qemu private data, but all the information required to recreate
these sets is in the domain definition. Therefore I am removing
the redundant data and adding a way to recalculate these sets.

Add a function that calculates the pci address set from
the domain definition.
The part of pci address assignment that is not a dryRun is
separated from qemuDomainAssignPCIAddresses into a new function:
qemuDomainPCIAddrSetCreateFromDomain. The first time this function
is run, it can allocate some new addresses. After all the pci
addresses are assigned, on subsequent runs to this function, it just
recreates the pci address set.
---
 src/qemu/qemu_domain_address.c | 48 +++---
 src/qemu/qemu_domain_address.h |  5 +
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 8434124..a534df0 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1424,6 +1424,45 @@ qemuDomainAddressFindNewBusNr(virDomainDefPtr def)
 return lowestBusNr - 2;
 }
 
+virDomainPCIAddressSetPtr
+qemuDomainPCIAddrSetCreateFromDomain(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps)
+{
+virDomainPCIAddressSetPtr addrs = NULL;
+int max_idx = -1;
+int nbuses = 0;
+virDomainPCIAddressSetPtr ret = NULL;
+size_t i;
+
+for (i = 0; i < def->ncontrollers; i++) {
+if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
+if ((int) def->controllers[i]->idx > max_idx)
+max_idx = def->controllers[i]->idx;
+}
+}
+
+nbuses = max_idx + 1;
+
+if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, false)))
+goto cleanup;
+
+if (qemuDomainSupportsPCI(def, qemuCaps)) {
+if (qemuDomainValidateDevicePCISlotsChipsets(def, qemuCaps,
+ addrs) < 0)
+goto cleanup;
+
+if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0)
+goto cleanup;
+}
+
+ret = addrs;
+addrs = NULL;
+
+ cleanup:
+virDomainPCIAddressSetFree(addrs);
+
+return ret;
+}
 
 static int
 qemuDomainAssignPCIAddresses(virDomainDefPtr def,
@@ -1506,17 +1545,10 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
 goto cleanup;
 }
 
-if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, false)))
+if (!(addrs = qemuDomainPCIAddrSetCreateFromDomain(def, qemuCaps)))
 goto cleanup;
 
 if (qemuDomainSupportsPCI(def, qemuCaps)) {
-if (qemuDomainValidateDevicePCISlotsChipsets(def, qemuCaps,
- addrs) < 0)
-goto cleanup;
-
-if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0)
-goto cleanup;
-
 for (i = 0; i < def->ncontrollers; i++) {
 virDomainControllerDefPtr cont = def->controllers[i];
 int idx = cont->idx;
diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h
index 11d6e92..546c57f 100644
--- a/src/qemu/qemu_domain_address.h
+++ b/src/qemu/qemu_domain_address.h
@@ -45,6 +45,11 @@ virDomainCCWAddressSetPtr
 qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def)
 ATTRIBUTE_NONNULL(1);
 
+virDomainPCIAddressSetPtr
+qemuDomainPCIAddrSetCreateFromDomain(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
 # define __QEMU_DOMAIN_ADDRESS_H__
 
 #endif /* __QEMU_DOMAIN_ADDRESS_H__ */
-- 
2.7.4 (Apple Git-66)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 08/10] qemu_hotplug: generate pci address list on demand

[Tomasz Flendrich]

Dropping the caching of pci address set.
Instead of using the cached address set, functions in qemu_hotplug.c
now recalculate it on demand.
---
 src/qemu/qemu_domain_address.c | 18 -
 src/qemu/qemu_domain_address.h |  4 --
 src/qemu/qemu_hotplug.c| 89 ++
 3 files changed, 39 insertions(+), 72 deletions(-)

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index a534df0..a5add50 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1664,21 +1664,3 @@ qemuDomainAssignAddresses(virDomainDefPtr def,
 
 return 0;
 }
-
-
-void
-qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
-   virDomainDeviceInfoPtr info,
-   const char *devstr)
-{
-qemuDomainObjPrivatePtr priv = vm->privateData;
-
-if (!devstr)
-devstr = info->alias;
-
-else if (virDeviceInfoPCIAddressPresent(info) &&
- virDomainPCIAddressReleaseSlot(priv->pciaddrs,
->addr.pci) < 0)
-VIR_WARN("Unable to release PCI address on %s",
- NULLSTR(devstr));
-}
diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h
index 546c57f..5f35a92 100644
--- a/src/qemu/qemu_domain_address.h
+++ b/src/qemu/qemu_domain_address.h
@@ -37,10 +37,6 @@ int qemuDomainAssignAddresses(virDomainDefPtr def,
   bool newDomain)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
-void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
-virDomainDeviceInfoPtr info,
-const char *devstr);
-
 virDomainCCWAddressSetPtr
 qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def)
 ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ee77e15..a6404d8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -307,10 +307,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 char *devstr = NULL;
 char *drivestr = NULL;
 char *drivealias = NULL;
-bool releaseaddr = false;
 virDomainCCWAddressSetPtr ccwaddrs = NULL;
+virDomainPCIAddressSetPtr pciaddrs = NULL;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-const char *src = virDomainDiskGetSource(disk);
 
 if (!disk->info.type) {
 if (qemuDomainMachineIsS390CCW(vm->def) &&
@@ -335,10 +334,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 goto error;
 } else if (!disk->info.type ||
 disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, >info) < 0)
+if (!(pciaddrs = qemuDomainPCIAddrSetCreateFromDomain(vm->def,
+  priv->qemuCaps)))
+goto error;
+if (virDomainPCIAddressEnsureAddr(pciaddrs, >info) < 0)
 goto error;
 }
-releaseaddr = true;
+
 if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
 goto error;
 
@@ -366,10 +368,8 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
 goto failadddevice;
 
-if (qemuDomainObjExitMonitor(driver, vm) < 0) {
-releaseaddr = false;
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
 goto failexitmonitor;
-}
 
 virDomainAuditDisk(vm, NULL, disk->src, "attach", true);
 
@@ -379,6 +379,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
  cleanup:
 qemuDomainSecretDiskDestroy(disk);
 virDomainCCWAddressSetFree(ccwaddrs);
+virDomainPCIAddressSetFree(pciaddrs);
 VIR_FREE(devstr);
 VIR_FREE(drivestr);
 VIR_FREE(drivealias);
@@ -397,16 +398,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 }
 
  failadddrive:
-if (qemuDomainObjExitMonitor(driver, vm) < 0)
-releaseaddr = false;
-
  failexitmonitor:
 virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
 
  error:
-if (releaseaddr)
-qemuDomainReleaseDeviceAddress(vm, >info, src);
-
 ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true));
 goto cleanup;
 }
@@ -421,7 +416,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr 
driver,
 char *devstr = NULL;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virDomainCCWAddressSetPtr ccwaddrs = NULL;
-bool releaseaddr = false;
+virDomainPCIAddressSetPtr pciaddrs = NULL;
 
 if (controller->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
@@ -459,7 +454,10 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr 
driver,
 
 if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
 controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, >info) < 
0)

[libvirt] [PATCH 01/10] add virDomainVirtioSerialAddrSetCreateFromDomain

[Tomasz Flendrich]

The address sets (pci, ccw, virtio serial) are currently cached
in qemu private data, but all the information required to recreate
these sets is in the domain definition. Therefore I am removing
the redundant data and adding a way to recalculate these sets.

Add a function that calculates the virtio serial address set
from the domain definition.

Credit goes to Cole Robinson.
---
 src/conf/domain_addr.c | 31 +++
 src/conf/domain_addr.h |  3 +++
 src/libvirt_private.syms   |  1 +
 src/qemu/qemu_domain_address.c |  9 +
 4 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 794270d..fc3b9be 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -975,6 +975,37 @@ 
virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs)
 }
 }
 
+
+/* virDomainVirtioSerialAddrSetCreateFromDomain
++ *
++ * @def: Domain def to introspect
++ *
++ * Inspect the domain definition and return an address set containing
++ * every virtio serial address we find
++ */
+virDomainVirtioSerialAddrSetPtr
+virDomainVirtioSerialAddrSetCreateFromDomain(virDomainDefPtr def)
+{
+virDomainVirtioSerialAddrSetPtr addrs;
+virDomainVirtioSerialAddrSetPtr ret = NULL;
+
+if (!(addrs = virDomainVirtioSerialAddrSetCreate()))
+goto cleanup;
+
+if (virDomainVirtioSerialAddrSetAddControllers(addrs, def) < 0)
+goto cleanup;
+
+if (virDomainDeviceInfoIterate(def, virDomainVirtioSerialAddrReserve,
+   addrs) < 0)
+goto cleanup;
+
+ret = addrs;
+ cleanup:
+if (!ret)
+virDomainVirtioSerialAddrSetFree(addrs);
+return ret;
+}
+
 static int
 virDomainVirtioSerialAddrSetAutoaddController(virDomainDefPtr def,
   virDomainVirtioSerialAddrSetPtr 
addrs,
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index f3eda89..73d7474 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -208,6 +208,9 @@ 
virDomainVirtioSerialAddrSetAddControllers(virDomainVirtioSerialAddrSetPtr addrs
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 void
 virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs);
+virDomainVirtioSerialAddrSetPtr
+virDomainVirtioSerialAddrSetCreateFromDomain(virDomainDefPtr def)
+ATTRIBUTE_NONNULL(1);
 bool
 virDomainVirtioSerialAddrIsComplete(virDomainDeviceInfoPtr info);
 int
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ba718b8..f0a904c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -114,6 +114,7 @@ virDomainVirtioSerialAddrRelease;
 virDomainVirtioSerialAddrReserve;
 virDomainVirtioSerialAddrSetAddControllers;
 virDomainVirtioSerialAddrSetCreate;
+virDomainVirtioSerialAddrSetCreateFromDomain;
 virDomainVirtioSerialAddrSetFree;
 
 
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index ee44d45..3033ab9 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -114,14 +114,7 @@ qemuDomainAssignVirtioSerialAddresses(virDomainDefPtr def,
 virDomainVirtioSerialAddrSetPtr addrs = NULL;
 qemuDomainObjPrivatePtr priv = NULL;
 
-if (!(addrs = virDomainVirtioSerialAddrSetCreate()))
-goto cleanup;
-
-if (virDomainVirtioSerialAddrSetAddControllers(addrs, def) < 0)
-goto cleanup;
-
-if (virDomainDeviceInfoIterate(def, virDomainVirtioSerialAddrReserve,
-   addrs) < 0)
+if (!(addrs = virDomainVirtioSerialAddrSetCreateFromDomain(def)))
 goto cleanup;
 
 VIR_DEBUG("Finished reserving existing ports");
-- 
2.7.4 (Apple Git-66)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 02/10] qemu_hotplug: generate vioserial address list on demand

[Tomasz Flendrich]

Dropping the caching of virtio serial address set.
Instead of using the cached address set, a function in qemu_hotplug.c
now recalculates it on demand.

Credit goes to Cole Robinson.
---
 src/qemu/qemu_hotplug.c | 38 ++
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 04e11b4..5101ae1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1470,38 +1470,52 @@ qemuDomainChrRemove(virDomainDefPtr vmdef,
 }
 
 static int
-qemuDomainAttachChrDeviceAssignAddr(qemuDomainObjPrivatePtr priv,
+qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def,
+qemuDomainObjPrivatePtr priv,
 virDomainChrDefPtr chr)
 {
+int ret = -1;
+virDomainVirtioSerialAddrSetPtr vioaddrs = NULL;
+
+if (!(vioaddrs = virDomainVirtioSerialAddrSetCreateFromDomain(def)))
+goto cleanup;
+
 if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
 chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) {
-if (virDomainVirtioSerialAddrAutoAssign(NULL, priv->vioserialaddrs,
+if (virDomainVirtioSerialAddrAutoAssign(NULL, vioaddrs,
 >info, true) < 0)
-return -1;
-return 1;
+goto cleanup;
+ret = 1;
 
 } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) {
 if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, >info) < 0)
-return -1;
-return 1;
+goto cleanup;
+ret = 1;
 
 } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) {
-if (virDomainVirtioSerialAddrAutoAssign(NULL, priv->vioserialaddrs,
+if (virDomainVirtioSerialAddrAutoAssign(NULL, vioaddrs,
 >info, false) < 0)
-return -1;
-return 1;
+goto cleanup;
+ret = 1;
 }
 
+if (ret == 1)
+goto cleanup;
+
 if (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL ||
 chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Unsupported address type for character device"));
-return -1;
+goto cleanup;
 }
 
-return 0;
+ret = 0;
+
+ cleanup:
+virDomainVirtioSerialAddrSetFree(vioaddrs);
+return ret;
 }
 
 int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
@@ -1522,7 +1536,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0)
 goto cleanup;
 
-if ((rc = qemuDomainAttachChrDeviceAssignAddr(priv, chr)) < 0)
+if ((rc = qemuDomainAttachChrDeviceAssignAddr(vm->def, priv, chr)) < 0)
 goto cleanup;
 if (rc == 1)
 need_release = true;
-- 
2.7.4 (Apple Git-66)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 03/10] qemu: remove vioserialaddrs caching

[Tomasz Flendrich]

Dropping the caching of virtio serial address set.
The cached set is not required anymore, because the set is now being
recalculated from the domain definition on demand, so the cache
can be deleted.

Credit goes to Cole Robinson.
---
 src/qemu/qemu_domain.c |  1 -
 src/qemu/qemu_domain.h |  1 -
 src/qemu/qemu_domain_address.c | 17 ++---
 3 files changed, 2 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 286f096..543f21a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1300,7 +1300,6 @@ qemuDomainObjPrivateFree(void *data)
 virCgroupFree(>cgroup);
 virDomainPCIAddressSetFree(priv->pciaddrs);
 virDomainCCWAddressSetFree(priv->ccwaddrs);
-virDomainVirtioSerialAddrSetFree(priv->vioserialaddrs);
 virDomainChrSourceDefFree(priv->monConfig);
 qemuDomainObjFreeJob(priv);
 VIR_FREE(priv->lockState);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 114db98..cbdbdfc 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -186,7 +186,6 @@ struct _qemuDomainObjPrivate {
 
 virDomainPCIAddressSetPtr pciaddrs;
 virDomainCCWAddressSetPtr ccwaddrs;
-virDomainVirtioSerialAddrSetPtr vioserialaddrs;
 
 virQEMUCapsPtr qemuCaps;
 char *lockState;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 3033ab9..ec8ce40 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -106,13 +106,11 @@ qemuDomainSetSCSIControllerModel(const virDomainDef *def,
 
 
 static int
-qemuDomainAssignVirtioSerialAddresses(virDomainDefPtr def,
-  virDomainObjPtr obj)
+qemuDomainAssignVirtioSerialAddresses(virDomainDefPtr def)
 {
 int ret = -1;
 size_t i;
 virDomainVirtioSerialAddrSetPtr addrs = NULL;
-qemuDomainObjPrivatePtr priv = NULL;
 
 if (!(addrs = virDomainVirtioSerialAddrSetCreateFromDomain(def)))
 goto cleanup;
@@ -137,13 +135,6 @@ qemuDomainAssignVirtioSerialAddresses(virDomainDefPtr def,
 goto cleanup;
 }
 
-if (obj && obj->privateData) {
-priv = obj->privateData;
-/* if this is the live domain object, we persist the addresses */
-virDomainVirtioSerialAddrSetFree(priv->vioserialaddrs);
-priv->vioserialaddrs = addrs;
-addrs = NULL;
-}
 ret = 0;
 
  cleanup:
@@ -1621,7 +1612,7 @@ qemuDomainAssignAddresses(virDomainDefPtr def,
   virDomainObjPtr obj,
   bool newDomain ATTRIBUTE_UNUSED)
 {
-if (qemuDomainAssignVirtioSerialAddresses(def, obj) < 0)
+if (qemuDomainAssignVirtioSerialAddresses(def) < 0)
 return -1;
 
 if (qemuDomainAssignSpaprVIOAddresses(def, qemuCaps) < 0)
@@ -1660,8 +1651,4 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
 >addr.pci) < 0)
 VIR_WARN("Unable to release PCI address on %s",
  NULLSTR(devstr));
-if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL &&
-virDomainVirtioSerialAddrRelease(priv->vioserialaddrs, info) < 0)
-VIR_WARN("Unable to release virtio-serial address on %s",
- NULLSTR(devstr));
 }
-- 
2.7.4 (Apple Git-66)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 00/10] Remove caching of address sets

[Tomasz Flendrich]

These patches delete the caching of pci, virtioSerial and ccw address sets.
I am deleting them, because they can be recalculated from the domain definition,
and there's no point in keeping redundant data, especially because handling
a persistent cache of addresses required using functions that released 
addresses.
These functions aren't useful anymore, so they are dropped too.


Tomasz Flendrich (10):
  add virDomainVirtioSerialAddrSetCreateFromDomain
  qemu_hotplug: generate vioserial address list on demand
  qemu: remove vioserialaddrs caching
  Add qemuDomainCCWAddrSetCreateFromDomain
  qemu_hotplug: generate ccw address list on demand
  qemu: remove ccwaddrs caching
  add qemuDomainPCIAddrSetCreateFromDomain
  qemu_hotplug: generate pci address list on demand
  qemu: remove pciaddrs caching
  Remove unused functions that release addresses

 src/conf/domain_addr.c | 123 ---
 src/conf/domain_addr.h |  15 +---
 src/libvirt_private.syms   |   4 +-
 src/qemu/qemu_domain.c |   3 -
 src/qemu/qemu_domain.h |   4 --
 src/qemu/qemu_domain_address.c | 160 ++---
 src/qemu/qemu_domain_address.h |  11 ++-
 src/qemu/qemu_hotplug.c| 153 ++-
 8 files changed, 200 insertions(+), 273 deletions(-)

-- 
2.7.4 (Apple Git-66)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 06/10] qemu: remove ccwaddrs caching

[Tomasz Flendrich]

Dropping the caching of ccw address set.
The cached set is not required anymore, because the set is now being
recalculated from the domain definition on demand, so the cache
can be deleted.
---
 src/qemu/qemu_domain.c |  1 -
 src/qemu/qemu_domain.h |  1 -
 src/qemu/qemu_domain_address.c | 21 ++---
 3 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 543f21a..5054ffe 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1299,7 +1299,6 @@ qemuDomainObjPrivateFree(void *data)
 
 virCgroupFree(>cgroup);
 virDomainPCIAddressSetFree(priv->pciaddrs);
-virDomainCCWAddressSetFree(priv->ccwaddrs);
 virDomainChrSourceDefFree(priv->monConfig);
 qemuDomainObjFreeJob(priv);
 VIR_FREE(priv->lockState);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index cbdbdfc..fdabbf9 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -185,7 +185,6 @@ struct _qemuDomainObjPrivate {
 char *pidfile;
 
 virDomainPCIAddressSetPtr pciaddrs;
-virDomainCCWAddressSetPtr ccwaddrs;
 
 virQEMUCapsPtr qemuCaps;
 char *lockState;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 5cc6d29..8434124 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -351,12 +351,10 @@ qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def)
  */
 static int
 qemuDomainAssignS390Addresses(virDomainDefPtr def,
-  virQEMUCapsPtr qemuCaps,
-  virDomainObjPtr obj)
+  virQEMUCapsPtr qemuCaps)
 {
 int ret = -1;
 virDomainCCWAddressSetPtr addrs = NULL;
-qemuDomainObjPrivatePtr priv = NULL;
 
 if (qemuDomainMachineIsS390CCW(def) &&
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
@@ -372,15 +370,6 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def,
 def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390);
 }
 
-if (obj && obj->privateData) {
-priv = obj->privateData;
-if (addrs) {
-/* if this is the live domain object, we persist the CCW 
addresses*/
-virDomainCCWAddressSetFree(priv->ccwaddrs);
-priv->ccwaddrs = addrs;
-addrs = NULL;
-}
-}
 ret = 0;
 
  cleanup:
@@ -1633,7 +1622,7 @@ qemuDomainAssignAddresses(virDomainDefPtr def,
 if (qemuDomainAssignSpaprVIOAddresses(def, qemuCaps) < 0)
 return -1;
 
-if (qemuDomainAssignS390Addresses(def, qemuCaps, obj) < 0)
+if (qemuDomainAssignS390Addresses(def, qemuCaps) < 0)
 return -1;
 
 qemuDomainAssignARMVirtioMMIOAddresses(def, qemuCaps);
@@ -1655,12 +1644,6 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
 if (!devstr)
 devstr = info->alias;
 
-if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
-qemuDomainMachineIsS390CCW(vm->def) &&
-virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW) &&
-virDomainCCWAddressReleaseAddr(priv->ccwaddrs, info) < 0)
-VIR_WARN("Unable to release CCW address on %s",
- NULLSTR(devstr));
 else if (virDeviceInfoPCIAddressPresent(info) &&
  virDomainPCIAddressReleaseSlot(priv->pciaddrs,
 >addr.pci) < 0)
-- 
2.7.4 (Apple Git-66)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 04/10] Add qemuDomainCCWAddrSetCreateFromDomain

[Tomasz Flendrich]

The address sets (pci, ccw, virtio serial) are currently cached
in qemu private data, but all the information required to recreate
these sets is in the domain definition. Therefore I am removing
the redundant data and adding a way to recalculate these sets.

Add a function that calculates the ccw address set
from the domain definition.
---
 src/qemu/qemu_domain_address.c | 31 +++
 src/qemu/qemu_domain_address.h |  4 
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index ec8ce40..5cc6d29 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -320,6 +320,28 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def,
 }
 }
 
+virDomainCCWAddressSetPtr
+qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def)
+{
+virDomainCCWAddressSetPtr addrs = NULL;
+
+if (!(addrs = virDomainCCWAddressSetCreate()))
+goto error;
+
+if (virDomainDeviceInfoIterate(def, virDomainCCWAddressValidate,
+   addrs) < 0)
+goto error;
+
+if (virDomainDeviceInfoIterate(def, virDomainCCWAddressAllocate,
+   addrs) < 0)
+goto error;
+
+return addrs;
+
+ error:
+virDomainCCWAddressSetFree(addrs);
+return NULL;
+}
 
 /*
  * Three steps populating CCW devnos
@@ -341,16 +363,9 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def,
 qemuDomainPrimeVirtioDeviceAddresses(
 def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
 
-if (!(addrs = virDomainCCWAddressSetCreate()))
-goto cleanup;
-
-if (virDomainDeviceInfoIterate(def, virDomainCCWAddressValidate,
-   addrs) < 0)
+if (!(addrs = qemuDomainCCWAddrSetCreateFromDomain(def)))
 goto cleanup;
 
-if (virDomainDeviceInfoIterate(def, virDomainCCWAddressAllocate,
-   addrs) < 0)
-goto cleanup;
 } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
 /* deal with legacy virtio-s390 */
 qemuDomainPrimeVirtioDeviceAddresses(
diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h
index ee326d7..11d6e92 100644
--- a/src/qemu/qemu_domain_address.h
+++ b/src/qemu/qemu_domain_address.h
@@ -41,6 +41,10 @@ void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
 virDomainDeviceInfoPtr info,
 const char *devstr);
 
+virDomainCCWAddressSetPtr
+qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def)
+ATTRIBUTE_NONNULL(1);
+
 # define __QEMU_DOMAIN_ADDRESS_H__
 
 #endif /* __QEMU_DOMAIN_ADDRESS_H__ */
-- 
2.7.4 (Apple Git-66)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/2] vz: add tcp and udp serial device support

[Maxim Nestratov]

31-May-16 12:38, Nikolay Shirokovskiy пишет:


Nikolay Shirokovskiy (2):
   vz: add mode of unix socket serial device to xml dump
   vz: add tcp and udp serial device support

  src/vz/vz_sdk.c | 111 
  1 file changed, 96 insertions(+), 15 deletions(-)



ACKed and pushed the series. Thanks.

Maxim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/9] vz: define and other cleanups

[Maxim Nestratov]

14-Jun-16 11:45, Nikolay Shirokovskiy пишет:


  'vz: cleanup loading domain code' is main patch and
  'vz: use domain list infrastructure to deal with private domain' is a
  preparation for it. Other patches are mostly minor bugfixes and cleanups.

  After the main patch driver lock is locked for very small time so massively
parallel domain definition is not serialized by vz driver anymore.

NOTE:
   Applied on top of:
   [PATCH] vz: get rid of unused home state variable in private domain obj
   [PATCH 0/5] fix destination domain synchronization

Nikolay Shirokovskiy (9):
   vz: remove unnecessary labels in simple API calls
   vz: restore accidentally removed locks around close callback calls
   vz: fix leaks in prlsdkCreate* functions
   vz: make error handling idiomatic in prlsdkCreateVm
   vz: use domain list infrastructure to deal with private domain
   vz: cleanup loading domain code
   vz: dont remove domain from list on client object error
   vz: use single variable for domain
   vz: don't fail unregister on sending event error

  src/vz/vz_driver.c | 154 +--
  src/vz/vz_sdk.c| 229 -
  src/vz/vz_sdk.h|  11 +--
  src/vz/vz_utils.c  |  58 ++
  src/vz/vz_utils.h  |   7 +-
  5 files changed, 194 insertions(+), 265 deletions(-)


Pushed the series. Thanks!

Maxim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 6/9] vz: cleanup loading domain code

[Maxim Nestratov]

14-Jun-16 11:45, Nikolay Shirokovskiy пишет:


   9c14a9ab introduced vzNewDomain function to enlist libvirt domain
object before actually creating vz sdk domain. Fix should fix
race on same vz sdk domain added event where libvirt domain object is
enlisted too. But later eb5e9c1e added locked checks for
adding livirtd domain object to list on vz sdk domain added event.
Thus now approach of 9c14a9ab is unnecessary complicated.

   See we have otherwise unuseful prlsdkGetDomainIds function only
to create minimal domain definition to create libvirt domain object.
Also vzNewDomain is difficult to use as it creates partially
constructed domain object.

   Let's move back to original approach where prlsdkLoadDomain do
all the necessary job. Another benefit is that we can now
take driver lock for bare minimum and in single place. Reducing
locking time have small disadvatage of double parsing on race
conditions which is typical if domain is added thru vz driver.
Well we have this double parse inevitably with current vz sdk api
on any domain updates so i would not take it here seriously.

   Performance events subscribtion is done before locked check and
therefore could be done twice on races but this is not the problem.

Signed-off-by: Nikolay Shirokovskiy 
---
  src/vz/vz_driver.c |  14 +---
  src/vz/vz_sdk.c| 184 +
  src/vz/vz_sdk.h|   9 ++-
  src/vz/vz_utils.c  |  24 ---
  src/vz/vz_utils.h  |   4 --
  5 files changed, 94 insertions(+), 141 deletions(-)


Yeah, looks reasonable and works fine. ACK

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 7/9] vz: dont remove domain from list on client object error

[Maxim Nestratov]

14-Jun-16 11:45, Nikolay Shirokovskiy пишет:


After domain is in the domains list let's keep it there. This
is approach taken by qemu driver and vz vzDomainMigrateFinish3Params too.
It quite reasonable, driver domain object is fully constructed and
can be discovered by client later.

Signed-off-by: Nikolay Shirokovskiy 
---
  src/vz/vz_driver.c | 8 ++--
  1 file changed, 2 insertions(+), 6 deletions(-)


ACK

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 9/9] vz: don't fail unregister on sending event error

[Maxim Nestratov]

14-Jun-16 12:09, Nikolay Shirokovskiy пишет:



On 14.06.2016 11:46, Nikolay Shirokovskiy wrote:

Signed-off-by: Nikolay Shirokovskiy 
---
  src/vz/vz_sdk.c | 21 +
  1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 612a059..12691ba 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1669,21 +1669,19 @@ prlsdkUpdateDomain(vzDriverPtr driver, virDomainObjPtr 
dom)
  return prlsdkLoadDomain(driver, pdom->sdkdom, dom) ? 0 : -1;
  }
  
-static int prlsdkSendEvent(vzDriverPtr driver,

-   virDomainObjPtr dom,
-   virDomainEventType lvEventType,
-   int lvEventTypeDetails)
+static void
+prlsdkSendEvent(vzDriverPtr driver,
+virDomainObjPtr dom,
+virDomainEventType lvEventType,
+int lvEventTypeDetails)
  {
-virObjectEventPtr event = NULL;
+virObjectEventPtr event;
  
  event = virDomainEventLifecycleNewFromObj(dom,

lvEventType,
lvEventTypeDetails);
  if (!event)
-return -1;

Phhh, if (event) of course.


ACK with phhh

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 8/9] vz: use single variable for domain

[Maxim Nestratov]

14-Jun-16 11:45, Nikolay Shirokovskiy пишет:


Signed-off-by: Nikolay Shirokovskiy 
---
  src/vz/vz_driver.c | 23 ++-
  1 file changed, 10 insertions(+), 13 deletions(-)

ACK

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 5/9] vz: use domain list infrastructure to deal with private domain

[Maxim Nestratov]

14-Jun-16 11:45, Nikolay Shirokovskiy пишет:


Signed-off-by: Nikolay Shirokovskiy 
---
  src/vz/vz_driver.c |  7 ++-
  src/vz/vz_sdk.c| 13 -
  src/vz/vz_sdk.h|  2 --
  src/vz/vz_utils.c  | 34 ++
  src/vz/vz_utils.h  |  3 +++
  5 files changed, 35 insertions(+), 24 deletions(-)


ACK

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] security: compilation error due to wrong parameter for vah_add_path().

[Julio Faracco]

The commit da665fbd introduced virStorageSourcePtr inside the structure
_virDomainFSDef. This is causing an error when libvirt is being compiled.

make[3]: Entering directory `/media/julio/8d65c59c-6ade-4740-9cdc-38016a4cb8ae
/home/julio/Desktop/virt/libvirt/src'
  CC   security/virt_aa_helper-virt-aa-helper.o
security/virt-aa-helper.c: In function 'get_files':
security/virt-aa-helper.c:1087:13: error: passing argument 2 of 'vah_add_path'
from incompatible pointer type [-Werror]
 if (vah_add_path(, fs->src, "rw", true) != 0)
 ^
security/virt-aa-helper.c:732:1: note: expected 'const char *' but argument is
of type 'virStorageSourcePtr'
 vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool
recursive)
 ^
cc1: all warnings being treated as errors

Adding the attribute "path" from virStorageSourcePtr fixes this issue.

Signed-off-by: Julio Faracco 
---
 src/security/virt-aa-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 9eafaee..805d7d7 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1084,7 +1084,7 @@ get_files(vahControl * ctl)
 /* We don't need to add deny rw rules for readonly mounts,
  * this can only lead to troubles when mounting / readonly.
  */
-if (vah_add_path(, fs->src, "rw", true) != 0)
+if (vah_add_path(, fs->src->path, "rw", true) != 0)
 goto cleanup;
 }
 }
-- 
1.9.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/9] vz: make error handling idiomatic in prlsdkCreateVm

[Maxim Nestratov]

14-Jun-16 11:45, Nikolay Shirokovskiy пишет:


Signed-off-by: Nikolay Shirokovskiy 
---
  src/vz/vz_sdk.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)


ACK

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/9] vz: fix leaks in prlsdkCreate* functions

[Maxim Nestratov]

14-Jun-16 11:45, Nikolay Shirokovskiy пишет:


Signed-off-by: Nikolay Shirokovskiy 
---
  src/vz/vz_sdk.c | 4 
  1 file changed, 4 insertions(+)


ACK

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/9] vz: restore accidentally removed locks around close callback calls

[Maxim Nestratov]

14-Jun-16 11:45, Nikolay Shirokovskiy пишет:


Signed-off-by: Nikolay Shirokovskiy 
---
  src/vz/vz_driver.c | 5 +
  1 file changed, 5 insertions(+)

ACK

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/9] vz: remove unnecessary labels in simple API calls

[Maxim Nestratov]

14-Jun-16 11:45, Nikolay Shirokovskiy пишет:


Signed-off-by: Nikolay Shirokovskiy 
---
  src/vz/vz_driver.c | 103 ++---
  1 file changed, 34 insertions(+), 69 deletions(-)


I'm not sure it is necessary, but will not object pushing this not to 
violate your intents to make the world better.

Thus, ACK

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Changes to support Veritas HyperScale (VxHS) block device protocol with qemu-kvm.

[Ashish Mittal]

>From 9036f749f12d3bf4bf08e7e55b6d98109dd5e5c0 Mon Sep 17 00:00:00 2001
From: Ashish Mittal 
Date: Mon, 18 Jul 2016 16:21:37 -0700
Subject: [PATCH] Changes to support Veritas HyperScale (VxHS) block device 
protocol with qemu-kvm.

Changes to support Veritas HyperScale (VxHS) block device protocol with 
qemu-kvm.
Sample XML for a vxhs vdisk is as follows:









f7dea4ad-b3b1-4a54-ad78-468b248ebdd8




---
 docs/formatdomain.html.in  | 12 +++--
 docs/schemas/domaincommon.rng  |  1 +
 src/qemu/qemu_command.c| 51 ++
 src/qemu/qemu_driver.c |  3 ++
 src/qemu/qemu_parse_command.c  | 21 +
 src/util/virstoragefile.c  |  4 +-
 src/util/virstoragefile.h  |  1 +
 .../qemuxml2argv-disk-drive-network-vxhs.args  | 24 ++
 .../qemuxml2argv-disk-drive-network-vxhs.xml   | 35 +++
 tests/qemuxml2argvtest.c   |  1 +
 10 files changed, 149 insertions(+), 4 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 59a8bb9..13705fb 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2171,9 +2171,9 @@
   
   The protocol attribute specifies the protocol to
   access to the requested image. Possible values are "nbd",
-  "iscsi", "rbd", "sheepdog" or "gluster".  If the
-  protocol attribute is "rbd", "sheepdog" or
-  "gluster", an additional attribute name is
+  "iscsi", "rbd", "sheepdog", "gluster" or "vxhs".  If the
+  protocol attribute is "rbd", "sheepdog", "gluster"
+  or "vxhs", an additional attribute name is
   mandatory to specify which volume/image will be used. For "nbd",
   the name attribute is optional. For "iscsi"
   (since 1.0.4), the name
@@ -2283,6 +2283,12 @@
  only one 
  24007 
   
+  
+ vxhs 
+ a server running Veritas HyperScale daemon 
+ one or more 
+  
+  
 
 
 gluster supports "tcp", "rdma", "unix" as valid values for the
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 348dbfe..f279bf2 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1416,6 +1416,7 @@
 ftp
 ftps
 tftp
+vxhs
   
 
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fe4bb88..a52b78c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -497,6 +497,7 @@ qemuNetworkDriveGetPort(int protocol,
 return 0;

 case VIR_STORAGE_NET_PROTOCOL_RBD:
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
 /* not applicable */
@@ -889,6 +890,56 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
 ret = virBufferContentAndReset();
 break;

+case VIR_STORAGE_NET_PROTOCOL_VXHS:
+if (VIR_ALLOC(uri) < 0)
+goto cleanup;
+
+if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) {
+if (VIR_STRDUP(uri->scheme,
+   
virStorageNetProtocolTypeToString(src->protocol)) < 0)
+goto cleanup;
+} else {
+if (virAsprintf(>scheme, "%s+%s",
+
virStorageNetProtocolTypeToString(src->protocol),
+
virStorageNetHostTransportTypeToString(src->hosts->transport)) < 0)
+goto cleanup;
+}
+
+if (src->path) {
+if (src->volume) {
+if (virAsprintf(>path, "/%s%s",
+src->volume, src->path) < 0)
+goto cleanup;
+} else {
+if (virAsprintf(>path, "%s%s",
+src->path[0] == '/' ? "" : "/",
+src->path) < 0)
+goto cleanup;
+}
+}
+
+if (src->hosts->socket &&
+virAsprintf(>query, "socket=%s", src->hosts->socket) < 0)
+goto cleanup;
+
+if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0)
+goto cleanup;
+
+for (i = 0; i < src->nhosts; i++) {
+if ((uri->port = qemuNetworkDriveGetPort(src->protocol, 

[libvirt] ANNOUNCE: libvirt 1.3.3.2 maintenance release

[Cole Robinson]

libvirt 1.3.3.2 is now available. This is a maintenance release of
libvirt 1.3.3 with additional bugfixes that have accumulated
upstream since the initial release.

This release can be downloaded at:

http://libvirt.org/sources/stable_updates/libvirt-1.3.3.2.tar.gz

Changes in this version:

* spec: Fix indentation
* conf: Allow disks with identical WWN or serial
* libvirt.spec.in: require systemd-container on >= f24
* qemu: SCSI hostdev hot-plug: Fix automatic creation of SCSI controllers
* qemu: hot-plug: Fix broken SCSI disk hot-plug
* qemu: Let empty default VNC password work as documented
* virCgroupValidateMachineGroup: Reflect change in CGroup struct naming
* spec: Advertise nvram paths of official fedora edk2 builds
* qemu: hotplug: wait for the tray to eject only for drives with a tray
* qemu: hotplug: Fix error reported when cdrom tray is locked
* qemu: hotplug: Extract code for waiting for tray eject
* qemu: hotplug: Report error if we hit tray status timeout
* qemu: hotplug: Skip waiting for tray opening if qemu doesn't notify us
* qemu: process: Fix and improve disk data extraction
* qemu: Move and rename qemuDomainCheckEjectableMedia to
  qemuProcessRefreshDisks
* qemu: Extract more information about qemu drives
* qemu: Move struct qemuDomainDiskInfo to qemu_domain.h
* qemu: process: Refresh ejectable media tray state on VM start
* iscsi: Remove initiatoriqn from virISCSIScanTargets
* util: Remove disabling of autologin for iscsi-targets
* iscsi: Add exit status checking for virISCSIGetSession
* util: Add exitstatus parameter to virCommandRunRegex
* xlconfigtests: use qemu-xen in all test data files
* libxl: don't attempt to probe a non-existent emulator
* Fix tests to include video ram size
* Fill out default vram in DeviceDefPostParse
* Call per-device post-parse callback even on implicit video
* Move virDomainDefPostParseInternal after virDomainDeviceDefPostParse
* conf: use VIR_APPEND_ELEMENT in virDomainDefAddImplicitVideo
* conf: reduce indentation in virDomainDefAddImplicitVideo
* domain_conf: fix migration/managedsave with usb keyboard

For info about past maintenance releases, see:

http://wiki.libvirt.org/page/Maintenance_Releases

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] ANNOUNCE: libvirt 1.2.18.4 maintenance release

[Cole Robinson]

libvirt 1.2.18.4 is now available. This is a maintenance release of
libvirt 1.2.18 with additional bugfixes that have accumulated
upstream since the initial release.

This release can be downloaded at:

http://libvirt.org/sources/stable_updates/libvirt-1.2.18.4.tar.gz

Changes in this version:

* qemu: Let empty default VNC password work as documented
* spec: Fix error in last backport
* spec: Advertise nvram paths of official fedora edk2 builds

For info about past maintenance releases, see:

http://wiki.libvirt.org/page/Maintenance_Releases

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] network: Added hook for network modification event

[Cole Robinson]

On 07/13/2016 07:06 AM, Khramov Anton wrote:
> From: Anton Khramov 
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1181539
> ---
>  docs/hooks.html.in  | 2 ++
>  src/network/bridge_driver.c | 6 ++
>  src/util/virhook.c  | 3 ++-
>  src/util/virhook.h  | 1 +
>  4 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/hooks.html.in b/docs/hooks.html.in
> index 1aae00c..d4f4ac3 100644
> --- a/docs/hooks.html.in
> +++ b/docs/hooks.html.in
> @@ -250,6 +250,8 @@
>/etc/libvirt/hooks/network network_name plugged begin -
>  Please note, that in this case, the script is passed both network and
>  domain XMLs on its stdin.
> +  When network is updated, the hook script is called as:
> +  /etc/libvirt/hooks/network network_name updated begin 
> -
>When the domain from previous case is shutting down, the interface
>  is unplugged. This leads to another script invocation:
>/etc/libvirt/hooks/network network_name unplugged begin 
> -
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 0fd2095..61ab17b 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -3460,6 +3460,12 @@ networkUpdate(virNetworkPtr net,
>  goto cleanup;
>  }
>  }
> +
> +/* call the 'updated' network hook script */
> +if (networkRunHook(network, NULL, NULL, VIR_HOOK_NETWORK_OP_UPDATED,
> +   VIR_HOOK_SUBOP_BEGIN) < 0)
> +goto cleanup;
> +
>  ret = 0;
>   cleanup:
>  virNetworkObjEndAPI();
> diff --git a/src/util/virhook.c b/src/util/virhook.c
> index d37d6da..a8422a2 100644
> --- a/src/util/virhook.c
> +++ b/src/util/virhook.c
> @@ -93,7 +93,8 @@ VIR_ENUM_IMPL(virHookNetworkOp, VIR_HOOK_NETWORK_OP_LAST,
>"started",
>"stopped",
>"plugged",
> -  "unplugged")
> +  "unplugged",
> +  "updated")
>  
>  static int virHooksFound = -1;
>  
> diff --git a/src/util/virhook.h b/src/util/virhook.h
> index 550ef84..4015426 100644
> --- a/src/util/virhook.h
> +++ b/src/util/virhook.h
> @@ -82,6 +82,7 @@ typedef enum {
>  VIR_HOOK_NETWORK_OP_STOPPED,/* network has stopped */
>  VIR_HOOK_NETWORK_OP_IFACE_PLUGGED,  /* an interface has been plugged 
> into the network */
>  VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED,/* an interface was unplugged 
> from the network */
> +VIR_HOOK_NETWORK_OP_UPDATED,/* network has been updated */
>  
>  VIR_HOOK_NETWORK_OP_LAST,
>  } virHookNetworkOpType;
> 

ACK this looks good to me. CCing laine to see if he wants to ACK when he's
back online, if not I'll push it in a week.

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 0/4] devices: filesystem: type volume

[maxim nestratov]

15-Jul-16 11:00, Ján Tomko пишет:


On Thu, Jul 14, 2016 at 04:52:37PM +0300, Olga Krishtal wrote:

The patches introduce new filesystem type volume:

 
 
 


This makes possible to use storage volumes as the source for disks.

v2:
-split patches
-rebased

v3:
- fixed check for template fs.
- refactoring




Olga Krishtal (4):
 filesystem: adds possibility to use storage pool as fs source
 devices: filesystems: added volume type


ACK to these two.


 vz: refactoring of prlsdkCreateCt
 vz: support filesystem type volume



These look good to me.

Jan


ACK to vz part as well. Works fine. The series is pushed now with a tiny 
fix of functions alphabetial arder in #1 patch.

Thanks!

Maxim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH python] allow pkg-config binary to be set by env

[Cole Robinson]

ping reviewers

On 06/28/2016 03:10 PM, Cole Robinson wrote:
> From: Markus Rothe 
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1350523
> ---
>  setup.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/setup.py b/setup.py
> index 099b1e0..a4cfb88 100755
> --- a/setup.py
> +++ b/setup.py
> @@ -28,6 +28,8 @@ _pkgcfg = -1
>  def get_pkgcfg(do_fail=True):
>  global _pkgcfg
>  if _pkgcfg == -1:
> +_pkgcfg = os.getenv('PKG_CONFIG')
> +if _pkgcfg is None:
>  _pkgcfg = distutils.spawn.find_executable("pkg-config")
>  if _pkgcfg is None and do_fail:
>  raise Exception("pkg-config binary is required to compile 
> libvirt-python")
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] vz: fixed null-pointer dereference in applying graphic params

[maxim nestratov]

29-Jun-16 20:23, Olga Krishtal пишет:


Signed-off-by: Olga Krishtal 
---
  src/vz/vz_sdk.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f4c2b3b..848bfb9 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2790,6 +2790,7 @@ static int prlsdkApplyGraphicsParams(PRL_HANDLE sdkdom,
  if (!gr) {
  pret = PrlVmCfg_SetVNCMode(sdkdom, PRD_DISABLED);
  prlsdkCheckRetExit(pret, -1);
+return 0;
  }
  
  pret = PrlVmCfg_SetVNCPassword(sdkdom, gr->data.vnc.auth.passwd ? : "");


ACK and pushed now. Thanks!

Maxim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 1/4] filesystem: adds possibility to use storage pool as fs source

[maxim nestratov]

14-Jul-16 16:52, Olga Krishtal пишет:


Signed-off-by: Olga Krishtal 
---
  src/conf/domain_audit.c|  4 ++--
  src/conf/domain_conf.c | 34 ---
  src/conf/domain_conf.h |  3 ++-
  src/libvirt_private.syms   |  1 +
  src/lxc/lxc_cgroup.c   |  2 +-
  src/lxc/lxc_container.c| 50 +++---
  src/lxc/lxc_controller.c   | 18 -
  src/lxc/lxc_native.c   |  5 +++--
  src/lxc/lxc_process.c  |  4 ++--
  src/openvz/openvz_conf.c   |  6 +++---
  src/openvz/openvz_driver.c |  4 ++--
  src/qemu/qemu_command.c|  2 +-
  src/vbox/vbox_common.c |  6 +++---
  src/vmx/vmx.c  |  6 +++---
  src/vz/vz_sdk.c| 12 +--
  15 files changed, 90 insertions(+), 67 deletions(-)

diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 6ad0acb..53a58ac 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -296,8 +296,8 @@ virDomainAuditFS(virDomainObjPtr vm,
   const char *reason, bool success)
  {
  virDomainAuditGenericDev(vm, "fs",
- oldDef ? oldDef->src : NULL,
- newDef ? newDef->src : NULL,
+ oldDef ? oldDef->src->path : NULL,
+ newDef ? newDef->src->path : NULL,
   reason, success);
  }
  
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

index 9f7b906..0b33b3b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1698,12 +1698,31 @@ void 
virDomainControllerDefFree(virDomainControllerDefPtr def)
  VIR_FREE(def);
  }
  
+virDomainFSDefPtr

+virDomainFSDefNew(void)
+{
+virDomainFSDefPtr ret;
+
+if (VIR_ALLOC(ret) < 0)
+return NULL;
+
+if (VIR_ALLOC(ret->src) < 0)
+goto cleanup;
+
+return ret;
+
+ cleanup:
+virDomainFSDefFree(ret);
+return NULL;
+
+}
+
  void virDomainFSDefFree(virDomainFSDefPtr def)
  {
  if (!def)
  return;
  
-VIR_FREE(def->src);

+virStorageSourceFree(def->src);
  VIR_FREE(def->dst);
  virDomainDeviceInfoClear(>info);
  
@@ -8528,7 +8547,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
  
  ctxt->node = node;
  
-if (VIR_ALLOC(def) < 0)

+if (!(def = virDomainFSDefNew()))
  return NULL;
  
  type = virXMLPropString(node, "type");

@@ -8655,7 +8674,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
  goto error;
  }
  
-def->src = source;

+def->src->path = source;
  source = NULL;
  def->dst = target;
  target = NULL;
@@ -20144,6 +20163,7 @@ virDomainFSDefFormat(virBufferPtr buf,
  const char *accessmode = 
virDomainFSAccessModeTypeToString(def->accessmode);
  const char *fsdriver = virDomainFSDriverTypeToString(def->fsdriver);
  const char *wrpolicy = virDomainFSWrpolicyTypeToString(def->wrpolicy);
+const char *src = def->src->path;
  
  if (!type) {

  virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -20180,22 +20200,22 @@ virDomainFSDefFormat(virBufferPtr buf,
  case VIR_DOMAIN_FS_TYPE_MOUNT:
  case VIR_DOMAIN_FS_TYPE_BIND:
  virBufferEscapeString(buf, "\n",
-  def->src);
+  src);
  break;
  
  case VIR_DOMAIN_FS_TYPE_BLOCK:

  virBufferEscapeString(buf, "\n",
-  def->src);
+  src);
  break;
  
  case VIR_DOMAIN_FS_TYPE_FILE:

  virBufferEscapeString(buf, "\n",
-  def->src);
+  src);
  break;
  
  case VIR_DOMAIN_FS_TYPE_TEMPLATE:

  virBufferEscapeString(buf, "\n",
-  def->src);
+  src);
  break;
  
  case VIR_DOMAIN_FS_TYPE_RAM:

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ba0ad5f..281abba 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -802,7 +802,7 @@ struct _virDomainFSDef {
  int wrpolicy; /* enum virDomainFSWrpolicy */
  int format; /* virStorageFileFormat */
  unsigned long long usage; /* in bytes */
-char *src;
+virStorageSourcePtr src;
  char *dst;
  bool readonly;
  virDomainDeviceInfo info;
@@ -2474,6 +2474,7 @@ virDomainDiskDefPtr 
virDomainDiskFindByBusAndDst(virDomainDefPtr def,
   int bus,
   char *dst);
  void virDomainControllerDefFree(virDomainControllerDefPtr def);
+virDomainFSDefPtr virDomainFSDefNew(void);
  virDomainControllerDefPtr
  virDomainControllerDefNew(virDomainControllerType type);
  void virDomainFSDefFree(virDomainFSDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ccb4c5e..5f3a809 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms

Re: [libvirt] [PATCH 0/5] fix destination domain synchronization

[maxim nestratov]

08-Jun-16 10:17, Nikolay Shirokovskiy пишет:


  Patch 4 is main one. Others are fixes and ground works.

Nikolay Shirokovskiy (5):
   vz: don't pass empty and unused fields in migration cookie
   vz: fix missed defined domain event
   vz: fix memory leaks in prlsdkLoadDomains
   vz: fix destination domain synchronization
   vz: issue domain undefined event on finish step if needed

  src/vz/vz_driver.c | 175 -
  src/vz/vz_sdk.c|  28 ++---
  src/vz/vz_sdk.h|   5 ++
  3 files changed, 118 insertions(+), 90 deletions(-)


ACK and pushed now with suggested fixup. Thanks!

Maxim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 8/9] openvz: Remove need for strtok_r in openvzGenerateContainerVethName

[John Ferlan]

Rather than use strtok_i to parse the VethName, use the virStringSplitCount
helper. The Coverity checker would "mark" that it's possible that the results
of the strtok_r may not be what's expected if openvzReadVPSConfigParam
returned a NULL (although logically it shouldn't, but Coverity got lost).

Signed-off-by: John Ferlan 
---
 src/openvz/openvz_driver.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index f9a89cf..22dc333 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -798,20 +798,25 @@ openvzGenerateContainerVethName(int veid)
 {
 char *temp = NULL;
 char *name = NULL;
+char **tmp = NULL;
+size_t i;
+size_t ntmp = 0;
 
 /* try to get line "^NETIF=..." from config */
 if (openvzReadVPSConfigParam(veid, "NETIF", ) <= 0) {
 ignore_value(VIR_STRDUP(name, "eth0"));
 } else {
-char *saveptr = NULL;
-char *s;
 int max = 0;
 
+if (!(tmp = virStringSplitCount(temp, ";", 0, )))
+goto cleanup;
+
 /* get maximum interface number (actually, it is the last one) */
-for (s = strtok_r(temp, ";", ); s; s = strtok_r(NULL, ";", 
)) {
+for (i = 0; i < ntmp; i++) {
 int x;
 
-if (sscanf(s, "ifname=eth%d", ) != 1) return NULL;
+if (sscanf(tmp[i], "ifname=eth%d", ) != 1)
+goto cleanup;
 if (x > max) max = x;
 }
 
@@ -819,8 +824,9 @@ openvzGenerateContainerVethName(int veid)
 ignore_value(virAsprintf(, "eth%d", max + 1));
 }
 
+ cleanup:
 VIR_FREE(temp);
-
+virStringFreeListCount(tmp, ntmp);
 return name;
 }
 
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 6/9] esx: Replace strtok_r usage with virStringSplitCount

[John Ferlan]

Rather than use strtok_r using an input "path" variable which may or may not
be NULL and thus alter the results, use the virStringSplitCount API in order
to parse the path.

Found by Coverity

Signed-off-by: John Ferlan 
---
 src/esx/esx_vi.c | 61 
 1 file changed, 26 insertions(+), 35 deletions(-)

diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index a28ac7b..f151dc4 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -1179,22 +1179,17 @@ esxVI_Context_LookupManagedObjects(esxVI_Context *ctx)
 int
 esxVI_Context_LookupManagedObjectsByPath(esxVI_Context *ctx, const char *path)
 {
+char **tmp = NULL;
+size_t idx, ntmp = 0;
 int result = -1;
-char *tmp = NULL;
-char *saveptr = NULL;
-char *previousItem = NULL;
-char *item = NULL;
 virBuffer buffer = VIR_BUFFER_INITIALIZER;
 esxVI_ManagedObjectReference *root = NULL;
 esxVI_Folder *folder = NULL;
 
-if (VIR_STRDUP(tmp, path) < 0)
+if (!(tmp = virStringSplitCount(path, "/", 0, )))
 goto cleanup;
 
-/* Lookup Datacenter */
-item = strtok_r(tmp, "/", );
-
-if (!item) {
+if (ntmp == 0) {
 virReportError(VIR_ERR_INVALID_ARG,
_("Path '%s' does not specify a datacenter"), path);
 goto cleanup;
@@ -1202,11 +1197,12 @@ esxVI_Context_LookupManagedObjectsByPath(esxVI_Context 
*ctx, const char *path)
 
 root = ctx->service->rootFolder;
 
-while (!ctx->datacenter && item) {
+for (idx = 0; idx < ntmp && !ctx->datacenter; ++idx) {
+
 esxVI_Folder_Free();
 
-/* Try to lookup item as a folder */
-if (esxVI_LookupFolder(ctx, item, root, NULL, ,
+/* Try to lookup entry as a folder */
+if (esxVI_LookupFolder(ctx, tmp[idx], root, NULL, ,
esxVI_Occurrence_OptionalItem) < 0) {
 goto cleanup;
 }
@@ -1219,8 +1215,9 @@ esxVI_Context_LookupManagedObjectsByPath(esxVI_Context 
*ctx, const char *path)
 root = folder->_reference;
 folder->_reference = NULL;
 } else {
-/* Try to lookup item as a datacenter */
-if (esxVI_LookupDatacenter(ctx, item, root, NULL, >datacenter,
+/* Try to lookup entry as a datacenter */
+if (esxVI_LookupDatacenter(ctx, tmp[idx], root, NULL,
+   >datacenter,
esxVI_Occurrence_OptionalItem) < 0) {
 goto cleanup;
 }
@@ -1230,10 +1227,7 @@ esxVI_Context_LookupManagedObjectsByPath(esxVI_Context 
*ctx, const char *path)
 if (virBufferUse() > 0)
 virBufferAddChar(, '/');
 
-virBufferAdd(, item, -1);
-
-previousItem = item;
-item = strtok_r(NULL, "/", );
+virBufferAdd(, tmp[idx], -1);
 }
 
 if (!ctx->datacenter) {
@@ -1248,9 +1242,10 @@ esxVI_Context_LookupManagedObjectsByPath(esxVI_Context 
*ctx, const char *path)
 ctx->datacenterPath = virBufferContentAndReset();
 
 /* Lookup (Cluster)ComputeResource */
-if (!item) {
+if (!tmp[idx]) {
 virReportError(VIR_ERR_INVALID_ARG,
-   _("Path '%s' does not specify a compute resource"), 
path);
+   _("Path '%s' does not specify a compute resource"),
+   path);
 goto cleanup;
 }
 
@@ -1259,11 +1254,11 @@ esxVI_Context_LookupManagedObjectsByPath(esxVI_Context 
*ctx, const char *path)
 
 root = ctx->datacenter->hostFolder;
 
-while (!ctx->computeResource && item) {
+for (; idx < ntmp && !ctx->computeResource; ++idx) {
 esxVI_Folder_Free();
 
-/* Try to lookup item as a folder */
-if (esxVI_LookupFolder(ctx, item, root, NULL, ,
+/* Try to lookup entry as a folder */
+if (esxVI_LookupFolder(ctx, tmp[idx], root, NULL, ,
esxVI_Occurrence_OptionalItem) < 0) {
 goto cleanup;
 }
@@ -1276,8 +1271,8 @@ esxVI_Context_LookupManagedObjectsByPath(esxVI_Context 
*ctx, const char *path)
 root = folder->_reference;
 folder->_reference = NULL;
 } else {
-/* Try to lookup item as a compute resource */
-if (esxVI_LookupComputeResource(ctx, item, root, NULL,
+/* Try to lookup entry as a compute resource */
+if (esxVI_LookupComputeResource(ctx, tmp[idx], root, NULL,
 >computeResource,
 esxVI_Occurrence_OptionalItem) < 
0) {
 goto cleanup;
@@ -1288,10 +1283,7 @@ esxVI_Context_LookupManagedObjectsByPath(esxVI_Context 
*ctx, const char *path)
 if (virBufferUse() > 0)
 virBufferAddChar(, '/');
 
-virBufferAdd(, item, -1);
-
-previousItem = item;
-item = strtok_r(NULL, "/", );
+virBufferAdd(, 

[libvirt] [PATCH 7/9] conf: Add ignore_value to virDomainDeviceInfoIterate calls for Clear helpers

[John Ferlan]

Since we don't necessarily care about the status return, just add an
ignore_value to the calls for virDomainDefClear* calls

Found by Coverity

Signed-off-by: John Ferlan 
---
 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 87a9a8d..3cf1f59 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4736,17 +4736,17 @@ virDomainDefValidate(virDomainDefPtr def,
 
 void virDomainDefClearPCIAddresses(virDomainDefPtr def)
 {
-virDomainDeviceInfoIterate(def, virDomainDeviceInfoClearPCIAddress, NULL);
+ignore_value(virDomainDeviceInfoIterate(def, 
virDomainDeviceInfoClearPCIAddress, NULL));
 }
 
 void virDomainDefClearCCWAddresses(virDomainDefPtr def)
 {
-virDomainDeviceInfoIterate(def, virDomainDeviceInfoClearCCWAddress, NULL);
+ignore_value(virDomainDeviceInfoIterate(def, 
virDomainDeviceInfoClearCCWAddress, NULL));
 }
 
 void virDomainDefClearDeviceAliases(virDomainDefPtr def)
 {
-virDomainDeviceInfoIterate(def, virDomainDeviceInfoClearAlias, NULL);
+ignore_value(virDomainDeviceInfoIterate(def, 
virDomainDeviceInfoClearAlias, NULL));
 }
 
 
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 9/9] openvz: Remove strtok_r calls from openvzReadNetworkConf

[John Ferlan]

Rather than rely on strtok_r calls, use virStringSplitCount in order
to process the conf variable. Coverity complains because it doesn't
know that the returned temp variable in openvzReadVPSConfigParam
should be non-NULL.

Signed-off-by: John Ferlan 
---
 src/openvz/openvz_conf.c | 33 -
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 99ce95c..6562fcc 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -200,7 +200,9 @@ openvzReadNetworkConf(virDomainDefPtr def,
 int ret;
 virDomainNetDefPtr net = NULL;
 char *temp = NULL;
-char *token, *saveptr = NULL;
+char **tmp = NULL;
+size_t ntmp = 0;
+size_t i;
 
 /*parse routing network configuration*
  * Sample from config:
@@ -214,20 +216,23 @@ openvzReadNetworkConf(virDomainDefPtr def,
veid);
 goto error;
 } else if (ret > 0) {
-token = strtok_r(temp, " ", );
-while (token != NULL) {
+if (!(tmp = virStringSplitCount(temp, " ", 0, )))
+goto error;
+
+for (i = 0; i < ntmp; i++) {
 if (VIR_ALLOC(net) < 0)
 goto error;
 
 net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
-if (virDomainNetAppendIPAddress(net, token, AF_UNSPEC, 0) < 0)
+if (virDomainNetAppendIPAddress(net, tmp[i], AF_UNSPEC, 0) < 0)
 goto error;
 
 if (VIR_APPEND_ELEMENT_COPY(def->nets, def->nnets, net) < 0)
 goto error;
-
-token = strtok_r(NULL, " ", );
 }
+virStringFreeListCount(tmp, ntmp);
+ntmp = 0;
+tmp = NULL;
 }
 
 /*parse bridge devices*/
@@ -242,15 +247,17 @@ openvzReadNetworkConf(virDomainDefPtr def,
veid);
 goto error;
 } else if (ret > 0) {
-token = strtok_r(temp, ";", );
-while (token != NULL) {
-/*add new device to list*/
+if (!(tmp = virStringSplitCount(temp, " ", 0, )))
+goto error;
+
+for (i = 0; i < ntmp; i++) {
+/* add new device to list */
 if (VIR_ALLOC(net) < 0)
 goto error;
 
 net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
 
-char *p = token;
+char *p = tmp[i];
 char cpy_temp[32];
 int len;
 
@@ -313,21 +320,21 @@ openvzReadNetworkConf(virDomainDefPtr def,
 }
 }
 p = ++next;
-} while (p < token + strlen(token));
+} while (p < tmp[i] + strlen(tmp[i]));
 
 if (VIR_APPEND_ELEMENT_COPY(def->nets, def->nnets, net) < 0)
 goto error;
-
-token = strtok_r(NULL, ";", );
 }
 }
 
 VIR_FREE(temp);
+virStringFreeListCount(tmp, ntmp);
 
 return 0;
 
  error:
 VIR_FREE(temp);
+virStringFreeListCount(tmp, ntmp);
 virDomainNetDefFree(net);
 return -1;
 }
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 4/9] conf: Need to check for glisten before accessing

[John Ferlan]

When formatting the graphics data for TYPE_SPICE, check if the glisten
is NULL before blindly referencing

Found by Coverity

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 82d9d1d..87a9a8d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22150,6 +22150,12 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
 break;
 
 case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
+if (!glisten) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("missing listen element for spice graphics"));
+return -1;
+}
+
 switch (glisten->type) {
 case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
 case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/9] tools: Fix comparison in virLoginShellGetShellArgv

[John Ferlan]

Commit id '740e4d70' altered the logic to fetch the sysconf values and
added a new virConfGetValueStringList which returns -1 on failure, 0 if
missing, and 1 if the value was present.

However, the caller only checked !shargv which caught Coverity's attention
since the following VIR_ALLOC_N(*shargv, 2) would be a NULL ptr deref

Signed-off-by: John Ferlan 
---
 tools/virt-login-shell.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index 632d493..a2b32ac 100644
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -99,10 +99,12 @@ static int virLoginShellGetShellArgv(virConfPtr conf,
  char ***shargv,
  size_t *shargvlen)
 {
-if (virConfGetValueStringList(conf, "shell", true, shargv) < 0)
+int rv;
+
+if ((rv = virConfGetValueStringList(conf, "shell", true, shargv)) < 0)
 return -1;
 
-if (!shargv) {
+if (rv == 0) {
 if (VIR_ALLOC_N(*shargv, 2) < 0)
 return -1;
 if (VIR_STRDUP((*shargv)[0], "/bin/sh") < 0) {
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 5/9] vsh: Properly initialize res

[John Ferlan]

The 'res' variable was only being initialized to NULL in the
if (!state) path; however, that path never used res and evenutally
res is assigned one of two results based on a pair of if then else if
conditions. If for some reason neither of those paths was taken and
the (!state) path wasn't take, then 'res' would be indeterminate.

Found by Coverity, probably a false positive based on code paths, but
better safe than sorry for the future.

Signed-off-by: John Ferlan 
---
 tools/vsh.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 68f7785..9ac4f21 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2636,7 +2636,8 @@ vshReadlineParse(const char *text, int state)
 vshCommandToken tk;
 static const vshCmdDef *cmd;
 const vshCmdOptDef *opt;
-char *tkdata, *optstr, *const_tkdata, *res;
+char *tkdata, *optstr, *const_tkdata;
+char *res = NULL;
 static char *ctext, *sanitized_text;
 static uint64_t const_opts_need_arg, const_opts_required, const_opts_seen;
 uint64_t opts_need_arg, opts_required, opts_seen;
@@ -2656,7 +2657,6 @@ vshReadlineParse(const char *text, int state)
 tkdata = NULL;
 sanitized_text = NULL;
 optstr = NULL;
-res = NULL;
 
 /* Sanitize/de-quote the autocomplete text */
 tk = sanitizer.getNextArg(NULL, , _text, false);
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/9] tests: Need to check return of virGetLastError

[John Ferlan]

Cannot assume virGetLastError returns non-NULL value - modify the code to
fetch err and check if err && err->code

Found by Coverity

Signed-off-by: John Ferlan 
---
 tests/qemuhelptest.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 8aac997..7c8b841 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -60,7 +60,9 @@ static int testHelpStrParsing(const void *data)
 
 if (virQEMUCapsParseHelpStr("QEMU", help, flags,
 , _kvm, _version, false, NULL) 
== -1) {
-if (info->error && virGetLastError()->code == info->error)
+virErrorPtr err = virGetLastError();
+
+if (info->error && err && err->code == info->error)
 ret = 0;
 goto cleanup;
 }
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/9] util: Fix incorrect VIR_FREE in virConfGetValueStringList

[John Ferlan]

Since we VIR_ALLOC_N to *values, the VIR_FREE should be done likewise

Signed-off-by: John Ferlan 
---
 src/util/virconf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virconf.c b/src/util/virconf.c
index a41f896..ee54072 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -1016,7 +1016,7 @@ int virConfGetValueStringList(virConfPtr conf,
 return -1;
 if (cval->str &&
 VIR_STRDUP((*values)[0], cval->str) < 0) {
-VIR_FREE(values);
+VIR_FREE(*values);
 return -1;
 }
 break;
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/9] Various patches as a result of Coverity

[John Ferlan]

Been piling up a few Coverity changes, the later patches aren't anything
critical, but the first few are newer and it seems more likely to be hit
in common paths.  In particular, the first 2 are from recent virconf changes.

John Ferlan (9):
  util: Fix incorrect VIR_FREE in virConfGetValueStringList
  tools: Fix comparison in virLoginShellGetShellArgv
  tests: Need to check return of virGetLastError
  conf: Need to check for glisten before accessing
  vsh: Properly initialize res
  esx: Replace strtok_r usage with virStringSplitCount
  conf: Add ignore_value to virDomainDeviceInfoIterate calls for Clear
helpers
  openvz: Remove need for strtok_r in openvzGenerateContainerVethName
  openvz: Remove strtok_r calls from openvzReadNetworkConf

 src/conf/domain_conf.c | 12 ++---
 src/esx/esx_vi.c   | 61 --
 src/openvz/openvz_conf.c   | 33 +++--
 src/openvz/openvz_driver.c | 16 
 src/util/virconf.c |  2 +-
 tests/qemuhelptest.c   |  4 ++-
 tools/virt-login-shell.c   |  6 +++--
 tools/vsh.c|  4 +--
 8 files changed, 76 insertions(+), 62 deletions(-)

-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] qemuhotplugtest: Add tests for ccw devices

[Tomasz Flendrich]

Thank you for your reply!

> I know you weren't aiming for that, but this got me thinking, are we
> checking that unplug works with not fully specified XML?  I mean that
> when unplugging, you don't need to specify everything.  Whatever
> uniquely identifies the device should be enough.  So for example just
> address or target, definitely no need for the readonly and shareable
> flags.  But as said, that's not meant to be part of this patch.
I have thought about that before and I agree that a few tests for this
would be useful. I decided not to add them yet, because of the issue
I explained in another patch:
> So far, there is only one test for persistent attachment. I will
> add more testcases as soon as I hear some feedback on these changes.
> qemuhotplugtest.c should first be modified anyway, so that the three
> xmls's filenames (basis domain xml, device xml, expected xml) are stated
> explicitly instead of being calculated. This will allow for more
> flexibility in testing and less xml files duplicates.
link to that patch:
https://www.redhat.com/archives/libvir-list/2016-July/msg00595.html

Adding tests will be a lot easier and without xml files duplication.
I can implement it now, but should I do it on top of the patch
“Test persistent device attachment”, parallelly to it, or wait for the feedback?


>> […]
> 
> vde2 is an address for a partition.  This works?  I don't think it
> should.
> 
> [...]
> 
>> […]
> 
> This ...
> 
>> […]
> 
> ... and this?  This will never work, so that's another bug right here.
Yay, I am very happy that my tests proved to be useful so fast :)

What exactly is the problem with “vde2”? And is it working properly
if we change “vde2” to “vdea”?

The second problem is that the device aliases are the same for
two different devices. Am I correct?


>> +  
> 
> By the way, attaching a device without explicit address and without
> target dev= specified (only bus='virtio')  Should work and that would
> actually test address allocation.  By allocating disk with target
> dev='XX' the address gets calculated from that device, so no allocation
> is being tested.
I consider reserving/validating addresses as a part of address
allocation, because they are connected to each other: if some
address is reserved, it can’t be automatically allocated for another
device, and vice versa. So I want to test both of them.  But you are
totally right and thank you :)

In this patch I added a test for both of these mixed together:
please, take a look at the last three DO_TEST_* in my patch.
The first two actions (an attachment and a detachment) are
with explicit addresses, and the last action (an attachment)
is with an implicit address.
There are a lot of possibilities to test, for example the first attachment
could also be done with an implicit address. I will write
tests for more cases with pleasure.

Tomasz

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Adding ips and routes to type="ethernet" devices

[Ruben Kerkhof]

Hi all,

I've been playing with the recently added support for adding ips and
routes to a type"ethernet" kind of device.
My goal is to get rid of bridging and use host routes (/32) combined
with Bird for a pure layer3 setup.
Something similar to what https://www.projectcalico.org/ is doing

Instead of p2p addresses with peers they use device routes (ip route
add 192.168.0.1/32 dev vnet0) combined with static arp entries.

I am a bit stuck though. I've created the vm with a tap device:

 









After starting, the link stays down and no ips or routes are being created:

root@test1: ~# ip link show dev vnet0
15: vnet0:  mtu 1500 qdisc noop state DOWN mode
DEFAULT qlen 1000
link/ether fe:1a:4a:1b:d9:cc brd ff:ff:ff:ff:ff:ff
root@test1: ~# ip route show
default via 10.10.5.1 dev br1010
10.10.5.0/24 dev br1010  proto kernel  scope link  src 10.10.5.2

I feel like I'm missing a step. Who is responsible for setting the
link up, it this libvirtd, qemu, or do I need to write a qemu-ifup
script?

Any help is greatly appreciated.

Kind regards,

Ruben Kerkhof

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv5 3/8] Add functions for adding USB hubs to addrs

[Ján Tomko]

Walk through all the usb hubs in the domain definition
that have a USB address specified, create the
corresponding structures in the virDomainUSBAddressSet
and mark the port it occupies as used.
---
 src/conf/domain_addr.c | 115 +
 1 file changed, 115 insertions(+)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index ea37a42..ad20fef 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1432,6 +1432,109 @@ 
virDomainUSBAddressSetAddController(virDomainUSBAddressSetPtr addrs,
 }
 
 
+static ssize_t
+virDomainUSBAddressGetLastIdx(virDomainDeviceInfoPtr info)
+{
+ssize_t i;
+for (i = VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH - 1; i > 0; i--) {
+if (info->addr.usb.port[i] != 0)
+break;
+}
+return i;
+}
+
+
+/* Find the USBAddressHub structure representing the hub/controller
+ * that corresponds to the bus/port path specified by info.
+ * Returns the index of the requested port in targetIdx.
+ */
+static virDomainUSBAddressHubPtr
+virDomainUSBAddressFindPort(virDomainUSBAddressSetPtr addrs,
+virDomainDeviceInfoPtr info,
+int *targetIdx,
+const char *portStr)
+{
+virDomainUSBAddressHubPtr hub = NULL;
+ssize_t i, lastIdx;
+
+if (info->addr.usb.bus >= addrs->nbuses ||
+!addrs->buses[info->addr.usb.bus]) {
+virReportError(VIR_ERR_XML_ERROR, _("Missing USB bus %u"),
+   info->addr.usb.bus);
+return NULL;
+}
+hub = addrs->buses[info->addr.usb.bus];
+
+lastIdx = virDomainUSBAddressGetLastIdx(info);
+
+for (i = 0; i < lastIdx; i++) {
+/* ports are numbered from 1 */
+int portIdx = info->addr.usb.port[i] - 1;
+
+if (hub->nports <= portIdx) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("port %u out of range in USB address bus: %u 
port: %s"),
+   info->addr.usb.port[i],
+   info->addr.usb.bus,
+   portStr);
+return NULL;
+}
+hub = hub->ports[portIdx];
+}
+
+*targetIdx = info->addr.usb.port[lastIdx] - 1;
+return hub;
+}
+
+
+#define VIR_DOMAIN_USB_HUB_PORTS 8
+
+static int
+virDomainUSBAddressSetAddHub(virDomainUSBAddressSetPtr addrs,
+ virDomainHubDefPtr hub)
+{
+virDomainUSBAddressHubPtr targetHub = NULL, newHub = NULL;
+int ret = -1;
+int targetPort;
+char *portStr = NULL;
+
+if (hub->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Wrong address type for USB hub"));
+goto cleanup;
+}
+
+if (!(portStr = virDomainUSBAddressPortFormat(hub->info.addr.usb.port)))
+goto cleanup;
+
+VIR_DEBUG("Adding a USB hub with 8 ports on bus=%u port=%s",
+  hub->info.addr.usb.bus, portStr);
+
+if (!(newHub = virDomainUSBAddressHubNew(VIR_DOMAIN_USB_HUB_PORTS)))
+goto cleanup;
+
+if (!(targetHub = virDomainUSBAddressFindPort(addrs, &(hub->info), 
,
+  portStr)))
+goto cleanup;
+
+if (targetHub->ports[targetPort]) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Duplicate USB hub on bus %u port %s"),
+   hub->info.addr.usb.bus, portStr);
+goto cleanup;
+}
+ignore_value(virBitmapSetBit(targetHub->portmap, targetPort));
+targetHub->ports[targetPort] = newHub;
+newHub = NULL;
+
+ret = 0;
+ cleanup:
+virDomainUSBAddressHubFree(newHub);
+VIR_FREE(portStr);
+return ret;
+}
+
+
 int
 virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs,
  virDomainDefPtr def)
@@ -1445,5 +1548,17 @@ 
virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs,
 return -1;
 }
 }
+
+for (i = 0; i < def->nhubs; i++) {
+virDomainHubDefPtr hub = def->hubs[i];
+if (hub->type == VIR_DOMAIN_HUB_TYPE_USB &&
+hub->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB &&
+virDomainUSBAddressPortIsValid(hub->info.addr.usb.port)) {
+/* USB hubs that do not yet have an USB address have to be
+ * dealt with later */
+if (virDomainUSBAddressSetAddHub(addrs, hub) < 0)
+return -1;
+}
+}
 return 0;
 }
-- 
2.7.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv5 2/8] Add functions for adding USB controllers to addrs

[Ján Tomko]

Walk through all the usb controllers in the domain definition
and create the corresponding structures in the virDomainUSBAddressSet.
---
 src/conf/domain_addr.c   | 121 +++
 src/conf/domain_addr.h   |   4 ++
 src/libvirt_private.syms |   1 +
 3 files changed, 126 insertions(+)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 658aad5..ea37a42 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1326,3 +1326,124 @@ virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr 
addrs)
 VIR_FREE(addrs->buses);
 VIR_FREE(addrs);
 }
+
+
+static size_t
+virDomainUSBAddressControllerModelToPorts(virDomainControllerDefPtr cont)
+{
+int model = cont->model;
+
+if (model == -1)
+model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
+
+switch ((virDomainControllerModelUSB) model) {
+case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI:
+case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI:
+case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI:
+return 2;
+
+case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI:
+case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1:
+return 6;
+
+case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1:
+case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2:
+case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3:
+/* These have two ports each and are used to provide USB1.1
+ * ports while ICH9_EHCI1 provides 6 USB2.0 ports.
+ * Ignore these since we will add the EHCI1 too. */
+return 0;
+
+case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI:
+return 3;
+
+case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
+if (cont->opts.usbopts.ports != -1)
+return cont->opts.usbopts.ports;
+return 4;
+
+case VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE:
+case VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST:
+break;
+}
+return 0;
+}
+
+
+static virDomainUSBAddressHubPtr
+virDomainUSBAddressHubNew(size_t nports)
+{
+virDomainUSBAddressHubPtr hub = NULL, ret = NULL;
+
+if (VIR_ALLOC(hub) < 0)
+goto cleanup;
+
+if (!(hub->portmap = virBitmapNew(nports)))
+goto cleanup;
+
+if (VIR_ALLOC_N(hub->ports, nports) < 0)
+goto cleanup;
+hub->nports = nports;
+
+ret = hub;
+hub = NULL;
+ cleanup:
+virDomainUSBAddressHubFree(hub);
+return ret;
+}
+
+
+static int
+virDomainUSBAddressSetAddController(virDomainUSBAddressSetPtr addrs,
+virDomainControllerDefPtr cont)
+{
+size_t nports = virDomainUSBAddressControllerModelToPorts(cont);
+virDomainUSBAddressHubPtr hub = NULL;
+int ret = -1;
+
+VIR_DEBUG("Adding a USB controller model=%s with %zu ports",
+  virDomainControllerModelUSBTypeToString(cont->model),
+  nports);
+
+/* Skip UHCI{1,2,3} companions; only add the EHCI1 */
+if (nports == 0)
+return 0;
+
+if (addrs->nbuses <= cont->idx) {
+if (VIR_EXPAND_N(addrs->buses, addrs->nbuses, cont->idx - 
addrs->nbuses + 1) < 0)
+goto cleanup;
+} else if (addrs->buses[cont->idx]) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Duplicate USB controllers with index %u"),
+   cont->idx);
+goto cleanup;
+}
+
+if (!(hub = virDomainUSBAddressHubNew(nports)))
+goto cleanup;
+
+addrs->buses[cont->idx] = hub;
+hub = NULL;
+
+ret = 0;
+ cleanup:
+virDomainUSBAddressHubFree(hub);
+return ret;
+}
+
+
+int
+virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs,
+ virDomainDefPtr def)
+{
+size_t i;
+
+for (i = 0; i < def->ncontrollers; i++) {
+virDomainControllerDefPtr cont = def->controllers[i];
+if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) {
+if (virDomainUSBAddressSetAddController(addrs, cont) < 0)
+return -1;
+}
+}
+return 0;
+}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 168d3ed..2bd4a0d 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -269,6 +269,10 @@ typedef struct _virDomainUSBAddressSet 
virDomainUSBAddressSet;
 typedef virDomainUSBAddressSet *virDomainUSBAddressSetPtr;
 
 virDomainUSBAddressSetPtr virDomainUSBAddressSetCreate(void);
+
+int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs,
+ virDomainDefPtr def)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs);
 
 #endif /* __DOMAIN_ADDR_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 089b66b..62d63f4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -110,6 +110,7 @@ virDomainPCIControllerModelToConnectType;
 virDomainUSBAddressPortFormat;
 virDomainUSBAddressPortFormatBuf;
 virDomainUSBAddressPortIsValid;

[libvirt] [PATCHv5 6/8] Assign addresses to USB devices

[Ján Tomko]

Automatically assign addresses to USB devices.

Just like reserving, this is only done for newly defined domains.

https://bugzilla.redhat.com/show_bug.cgi?id=1215968
---
 src/conf/domain_addr.c | 133 -
 src/conf/domain_addr.h |  14 +++
 src/libvirt_private.syms   |   3 +
 src/qemu/qemu_domain_address.c |  65 ++
 .../qemuxml2argvdata/qemuxml2argv-bios-nvram.args  |   2 +-
 tests/qemuxml2argvdata/qemuxml2argv-bios.args  |   2 +-
 .../qemuxml2argv-controller-order.args |   8 +-
 .../qemuxml2argv-disk-usb-device-removable.args|   3 +-
 .../qemuxml2argv-disk-usb-device.args  |   2 +-
 .../qemuxml2argv-graphics-spice-timeout.args   |   2 +-
 .../qemuxml2argv-graphics-spice-usb-redir.args |   2 +-
 ...muxml2argv-hostdev-usb-address-device-boot.args |   3 +-
 .../qemuxml2argv-hostdev-usb-address-device.args   |   2 +-
 .../qemuxml2argv-hostdev-usb-address.args  |   2 +-
 .../qemuxml2argv-hugepages-numa.args   |   6 +-
 .../qemuxml2argv-input-usbmouse.args   |   2 +-
 .../qemuxml2argv-input-usbtablet.args  |   2 +-
 .../qemuxml2argv-pseries-usb-kbd.args  |   2 +-
 .../qemuxml2argv-serial-spiceport.args |   2 +-
 .../qemuxml2argv-smartcard-controller.args |   2 +-
 .../qemuxml2argv-smartcard-host-certificates.args  |   2 +-
 .../qemuxml2argv-smartcard-host.args   |   2 +-
 ...emuxml2argv-smartcard-passthrough-spicevmc.args |   2 +-
 .../qemuxml2argv-smartcard-passthrough-tcp.args|   2 +-
 .../qemuxml2argv-sound-device.args |   2 +-
 .../qemuxml2argv-usb-ich9-autoassign.args  |  10 +-
 .../qemuxml2argv-usb-port-autoassign.args  |   8 +-
 .../qemuxml2argv-usb-port-missing.args |   4 +-
 .../qemuxml2argv-usb-redir-boot.args   |   2 +-
 tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args |   2 +-
 .../qemuxml2argv-usb-xhci-autoassign.args  |  10 +-
 31 files changed, 260 insertions(+), 45 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index bbac399..3b0c205 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1489,7 +1489,7 @@ virDomainUSBAddressFindPort(virDomainUSBAddressSetPtr 
addrs,
 
 #define VIR_DOMAIN_USB_HUB_PORTS 8
 
-static int
+int
 virDomainUSBAddressSetAddHub(virDomainUSBAddressSetPtr addrs,
  virDomainHubDefPtr hub)
 {
@@ -1564,6 +1564,119 @@ 
virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs,
 }
 
 
+static int
+virDomainUSBAddressFindFreePort(virDomainUSBAddressHubPtr hub,
+unsigned int *portpath,
+unsigned int level)
+{
+unsigned int port;
+ssize_t portIdx;
+size_t i;
+
+/* Look for free ports on the current hub */
+if ((portIdx = virBitmapNextClearBit(hub->portmap, -1)) >= 0) {
+port = portIdx + 1;
+VIR_DEBUG("Found a free port %u at level %u", port, level);
+portpath[level] = port;
+return 0;
+}
+
+VIR_DEBUG("No ports found on hub %p, trying the hubs on it", hub);
+
+if (level >= VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH - 1)
+return -1;
+
+/* Recursively search through the ports that contain another hub */
+for (i = 0; i < hub->nports; i++) {
+if (!hub->ports[i])
+continue;
+
+port = i + 1;
+VIR_DEBUG("Looking at USB hub at level: %u port: %u", level, port);
+if (virDomainUSBAddressFindFreePort(hub->ports[i], portpath,
+level + 1) < 0)
+continue;
+
+portpath[level] = port;
+return 0;
+}
+return -1;
+}
+
+
+/* Try to find a free port on bus @bus.
+ *
+ * Returns  0 on success
+ * -1 on fatal error (OOM)
+ * -2 if there is no bus at @bus or no free port on this bus
+ */
+static int
+virDomainUSBAddressAssignFromBus(virDomainUSBAddressSetPtr addrs,
+ virDomainDeviceInfoPtr info,
+ size_t bus)
+{
+unsigned int portpath[VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH] = { 0 };
+virDomainUSBAddressHubPtr hub = addrs->buses[bus];
+char *portStr = NULL;
+int ret = -1;
+
+if (!hub)
+return -2;
+
+if (virDomainUSBAddressFindFreePort(hub, portpath, 0) < 0)
+return -2;
+
+/* we found a free port */
+if (!(portStr = virDomainUSBAddressPortFormat(portpath)))
+goto cleanup;
+
+info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB;
+info->addr.usb.bus = bus;
+memcpy(info->addr.usb.port, portpath, sizeof(portpath));
+VIR_DEBUG("Assigning USB addr bus=%u port=%s",
+  info->addr.usb.bus, portStr);
+if (virDomainUSBAddressReserve(info, addrs) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+

[libvirt] [PATCHv5 7/8] Assign addresses on USB device hotplug

[Ján Tomko]

USB disks, redirected devices, host devices and serial devices
are supported.
---
 src/conf/domain_addr.c | 30 ++
 src/conf/domain_addr.h |  5 
 src/libvirt_private.syms   |  1 +
 src/qemu/qemu_domain_address.c |  5 
 src/qemu/qemu_hotplug.c| 27 +++
 .../qemuhotplug-base-live+disk-usb.xml |  1 +
 6 files changed, 69 insertions(+)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 3b0c205..365ee40 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1735,3 +1735,33 @@ virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr 
addrs,
 
 return 0;
 }
+
+
+int
+virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
+   virDomainDeviceInfoPtr info)
+{
+virDomainUSBAddressHubPtr targetHub = NULL;
+char *portStr = NULL;
+int targetPort;
+int ret = -1;
+
+if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB ||
+!virDomainUSBAddressPortIsValid(info->addr.usb.port))
+return 0;
+
+portStr = virDomainUSBAddressPortFormat(info->addr.usb.port);
+VIR_DEBUG("Releasing USB addr bus=%u port=%s", info->addr.usb.bus, 
portStr);
+
+if (!(targetHub = virDomainUSBAddressFindPort(addrs, info, ,
+  portStr)))
+goto cleanup;
+
+ignore_value(virBitmapClearBit(targetHub->portmap, targetPort));
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(portStr);
+return ret;
+}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 633aa16..cc36aed 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -293,4 +293,9 @@ int
 virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs,
   virDomainDeviceInfoPtr info)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
+int
+virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
+   virDomainDeviceInfoPtr info)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 #endif /* __DOMAIN_ADDR_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 35b67d5..bae0470 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -112,6 +112,7 @@ virDomainUSBAddressEnsure;
 virDomainUSBAddressPortFormat;
 virDomainUSBAddressPortFormatBuf;
 virDomainUSBAddressPortIsValid;
+virDomainUSBAddressRelease;
 virDomainUSBAddressReserve;
 virDomainUSBAddressSetAddControllers;
 virDomainUSBAddressSetAddHub;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 21c2ecf..7499026 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1772,4 +1772,9 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
 virDomainVirtioSerialAddrRelease(priv->vioserialaddrs, info) < 0)
 VIR_WARN("Unable to release virtio-serial address on %s",
  NULLSTR(devstr));
+if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB &&
+priv->usbaddrs &&
+virDomainUSBAddressRelease(priv->usbaddrs, info) < 0)
+VIR_WARN("Unable to release USB address on %s",
+ NULLSTR(devstr));
 }
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 04e11b4..2afc7f3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -640,6 +640,13 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr 
driver,
 char *devstr = NULL;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 const char *src = virDomainDiskGetSource(disk);
+bool releaseaddr = false;
+
+if (priv->usbaddrs) {
+if (virDomainUSBAddressEnsure(priv->usbaddrs, >info) < 0)
+goto cleanup;
+releaseaddr = true;
+}
 
 if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
 goto cleanup;
@@ -685,6 +692,8 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr 
driver,
 virDomainDiskInsertPreAlloced(vm->def, disk);
 
  cleanup:
+if (ret < 0 && releaseaddr)
+virDomainUSBAddressRelease(priv->usbaddrs, >info);
 VIR_FREE(devstr);
 VIR_FREE(drivestr);
 virObjectUnref(cfg);
@@ -1486,6 +1495,12 @@ 
qemuDomainAttachChrDeviceAssignAddr(qemuDomainObjPrivatePtr priv,
 return -1;
 return 1;
 
+} else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+   chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) {
+if (virDomainUSBAddressEnsure(priv->usbaddrs, >info) < 0)
+return -1;
+return 1;
+
 } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) {
 if (virDomainVirtioSerialAddrAutoAssign(NULL, priv->vioserialaddrs,
@@ -1804,11 +1819,18 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
 {
 qemuDomainObjPrivatePtr priv = 

[libvirt] [PATCHv5 5/8] Add tests for USB address assignment

[Ján Tomko]

Introduce tests with the ich9, xhci and the default (piix3) usb
controller to demonstrate the effect of the next patch.
---
 .../qemuxml2argv-usb-ich9-autoassign.args  | 32 ++
 .../qemuxml2argv-usb-ich9-autoassign.xml   | 39 ++
 .../qemuxml2argv-usb-port-autoassign.args  | 28 
 .../qemuxml2argv-usb-port-autoassign.xml   | 27 +++
 .../qemuxml2argv-usb-xhci-autoassign.args  | 27 +++
 .../qemuxml2argv-usb-xhci-autoassign.xml   | 25 ++
 tests/qemuxml2argvtest.c   | 11 ++
 7 files changed, 189 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-autoassign.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-autoassign.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-xhci-autoassign.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-xhci-autoassign.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-autoassign.args 
b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-autoassign.args
new file mode 100644
index 000..929fa1e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-autoassign.args
@@ -0,0 +1,32 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefconfig \
+-nodefaults \
+-chardev 
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 \
+-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,\
+addr=0x4 \
+-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 \
+-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 \
+-device usb-hub,id=hub0 \
+-device usb-hub,id=hub1 \
+-device usb-mouse,id=input0 \
+-device usb-mouse,id=input1 \
+-device usb-mouse,id=input2 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-autoassign.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-autoassign.xml
new file mode 100644
index 000..6425c50
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-autoassign.xml
@@ -0,0 +1,39 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+/usr/bin/qemu
+
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+
+
+
+
+
+
+
+
+
+  
+
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args 
b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args
new file mode 100644
index 000..3515fad
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args
@@ -0,0 +1,28 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefconfig \
+-nodefaults \
+-chardev 
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-usb \
+-device usb-hub,id=hub0,bus=usb.0,port=1 \
+-device usb-hub,id=hub1 \
+-device usb-mouse,id=input0 \
+-device usb-mouse,id=input1 \
+-device usb-mouse,id=input2 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.xml
new file mode 100644
index 000..a2fe34e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.xml
@@ -0,0 +1,27 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+/usr/bin/qemu
+
+
+
+  
+
+
+
+
+
+
+
+
+
+  
+
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-xhci-autoassign.args 
b/tests/qemuxml2argvdata/qemuxml2argv-usb-xhci-autoassign.args
new file mode 100644
index 000..3034d21
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-xhci-autoassign.args
@@ -0,0 +1,27 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefconfig \

[libvirt] [PATCHv5 8/8] Auto-add one hub if there are too many USB devices

[Ján Tomko]

When parsing a command line with USB devices that have
no address specified, QEMU automatically adds a USB hub
if the device would fill up all the available USB ports.

To help most of the users, add one hub if there are more
USB devices than available ports. For wilder configurations,
expect the user to provide us with more hubs and/or controllers.
---
 src/conf/domain_addr.c | 20 +
 src/conf/domain_addr.h |  2 +
 src/libvirt_private.syms   |  1 +
 src/qemu/qemu_domain_address.c | 48 ++
 .../qemuxml2argv-usb-hub-autoadd.args  | 28 +
 .../qemuxml2argv-usb-hub-autoadd.xml   | 23 +++
 tests/qemuxml2argvtest.c   |  3 ++
 7 files changed, 125 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 365ee40..c3469ee 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1604,6 +1604,26 @@ 
virDomainUSBAddressFindFreePort(virDomainUSBAddressHubPtr hub,
 }
 
 
+size_t
+virDomainUSBAddressCountAllPorts(virDomainDefPtr def)
+{
+size_t i, ret = 0;
+
+for (i = 0; i < def->ncontrollers; i++) {
+virDomainControllerDefPtr cont = def->controllers[i];
+if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB)
+ret += virDomainUSBAddressControllerModelToPorts(cont);
+}
+
+for (i = 0; i < def->nhubs; i++) {
+virDomainHubDefPtr hub = def->hubs[i];
+if (hub->type == VIR_DOMAIN_HUB_TYPE_USB)
+ret += VIR_DOMAIN_USB_HUB_PORTS;
+}
+return ret;
+}
+
+
 /* Try to find a free port on bus @bus.
  *
  * Returns  0 on success
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index cc36aed..ce94981 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -277,6 +277,8 @@ int
 virDomainUSBAddressSetAddHub(virDomainUSBAddressSetPtr addrs,
  virDomainHubDefPtr hub)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+size_t
+virDomainUSBAddressCountAllPorts(virDomainDefPtr def);
 void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs);
 
 int
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bae0470..466064f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -108,6 +108,7 @@ virDomainPCIAddressSlotInUse;
 virDomainPCIAddressValidate;
 virDomainPCIControllerModelToConnectType;
 virDomainUSBAddressAssign;
+virDomainUSBAddressCountAllPorts;
 virDomainUSBAddressEnsure;
 virDomainUSBAddressPortFormat;
 virDomainUSBAddressPortFormatBuf;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 7499026..787b357 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1680,6 +1680,51 @@ qemuDomainAssignUSBPorts(virDomainUSBAddressSetPtr addrs,
 
 
 static int
+qemuDomainAssignUSBPortsCounter(virDomainDeviceInfoPtr info ATTRIBUTE_UNUSED,
+void *opaque)
+{
+struct qemuAssignUSBIteratorInfo *data = opaque;
+
+data->count++;
+return 0;
+}
+
+
+static int
+qemuDomainUSBAddressAddHubs(virDomainDefPtr def)
+{
+struct qemuAssignUSBIteratorInfo data = { .count = 0 };
+virDomainHubDefPtr hub = NULL;
+size_t available_ports;
+int ret = -1;
+
+available_ports = virDomainUSBAddressCountAllPorts(def);
+ignore_value(virDomainUSBDeviceDefForeach(def,
+  qemuDomainAssignUSBPortsCounter,
+  ,
+  false));
+VIR_DEBUG("Found %zu USB devices and %zu provided USB ports",
+  data.count, available_ports);
+
+/* Add one hub if there are more devices than ports
+ * otherwise it's up to the user to specify more hubs/controllers */
+if (data.count > available_ports) {
+if (VIR_ALLOC(hub) < 0)
+return -1;
+hub->type = VIR_DOMAIN_HUB_TYPE_USB;
+
+if (VIR_APPEND_ELEMENT(def->hubs, def->nhubs, hub) < 0)
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(hub);
+return ret;
+}
+
+
+static int
 qemuDomainAssignUSBAddresses(virDomainDefPtr def,
  virDomainObjPtr obj)
 {
@@ -1690,6 +1735,9 @@ qemuDomainAssignUSBAddresses(virDomainDefPtr def,
 if (!(addrs = virDomainUSBAddressSetCreate()))
 goto cleanup;
 
+if (qemuDomainUSBAddressAddHubs(def) < 0)
+goto cleanup;
+
 if (virDomainUSBAddressSetAddControllers(addrs, def) < 0)
 goto cleanup;
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args 
b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args
new file mode 100644
index 000..7467893
--- /dev/null
+++ 

[libvirt] [PATCHv5 4/8] Reserve existing USB addresses

[Ján Tomko]

Check if they fit on the USB controllers the domain has,
and error out if two devices try to use the same address.
---
 src/conf/domain_addr.c | 42 ++
 src/conf/domain_addr.h |  4 +++
 src/libvirt_private.syms   |  1 +
 src/qemu/qemu_domain.c |  1 +
 src/qemu/qemu_domain.h |  1 +
 src/qemu/qemu_domain_address.c | 38 +++-
 .../qemuxml2argv-usb-hub-conflict.xml  | 22 
 tests/qemuxml2argvtest.c   |  3 ++
 8 files changed, 111 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.xml

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index ad20fef..bbac399 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1562,3 +1562,45 @@ 
virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs,
 }
 return 0;
 }
+
+
+int
+virDomainUSBAddressReserve(virDomainDeviceInfoPtr info,
+   void *data)
+{
+virDomainUSBAddressSetPtr addrs = data;
+virDomainUSBAddressHubPtr targetHub = NULL;
+char *portStr = NULL;
+int ret = -1;
+int targetPort;
+
+if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+return 0;
+
+if (!virDomainUSBAddressPortIsValid(info->addr.usb.port))
+return 0;
+
+portStr = virDomainUSBAddressPortFormat(info->addr.usb.port);
+if (!portStr)
+goto cleanup;
+VIR_DEBUG("Reserving USB address bus=%u port=%s", info->addr.usb.bus, 
portStr);
+
+if (!(targetHub = virDomainUSBAddressFindPort(addrs, info, ,
+  portStr)))
+goto cleanup;
+
+if (virBitmapIsBitSet(targetHub->portmap, targetPort)) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Duplicate USB address bus %u port %s"),
+   info->addr.usb.bus, portStr);
+goto cleanup;
+}
+
+ignore_value(virBitmapSetBit(targetHub->portmap, targetPort));
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(portStr);
+return ret;
+}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 2bd4a0d..a24d216 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -275,4 +275,8 @@ int 
virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs,
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs);
 
+int
+virDomainUSBAddressReserve(virDomainDeviceInfoPtr info,
+   void *data)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 #endif /* __DOMAIN_ADDR_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 62d63f4..b124342 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -110,6 +110,7 @@ virDomainPCIControllerModelToConnectType;
 virDomainUSBAddressPortFormat;
 virDomainUSBAddressPortFormatBuf;
 virDomainUSBAddressPortIsValid;
+virDomainUSBAddressReserve;
 virDomainUSBAddressSetAddControllers;
 virDomainUSBAddressSetCreate;
 virDomainUSBAddressSetFree;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 319293a..c87db01 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1301,6 +1301,7 @@ qemuDomainObjPrivateFree(void *data)
 virDomainPCIAddressSetFree(priv->pciaddrs);
 virDomainCCWAddressSetFree(priv->ccwaddrs);
 virDomainVirtioSerialAddrSetFree(priv->vioserialaddrs);
+virDomainUSBAddressSetFree(priv->usbaddrs);
 virDomainChrSourceDefFree(priv->monConfig);
 qemuDomainObjFreeJob(priv);
 VIR_FREE(priv->lockState);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 114db98..d10ebcc 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -187,6 +187,7 @@ struct _qemuDomainObjPrivate {
 virDomainPCIAddressSetPtr pciaddrs;
 virDomainCCWAddressSetPtr ccwaddrs;
 virDomainVirtioSerialAddrSetPtr vioserialaddrs;
+virDomainUSBAddressSetPtr usbaddrs;
 
 virQEMUCapsPtr qemuCaps;
 char *lockState;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index ee44d45..f66b2f0 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1622,11 +1622,44 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
 }
 
 
+static int
+qemuDomainAssignUSBAddresses(virDomainDefPtr def,
+ virDomainObjPtr obj)
+{
+int ret = -1;
+virDomainUSBAddressSetPtr addrs = NULL;
+qemuDomainObjPrivatePtr priv = NULL;
+
+if (!(addrs = virDomainUSBAddressSetCreate()))
+goto cleanup;
+
+if (virDomainUSBAddressSetAddControllers(addrs, def) < 0)
+goto cleanup;
+
+if (virDomainUSBDeviceDefForeach(def, virDomainUSBAddressReserve, addrs,
+ true) < 0)
+goto cleanup;
+
+

[libvirt] [PATCHv5 0/8] Assign addresses to USB devices

[Ján Tomko]

Patches 1-2 from v4 are pushed already.

Patch 5/8 is new, adding new tests.
Patches 6/8 and 8/8 have not been ACKed in the previous version.


Ján Tomko (8):
  Introduce virDomainUSBAddressSet
  Add functions for adding USB controllers to addrs
  Add functions for adding USB hubs to addrs
  Reserve existing USB addresses
  Add tests for USB address assignment
  Assign addresses to USB devices
  Assign addresses on USB device hotplug
  Auto-add one hub if there are too many USB devices

 src/conf/domain_addr.c | 501 +
 src/conf/domain_addr.h |  51 +++
 src/libvirt_private.syms   |   9 +
 src/qemu/qemu_domain.c |   1 +
 src/qemu/qemu_domain.h |   1 +
 src/qemu/qemu_domain_address.c | 156 ++-
 src/qemu/qemu_hotplug.c|  27 ++
 .../qemuhotplug-base-live+disk-usb.xml |   1 +
 .../qemuxml2argvdata/qemuxml2argv-bios-nvram.args  |   2 +-
 tests/qemuxml2argvdata/qemuxml2argv-bios.args  |   2 +-
 .../qemuxml2argv-controller-order.args |   8 +-
 .../qemuxml2argv-disk-usb-device-removable.args|   3 +-
 .../qemuxml2argv-disk-usb-device.args  |   2 +-
 .../qemuxml2argv-graphics-spice-timeout.args   |   2 +-
 .../qemuxml2argv-graphics-spice-usb-redir.args |   2 +-
 ...muxml2argv-hostdev-usb-address-device-boot.args |   3 +-
 .../qemuxml2argv-hostdev-usb-address-device.args   |   2 +-
 .../qemuxml2argv-hostdev-usb-address.args  |   2 +-
 .../qemuxml2argv-hugepages-numa.args   |   6 +-
 .../qemuxml2argv-input-usbmouse.args   |   2 +-
 .../qemuxml2argv-input-usbtablet.args  |   2 +-
 .../qemuxml2argv-pseries-usb-kbd.args  |   2 +-
 .../qemuxml2argv-serial-spiceport.args |   2 +-
 .../qemuxml2argv-smartcard-controller.args |   2 +-
 .../qemuxml2argv-smartcard-host-certificates.args  |   2 +-
 .../qemuxml2argv-smartcard-host.args   |   2 +-
 ...emuxml2argv-smartcard-passthrough-spicevmc.args |   2 +-
 .../qemuxml2argv-smartcard-passthrough-tcp.args|   2 +-
 .../qemuxml2argv-sound-device.args |   2 +-
 .../qemuxml2argv-usb-hub-autoadd.args  |  28 ++
 .../qemuxml2argv-usb-hub-autoadd.xml   |  23 +
 .../qemuxml2argv-usb-hub-conflict.xml  |  22 +
 .../qemuxml2argv-usb-ich9-autoassign.args  |  32 ++
 .../qemuxml2argv-usb-ich9-autoassign.xml   |  39 ++
 .../qemuxml2argv-usb-port-autoassign.args  |  28 ++
 .../qemuxml2argv-usb-port-autoassign.xml   |  27 ++
 .../qemuxml2argv-usb-port-missing.args |   4 +-
 .../qemuxml2argv-usb-redir-boot.args   |   2 +-
 tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args |   2 +-
 .../qemuxml2argv-usb-xhci-autoassign.args  |  27 ++
 .../qemuxml2argv-usb-xhci-autoassign.xml   |  25 +
 tests/qemuxml2argvtest.c   |  17 +
 42 files changed, 1046 insertions(+), 31 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-autoassign.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-autoassign.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-xhci-autoassign.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-xhci-autoassign.xml

-- 
2.7.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv5 1/8] Introduce virDomainUSBAddressSet

[Ján Tomko]

A new type to track USB addresses.

Every  is represented by an
object of type virDomainUSBAddressHub located at buses[i].

Each of these hubs has up to 'nports' ports.
If a port is occupied, it has the corresponding bit set in
the 'ports' bitmap, e.g. port 1 would have the 0th bit set.
If there is a hub on this port, then hubs[i] will point
to this hub.
---
 src/conf/domain_addr.c   | 42 ++
 src/conf/domain_addr.h   | 22 ++
 src/libvirt_private.syms |  2 ++
 3 files changed, 66 insertions(+)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 741d045..658aad5 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1284,3 +1284,45 @@ virDomainUSBAddressPortFormat(unsigned int *port)
 return NULL;
 return virBufferContentAndReset();
 }
+
+
+virDomainUSBAddressSetPtr
+virDomainUSBAddressSetCreate(void)
+{
+virDomainUSBAddressSetPtr addrs;
+
+if (VIR_ALLOC(addrs) < 0)
+return NULL;
+
+return addrs;
+}
+
+
+static void
+virDomainUSBAddressHubFree(virDomainUSBAddressHubPtr hub)
+{
+size_t i;
+
+if (!hub)
+return;
+
+for (i = 0; i < hub->nports; i++)
+virDomainUSBAddressHubFree(hub->ports[i]);
+virBitmapFree(hub->portmap);
+VIR_FREE(hub);
+}
+
+
+void
+virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs)
+{
+size_t i;
+
+if (!addrs)
+return;
+
+for (i = 0; i < addrs->nbuses; i++)
+virDomainUSBAddressHubFree(addrs->buses[i]);
+VIR_FREE(addrs->buses);
+VIR_FREE(addrs);
+}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index cfc74d5..168d3ed 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -249,4 +249,26 @@ char *
 virDomainUSBAddressPortFormat(unsigned int *port)
 ATTRIBUTE_NONNULL(1);
 
+typedef struct _virDomainUSBAddressHub virDomainUSBAddressHub;
+typedef virDomainUSBAddressHub *virDomainUSBAddressHubPtr;
+struct _virDomainUSBAddressHub {
+/* indexes are shifted by one:
+ * ports[0] represents port 1, because ports are numbered from 1 */
+virBitmapPtr portmap;
+size_t nports;
+virDomainUSBAddressHubPtr *ports;
+};
+
+struct _virDomainUSBAddressSet {
+/* every  is represented
+ * as a hub at buses[i] */
+virDomainUSBAddressHubPtr *buses;
+size_t nbuses;
+};
+typedef struct _virDomainUSBAddressSet virDomainUSBAddressSet;
+typedef virDomainUSBAddressSet *virDomainUSBAddressSetPtr;
+
+virDomainUSBAddressSetPtr virDomainUSBAddressSetCreate(void);
+void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs);
+
 #endif /* __DOMAIN_ADDR_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3580a72..089b66b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -110,6 +110,8 @@ virDomainPCIControllerModelToConnectType;
 virDomainUSBAddressPortFormat;
 virDomainUSBAddressPortFormatBuf;
 virDomainUSBAddressPortIsValid;
+virDomainUSBAddressSetCreate;
+virDomainUSBAddressSetFree;
 virDomainVirtioSerialAddrAssign;
 virDomainVirtioSerialAddrAutoAssign;
 virDomainVirtioSerialAddrIsComplete;
-- 
2.7.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv4 9/9] Auto-add one hub if there are too many USB devices

[Ján Tomko]

On Thu, Jul 14, 2016 at 01:00:09PM -0400, John Ferlan wrote:



On 07/01/2016 11:38 AM, Ján Tomko wrote:

When parsing a command line with USB devices that have
no address specified, QEMU automatically adds a USB hub
if the device would fill up all the available USB ports.

To help most of the users, add one hub if there are more
USB devices than available ports. For wilder configurations,
expect the user to provide us with more hubs and/or controllers.
---
 src/conf/domain_addr.c | 20 +
 src/conf/domain_addr.h |  2 +
 src/libvirt_private.syms   |  1 +
 src/qemu/qemu_domain_address.c | 48 ++
 .../qemuxml2argv-usb-hub-autoadd.args  | 28 +
 .../qemuxml2argv-usb-hub-autoadd.xml   | 23 +++
 tests/qemuxml2argvtest.c   |  3 ++
 7 files changed, 125 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml




+static int
+qemuDomainUSBAddressAddHubs(virDomainDefPtr def)
+{
+struct qemuAssignUSBIteratorInfo data = { .count = 0 };
+virDomainHubDefPtr hub = NULL;
+size_t available_ports;
+int ret = -1;
+
+available_ports = virDomainUSBAddressCountAvailablePorts(def);
+ignore_value(virDomainUSBDeviceDefForeach(def,
+  qemuDomainAssignUSBPortsCounter,
+  ,
+  false));
+VIR_DEBUG("Found %zu USB devices and %zu provided USB ports",
+  data.count, available_ports);
+
+/* Add one hub if there are more devices than ports
+ * otherwise it's up to the user to specify more hubs/controllers */


What if one hub isn't enough? Wouldn't we need to keep adding until we
have enough?   I'm sure a test could be created for it...



For that many devices, most of the users would probably be better off
with more USB controllers than chaining hubs.

The aim of this code is to cover most of the cases, not all of them.


diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml
new file mode 100644
index 000..43e0f1f
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml
@@ -0,0 +1,23 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+/usr/bin/qemu
+


So to try and answer something I was thinking about earlier... If I just
change index from 0 to 1, then I get an error:

libvirt: QEMU Driver error : unsupported configuration: Multiple legacy
USB controllers are not supported

If I then adjust the qemuxml2argvtest to add QEMU_CAPS_PIIX3_USB_UHCI,
QEMU_CAPS_PCI_MULTIFUNCTION, then I get the failure:

error : virDomainUSBAddressReserve:1724 : XML error: Duplicate USB
address bus 0 port 1



I could not reproduce this error. Maybe I fixed it along the way?

Jan


So is it a "valid" test to have one defined for index='1' and allow the
code to create index='0'?


John


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [gconfig v2 1/2] config: Add spice listen setter

[Christophe Fergeau]

Hey,

On Mon, Jul 18, 2016 at 04:36:39PM +0300, Visarion Alexandru wrote:
> Learn to set the address that spice is listening on.
> ---
>  .../libvirt-gconfig-domain-graphics-spice.c| 18 
> ++
>  .../libvirt-gconfig-domain-graphics-spice.h|  3 +++
>  libvirt-gconfig/libvirt-gconfig.sym|  1 +
>  3 files changed, 22 insertions(+)
> 
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c 
> b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c
> index 079ea27..a773084 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c
> @@ -165,3 +165,21 @@ void 
> gvir_config_domain_graphics_spice_set_gl(GVirConfigDomainGraphicsSpice *gra
>  gvir_config_object_replace_child_with_attribute_enum
>(GVIR_CONFIG_OBJECT(graphics), "gl", "enable", G_TYPE_BOOLEAN, gl);
>  }
> +
> +void 
> gvir_config_domain_graphics_spice_set_listen_address(GVirConfigDomainGraphicsSpice
>  *graphics,
> +const char *address)
> +{
> +g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE(graphics));
> +
> +gvir_config_object_set_attribute(GVIR_CONFIG_OBJECT(graphics), "listen", 
> address, NULL);



> +
> +gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(graphics),
> +"listen",
> +"address",
> +address);
> +
> +gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(graphics),
> +"listen",
> +"type",
> +"address");


and 
are 2 ways of expressing the same thing, with the  one 
being fairly new,
and  is slowly being deprecated. When  is used, a  node is automatically created iirc.

To handle the  nodes, I think it would make more sense to have
dedicated classes to handle it, and a
gvir_config_domain_graphics_spice_set_listen(GVirConfigDomainGraphicsSpice
*graphics, GVirConfigDomainListen *listen) method.

So for now, in gvir_config_domain_graphics_spice_set_listen_address I
would only set the "listen" attribute. Then I wonder if the function
should take a GInetAddress rather than a const char *?

The alignment of the code is also odd, we try to align the split
argumetns to function calls with the parens on the first line, does this
show up this way in your editor?
For example

gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(graphics),
"listen",
"type",
"address");

Christophe


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv4 5/9] Add functions for adding USB hubs to addrs

[Ján Tomko]

On Wed, Jul 13, 2016 at 06:43:51PM -0400, John Ferlan wrote:



On 07/01/2016 11:38 AM, Ján Tomko wrote:

Walk through all the usb hubs in the domain definition
that have a USB address specified, create the
corresponding structures in the virDomainUSBAddressSet
and mark the port it occupies as used.
---
 src/conf/domain_addr.c | 115 +
 1 file changed, 115 insertions(+)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 82fe295..2c511ba 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1437,6 +1437,109 @@ 
virDomainUSBAddressSetAddController(virDomainUSBAddressSetPtr addrs,
 }


+static ssize_t
+virDomainUSBAddressGetLastIdx(virDomainDeviceInfoPtr info)
+{
+ssize_t i;
+for (i = VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH - 1; i > 0; i--) {
+if (info->addr.usb.port[i] != 0)
+break;
+}
+return i;
+}


I would say the one thing that concerns me if some code calls
virDomainUSBAddressSetAddHub or virDomainUSBAddressFindPort without
first checking virDomainUSBAddressPortIsValid


+
+
+/* Find the USBAddressHub structure representing the hub/controller
+ * that corresponds to the bus/port path specified by info.
+ * Returns the index of the requested port in targetIdx.
+ */
+static virDomainUSBAddressHubPtr
+virDomainUSBAddressFindPort(virDomainUSBAddressSetPtr addrs,
+virDomainDeviceInfoPtr info,
+int *targetIdx,
+const char *portStr)
+{
+virDomainUSBAddressHubPtr hub = NULL;
+ssize_t i, lastIdx;
+
+if (info->addr.usb.bus >= addrs->nbuses ||
+!addrs->buses[info->addr.usb.bus]) {
+virReportError(VIR_ERR_XML_ERROR, _("Missing USB bus %u"),
+   info->addr.usb.bus);
+return NULL;
+}
+hub = addrs->buses[info->addr.usb.bus];
+
+lastIdx = virDomainUSBAddressGetLastIdx(info);


Again to the same point if lastIdx == 0, then someone didn't check
virDomainUSBAddressPortIsValid before calling...  and there's some
internal error, but I'm not sure it's worth checking... on the fence for
now I guess.



I think checking it would be paranoid.


+
+for (i = 0; i < lastIdx; i++) {
+/* ports are numbered from 1 */
+int portIdx = info->addr.usb.port[i] - 1;
+
+if (hub->nports <= portIdx) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("port %u out of range in USB address bus: %u port: 
%s"),
+   info->addr.usb.port[i],
+   info->addr.usb.bus,
+   portStr);
+return NULL;
+}
+hub = hub->hubs[portIdx];
+}
+
+*targetIdx = info->addr.usb.port[lastIdx] - 1;


It almost feels like the caller should do this since it's the only one
that cares, but that means the caller has not know about the
addr.usb.port... It's a coin flip.  Since you designed it this way, I'm
OK with it as is.



All the callers care.


+return hub;
+}
+
+
+#define VIR_DOMAIN_USB_HUB_PORTS 8
+
+static int
+virDomainUSBAddressSetAddHub(virDomainUSBAddressSetPtr addrs,
+ virDomainHubDefPtr hub)
+{
+virDomainUSBAddressHubPtr targetHub = NULL, newHub = NULL;
+int ret = -1;
+int targetPort;
+char *portStr = NULL;
+
+if (hub->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {


Oy - hub->type and hub->info.type... Seems there should be some other
check somewhere in the code that would ensure that the  used
for a  was USB 



Currently, usb is the only possible hub type.

Other address types get ignored by the condition below and are
overwritten by a freshly assigned USB address later.


+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Wrong address type for USB hub"));
+goto cleanup;
+}
+
+if (!(portStr = virDomainUSBAddressPortFormat(hub->info.addr.usb.port)))
+goto cleanup;
+
+VIR_DEBUG("Adding a USB hub with 8 ports on bus=%u port=%s",
+  hub->info.addr.usb.bus, portStr);
+
+if (!(newHub = virDomainUSBAddressHubNew(VIR_DOMAIN_USB_HUB_PORTS)))
+goto cleanup;
+
+if (!(targetHub = virDomainUSBAddressFindPort(addrs, &(hub->info), 
,
+  portStr)))
+goto cleanup;
+
+if (targetHub->hubs[targetPort]) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Dupicate USB hub on bus %u port %s"),


Duplicate

It could be a USB hub or another USB device, right?  IOW should we say
that the port is already in use by some other USB device.



At this point, only USB hubs have been added to the structure.


+   hub->info.addr.usb.bus, portStr);
+goto cleanup;
+}
+ignore_value(virBitmapSetBit(targetHub->ports, targetPort));
+targetHub->hubs[targetPort] = newHub;
+newHub = NULL;
+
+ 

Re: [libvirt] [PATCHv4 7/9] Assign addresses to USB devices

[Ján Tomko]

On Thu, Jul 14, 2016 at 10:59:11AM -0400, John Ferlan wrote:



On 07/01/2016 11:38 AM, Ján Tomko wrote:

Automatically assign addresses to USB devices.

Just like reserving, this is only done for newly defined domains.

https://bugzilla.redhat.com/show_bug.cgi?id=1215968
---
 src/conf/domain_addr.c | 125 -
 src/conf/domain_addr.h |  14 +++
 src/libvirt_private.syms   |   3 +
 src/qemu/qemu_domain_address.c |  64 +++
 .../qemuxml2argvdata/qemuxml2argv-bios-nvram.args  |   2 +-
 tests/qemuxml2argvdata/qemuxml2argv-bios.args  |   2 +-
 .../qemuxml2argv-controller-order.args |   8 +-
 .../qemuxml2argv-disk-usb-device-removable.args|   3 +-
 .../qemuxml2argv-disk-usb-device.args  |   2 +-
 .../qemuxml2argv-graphics-spice-timeout.args   |   2 +-
 .../qemuxml2argv-graphics-spice-usb-redir.args |   2 +-
 ...muxml2argv-hostdev-usb-address-device-boot.args |   2 +-
 .../qemuxml2argv-hostdev-usb-address-device.args   |   2 +-
 .../qemuxml2argv-hostdev-usb-address.args  |   2 +-
 .../qemuxml2argv-hugepages-numa.args   |   6 +-
 .../qemuxml2argv-input-usbmouse.args   |   2 +-
 .../qemuxml2argv-input-usbtablet.args  |   2 +-
 .../qemuxml2argv-pseries-usb-kbd.args  |   2 +-
 .../qemuxml2argv-serial-spiceport.args |   2 +-
 .../qemuxml2argv-smartcard-controller.args |   2 +-
 .../qemuxml2argv-smartcard-host-certificates.args  |   2 +-
 .../qemuxml2argv-smartcard-host.args   |   2 +-
 ...emuxml2argv-smartcard-passthrough-spicevmc.args |   2 +-
 .../qemuxml2argv-smartcard-passthrough-tcp.args|   2 +-
 .../qemuxml2argv-sound-device.args |   2 +-
 .../qemuxml2argv-usb-hub-conflict.args |  25 +
 .../qemuxml2argv-usb-port-autoassign.args  |  28 +
 .../qemuxml2argv-usb-port-autoassign.xml   |  27 +
 .../qemuxml2argv-usb-redir-boot.args   |   2 +-
 tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args |   2 +-
 tests/qemuxml2argvtest.c   |   3 +
 31 files changed, 317 insertions(+), 29 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.xml



Similar to 1/9, your .args file need ",sockets=1,cores=1,threads=1" due
to commit id 'e114b09157'.  To a degree I assume the values that were
generated for each args file are what qemu "expected" (or perhaps
generated before this change)?

The qemuxml2argv-usb-port-autoassign - I believe uses a default USB
controller - perhaps would be nice to have similar type tests for other
specific controller models to show/ensure the port allocation algorithm
does what is expected.


Will add those in a separate patch.



Furthermore a test similar to qemuxml2argv-pci-bridge-many-disks.xml -
that is something that will show progression through the algorithm in
order to assign more than a few port values.

Why would qemuxml2argv-usb-hub-conflict.args get generated?



Leftover from the development process.


+int
+virDomainUSBAddressAssign(virDomainUSBAddressSetPtr addrs,
+  virDomainDeviceInfoPtr info)
+{
+size_t i;
+int rc;
+
+if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
+if (!addrs->buses[info->addr.usb.bus]) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("USB bus %u requested but no controller "
+ "with that index is present"), 
info->addr.usb.bus);
+return -1;


For this case, the subsequent call at least won't get the !hub failure


+}
+rc = virDomainUSBAddressAssignFromBus(addrs, info, info->addr.usb.bus);
+if (rc >= -1)
+return rc;
+} else {
+for (i = 0; i < addrs->nbuses; i++) {
+rc = virDomainUSBAddressAssignFromBus(addrs, info, i);
+if (rc >= -1)
+return rc;


A !hub failure from caller falls into the code below...  Although I'm
not sure it's possible - I keep having to go back to remind myself...  A
info->type would I assume be NONE at this point, right?


Or any type other than USB.

At least one of the hubs should be non-NULL, otherwise we would have no
reason to add anything to addrs->buses. But even if it returns -2,
the error below is still true, even though it could be a bit cryptic.



John


+}
+}
+
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No free USB ports"));
+return -1;
+}
+
+
 int
 virDomainUSBAddressReserve(virDomainDeviceInfoPtr info,
void *data)
@@ -1608,3 +1713,21 @@ virDomainUSBAddressReserve(virDomainDeviceInfoPtr info,
 VIR_FREE(portStr);
 return ret;
 }
+
+

Re: [libvirt] [PATCHv4 1/9] Allow omitting USB port

[Ján Tomko]

On Wed, Jul 13, 2016 at 04:43:18PM -0400, John Ferlan wrote:



On 07/01/2016 11:38 AM, Ján Tomko wrote:

We were requiring a USB port path in the schema, but not enforcing it.
Omitting the USB port would lead to libvirt formatting it as (null).
Such domain cannot be started and will disappear after libvirtd restart
(since it cannot parse back the XML).

Only format the port if it has been specified and mark it as optional
in the XML schema.
---
 docs/schemas/domaincommon.rng  |  8 +++--
 src/conf/domain_conf.c |  5 ++-
 src/qemu/qemu_command.c|  3 +-
 .../qemuxml2argv-usb-port-missing.args | 26 
 .../qemuxml2argv-usb-port-missing.xml  | 25 +++
 tests/qemuxml2argvtest.c   |  3 ++
 .../qemuxml2xmlout-usb-port-missing.xml| 36 ++
 tests/qemuxml2xmltest.c|  1 +
 8 files changed, 100 insertions(+), 7 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-missing.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-missing.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-usb-port-missing.xml



As pointed out to me during the LUKS changes, if your "data" XML is
essentially the same as your output XML, you could make a file link from
output to data. See the *gic* files for an example. All I did was copy
the output to data, removed output, and then recreated output as file
link to data. It's not a "requirement" and it doesn't really matter to
me, but I think in the effort to reduce the size of things is what
started that. Of course, it's hard to know without tripping across it a
few times...


I believe there are quite a few cases like that and they deserve a
separate cleanup.

Even with a symlink we would end up checking the validity twice in
virschematest.

Jan

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [gconfig v2 2/2] config: Add vnc listen setter

[Christophe Fergeau]

Hey,

On Mon, Jul 18, 2016 at 04:36:40PM +0300, Visarion Alexandru wrote:
> Learn to set the address that vnc is listening on.

For what it's worth, this patch series
https://www.redhat.com/archives/libvir-list/2016-March/msg00993.html
would have allowed not to have to implement this both for VNC and SPICE.
Not sure why it did not land so far, I'll have to read back the old
thread/check with Fidencio.

Christophe


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/4] test: have default URI use file parsing helpers

[Cole Robinson]

On 07/12/2016 10:45 AM, John Ferlan wrote:
> 
> 
> On 07/01/2016 07:37 AM, Cole Robinson wrote:
>> This series reworks test:///default open handling to use the same
>> XML parsing helpers that a non-default test://$PATH open uses.
>> Saves some code, and makes it easier to extend test:///default,
>> for example in patch #4 which we want for testing nodedev events
>>
>> Cole Robinson (4):
>>   test: Introduce testOpenParse
>>   test: Move testOpenDefault definition later
>>   test: Have test:///default open use file parsing helpers
>>   test: Add scsi vport nodedev to test:///default
>>
>>  src/test/test_driver.c | 326 
>> +
>>  1 file changed, 141 insertions(+), 185 deletions(-)
>>
> 
> Nice...
> 
> Something for a "todo" list - adding snapshots for domains (to test
> testParseDomainSnapshots) and volumes for pools (to test
> testOpenVolumesForPool) for defaultConnXML.
> 
> w/r/t: 4/4...  If 'wwnn' and 'wwpn' weren't supplied, then
> virRandomGenerateWWN should have done the trick; however,
> testParseNodedevs passes a NULL virt_type so the generation fails.  Part
> of me wonders if should we create a TEST_DRIVER_OUI using "20" as
> the prefix just to "prove" that the code path works.  Your call though -
> easy enough to mock up a virRandomGenerateWWN or modify the non mocked
> version to support the test driver.
> 
> ACK series regardless of how you handle 4/4.

Thanks for the reviews. I'd pushed the patches as posted. I agree getting some
extra coverage for the WWN generation would be a nice addition, but this is
blocking some of the incoming GSOC work and I'm short on time at the moment...

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: Fix migration from old libvirt

[Pavel Hrdina]

On Mon, Jul 18, 2016 at 03:12:25PM +0200, Jiri Denemark wrote:
> Older libvirt versions send persistent XML in a migration cookie even
> when VIR_MIGRATE_PERSIST_DEST flag is not used, but current libvirt
> properly fails if the cookie contains unexpected flags. Thus migration
> from old libvirt fails with
> 
> internal error: Unsupported migration cookie feature persistent
> 
> unless VIR_MIGRATE_PERSIST_DEST flag is set.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1320500
> 
> Signed-off-by: Jiri Denemark 
> ---

ACK

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv5 5/5] Add test for GVirConfigDomainHostdevPci API

[Christophe Fergeau]

Acked-by: Christophe Fergeau 

On Wed, Jul 06, 2016 at 10:45:55PM +0100, Zeeshan Ali (Khattak) wrote:
> ---
>  tests/test-gconfig.c| 43 
> +
>  tests/xml/gconfig-domain-device-pci-hostdev.xml | 11 +++
>  2 files changed, 54 insertions(+)
>  create mode 100644 tests/xml/gconfig-domain-device-pci-hostdev.xml
> 
> diff --git a/tests/test-gconfig.c b/tests/test-gconfig.c
> index be55ef9..a26bb5f 100644
> --- a/tests/test-gconfig.c
> +++ b/tests/test-gconfig.c
> @@ -709,6 +709,47 @@ static void test_domain_device_usb_redir(void)
>  g_object_unref(G_OBJECT(domain));
>  }
>  
> +static void test_domain_device_pci_hostdev(void)
> +{
> +GVirConfigDomain *domain;
> +GVirConfigDomainAddressPci *address;
> +GVirConfigDomainHostdevPci *hostdev;
> +
> +domain = gvir_config_domain_new();
> +
> +hostdev = gvir_config_domain_hostdev_pci_new();
> +
> gvir_config_domain_hostdev_set_boot_order(GVIR_CONFIG_DOMAIN_HOSTDEV(hostdev),
>  1);
> +
> g_assert_cmpint(gvir_config_domain_hostdev_get_boot_order(GVIR_CONFIG_DOMAIN_HOSTDEV(hostdev)),
>  ==, 1);
> +gvir_config_domain_hostdev_pci_set_managed(hostdev, TRUE);
> +g_assert(gvir_config_domain_hostdev_pci_get_managed(hostdev) == TRUE);
> +gvir_config_domain_hostdev_pci_set_rom_bar(hostdev, TRUE);
> +gvir_config_domain_hostdev_pci_set_rom_file(hostdev, 
> "/etc/fake/boot.bin");
> +g_assert_cmpstr(gvir_config_domain_hostdev_pci_get_rom_file(hostdev), 
> ==, "/etc/fake/boot.bin");
> +g_assert(gvir_config_domain_hostdev_pci_get_rom_bar(hostdev));
> +
> +address = gvir_config_domain_address_pci_new();
> +gvir_config_domain_address_pci_set_domain(address, 1);
> +gvir_config_domain_address_pci_set_bus(address, 2);
> +gvir_config_domain_address_pci_set_slot(address, 3);
> +gvir_config_domain_address_pci_set_function(address, 4);
> +gvir_config_domain_hostdev_pci_set_address(hostdev, address);
> +g_object_unref(G_OBJECT(address));
> +
> +address = gvir_config_domain_hostdev_pci_get_address(hostdev);
> +g_assert(address != NULL);
> +g_assert_cmpint(gvir_config_domain_address_pci_get_domain(address), ==, 
> 1);
> +g_assert_cmpint(gvir_config_domain_address_pci_get_bus(address), ==, 2);
> +g_assert_cmpint(gvir_config_domain_address_pci_get_slot(address), ==, 3);
> +g_assert_cmpint(gvir_config_domain_address_pci_get_function(address), 
> ==, 4);
> +g_object_unref(G_OBJECT(address));
> +
> +gvir_config_domain_add_device(domain, GVIR_CONFIG_DOMAIN_DEVICE 
> (hostdev));
> +g_object_unref(G_OBJECT(hostdev));
> +
> +check_xml(domain, "gconfig-domain-device-pci-hostdev.xml");
> +
> +g_object_unref(G_OBJECT(domain));
> +}
>  
>  int main(int argc, char **argv)
>  {
> @@ -739,6 +780,8 @@ int main(int argc, char **argv)
>  test_domain_device_channel);
>  g_test_add_func("/libvirt-gconfig/domain-device-usb-redir",
>  test_domain_device_usb_redir);
> +g_test_add_func("/libvirt-gconfig/domain-device-pci-hostdev",
> +test_domain_device_pci_hostdev);
>  
>  return g_test_run();
>  }
> diff --git a/tests/xml/gconfig-domain-device-pci-hostdev.xml 
> b/tests/xml/gconfig-domain-device-pci-hostdev.xml
> new file mode 100644
> index 000..70e32ac
> --- /dev/null
> +++ b/tests/xml/gconfig-domain-device-pci-hostdev.xml
> @@ -0,0 +1,11 @@
> +
> +  
> +
> +  
> +  
> +  
> +
> +  
> +
> +  
> +
> -- 
> 2.7.4
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv5 3/5] Add GVirConfigDomainHostdevPci

[Christophe Fergeau]

gconfig: prefix to add to the subject line.

On Wed, Jul 06, 2016 at 10:45:53PM +0100, Zeeshan Ali (Khattak) wrote:
> +
> +const gchar 
> *gvir_config_domain_hostdev_pci_get_rom_file(GVirConfigDomainHostdevPci 
> *hostdev)
> +{
> +return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(hostdev), 
> "rom", "file");
> +}
> +
> +gboolean 
> gvir_config_domain_hostdev_pci_get_rom_bar(GVirConfigDomainHostdevPci 
> *hostdev)
> +{
> +xmlNodePtr hostdev_node;
> +xmlNodePtr rom_node;
> +const gchar *bar_str;
> +
> +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_HOSTDEV_PCI(hostdev), FALSE);
> +
> +hostdev_node = 
> gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(hostdev));
> +g_return_val_if_fail(hostdev_node != NULL, FALSE);
> +
> +rom_node = gvir_config_xml_get_element(hostdev_node, "rom", NULL);
> +if (!rom_node || !(rom_node->children))
> +/* When containain rom node is missing, default value is TRUE for 
> newer Qemu */
> +return TRUE;

"containain" typo

> +
> +bar_str = gvir_config_xml_get_attribute_content(rom_node, "bar");
> +if (g_strcmp0(bar_str, "on"))
> +return TRUE;
> +else
> +return FALSE;

Wouldn't:
bar_str = gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(hostdev), 
"rom", "bar");
return (g_strcmp0(bar_str, "on") == 0)
work too (and be simpler)?

Reviewed-by: Christophe Fergeau 

Christophe


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv5 2/5] Add GVirConfigDomainHostdev

[Christophe Fergeau]

Please add a gconfig: prefix to the subject line.

Acked-by: Christophe Fergeau 

Christophe

On Wed, Jul 06, 2016 at 10:45:52PM +0100, Zeeshan Ali (Khattak) wrote:
> Add API to read and write domain/devices/hostdev nodes. This patch only
> adds the baseclass and hence is not useful on it's own. A more specific
> subclass to represent PCI devices will be added in a following patch.
> ---
>  libvirt-gconfig/Makefile.am|   2 +
>  .../libvirt-gconfig-domain-device-private.h|   3 +
>  libvirt-gconfig/libvirt-gconfig-domain-device.c|   2 +-
>  libvirt-gconfig/libvirt-gconfig-domain-hostdev.c   | 190 
> +
>  libvirt-gconfig/libvirt-gconfig-domain-hostdev.h   |  76 +
>  libvirt-gconfig/libvirt-gconfig.h  |   1 +
>  libvirt-gconfig/libvirt-gconfig.sym|  10 ++
>  7 files changed, 283 insertions(+), 1 deletion(-)
>  create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-hostdev.c
>  create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-hostdev.h
> 
> diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am
> index f308539..a7c6c4e 100644
> --- a/libvirt-gconfig/Makefile.am
> +++ b/libvirt-gconfig/Makefile.am
> @@ -50,6 +50,7 @@ GCONFIG_HEADER_FILES = \
>   libvirt-gconfig-domain-graphics-sdl.h \
>   libvirt-gconfig-domain-graphics-spice.h \
>   libvirt-gconfig-domain-graphics-vnc.h \
> + libvirt-gconfig-domain-hostdev.h \
>   libvirt-gconfig-domain-input.h \
>   libvirt-gconfig-domain-interface.h \
>   libvirt-gconfig-domain-interface-bridge.h \
> @@ -141,6 +142,7 @@ GCONFIG_SOURCE_FILES = \
>   libvirt-gconfig-domain-graphics-sdl.c \
>   libvirt-gconfig-domain-graphics-spice.c \
>   libvirt-gconfig-domain-graphics-vnc.c \
> + libvirt-gconfig-domain-hostdev.c \
>   libvirt-gconfig-domain-input.c \
>   libvirt-gconfig-domain-interface.c \
>   libvirt-gconfig-domain-interface-bridge.c \
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-device-private.h 
> b/libvirt-gconfig/libvirt-gconfig-domain-device-private.h
> index 062c0e2..c45e1df 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain-device-private.h
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-device-private.h
> @@ -43,6 +43,9 @@ GVirConfigDomainDevice *
>  gvir_config_domain_graphics_new_from_tree(GVirConfigXmlDoc *doc,
>xmlNodePtr tree);
>  GVirConfigDomainDevice *
> +gvir_config_domain_hostdev_new_from_tree(GVirConfigXmlDoc *doc,
> + xmlNodePtr tree);
> +GVirConfigDomainDevice *
>  gvir_config_domain_interface_new_from_tree(GVirConfigXmlDoc *doc,
> xmlNodePtr tree);
>  GVirConfigDomainDevice *
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-device.c 
> b/libvirt-gconfig/libvirt-gconfig-domain-device.c
> index 3d2b9b3..8a75cea 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain-device.c
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-device.c
> @@ -66,7 +66,7 @@ gvir_config_domain_device_new_from_tree(GVirConfigXmlDoc 
> *doc,
>  } else if (xmlStrEqual(tree->name, (xmlChar*)"lease")) {
>  goto unimplemented;
>  } else if (xmlStrEqual(tree->name, (xmlChar*)"hostdev")) {
> -goto unimplemented;
> +return gvir_config_domain_hostdev_new_from_tree(doc, tree);
>  } else if (xmlStrEqual(tree->name, (xmlChar*)"redirdev")) {
>  type = GVIR_CONFIG_TYPE_DOMAIN_REDIRDEV;
>  } else if (xmlStrEqual(tree->name, (xmlChar*)"smartcard")) {
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c 
> b/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c
> new file mode 100644
> index 000..ce5f8aa
> --- /dev/null
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c
> @@ -0,0 +1,190 @@
> +/*
> + * libvirt-gconfig-domain-hostdev.c: libvirt domain hostdev configuration
> + *
> + * Copyright (C) 2016 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * .
> + *
> + * Authors: Zeeshan Ali (Khattak) 

Re: [libvirt] [PATCH 1/8] util: conf: Use long long when parsing

[Andrea Bolognani]

On Mon, 2016-07-18 at 09:16 +0100, Daniel P. Berrange wrote:
> On Fri, Jul 15, 2016 at 07:46:23PM +0200, Andrea Bolognani wrote:
> > 
> > Commit 6381c89f8cce changed virConfValue to store long long
> > integers instead of long integers; however, the temporary variable
> > used in virConfParseLong() was not updated accordingly, causing
> > trouble for 32-bit machines.
> > ---
> >  src/util/virconf.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/util/virconf.c b/src/util/virconf.c
> > index 33d6d92..66f8144 100644
> > --- a/src/util/virconf.c
> > +++ b/src/util/virconf.c
> > @@ -364,9 +364,9 @@ virConfSaveEntry(virBufferPtr buf, virConfEntryPtr cur)
> >   * Returns 0 in case of success and -1 in case of error
> >   */
> >  static int
> > -virConfParseLong(virConfParserCtxtPtr ctxt, long *val)
> > +virConfParseLong(virConfParserCtxtPtr ctxt, long long *val)
> >  {
> > -long l = 0;
> > +long long l = 0;
> >  int neg = 0;
> >  
> >  if (CUR == '-') {
> > @@ -476,7 +476,7 @@ virConfParseValue(virConfParserCtxtPtr ctxt)
> >  virConfValuePtr ret, lst = NULL, tmp, prev;
> >  virConfType type = VIR_CONF_NONE;
> >  char *str = NULL;
> > -long  l = 0;
> > +long long l = 0;
> >  
> >  SKIP_BLANKS;
> >  if (ctxt->cur >= ctxt->end) {
> 
> ACK, but perhaps you could add a test case to tests/virconftest to
> validate this too.

virconftest will already fail without this patch:

  4) int   ... Expected -6963472309248 got -1330322432

because the call to virConfGetValueLLong() will not be able
to return the correct value. So I think we're covered.

I've pushed the whole series, thanks for reviewing it!

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [gconfig v2 1/2] config: Add spice listen setter

[Visarion Alexandru]

Learn to set the address that spice is listening on.
---
 .../libvirt-gconfig-domain-graphics-spice.c| 18 ++
 .../libvirt-gconfig-domain-graphics-spice.h|  3 +++
 libvirt-gconfig/libvirt-gconfig.sym|  1 +
 3 files changed, 22 insertions(+)

diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c 
b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c
index 079ea27..a773084 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c
@@ -165,3 +165,21 @@ void 
gvir_config_domain_graphics_spice_set_gl(GVirConfigDomainGraphicsSpice *gra
 gvir_config_object_replace_child_with_attribute_enum
   (GVIR_CONFIG_OBJECT(graphics), "gl", "enable", G_TYPE_BOOLEAN, gl);
 }
+
+void 
gvir_config_domain_graphics_spice_set_listen_address(GVirConfigDomainGraphicsSpice
 *graphics,
+const char *address)
+{
+g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE(graphics));
+
+gvir_config_object_set_attribute(GVIR_CONFIG_OBJECT(graphics), "listen", 
address, NULL);
+
+gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(graphics),
+"listen",
+"address",
+address);
+
+gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(graphics),
+"listen",
+"type",
+"address");
+}
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.h 
b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.h
index 25c132e..4bca850 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.h
+++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.h
@@ -95,6 +95,9 @@ gvir_config_domain_graphics_spice_get_image_compression
 void gvir_config_domain_graphics_spice_set_gl(GVirConfigDomainGraphicsSpice 
*graphics,
   gboolean gl);
 
+void 
gvir_config_domain_graphics_spice_set_listen_address(GVirConfigDomainGraphicsSpice
 *graphics,
+const char *address);
+
 G_END_DECLS
 
 #endif /* __LIBVIRT_GCONFIG_DOMAIN_GRAPHICS_SPICE_H__ */
diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
b/libvirt-gconfig/libvirt-gconfig.sym
index f11f97a..86768ae 100644
--- a/libvirt-gconfig/libvirt-gconfig.sym
+++ b/libvirt-gconfig/libvirt-gconfig.sym
@@ -736,6 +736,7 @@ global:
 LIBVIRT_GCONFIG_0.2.4 {
gvir_config_domain_graphics_spice_set_gl;
gvir_config_domain_video_set_accel3d;
+   gvir_config_domain_graphics_spice_set_listen_address;
 } LIBVIRT_GCONFIG_0.2.2;
 
 #  define new API here using predicted next version number 
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [gconfig v2 2/2] config: Add vnc listen setter

[Visarion Alexandru]

Learn to set the address that vnc is listening on.
---
 libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.c | 18 ++
 libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.h |  3 +++
 libvirt-gconfig/libvirt-gconfig.sym   |  1 +
 3 files changed, 22 insertions(+)

diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.c 
b/libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.c
index fc26bb9..4187a8e 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.c
@@ -129,3 +129,21 @@ void 
gvir_config_domain_graphics_vnc_set_password(GVirConfigDomainGraphicsVnc *g
  "passwd", password,
  NULL);
 }
+
+void 
gvir_config_domain_graphics_vnc_set_listen_address(GVirConfigDomainGraphicsVnc 
*graphics,
+const char *address)
+{
+g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_VNC(graphics));
+
+gvir_config_object_set_attribute(GVIR_CONFIG_OBJECT(graphics), "listen", 
address, NULL);
+
+gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(graphics),
+"listen",
+"address",
+address);
+
+gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(graphics),
+"listen",
+"type",
+"address");
+}
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.h 
b/libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.h
index fe78621..e848cd7 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.h
+++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.h
@@ -76,6 +76,9 @@ void 
gvir_config_domain_graphics_vnc_set_port(GVirConfigDomainGraphicsVnc *graph
 void gvir_config_domain_graphics_vnc_set_password(GVirConfigDomainGraphicsVnc 
*graphics,
   const char *password);
 
+void 
gvir_config_domain_graphics_vnc_set_listen_address(GVirConfigDomainGraphicsVnc 
*graphics,
+const char *address);
+
 G_END_DECLS
 
 #endif /* __LIBVIRT_GCONFIG_DOMAIN_GRAPHICS_VNC_H__ */
diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
b/libvirt-gconfig/libvirt-gconfig.sym
index 86768ae..58b78b4 100644
--- a/libvirt-gconfig/libvirt-gconfig.sym
+++ b/libvirt-gconfig/libvirt-gconfig.sym
@@ -737,6 +737,7 @@ LIBVIRT_GCONFIG_0.2.4 {
gvir_config_domain_graphics_spice_set_gl;
gvir_config_domain_video_set_accel3d;
gvir_config_domain_graphics_spice_set_listen_address;
+   gvir_config_domain_graphics_vnc_set_listen_address;
 } LIBVIRT_GCONFIG_0.2.2;
 
 #  define new API here using predicted next version number 
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] qemu: Fix migration from old libvirt

[Jiri Denemark]

Older libvirt versions send persistent XML in a migration cookie even
when VIR_MIGRATE_PERSIST_DEST flag is not used, but current libvirt
properly fails if the cookie contains unexpected flags. Thus migration
from old libvirt fails with

internal error: Unsupported migration cookie feature persistent

unless VIR_MIGRATE_PERSIST_DEST flag is set.

https://bugzilla.redhat.com/show_bug.cgi?id=1320500

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_migration.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 0b1770b..463e624 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -6183,8 +6183,9 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
 cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK |
QEMU_MIGRATION_COOKIE_STATS |
QEMU_MIGRATION_COOKIE_NBD;
-if (flags & VIR_MIGRATE_PERSIST_DEST)
-cookie_flags |= QEMU_MIGRATION_COOKIE_PERSISTENT;
+/* Some older versions of libvirt always send persistent XML in the cookie
+ * even though VIR_MIGRATE_PERSIST_DEST was not used. */
+cookie_flags |= QEMU_MIGRATION_COOKIE_PERSISTENT;
 
 if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein,
cookieinlen, cookie_flags)))
-- 
2.9.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv4 3/9] Introduce virDomainUSBAddressSet

[Erik Skultety]

On 13/07/16 23:41, John Ferlan wrote:
> 
> 
> On 07/01/2016 11:38 AM, Ján Tomko wrote:
>> A new type to track USB addresses.
>>
>> Every  is represented by an
>> object of type virDomainUSBAddressHub located at buses[i].
>>
>> Each of these hubs has up to 'nports' ports.
>> If a port is occupied, it has the corresponding bit set in
>> the 'ports' bitmap, e.g. port 1 would have the 0th bit set.
>> If there is a hub on this port, then hubs[i] will point
>> to this hub.
>> ---
>>  src/conf/domain_addr.c   | 46 ++
>>  src/conf/domain_addr.h   | 22 ++
>>  src/libvirt_private.syms |  2 ++
>>  3 files changed, 70 insertions(+)
>>
> 
> Possibly trading fast/direct array access for linear searches matching
> numerical controller values.
> 
> I wonder how this would play in a "common"  algorithm. That is
> could PCI, SCSI, etc. make use of this paradigm. Secondarily whether
> "usb#" could be used as a key to a hashTable of addresses (where # is
> the controller index).  OK beyond the scope of this change...
> 
> The one confusing thing to see is "nports" is used to walk the 'hubs'
> array.  Maybe if 'ports' was renamed 'portmap' and then 'hubs' changed
> to 'ports'... I dunno, guess you understand where things are going
> better so I'm fine as is, albeit a bit odd.

^^^ I agree with John's rename proposal. It felt a bit odd for me as
well, but since I could not come up with a suitable alternative I didn't
want to just say "I'm not sure about this naming because it's not clear
enough..."

Erik

> 
> ACK for what's here
> 
> John
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-glib] gconfig: Add gvir_config_domain_video_get_model()

[Christophe Fergeau]

On Thu, Jul 14, 2016 at 02:11:05PM +0100, Zeeshan Ali (Khattak) wrote:
> Add a getter for model of domain video device.
> ---
>  libvirt-gconfig/libvirt-gconfig-domain-video.c | 12 
>  libvirt-gconfig/libvirt-gconfig-domain-video.h |  1 +
>  libvirt-gconfig/libvirt-gconfig.sym|  1 +
>  3 files changed, 14 insertions(+)
> 
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-video.c 
> b/libvirt-gconfig/libvirt-gconfig-domain-video.c
> index 64353bd..e38af6f 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain-video.c
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-video.c
> @@ -68,6 +68,18 @@ GVirConfigDomainVideo 
> *gvir_config_domain_video_new_from_xml(const gchar *xml,
>  return GVIR_CONFIG_DOMAIN_VIDEO(object);
>  }
>  
> +GVirConfigDomainVideoModel 
> gvir_config_domain_video_get_model(GVirConfigDomainVideo *video)
> +{
> +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_VIDEO(video), 
> GVIR_CONFIG_DOMAIN_VIDEO_MODEL_VGA);
> +
> +return gvir_config_object_get_attribute_genum
> +(GVIR_CONFIG_OBJECT(video),

I don't think you need to split these first 2 lines.

Looks good otherwise.

Christophe


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] config: Add listen setter

[Christophe Fergeau]

On Thu, Jul 14, 2016 at 06:57:35PM +0100, Zeeshan Ali (Khattak) wrote:
> Oh and when you provide updated patches, please make the subject
> prefix on emails 'gconfig' or 'libvirt-gconfig' so context is very
> clear. You'll want version too so --subject-prefix="gconfig v2" would
> be good.

I usually configure this once and for all for a given repository using
$ git config format.subjectprefix=libvirt-glib

Then I only need to use "git send-email -v2" and I get the right prefix.
with not much typing.

Christophe


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/2] Use static configuration method for iSCSI target discovery

[John Ferlan]

https://bugzilla.redhat.com/show_bug.cgi?id=1356436

Details in patches.

John Ferlan (2):
  util: Introduce virISCSINodeNew
  iscsi: Establish connection to target via static target login

 src/libvirt_private.syms|  1 +
 src/storage/storage_backend_iscsi.c |  9 
 src/util/viriscsi.c | 45 +
 src/util/viriscsi.h |  6 +
 4 files changed, 56 insertions(+), 5 deletions(-)

-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2] util: Introduce virISCSINodeNew

[John Ferlan]

https://bugzilla.redhat.com/show_bug.cgi?id=1356436

According to RFC 3721 (https://www.ietf.org/rfc/rfc3721.txt), there are
two ways to "discover" targets in/for the iSCSI environment. Discovery
is the process which allows the initiator to find the targets to which
it has access and at least one address at which each target may be
accessed.

The method currently implemented in libvirt using the virISCSIScanTargets
API is known as "SendTargets" discovery. This method is more useful when
the target IP Address and TCP port information are available, e.g. in
libvirt terms the "portal". It returns a list of targets for the portal.
>From that list, the target can be found. This operation can also fill an
iSCSI node table into which iSCSI logins may occur. Commit id '56057900'
altered that filling by adding the "--op nonpersistent" since it was
not necessarily desired to perform that for non libvirt related targets.

The second method is "Static Configuration". This method not only needs
the IP Address and TCP port (e.g. portal), but also the iSCSI target name.
In libvirt terms this would be the device path field from the iSCSI pool
 XML. This patch implements the second methodology using that
required device path as the targetname.

Signed-off-by: John Ferlan 
---
 src/libvirt_private.syms |  1 +
 src/util/viriscsi.c  | 45 +
 src/util/viriscsi.h  |  6 ++
 3 files changed, 52 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3580a72..edef70b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1714,6 +1714,7 @@ iptablesRemoveUdpOutput;
 virISCSIConnectionLogin;
 virISCSIConnectionLogout;
 virISCSIGetSession;
+virISCSINodeNew;
 virISCSINodeUpdate;
 virISCSIRescanLUNs;
 virISCSIScanTargets;
diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
index e705517..8115d09 100644
--- a/src/util/viriscsi.c
+++ b/src/util/viriscsi.c
@@ -444,6 +444,51 @@ virISCSIScanTargets(const char *portal,
 return ret;
 }
 
+/*
+ * virISCSINodeNew:
+ * @portal: address for iSCSI target
+ * @target: IQN and specific LUN target
+ *
+ * Usage of nonpersistent discovery in virISCSIScanTargets is useful primarily
+ * only when the target IQN is not known; however, since we have the target IQN
+ * usage of the "--op new" can be done. This avoids problems if "--op delete"
+ * had been used wiping out the static nodes determined by the scanning of
+ * all targets.
+ *
+ * NB: If already created, subsequent "--op new" commands do not return
+ * an error.
+ *
+ * Returns 0 on success, -1 w/ error message on error
+ */
+int
+virISCSINodeNew(const char *portal,
+const char *target)
+{
+virCommandPtr cmd = NULL;
+int status;
+int ret = -1;
+
+cmd = virCommandNewArgList(ISCSIADM,
+   "--mode", "node",
+   "--portal", portal,
+   "--targetname", target,
+   "--op", "new",
+   NULL);
+
+/* Ignore non-zero status.  */
+if (virCommandRun(cmd, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed new node mode for target '%s'"),
+   target);
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+virCommandFree(cmd);
+return ret;
+}
+
 
 int
 virISCSINodeUpdate(const char *portal,
diff --git a/src/util/viriscsi.h b/src/util/viriscsi.h
index 459249e..a44beea 100644
--- a/src/util/viriscsi.h
+++ b/src/util/viriscsi.h
@@ -54,6 +54,12 @@ virISCSIScanTargets(const char *portal,
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
 
 int
+virISCSINodeNew(const char *portal,
+const char *target)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
+ATTRIBUTE_RETURN_CHECK;
+
+int
 virISCSINodeUpdate(const char *portal,
const char *target,
const char *name,
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/2] iscsi: Establish connection to target via static target login

[John Ferlan]

https://bugzilla.redhat.com/show_bug.cgi?id=1356436

Commit id '56057900' altered the discovery of iSCSI node targets by
using the "--op nonpersistent". This caused issues for clean environments
or if by chance a "-m node -o delete" was executed.

Since each iSCSI Storage Pool has the required iSCSI target path, use
that and the virISCSINodeNew API in order to generate the iSCSI node record.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend_iscsi.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/storage/storage_backend_iscsi.c 
b/src/storage/storage_backend_iscsi.c
index bb33da2..84ad6f3 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -353,11 +353,10 @@ virStorageBackendISCSIStartPool(virConnectPtr conn,
 if ((session = virStorageBackendISCSISession(pool, true)) == NULL) {
 if ((portal = virStorageBackendISCSIPortal(>def->source)) == 
NULL)
 goto cleanup;
-/*
- * iscsiadm doesn't let you login to a target, unless you've
- * first issued a 'sendtargets' command to the portal :-(
- */
-if (virISCSIScanTargets(portal, NULL, NULL) < 0)
+
+/* Create a static node record for the IQN target. Must be done
+ * in order for login to the target */
+if (virISCSINodeNew(portal, pool->def->source.devices[0].path) < 0)
 goto cleanup;
 
 if (virStorageBackendISCSISetAuth(portal, conn, >def->source) < 
0)
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] virt-admin: Output srv-threadpool-info data as unsigned int rather than signed

[Erik Skultety]

Internally, all the data are represented as unsigned int, it is also documented
in the header file that users should use our exported constants that also
indicate that the data should be unsigned int. However, when polling for the
current server threadpool's configuration, virt-admin uses an incorrect
formatting parameter '%d' for printf. Instead, virt-admin should use formatting
parameter '%u'.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1356769

Signed-off-by: Erik Skultety 
---
 tools/virt-admin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index c4ee8f5..a59c4c7 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -458,7 +458,7 @@ cmdSrvThreadpoolInfo(vshControl *ctl, const vshCmd *cmd)
 }
 
 for (i = 0; i < nparams; i++)
-vshPrint(ctl, "%-15s: %d\n", params[i].field, params[i].value.ui);
+vshPrint(ctl, "%-15s: %u\n", params[i].field, params[i].value.ui);
 
 ret = true;
 
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/5] qemu: Add monitor APIs for the detection of halted VCPUs

[Peter Krempa]

On Thu, Jul 14, 2016 at 16:35:39 +0200, Viktor Mihajlovski wrote:
> New monitor function qemuMonitorGetCPUState added to retrieve the
> halted state of VCPUs via the query-cpus command.
> 
> Signed-off-by: Viktor Mihajlovski 
> Reviewed-by: Bjoern Walk 
> Signed-off-by: Boris Fiuczynski 
> ---
>  src/qemu/qemu_monitor.c  | 22 ++
>  src/qemu/qemu_monitor.h  |  2 ++
>  src/qemu/qemu_monitor_json.c | 52 ++
>  src/qemu/qemu_monitor_json.h |  2 ++
>  src/qemu/qemu_monitor_text.c | 54 
> +++-
>  src/qemu/qemu_monitor_text.h |  2 ++
>  6 files changed, 119 insertions(+), 15 deletions(-)
> 
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 098e654..0fdee29 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -1667,6 +1667,28 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
>  }
>  
>  
> +/**
> + * qemuMonitorGetCPUState:
> + * @mon: monitor
> + * @halted: returned array of boolean containing the vCPUs halted state
> + *
> + * Detects whether vCPUs are halted. Returns count of detected vCPUs on 
> success,
> + * 0 if qemu didn't report vcpus (does not report libvirt error),
> + * -1 on error (reports libvirt error).
> + */
> +int
> +qemuMonitorGetCPUState(qemuMonitorPtr mon,
> +   bool **halted)

Extraction of the data is cheap compared to calling the monitor. Also
this adds a lot of duplicated code. I've merged this to the code
returnign the thread ids and dropped the duplicate monitor helpers.

I've kept the extraction code though.

Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/5] qemu: Add domain support for VCPU halted state

[Peter Krempa]

On Thu, Jul 14, 2016 at 16:35:41 +0200, Viktor Mihajlovski wrote:
> Adding a field to the domain's private vcpu object to hold the halted
> state information.
> Adding two functions in support of the halted state:
> - qemuDomainGetVcpuHalted: retrieve the halted state from a
>   private vcpu object
> - qemuDomainRefreshVcpuHalted: obtain the per-vcpu halted states
>   via qemu monitor and store the results in the private vcpu objects
> 
> Signed-off-by: Viktor Mihajlovski 
> Reviewed-by: Bjoern Walk 
> Signed-off-by: Boris Fiuczynski 
> ---
>  src/qemu/qemu_domain.c | 82 
> ++
>  src/qemu/qemu_domain.h |  4 +++
>  2 files changed, 86 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 286f096..945a75d 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5650,6 +5650,88 @@ qemuDomainDetectVcpuPids(virQEMUDriverPtr driver,
>  return ret;
>  }
>  
> +/**
> + * qemuDomainGetVcpuHalted:
> + * @vm: domain object
> + * @vcpu: cpu id
> + *
> + * Returns the vCPU halted state.
> +  */
> +bool
> +qemuDomainGetVcpuHalted(virDomainObjPtr vm,
> +unsigned int vcpuid)
> +{
> +virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid);
> +return QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted;
> +}
> +
> +/**
> + * qemuDomainRefreshVcpuHalted:
> + * @driver: qemu driver data
> + * @vm: domain object
> + * @asyncJob: current asynchronous job type
> + *
> + * Updates vCPU halted state in the private data of @vm.
> + *
> + * Returns number of detected vCPUs on success, -1 on error and reports
> + * an appropriate error, -2 if the domain doesn't exist any more.
> + */
> +int
> +qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver,
> +virDomainObjPtr vm,
> +int asyncJob)
> +{
> +virDomainVcpuDefPtr vcpu;
> +size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
> +bool *cpuhalted = NULL;
> +int ncpuhalted;
> +size_t i;
> +int ret = -1;
> +
> +/* Not supported currently for TCG, see above
> + */
> +if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU)
> +return 0;
> +
> +if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> +return -1;
> +ncpuhalted = qemuMonitorGetCPUState(qemuDomainGetMonitor(vm), 
> );
> +if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> +ret = -2;
> +goto cleanup;
> +}
> +
> +/* Don't fail for older QEMUs
> + */
> +if (ncpuhalted <= 0) {
> +virResetLastError();
> +ret = 0;
> +goto cleanup;
> +}
> +
> +for (i = 0; i < maxvcpus; i++) {
> +vcpu = virDomainDefGetVcpu(vm->def, i);
> +
> +if (i < ncpuhalted)
> +QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted = cpuhalted[i];
> +else
> +QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted = false;

I've merged the 4 lines above to the function refreshing other VCPU
(since I'll be adding a patch that will make it not refresh the pids all
the time) and thus dropped the rest of this function.

I've tweaked the commit message an I'll resend it with the rest of the
patches soon.

Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 5/5] qemu: Ensure reported VCPU state is current in driver API

[Peter Krempa]

On Thu, Jul 14, 2016 at 16:35:42 +0200, Viktor Mihajlovski wrote:
> Refresh the VCPU halted states in API functions returning domain
> VCPU state information to make sure it's current. This affects
> qemuDomainGetVcpus and  qemuDomainGetStatsVcpu
> 
> Signed-off-by: Viktor Mihajlovski 
> Signed-off-by: Boris Fiuczynski 
> ---
>  src/qemu/qemu_driver.c | 23 +--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 

[..]

> @@ -5283,6 +5288,13 @@ qemuDomainGetVcpus(virDomainPtr dom,
>  goto cleanup;
>  }
>  
> +if (qemuDomainRefreshVcpuHalted(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) {

APIs calling the monitor need to enter a job since monitor calls unlock
the domain. There wouldn't be anything guarding the API.

> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   "%s",
> +   _("could not refresh CPU states"));
> +return -1;

This skips cleanup section where the VM object needs to be unlocked.

> +}
> +
>  ret = qemuDomainHelperGetVcpus(vm, info, NULL, maxinfo, cpumaps, maplen);
>  
>   cleanup:


> @@ -18565,6 +18577,13 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver 
> ATTRIBUTE_UNUSED,
>  VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0)
>  goto cleanup;

[1]

>  
> +if (qemuDomainRefreshVcpuHalted(driver, dom, QEMU_ASYNC_JOB_NONE) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   "%s",
> +   _("could not refresh CPU states"));
> +return -1;

This would leak memory allocated just above this hunk.

> +}
> +

I've fixed the problems mentioned above and tweaked the code to comply
with changes done to previous patches. I've also dropped the reviewed-by
tag since I'll be sending this for review again due to the changes
needed.

Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH sandbox 24/24] image: add 'list' command for viewing local templates

[Cedric Bosdonnat]

On Fri, 2016-07-15 at 14:08 +0100, Daniel P. Berrange wrote:
> Introduce a command able to list locally stored image
> templates:
> 
>   $ virt-sandbox-image list
>   docker:library/ubuntu?tag=14.04.1
>   docker:library/ubuntu?tag=14.04.2
>   virt-builder:/fedora-23
> 
> or restrict to a single source type
> 
>   $ virt-sandbox-image list --source docker
>   docker:library/ubuntu?tag=14.04.1
>   docker:library/ubuntu?tag=14.04.2
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  libvirt-sandbox/image/cli.py | 24 ++
>  libvirt-sandbox/image/sources/base.py| 10 ++
>  libvirt-sandbox/image/sources/docker.py  | 22 
>  libvirt-sandbox/image/sources/virtbuilder.py | 18 +
>  libvirt-sandbox/image/template.py| 30 
> +++-
>  5 files changed, 95 insertions(+), 9 deletions(-)
> 
> diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
> index b0d864f..66854e4 100644
> --- a/libvirt-sandbox/image/cli.py
> +++ b/libvirt-sandbox/image/cli.py
> @@ -135,6 +135,18 @@ def run(args):
>  os.unlink(diskfile)
>  source.post_run(tmpl, template_dir, name)
>  
> +def list_cached(args):
> +tmpls = []
> +if args.source is not None:
> +tmpls.extend(template.Template.get_all(args.source,
> +   "%s/%s" % (args.template_dir, 
> args.source)))
> +else:
> +for source in ["docker", "virt-builder"]:
> +tmpls.extend(template.Template.get_all(source,
> +   "%s/%s" % 
> (args.template_dir, source)))
> +for tmpl in tmpls:
> +print tmpl
> +
>  def requires_template(parser):
>  parser.add_argument("template",
>  help=_("URI of the template"))
> @@ -221,6 +233,17 @@ def gen_run_args(subparser):
>  
>  parser.set_defaults(func=run)
>  
> +def gen_list_args(subparser):
> +parser = gen_command_parser(subparser, "list",
> +_("List locally cached images"))
> +requires_debug(parser)
> +requires_template_dir(parser)
> +
> +parser.add_argument("-s","--source",
> +help=_("Name of the template source"))
> +
> +parser.set_defaults(func=list_cached)
> +
>  def main():
>  parser = argparse.ArgumentParser(description="Sandbox Container Image 
> Tool")
>  
> @@ -228,6 +251,7 @@ def main():
>  gen_delete_args(subparser)
>  gen_create_args(subparser)
>  gen_run_args(subparser)
> +gen_list_args(subparser)
>  
>  args = parser.parse_args()
>  if args.debug:
> diff --git a/libvirt-sandbox/image/sources/base.py 
> b/libvirt-sandbox/image/sources/base.py
> index f70551d..e4e4e41 100644
> --- a/libvirt-sandbox/image/sources/base.py
> +++ b/libvirt-sandbox/image/sources/base.py
> @@ -34,6 +34,16 @@ class Source():
>  def __init__(self):
>  pass
>  
> +@abstractmethod
> +def list_templates(self, templatedir):
> +"""
> +:param templatedir: local directory path in which to store the 
> template
> +
> +Get a list of all templates that are locally cached
> +
> +:returns: a list of libvirt_sandbox.template.Template objects
> +"""
> +pass
>  
>  @abstractmethod
>  def has_template(self, template, templatedir):
> diff --git a/libvirt-sandbox/image/sources/docker.py 
> b/libvirt-sandbox/image/sources/docker.py
> index 291a305..dd72db7 100644
> --- a/libvirt-sandbox/image/sources/docker.py
> +++ b/libvirt-sandbox/image/sources/docker.py
> @@ -32,6 +32,7 @@ import urlparse
>  import hashlib
>  from abc import ABCMeta, abstractmethod
>  import copy
> +from libvirt_sandbox.image.template import Template
>  
>  from . import base
>  
> @@ -360,6 +361,27 @@ class DockerSource(base.Source):
>  except Exception:
>  return False
>  
> +def list_templates(self, templatedir):
> +indexes = []
> +imagedirs = os.listdir(templatedir)
> +for imagetagid in imagedirs:
> +indexfile = templatedir + "/" + imagetagid + "/index.json"
> +if os.path.exists(indexfile):
> +with open(indexfile,"r") as f:
> +index = json.load(f)
> +indexes.append(index)
> +
> +return [
> +Template(source="docker",
> + protocol=None,
> + hostname=None,
> + port=None,
> + username=None,
> + password=None,
> + path="/%s/%s" % (index.get("repo", "library"), 
> index["name"]),
> + params={
> + "tag": index.get("tag", "latest"),
> + }) for index in indexes]
>  
>  def has_template(self, template, templatedir):
>  try:
> diff --git 

Re: [libvirt] [PATCH v2] qemuhotplugtest: Add tests for ccw devices

[Martin Kletzander]

On Tue, Jul 12, 2016 at 03:08:22PM +0200, Tomasz Flendrich wrote:

These are a few simple and one complex testcases.
The simple ones test attaching and detaching ccw devices
with both implicitly and explicitly stated addresses.
In the complex one, attaching and detaching a device
should make the address free to reuse.

I have plan to rework the address handling, so testcases
that verify hotplugging ccw devices will help in avoiding
regression.

In this commit, some device files are duplicated because
of the way qemuhotplug.c calculates the expected xml filenames.
I plan on changing that to explicitly stating the basis domain
xml, the device xml, and the expected xml.
---

Changed in v2:
* Add a newline to the end of qemuhotplug-ccw-virtio-2-explicit.xml,
 so that syntax-check doesn't complain.
* Rename two files: remove the "explicit" word where the address
 is actually implicit.

Link to the previous patch:
https://www.redhat.com/archives/libvir-list/2016-July/msg00264.html




[...]


diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-ccw-virtio-1-explicit.xml 
b/tests/qemuhotplugtestdevices/qemuhotplug-ccw-virtio-1-explicit.xml
new file mode 100644
index 000..74bd6a9
--- /dev/null
+++ b/tests/qemuhotplugtestdevices/qemuhotplug-ccw-virtio-1-explicit.xml
@@ -0,0 +1,8 @@
+
+  
+  
+  
+  
+  
+  
+


I know you weren't aiming for that, but this got me thinking, are we
checking that unplug works with not fully specified XML?  I mean that
when unplugging, you don't need to specify everything.  Whatever
uniquely identifies the device should be enough.  So for example just
address or target, definitely no need for the readonly and shareable
flags.  But as said, that's not meant to be part of this patch.

[...]


diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-ccw-virtio-2-explicit.xml 
b/tests/qemuhotplugtestdevices/qemuhotplug-ccw-virtio-2-explicit.xml
new file mode 100644
index 000..b2cb161
--- /dev/null
+++ b/tests/qemuhotplugtestdevices/qemuhotplug-ccw-virtio-2-explicit.xml
@@ -0,0 +1,8 @@
+
+  
+  
+  


vde2 is an address for a partition.  This works?  I don't think it
should.

[...]


diff --git 
a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-explicit.xml
 
b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-explicit.xml
new file mode 100644
index 000..ff94b6c
--- /dev/null
+++ 
b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-explicit.xml
@@ -0,0 +1,73 @@
+
+  hotplug
+  d091ea82-29e6-2e34-3005-f02617b36e87
+  4194304
+  4194304
+  4
+  
+hvm
+
+  
+  
+
+
+
+  
+  
+  destroy
+  restart
+  restart
+  
+/usr/libexec/qemu-kvm
+
+  
+  
+  
+  
+  
+  
+  


This ...


+  
+
+
+  
+  
+  
+  
+  
+  
+  


... and this?  This will never work, so that's another bug right here.


+  


By the way, attaching a device without explicit address and without
target dev= specified (only bus='virtio')  Should work and that would
actually test address allocation.  By allocating disk with target
dev='XX' the address gets calculated from that device, so no allocation
is being tested.

Other than these problems the approach is fine.

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 7/8] util: conf: Clarify choice between VIR_CONF_LONG and VIR_CONF_ULONG

[Daniel P. Berrange]

On Fri, Jul 15, 2016 at 07:46:29PM +0200, Andrea Bolognani wrote:
> We use unsigned long long integers unless we need to store a
> negative value. Rewrite the condition to make this more obvious.
> ---
>  src/util/virconf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

ACK

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 8/8] util: conf: Rename VIR_CONF_{U, }LONG -> VIR_CONF_{U, }LLONG

[Daniel P. Berrange]

On Fri, Jul 15, 2016 at 07:46:30PM +0200, Andrea Bolognani wrote:
> Since commit 6381c89f8cce, we're storing long long integers
> instead of long integers. Rename the corresponding virConfType
> value accordingly.
> ---
>  src/util/virconf.c | 26 +-
>  src/util/virconf.h |  4 ++--
>  src/xenconfig/xen_common.c |  8 
>  tests/virconftest.c| 16 
>  4 files changed, 27 insertions(+), 27 deletions(-)

ACK


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/8] util: conf: Claim the proper range for signed numbers

[Daniel P. Berrange]

On Fri, Jul 15, 2016 at 07:46:26PM +0200, Andrea Bolognani wrote:
> virConfGetValueLLong() errors out if the value is too big to
> fit into a long long integer, but claims the supported range
> to be (0,LLONG_MAX) instead of (LLONG_MIN,LLONG_MAX).
> ---
>  src/util/virconf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

ACK


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 6/8] util: conf: Fix parameters alignment

[Daniel P. Berrange]

On Fri, Jul 15, 2016 at 07:46:28PM +0200, Andrea Bolognani wrote:
> The parameters for virConfGetValueLLong() were not aligned
> properly.
> ---
>  src/util/virconf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/virconf.c b/src/util/virconf.c
> index d635539..f2d543c 100644
> --- a/src/util/virconf.c
> +++ b/src/util/virconf.c
> @@ -1304,8 +1304,8 @@ int virConfGetValueSSizeT(virConfPtr conf,
>   * Returns: 1 if the value was present, 0 if missing, -1 on error
>   */
>  int virConfGetValueLLong(virConfPtr conf,
> -const char *setting,
> -long long *value)
> + const char *setting,
> + long long *value)

ACK

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/8] util: conf: Improve virConfGet*() logic

[Daniel P. Berrange]

On Fri, Jul 15, 2016 at 07:46:24PM +0200, Andrea Bolognani wrote:
> When parsing numeric values, we always store them as unsigned
> unless they're negative. We can use this fact to simplify the
> logic by removing a bunch of unnecessary checks.
> ---
>  src/util/virconf.c | 40 +---
>  1 file changed, 13 insertions(+), 27 deletions(-)

ACK


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 5/8] util: conf: Fix comment for virConfGetValueULLong()

[Daniel P. Berrange]

On Fri, Jul 15, 2016 at 07:46:27PM +0200, Andrea Bolognani wrote:
> The name of the function is not virConfGetValueULongLong().
> ---
>  src/util/virconf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

ACK

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/8] util: conf: Add integer casts

[Daniel P. Berrange]

On Fri, Jul 15, 2016 at 07:46:25PM +0200, Andrea Bolognani wrote:
> For good measure.
> ---
>  src/util/virconf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

ACK


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/8] util: conf: Use long long when parsing

[Daniel P. Berrange]

On Fri, Jul 15, 2016 at 07:46:23PM +0200, Andrea Bolognani wrote:
> Commit 6381c89f8cce changed virConfValue to store long long
> integers instead of long integers; however, the temporary variable
> used in virConfParseLong() was not updated accordingly, causing
> trouble for 32-bit machines.
> ---
>  src/util/virconf.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virconf.c b/src/util/virconf.c
> index 33d6d92..66f8144 100644
> --- a/src/util/virconf.c
> +++ b/src/util/virconf.c
> @@ -364,9 +364,9 @@ virConfSaveEntry(virBufferPtr buf, virConfEntryPtr cur)
>   * Returns 0 in case of success and -1 in case of error
>   */
>  static int
> -virConfParseLong(virConfParserCtxtPtr ctxt, long *val)
> +virConfParseLong(virConfParserCtxtPtr ctxt, long long *val)
>  {
> -long l = 0;
> +long long l = 0;
>  int neg = 0;
>  
>  if (CUR == '-') {
> @@ -476,7 +476,7 @@ virConfParseValue(virConfParserCtxtPtr ctxt)
>  virConfValuePtr ret, lst = NULL, tmp, prev;
>  virConfType type = VIR_CONF_NONE;
>  char *str = NULL;
> -long  l = 0;
> +long long l = 0;
>  
>  SKIP_BLANKS;
>  if (ctxt->cur >= ctxt->end) {

ACK, but perhaps you could add a test case to tests/virconftest to
validate this too.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] qemu: Drop default channel path during migration

[Jiri Denemark]

On Mon, Jul 11, 2016 at 08:18:58 +0200, Michal Privoznik wrote:
> On 08.07.2016 17:40, Jiri Denemark wrote:
> > Migration to an older libvirt (pre v1.3.0-175-g7140807) is broken
> > because older versions of libvirt generated different channel paths and
> > they didn't drop the default paths when parsing domain XMLs. We'd get
> > such a nice error message:
> > 
> > internal error: process exited while connecting to monitor:
> > 2016-07-08T15:28:02.665706Z qemu-kvm: -chardev socket,
> > id=charchannel0,path=/var/lib/libvirt/qemu/channel/target/
> > domain-3-nest/org.qemu.guest_agent.0,server,nowait: Failed to bind
> > socket to /var/lib/libvirt/qemu/channel/target/domain-3-nest/
> > org.qemu.guest_agent.0: No such file or directory
> > 
> > That said, we should not even format the default paths when generating a
> > migratable XML.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1320470
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> >  src/qemu/qemu_domain.c | 43 +++
> >  1 file changed, 27 insertions(+), 16 deletions(-)
> 
> ACK

Thanks, pushed.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-block] [libvirt RFC PATCH 00/10] Add support for qemu's JSON pseudo'protocol for backing store

[Peter Krempa]

On Fri, Jul 15, 2016 at 13:11:16 -0600, Eric Blake wrote:
> On 07/15/2016 07:49 AM, Daniel P. Berrange wrote:
> > On Fri, Jul 15, 2016 at 03:46:33PM +0200, Peter Krempa wrote:
> >> Libvirt didn't handle this for a long time and VMs with such config would 
> >> not
> >> start we should implement it.
> >>
> >> Using JSON is basically the only option to specify advanced configuration 
> >> for a
> >> backing file.
> >>
> >> Field names were taken from qemu's source since there isn't really
> >> documentation for this.
> > 
> > Actually, this syntax is formally specified via QEMU   QAPI schema
> > language. In particular for block device options see 
> > $QEMU/qapi/block-core.json
> 
> Except that it is incomplete; as long as BlockdevOptions doesn't cover

and wrong in some cases:

ftp,http and others implemented by curl use the 'url' filed but the
documentation still hints that 'filename' is used while the driver does
not allow it.

$ qemu-img create -f qcow2 -b 'json: { "file.driver":"http", 
"file.filename":"http://...; }' bla.img
qemu-img: bla.img: curl block driver requires an 'url' option

> every driver, such as NBD, gluster, and sheepdog, there are still some

Drivers such as NBD and SSH are actually implemented but not documented.

> und(er)ocumented aspects.

Mostly how this maps to the JSON pseudo-protocol as blockdev add uses
nested JSON objects whereas the JSON protocol requires the fileds in the
"flattened" form.

The code looks like the only credible source in this case.

Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Debug: Record the VM PID in per-VM logs.

[Prerna Saxena]

For certain situations such as Out-of-memory scenarios, it is helpful to
record the QEMU PID in the per-VM logs, so that correlation with kernel-reported
events is easier.

While this is already recorded in libvirt daemon logs as a DEBUG message,
I believe it merits more attention :-)
Hence introducing it in per-VM log.

Signed-off-by: Prerna Saxena 
---
 src/qemu/qemu_process.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4adb14e..2d3b0f5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4983,6 +4983,7 @@ qemuProcessLaunch(virConnectPtr conn,
 int ret = -1;
 int rv;
 int logfile = -1;
+char *msg_buffer = NULL;
 qemuDomainLogContextPtr logCtxt = NULL;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virCommandPtr cmd = NULL;
@@ -5095,8 +5096,14 @@ qemuProcessLaunch(virConnectPtr conn,
_("Domain %s didn't show up"), vm->def->name);
 rv = -1;
 }
+
 VIR_DEBUG("QEMU vm=%p name=%s running with pid=%llu",
   vm, vm->def->name, (unsigned long long)vm->pid);
+ignore_value(virAsprintf(_buffer,
+"QEMU vm=%p name=%s running with pid=%llu",
+vm, vm->def->name, (unsigned long long)vm->pid));
+qemuLogOperation(vm, msg_buffer, NULL, logCtxt);
+virFree(msg_buffer);
 } else {
 VIR_DEBUG("QEMU vm=%p name=%s failed to spawn",
   vm, vm->def->name);
-- 
1.8.1.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list