[libvirt] [PATCH 0/2] Abstracting device address allocation
More hypervisors may implement the possibility of specifying device addresses, so it makes sense to make QEMU-specific code more generic to be prepared for it. If this approach will be acceptable, all the other address sets will be moved from QEMU's private data to domainDef, the qemu_domain_address.c will shrink in size and the functionality will be moved to domain_addr.[hc]. Tomasz Flendrich (2): Move virtio *Free() functions from domain_addr to domain_conf Move vioserialaddrs from QEMU's private data to domainDef src/conf/domain_addr.c | 22 ---------------------- src/conf/domain_addr.h | 17 ----------------- src/conf/domain_conf.c | 23 +++++++++++++++++++++++ src/conf/domain_conf.h | 19 +++++++++++++++++++ src/libvirt_private.syms | 3 ++- src/qemu/qemu_domain.c | 1 - src/qemu/qemu_domain.h | 1 - src/qemu/qemu_domain_address.c | 20 ++++++++------------ src/qemu/qemu_hotplug.c | 9 +++++---- 9 files changed, 57 insertions(+), 58 deletions(-) -- 2.7.4 (Apple Git-66)
More hypervisors may implement the possibility of specifying device addresses, so it makes sense to make QEMU-specific code more generic to be prepared for it. The first step is to move data from obj->privateData to domainDef. Two functions and a few struct definitions have to be in domain_addr because virDomainDefFree() has to execute them when freeing the domainDef structure. --- src/conf/domain_addr.c | 22 ---------------------- src/conf/domain_addr.h | 17 ----------------- src/conf/domain_conf.c | 21 +++++++++++++++++++++ src/conf/domain_conf.h | 17 +++++++++++++++++ src/libvirt_private.syms | 3 ++- 5 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 794270d..e482328 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -856,15 +856,6 @@ virDomainVirtioSerialAddrSetCreate(void) return ret; } -static void -virDomainVirtioSerialControllerFree(virDomainVirtioSerialControllerPtr cont) -{ - if (cont) { - virBitmapFree(cont->ports); - VIR_FREE(cont); - } -} - static ssize_t virDomainVirtioSerialAddrPlaceController(virDomainVirtioSerialAddrSetPtr addrs, virDomainVirtioSerialControllerPtr cont) @@ -962,19 +953,6 @@ virDomainVirtioSerialAddrSetAddControllers(virDomainVirtioSerialAddrSetPtr addrs return 0; } - -void -virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs) -{ - size_t i; - if (addrs) { - for (i = 0; i < addrs->ncontrollers; i++) - virDomainVirtioSerialControllerFree(addrs->controllers[i]); - VIR_FREE(addrs->controllers); - VIR_FREE(addrs); - } -} - static int virDomainVirtioSerialAddrSetAutoaddController(virDomainDefPtr def, virDomainVirtioSerialAddrSetPtr addrs, diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index f3eda89..0fb5f85 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -185,29 +185,12 @@ int virDomainCCWAddressReleaseAddr(virDomainCCWAddressSetPtr addrs, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); virDomainCCWAddressSetPtr virDomainCCWAddressSetCreate(void); -struct _virDomainVirtioSerialController { - unsigned int idx; - virBitmapPtr ports; -}; - -typedef struct _virDomainVirtioSerialController virDomainVirtioSerialController; -typedef virDomainVirtioSerialController *virDomainVirtioSerialControllerPtr; - -struct _virDomainVirtioSerialAddrSet { - virDomainVirtioSerialControllerPtr *controllers; - size_t ncontrollers; -}; -typedef struct _virDomainVirtioSerialAddrSet virDomainVirtioSerialAddrSet; -typedef virDomainVirtioSerialAddrSet *virDomainVirtioSerialAddrSetPtr; - virDomainVirtioSerialAddrSetPtr virDomainVirtioSerialAddrSetCreate(void); int virDomainVirtioSerialAddrSetAddControllers(virDomainVirtioSerialAddrSetPtr addrs, virDomainDefPtr def) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -void -virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs); bool virDomainVirtioSerialAddrIsComplete(virDomainDeviceInfoPtr info); int diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4196537..6900eb9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2514,6 +2514,27 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader) VIR_FREE(loader); } +void +virDomainVirtioSerialControllerFree(virDomainVirtioSerialControllerPtr cont) +{ + if (cont) { + virBitmapFree(cont->ports); + VIR_FREE(cont); + } +} + +void +virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs) +{ + size_t i; + if (addrs) { + for (i = 0; i < addrs->ncontrollers; i++) + virDomainVirtioSerialControllerFree(addrs->controllers[i]); + VIR_FREE(addrs->controllers); + VIR_FREE(addrs); + } +} + void virDomainDefFree(virDomainDefPtr def) { size_t i; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6e81e52..7dcecdc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2089,6 +2089,21 @@ struct _virDomainMemtune { unsigned long long swap_hard_limit; /* in kibibytes, limit at off_t bytes */ }; +struct _virDomainVirtioSerialController { + unsigned int idx; + virBitmapPtr ports; +}; + +typedef struct _virDomainVirtioSerialController virDomainVirtioSerialController; +typedef virDomainVirtioSerialController *virDomainVirtioSerialControllerPtr; + +struct _virDomainVirtioSerialAddrSet { + virDomainVirtioSerialControllerPtr *controllers; + size_t ncontrollers; +}; +typedef struct _virDomainVirtioSerialAddrSet virDomainVirtioSerialAddrSet; +typedef virDomainVirtioSerialAddrSet *virDomainVirtioSerialAddrSetPtr; + typedef struct _virDomainPowerManagement virDomainPowerManagement; typedef virDomainPowerManagement *virDomainPowerManagementPtr; @@ -2526,6 +2541,8 @@ void virDomainDefClearPCIAddresses(virDomainDefPtr def); void virDomainDefClearCCWAddresses(virDomainDefPtr def); void virDomainDefClearDeviceAliases(virDomainDefPtr def); void virDomainTPMDefFree(virDomainTPMDefPtr def); +void virDomainVirtioSerialControllerFree(virDomainVirtioSerialControllerPtr cont); +void virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs); typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def, virDomainDeviceDefPtr dev, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f62dd70..d29eb07 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -116,7 +116,6 @@ virDomainVirtioSerialAddrRelease; virDomainVirtioSerialAddrReserve; virDomainVirtioSerialAddrSetAddControllers; virDomainVirtioSerialAddrSetCreate; -virDomainVirtioSerialAddrSetFree; # conf/domain_audit.h @@ -479,6 +478,8 @@ virDomainVideoDefaultType; virDomainVideoDefFree; virDomainVideoTypeFromString; virDomainVideoTypeToString; +virDomainVirtioSerialAddrSetFree; +virDomainVirtioSerialControllerFree; virDomainVirtTypeFromString; virDomainVirtTypeToString; virDomainWatchdogActionTypeFromString; -- 2.7.4 (Apple Git-66)
The end goal is to move all of the address types outside of QEMU's private data and make the functions from qemu_domain_address.c hypervisor-agnostic. --- src/conf/domain_conf.c | 2 ++ src/conf/domain_conf.h | 2 ++ src/qemu/qemu_domain.c | 1 - src/qemu/qemu_domain.h | 1 - src/qemu/qemu_domain_address.c | 20 ++++++++------------ src/qemu/qemu_hotplug.c | 9 +++++---- 6 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6900eb9..13230c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2705,6 +2705,8 @@ void virDomainDefFree(virDomainDefPtr def) xmlFreeNode(def->metadata); + virDomainVirtioSerialAddrSetFree(def->vioserialaddrs); + VIR_FREE(def); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7dcecdc..2bac992 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2270,6 +2270,8 @@ struct _virDomainDef { virDomainKeyWrapDefPtr keywrap; + virDomainVirtioSerialAddrSetPtr vioserialaddrs; + /* Application-specific custom metadata */ xmlNodePtr metadata; }; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 595ad64..f6ccbc0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1249,7 +1249,6 @@ qemuDomainObjPrivateFree(void *data) virCgroupFree(&priv->cgroup); virDomainPCIAddressSetFree(priv->pciaddrs); virDomainCCWAddressSetFree(priv->ccwaddrs); - virDomainVirtioSerialAddrSetFree(priv->vioserialaddrs); virDomainChrSourceDefFree(priv->monConfig); qemuDomainObjFreeJob(priv); VIR_FREE(priv->vcpupids); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2443e97..58221bb 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -189,7 +189,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 883264a..1633c9b 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -107,13 +107,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 = virDomainVirtioSerialAddrSetCreate())) goto cleanup; @@ -145,13 +143,11 @@ 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; - } + /* we persist the addresses */ + virDomainVirtioSerialAddrSetFree(def->vioserialaddrs); + def->vioserialaddrs = addrs; + addrs = NULL; + ret = 0; cleanup: @@ -1630,7 +1626,7 @@ qemuDomainAssignAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainObjPtr obj) { - if (qemuDomainAssignVirtioSerialAddresses(def, obj) < 0) + if (qemuDomainAssignVirtioSerialAddresses(def) < 0) return -1; if (qemuDomainAssignSpaprVIOAddresses(def, qemuCaps) < 0) @@ -1670,7 +1666,7 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, 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) + virDomainVirtioSerialAddrRelease(vm->def->vioserialaddrs, info) < 0) VIR_WARN("Unable to release virtio-serial address on %s", NULLSTR(devstr)); } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9aca853..696ec9e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1457,12 +1457,13 @@ qemuDomainChrRemove(virDomainDefPtr vmdef, } static int -qemuDomainAttachChrDeviceAssignAddr(qemuDomainObjPrivatePtr priv, +qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr vmdef, + qemuDomainObjPrivatePtr priv, virDomainChrDefPtr chr) { 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, vmdef->vioserialaddrs, &chr->info, true) < 0) return -1; return 1; @@ -1475,7 +1476,7 @@ 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, vmdef->vioserialaddrs, &chr->info, false) < 0) return -1; return 1; @@ -1509,7 +1510,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0) goto cleanup; - if ((rc = qemuDomainAttachChrDeviceAssignAddr(priv, chr)) < 0) + if ((rc = qemuDomainAttachChrDeviceAssignAddr(vmdef, priv, chr)) < 0) goto cleanup; if (rc == 1) need_release = true; -- 2.7.4 (Apple Git-66)
On Sun, Jun 19, 2016 at 07:03:59PM +0200, Tomasz Flendrich wrote:
More hypervisors may implement the possibility of specifying device addresses, so it makes sense to make QEMU-specific code more generic to be prepared for it.
If this approach will be acceptable, all the other address sets will be moved from QEMU's private data to domainDef, the qemu_domain_address.c will shrink in size and the functionality will be moved to domain_addr.[hc].
This is not bad, but the function that does the real job (qemuDomainAssignVirtioSerialAddresses()) was left in qemu driver. That is the key part that could help in the future. Let's postpone this until we reach a consensus on the second address allocation thread, so we know which way to take.
participants (2)
-
Martin Kletzander -
Tomasz Flendrich