handle missing SR-IOV VF hostdevs during running domains

[PATCH RESEND 08/20] security_dac.c: modernize hostdev label set/restore functions

Posted by Daniel Henrique Barboza 5 years ago

Use g_auto* cleanup to avoid free() calls.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 src/security/security_dac.c | 49 +++++++++++--------------------------
 1 file changed, 14 insertions(+), 35 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index b5e56feaa8..0085982bb1 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1251,7 +1251,7 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
 
     switch ((virDomainHostdevSubsysType)dev->source.subsys.type) {
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
-        virUSBDevicePtr usb;
+        g_autoptr(virUSBDevice) usb = NULL;
 
         if (dev->missing)
             return 0;
@@ -1262,41 +1262,35 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
         ret = virUSBDeviceFileIterate(usb,
                                       virSecurityDACSetUSBLabel,
                                       &cbdata);
-        virUSBDeviceFree(usb);
         break;
     }
 
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
-        virPCIDevicePtr pci =
-            virPCIDeviceNew(&pcisrc->addr);
+        g_autoptr(virPCIDevice) pci = virPCIDeviceNew(&pcisrc->addr);
 
         if (!pci)
             return -1;
 
         if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
-            char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci);
+            g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci);
 
-            if (!vfioGroupDev) {
-                virPCIDeviceFree(pci);
+            if (!vfioGroupDev)
                 return -1;
-            }
+
             ret = virSecurityDACSetHostdevLabelHelper(vfioGroupDev,
                                                       false,
                                                       &cbdata);
-            VIR_FREE(vfioGroupDev);
         } else {
             ret = virPCIDeviceFileIterate(pci,
                                           virSecurityDACSetPCILabel,
                                           &cbdata);
         }
-
-        virPCIDeviceFree(pci);
         break;
     }
 
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: {
         virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
-        virSCSIDevicePtr scsi =
+        g_autoptr(virSCSIDevice) scsi =
             virSCSIDeviceNew(NULL,
                              scsihostsrc->adapter, scsihostsrc->bus,
                              scsihostsrc->target, scsihostsrc->unit,
@@ -1308,13 +1302,11 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
         ret = virSCSIDeviceFileIterate(scsi,
                                        virSecurityDACSetSCSILabel,
                                        &cbdata);
-        virSCSIDeviceFree(scsi);
-
         break;
     }
 
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: {
-        virSCSIVHostDevicePtr host = virSCSIVHostDeviceNew(hostsrc->wwpn);
+        g_autoptr(virSCSIVHostDevice) host = virSCSIVHostDeviceNew(hostsrc->wwpn);
 
         if (!host)
             return -1;
@@ -1322,19 +1314,16 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
         ret = virSCSIVHostDeviceFileIterate(host,
                                             virSecurityDACSetHostLabel,
                                             &cbdata);
-        virSCSIVHostDeviceFree(host);
         break;
     }
 
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
-        char *vfiodev = NULL;
+        g_autofree char *vfiodev = NULL;
 
         if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
             return -1;
 
         ret = virSecurityDACSetHostdevLabelHelper(vfiodev, true, &cbdata);
-
-        VIR_FREE(vfiodev);
         break;
     }
 
@@ -1420,7 +1409,7 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
 
     switch ((virDomainHostdevSubsysType)dev->source.subsys.type) {
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
-        virUSBDevicePtr usb;
+        g_autoptr(virUSBDevice) usb = NULL;
 
         if (dev->missing)
             return 0;
@@ -1429,20 +1418,17 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
             return -1;
 
         ret = virUSBDeviceFileIterate(usb, virSecurityDACRestoreUSBLabel, mgr);
-        virUSBDeviceFree(usb);
-
         break;
     }
 
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
-        virPCIDevicePtr pci =
-            virPCIDeviceNew(&pcisrc->addr);
+        g_autoptr(virPCIDevice) pci = virPCIDeviceNew(&pcisrc->addr);
 
         if (!pci)
             return -1;
 
         if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
