On Mon, Feb 20, 2017 at 03:28:24PM +0100, Erik Skultety wrote:
> Keep track of the assigned mediated devices the same way we do it for
> the rest of hostdevs.
>
> Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
> ---
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_hostdev.c | 22 ++++++
> src/qemu/qemu_hostdev.h | 4 ++
> src/util/virhostdev.c | 176 ++++++++++++++++++++++++++++++++++++++++++++++-
> src/util/virhostdev.h | 9 +++
> 5 files changed, 211 insertions(+), 1 deletion(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 8af65a1..e6d1282 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1706,6 +1706,7 @@ virHostdevPCINodeDeviceDetach;
> virHostdevPCINodeDeviceReAttach;
> virHostdevPCINodeDeviceReset;
> virHostdevPrepareDomainDevices;
> +virHostdevPrepareMediatedDevices;
> virHostdevPreparePCIDevices;
> virHostdevPrepareSCSIDevices;
> virHostdevPrepareSCSIVHostDevices;
> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> index 7cd49e4..45b731c 100644
> --- a/src/qemu/qemu_hostdev.c
> +++ b/src/qemu/qemu_hostdev.c
> @@ -305,6 +305,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 +348,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;
> }
>
> diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
> index 74a7d4f..9399241 100644
> --- a/src/qemu/qemu_hostdev.h
> +++ b/src/qemu/qemu_hostdev.h
> @@ -59,6 +59,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,
> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index 86ca8e0..681f720 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;
> @@ -1595,6 +1599,176 @@ virHostdevPrepareSCSIVHostDevices(virHostdevManagerPtr mgr,
> return -1;
> }
>
> +
> +static bool
> +virHostdevMediatedDeviceIsUsed(virMediatedDevicePtr dev,
> + virMediatedDeviceListPtr list)
> +{
This function can be moved into util/virmdev.c.
> + const char *drvname, *domname;
> + virMediatedDevicePtr tmp = NULL;
> +
> + virObjectLock(list);
> +
> + if ((tmp = virMediatedDeviceListFind(list, dev))) {
> + virMediatedDeviceGetUsedBy(tmp, &drvname, &domname);
> + virReportError(VIR_ERR_OPERATION_INVALID,
> + _("Mediated device %s is in use by "
> + "driver %s, domain %s"),
> + virMediatedDeviceGetPath(tmp),
> + drvname, domname);
> + }
> +
> + virObjectUnlock(list);
> + return !!tmp;
> +}
> +
> +
> +static int
> +virHostdevMediatedDeviceCheckModel(virMediatedDevicePtr dev,
> + virMediatedDeviceModelType model)
> +{
This function can be moved into util/virmedev.c.
> + int ret = -1;
> + char *dev_api = NULL;
> + int actual_model;
> +
> + if (virMediatedDeviceGetDeviceAPI(dev, &dev_api) < 0)
> + return -1;
> +
> + /* safeguard in case we've got an older libvirt which doesn't know newer
> + * device_api models yet
> + */
> + if ((actual_model = virMediatedDeviceModelTypeFromString(dev_api)) <= 0)
> + goto cleanup;
> +
> + if (actual_model != model) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Invalid device API '%s' for device %s: "
> + "device only supports '%s'"),
> + virMediatedDeviceModelTypeToString(model),
> + virMediatedDeviceGetPath(dev), dev_api);
> + goto cleanup;
> + }
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(dev_api);
> + return ret;
> +}
> +
> +
> +static int
> +virHostdevMarkMediatedDevices(virHostdevManagerPtr mgr,
> + const char *drvname,
> + const char *domname,
> + virMediatedDeviceListPtr list)
> +{
You can pass only mgr->activeMediatedHostdevs and this function can be moved
to util/virmdev.c.
> + int ret = -1;
> + size_t i, j, count;
> +
> + virObjectLock(mgr->activeMediatedHostdevs);
> +
> + count = virMediatedDeviceListCount(list);
> + for (i = 0; i < count; i++) {
> + virMediatedDevicePtr mdev = virMediatedDeviceListGet(list, i);
> + VIR_DEBUG("Adding %s to activeMediatedHostdevs",
> + virMediatedDeviceGetPath(mdev));
> +
> + virMediatedDeviceSetUsedBy(mdev, drvname, domname);
> +
> + /* Copy mdev references to the driver list:
> + * - caller is responsible for NOT freeing devices in @list on success
> + * - we're responsible for performing a rollback on failure
> + */
> + if (virMediatedDeviceListAdd(mgr->activeMediatedHostdevs, mdev) < 0)
> + goto rollback;
> + }
> +
> + ret = 0;
> + cleanup:
> + virObjectUnlock(mgr->activeMediatedHostdevs);
> + return ret;
> +
> + rollback:
> + for (j = 0; j < i; j++) {
> + virMediatedDevicePtr tmp = virMediatedDeviceListGet(list, j);
> + virMediatedDeviceListSteal(mgr->activeMediatedHostdevs, tmp);
> + }
> + goto cleanup;
> +}
> +
> +
> +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 unused 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)))
> + goto cleanup;
> +
> + if (virHostdevMediatedDeviceIsUsed(mdev, mgr->activeMediatedHostdevs))
> + goto cleanup;
This checks whether the mdev is used by another domain but the
*activeMediatedHostdevs* list is unlocked in this whole function so
if two domains starts at the same time they both can get the same device
as unused ... [1]
> +
> + /* Check whether the user-provided model corresponds with the actually
> + * supported mediated device's API.
> + */
> + if (virHostdevMediatedDeviceCheckModel(mdev, src->model) < 0)
> + 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 (virHostdevMarkMediatedDevices(mgr, drv_name, dom_name, list) < 0)
> + goto cleanup;
[1] and there both domain can mark the mdev as used without triggering any
error and with the side effect that the last domain to call this function would
be stored as an owner of that mdev.
To fix this issue you can move the virHostdevMediatedDeviceIsUsed check into
virHostdevMarkMediatedDevices where you can hold the lock of
*activeMediatedHostdevs* list for the whole time.
Pavel
> +
> + /* 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,
> diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h
> index 4c1fea3..b077089 100644
> --- a/src/util/virhostdev.h
> +++ b/src/util/virhostdev.h
> @@ -31,6 +31,7 @@
> # include "virusb.h"
> # include "virscsi.h"
> # include "virscsivhost.h"
> +# include "virmdev.h"
> # include "domain_conf.h"
>
> typedef enum {
> @@ -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,
> --
> 2.10.2
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
