[libvirt] [PATCH 0/2] More hotplug cleanups

Remove some dead code. Ján Tomko (2): qemuIsMultiFunctionDevice: return early for non-PCI addresses qemu: remove pointless address validation on hot unplug src/qemu/qemu_hotplug.c | 56 ++++++++----------------------------------------- 1 file changed, 9 insertions(+), 47 deletions(-) -- 2.13.0

There is no point in iterating over all devices if none of them could possibly match. --- src/qemu/qemu_hotplug.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0288986d8..aebd00598 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3752,6 +3752,9 @@ static int qemuComparePCIDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, static bool qemuIsMultiFunctionDevice(virDomainDefPtr def, virDomainDeviceInfoPtr dev) { + if (dev->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + return false; + if (virDomainDeviceInfoIterate(def, qemuComparePCIDevice, dev) < 0) return true; return false; @@ -4828,8 +4831,7 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver, goto cleanup; } - if (detach->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && - qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { virReportError(VIR_ERR_OPERATION_FAILED, _("cannot hot unplug multifunction PCI device: %s"), dev->data.disk->dst); -- 2.13.0

On 10/18/2017 09:55 AM, Ján Tomko wrote:
There is no point in iterating over all devices if none of them could possibly match. --- src/qemu/qemu_hotplug.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Back in the times of using 'pci_del', unplugging a device without a PCI address was not wired up. After completely removing support for qemu without QEMU_CAPS_DEVICE, aliases are used to uniquely identify devices in all cases. Remove the pointless validation of data that was already present in the domain definition. --- src/qemu/qemu_hotplug.c | 50 +++++-------------------------------------------- 1 file changed, 5 insertions(+), 45 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index aebd00598..ac18a94d9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4610,23 +4610,6 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, goto cleanup; } - if (qemuDomainIsS390CCW(vm->def) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { - if (!virDomainDeviceAddressIsValid(&detach->info, - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("device cannot be detached without a valid CCW address")); - goto cleanup; - } - } else { - if (!virDomainDeviceAddressIsValid(&detach->info, - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("device cannot be detached without a valid PCI address")); - goto cleanup; - } - } - if (!detach->info.alias) { if (qemuAssignDeviceDiskAlias(vm->def, detach, priv->qemuCaps) < 0) goto cleanup; @@ -4884,13 +4867,6 @@ qemuDomainDetachHostPCIDevice(virQEMUDriverPtr driver, return -1; } - if (!virDomainDeviceAddressIsValid(detach->info, - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("device cannot be detached without a PCI address")); - return -1; - } - qemuDomainMarkDeviceForRemoval(vm, detach->info); qemuDomainObjEnterMonitor(driver, vm); @@ -5216,28 +5192,12 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, virDomainNetGetActualHostdev(detach)); goto cleanup; } - if (qemuDomainIsS390CCW(vm->def) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { - if (!virDomainDeviceAddressIsValid(&detach->info, - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("device cannot be detached without a CCW address")); - goto cleanup; - } - } else { - if (!virDomainDeviceAddressIsValid(&detach->info, - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("device cannot be detached without a PCI address")); - goto cleanup; - } - if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("cannot hot unplug multifunction PCI device :%s"), - dev->data.disk->dst); - goto cleanup; - } + if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device :%s"), + dev->data.disk->dst); + goto cleanup; } if (!detach->info.alias) { -- 2.13.0

On 10/18/2017 09:55 AM, Ján Tomko wrote:
Back in the times of using 'pci_del', unplugging a device without a PCI address was not wired up.
After completely removing support for qemu without QEMU_CAPS_DEVICE, aliases are used to uniquely identify devices in all cases.
Remove the pointless validation of data that was already present in the domain definition. --- src/qemu/qemu_hotplug.c | 50 +++++-------------------------------------------- 1 file changed, 5 insertions(+), 45 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index aebd00598..ac18a94d9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4610,23 +4610,6 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, goto cleanup; }
- if (qemuDomainIsS390CCW(vm->def) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { - if (!virDomainDeviceAddressIsValid(&detach->info, - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("device cannot be detached without a valid CCW address")); - goto cleanup; - } - } else { - if (!virDomainDeviceAddressIsValid(&detach->info, - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("device cannot be detached without a valid PCI address")); - goto cleanup; - } - } - if (!detach->info.alias) { if (qemuAssignDeviceDiskAlias(vm->def, detach, priv->qemuCaps) < 0) goto cleanup; @@ -4884,13 +4867,6 @@ qemuDomainDetachHostPCIDevice(virQEMUDriverPtr driver, return -1; }
- if (!virDomainDeviceAddressIsValid(detach->info, - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("device cannot be detached without a PCI address")); - return -1; - } - qemuDomainMarkDeviceForRemoval(vm, detach->info);
qemuDomainObjEnterMonitor(driver, vm); @@ -5216,28 +5192,12 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, virDomainNetGetActualHostdev(detach)); goto cleanup; } - if (qemuDomainIsS390CCW(vm->def) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { - if (!virDomainDeviceAddressIsValid(&detach->info, - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("device cannot be detached without a CCW address")); - goto cleanup; - } - } else { - if (!virDomainDeviceAddressIsValid(&detach->info, - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("device cannot be detached without a PCI address")); - goto cleanup; - }
- if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("cannot hot unplug multifunction PCI device :%s"), - dev->data.disk->dst); - goto cleanup; - } + if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device :%s"),
May as well clean up the spacing while we're at it... s/device :%s/device: %s/ Reviewed-by: John Ferlan <jferlan@redhat.com> John
+ dev->data.disk->dst); + goto cleanup; }
if (!detach->info.alias) {
participants (2)
-
John Ferlan
-
Ján Tomko