We can't retrieve the isolation group of a device that's
not present in the system. However, it's very common for
VFs to be created late in the boot, so they might not be
present yet when libvirtd starts, which would cause the
guests using them to disappear.

Moreover, for other architectures and even ppc64 before
isolation groups were introduced, it's considered
perfectly fine to configure a guest to use a device
that's not yet (or no longer) available to the host,
with the obvious caveat that such a guest won't be able
to start before the device is available.

In order to be consistent, when a device's isolation
group can't be determined fall back to not isolating it
rather than erroring out or, worse, making the guest
disappear.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1484254

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
Changes from [v1]:

  * rewrite to always skip on error

[v1] https://www.redhat.com/archives/libvir-list/2017-August/msg00729.html

 src/qemu/qemu_domain_address.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 16bf0cdf9..f27d0ce8e 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -982,8 +982,6 @@ int
 qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
                                    virDomainDeviceDefPtr dev)
 {
-    int ret = -1;
-
     /* Only host devices need their isolation group to be different from
      * the default. Interfaces of type hostdev are just host devices in
      * disguise, but we don't need to handle them separately because for
@@ -1012,12 +1010,11 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
         tmp = virPCIDeviceAddressGetIOMMUGroupNum(hostAddr);
 
         if (tmp < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Can't look up isolation group for host device "
-                             "%04x:%02x:%02x.%x"),
-                           hostAddr->domain, hostAddr->bus,
-                           hostAddr->slot, hostAddr->function);
-            goto cleanup;
+            VIR_WARN("Can't look up isolation group for host device "
+                     "%04x:%02x:%02x.%x, device won't be isolated",
+                     hostAddr->domain, hostAddr->bus,
+                     hostAddr->slot, hostAddr->function);
+            goto skip;
         }
 
         /* The isolation group for a host device is its IOMMU group,
@@ -1057,12 +1054,11 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
         tmp = qemuDomainFindUnusedIsolationGroup(def);
 
         if (tmp == 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Can't obtain usable isolation group for "
-                             "interface configured to use hostdev-backed "
-                             "network '%s'"),
-                             iface->data.network.name);
-            goto cleanup;
+            VIR_WARN("Can't obtain usable isolation group for interface "
+                     "configured to use hostdev-backed network '%s', "
+                     "device won't be isolated",
+                     iface->data.network.name);
+            goto skip;
         }
 
         info->isolationGroup = tmp;
@@ -1073,10 +1069,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
     }
 
  skip:
-    ret = 0;
-
- cleanup:
-    return ret;
+    return 0;
 }
 
 
-- 
2.13.5

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

On Fri, Aug 25, 2017 at 18:15:37 +0200, Andrea Bolognani wrote:
> We can't retrieve the isolation group of a device that's
> not present in the system. However, it's very common for
> VFs to be created late in the boot, so they might not be
> present yet when libvirtd starts, which would cause the
> guests using them to disappear.
> 
> Moreover, for other architectures and even ppc64 before
> isolation groups were introduced, it's considered
> perfectly fine to configure a guest to use a device
> that's not yet (or no longer) available to the host,
> with the obvious caveat that such a guest won't be able
> to start before the device is available.
> 
> In order to be consistent, when a device's isolation
> group can't be determined fall back to not isolating it
> rather than erroring out or, worse, making the guest
> disappear.

(Didn't you set your text width for commit messages too narrow?)


> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1484254
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> Changes from [v1]:
> 
>   * rewrite to always skip on error
> 
> [v1] https://www.redhat.com/archives/libvir-list/2017-August/msg00729.html
> 
>  src/qemu/qemu_domain_address.c | 29 +++++++++++------------------
>  1 file changed, 11 insertions(+), 18 deletions(-)

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

On Mon, 2017-08-28 at 13:14 +0200, Peter Krempa wrote:
> > In order to be consistent, when a device's isolation
> > group can't be determined fall back to not isolating it
> > rather than erroring out or, worse, making the guest
> > disappear.
> 
> (Didn't you set your text width for commit messages too narrow?)

I've reflowed the commit message before pushing.

Thanks for the review :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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