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);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
