[libvirt] [PATCH v4 09/14] hostdev: Maintain a driver list of active mediated devices

Erik Skultety posted 14 patches 8 years, 3 months ago
[libvirt] [PATCH v4 09/14] hostdev: Maintain a driver list of active mediated devices
Posted by Erik Skultety 8 years, 3 months ago
Keep track of the assigned mediated devices the same way we do it for
the rest of hostdevs. Methods like 'Prepare', 'Update', and 'ReAttach'
are introduced by this patch.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 src/libvirt_private.syms |   3 +
 src/qemu/qemu_hostdev.c  |  56 ++++++++++++++++
 src/qemu/qemu_hostdev.h  |  10 +++
 src/util/virhostdev.c    | 165 ++++++++++++++++++++++++++++++++++++++++++++++-
 src/util/virhostdev.h    |  23 +++++++
 5 files changed, 256 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c51b295d30..8f3b9697e4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1730,16 +1730,19 @@ virHostdevPCINodeDeviceDetach;
 virHostdevPCINodeDeviceReAttach;
 virHostdevPCINodeDeviceReset;
 virHostdevPrepareDomainDevices;
+virHostdevPrepareMediatedDevices;
 virHostdevPreparePCIDevices;
 virHostdevPrepareSCSIDevices;
 virHostdevPrepareSCSIVHostDevices;
 virHostdevPrepareUSBDevices;
 virHostdevReAttachDomainDevices;
+virHostdevReAttachMediatedDevices;
 virHostdevReAttachPCIDevices;
 virHostdevReAttachSCSIDevices;
 virHostdevReAttachSCSIVHostDevices;
 virHostdevReAttachUSBDevices;
 virHostdevUpdateActiveDomainDevices;
+virHostdevUpdateActiveMediatedDevices;
 virHostdevUpdateActivePCIDevices;
 virHostdevUpdateActiveSCSIDevices;
 virHostdevUpdateActiveUSBDevices;
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 7cd49e4aa5..685bf5b59f 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -83,6 +83,22 @@ qemuHostdevUpdateActiveSCSIDevices(virQEMUDriverPtr driver,
                                              QEMU_DRIVER_NAME, def->name);
 }
 
+
+int
+qemuHostdevUpdateActiveMediatedDevices(virQEMUDriverPtr driver,
+                                       virDomainDefPtr def)
+{
+    virHostdevManagerPtr mgr = driver->hostdevMgr;
+
+    if (!def->nhostdevs)
+        return 0;
+
+    return virHostdevUpdateActiveMediatedDevices(mgr, def->hostdevs,
+                                                 def->nhostdevs,
+                                                 QEMU_DRIVER_NAME, def->name);
+}
+
+
 int
 qemuHostdevUpdateActiveDomainDevices(virQEMUDriverPtr driver,
                                      virDomainDefPtr def)
@@ -99,6 +115,9 @@ qemuHostdevUpdateActiveDomainDevices(virQEMUDriverPtr driver,
     if (qemuHostdevUpdateActiveSCSIDevices(driver, def) < 0)
         return -1;
 
+    if (qemuHostdevUpdateActiveMediatedDevices(driver, def) < 0)
+        return -1;
+
     return 0;
 }
 
@@ -305,6 +324,24 @@ qemuHostdevPrepareSCSIVHostDevices(virQEMUDriverPtr driver,
 }
 
 int
+qemuHostdevPrepareMediatedDevices(virQEMUDriverPtr driver,
+                                  const char *name,
+                                  virDomainHostdevDefPtr *hostdevs,
+                                  int nhostdevs)
+{
+    virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
+
+    if (!qemuHostdevHostSupportsPassthroughVFIO()) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("host doesn't support VFIO PCI interface"));
+        return -1;
+    }
+
+    return virHostdevPrepareMediatedDevices(hostdev_mgr, QEMU_DRIVER_NAME,
+                                            name, hostdevs, nhostdevs);
+}
+
+int
 qemuHostdevPrepareDomainDevices(virQEMUDriverPtr driver,
                                 virDomainDefPtr def,
                                 virQEMUCapsPtr qemuCaps,
@@ -330,6 +367,10 @@ qemuHostdevPrepareDomainDevices(virQEMUDriverPtr driver,
                                            def->hostdevs, def->nhostdevs) < 0)
         return -1;
 
+    if (qemuHostdevPrepareMediatedDevices(driver, def->name,
+                                          def->hostdevs, def->nhostdevs) < 0)
+        return -1;
+
     return 0;
 }
 
@@ -397,6 +438,18 @@ qemuHostdevReAttachSCSIVHostDevices(virQEMUDriverPtr driver,
 }
 
 void
