On 03/22/2017 11:27 AM, Erik Skultety wrote:
> A mediated device will be identified by a UUID (with 'model' now being
> a mandatory <hostdev> attribute to represent the mediated device API) of
> the user pre-created mediated device. The data necessary to identify a
> mediated device can be easily extended in the future, e.g. when
> auto-creation of mediated devices should be enabled. We also need to
s/should be/is/
(although... it really goes without saying that the data needed can be
easily extended. After all, you wrote it, right? :-P)
> make sure that if user explicitly provides a guest address
> for a mdev device, the address type will be matching the device API
> supported on that specific mediated device and error out with an
> incorrect XML message.
Please include a sample of the xml for an mdev <hostdev> in the commit log.
>
> Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
> ---
> docs/schemas/domaincommon.rng | 22 +++++++++
> src/conf/domain_conf.c | 99 ++++++++++++++++++++++++++++++++++++-
> src/conf/domain_conf.h | 9 ++++
> src/qemu/qemu_domain.c | 1 +
> src/qemu/qemu_hotplug.c | 2 +
> src/security/security_apparmor.c | 3 ++
> src/security/security_dac.c | 2 +
> src/security/security_selinux.c | 2 +
> tests/domaincapsschemadata/full.xml | 1 +
> 9 files changed, 140 insertions(+), 1 deletion(-)
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index fbedc9b1f9..edc225fe50 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4034,6 +4034,7 @@
> <ref name="hostdevsubsysusb"/>
> <ref name="hostdevsubsysscsi"/>
> <ref name="hostdevsubsyshost"/>
> + <ref name="hostdevsubsysmdev"/>
> </choice>
> </define>
>
> @@ -4184,6 +4185,20 @@
> </element>
> </define>
>
> + <define name="hostdevsubsysmdev">
> + <attribute name="type">
> + <value>mdev</value>
> + </attribute>
> + <attribute name="model">
> + <choice>
> + <value>vfio-pci</value>
> + </choice>
> + </attribute>
> + <element name="source">
> + <ref name="mdevaddress"/>
> + </element>
> + </define>
> +
> <define name="hostdevcapsstorage">
> <attribute name="type">
> <value>storage</value>
> @@ -4342,6 +4357,13 @@
> </attribute>
> </optional>
> </define>
> + <define name="mdevaddress">
> + <element name="address">
> + <attribute name="uuid">
> + <ref name="UUID"/>
> + </attribute>
> + </element>
> + </define>
> <define name="devices">
> <element name="devices">
> <interleave>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a5ab42297d..63ac65e8ab 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -56,6 +56,7 @@
> #include "virstring.h"
> #include "virnetdev.h"
> #include "virhostdev.h"
> +#include "virmdev.h"
>
> #define VIR_FROM_THIS VIR_FROM_DOMAIN
>
> @@ -652,7 +653,8 @@ VIR_ENUM_IMPL(virDomainHostdevSubsys, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST,
> "usb",
> "pci",
> "scsi",
> - "scsi_host")
> + "scsi_host",
> + "mdev")
>
> VIR_ENUM_IMPL(virDomainHostdevSubsysPCIBackend,
> VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST,
> @@ -2356,6 +2358,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
> break;
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> break;
> }
> @@ -4251,6 +4254,23 @@ virDomainHostdevDefPostParse(virDomainHostdevDefPtr dev,
> }
> }
> break;
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
> + int model = dev->source.subsys.u.mdev.model;
> +
> + if (dev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> + return 0;
> +
> + if (model == VIR_MDEV_MODEL_TYPE_VFIO_PCI &&
> + dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Unsupported address type '%s' with mediated "
> + "device model '%s'"),
> + virDomainDeviceAddressTypeToString(dev->info->type),
> + virMediatedDeviceModelTypeToString(model));
> + return -1;
> + }
> + }
> +
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
> @@ -6397,6 +6417,41 @@ virDomainHostdevSubsysSCSIVHostDefParseXML(xmlNodePtr sourcenode,
> return ret;
> }
>
> +static int
> +virDomainHostdevSubsysMediatedDevDefParseXML(virDomainHostdevDefPtr def,
> + xmlXPathContextPtr ctxt)
> +{
> + int ret = -1;
> + unsigned char uuid[VIR_UUID_BUFLEN] = {0};
> + char *uuidxml = NULL;
> + xmlNodePtr node = NULL;
> + virDomainHostdevSubsysMediatedDevPtr mdevsrc = &def->source.subsys.u.mdev;
> +
> + if (!(node = virXPathNode("./source/address", ctxt))) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Missing <address> element"));
> + goto cleanup;
> + }
> +
> + if (!(uuidxml = virXMLPropString(node, "uuid"))) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Missing 'uuid' attribute for element <address>"));
> + goto cleanup;
> + }
> +
> + if (virUUIDParse(uuidxml, uuid) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s",
> + _("Cannot parse uuid attribute of element <address>"));
> + goto cleanup;
> + }
> +
> + virUUIDFormat(uuid, mdevsrc->uuidstr);
> + ret = 0;
> + cleanup:
> + VIR_FREE(uuidxml);
> + return ret;
> +}
>
> static int
> virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
> @@ -6410,10 +6465,12 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
> char *sgio = NULL;
> char *rawio = NULL;
> char *backendStr = NULL;
> + char *model = NULL;
> int backend;
> int ret = -1;
> virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci;
> virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi;
> + virDomainHostdevSubsysMediatedDevPtr mdevsrc = &def->source.subsys.u.mdev;
>
> /* @managed can be read from the xml document - it is always an
> * attribute of the toplevel element, no matter what type of
> @@ -6427,6 +6484,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
>
> sgio = virXMLPropString(node, "sgio");
> rawio = virXMLPropString(node, "rawio");
> + model = virXMLPropString(node, "model");
>
> /* @type is passed in from the caller rather than read from the
> * xml document, because it is specified in different places for
> @@ -6493,6 +6551,28 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
> }
> }
>
> + if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
> + if (model) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("model is only supported with mediated devices"));
This error message should be more specific, e.g. "model attribute is
supported in <hostdev> devices only if type='mdev'".
> + goto error;
> + }
> + } else {
> + if (!model) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Missing 'model' attribute in mediated device's "
> + "<hostdev> element"));
Do you actually required it in the original config? I had thought it
should be optional, and filled in from the device-api if it's missing
(hmm, although I guess the problem is that we don't have access to the
device-api at the time of definition, so maybe not :-(. Still, maybe we
can divine a default based on the machinetype - for x86 vfio-pci is an
extremely safe bet, for example).
Anyway, that could be a lengthy discussion, so not worth holding
everything else up for it - we can always loosen the restrictions and
supply a default later. (it *would* be nice to be able to completely
specify the config without needing to use "vfio" anywhere...)
(end of comments)
ACK with the commit log changed, and the one error log made more specific.
> + goto error;
> + }
> +
> + if ((mdevsrc->model = virMediatedDeviceModelTypeFromString(model)) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("unknown hostdev model '%s'"),
> + model);
> + goto error;
> + }
> + }
> +
> switch (def->source.subsys.type) {
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> if (virDomainHostdevSubsysPCIDefParseXML(sourcenode, def, flags) < 0)
> @@ -6525,6 +6605,10 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
> if (virDomainHostdevSubsysSCSIVHostDefParseXML(sourcenode, def) < 0)
> goto error;
> break;
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> + if (virDomainHostdevSubsysMediatedDevDefParseXML(def, ctxt) < 0)
> + goto error;
> + break;
>
> default:
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -6539,6 +6623,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
> VIR_FREE(sgio);
> VIR_FREE(rawio);
> VIR_FREE(backendStr);
> + VIR_FREE(model);
> return ret;
> }
>
> @@ -13384,6 +13469,7 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt,
> }
> break;
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> break;
> }
> @@ -14318,6 +14404,7 @@ virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
> return 1;
> else
> return 0;
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> return 0;
> }
> @@ -21319,6 +21406,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
> virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci;
> virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi;
> virDomainHostdevSubsysSCSIVHostPtr hostsrc = &def->source.subsys.u.scsi_host;
> + virDomainHostdevSubsysMediatedDevPtr mdevsrc = &def->source.subsys.u.mdev;
> virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
> virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
>
> @@ -21423,6 +21511,10 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
> break;
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
> break;
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> + virBufferAsprintf(buf, "<address uuid='%s'/>\n",
> + mdevsrc->uuidstr);
> + break;
> default:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("unexpected hostdev type %d"),
> @@ -23318,6 +23410,7 @@ virDomainHostdevDefFormat(virBufferPtr buf,
> {
> const char *mode = virDomainHostdevModeTypeToString(def->mode);
> virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi;
> + virDomainHostdevSubsysMediatedDevPtr mdevsrc = &def->source.subsys.u.mdev;
> const char *type;
>
> if (!mode) {
> @@ -23367,6 +23460,10 @@ virDomainHostdevDefFormat(virBufferPtr buf,
> virBufferAsprintf(buf, " rawio='%s'",
> virTristateBoolTypeToString(scsisrc->rawio));
> }
> +
> + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV)
> + virBufferAsprintf(buf, " model='%s'",
> + virMediatedDeviceModelTypeToString(mdevsrc->model));
> }
> virBufferAddLit(buf, ">\n");
> virBufferAdjustIndent(buf, 2);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 8e6d874178..47eaacef3d 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -295,6 +295,7 @@ typedef enum {
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI,
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI,
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST,
> + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV,
>
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST
> } virDomainHostdevSubsysType;
> @@ -369,6 +370,13 @@ struct _virDomainHostdevSubsysSCSI {
> } u;
> };
>
> +typedef struct _virDomainHostdevSubsysMediatedDev virDomainHostdevSubsysMediatedDev;
> +typedef virDomainHostdevSubsysMediatedDev *virDomainHostdevSubsysMediatedDevPtr;
> +struct _virDomainHostdevSubsysMediatedDev {
> + int model; /* enum virMediatedDeviceModelType */
> + char uuidstr[VIR_UUID_STRING_BUFLEN]; /* mediated device's uuid string */
> +};
> +
> typedef enum {
> VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_HOST_PROTOCOL_TYPE_NONE,
> VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_HOST_PROTOCOL_TYPE_VHOST,
> @@ -394,6 +402,7 @@ struct _virDomainHostdevSubsys {
> virDomainHostdevSubsysPCI pci;
> virDomainHostdevSubsysSCSI scsi;
> virDomainHostdevSubsysSCSIVHost scsi_host;
> + virDomainHostdevSubsysMediatedDev mdev;
> } u;
> };
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index c239a06767..c0f060b0a3 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7102,6 +7102,7 @@ qemuDomainGetHostdevPath(virDomainDefPtr def,
> break;
> }
>
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> break;
> }
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index ddcbc5e259..66c469e55e 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3907,6 +3907,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
> qemuDomainRemoveSCSIVHostDevice(driver, vm, hostdev);
> break;
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> + break;
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> break;
> }
> diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
> index 0d3e891a71..f5b72e1c2d 100644
> --- a/src/security/security_apparmor.c
> +++ b/src/security/security_apparmor.c
> @@ -901,6 +901,9 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
> break;
> }
>
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> + break;
> +
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> ret = 0;
> break;
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index d6a2daf747..4e968f29c0 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -964,6 +964,7 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
> break;
> }
>
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> ret = 0;
> break;
> @@ -1119,6 +1120,7 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
> break;
> }
>
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> ret = 0;
> break;
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index a6bcf9e01f..7b3276dc34 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -1838,6 +1838,7 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr,
> break;
> }
>
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> ret = 0;
> break;
> @@ -2065,6 +2066,7 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr,
> break;
> }
>
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> ret = 0;
> break;
> diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml
> index 6a676253c3..82a92322e5 100644
> --- a/tests/domaincapsschemadata/full.xml
> +++ b/tests/domaincapsschemadata/full.xml
> @@ -89,6 +89,7 @@
> <value>pci</value>
> <value>scsi</value>
> <value>scsi_host</value>
> + <value>mdev</value>
> </enum>
> <enum name='capsType'>
> <value>storage</value>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