-            char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci);
+            g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci);
 
             if (!vfioGroupDev) {
                 virPCIDeviceFree(pci);
@@ -1450,17 +1436,15 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
             }
             ret = virSecurityDACRestoreFileLabelInternal(mgr, NULL,
                                                          vfioGroupDev, false);
-            VIR_FREE(vfioGroupDev);
         } else {
             ret = virPCIDeviceFileIterate(pci, virSecurityDACRestorePCILabel, mgr);
         }
-        virPCIDeviceFree(pci);
         break;
     }
 
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: {
         virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
-        virSCSIDevicePtr scsi =
+        g_autoptr(virSCSIDevice) scsi =
             virSCSIDeviceNew(NULL,
                              scsihostsrc->adapter, scsihostsrc->bus,
                              scsihostsrc->target, scsihostsrc->unit,
@@ -1470,13 +1454,11 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
             return -1;
 
         ret = virSCSIDeviceFileIterate(scsi, virSecurityDACRestoreSCSILabel, mgr);
-        virSCSIDeviceFree(scsi);
-
         break;
     }
 
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: {
-        virSCSIVHostDevicePtr host = virSCSIVHostDeviceNew(hostsrc->wwpn);
+        g_autoptr(virSCSIVHostDevice) host = virSCSIVHostDeviceNew(hostsrc->wwpn);
 
         if (!host)
             return -1;
@@ -1484,19 +1466,16 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
         ret = virSCSIVHostDeviceFileIterate(host,
                                             virSecurityDACRestoreHostLabel,
                                             mgr);
-        virSCSIVHostDeviceFree(host);
-
         break;
     }
 
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
-        char *vfiodev = NULL;
+        g_autofree char *vfiodev = NULL;
 
         if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
             return -1;
 
         ret = virSecurityDACRestoreFileLabel(mgr, vfiodev);
-        VIR_FREE(vfiodev);
         break;
     }
 
-- 
2.26.2

Re: [PATCH RESEND 08/20] security_dac.c: modernize hostdev label set/restore functions

Posted by Laine Stump 5 years ago

Again, don't use "modernize". Just "convert to use g_auto*" or something 
like that.


