Re: [libvirt] [RFC PATCH v2 REBASE 02/18] conf: Introduce new hostdev device type mdev

[Pavel Hrdina]

On Mon, Feb 20, 2017 at 03:28:15PM +0100, Erik Skultety wrote:
> A mediated device will be identified by a UUID 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.
> 
> Signed-off-by: Erik Skultety 
> ---
>  src/conf/domain_conf.c  | 36 +++-
>  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 +
>  8 files changed, 55 insertions(+), 1 deletion(-)

Patches 04/18 and 06/18 should be merged into this one, it adds the same
functionality.

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a56ea82..83aa15f 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
>  
> @@ -649,7 +650,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,
> @@ -668,6 +670,11 @@ VIR_ENUM_IMPL(virDomainHostdevSubsysSCSIHostProtocol,
>"none",
>"vhost")
>  
> +VIR_ENUM_IMPL(virMediatedDeviceModel,
> +  VIR_MDEV_MODEL_TYPE_LAST,
> +  "default",
> +  "vfio-pci")
> +
>  VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST,
>"storage",
>"misc",
> @@ -6349,10 +6356,12 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
>  char *sgio = NULL;
>  char *rawio = NULL;
>  char *backendStr = NULL;
> +char *model = NULL;
>  int backend;
>  int ret = -1;
>  virDomainHostdevSubsysPCIPtr pcisrc = >source.subsys.u.pci;
>  virDomainHostdevSubsysSCSIPtr scsisrc = >source.subsys.u.scsi;
> +virDomainHostdevSubsysMediatedDevPtr mdevsrc = 
> >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
> @@ -6432,6 +6441,21 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
>  }
>  }
>  
> +if (model) {
> +if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("model is only supported with mediated 
> devices"));
> +goto error;
> +}
> +
> +if ((mdevsrc->model = virMediatedDeviceModelTypeFromString(model)) 
> <= 0) {
> +virReportError(VIR_ERR_XML_ERROR,
> +   _("unknown hostdev model '%s'"),
> +   model);
> +goto error;
> +}

This condition means that "VIR_MDEV_MODEL_TYPE_DEFAULT" is also unknown
which is not correct, however your code later on depends on this check because
in function virDomainHostdevSubsysMediatedDevDefParseXML you require that the
model must by > 0.  This should be handled by postParse code to check whether
the model has a correct value.

If you want to check whether model attribute was provided you shouldn't do
it by the parsed value but whether the attribute was present in the XML.

The rest of the patch looks good.

Pavel

> +}
> +
>  switch (def->source.subsys.type) {
>  case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
>  if (virDomainHostdevSubsysPCIDefParseXML(sourcenode, def, flags) < 0)
> @@ -6464,6 +6488,8 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
>  if (virDomainHostdevSubsysSCSIVHostDefParseXML(sourcenode, def) < 0)
>  goto error;
>  break;
> +case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> +break;
>  
>  default:
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -13297,6 +13323,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;
>  }
> @@ -14188,6 +14215,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;
>  }
> @@ -23121,6 +23149,7 @@ 

[libvirt] [RFC PATCH v2 REBASE 02/18] conf: Introduce new hostdev device type mdev

[Erik Skultety]

A mediated device will be identified by a UUID 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.

Signed-off-by: Erik Skultety 
---
 src/conf/domain_conf.c  | 36 +++-
 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 +
 8 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a56ea82..83aa15f 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
 
@@ -649,7 +650,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,
@@ -668,6 +670,11 @@ VIR_ENUM_IMPL(virDomainHostdevSubsysSCSIHostProtocol,
   "none",
   "vhost")
 
+VIR_ENUM_IMPL(virMediatedDeviceModel,
+  VIR_MDEV_MODEL_TYPE_LAST,
+  "default",
+  "vfio-pci")
+
 VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST,
   "storage",
   "misc",
@@ -6349,10 +6356,12 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 char *sgio = NULL;
 char *rawio = NULL;
 char *backendStr = NULL;
+char *model = NULL;
 int backend;
 int ret = -1;
 virDomainHostdevSubsysPCIPtr pcisrc = >source.subsys.u.pci;
 virDomainHostdevSubsysSCSIPtr scsisrc = >source.subsys.u.scsi;
+virDomainHostdevSubsysMediatedDevPtr mdevsrc = >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
@@ -6432,6 +6441,21 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 }
 }
 
+if (model) {
+if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("model is only supported with mediated devices"));
+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)
@@ -6464,6 +6488,8 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 if (virDomainHostdevSubsysSCSIVHostDefParseXML(sourcenode, def) < 0)
 goto error;
 break;
+case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
+break;
 
 default:
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -13297,6 +13323,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;
 }
@@ -14188,6 +14215,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;
 }
@@ -23121,6 +23149,7 @@ virDomainHostdevDefFormat(virBufferPtr buf,
 {
 const char *mode = virDomainHostdevModeTypeToString(def->mode);
 virDomainHostdevSubsysSCSIPtr scsisrc = >source.subsys.u.scsi;
+virDomainHostdevSubsysMediatedDevPtr mdevsrc = >source.subsys.u.mdev;
 const char *type;
 
 if (!mode) {
@@ -23170,6 +23199,11 @@ virDomainHostdevDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, " rawio='%s'",
   virTristateBoolTypeToString(scsisrc->rawio));
 }
+
+if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV &&
+mdevsrc->model)
+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 7e1afa4..e1a7a36 100644
---