[libvirt] [PATCH 2/2] qemu: remove pointless address validation on hot unplug

Ján Tomko posted 2 patches 8 years, 3 months ago
[libvirt] [PATCH 2/2] qemu: remove pointless address validation on hot unplug
Posted by Ján Tomko 8 years, 3 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: remove pointless address validation on hot unplug
Posted by John Ferlan 8 years, 3 months ago

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) {
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list