Starting on commit 1f43393283ff, qemuDomainFillDeviceIsolationGroup()
returns 0 in all circunstances. Let's turn it to 'void' make it
clearer that the function will not fail. This also spares a
check for < 0 return in qemu_hotplug.c. The
qemuDomainFillDeviceIsolationGroupIter() callback now returns
0 at all times - which is already happening anyway.

Refer to 1f43393283ff commit message for more details on why
the function was changed to never return an error.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 src/qemu/qemu_domain_address.c | 20 ++++++++++----------
 src/qemu/qemu_domain_address.h |  2 +-
 src/qemu/qemu_hotplug.c        |  6 ++----
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 9e3bcc434d..4fb9db1e14 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1252,7 +1252,7 @@ qemuDomainFindUnusedIsolationGroup(virDomainDefPtr def)
  *
  * Return: 0 on success, <0 on failure
  * */
-int
+void
 qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
                                    virDomainDeviceDefPtr dev)
 {
@@ -1270,7 +1270,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
         /* Only PCI host devices are subject to isolation */
         if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
             hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
-            return 0;
+            return;
         }
 
         hostAddr = &hostdev->source.subsys.u.pci.addr;
@@ -1278,7 +1278,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
         /* If a non-default isolation has already been assigned to the
          * device, we can avoid looking up the information again */
         if (info->isolationGroup > 0)
-            return 0;
+            return;
 
         /* The isolation group depends on the IOMMU group assigned by the host */
         tmp = virPCIDeviceAddressGetIOMMUGroupNum(hostAddr);
@@ -1288,7 +1288,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
                      "%04x:%02x:%02x.%x, device won't be isolated",
                      hostAddr->domain, hostAddr->bus,
                      hostAddr->slot, hostAddr->function);
-            return 0;
+            return;
         }
 
         /* The isolation group for a host device is its IOMMU group,
@@ -1314,13 +1314,13 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
          * require us to isolate the guest device, so we can skip them */
         if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK ||
             virDomainNetResolveActualType(iface) != VIR_DOMAIN_NET_TYPE_HOSTDEV) {
-            return 0;
+            return;
         }
 
         /* If a non-default isolation has already been assigned to the
          * device, we can avoid looking up the information again */
         if (info->isolationGroup > 0)
-            return 0;
+            return;
 
         /* Obtain a synthetic isolation group for the device, since at this
          * point in time we don't have access to the IOMMU group of the host
@@ -1332,7 +1332,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
                      "configured to use hostdev-backed network '%s', "
                      "device won't be isolated",
                      iface->data.network.name);
-            return 0;
+            return;
         }
 
         info->isolationGroup = tmp;
@@ -1341,8 +1341,6 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
                   "hostdev-backed network '%s' is %u",
                   iface->data.network.name, info->isolationGroup);
     }
-
-    return 0;
 }
 
 
@@ -1364,7 +1362,9 @@ qemuDomainFillDeviceIsolationGroupIter(virDomainDefPtr def,
                                        virDomainDeviceInfoPtr info G_GNUC_UNUSED,
                                        void *opaque G_GNUC_UNUSED)
 {
-    return qemuDomainFillDeviceIsolationGroup(def, dev);
+    qemuDomainFillDeviceIsolationGroup(def, dev);
+
+    return 0;
 }
 
 
diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h
index bf04e6bfdb..7ef3308246 100644
--- a/src/qemu/qemu_domain_address.h
+++ b/src/qemu/qemu_domain_address.h
@@ -49,7 +49,7 @@ int qemuDomainEnsurePCIAddress(virDomainObjPtr obj,
                                virQEMUDriverPtr driver)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
 
-int qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
+void qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
                                        virDomainDeviceDefPtr dev)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 31d455505b..cb5d587940 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1598,11 +1598,9 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
     if (qemuAssignDeviceHostdevAlias(vm->def, &info->alias, -1) < 0)
         goto error;
 
-    if (qemuDomainIsPSeries(vm->def)) {
+    if (qemuDomainIsPSeries(vm->def))
         /* Isolation groups are only relevant for pSeries guests */
-        if (qemuDomainFillDeviceIsolationGroup(vm->def, &dev) < 0)
-            goto error;
-    }
+        qemuDomainFillDeviceIsolationGroup(vm->def, &dev);
 
     if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
         goto error;
-- 
2.24.1


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

