[libvirt] [PATCH v4 09/14] hostdev: Maintain a driver list of active mediated devices
Laine Stump
laine at laine.org
Sun Mar 26 19:00:58 UTC 2017
(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 at 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- at 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.
More information about the libvir-list
mailing list