On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:
> Use g_auto* cleanup to avoid free() calls.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>   src/security/security_dac.c | 49 +++++++++++--------------------------
>   1 file changed, 14 insertions(+), 35 deletions(-)
>
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index b5e56feaa8..0085982bb1 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -1251,7 +1251,7 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
>   
>       switch ((virDomainHostdevSubsysType)dev->source.subsys.type) {
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
> -        virUSBDevicePtr usb;
> +        g_autoptr(virUSBDevice) usb = NULL;
>   
>           if (dev->missing)
>               return 0;
> @@ -1262,41 +1262,35 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
>           ret = virUSBDeviceFileIterate(usb,
>                                         virSecurityDACSetUSBLabel,
>                                         &cbdata);
> -        virUSBDeviceFree(usb);
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
> -        virPCIDevicePtr pci =
> -            virPCIDeviceNew(&pcisrc->addr);
> +        g_autoptr(virPCIDevice) pci = virPCIDeviceNew(&pcisrc->addr);
>   
>           if (!pci)
>               return -1;
>   
>           if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
> -            char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci);
> +            g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci);
>   
> -            if (!vfioGroupDev) {
> -                virPCIDeviceFree(pci);
> +            if (!vfioGroupDev)
>                   return -1;
> -            }
> +
>               ret = virSecurityDACSetHostdevLabelHelper(vfioGroupDev,
>                                                         false,
>                                                         &cbdata);
> -            VIR_FREE(vfioGroupDev);
>           } else {
>               ret = virPCIDeviceFileIterate(pci,
>                                             virSecurityDACSetPCILabel,
>                                             &cbdata);
>           }
> -
> -        virPCIDeviceFree(pci);
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: {
>           virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
> -        virSCSIDevicePtr scsi =
> +        g_autoptr(virSCSIDevice) scsi =
>               virSCSIDeviceNew(NULL,
>                                scsihostsrc->adapter, scsihostsrc->bus,
>                                scsihostsrc->target, scsihostsrc->unit,
> @@ -1308,13 +1302,11 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
>           ret = virSCSIDeviceFileIterate(scsi,
>                                          virSecurityDACSetSCSILabel,
>                                          &cbdata);
> -        virSCSIDeviceFree(scsi);
> -
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: {
> -        virSCSIVHostDevicePtr host = virSCSIVHostDeviceNew(hostsrc->wwpn);
> +        g_autoptr(virSCSIVHostDevice) host = virSCSIVHostDeviceNew(hostsrc->wwpn);
>   
>           if (!host)
>               return -1;
> @@ -1322,19 +1314,16 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
>           ret = virSCSIVHostDeviceFileIterate(host,
>                                               virSecurityDACSetHostLabel,
>                                               &cbdata);
> -        virSCSIVHostDeviceFree(host);
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
> -        char *vfiodev = NULL;
> +        g_autofree char *vfiodev = NULL;
>   
>           if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
>               return -1;
>   
>           ret = virSecurityDACSetHostdevLabelHelper(vfiodev, true, &cbdata);
> -
> -        VIR_FREE(vfiodev);
>           break;
>       }
>   
> @@ -1420,7 +1409,7 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
>   
>       switch ((virDomainHostdevSubsysType)dev->source.subsys.type) {
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
> -        virUSBDevicePtr usb;
> +        g_autoptr(virUSBDevice) usb = NULL;
>   
>           if (dev->missing)
>               return 0;
> @@ -1429,20 +1418,17 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
>               return -1;
>   
>           ret = virUSBDeviceFileIterate(usb, virSecurityDACRestoreUSBLabel, mgr);
> -        virUSBDeviceFree(usb);
> -
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
> -        virPCIDevicePtr pci =
> -            virPCIDeviceNew(&pcisrc->addr);
> +        g_autoptr(virPCIDevice) pci = virPCIDeviceNew(&pcisrc->addr);
>   
>           if (!pci)
>               return -1;
>   
>           if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
> -            char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci);
> +            g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci);
>   
>               if (!vfioGroupDev) {
>                   virPCIDeviceFree(pci);
> @@ -1450,17 +1436,15 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
>               }
>               ret = virSecurityDACRestoreFileLabelInternal(mgr, NULL,
>                                                            vfioGroupDev, false);
> -            VIR_FREE(vfioGroupDev);
>           } else {
>               ret = virPCIDeviceFileIterate(pci, virSecurityDACRestorePCILabel, mgr);
>           }
> -        virPCIDeviceFree(pci);
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: {
>           virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
> -        virSCSIDevicePtr scsi =
> +        g_autoptr(virSCSIDevice) scsi =
>               virSCSIDeviceNew(NULL,
>                                scsihostsrc->adapter, scsihostsrc->bus,
>                                scsihostsrc->target, scsihostsrc->unit,
> @@ -1470,13 +1454,11 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
>               return -1;
>   
>           ret = virSCSIDeviceFileIterate(scsi, virSecurityDACRestoreSCSILabel, mgr);
> -        virSCSIDeviceFree(scsi);
> -
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: {
> -        virSCSIVHostDevicePtr host = virSCSIVHostDeviceNew(hostsrc->wwpn);
> +        g_autoptr(virSCSIVHostDevice) host = virSCSIVHostDeviceNew(hostsrc->wwpn);
>   
>           if (!host)
>               return -1;
> @@ -1484,19 +1466,16 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
>           ret = virSCSIVHostDeviceFileIterate(host,
>                                               virSecurityDACRestoreHostLabel,
>                                               mgr);
> -        virSCSIVHostDeviceFree(host);
> -
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
> -        char *vfiodev = NULL;
> +        g_autofree char *vfiodev = NULL;
>   
>           if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
>               return -1;
>   
>           ret = virSecurityDACRestoreFileLabel(mgr, vfiodev);
> -        VIR_FREE(vfiodev);
>           break;
>       }
>