On 1/9/20 3:33 PM, Daniel Henrique Barboza wrote:
> Starting on commit 1f43393283ff, qemuDomainFillDeviceIsolationGroup()
> returns 0 in all circunstances. Let's turn it to 'void' make it
> clearer that the function will not fail. This also spares a
> check for < 0 return in qemu_hotplug.c. The
> qemuDomainFillDeviceIsolationGroupIter() callback now returns
> 0 at all times - which is already happening anyway.
> 
> Refer to 1f43393283ff commit message for more details on why
> the function was changed to never return an error.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>   src/qemu/qemu_domain_address.c | 20 ++++++++++----------
>   src/qemu/qemu_domain_address.h |  2 +-
>   src/qemu/qemu_hotplug.c        |  6 ++----
>   3 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index 9e3bcc434d..4fb9db1e14 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -1252,7 +1252,7 @@ qemuDomainFindUnusedIsolationGroup(virDomainDefPtr def)
>    *
>    * Return: 0 on success, <0 on failure
>    * */

I forgot to update the return value to 'void' in the function docs
up here ^

I can fix it in a next spin together with other review corrections from
the rest of the series.



DHB


> -int
> +void
>   qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
>                                      virDomainDeviceDefPtr dev)
>   {
> @@ -1270,7 +1270,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
>           /* Only PCI host devices are subject to isolation */
>           if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
>               hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
> -            return 0;
> +            return;
>           }
>   
>           hostAddr = &hostdev->source.subsys.u.pci.addr;
> @@ -1278,7 +1278,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
>           /* If a non-default isolation has already been assigned to the
>            * device, we can avoid looking up the information again */
>           if (info->isolationGroup > 0)
> -            return 0;
> +            return;
>   
>           /* The isolation group depends on the IOMMU group assigned by the host */
>           tmp = virPCIDeviceAddressGetIOMMUGroupNum(hostAddr);
> @@ -1288,7 +1288,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
>                        "%04x:%02x:%02x.%x, device won't be isolated",
>                        hostAddr->domain, hostAddr->bus,
>                        hostAddr->slot, hostAddr->function);
> -            return 0;
> +            return;
>           }
>   
>           /* The isolation group for a host device is its IOMMU group,
> @@ -1314,13 +1314,13 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
>            * require us to isolate the guest device, so we can skip them */
>           if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK ||
>               virDomainNetResolveActualType(iface) != VIR_DOMAIN_NET_TYPE_HOSTDEV) {
> -            return 0;
> +            return;
>           }
>   
>           /* If a non-default isolation has already been assigned to the
>            * device, we can avoid looking up the information again */
>           if (info->isolationGroup > 0)
> -            return 0;
> +            return;
>   
>           /* Obtain a synthetic isolation group for the device, since at this
>            * point in time we don't have access to the IOMMU group of the host
> @@ -1332,7 +1332,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
>                        "configured to use hostdev-backed network '%s', "
>                        "device won't be isolated",
>                        iface->data.network.name);
> -            return 0;
> +            return;
>           }
>   
>           info->isolationGroup = tmp;
> @@ -1341,8 +1341,6 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
>                     "hostdev-backed network '%s' is %u",
>                     iface->data.network.name, info->isolationGroup);
>       }
> -
> -    return 0;
>   }
>   
>   
> @@ -1364,7 +1362,9 @@ qemuDomainFillDeviceIsolationGroupIter(virDomainDefPtr def,
>                                          virDomainDeviceInfoPtr info G_GNUC_UNUSED,
>                                          void *opaque G_GNUC_UNUSED)
>   {
> -    return qemuDomainFillDeviceIsolationGroup(def, dev);
> +    qemuDomainFillDeviceIsolationGroup(def, dev);
> +
> +    return 0;
>   }
>   
>   
> diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h
> index bf04e6bfdb..7ef3308246 100644
> --- a/src/qemu/qemu_domain_address.h
> +++ b/src/qemu/qemu_domain_address.h
> @@ -49,7 +49,7 @@ int qemuDomainEnsurePCIAddress(virDomainObjPtr obj,
>                                  virQEMUDriverPtr driver)
>       ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>   
> -int qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
> +void qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
>                                          virDomainDeviceDefPtr dev)
>       ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>   
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 31d455505b..cb5d587940 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1598,11 +1598,9 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
>       if (qemuAssignDeviceHostdevAlias(vm->def, &info->alias, -1) < 0)
>           goto error;
>   
> -    if (qemuDomainIsPSeries(vm->def)) {
> +    if (qemuDomainIsPSeries(vm->def))
>           /* Isolation groups are only relevant for pSeries guests */
> -        if (qemuDomainFillDeviceIsolationGroup(vm->def, &dev) < 0)
> -            goto error;
> -    }
> +        qemuDomainFillDeviceIsolationGroup(vm->def, &dev);
>   
>       if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
>           goto error;
> 

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