8:47 p.m.
From: Tomasz Flendrich <Darge(a)users.noreply.github.com>
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.
I know that USB addresses were added and that they are cached now. I am sure
that they can be calculated on demand too though.
If this gets accepted, I will rebase the other patches that depend on this one,
making functions less dependant on qemu, and removing the code duplication.
Changes in v2:
* I rebased it, because there were some changes that added a usb address set.
* virDomainReleaseDeviceAddress is no longer deleted, because it's used
for usb addresses. It also means that in qemu_hotplug.c, this function's
calls are not deleted anymore.
* In qemu_hotplug.c, in one place, virDomainPCIAddressSetFree() was previously
called after the "error:" label. I moved it to the "cleanup:", so
the address
set is freed even if an error occurs.
* Add one missing free of pci address set, in qemuDomainAttachNetDevice().
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 | 131 +++++++++---------------------------
src/conf/domain_addr.h | 19 +-----
src/libvirt_private.syms | 4 +-
src/qemu/qemu_domain.c | 3 -
src/qemu/qemu_domain.h | 3 -
src/qemu/qemu_domain_address.c | 147 ++++++++++++++++++++---------------------
src/qemu/qemu_domain_address.h | 9 +++
src/qemu/qemu_hotplug.c | 111 ++++++++++++++++++++++++-------
8 files changed, 201 insertions(+), 226 deletions(-)
--
2.7.4 (Apple Git-66)
8:47 p.m.
New subject: [libvirt] [PATCH v2 01/10] add virDomainVirtioSerialAddrSetCreateFromDomain
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 c3469ee..e09b409 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 ce94981..4584e0a 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 9396c4e..6b54812 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -126,6 +126,7 @@ virDomainVirtioSerialAddrRelease;
virDomainVirtioSerialAddrReserve;
virDomainVirtioSerialAddrSetAddControllers;
virDomainVirtioSerialAddrSetCreate;
+virDomainVirtioSerialAddrSetCreateFromDomain;
virDomainVirtioSerialAddrSetFree;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 787b357..2bd095f 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)
9:27 a.m.
New subject: [libvirt] [PATCH v2 01/10] add virDomainVirtioSerialAddrSetCreateFromDomain
On Sat, Jul 23, 2016 at 03:47:06AM +0200, Tomasz Flendrich wrote:
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 c3469ee..e09b409 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;
One common thing to do in libvirt's codebase is (although of course
there are too many ways to approach this):
Initialize this to NULL,
+ 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;
set it to NULL here as well,
+ cleanup:
+ if (!ret)
+ virDomainVirtioSerialAddrSetFree(addrs);
and call this unconditionally.
ACK with that changed.
8:47 p.m.
New subject: [libvirt] [PATCH v2 02/10] qemu_hotplug: generate vioserial address list on demand
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 4e4bf82..5c82361 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1551,21 +1551,28 @@ 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,
&chr->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, &chr->info) < 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_USB) {
@@ -1575,20 +1582,27 @@ qemuDomainAttachChrDeviceAssignAddr(qemuDomainObjPrivatePtr priv,
} 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,
&chr->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,
@@ -1611,7 +1625,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)
10:01 a.m.
New subject: [libvirt] [PATCH v2 02/10] qemu_hotplug: generate vioserial address list on demand
On Sat, Jul 23, 2016 at 03:47:07AM +0200, Tomasz Flendrich wrote:
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 4e4bf82..5c82361 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1551,21 +1551,28 @@ 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,
&chr->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, &chr->info) < 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_USB) {
You're missing one goto cleanup/ret = 1 here between these hunks.
ACK with that changed.
8:47 p.m.
New subject: [libvirt] [PATCH v2 03/10] qemu: remove vioserialaddrs caching
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 | 18 +++---------------
3 files changed, 3 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ceac22f..4aa9b51 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1329,7 +1329,6 @@ qemuDomainObjPrivateFree(void *data)
virCgroupFree(&priv->cgroup);
virDomainPCIAddressSetFree(priv->pciaddrs);
virDomainCCWAddressSetFree(priv->ccwaddrs);
- virDomainVirtioSerialAddrSetFree(priv->vioserialaddrs);
virDomainUSBAddressSetFree(priv->usbaddrs);
virDomainChrSourceDefFree(priv->monConfig);
qemuDomainObjFreeJob(priv);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index d10ebcc..eefd8d4 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;
virDomainUSBAddressSetPtr usbaddrs;
virQEMUCapsPtr qemuCaps;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 2bd095f..0df112b 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:
@@ -1767,7 +1758,7 @@ qemuDomainAssignAddresses(virDomainDefPtr def,
virDomainObjPtr obj,
bool newDomain)
{
- if (qemuDomainAssignVirtioSerialAddresses(def, obj) < 0)
+ if (qemuDomainAssignVirtioSerialAddresses(def) < 0)
return -1;
if (qemuDomainAssignSpaprVIOAddresses(def, qemuCaps) < 0)
@@ -1809,10 +1800,7 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
&info->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));
+
if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB &&
priv->usbaddrs &&
virDomainUSBAddressRelease(priv->usbaddrs, info) < 0)
--
2.7.4 (Apple Git-66)
10:03 a.m.
New subject: [libvirt] [PATCH v2 03/10] qemu: remove vioserialaddrs caching
On Sat, Jul 23, 2016 at 03:47:08AM +0200, Tomasz Flendrich wrote:
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.
This patch shows how useless the keeping of that set was so neatly :) ACK
8:47 p.m.
New subject: [libvirt] [PATCH v2 04/10] Add qemuDomainCCWAddrSetCreateFromDomain
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 0df112b..b23790f 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)
8:47 p.m.
New subject: [libvirt] [PATCH v2 05/10] qemu_hotplug: generate ccw address list on demand
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 5c82361..3deeb0b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -313,6 +313,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
bool driveAdded = false;
bool secobjAdded = false;
bool encobjAdded = false;
+ virDomainCCWAddressSetPtr ccwaddrs = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
const char *src = virDomainDiskGetSource(disk);
virJSONValuePtr secobjProps = NULL;
@@ -337,7 +338,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
goto cleanup;
if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
- if (virDomainCCWAddressAssign(&disk->info, priv->ccwaddrs,
+ if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
+ goto error;
+ if (virDomainCCWAddressAssign(&disk->info, ccwaddrs,
!disk->info.addr.ccw.assigned) < 0)
goto error;
} else if (!disk->info.type ||
@@ -416,6 +419,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
virJSONValueFree(secobjProps);
virJSONValueFree(encobjProps);
qemuDomainSecretDiskDestroy(disk);
+ virDomainCCWAddressSetFree(ccwaddrs);
VIR_FREE(devstr);
VIR_FREE(drivestr);
VIR_FREE(drivealias);
@@ -459,6 +463,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) {
@@ -500,7 +505,9 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &controller->info)
< 0)
goto cleanup;
} else if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
- if (virDomainCCWAddressAssign(&controller->info, priv->ccwaddrs,
+ if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
+ goto cleanup;
+ if (virDomainCCWAddressAssign(&controller->info, ccwaddrs,
!controller->info.addr.ccw.assigned) < 0)
goto cleanup;
}
@@ -533,6 +540,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
qemuDomainReleaseDeviceAddress(vm, &controller->info, NULL);
VIR_FREE(devstr);
+ virDomainCCWAddressSetFree(ccwaddrs);
return ret;
}
@@ -901,6 +909,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
int actualType;
virNetDevBandwidthPtr actualBandwidth;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ virDomainCCWAddressSetPtr ccwaddrs = NULL;
size_t i;
/* preallocate new slot for device */
@@ -1038,7 +1047,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(&net->info, priv->ccwaddrs,
+ if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
+ goto cleanup;
+ if (virDomainCCWAddressAssign(&net->info, ccwaddrs,
!net->info.addr.ccw.assigned) < 0)
goto cleanup;
} else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
@@ -1212,6 +1223,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
VIR_FREE(vhostfd);
VIR_FREE(vhostfdName);
virObjectUnref(cfg);
+ virDomainCCWAddressSetFree(ccwaddrs);
return ret;
@@ -1692,6 +1704,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
bool chardevAdded = false;
bool objAdded = false;
virJSONValuePtr props = NULL;
+ virDomainCCWAddressSetPtr ccwaddrs = NULL;
const char *type;
int ret = -1;
int rv;
@@ -1722,9 +1735,11 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &rng->info) < 0)
return -1;
} else if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
- if (virDomainCCWAddressAssign(&rng->info, priv->ccwaddrs,
+ if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
+ goto cleanup;
+ if (virDomainCCWAddressAssign(&rng->info, ccwaddrs,
!rng->info.addr.ccw.assigned) < 0)
- return -1;
+ goto cleanup;
}
/* build required metadata */
@@ -1775,6 +1790,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
VIR_FREE(charAlias);
VIR_FREE(objAlias);
VIR_FREE(devstr);
+ virDomainCCWAddressSetFree(ccwaddrs);
return ret;
exit_monitor:
--
2.7.4 (Apple Git-66)
10:11 a.m.
New subject: [libvirt] [PATCH v2 05/10] qemu_hotplug: generate ccw address list on demand
On Sat, Jul 23, 2016 at 03:47:10AM +0200, Tomasz Flendrich wrote:
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 5c82361..3deeb0b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -337,7 +338,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
goto cleanup;
if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
- if (virDomainCCWAddressAssign(&disk->info, priv->ccwaddrs,
+ if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
+ goto error;
+ if (virDomainCCWAddressAssign(&disk->info, ccwaddrs,
!disk->info.addr.ccw.assigned) < 0)
Ideally, we would create a wrapper for these two functions but that's
just a nit for the future. Also just a guess, maybe it'll turn out that
we won't need it at all...
ACK
8:47 p.m.
New subject: [libvirt] [PATCH v2 06/10] qemu: remove ccwaddrs caching
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 | 23 +++--------------------
3 files changed, 3 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4aa9b51..9045f37 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1328,7 +1328,6 @@ qemuDomainObjPrivateFree(void *data)
virCgroupFree(&priv->cgroup);
virDomainPCIAddressSetFree(priv->pciaddrs);
- virDomainCCWAddressSetFree(priv->ccwaddrs);
virDomainUSBAddressSetFree(priv->usbaddrs);
virDomainChrSourceDefFree(priv->monConfig);
qemuDomainObjFreeJob(priv);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index eefd8d4..6f349a1 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;
virDomainUSBAddressSetPtr usbaddrs;
virQEMUCapsPtr qemuCaps;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index b23790f..66ab70e 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:
@@ -1779,7 +1768,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);
@@ -1804,13 +1793,7 @@ 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) &&
+ if (virDeviceInfoPCIAddressPresent(info) &&
virDomainPCIAddressReleaseSlot(priv->pciaddrs,
&info->addr.pci) < 0)
VIR_WARN("Unable to release PCI address on %s",
--
2.7.4 (Apple Git-66)
10:15 a.m.
New subject: [libvirt] [PATCH v2 06/10] qemu: remove ccwaddrs caching
On Sat, Jul 23, 2016 at 03:47:11AM +0200, Tomasz Flendrich wrote:
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 | 23 +++--------------------
3 files changed, 3 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index b23790f..66ab70e 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1804,13 +1793,7 @@ 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) &&
+ if (virDeviceInfoPCIAddressPresent(info) &&
virDomainPCIAddressReleaseSlot(priv->pciaddrs,
&info->addr.pci) < 0)
Indentation's off.
8:47 p.m.
New subject: [libvirt] [PATCH v2 07/10] add qemuDomainPCIAddrSetCreateFromDomain
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 66ab70e..add6fb5 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)
10:42 a.m.
New subject: [libvirt] [PATCH v2 07/10] add qemuDomainPCIAddrSetCreateFromDomain
On Sat, Jul 23, 2016 at 03:47:12AM +0200, Tomasz Flendrich wrote:
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(-)
The patch, technically, is not flawed. However the naming got me
confused a lot. That's because even though the function is named
AddrSetCreate, it is actually the one that calls
qemuDomainAssignDevicePCISlots(). I think we should split the calling
function a bit more and have part of it named '...Prepare...' and then
the rest. Also, even though it is still now totally broken, I'm somehow
afraid we'll be missing some of the preparation stuff at some point.
Although there is no possibility of such case currently. I'll continue
with the review tomorrow, all previous patches are fine for now.
Have a nice day,
Martin
6:01 a.m.
New subject: [libvirt] [PATCH v2 07/10] add qemuDomainPCIAddrSetCreateFromDomain
On Sat, Jul 23, 2016 at 03:47:12AM +0200, Tomasz Flendrich wrote:
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 66ab70e..add6fb5 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;
+
There actually is a call that does what we need and it's this one ^ We
just need the number for buses (for some reason), which we can easily
eliminate in that function itself. All the information that the
functions below do is only needed when creating a new domain object as
all the information is saved there. So we don't need to call these,
neither have the calculation for the number of buses outside of that
function.
NACK to this approach, just fix qemuDomainPCIAddressSetCreate() itself
and use that in the later patch.
8:47 p.m.
New subject: [libvirt] [PATCH v2 08/10] qemu_hotplug: generate pci address list on demand
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 | 6 ------
src/qemu/qemu_hotplug.c | 47 +++++++++++++++++++++++++++++++++++-------
2 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index add6fb5..2191009 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1825,12 +1825,6 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
if (!devstr)
devstr = info->alias;
- if (virDeviceInfoPCIAddressPresent(info) &&
- virDomainPCIAddressReleaseSlot(priv->pciaddrs,
- &info->addr.pci) < 0)
- VIR_WARN("Unable to release PCI address on %s",
- NULLSTR(devstr));
-
if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB &&
priv->usbaddrs &&
virDomainUSBAddressRelease(priv->usbaddrs, info) < 0)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3deeb0b..aafd937 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -314,6 +314,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
bool secobjAdded = false;
bool encobjAdded = false;
virDomainCCWAddressSetPtr ccwaddrs = NULL;
+ virDomainPCIAddressSetPtr pciaddrs = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
const char *src = virDomainDiskGetSource(disk);
virJSONValuePtr secobjProps = NULL;
@@ -345,7 +346,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
goto error;
} else if (!disk->info.type ||
disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
- if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0)
+ if (!(pciaddrs = qemuDomainPCIAddrSetCreateFromDomain(vm->def,
+ priv->qemuCaps)))
+ goto error;
+ if (virDomainPCIAddressEnsureAddr(pciaddrs, &disk->info) < 0)
goto error;
}
releaseaddr = true;
@@ -420,6 +424,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
virJSONValueFree(encobjProps);
qemuDomainSecretDiskDestroy(disk);
virDomainCCWAddressSetFree(ccwaddrs);
+ virDomainPCIAddressSetFree(pciaddrs);
VIR_FREE(devstr);
VIR_FREE(drivestr);
VIR_FREE(drivealias);
@@ -464,6 +469,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
char *devstr = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
virDomainCCWAddressSetPtr ccwaddrs = NULL;
+ virDomainPCIAddressSetPtr pciaddrs = NULL;
bool releaseaddr = false;
if (controller->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
@@ -502,7 +508,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, &controller->info)
< 0)
+ if (!(pciaddrs = qemuDomainPCIAddrSetCreateFromDomain(vm->def,
+ priv->qemuCaps)))
+ goto cleanup;
+ if (virDomainPCIAddressEnsureAddr(pciaddrs, &controller->info) < 0)
goto cleanup;
} else if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
@@ -541,6 +550,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
VIR_FREE(devstr);
virDomainCCWAddressSetFree(ccwaddrs);
+ virDomainPCIAddressSetFree(pciaddrs);
return ret;
}
@@ -910,6 +920,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
virNetDevBandwidthPtr actualBandwidth;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virDomainCCWAddressSetPtr ccwaddrs = NULL;
+ virDomainPCIAddressSetPtr pciaddrs = NULL;
size_t i;
/* preallocate new slot for device */
@@ -1056,8 +1067,12 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("virtio-s390 net device cannot be hotplugged."));
goto cleanup;
- } else if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &net->info) <
0) {
- goto cleanup;
+ } else {
+ if (!(pciaddrs = qemuDomainPCIAddrSetCreateFromDomain(vm->def,
+ priv->qemuCaps)))
+ goto cleanup;
+ if (virDomainPCIAddressEnsureAddr(pciaddrs, &net->info) < 0)
+ goto cleanup;
}
releaseaddr = true;
@@ -1224,6 +1239,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
VIR_FREE(vhostfdName);
virObjectUnref(cfg);
virDomainCCWAddressSetFree(ccwaddrs);
+ virDomainPCIAddressSetFree(pciaddrs);
return ret;
@@ -1275,6 +1291,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
bool teardownlabel = false;
int backend;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ virDomainPCIAddressSetPtr pciaddrs = NULL;
unsigned int flags = 0;
if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
@@ -1336,8 +1353,13 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1)
< 0)
goto error;
- if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, hostdev->info) < 0)
+
+ if (!(pciaddrs = qemuDomainPCIAddrSetCreateFromDomain(vm->def,
priv->qemuCaps)))
+ goto error;
+
+ if (virDomainPCIAddressEnsureAddr(pciaddrs, hostdev->info) < 0)
goto error;
+
releaseaddr = true;
if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PCI_CONFIGFD)) {
@@ -1397,6 +1419,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
cleanup:
virObjectUnref(cfg);
+ virDomainPCIAddressSetFree(pciaddrs);
return -1;
}
@@ -1569,6 +1592,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def,
{
int ret = -1;
virDomainVirtioSerialAddrSetPtr vioaddrs = NULL;
+ virDomainPCIAddressSetPtr pciaddrs = NULL;
if (!(vioaddrs = virDomainVirtioSerialAddrSetCreateFromDomain(def)))
goto cleanup;
@@ -1582,7 +1606,9 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def,
} else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) {
- if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &chr->info) < 0)
+ if (!(pciaddrs = qemuDomainPCIAddrSetCreateFromDomain(def, priv->qemuCaps)))
+ goto cleanup;
+ if (virDomainPCIAddressEnsureAddr(pciaddrs, &chr->info) < 0)
goto cleanup;
ret = 1;
@@ -1614,6 +1640,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def,
cleanup:
virDomainVirtioSerialAddrSetFree(vioaddrs);
+ virDomainPCIAddressSetFree(pciaddrs);
return ret;
}
@@ -1705,6 +1732,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
bool objAdded = false;
virJSONValuePtr props = NULL;
virDomainCCWAddressSetPtr ccwaddrs = NULL;
+ virDomainPCIAddressSetPtr pciaddrs = NULL;
const char *type;
int ret = -1;
int rv;
@@ -1732,8 +1760,10 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
- if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &rng->info) < 0)
- return -1;
+ if (!(pciaddrs = qemuDomainPCIAddrSetCreateFromDomain(vm->def,
priv->qemuCaps)))
+ goto cleanup;
+ if (virDomainPCIAddressEnsureAddr(pciaddrs, &rng->info) < 0)
+ goto cleanup;
} else if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
goto cleanup;
@@ -1791,6 +1821,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
VIR_FREE(objAlias);
VIR_FREE(devstr);
virDomainCCWAddressSetFree(ccwaddrs);
+ virDomainPCIAddressSetFree(pciaddrs);
return ret;
exit_monitor:
--
2.7.4 (Apple Git-66)
8:47 p.m.
New subject: [libvirt] [PATCH v2 09/10] qemu: remove pciaddrs caching
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 | 1 -
src/qemu/qemu_domain_address.c | 16 +++-------------
3 files changed, 3 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9045f37..d3abbaa 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1327,7 +1327,6 @@ qemuDomainObjPrivateFree(void *data)
virObjectUnref(priv->qemuCaps);
virCgroupFree(&priv->cgroup);
- virDomainPCIAddressSetFree(priv->pciaddrs);
virDomainUSBAddressSetFree(priv->usbaddrs);
virDomainChrSourceDefFree(priv->monConfig);
qemuDomainObjFreeJob(priv);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 6f349a1..35e3d09 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -184,7 +184,6 @@ struct _qemuDomainObjPrivate {
bool beingDestroyed;
char *pidfile;
- virDomainPCIAddressSetPtr pciaddrs;
virDomainUSBAddressSetPtr usbaddrs;
virQEMUCapsPtr qemuCaps;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 2191009..7d70089 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:
@@ -1791,7 +1781,7 @@ qemuDomainAssignUSBAddresses(virDomainDefPtr def,
int
qemuDomainAssignAddresses(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps,
- virDomainObjPtr obj,
+ virDomainObjPtr obj ATTRIBUTE_UNUSED,
bool newDomain)
{
if (qemuDomainAssignVirtioSerialAddresses(def) < 0)
@@ -1805,7 +1795,7 @@ qemuDomainAssignAddresses(virDomainDefPtr def,
qemuDomainAssignARMVirtioMMIOAddresses(def, qemuCaps);
- if (qemuDomainAssignPCIAddresses(def, qemuCaps, obj) < 0)
+ if (qemuDomainAssignPCIAddresses(def, qemuCaps) < 0)
return -1;
if (newDomain && qemuDomainAssignUSBAddresses(def, obj) < 0)
--
2.7.4 (Apple Git-66)
8:47 p.m.
New subject: [libvirt] [PATCH v2 10/10] Remove unused functions that release addresses
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 | 100 -----------------------------------------------
src/conf/domain_addr.h | 16 --------
src/libvirt_private.syms | 3 --
3 files changed, 119 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index e09b409..5368aa0 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -497,39 +497,6 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,
}
-int
-virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs,
- virPCIDeviceAddressPtr addr)
-{
- addrs->buses[addr->bus].slots[addr->slot] &= ~(1 <<
addr->function);
- 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,29 +748,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)
{
@@ -1239,50 +1183,6 @@ virDomainVirtioSerialAddrReserve(virDomainDefPtr def
ATTRIBUTE_UNUSED,
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;
-}
-
bool
virDomainUSBAddressPortIsValid(unsigned int *port)
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 4584e0a..87865e6 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -139,14 +139,6 @@ int virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,
virDomainDeviceInfoPtr dev)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-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 +172,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,11 +224,6 @@ virDomainVirtioSerialAddrReserve(virDomainDefPtr def,
void *data)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
-int
-virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr addrs,
- virDomainDeviceInfoPtr info)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-
bool
virDomainUSBAddressPortIsValid(unsigned int *port)
ATTRIBUTE_NONNULL(1);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6b54812..f020e2f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -87,7 +87,6 @@ virPCIDeviceAddressParseXML;
# conf/domain_addr.h
virDomainCCWAddressAllocate;
virDomainCCWAddressAssign;
-virDomainCCWAddressReleaseAddr;
virDomainCCWAddressSetCreate;
virDomainCCWAddressSetFree;
virDomainCCWAddressValidate;
@@ -97,7 +96,6 @@ virDomainPCIAddressBusSetModel;
virDomainPCIAddressEnsureAddr;
virDomainPCIAddressFlagsCompatible;
virDomainPCIAddressGetNextSlot;
-virDomainPCIAddressReleaseSlot;
virDomainPCIAddressReserveAddr;
virDomainPCIAddressReserveNextSlot;
virDomainPCIAddressReserveSlot;
@@ -122,7 +120,6 @@ virDomainUSBAddressSetFree;
virDomainVirtioSerialAddrAssign;
virDomainVirtioSerialAddrAutoAssign;
virDomainVirtioSerialAddrIsComplete;
-virDomainVirtioSerialAddrRelease;
virDomainVirtioSerialAddrReserve;
virDomainVirtioSerialAddrSetAddControllers;
virDomainVirtioSerialAddrSetCreate;
--
2.7.4 (Apple Git-66)
10:16 a.m.
New subject: [libvirt] [PATCH v2 10/10] Remove unused functions that release addresses
On 07/22/2016 09:47 PM, Tomasz Flendrich wrote:
Since address sets are now recalculated on demand instead of being
cached, there's no need for functions that release addresses.
Note there's other bits to clean up here, some functions can be made private
to this file. For example VirtioSerialAddrReserve and
VirtioSerialAddrSetCreate, and probably many others. Just an idea for an
additional patch!
Thanks,
Cole
1:40 a.m.
On Sat, Jul 23, 2016 at 03:47:05AM +0200, Tomasz Flendrich wrote:
From: Tomasz Flendrich <Darge(a)users.noreply.github.com>
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.
I know that USB addresses were added and that they are cached now. I am sure
that they can be calculated on demand too though.
If this gets accepted, I will rebase the other patches that depend on this one,
making functions less dependant on qemu, and removing the code duplication.
I went ahead and pushed patches 1-6 (that is ccw and virtio-serial
related). Looking forward to v2 of the rest.
I forgot to send this yesterday and it got stuck in my drafts O:-)
Martin
3043
days inactive
3047
days old
19 comments
3 participants
participants (3)
-
Cole Robinson -
Martin Kletzander -
Tomasz Flendrich