7:09 a.m.
On 10/17/2017 11:04 AM, Ján Tomko wrote:
Split out the common code responsible for reserving/assigning
PCI/CCW addresses for virtio disks into a helper function
for reuse by other virtio devices.
---
src/qemu/qemu_domain_address.c | 45 ++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain_address.h | 4 ++++
src/qemu/qemu_hotplug.c | 29 +++------------------------
3 files changed, 52 insertions(+), 26 deletions(-)
Not an issue - just a note from review...
diff --git a/src/qemu/qemu_domain_address.c
b/src/qemu/qemu_domain_address.c
index c4485d4ab..ebe9eb861 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -2904,3 +2904,48 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
VIR_WARN("Unable to release USB address on %s",
NULLSTR(devstr));
}
+
+
+int
+qemuDomainEnsureVirtioAddress(bool *releaseAddr,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev,
+ const char *devicename)
+{
+ virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev);
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virDomainCCWAddressSetPtr ccwaddrs = NULL;
+ virQEMUDriverPtr driver = priv->driver;
+ int ret = -1;
+
+ if (!info->type) {
+ if (qemuDomainIsS390CCW(vm->def) &&
+ virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW))
+ info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
+ else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390))
+ info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
+ } else {
+ if (!qemuCheckCCWS390AddressSupport(vm->def, *info, priv->qemuCaps,
+ devicename))
+ return -1;
+ }
+
+ if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
+ if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
+ goto cleanup;
+ if (virDomainCCWAddressAssign(info, ccwaddrs,
+ !info->addr.ccw.assigned) < 0)
+ goto cleanup;
+ } else if (!info->type ||
+ info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+ if (qemuDomainEnsurePCIAddress(vm, dev, driver) < 0)
+ goto cleanup;
+ *releaseAddr = true;
This is only setting *releaseAddr in this instance; whereas, the
previous code would also set it for info->type CCW [1]
Still looking at how @releaseaddr is used, we see that it's used to call
qemuDomainReleaseDeviceAddress which only cares about PCI and USB, so I
suppose this is fine - just different.
+ }
+
+ ret = 0;
+
+ cleanup:
+ virDomainCCWAddressSetFree(ccwaddrs);
+ return ret;
+}
diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h
index b5644fa9c..e951a4c88 100644
--- a/src/qemu/qemu_domain_address.h
+++ b/src/qemu/qemu_domain_address.h
@@ -59,6 +59,10 @@ qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def)
int qemuDomainAssignMemoryDeviceSlot(virDomainDefPtr def,
virDomainMemoryDefPtr mem);
+int qemuDomainEnsureVirtioAddress(bool *releaseAddr,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev,
+ const char *devicename);
# define __QEMU_DOMAIN_ADDRESS_H__
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1e7931a84..177c8a01d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -363,7 +363,6 @@ 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;
@@ -372,33 +371,12 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
qemuDomainSecretInfoPtr secinfo;
qemuDomainSecretInfoPtr encinfo;
- if (!disk->info.type) {
- if (qemuDomainIsS390CCW(vm->def) &&
- virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW))
- disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
- else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390))
- disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
- } else {
- if (!qemuCheckCCWS390AddressSupport(vm->def, disk->info,
priv->qemuCaps,
- disk->dst))
- goto cleanup;
- }
-
if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
goto cleanup;
- if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
- 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 ||
- disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
- if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
- goto error;
- }
- releaseaddr = true;
[1] @releaseaddr is set for both CCW and PCI here.
+ if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm,
&dev, disk->dst) < 0)
+ goto error;
+
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
goto error;
@@ -477,7 +455,6 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
virJSONValueFree(secobjProps);
virJSONValueFree(encobjProps);
qemuDomainSecretDiskDestroy(disk);
- virDomainCCWAddressSetFree(ccwaddrs);
VIR_FREE(devstr);
VIR_FREE(drivestr);
VIR_FREE(drivealias);