[libvirt] [PATCH] qemu: Fix mdev checking for VFIO support

Erik Skultety posted 1 patch 7 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/d68ccdfeef489c2a42f8211c291743c89701b419.1491992753.git.eskultet@redhat.com
src/qemu/qemu_hostdev.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
[libvirt] [PATCH] qemu: Fix mdev checking for VFIO support
Posted by Erik Skultety 7 years ago
Commit a4a39d90 added a check that checks for VFIO support with mediated
devices. The problem is that the hostdev preparing functions behave like
a fallthrough if device of that specific type doesn't exist. However,
the check for VFIO support was independent of the existence of a mdev
device which caused the guest to fail to start with any device to be
directly assigned if VFIO was disabled/unavailable in the kernel.
The proposed change first ensures that it makes sense to check for VFIO
support in the first place, and only then performs the VFIO support check
itself.

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

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 src/qemu/qemu_hostdev.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 685bf5b59..9b5504832 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -330,11 +330,19 @@ qemuHostdevPrepareMediatedDevices(virQEMUDriverPtr driver,
                                   int nhostdevs)
 {
     virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
+    bool supportsVFIO = qemuHostdevHostSupportsPassthroughVFIO();
+    size_t i;
 
-    if (!qemuHostdevHostSupportsPassthroughVFIO()) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("host doesn't support VFIO PCI interface"));
-        return -1;
+    for (i = 0; i < nhostdevs; i++) {
+        if (hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+            hostdevs[i]->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
+            if (!supportsVFIO) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("host doesn't support VFIO PCI interface"));
+                return -1;
+            }
+            break;
+        }
     }
 
     return virHostdevPrepareMediatedDevices(hostdev_mgr, QEMU_DRIVER_NAME,
-- 
2.12.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix mdev checking for VFIO support
Posted by Daniel P. Berrange 7 years ago
On Wed, Apr 12, 2017 at 12:26:35PM +0200, Erik Skultety wrote:
> Commit a4a39d90 added a check that checks for VFIO support with mediated
> devices. The problem is that the hostdev preparing functions behave like
> a fallthrough if device of that specific type doesn't exist. However,
> the check for VFIO support was independent of the existence of a mdev
> device which caused the guest to fail to start with any device to be
> directly assigned if VFIO was disabled/unavailable in the kernel.
> The proposed change first ensures that it makes sense to check for VFIO
> support in the first place, and only then performs the VFIO support check
> itself.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441291
> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  src/qemu/qemu_hostdev.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> index 685bf5b59..9b5504832 100644
> --- a/src/qemu/qemu_hostdev.c
> +++ b/src/qemu/qemu_hostdev.c
> @@ -330,11 +330,19 @@ qemuHostdevPrepareMediatedDevices(virQEMUDriverPtr driver,
>                                    int nhostdevs)
>  {
>      virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> +    bool supportsVFIO = qemuHostdevHostSupportsPassthroughVFIO();
> +    size_t i;
>  
> -    if (!qemuHostdevHostSupportsPassthroughVFIO()) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("host doesn't support VFIO PCI interface"));
> -        return -1;
> +    for (i = 0; i < nhostdevs; i++) {
> +        if (hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
> +            hostdevs[i]->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
> +            if (!supportsVFIO) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("host doesn't support VFIO PCI interface"));

BTW, I'd suggest changing that message to

  "Mediated host device assignment requires VFIO support"

to make it clear what feature usage is triggering the error

> +                return -1;
> +            }
> +            break;
> +        }
>      }
>  
>      return virHostdevPrepareMediatedDevices(hostdev_mgr, QEMU_DRIVER_NAME,

ACK,

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix mdev checking for VFIO support
Posted by Erik Skultety 7 years ago
On Wed, Apr 12, 2017 at 11:35:14AM +0100, Daniel P. Berrange wrote:
> On Wed, Apr 12, 2017 at 12:26:35PM +0200, Erik Skultety wrote:
> > Commit a4a39d90 added a check that checks for VFIO support with mediated
> > devices. The problem is that the hostdev preparing functions behave like
> > a fallthrough if device of that specific type doesn't exist. However,
> > the check for VFIO support was independent of the existence of a mdev
> > device which caused the guest to fail to start with any device to be
> > directly assigned if VFIO was disabled/unavailable in the kernel.
> > The proposed change first ensures that it makes sense to check for VFIO
> > support in the first place, and only then performs the VFIO support check
> > itself.
> >
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441291
> >
> > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > ---
> >  src/qemu/qemu_hostdev.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> > index 685bf5b59..9b5504832 100644
> > --- a/src/qemu/qemu_hostdev.c
> > +++ b/src/qemu/qemu_hostdev.c
> > @@ -330,11 +330,19 @@ qemuHostdevPrepareMediatedDevices(virQEMUDriverPtr driver,
> >                                    int nhostdevs)
> >  {
> >      virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> > +    bool supportsVFIO = qemuHostdevHostSupportsPassthroughVFIO();
> > +    size_t i;
> >
> > -    if (!qemuHostdevHostSupportsPassthroughVFIO()) {
> > -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > -                       _("host doesn't support VFIO PCI interface"));
> > -        return -1;
> > +    for (i = 0; i < nhostdevs; i++) {
> > +        if (hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
> > +            hostdevs[i]->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
> > +            if (!supportsVFIO) {
> > +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > +                               _("host doesn't support VFIO PCI interface"));
>
> BTW, I'd suggest changing that message to
>
>   "Mediated host device assignment requires VFIO support"
>
> to make it clear what feature usage is triggering the error
>
> > +                return -1;
> > +            }
> > +            break;
> > +        }
> >      }
> >
> >      return virHostdevPrepareMediatedDevices(hostdev_mgr, QEMU_DRIVER_NAME,
>
> ACK,

Adjusted and pushed, thanks.
Erik

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