+qemuHostdevReAttachMediatedDevices(virQEMUDriverPtr driver,
+                                   const char *name,
+                                   virDomainHostdevDefPtr *hostdevs,
+                                   int nhostdevs)
+{
+    virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
+
+    virHostdevReAttachMediatedDevices(hostdev_mgr, QEMU_DRIVER_NAME,
+                                      name, hostdevs, nhostdevs);
+}
+
+void
 qemuHostdevReAttachDomainDevices(virQEMUDriverPtr driver,
                                  virDomainDefPtr def)
 {
@@ -414,4 +467,7 @@ qemuHostdevReAttachDomainDevices(virQEMUDriverPtr driver,
 
     qemuHostdevReAttachSCSIVHostDevices(driver, def->name, def->hostdevs,
                                         def->nhostdevs);
+
+    qemuHostdevReAttachMediatedDevices(driver, def->name, def->hostdevs,
+                                       def->nhostdevs);
 }
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index 74a7d4f34e..9a7c7f143c 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -30,6 +30,8 @@
 bool qemuHostdevHostSupportsPassthroughLegacy(void);
 bool qemuHostdevHostSupportsPassthroughVFIO(void);
 
+int qemuHostdevUpdateActiveMediatedDevices(virQEMUDriverPtr driver,
+                                           virDomainDefPtr def);
 int qemuHostdevUpdateActivePCIDevices(virQEMUDriverPtr driver,
                                       virDomainDefPtr def);
 int qemuHostdevUpdateActiveUSBDevices(virQEMUDriverPtr driver,
@@ -59,6 +61,10 @@ int qemuHostdevPrepareSCSIVHostDevices(virQEMUDriverPtr driver,
                                        const char *name,
                                        virDomainHostdevDefPtr *hostdevs,
                                        int nhostdevs);
+int qemuHostdevPrepareMediatedDevices(virQEMUDriverPtr driver,
+                                      const char *name,
+                                      virDomainHostdevDefPtr *hostdevs,
+                                      int nhostdevs);
 int qemuHostdevPrepareDomainDevices(virQEMUDriverPtr driver,
                                     virDomainDefPtr def,
                                     virQEMUCapsPtr qemuCaps,
@@ -80,6 +86,10 @@ void qemuHostdevReAttachSCSIVHostDevices(virQEMUDriverPtr driver,
                                          const char *name,
                                          virDomainHostdevDefPtr *hostdevs,
                                          int nhostdevs);
+void qemuHostdevReAttachMediatedDevices(virQEMUDriverPtr driver,
+                                        const char *name,
+                                        virDomainHostdevDefPtr *hostdevs,
+                                        int nhostdevs);
 void qemuHostdevReAttachDomainDevices(virQEMUDriverPtr driver,
                                       virDomainDefPtr def);
 
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 86ca8e0473..0c337e6116 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1,6 +1,6 @@
 /* virhostdev.c: hostdev management
  *
- * Copyright (C) 2006-2007, 2009-2016 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2017 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  * Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
  *
@@ -147,6 +147,7 @@ virHostdevManagerDispose(void *obj)
     virObjectUnref(hostdevMgr->activeUSBHostdevs);
     virObjectUnref(hostdevMgr->activeSCSIHostdevs);
     virObjectUnref(hostdevMgr->activeSCSIVHostHostdevs);
+    virObjectUnref(hostdevMgr->activeMediatedHostdevs);
     VIR_FREE(hostdevMgr->stateDir);
 }
 
@@ -174,6 +175,9 @@ virHostdevManagerNew(void)
     if (!(hostdevMgr->activeSCSIVHostHostdevs = virSCSIVHostDeviceListNew()))
         goto error;
 
+    if (!(hostdevMgr->activeMediatedHostdevs = virMediatedDeviceListNew()))
+        goto error;
+
     if (privileged) {
         if (VIR_STRDUP(hostdevMgr->stateDir, HOSTDEV_STATE_DIR) < 0)
             goto error;
@@ -1147,6 +1151,50 @@ virHostdevUpdateActiveSCSIDevices(virHostdevManagerPtr mgr,
     return ret;
 }
 
+
+int
+virHostdevUpdateActiveMediatedDevices(virHostdevManagerPtr mgr,
+                                      virDomainHostdevDefPtr *hostdevs,
+                                      int nhostdevs,
+                                      const char *drv_name,
+                                      const char *dom_name)
+{
+    int ret = -1;
+    size_t i;
+    virMediatedDevicePtr mdev = NULL;
+
+    if (nhostdevs == 0)
+        return 0;
+
+    virObjectLock(mgr->activeMediatedHostdevs);
+    for (i = 0; i < nhostdevs; i++) {
+        virDomainHostdevDefPtr hostdev = hostdevs[i];
+        virDomainHostdevSubsysMediatedDevPtr mdevsrc;
+
+        mdevsrc = &hostdev->source.subsys.u.mdev;
+
+        if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
+            hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
+            continue;
+        }
+
+        if (!(mdev = virMediatedDeviceNew(mdevsrc->uuidstr, mdevsrc->model)))
+            goto cleanup;
+
+        virMediatedDeviceSetUsedBy(mdev, drv_name, dom_name);
+
+        if (virMediatedDeviceListAdd(mgr->activeMediatedHostdevs, mdev) < 0)
+            goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virMediatedDeviceFree(mdev);
+    virObjectUnlock(mgr->activeMediatedHostdevs);
+    return ret;
+}
+
+
 static int
 virHostdevMarkUSBDevices(virHostdevManagerPtr mgr,
                          const char *drv_name,
@@ -1595,6 +1643,70 @@ virHostdevPrepareSCSIVHostDevices(virHostdevManagerPtr mgr,
     return -1;
 }
 
+
+int
+virHostdevPrepareMediatedDevices(virHostdevManagerPtr mgr,
+                                 const char *drv_name,
+                                 const char *dom_name,
+                                 virDomainHostdevDefPtr *hostdevs,
+                                 int nhostdevs)
+{
+    size_t i;
+    int ret = -1;
+    virMediatedDeviceListPtr list;
+
+    if (!nhostdevs)
+        return 0;
+
+    /* To prevent situation where mediated device is assigned to multiple
+     * domains we maintain a driver list of currently assigned mediated devices.
+     * A device is appended to the driver list after a series of preparations.
+     */
+    if (!(list = virMediatedDeviceListNew()))
+        goto cleanup;
+
+    /* Loop 1: Build a temporary list of ALL mediated devices. */
+    for (i = 0; i < nhostdevs; i++) {
+        virDomainHostdevDefPtr hostdev = hostdevs[i];
+        virDomainHostdevSubsysMediatedDevPtr src = &hostdev->source.subsys.u.mdev;
+        virMediatedDevicePtr mdev;
+
+        if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
+            continue;
+        if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV)
+            continue;
+
+        if (!(mdev = virMediatedDeviceNew(src->uuidstr, src->model)))
+            goto cleanup;
+
+        if (virMediatedDeviceListAdd(list, mdev) < 0) {
+            virMediatedDeviceFree(mdev);
+            goto cleanup;
+        }
+    }
+
+    /* Mark the devices in the list as used by @drv_name-@dom_name and copy the
+     * references to the driver list
+     */
+    if (virMediatedDeviceListMarkDevices(mgr->activeMediatedHostdevs,
+                                         list, drv_name, dom_name) < 0)
+        goto cleanup;
+
+    /* Loop 2: Temporary list was successfully merged with
+     * driver list, so steal all items to avoid freeing them
+     * in cleanup label.
+     */
+    while (virMediatedDeviceListCount(list) > 0) {
+        virMediatedDevicePtr tmp = virMediatedDeviceListGet(list, 0);
+        virMediatedDeviceListSteal(list, tmp);
+    }
+
+    ret = 0;
+ cleanup:
+    virObjectUnref(list);
+    return ret;
+}
+
 void
 virHostdevReAttachUSBDevices(virHostdevManagerPtr mgr,
                              const char *drv_name,
@@ -1789,6 +1901,57 @@ virHostdevReAttachSCSIVHostDevices(virHostdevManagerPtr mgr,
     virObjectUnlock(mgr->activeSCSIVHostHostdevs);
 }
 
+void
+virHostdevReAttachMediatedDevices(virHostdevManagerPtr mgr,
+                                  const char *drv_name,
+                                  const char *dom_name,
+                                  virDomainHostdevDefPtr *hostdevs,
+                                  int nhostdevs)
+{
+    const char *used_by_drvname = NULL;
+    const char *used_by_domname = NULL;
+    size_t i;
+
+    if (nhostdevs == 0)
+        return;
+
+    virObjectLock(mgr->activeMediatedHostdevs);
+    for (i = 0; i < nhostdevs; i++) {
+        virMediatedDevicePtr mdev, tmp;
+        virDomainHostdevSubsysMediatedDevPtr mdevsrc;
+        virDomainHostdevDefPtr hostdev = hostdevs[i];
+
+        mdevsrc = &hostdev->source.subsys.u.mdev;
+
+        if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
+            hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
+            continue;
+        }
+
+        if (!(mdev = virMediatedDeviceNew(mdevsrc->uuidstr,
+                                          mdevsrc->model)))
+            continue;
+
+        /* Remove from the list only mdevs assigned to @drv_name/@dom_name */
+
+        tmp = virMediatedDeviceListFind(mgr->activeMediatedHostdevs, mdev);
+        virMediatedDeviceFree(mdev);
+
+        /* skip inactive devices */
+        if (!tmp)
+            continue;
+
+        virMediatedDeviceGetUsedBy(tmp, &used_by_drvname, &used_by_domname);
+        if (STREQ_NULLABLE(drv_name, used_by_drvname) &&
+            STREQ_NULLABLE(dom_name, used_by_domname)) {
+            VIR_DEBUG("Removing %s dom=%s from activeMediatedHostdevs",
+                      mdevsrc->uuidstr, dom_name);
+            virMediatedDeviceListDel(mgr->activeMediatedHostdevs, tmp);
+        }
+    }
+    virObjectUnlock(mgr->activeMediatedHostdevs);
+}
+
 int
 virHostdevPCINodeDeviceDetach(virHostdevManagerPtr mgr,
                               virPCIDevicePtr pci)
diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h
index 1202136c29..42e898211e 100644
--- a/src/util/virhostdev.h
+++ b/src/util/virhostdev.h
@@ -32,6 +32,7 @@
 # include "virscsi.h"
 # include "virscsivhost.h"
 # include "conf/domain_conf.h"
+# include "virmdev.h"
 
 typedef enum {
     VIR_HOSTDEV_STRICT_ACS_CHECK     = (1 << 0), /* strict acs check */
@@ -55,6 +56,7 @@ struct _virHostdevManager {
     virUSBDeviceListPtr activeUSBHostdevs;
     virSCSIDeviceListPtr activeSCSIHostdevs;
     virSCSIVHostDeviceListPtr activeSCSIVHostHostdevs;
+    virMediatedDeviceListPtr activeMediatedHostdevs;
 };
 
 virHostdevManagerPtr virHostdevManagerGetDefault(void);
@@ -96,6 +98,13 @@ virHostdevPrepareSCSIVHostDevices(virHostdevManagerPtr hostdev_mgr,
                                   virDomainHostdevDefPtr *hostdevs,
                                   int nhostdevs)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+int
+virHostdevPrepareMediatedDevices(virHostdevManagerPtr hostdev_mgr,
+                                 const char *drv_name,
+                                 const char *dom_name,
+                                 virDomainHostdevDefPtr *hostdevs,
+                                 int nhostdevs)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
 void
 virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
                              const char *drv_name,
@@ -125,6 +134,13 @@ virHostdevReAttachSCSIVHostDevices(virHostdevManagerPtr hostdev_mgr,
                                    virDomainHostdevDefPtr *hostdevs,
                                    int nhostdevs)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+void
+virHostdevReAttachMediatedDevices(virHostdevManagerPtr hostdev_mgr,
+                                  const char *drv_name,
+                                  const char *dom_name,
+                                  virDomainHostdevDefPtr *hostdevs,
+                                  int nhostdevs)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
 int
 virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr,
                                  virDomainHostdevDefPtr *hostdevs,
@@ -147,6 +163,13 @@ virHostdevUpdateActiveSCSIDevices(virHostdevManagerPtr mgr,
                                   const char *dom_name)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
 int
+virHostdevUpdateActiveMediatedDevices(virHostdevManagerPtr mgr,
+                                      virDomainHostdevDefPtr *hostdevs,
+                                      int nhostdevs,
+                                      const char *drv_name,
+                                      const char *dom_name)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
+int
 virHostdevUpdateActiveDomainDevices(virHostdevManagerPtr mgr,
                                     const char *driver,
                                     virDomainDefPtr def,
-- 
2.12.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 09/14] hostdev: Maintain a driver list of active mediated devices
Posted by Laine Stump 8 years, 3 months ago
(I'm unable to apply this patch to the head of master with "git am -3",
and it won't show me the conflicts (it just fails saying "fatal: sha1
information is lacking or useless (src/libvirt_private.syms), error:
could not build fake ancestor". Because of this, all further review is
based purely on examining the patch emails.)

On 03/22/2017 11:27 AM, Erik Skultety wrote:
> Keep track of the assigned mediated devices the same way we do it for
> the rest of hostdevs. Methods like 'Prepare', 'Update', and 'ReAttach'
> are introduced by this patch.
> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  src/libvirt_private.syms |   3 +
>  src/qemu/qemu_hostdev.c  |  56 ++++++++++++++++
>  src/qemu/qemu_hostdev.h  |  10 +++
>  src/util/virhostdev.c    | 165 ++++++++++++++++++++++++++++++++++++++++++++++-
>  src/util/virhostdev.h    |  23 +++++++
>  5 files changed, 256 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index c51b295d30..8f3b9697e4 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1730,16 +1730,19 @@ virHostdevPCINodeDeviceDetach;
>  virHostdevPCINodeDeviceReAttach;
>  virHostdevPCINodeDeviceReset;
>  virHostdevPrepareDomainDevices;
> +virHostdevPrepareMediatedDevices;
>  virHostdevPreparePCIDevices;
>  virHostdevPrepareSCSIDevices;
>  virHostdevPrepareSCSIVHostDevices;
>  virHostdevPrepareUSBDevices;
>  virHostdevReAttachDomainDevices;
> +virHostdevReAttachMediatedDevices;
>  virHostdevReAttachPCIDevices;
>  virHostdevReAttachSCSIDevices;
>  virHostdevReAttachSCSIVHostDevices;
>  virHostdevReAttachUSBDevices;
>  virHostdevUpdateActiveDomainDevices;
> +virHostdevUpdateActiveMediatedDevices;
>  virHostdevUpdateActivePCIDevices;
>  virHostdevUpdateActiveSCSIDevices;
>  virHostdevUpdateActiveUSBDevices;

BTW, I'm assuming that you've run "make syntax-check && make check" as
each patch is applied, so that we're sure the functions in
libvirt_private.syms are in proper alphabetic order (among other
things). I've run it at a few stages of applying the patches, but not all.


> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> index 7cd49e4aa5..685bf5b59f 100644
> --- a/src/qemu/qemu_hostdev.c
> +++ b/src/qemu/qemu_hostdev.c
> @@ -83,6 +83,22 @@ qemuHostdevUpdateActiveSCSIDevices(virQEMUDriverPtr driver,
>                                               QEMU_DRIVER_NAME, def->name);
>  }
>  
> +
> +int
> +qemuHostdevUpdateActiveMediatedDevices(virQEMUDriverPtr driver,
> +                                       virDomainDefPtr def)
> +{
> +    virHostdevManagerPtr mgr = driver->hostdevMgr;
> +
> +    if (!def->nhostdevs)
> +        return 0;
> +
> +    return virHostdevUpdateActiveMediatedDevices(mgr, def->hostdevs,
> +                                                 def->nhostdevs,
> +                                                 QEMU_DRIVER_NAME, def->name);
> +}
> +
> +
>  int
>  qemuHostdevUpdateActiveDomainDevices(virQEMUDriverPtr driver,
>                                       virDomainDefPtr def)
> @@ -99,6 +115,9 @@ qemuHostdevUpdateActiveDomainDevices(virQEMUDriverPtr driver,
>      if (qemuHostdevUpdateActiveSCSIDevices(driver, def) < 0)
>          return -1;
>  
> +    if (qemuHostdevUpdateActiveMediatedDevices(driver, def) < 0)
> +        return -1;
> +
>      return 0;
>  }
>  
> @@ -305,6 +324,24 @@ qemuHostdevPrepareSCSIVHostDevices(virQEMUDriverPtr driver,
>  }
>  
>  int
> +qemuHostdevPrepareMediatedDevices(virQEMUDriverPtr driver,
> +                                  const char *name,
> +                                  virDomainHostdevDefPtr *hostdevs,
> +                                  int nhostdevs)
> +{
> +    virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> +
> +    if (!qemuHostdevHostSupportsPassthroughVFIO()) {

Sshhhh! We won't let Alex know this function exists :-)


> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("host doesn't support VFIO PCI interface"));
> +        return -1;
> +    }
> +
> +    return virHostdevPrepareMediatedDevices(hostdev_mgr, QEMU_DRIVER_NAME,
> +                                            name, hostdevs, nhostdevs);
> +}
> +
> +int
>  qemuHostdevPrepareDomainDevices(virQEMUDriverPtr driver,
>                                  virDomainDefPtr def,
>                                  virQEMUCapsPtr qemuCaps,
> @@ -330,6 +367,10 @@ qemuHostdevPrepareDomainDevices(virQEMUDriverPtr driver,
>                                             def->hostdevs, def->nhostdevs) < 0)
>          return -1;
>  
> +    if (qemuHostdevPrepareMediatedDevices(driver, def->name,
> +                                          def->hostdevs, def->nhostdevs) < 0)
> +        return -1;
> +
>      return 0;
>  }
>  
> @@ -397,6 +438,18 @@ qemuHostdevReAttachSCSIVHostDevices(virQEMUDriverPtr driver,
>  }
>  
>  void
> +qemuHostdevReAttachMediatedDevices(virQEMUDriverPtr driver,
> +                                   const char *name,
> +                                   virDomainHostdevDefPtr *hostdevs,
> +                                   int nhostdevs)
> +{
> +    virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> +
> +    virHostdevReAttachMediatedDevices(hostdev_mgr, QEMU_DRIVER_NAME,
> +                                      name, hostdevs, nhostdevs);


What does "ReAttach" mean for a mediated device? Isn't this a NOP?

Oh, nevermind - I looked ahead and compared to the PCI version of the
ReAttach function. Turns out the PCI version does a *bunch* of things
other than reattaching the device to its host driver, including
restoring netdev config (which doesn't apply for mdevs and calling
virPCIDeviceReset() (which is a NOP even for PCI devices as long as
we're using VFIO, which *everybody* is these days). *BUT* the one other
thing it does is move the devices that had been in use by the domain
from the active list to the inactive list, and that's what the mdev
version of the function does.

Someday in the future we may want to clean these up and rename the
functions appropriately, but for now having them named similarly makes
it easier to review, so forget I said anything :-)




> +}
> +
> +void
>  qemuHostdevReAttachDomainDevices(virQEMUDriverPtr driver,
>                                   virDomainDefPtr def)
>  {
> @@ -414,4 +467,7 @@ qemuHostdevReAttachDomainDevices(virQEMUDriverPtr driver,
>  
>      qemuHostdevReAttachSCSIVHostDevices(driver, def->name, def->hostdevs,
>                                          def->nhostdevs);
> +
> +    qemuHostdevReAttachMediatedDevices(driver, def->name, def->hostdevs,
> +                                       def->nhostdevs);
>  }
> diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
> index 74a7d4f34e..9a7c7f143c 100644
> --- a/src/qemu/qemu_hostdev.h
> +++ b/src/qemu/qemu_hostdev.h
> @@ -30,6 +30,8 @@
>  bool qemuHostdevHostSupportsPassthroughLegacy(void);
>  bool qemuHostdevHostSupportsPassthroughVFIO(void);
>  
> +int qemuHostdevUpdateActiveMediatedDevices(virQEMUDriverPtr driver,
> +                                           virDomainDefPtr def);
>  int qemuHostdevUpdateActivePCIDevices(virQEMUDriverPtr driver,
>                                        virDomainDefPtr def);
>  int qemuHostdevUpdateActiveUSBDevices(virQEMUDriverPtr driver,
> @@ -59,6 +61,10 @@ int qemuHostdevPrepareSCSIVHostDevices(virQEMUDriverPtr driver,
>                                         const char *name,
>                                         virDomainHostdevDefPtr *hostdevs,
>                                         int nhostdevs);
> +int qemuHostdevPrepareMediatedDevices(virQEMUDriverPtr driver,
> +                                      const char *name,
> +                                      virDomainHostdevDefPtr *hostdevs,
> +                                      int nhostdevs);
>  int qemuHostdevPrepareDomainDevices(virQEMUDriverPtr driver,
>                                      virDomainDefPtr def,
>                                      virQEMUCapsPtr qemuCaps,
> @@ -80,6 +86,10 @@ void qemuHostdevReAttachSCSIVHostDevices(virQEMUDriverPtr driver,
>                                           const char *name,
>                                           virDomainHostdevDefPtr *hostdevs,
>                                           int nhostdevs);
> +void qemuHostdevReAttachMediatedDevices(virQEMUDriverPtr driver,
> +                                        const char *name,
> +                                        virDomainHostdevDefPtr *hostdevs,
> +                                        int nhostdevs);
>  void qemuHostdevReAttachDomainDevices(virQEMUDriverPtr driver,
>                                        virDomainDefPtr def);
>  
> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index 86ca8e0473..0c337e6116 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -1,6 +1,6 @@
>  /* virhostdev.c: hostdev management
>   *
> - * Copyright (C) 2006-2007, 2009-2016 Red Hat, Inc.
> + * Copyright (C) 2006-2007, 2009-2017 Red Hat, Inc.
>   * Copyright (C) 2006 Daniel P. Berrange
>   * Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
>   *
> @@ -147,6 +147,7 @@ virHostdevManagerDispose(void *obj)
>      virObjectUnref(hostdevMgr->activeUSBHostdevs);
>      virObjectUnref(hostdevMgr->activeSCSIHostdevs);
>      virObjectUnref(hostdevMgr->activeSCSIVHostHostdevs);
> +    virObjectUnref(hostdevMgr->activeMediatedHostdevs);
>      VIR_FREE(hostdevMgr->stateDir);
>  }
>  
> @@ -174,6 +175,9 @@ virHostdevManagerNew(void)
>      if (!(hostdevMgr->activeSCSIVHostHostdevs = virSCSIVHostDeviceListNew()))
>          goto error;
>  
> +    if (!(hostdevMgr->activeMediatedHostdevs = virMediatedDeviceListNew()))
> +        goto error;
> +
>      if (privileged) {
>          if (VIR_STRDUP(hostdevMgr->stateDir, HOSTDEV_STATE_DIR) < 0)
>              goto error;
> @@ -1147,6 +1151,50 @@ virHostdevUpdateActiveSCSIDevices(virHostdevManagerPtr mgr,
>      return ret;
>  }
>  
> +
> +int
> +virHostdevUpdateActiveMediatedDevices(virHostdevManagerPtr mgr,
> +                                      virDomainHostdevDefPtr *hostdevs,
> +                                      int nhostdevs,
> +                                      const char *drv_name,
> +                                      const char *dom_name)
> +{
> +    int ret = -1;
> +    size_t i;
> +    virMediatedDevicePtr mdev = NULL;
> +
> +    if (nhostdevs == 0)
> +        return 0;
> +
> +    virObjectLock(mgr->activeMediatedHostdevs);
> +    for (i = 0; i < nhostdevs; i++) {
> +        virDomainHostdevDefPtr hostdev = hostdevs[i];
> +        virDomainHostdevSubsysMediatedDevPtr mdevsrc;
> +
> +        mdevsrc = &hostdev->source.subsys.u.mdev;
> +
> +        if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
> +            hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
> +            continue;
> +        }
> +
> +        if (!(mdev = virMediatedDeviceNew(mdevsrc->uuidstr, mdevsrc->model)))
> +            goto cleanup;

Sigh. *still* with the temporary object creation just to call some
utility function that could have just as easily taken different args...

> +
> +        virMediatedDeviceSetUsedBy(mdev, drv_name, dom_name);
> +
> +        if (virMediatedDeviceListAdd(mgr->activeMediatedHostdevs, mdev) < 0)
> +            goto cleanup;
> +    }
> +
> +    ret = 0;
> + cleanup:
> +    virMediatedDeviceFree(mdev);
> +    virObjectUnlock(mgr->activeMediatedHostdevs);
> +    return ret;
> +}
> +
> +
>  static int
>  virHostdevMarkUSBDevices(virHostdevManagerPtr mgr,
>                           const char *drv_name,
> @@ -1595,6 +1643,70 @@ virHostdevPrepareSCSIVHostDevices(virHostdevManagerPtr mgr,
>      return -1;
>  }
>  
> +
> +int
> +virHostdevPrepareMediatedDevices(virHostdevManagerPtr mgr,
> +                                 const char *drv_name,
> +                                 const char *dom_name,
> +                                 virDomainHostdevDefPtr *hostdevs,
> +                                 int nhostdevs)
> +{
> +    size_t i;
> +    int ret = -1;
> +    virMediatedDeviceListPtr list;
> +
> +    if (!nhostdevs)
> +        return 0;
> +
> +    /* To prevent situation where mediated device is assigned to multiple
> +     * domains we maintain a driver list of currently assigned mediated devices.
> +     * A device is appended to the driver list after a series of preparations.
> +     */
> +    if (!(list = virMediatedDeviceListNew()))
> +        goto cleanup;
> +
> +    /* Loop 1: Build a temporary list of ALL mediated devices. */
> +    for (i = 0; i < nhostdevs; i++) {
> +        virDomainHostdevDefPtr hostdev = hostdevs[i];
> +        virDomainHostdevSubsysMediatedDevPtr src = &hostdev->source.subsys.u.mdev;
> +        virMediatedDevicePtr mdev;
> +
> +        if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
> +            continue;
> +        if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV)
> +            continue;
> +
> +        if (!(mdev = virMediatedDeviceNew(src->uuidstr, src->model)))
> +            goto cleanup;
> +
> +        if (virMediatedDeviceListAdd(list, mdev) < 0) {
> +            virMediatedDeviceFree(mdev);
> +            goto cleanup;
> +        }
> +    }
> +
> +    /* Mark the devices in the list as used by @drv_name-@dom_name and copy the
> +     * references to the driver list
> +     */
> +    if (virMediatedDeviceListMarkDevices(mgr->activeMediatedHostdevs,
> +                                         list, drv_name, dom_name) < 0)
> +        goto cleanup;
> +
> +    /* Loop 2: Temporary list was successfully merged with
> +     * driver list, so steal all items to avoid freeing them
> +     * in cleanup label.
> +     */
> +    while (virMediatedDeviceListCount(list) > 0) {
> +        virMediatedDevicePtr tmp = virMediatedDeviceListGet(list, 0);
> +        virMediatedDeviceListSteal(list, tmp);
> +    }
> +
> +    ret = 0;
> + cleanup:
> +    virObjectUnref(list);
> +    return ret;
> +}
> +
>  void
>  virHostdevReAttachUSBDevices(virHostdevManagerPtr mgr,
>                               const char *drv_name,
> @@ -1789,6 +1901,57 @@ virHostdevReAttachSCSIVHostDevices(virHostdevManagerPtr mgr,
>      virObjectUnlock(mgr->activeSCSIVHostHostdevs);
>  }
>  
> +void
> +virHostdevReAttachMediatedDevices(virHostdevManagerPtr mgr,
> +                                  const char *drv_name,
> +                                  const char *dom_name,
> +                                  virDomainHostdevDefPtr *hostdevs,
> +                                  int nhostdevs)
> +{
> +    const char *used_by_drvname = NULL;
> +    const char *used_by_domname = NULL;
> +    size_t i;
> +
> +    if (nhostdevs == 0)
> +        return;
> +
> +    virObjectLock(mgr->activeMediatedHostdevs);
> +    for (i = 0; i < nhostdevs; i++) {
> +        virMediatedDevicePtr mdev, tmp;
> +        virDomainHostdevSubsysMediatedDevPtr mdevsrc;
> +        virDomainHostdevDefPtr hostdev = hostdevs[i];
> +
> +        mdevsrc = &hostdev->source.subsys.u.mdev;
> +
> +        if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
> +            hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
> +            continue;
> +        }
> +
> +        if (!(mdev = virMediatedDeviceNew(mdevsrc->uuidstr,
> +                                          mdevsrc->model)))
> +            continue;
> +
> +        /* Remove from the list only mdevs assigned to @drv_name/@dom_name */
> +
> +        tmp = virMediatedDeviceListFind(mgr->activeMediatedHostdevs, mdev);
> +        virMediatedDeviceFree(mdev);
> +
> +        /* skip inactive devices */
> +        if (!tmp)
> +            continue;
> +
> +        virMediatedDeviceGetUsedBy(tmp, &used_by_drvname, &used_by_domname);
> +        if (STREQ_NULLABLE(drv_name, used_by_drvname) &&
> +            STREQ_NULLABLE(dom_name, used_by_domname)) {
> +            VIR_DEBUG("Removing %s dom=%s from activeMediatedHostdevs",
> +                      mdevsrc->uuidstr, dom_name);
> +            virMediatedDeviceListDel(mgr->activeMediatedHostdevs, tmp);
> +        }
> +    }
> +    virObjectUnlock(mgr->activeMediatedHostdevs);
> +}
> +
>  int
>  virHostdevPCINodeDeviceDetach(virHostdevManagerPtr mgr,
>                                virPCIDevicePtr pci)
> diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h
> index 1202136c29..42e898211e 100644
> --- a/src/util/virhostdev.h
> +++ b/src/util/virhostdev.h
> @@ -32,6 +32,7 @@
>  # include "virscsi.h"
>  # include "virscsivhost.h"
>  # include "conf/domain_conf.h"
> +# include "virmdev.h"
>  
>  typedef enum {
>      VIR_HOSTDEV_STRICT_ACS_CHECK     = (1 << 0), /* strict acs check */
> @@ -55,6 +56,7 @@ struct _virHostdevManager {
>      virUSBDeviceListPtr activeUSBHostdevs;
>      virSCSIDeviceListPtr activeSCSIHostdevs;
>      virSCSIVHostDeviceListPtr activeSCSIVHostHostdevs;
> +    virMediatedDeviceListPtr activeMediatedHostdevs;
>  };
>  
>  virHostdevManagerPtr virHostdevManagerGetDefault(void);
> @@ -96,6 +98,13 @@ virHostdevPrepareSCSIVHostDevices(virHostdevManagerPtr hostdev_mgr,
>                                    virDomainHostdevDefPtr *hostdevs,
>                                    int nhostdevs)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
> +int
> +virHostdevPrepareMediatedDevices(virHostdevManagerPtr hostdev_mgr,
> +                                 const char *drv_name,
> +                                 const char *dom_name,
> +                                 virDomainHostdevDefPtr *hostdevs,
> +                                 int nhostdevs)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>  void
>  virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
>                               const char *drv_name,
> @@ -125,6 +134,13 @@ virHostdevReAttachSCSIVHostDevices(virHostdevManagerPtr hostdev_mgr,
>                                     virDomainHostdevDefPtr *hostdevs,
>                                     int nhostdevs)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
> +void
> +virHostdevReAttachMediatedDevices(virHostdevManagerPtr hostdev_mgr,
> +                                  const char *drv_name,
> +                                  const char *dom_name,
> +                                  virDomainHostdevDefPtr *hostdevs,
> +                                  int nhostdevs)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>  int
>  virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr,
>                                   virDomainHostdevDefPtr *hostdevs,
> @@ -147,6 +163,13 @@ virHostdevUpdateActiveSCSIDevices(virHostdevManagerPtr mgr,
>                                    const char *dom_name)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
>  int
> +virHostdevUpdateActiveMediatedDevices(virHostdevManagerPtr mgr,
> +                                      virDomainHostdevDefPtr *hostdevs,
> +                                      int nhostdevs,
> +                                      const char *drv_name,
> +                                      const char *dom_name)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
> +int
>  virHostdevUpdateActiveDomainDevices(virHostdevManagerPtr mgr,
>                                      const char *driver,
>                                      virDomainDefPtr def,
> 


ACK.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 09/14] hostdev: Maintain a driver list of active mediated devices
Posted by Erik Skultety 8 years, 3 months ago
On Sun, Mar 26, 2017 at 03:00:58PM -0400, Laine Stump wrote:
> (I'm unable to apply this patch to the head of master with "git am -3",
> and it won't show me the conflicts (it just fails saying "fatal: sha1
> information is lacking or useless (src/libvirt_private.syms), error:
> could not build fake ancestor". Because of this, all further review is
> based purely on examining the patch emails.)

Hmm, I also got a merge conflict (not sure if it's the same one you got) at
virHostdevReAttachMediatedDevices, but not with a fatal error. Wondering what
the problem at your end might be.

> BTW, I'm assuming that you've run "make syntax-check && make check" as
> each patch is applied, so that we're sure the functions in

I have.

> libvirt_private.syms are in proper alphabetic order (among other
> things). I've run it at a few stages of applying the patches, but not all.
> 
> 

[..]

> >  void
> > +qemuHostdevReAttachMediatedDevices(virQEMUDriverPtr driver,
> > +                                   const char *name,
> > +                                   virDomainHostdevDefPtr *hostdevs,
> > +                                   int nhostdevs)
> > +{
> > +    virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> > +
> > +    virHostdevReAttachMediatedDevices(hostdev_mgr, QEMU_DRIVER_NAME,
> > +                                      name, hostdevs, nhostdevs);
> 
> 
> What does "ReAttach" mean for a mediated device? Isn't this a NOP?
> 
> Oh, nevermind - I looked ahead and compared to the PCI version of the
> ReAttach function. Turns out the PCI version does a *bunch* of things
> other than reattaching the device to its host driver, including
> restoring netdev config (which doesn't apply for mdevs and calling
> virPCIDeviceReset() (which is a NOP even for PCI devices as long as
> we're using VFIO, which *everybody* is these days). *BUT* the one other
> thing it does is move the devices that had been in use by the domain
> from the active list to the inactive list, and that's what the mdev
> version of the function does.

Yeah, this was merely just to stay consistent, I hated it from the very moment
I introduced the function, but I checked with other types of devices and
thought "it just might be easier to read", but I will add a comment that it's
there just because of consistency reasons and re-attach doesn't make any sense
for mdevs.

> 
> Someday in the future we may want to clean these up and rename the
> functions appropriately, but for now having them named similarly makes
> it easier to review, so forget I said anything :-)

Yep.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 09/14] hostdev: Maintain a driver list of active mediated devices
Posted by Daniel P. Berrange 8 years, 3 months ago
On Wed, Mar 22, 2017 at 04:27:37PM +0100, Erik Skultety wrote:
> Keep track of the assigned mediated devices the same way we do it for
> the rest of hostdevs. Methods like 'Prepare', 'Update', and 'ReAttach'
> are introduced by this patch.
> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  src/libvirt_private.syms |   3 +
>  src/qemu/qemu_hostdev.c  |  56 ++++++++++++++++
>  src/qemu/qemu_hostdev.h  |  10 +++
>  src/util/virhostdev.c    | 165 ++++++++++++++++++++++++++++++++++++++++++++++-
>  src/util/virhostdev.h    |  23 +++++++
>  5 files changed, 256 insertions(+), 1 deletion(-)


> @@ -305,6 +324,24 @@ qemuHostdevPrepareSCSIVHostDevices(virQEMUDriverPtr driver,
>  }
>  
>  int
> +qemuHostdevPrepareMediatedDevices(virQEMUDriverPtr driver,
> +                                  const char *name,
> +                                  virDomainHostdevDefPtr *hostdevs,
> +                                  int nhostdevs)
> +{
> +    virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> +
> +    if (!qemuHostdevHostSupportsPassthroughVFIO()) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("host doesn't support VFIO PCI interface"));
> +        return -1;
> +    }

This is unconditionally breaking *all* use of host devices on libvirt
if the system lacks VFIO, as it is not actually checking if any of
the 'hostdevs' are actually mediated devices, or indeed whether they
are even PCI devices. ie i can no longer boot a guest that uses USB
host device passthrough.

> +
> +    return virHostdevPrepareMediatedDevices(hostdev_mgr, QEMU_DRIVER_NAME,
> +                                            name, hostdevs, nhostdevs);
> +}
> +
> +int
>  qemuHostdevPrepareDomainDevices(virQEMUDriverPtr driver,
>                                  virDomainDefPtr def,
>                                  virQEMUCapsPtr qemuCaps,
> @@ -330,6 +367,10 @@ qemuHostdevPrepareDomainDevices(virQEMUDriverPtr driver,
>                                             def->hostdevs, def->nhostdevs) < 0)
>          return -1;
>  
> +    if (qemuHostdevPrepareMediatedDevices(driver, def->name,
> +                                          def->hostdevs, def->nhostdevs) < 0)
> +        return -1;



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