[PATCH 8/8] src: drop some forward declarations in src/storage/storage_backend_sheepdog.c

[peili]

---
 src/storage/storage_backend_sheepdog.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/src/storage/storage_backend_sheepdog.c 
b/src/storage/storage_backend_sheepdog.c
index 8c37947308..010e86aa14 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -35,12 +35,6 @@
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
-static int virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool,
-   virStorageVolDefPtr vol);
-
-void virStorageBackendSheepdogAddHostArg(virCommandPtr cmd,
- virStoragePoolObjPtr pool);
-
 int
 virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool,
char *output)
-- 
2.20.1

[PATCH] src: drop some forward declarations in src/storage/storage_backend_sheepdog.c

[peili]

---
 src/storage/storage_backend_sheepdog.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/src/storage/storage_backend_sheepdog.c 
b/src/storage/storage_backend_sheepdog.c
index 8c37947308..010e86aa14 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -35,12 +35,6 @@
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
-static int virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool,
-   virStorageVolDefPtr vol);
-
-void virStorageBackendSheepdogAddHostArg(virCommandPtr cmd,
- virStoragePoolObjPtr pool);
-
 int
 virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool,
char *output)
-- 
2.20.1

Re: [libvirt PATCH 2/2] ci: Call meson consistently

[Pavel Hrdina]

On Fri, Mar 26, 2021 at 05:11:04PM +0100, Andrea Bolognani wrote:
> On Fri, 2021-03-26 at 17:00 +0100, Erik Skultety wrote:
> > On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
> > > We should always pass --werror and display the contents of the
> > > log file in case of failure.
> > > 
> > > Signed-off-by: Andrea Bolognani 
> > > ---
> > Reviewed-by: Erik Skultety 
> 
> Even though you didn't spell that out explicitly, I assume you're
> okay with me pushing this series during the freeze?

I know it's trivial and most likely will not affect the release at all
but my understanding of freeze is that we try to stabilize the build
by fixing bugs or any issues that would prevent us to deliver the final
release.

To me that sounds like reasonable definition that we should try to
follow and in my mind creates a clear decision process if something
should/can be pushed during freeze.

Based on that definition I would say that these patches does not
qualify and can easily wait once the release is done.

Pavel


signature.asc
Description: PGP signature

Re: [PATCH] qemu: virtiofs: support

[Cole Robinson]

On 3/26/21 11:53 AM, Peter Krempa wrote:
> On Fri, Mar 26, 2021 at 11:37:48 -0400, Cole Robinson wrote:
>> Add a new XML element
>>
>> 
>>   
>> 
>>   
>> 
>>
>> Which maps to `virtiofsd -o sandbox=chroot|namespace`, which was added
>> in qemu 5.2.0:
>>
>> https://git.qemu.org/?p=qemu.git;a=commit;h=06844584b62a43384642f7243b0fc01c9fff0fc7
>>
>> Signed-off-by: Cole Robinson 
>> ---
>>  docs/formatdomain.rst |  4 
>>  docs/schemas/domaincommon.rng | 12 ++
>>  src/conf/domain_conf.c| 23 +++
>>  src/conf/domain_conf.h| 10 
>>  src/libvirt_private.syms  |  1 +
>>  src/qemu/qemu_virtiofs.c  |  2 ++
>>  .../vhost-user-fs-fd-memory.xml   |  1 +
>>  7 files changed, 53 insertions(+)
> 
> Please split the commit as it's usual for libvirt patches.
> 

Okay, fixed in v2. I addressed the docs and validation piece in v2 too

> Also a test case modifying any of the .args files in qemuxml2argv test
> is missing.
> 

This option affects the virtiofsd command line only, so it won't be
reflected in .args files

Thanks,
Cole

[PATCH v2 2/2] qemu: virtiofs: support

[Cole Robinson]

This maps to `virtiofsd -o sandbox=chroot|namespace`, which was added
in qemu 5.2.0:

https://git.qemu.org/?p=qemu.git;a=commit;h=06844584b62a43384642f7243b0fc01c9fff0fc7

Signed-off-by: Cole Robinson 
---
 src/qemu/qemu_validate.c | 7 +++
 src/qemu/qemu_virtiofs.c | 2 ++
 2 files changed, 9 insertions(+)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 6043f974ce..b272ab0087 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4081,6 +4081,13 @@ qemuValidateDomainDeviceDefFS(virDomainFSDefPtr fs,
 }
 }
 
+if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS &&
+fs->sandbox != VIR_DOMAIN_FS_SANDBOX_MODE_DEFAULT) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("sandbox can only be used with driver=virtiofs"));
+return -1;
+}
+
 switch ((virDomainFSDriverType) fs->fsdriver) {
 case VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT:
 case VIR_DOMAIN_FS_DRIVER_TYPE_PATH:
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 2e239cad66..988b757d6f 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -131,6 +131,8 @@ qemuVirtioFSBuildCommandLine(virQEMUDriverConfigPtr cfg,
 virQEMUBuildBufferEscapeComma(, fs->src->path);
 if (fs->cache)
 virBufferAsprintf(, ",cache=%s", 
virDomainFSCacheModeTypeToString(fs->cache));
+if (fs->sandbox)
+virBufferAsprintf(, ",sandbox=%s", 
virDomainFSSandboxModeTypeToString(fs->sandbox));
 
 if (fs->xattr == VIR_TRISTATE_SWITCH_ON)
 virBufferAddLit(, ",xattr");
-- 
2.30.2

[PATCH v2 1/2] conf: Introduce for

[Cole Robinson]

This adds a new XML element


  

  


This will be used by qemu virtiofs

Signed-off-by: Cole Robinson 
---
 docs/formatdomain.rst |  6 +
 docs/schemas/domaincommon.rng | 12 ++
 src/conf/domain_conf.c| 23 +++
 src/conf/domain_conf.h| 10 
 src/libvirt_private.syms  |  1 +
 .../vhost-user-fs-fd-memory.xml   |  1 +
 6 files changed, 53 insertions(+)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 9392c80113..42217a4005 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3234,6 +3234,7 @@ A directory on the host that can be accessed directly 
from the guest.
  
  
 
+
 
  
  
@@ -3358,6 +3359,11 @@ A directory on the host that can be accessed directly 
from the guest.
``cache`` element, possible ``mode`` values being ``none`` and ``always``.
Locking can be controlled via the ``lock`` element - attributes ``posix`` 
and
``flock`` both accepting values ``on`` or ``off``. ( :since:`Since 6.2.0` )
+   The sandboxing method used by virtiofsd can be configured with the 
``sandbox``
+   element, possible ``mode`` values being ``namespace`` and
+   ``chroot``, see the
+   `virtiofsd documentation 
`__
+   for more details. ( :since:`Since 7.2.0` )
 ``source``
The resource on the host that is being accessed in the guest. The ``name``
attribute must be used with ``type='template'``, and the ``dir`` attribute
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 1dbfc68f18..6404ebf210 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2960,6 +2960,18 @@
 
   
 
+
+  
+
+  
+
+  namespace
+  chroot
+
+  
+
+  
+
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b0eba9f7bd..70a900ee25 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -538,6 +538,13 @@ VIR_ENUM_IMPL(virDomainFSCacheMode,
   "always",
 );
 
+VIR_ENUM_IMPL(virDomainFSSandboxMode,
+  VIR_DOMAIN_FS_SANDBOX_MODE_LAST,
+  "default",
+  "namespace",
+  "chroot",
+);
+
 
 VIR_ENUM_IMPL(virDomainNet,
   VIR_DOMAIN_NET_TYPE_LAST,
@@ -10373,6 +10380,7 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
 g_autofree char *binary = virXPathString("string(./binary/@path)", 
ctxt);
 g_autofree char *xattr = virXPathString("string(./binary/@xattr)", 
ctxt);
 g_autofree char *cache = 
virXPathString("string(./binary/cache/@mode)", ctxt);
+g_autofree char *sandbox = 
virXPathString("string(./binary/sandbox/@mode)", ctxt);
 g_autofree char *posix_lock = 
virXPathString("string(./binary/lock/@posix)", ctxt);
 g_autofree char *flock = 
virXPathString("string(./binary/lock/@flock)", ctxt);
 int val;
@@ -10406,6 +10414,16 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
 def->cache = val;
 }
 
+if (sandbox) {
+if ((val = virDomainFSSandboxModeTypeFromString(sandbox)) <= 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("cannot parse sandbox mode '%s' for 
virtiofs"),
+   sandbox);
+goto error;
+}
+def->sandbox = val;
+}
+
 if (posix_lock) {
 if ((val = virTristateSwitchTypeFromString(posix_lock)) <= 0) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -25483,6 +25501,11 @@ virDomainFSDefFormat(virBufferPtr buf,
   virDomainFSCacheModeTypeToString(def->cache));
 }
 
+if (def->sandbox != VIR_DOMAIN_FS_SANDBOX_MODE_DEFAULT) {
+virBufferAsprintf(, "\n",
+  
virDomainFSSandboxModeTypeToString(def->sandbox));
+}
+
 if (def->posix_lock != VIR_TRISTATE_SWITCH_ABSENT) {
 virBufferAsprintf(, " posix='%s'",
   virTristateSwitchTypeToString(def->posix_lock));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0b8895bbdf..d77b04847b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -846,6 +846,14 @@ typedef enum {
 VIR_DOMAIN_FS_CACHE_MODE_LAST
 } virDomainFSCacheMode;
 
+typedef enum {
+VIR_DOMAIN_FS_SANDBOX_MODE_DEFAULT = 0,
+VIR_DOMAIN_FS_SANDBOX_MODE_NAMESPACE,
+VIR_DOMAIN_FS_SANDBOX_MODE_CHROOT,
+
+VIR_DOMAIN_FS_SANDBOX_MODE_LAST
+} virDomainFSSandboxMode;
+
 struct _virDomainFSDef {
 int type;
 int fsdriver; /* enum 

[PATCH v2 0/2] qemu: virtiofs: support

[Cole Robinson]

Add support for virtiofsd -o sandbox=chroot|namespace

v2:
  Add link to virtiofsd docs in libvirt docs
  validate the new field for qemu
  break up the patch

Cole Robinson (2):
  conf: Introduce  for 
  qemu: virtiofs: support 

 docs/formatdomain.rst |  6 +
 docs/schemas/domaincommon.rng | 12 ++
 src/conf/domain_conf.c| 23 +++
 src/conf/domain_conf.h| 10 
 src/libvirt_private.syms  |  1 +
 src/qemu/qemu_validate.c  |  7 ++
 src/qemu/qemu_virtiofs.c  |  2 ++
 .../vhost-user-fs-fd-memory.xml   |  1 +
 8 files changed, 62 insertions(+)

-- 
2.30.2

[libvirt PATCH v6 12/30] nodedev: add helper functions to remove node devices

[Jonathon Jongsma]

When a mediated device is stopped or undefined by an application outside
of libvirt, we need to remove it from our list of node devices within
libvirt. This patch introduces virNodeDeviceObjListRemoveLocked() and
virNodeDeviceObjListForEachRemove() (which are analogous to other types
of object lists in libvirt) to facilitate that. They will be used in
coming commits.

Signed-off-by: Jonathon Jongsma 
---
 src/conf/virnodedeviceobj.c | 58 ++---
 src/conf/virnodedeviceobj.h | 11 +++
 src/libvirt_private.syms|  2 ++
 3 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index ce84e4d8c1..97e7d7ab11 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -507,23 +507,29 @@ void
 virNodeDeviceObjListRemove(virNodeDeviceObjListPtr devs,
virNodeDeviceObjPtr obj)
 {
-virNodeDeviceDefPtr def;
-
 if (!obj)
 return;
-def = obj->def;
 
 virObjectRef(obj);
 virObjectUnlock(obj);
 virObjectRWLockWrite(devs);
 virObjectLock(obj);
-virHashRemoveEntry(devs->objs, def->name);
+virNodeDeviceObjListRemoveLocked(devs, obj);
 virObjectUnlock(obj);
 virObjectUnref(obj);
 virObjectRWUnlock(devs);
 }
 
 
+/* The caller must hold lock on 'devs' */
+void
+virNodeDeviceObjListRemoveLocked(virNodeDeviceObjList *devs,
+ virNodeDeviceObj *dev)
+{
+virHashRemoveEntry(devs->objs, dev->def->name);
+}
+
+
 /*
  * Return the NPIV dev's parent device name
  */
@@ -1019,3 +1025,47 @@ virNodeDeviceObjSetPersistent(virNodeDeviceObj *obj,
 {
 obj->persistent = persistent;
 }
+
+
+struct virNodeDeviceObjListRemoveData
+{
+virNodeDeviceObjListRemoveIter iter;
+void *opaque;
+};
+
+static int virNodeDeviceObjListRemoveCb(void *key G_GNUC_UNUSED,
+void *value,
+void *opaque)
+{
+struct virNodeDeviceObjListRemoveData *data = opaque;
+
+return data->iter(value, data->opaque);
+}
+
+
+/**
+ * virNodeDeviceObjListForEachRemove
+ * @devs: Pointer to object list
+ * @iter: function to call for each device object
+ * @opaque: Opaque data to use as argument to helper
+ *
+ * For each object in @devs, call the @iter helper using @opaque as
+ * an argument. If @iter returns true, that item will be removed from the
+ * object list.
+ */
+void
+virNodeDeviceObjListForEachRemove(virNodeDeviceObjList *devs,
+  virNodeDeviceObjListRemoveIter iter,
+  void *opaque)
+{
+struct virNodeDeviceObjListRemoveData data = {
+.iter = iter,
+.opaque = opaque
+};
+
+virObjectRWLockWrite(devs);
+g_hash_table_foreach_remove(devs->objs,
+virNodeDeviceObjListRemoveCb,
+);
+virObjectRWUnlock(devs);
+}
diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h
index 7f682b9dca..249115cf68 100644
--- a/src/conf/virnodedeviceobj.h
+++ b/src/conf/virnodedeviceobj.h
@@ -80,6 +80,10 @@ void
 virNodeDeviceObjListRemove(virNodeDeviceObjListPtr devs,
virNodeDeviceObjPtr dev);
 
+void
+virNodeDeviceObjListRemoveLocked(virNodeDeviceObjList *devs,
+ virNodeDeviceObj *dev);
+
 int
 virNodeDeviceObjListGetParentHost(virNodeDeviceObjListPtr devs,
   virNodeDeviceDefPtr def);
@@ -134,3 +138,10 @@ virNodeDeviceObjIsPersistent(virNodeDeviceObj *obj);
 void
 virNodeDeviceObjSetPersistent(virNodeDeviceObj *obj,
   bool persistent);
+
+typedef bool (*virNodeDeviceObjListRemoveIter)(virNodeDeviceObj *obj,
+   const void *opaque);
+
+void virNodeDeviceObjListForEachRemove(virNodeDeviceObjList *devs,
+   virNodeDeviceObjListRemoveIter iter,
+   void *opaque);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 047314ec19..f36400b5f6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1280,12 +1280,14 @@ virNodeDeviceObjListFindByName;
 virNodeDeviceObjListFindBySysfsPath;
 virNodeDeviceObjListFindMediatedDeviceByUUID;
 virNodeDeviceObjListFindSCSIHostByWWNs;
+virNodeDeviceObjListForEachRemove;
 virNodeDeviceObjListFree;
 virNodeDeviceObjListGetNames;
 virNodeDeviceObjListGetParentHost;
 virNodeDeviceObjListNew;
 virNodeDeviceObjListNumOfDevices;
 virNodeDeviceObjListRemove;
+virNodeDeviceObjListRemoveLocked;
 virNodeDeviceObjSetActive;
 virNodeDeviceObjSetPersistent;
 
-- 
2.26.3

[libvirt PATCH v6 30/30] nodedev: avoid delay when defining a new mdev

[Jonathon Jongsma]

When calling virNodeDeviceDefineXML() to define a new mediated device,
we call virMdevctlDefine() and then wait for the new device to appear in
the driver's device list before returning. This caused long delays due
to the behavior of nodeDeviceFindNewMediatedDevice(). This function
checks to see if the device is in the list and then waits for 5s before
checking again.

Because mdevctl is relatively slow to query the list of defined
devices[0], the newly-defined device was generally not in the device
list when we first checked. This results in libvirt almost always taking
at least 5s to complete this API call for mediated devices, which is
unacceptable.

In order to avoid this long delay, we resort to a workaround. If the
call to virMdevctlDefine() was successful, we can assume that this new
device will exist the next time we query mdevctl for new devices. So we
simply add this provisional device definition directly to the nodedev
driver's device list and return from the function. At some point in the
future, the mdevctl handler will run and the "official" device will be
processed, which will update the provisional device if any new details
need to be added.

The reason that this is not necessary for virNodeDeviceCreateXML() is
because detecting newly-created (not defined) mdevs happens through
udev instead of mdevctl. And nodeDeviceFindNewMediatedDevice() always
calls 'udevadm settle' before checking to see whether the device is in
the list. This allows us to wait just long enough for all udev events to
be processed, so the device is almost always in the list the first time
we check and so we almost never end up hitting the 5s sleep.

[0] on my machine, 'mdevctl list --defined' took around 0.8s to
complete for only 3 defined mdevs.
---
 src/node_device/node_device_driver.c | 126 +++
 1 file changed, 68 insertions(+), 58 deletions(-)

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 35db24817a..a1b79d93f7 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1215,16 +1215,70 @@ nodeDeviceDestroy(virNodeDevicePtr device)
 return ret;
 }
 
+
+/* takes ownership of @def and potentially frees it. @def should not be used
+ * after returning from this function */
+static int
+nodeDeviceUpdateMediatedDevice(virNodeDeviceDef *def)
+{
+virNodeDeviceObj *obj;
+virObjectEvent *event;
+bool defined = false;
+g_autoptr(virNodeDeviceDef) owned = def;
+g_autofree char *name = g_strdup(owned->name);
+
+owned->driver = g_strdup("vfio_mdev");
+
+if (!(obj = virNodeDeviceObjListFindByName(driver->devs, owned->name))) {
+virNodeDeviceDef *d = g_steal_pointer();
+if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, d))) {
+virNodeDeviceDefFree(d);
+return -1;
+}
+} else {
+bool changed;
+virNodeDeviceDef *olddef = virNodeDeviceObjGetDef(obj);
+
+defined = virNodeDeviceObjIsPersistent(obj);
+/* Active devices contain some additional information (e.g. sysfs
+ * path) that is not provided by mdevctl, so re-use the existing
+ * definition and copy over new mdev data */
+changed = nodeDeviceDefCopyFromMdevctl(olddef, owned);
+
+if (defined && !changed) {
+/* if this device was already defined and the definition
+ * hasn't changed, there's nothing to do for this device */
+virNodeDeviceObjEndAPI();
+return 0;
+}
+}
+
+/* all devices returned by virMdevctlListDefined() are persistent */
+virNodeDeviceObjSetPersistent(obj, true);
+
+if (!defined)
+event = virNodeDeviceEventLifecycleNew(name,
+   VIR_NODE_DEVICE_EVENT_DEFINED,
+   0);
+else
+event = virNodeDeviceEventUpdateNew(name);
+
+virNodeDeviceObjEndAPI();
+virObjectEventStateQueue(driver->nodeDeviceEventState, event);
+
+return 0;
+}
+
 virNodeDevice*
-nodeDeviceDefineXML(virConnect *conn,
+nodeDeviceDefineXML(virConnectPtr conn,
 const char *xmlDesc,
 unsigned int flags)
 {
 g_autoptr(virNodeDeviceDef) def = NULL;
-virNodeDevice *device = NULL;
 const char *virt_type = NULL;
 g_autofree char *uuid = NULL;
 g_autofree char *errmsg = NULL;
+g_autofree char *name = NULL;
 
 virCheckFlags(0, NULL);
 
@@ -1264,9 +1318,19 @@ nodeDeviceDefineXML(virConnect *conn,
 }
 
 mdevGenerateDeviceName(def);
-device = nodeDeviceFindNewMediatedDevice(conn, def->caps->data.mdev.uuid);
+name = g_strdup(def->name);
+
+/* Normally we would call nodeDeviceFindNewMediatedDevice() here to wait
+ * for the new device to appear. But mdevctl can take a while to query
+ * devices, and if nodeDeviceFindNewMediatedDevice() doesn't find the new
+ * device 

[libvirt PATCH v6 21/30] virsh: Factor out function to find node device

[Jonathon Jongsma]

Several functions accept providing a node device by name or by wwnn,wwpn
pair. Extract the logic to do this into a function that can be used by
both callers.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 tools/virsh-nodedev.c | 62 +++
 1 file changed, 28 insertions(+), 34 deletions(-)

diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 1cbf2e082b..5589a5862d 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -111,23 +111,18 @@ static const vshCmdOptDef opts_node_device_destroy[] = {
 {.name = NULL}
 };
 
-static bool
-cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
+static virNodeDevice*
+vshFindNodeDevice(vshControl *ctl, const char *value)
 {
 virNodeDevicePtr dev = NULL;
-bool ret = false;
-const char *device_value = NULL;
 char **arr = NULL;
 int narr;
 virshControlPtr priv = ctl->privData;
 
-if (vshCommandOptStringReq(ctl, cmd, "device", _value) < 0)
-return false;
-
-if (strchr(device_value, ',')) {
-narr = vshStringToArray(device_value, );
+if (strchr(value, ',')) {
+narr = vshStringToArray(value, );
 if (narr != 2) {
-vshError(ctl, _("Malformed device value '%s'"), device_value);
+vshError(ctl, _("Malformed device value '%s'"), value);
 goto cleanup;
 }
 
@@ -136,14 +131,33 @@ cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
 
 dev = virNodeDeviceLookupSCSIHostByWWN(priv->conn, arr[0], arr[1], 0);
 } else {
-dev = virNodeDeviceLookupByName(priv->conn, device_value);
+dev = virNodeDeviceLookupByName(priv->conn, value);
 }
 
 if (!dev) {
-vshError(ctl, "%s '%s'", _("Could not find matching device"), 
device_value);
+vshError(ctl, "%s '%s'", _("Could not find matching device"), value);
 goto cleanup;
 }
 
+ cleanup:
+g_strfreev(arr);
+return dev;
+}
+
+static bool
+cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
+{
+virNodeDevice *dev = NULL;
+bool ret = false;
+const char *device_value = NULL;
+
+if (vshCommandOptStringReq(ctl, cmd, "device", _value) < 0)
+return false;
+
+dev = vshFindNodeDevice(ctl, device_value);
+if (!dev)
+goto cleanup;
+
 if (virNodeDeviceDestroy(dev) == 0) {
 vshPrintExtra(ctl, _("Destroyed node device '%s'\n"), device_value);
 } else {
@@ -153,7 +167,6 @@ cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
 
 ret = true;
  cleanup:
-g_strfreev(arr);
 if (dev)
 virNodeDeviceFree(dev);
 return ret;
@@ -578,33 +591,15 @@ cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd)
 virNodeDevicePtr device = NULL;
 char *xml = NULL;
 const char *device_value = NULL;
-char **arr = NULL;
-int narr;
 bool ret = false;
-virshControlPtr priv = ctl->privData;
 
 if (vshCommandOptStringReq(ctl, cmd, "device", _value) < 0)
  return false;
 
-if (strchr(device_value, ',')) {
-narr = vshStringToArray(device_value, );
-if (narr != 2) {
-vshError(ctl, _("Malformed device value '%s'"), device_value);
-goto cleanup;
-}
+device = vshFindNodeDevice(ctl, device_value);
 
-if (!virValidateWWN(arr[0]) || !virValidateWWN(arr[1]))
-goto cleanup;
-
-device = virNodeDeviceLookupSCSIHostByWWN(priv->conn, arr[0], arr[1], 
0);
-} else {
-device = virNodeDeviceLookupByName(priv->conn, device_value);
-}
-
-if (!device) {
-vshError(ctl, "%s '%s'", _("Could not find matching device"), 
device_value);
+if (!device)
 goto cleanup;
-}
 
 if (!(xml = virNodeDeviceGetXMLDesc(device, 0)))
 goto cleanup;
@@ -613,7 +608,6 @@ cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd)
 
 ret = true;
  cleanup:
-g_strfreev(arr);
 VIR_FREE(xml);
 if (device)
 virNodeDeviceFree(device);
-- 
2.26.3

[libvirt PATCH v6 25/30] nodedev: add element to mdev caps

[Jonathon Jongsma]

It will be useful to be able to specify a particular UUID for a mediated
device when defining the node device. To accomodate that, allow this to
be specified in the xml schema. This patch also parses and formats that
value to the xml, but does not yet use it.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 docs/schemas/nodedev.rng  | 43 +++
 src/conf/node_device_conf.c   | 14 ++
 .../mdevctl-list-multiple.out.xml |  4 ++
 3 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 5840dc9f0d..777227c38a 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -606,27 +606,34 @@
   
 
   
-
-  mdev
-
-
-  
-
+
+  
+mdev
   
-
-
-  
-
-  
+  
+
+  
 
   
-
-
-  
-
-
-  
-
+  
+
+  
+
+  
+
+  
+  
+
+  
+
+  
+  
+
+  
+  
+
+  
+
   
 
   
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index b1739ceb01..d167da6171 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -527,6 +527,7 @@ virNodeDeviceCapMdevDefFormat(virBufferPtr buf,
 size_t i;
 
 virBufferEscapeString(buf, "\n", data->mdev.type);
+virBufferEscapeString(buf, "%s\n", data->mdev.uuid);
 virBufferAsprintf(buf, "\n",
   data->mdev.iommuGroupNumber);
 
@@ -1948,6 +1949,7 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt,
 int nattrs = 0;
 g_autofree xmlNodePtr *attrs = NULL;
 size_t i;
+g_autofree char *uuidstr = NULL;
 
 ctxt->node = node;
 
@@ -1957,6 +1959,18 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt,
 return -1;
 }
 
+if ((uuidstr = virXPathString("string(./uuid[1])", ctxt))) {
+unsigned char uuidbuf[VIR_UUID_BUFLEN];
+/* make sure that the provided uuid is valid */
+if (virUUIDParse(uuidstr, uuidbuf) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Invalid uuid '%s' for new mdev device"), 
uuidstr);
+return -1;
+}
+mdev->uuid = g_new0(char, VIR_UUID_STRING_BUFLEN);
+virUUIDFormat(uuidbuf, mdev->uuid);
+}
+
 /* 'iommuGroup' is optional, only report an error if the supplied value is
  * invalid (-2), not if it's missing (-1) */
 if (virXPathUInt("number(./iommuGroup[1]/@number)",
diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml 
b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
index 543ad916b7..cf7e966256 100644
--- a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
+++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
@@ -3,6 +3,7 @@
   :00:02.0
   
 
+200f228a-c80a-4d50-bfb7-f5a0e4e34045
 
   
 
@@ -11,6 +12,7 @@
   :00:02.0
   
 
+de807ffc-1923-4d5f-b6c9-b20ecebc6d4b
 
   
 
@@ -19,6 +21,7 @@
   :00:02.0
   
 
+435722ea-5f43-468a-874f-da34f1217f13
 
 
   
@@ -28,6 +31,7 @@
   matrix
   
 
+783e6dbb-ea0e-411f-94e2-717eaad438bf
 
 
 
-- 
2.26.3

[libvirt PATCH v6 26/30] nodedev: add ability to specify UUID for new mdevs

[Jonathon Jongsma]

Use the new  element in the mdev caps to define and start devices
with a specific UUID.

Signed-off-by: Jonathon Jongsma 
---
 src/node_device/node_device_driver.c  | 19 ---
 ...19_36ea_4111_8f0a_8c9a70e21366-define.argv |  3 ++-
 ...019_36ea_4111_8f0a_8c9a70e21366-start.argv |  3 ++-
 ...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml |  1 +
 4 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index cd7677633a..b1b3217afb 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -729,6 +729,10 @@ nodeDeviceGetMdevctlDefineStartCommand(virNodeDeviceDef 
*def,
NULL);
 
 virCommandSetInputBuffer(cmd, json);
+
+if (def->caps->data.mdev.uuid)
+virCommandAddArgPair(cmd, "--uuid", def->caps->data.mdev.uuid);
+
 virCommandSetOutputBuffer(cmd, uuid_out);
 virCommandSetErrorBuffer(cmd, errmsg);
 
@@ -816,7 +820,12 @@ nodeDeviceCreateXMLMdev(virConnectPtr conn,
 return NULL;
 }
 
-return nodeDeviceFindNewMediatedDevice(conn, uuid);
+if (uuid && uuid[0]) {
+g_free(def->caps->data.mdev.uuid);
+def->caps->data.mdev.uuid = g_steal_pointer();
+}
+
+return nodeDeviceFindNewMediatedDevice(conn, def->caps->data.mdev.uuid);
 }
 
 
@@ -1230,9 +1239,13 @@ nodeDeviceDefineXML(virConnect *conn,
 return NULL;
 }
 
-def->caps->data.mdev.uuid = g_strdup(uuid);
+if (uuid && uuid[0]) {
+g_free(def->caps->data.mdev.uuid);
+def->caps->data.mdev.uuid = g_steal_pointer();
+}
+
 mdevGenerateDeviceName(def);
-device = nodeDeviceFindNewMediatedDevice(conn, uuid);
+device = nodeDeviceFindNewMediatedDevice(conn, def->caps->data.mdev.uuid);
 
 return device;
 }
diff --git 
a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-define.argv
 
b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-define.argv
index 773e98b963..118ec7a8da 100644
--- 
a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-define.argv
+++ 
b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-define.argv
@@ -1 +1,2 @@
-$MDEVCTL_BINARY$ define -p :00:02.0 --jsonfile /dev/stdin
+$MDEVCTL_BINARY$ define -p :00:02.0 --jsonfile /dev/stdin \
+--uuid=d069d019-36ea-4111-8f0a-8c9a70e21366
diff --git 
a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv 
b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv
index eb7262035e..129f438e4a 100644
--- 
a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv
+++ 
b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv
@@ -1 +1,2 @@
-$MDEVCTL_BINARY$ start -p :00:02.0 --jsonfile /dev/stdin
+$MDEVCTL_BINARY$ start -p :00:02.0 --jsonfile /dev/stdin \
+--uuid=d069d019-36ea-4111-8f0a-8c9a70e21366
diff --git 
a/tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml 
b/tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml
index d6a2e99edc..605d8f63a1 100644
--- a/tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml
+++ b/tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml
@@ -3,5 +3,6 @@
   pci__00_02_0
   
 
+d069d019-36ea-4111-8f0a-8c9a70e21366
   
 
-- 
2.26.3

[libvirt PATCH v6 23/30] api: add virNodeDeviceCreate()

[Jonathon Jongsma]

This new API function provides a way to start a persistently-defined
mediate device that was defined by virNodeDeviceDefineXML() (or one that
was defined externally via mdevctl)

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 include/libvirt/libvirt-nodedev.h|  2 +
 src/driver-nodedev.h |  4 ++
 src/libvirt-nodedev.c| 35 ++
 src/libvirt_public.syms  |  1 +
 src/node_device/node_device_driver.c | 68 
 src/node_device/node_device_driver.h |  7 ++
 src/node_device/node_device_udev.c   |  1 +
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 13 +++-
 src/remote_protocol-structs  |  4 ++
 tests/nodedevmdevctldata/mdevctl-create.argv |  1 +
 tests/nodedevmdevctltest.c   | 11 +++-
 12 files changed, 146 insertions(+), 2 deletions(-)
 create mode 100644 tests/nodedevmdevctldata/mdevctl-create.argv

diff --git a/include/libvirt/libvirt-nodedev.h 
b/include/libvirt/libvirt-nodedev.h
index 623017f1fd..cf51c3d085 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -137,6 +137,8 @@ virNodeDevicePtr virNodeDeviceDefineXML(virConnectPtr conn,
 
 int virNodeDeviceUndefine(virNodeDevicePtr dev);
 
+int virNodeDeviceCreate(virNodeDevicePtr dev);
+
 /**
  * VIR_NODE_DEVICE_EVENT_CALLBACK:
  *
diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h
index c352462dda..22fe53bfc0 100644
--- a/src/driver-nodedev.h
+++ b/src/driver-nodedev.h
@@ -82,6 +82,9 @@ typedef virNodeDevice*
 typedef int
 (*virDrvNodeDeviceUndefine)(virNodeDevice *dev);
 
+typedef int
+(*virDrvNodeDeviceCreate)(virNodeDevice *def);
+
 typedef int
 (*virDrvConnectNodeDeviceEventRegisterAny)(virConnectPtr conn,
virNodeDevicePtr dev,
@@ -123,4 +126,5 @@ struct _virNodeDeviceDriver {
 virDrvNodeDeviceDestroy nodeDeviceDestroy;
 virDrvNodeDeviceDefineXML nodeDeviceDefineXML;
 virDrvNodeDeviceUndefine nodeDeviceUndefine;
+virDrvNodeDeviceCreate nodeDeviceCreate;
 };
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index 1d397c6610..a1b9e44ab3 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -815,6 +815,41 @@ virNodeDeviceUndefine(virNodeDevice *dev)
 }
 
 
+/**
+ * virNodeDeviceCreate:
+ * @dev: a device object
+ *
+ * Start a defined node device:
+ *
+ * Returns 0 in case of success and -1 in case of failure.
+ */
+int
+virNodeDeviceCreate(virNodeDevice *dev)
+{
+VIR_DEBUG("dev=%p", dev);
+
+virResetLastError();
+
+virCheckNodeDeviceReturn(dev, -1);
+virCheckReadOnlyGoto(dev->conn->flags, error);
+
+if (dev->conn->nodeDeviceDriver &&
+dev->conn->nodeDeviceDriver->nodeDeviceCreate) {
+int retval = dev->conn->nodeDeviceDriver->nodeDeviceCreate(dev);
+if (retval < 0)
+goto error;
+
+return 0;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(dev->conn);
+return -1;
+}
+
+
 /**
  * virConnectNodeDeviceEventRegisterAny:
  * @conn: pointer to the connection
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 99b46587b9..3a8f5d8fc6 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -889,6 +889,7 @@ LIBVIRT_7.2.0 {
 virDomainStartDirtyRateCalc;
 virNodeDeviceDefineXML;
 virNodeDeviceUndefine;
+virNodeDeviceCreate;
 } LIBVIRT_7.1.0;
 
 #  define new API here using predicted next version number 
diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 48b4b1f438..cd7677633a 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -901,6 +901,18 @@ nodeDeviceGetMdevctlUndefineCommand(const char *uuid, char 
**errmsg)
 return cmd;
 }
 
+virCommand*
+nodeDeviceGetMdevctlCreateCommand(const char *uuid, char **errmsg)
+{
+virCommand *cmd = virCommandNewArgList(MDEVCTL,
+   "start",
+   "-u",
+   uuid,
+   NULL);
+virCommandSetErrorBuffer(cmd, errmsg);
+return cmd;
+}
+
 static int
 virMdevctlStop(virNodeDeviceDefPtr def, char **errmsg)
 {
@@ -932,6 +944,21 @@ virMdevctlUndefine(virNodeDeviceDef *def, char **errmsg)
 }
 
 
+static int
+virMdevctlCreate(virNodeDeviceDef *def, char **errmsg)
+{
+int status;
+g_autoptr(virCommand) cmd = NULL;
+
+cmd = nodeDeviceGetMdevctlCreateCommand(def->caps->data.mdev.uuid, errmsg);
+
+if (virCommandRun(cmd, ) < 0 || status != 0)
+return -1;
+
+return 0;
+}
+
+
 virCommand*
 nodeDeviceGetMdevctlListCommand(bool defined,
 char **output)
@@ -1257,6 +1284,47 @@ nodeDeviceUndefine(virNodeDevice *device)
 }
 
 
+int

[libvirt PATCH v6 29/30] nodedev: factor out function to add mediated devices

[Jonathon Jongsma]

To accomodate re-use of this functionality in a following patch, split
out the processing of an individual mdev definition into a separate
function.
---
 src/node_device/node_device_driver.c | 103 +++
 1 file changed, 57 insertions(+), 46 deletions(-)

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 31322e3afb..35db24817a 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1506,6 +1506,60 @@ removeMissingPersistentMdevs(virNodeDeviceObj *obj,
 }
 
 
+/* takes ownership of @def and potentially frees it. @def should not be used
+ * after returning from this function */
+static int
+nodeDeviceUpdateMediatedDevice(virNodeDeviceDef *def)
+{
+virNodeDeviceObj *obj;
+virObjectEvent *event;
+bool defined = false;
+g_autoptr(virNodeDeviceDef) owned = def;
+g_autofree char *name = g_strdup(owned->name);
+
+owned->driver = g_strdup("vfio_mdev");
+
+if (!(obj = virNodeDeviceObjListFindByName(driver->devs, owned->name))) {
+virNodeDeviceDef *d = g_steal_pointer();
+if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, d))) {
+virNodeDeviceDefFree(d);
+return -1;
+}
+} else {
+bool changed;
+virNodeDeviceDef *olddef = virNodeDeviceObjGetDef(obj);
+
+defined = virNodeDeviceObjIsPersistent(obj);
+/* Active devices contain some additional information (e.g. sysfs
+ * path) that is not provided by mdevctl, so re-use the existing
+ * definition and copy over new mdev data */
+changed = nodeDeviceDefCopyFromMdevctl(olddef, owned);
+
+if (defined && !changed) {
+/* if this device was already defined and the definition
+ * hasn't changed, there's nothing to do for this device */
+virNodeDeviceObjEndAPI();
+return 0;
+}
+}
+
+/* all devices returned by virMdevctlListDefined() are persistent */
+virNodeDeviceObjSetPersistent(obj, true);
+
+if (!defined)
+event = virNodeDeviceEventLifecycleNew(name,
+   VIR_NODE_DEVICE_EVENT_DEFINED,
+   0);
+else
+event = virNodeDeviceEventUpdateNew(name);
+
+virNodeDeviceObjEndAPI();
+virObjectEventStateQueue(driver->nodeDeviceEventState, event);
+
+return 0;
+}
+
+
 int
 nodeDeviceUpdateMediatedDevices(void)
 {
@@ -1525,52 +1579,9 @@ nodeDeviceUpdateMediatedDevices(void)
 virNodeDeviceObjListForEachRemove(driver->devs,
   removeMissingPersistentMdevs, );
 
-for (i = 0; i < data.ndefs; i++) {
-virNodeDeviceObj *obj;
-virObjectEvent *event;
-g_autoptr(virNodeDeviceDef) def = defs[i];
-g_autofree char *name = g_strdup(def->name);
-bool defined = false;
-
-def->driver = g_strdup("vfio_mdev");
-
-if (!(obj = virNodeDeviceObjListFindByName(driver->devs, def->name))) {
-virNodeDeviceDef *d = g_steal_pointer();
-if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, d))) {
-virNodeDeviceDefFree(d);
-return -1;
-}
-} else {
-bool changed;
-virNodeDeviceDef *olddef = virNodeDeviceObjGetDef(obj);
-
-defined = virNodeDeviceObjIsPersistent(obj);
-/* Active devices contain some additional information (e.g. sysfs
- * path) that is not provided by mdevctl, so re-use the existing
- * definition and copy over new mdev data */
-changed = nodeDeviceDefCopyFromMdevctl(olddef, def);
-
-if (defined && !changed) {
-/* if this device was already defined and the definition
- * hasn't changed, there's nothing to do for this device */
-virNodeDeviceObjEndAPI();
-continue;
-}
-}
-
-/* all devices returned by virMdevctlListDefined() are persistent */
-virNodeDeviceObjSetPersistent(obj, true);
-
-if (!defined)
-event = virNodeDeviceEventLifecycleNew(name,
-   
VIR_NODE_DEVICE_EVENT_DEFINED,
-   0);
-else
-event = virNodeDeviceEventUpdateNew(name);
-
-virNodeDeviceObjEndAPI();
-virObjectEventStateQueue(driver->nodeDeviceEventState, event);
-}
+for (i = 0; i < data.ndefs; i++)
+if (nodeDeviceUpdateMediatedDevice(defs[i]) < 0)
+return -1;
 
 return 0;
 }
-- 
2.26.3

[libvirt PATCH v6 24/30] virsh: add "nodedev-start" command

[Jonathon Jongsma]

This virsh command maps to virNodeDeviceCreate(), which starts a node
device that has been previously defined by virNodeDeviceDefineXML().
This is only supported for mediated devices.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 tools/virsh-nodedev.c | 57 +++
 1 file changed, 57 insertions(+)

diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index d5b82ba285..e13c15b4b0 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -1112,6 +1112,57 @@ cmdNodeDeviceDefine(vshControl *ctl, const vshCmd *cmd 
G_GNUC_UNUSED)
 }
 
 
+/*
+ * "nodedev-start" command
+ */
+static const vshCmdInfo info_node_device_start[] = {
+{.name = "help",
+ .data = N_("Start an inactive node device")
+},
+{.name = "desc",
+ .data = N_("Starts an inactive node device that was previously defined")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_node_device_start[] = {
+{.name = "device",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("device name"),
+ .completer = virshNodeDeviceNameCompleter,
+},
+{.name = NULL}
+};
+
+static bool
+cmdNodeDeviceStart(vshControl *ctl, const vshCmd *cmd)
+{
+const char *name = NULL;
+virNodeDevice *device;
+bool ret = true;
+virshControl *priv = ctl->privData;
+
+if (vshCommandOptStringReq(ctl, cmd, "device", ) < 0)
+return false;
+
+if (!(device = virNodeDeviceLookupByName(priv->conn, name))) {
+vshError(ctl, _("Could not find matching device '%s'"), name);
+return false;
+}
+
+if (virNodeDeviceCreate(device) == 0) {
+vshPrintExtra(ctl, _("Device %s started\n"), name);
+} else {
+vshError(ctl, _("Failed to start device %s"), name);
+ret = false;
+}
+
+virNodeDeviceFree(device);
+return ret;
+}
+
+
 const vshCmdDef nodedevCmds[] = {
 {.name = "nodedev-create",
  .handler = cmdNodeDeviceCreate,
@@ -1177,5 +1228,11 @@ const vshCmdDef nodedevCmds[] = {
  .info = info_node_device_undefine,
  .flags = 0
 },
+{.name = "nodedev-start",
+ .handler = cmdNodeDeviceStart,
+ .opts = opts_node_device_start,
+ .info = info_node_device_start,
+ .flags = 0
+},
 {.name = NULL}
 };
-- 
2.26.3

[libvirt PATCH v6 22/30] virsh: add nodedev-undefine command

[Jonathon Jongsma]

Add a virsh command that maps to virNodeDeviceUndefine().

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 tools/virsh-nodedev.c | 59 +++
 1 file changed, 59 insertions(+)

diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 5589a5862d..d5b82ba285 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -1007,6 +1007,59 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd)
 }
 
 
+/*
+ * "nodedev-undefine" command
+ */
+static const vshCmdInfo info_node_device_undefine[] = {
+{.name = "help",
+ .data = N_("Undefine an inactive node device")
+},
+{.name = "desc",
+ .data = N_("Undefines the configuration for an inactive node device")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_node_device_undefine[] = {
+{.name = "device",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("device name or wwn pair in 'wwnn,wwpn' format"),
+ .completer = virshNodeDeviceNameCompleter,
+},
+{.name = NULL}
+};
+
+static bool
+cmdNodeDeviceUndefine(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
+{
+virNodeDevice *dev = NULL;
+bool ret = false;
+const char *device_value = NULL;
+
+if (vshCommandOptStringReq(ctl, cmd, "device", _value) < 0)
+return false;
+
+dev = vshFindNodeDevice(ctl, device_value);
+
+if (!dev)
+goto cleanup;
+
+if (virNodeDeviceUndefine(dev) == 0) {
+vshPrintExtra(ctl, _("Undefined node device '%s'\n"), device_value);
+} else {
+vshError(ctl, _("Failed to undefine node device '%s'"), device_value);
+goto cleanup;
+}
+
+ret = true;
+ cleanup:
+if (dev)
+virNodeDeviceFree(dev);
+return ret;
+}
+
+
 /*
  * "nodedev-define" command
  */
@@ -1118,5 +1171,11 @@ const vshCmdDef nodedevCmds[] = {
  .info = info_node_device_define,
  .flags = 0
 },
+{.name = "nodedev-undefine",
+ .handler = cmdNodeDeviceUndefine,
+ .opts = opts_node_device_undefine,
+ .info = info_node_device_undefine,
+ .flags = 0
+},
 {.name = NULL}
 };
-- 
2.26.3

[libvirt PATCH v6 28/30] nodedev: add docs about mdev attribute order

[Jonathon Jongsma]

Mention that mdev attribute order is significant.

Signed-off-by: Jonathon Jongsma 
---
 docs/formatnode.html.in | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index 5c7286df8a..c58cd01395 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -379,7 +379,10 @@
 This optional element can occur multiple times. It represents a
 vendor-specific attribute that is used to configure this
 mediated device. It has two required attributes:
-name and value.
+name and value. Note that the order
+in which attributes are set may be important for some devices.
+The order that they appear in the xml definition determines the
+order that they will be written to the device.
   
 
   
-- 
2.26.3

[libvirt PATCH v6 17/30] virsh: Add --inactive, --all to nodedev-list

[Jonathon Jongsma]

Now that we can filter active and inactive node devices in
virConnectListAllNodeDevices(), add these switches to the virsh command.

Eventual output (once everything is hooked up):

virsh # nodedev-list --cap mdev
mdev_bd2ea955_3402_4252_8c17_7468083a0f26

virsh # nodedev-list --inactive --cap mdev
mdev_07d8b8b0_7e04_4c0f_97ed_9214ce12723c
mdev_927c040f_ae7d_4a35_966e_286ba6ebbe1c

virsh # nodedev-list --all --cap mdev
mdev_07d8b8b0_7e04_4c0f_97ed_9214ce12723c
mdev_927c040f_ae7d_4a35_966e_286ba6ebbe1c
mdev_bd2ea955_3402_4252_8c17_7468083a0f26

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 tools/virsh-nodedev.c | 29 +
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index b9fe9b8be1..7ab5b264fc 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -378,6 +378,14 @@ static const vshCmdOptDef opts_node_list_devices[] = {
  .completer = virshNodeDeviceCapabilityNameCompleter,
  .help = N_("capability names, separated by comma")
 },
+{.name = "inactive",
+ .type = VSH_OT_BOOL,
+ .help = N_("list inactive devices")
+},
+{.name = "all",
+ .type = VSH_OT_BOOL,
+ .help = N_("list inactive & active devices")
+},
 {.name = NULL}
 };
 
@@ -393,18 +401,26 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd 
G_GNUC_UNUSED)
 int ncaps = 0;
 virshNodeDeviceListPtr list = NULL;
 int cap_type = -1;
+bool inactive = vshCommandOptBool(cmd, "inactive");
+bool all = vshCommandOptBool(cmd, "all");
 
 ignore_value(vshCommandOptStringQuiet(ctl, cmd, "cap", _str));
 
 if (cap_str) {
-if (tree) {
-vshError(ctl, "%s", _("Options --tree and --cap are 
incompatible"));
-return false;
-}
 if ((ncaps = vshStringToArray(cap_str, )) < 0)
 return false;
 }
 
+if (all && inactive) {
+vshError(ctl, "%s", _("Option --all is incompatible with --inactive"));
+return false;
+}
+
+if (tree && (cap_str || inactive || all)) {
+vshError(ctl, "%s", _("Option --tree is incompatible with other 
options"));
+return false;
+}
+
 for (i = 0; i < ncaps; i++) {
 if ((cap_type = virNodeDevCapTypeFromString(caps[i])) < 0) {
 vshError(ctl, "%s", _("Invalid capability type"));
@@ -481,6 +497,11 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd 
G_GNUC_UNUSED)
 }
 }
 
+if (inactive || all)
+flags |= VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE;
+if (!inactive)
+flags |= VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE;
+
 if (!(list = virshNodeDeviceListCollect(ctl, caps, ncaps, flags))) {
 ret = false;
 goto cleanup;
-- 
2.26.3

[libvirt PATCH v6 20/30] api: add virNodeDeviceUndefine()

[Jonathon Jongsma]

This interface allows you to undefine a persistently defined (but
inactive) mediated devices. It is implemented via 'mdevctl'

Signed-off-by: Jonathon Jongsma 
---
 include/libvirt/libvirt-nodedev.h |  2 +
 src/access/viraccessperm.c|  2 +-
 src/access/viraccessperm.h|  6 ++
 src/driver-nodedev.h  |  4 +
 src/libvirt-nodedev.c | 36 +
 src/libvirt_public.syms   |  1 +
 src/node_device/node_device_driver.c  | 73 +++
 src/node_device/node_device_driver.h  |  7 ++
 src/node_device/node_device_udev.c|  1 +
 src/remote/remote_driver.c|  1 +
 src/remote/remote_protocol.x  | 14 +++-
 src/remote_protocol-structs   |  4 +
 .../nodedevmdevctldata/mdevctl-undefine.argv  |  1 +
 tests/nodedevmdevctltest.c|  8 ++
 14 files changed, 158 insertions(+), 2 deletions(-)
 create mode 100644 tests/nodedevmdevctldata/mdevctl-undefine.argv

diff --git a/include/libvirt/libvirt-nodedev.h 
b/include/libvirt/libvirt-nodedev.h
index 33eb46b3cd..623017f1fd 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -135,6 +135,8 @@ virNodeDevicePtr virNodeDeviceDefineXML(virConnectPtr conn,
 const char *xmlDesc,
 unsigned int flags);
 
+int virNodeDeviceUndefine(virNodeDevicePtr dev);
+
 /**
  * VIR_NODE_DEVICE_EVENT_CALLBACK:
  *
diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c
index 33db7752b6..d4a0c98b9b 100644
--- a/src/access/viraccessperm.c
+++ b/src/access/viraccessperm.c
@@ -70,7 +70,7 @@ VIR_ENUM_IMPL(virAccessPermNodeDevice,
   VIR_ACCESS_PERM_NODE_DEVICE_LAST,
   "getattr", "read", "write",
   "start", "stop",
-  "detach",
+  "detach", "delete",
 );
 
 VIR_ENUM_IMPL(virAccessPermNWFilter,
diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h
index 42996b9741..051246a7b6 100644
--- a/src/access/viraccessperm.h
+++ b/src/access/viraccessperm.h
@@ -500,6 +500,12 @@ typedef enum {
  */
 VIR_ACCESS_PERM_NODE_DEVICE_DETACH,
 
+/**
+ * @desc: Delete node device
+ * @message: Deleting node device driver requires authorization
+ */
+VIR_ACCESS_PERM_NODE_DEVICE_DELETE,
+
 VIR_ACCESS_PERM_NODE_DEVICE_LAST
 } virAccessPermNodeDevice;
 
diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h
index 64a0a7c473..c352462dda 100644
--- a/src/driver-nodedev.h
+++ b/src/driver-nodedev.h
@@ -79,6 +79,9 @@ typedef virNodeDevice*
  const char *xmlDesc,
  unsigned int flags);
 
+typedef int
+(*virDrvNodeDeviceUndefine)(virNodeDevice *dev);
+
 typedef int
 (*virDrvConnectNodeDeviceEventRegisterAny)(virConnectPtr conn,
virNodeDevicePtr dev,
@@ -119,4 +122,5 @@ struct _virNodeDeviceDriver {
 virDrvNodeDeviceCreateXML nodeDeviceCreateXML;
 virDrvNodeDeviceDestroy nodeDeviceDestroy;
 virDrvNodeDeviceDefineXML nodeDeviceDefineXML;
+virDrvNodeDeviceUndefine nodeDeviceUndefine;
 };
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index cfc0c9de5b..1d397c6610 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -779,6 +779,42 @@ virNodeDeviceDefineXML(virConnect *conn,
 }
 
 
+/**
+ * virNodeDeviceUndefine:
+ * @dev: a device object
+ *
+ * Undefine the device object. The virtual device  is removed from the host
+ * operating system.  This function may require privileged access.
+ *
+ * Returns 0 in case of success and -1 in case of failure.
+ */
+int
+virNodeDeviceUndefine(virNodeDevice *dev)
+{
+VIR_DEBUG("dev=%p", dev);
+
+virResetLastError();
+
+virCheckNodeDeviceReturn(dev, -1);
+virCheckReadOnlyGoto(dev->conn->flags, error);
+
+if (dev->conn->nodeDeviceDriver &&
+dev->conn->nodeDeviceDriver->nodeDeviceUndefine) {
+int retval = dev->conn->nodeDeviceDriver->nodeDeviceUndefine(dev);
+if (retval < 0)
+goto error;
+
+return 0;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(dev->conn);
+return -1;
+}
+
+
 /**
  * virConnectNodeDeviceEventRegisterAny:
  * @conn: pointer to the connection
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 3d8176351c..99b46587b9 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -888,6 +888,7 @@ LIBVIRT_7.2.0 {
 global:
 virDomainStartDirtyRateCalc;
 virNodeDeviceDefineXML;
+virNodeDeviceUndefine;
 } LIBVIRT_7.1.0;
 
 #  define new API here using predicted next version number 
diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 418faa9fb9..48b4b1f438 100644
--- a/src/node_device/node_device_driver.c
+++ 

[libvirt PATCH v6 11/30] nodedev: add mdevctl devices to node device list

[Jonathon Jongsma]

At startup, query devices that are defined by 'mdevctl' and add them to
the node device list.

This adds a complication: we now have two potential sources of
information for a node device:
 - udev for all devices and for activated mediated devices
 - mdevctl for persistent mediated devices

Unfortunately, neither backend returns full information for a mediated
device. For example, if a persistent mediated device in the list (with
information provided from mdevctl) is 'started', that same device will
now be detected by udev. If we simply overwrite the existing device
definition with the new one provided by the udev backend, we will lose
extra information that was provided by mdevctl (e.g. attributes, etc).
To avoid this, make sure to copy the extra information into the new
device definition.

Signed-off-by: Jonathon Jongsma 
---
 src/node_device/node_device_driver.c | 153 +++
 src/node_device/node_device_driver.h |   6 ++
 src/node_device/node_device_udev.c   |  31 +-
 3 files changed, 185 insertions(+), 5 deletions(-)

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 6911fa36da..efa524f317 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1164,3 +1164,156 @@ nodeDeviceGenerateName(virNodeDeviceDef *def,
 *(def->name + i) = '_';
 }
 }
+
+
+static int
+virMdevctlListDefined(virNodeDeviceDef ***devs)
+{
+int status;
+g_autofree char *output = NULL;
+g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlListCommand(true, );
+
+if (virCommandRun(cmd, ) < 0 || status != 0)
+return -1;
+
+if (!output)
+return -1;
+
+return nodeDeviceParseMdevctlJSON(output, devs);
+}
+
+
+int
+nodeDeviceUpdateMediatedDevices(void)
+{
+g_autofree virNodeDeviceDef **defs = NULL;
+int ndefs;
+size_t i;
+
+if ((ndefs = virMdevctlListDefined()) < 0) {
+virReportSystemError(errno, "%s",
+ _("failed to query mdevs from mdevctl"));
+return -1;
+}
+
+for (i = 0; i < ndefs; i++) {
+virNodeDeviceObj *obj;
+virObjectEvent *event;
+g_autoptr(virNodeDeviceDef) def = defs[i];
+g_autofree char *name = g_strdup(def->name);
+bool defined = false;
+
+def->driver = g_strdup("vfio_mdev");
+
+if (!(obj = virNodeDeviceObjListFindByName(driver->devs, def->name))) {
+virNodeDeviceDef *d = g_steal_pointer();
+if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, d))) {
+virNodeDeviceDefFree(d);
+return -1;
+}
+} else {
+bool changed;
+virNodeDeviceDef *olddef = virNodeDeviceObjGetDef(obj);
+
+defined = virNodeDeviceObjIsPersistent(obj);
+/* Active devices contain some additional information (e.g. sysfs
+ * path) that is not provided by mdevctl, so re-use the existing
+ * definition and copy over new mdev data */
+changed = nodeDeviceDefCopyFromMdevctl(olddef, def);
+
+if (defined && !changed) {
+/* if this device was already defined and the definition
+ * hasn't changed, there's nothing to do for this device */
+virNodeDeviceObjEndAPI();
+continue;
+}
+}
+
+/* all devices returned by virMdevctlListDefined() are persistent */
+virNodeDeviceObjSetPersistent(obj, true);
+
+if (!defined)
+event = virNodeDeviceEventLifecycleNew(name,
+   
VIR_NODE_DEVICE_EVENT_DEFINED,
+   0);
+else
+event = virNodeDeviceEventUpdateNew(name);
+
+virNodeDeviceObjEndAPI();
+virObjectEventStateQueue(driver->nodeDeviceEventState, event);
+}
+
+return 0;
+}
+
+
+/* returns true if any attributes were copied, else returns false */
+static bool
+virMediatedDeviceAttrsCopy(virNodeDevCapMdev *dst,
+   virNodeDevCapMdev *src)
+{
+bool ret = false;
+size_t i;
+
+if (src->nattributes != dst->nattributes) {
+ret = true;
+for (i = 0; i < dst->nattributes; i++)
+virMediatedDeviceAttrFree(dst->attributes[i]);
+g_free(dst->attributes);
+
+dst->nattributes = src->nattributes;
+dst->attributes = g_new0(virMediatedDeviceAttr*,
+ src->nattributes);
+for (i = 0; i < dst->nattributes; i++)
+dst->attributes[i] = virMediatedDeviceAttrNew();
+}
+
+for (i = 0; i < src->nattributes; i++) {
+if (STRNEQ_NULLABLE(src->attributes[i]->name,
+dst->attributes[i]->name)) {
+ret = true;
+g_free(dst->attributes[i]->name);
+dst->attributes[i]->name =
+

[libvirt PATCH v6 19/30] nodedev: refactor tests to support mdev undefine

[Jonathon Jongsma]

mdevctl 'stop' and 'undefine' commands take the same uuid parameter, so
refactor the test infrastructure to share common implementation for both
of these commands. The 'undefine' command will be introduced in a
following patch.

Signed-off-by: Jonathon Jongsma 
---
 tests/nodedevmdevctltest.c | 48 +++---
 1 file changed, 40 insertions(+), 8 deletions(-)

diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index 99fa328d24..7cf9fa7c67 100644
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -12,7 +12,9 @@
 
 typedef enum {
 MDEVCTL_CMD_START,
+MDEVCTL_CMD_STOP,
 MDEVCTL_CMD_DEFINE,
+MDEVCTL_CMD_UNDEFINE
 } MdevctlCmd;
 
 struct startTestInfo {
@@ -135,20 +137,22 @@ testMdevctlStartOrDefineHelper(const void *data)
 mdevxml, cmdlinefile, jsonfile);
 }
 
+typedef virCommand* (*GetStopUndefineCmdFunc)(const char *uuid, char **errbuf);
+struct UuidCommandTestInfo {
+const char *uuid;
+MdevctlCmd command;
+};
+
 static int
-testMdevctlStop(const void *data)
+testMdevctlUuidCommand(const char *uuid, GetStopUndefineCmdFunc func, const 
char *outfile)
 {
-const char *uuid = data;
 g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
 const char *actualCmdline = NULL;
 int ret = -1;
 g_autoptr(virCommand) cmd = NULL;
 g_autofree char *errmsg = NULL;
-g_autofree char *cmdlinefile =
-g_strdup_printf("%s/nodedevmdevctldata/mdevctl-stop.argv",
-abs_srcdir);
 
-cmd = nodeDeviceGetMdevctlStopCommand(uuid, );
+cmd = func(uuid, );
 
 if (!cmd)
 goto cleanup;
@@ -160,7 +164,7 @@ testMdevctlStop(const void *data)
 if (!(actualCmdline = virBufferCurrentContent()))
 goto cleanup;
 
-if (nodedevCompareToFile(actualCmdline, cmdlinefile) < 0)
+if (nodedevCompareToFile(actualCmdline, outfile) < 0)
 goto cleanup;
 
 ret = 0;
@@ -170,6 +174,27 @@ testMdevctlStop(const void *data)
 return ret;
 }
 
+static int
+testMdevctlUuidCommandHelper(const void *data)
+{
+const struct UuidCommandTestInfo *info = data;
+GetStopUndefineCmdFunc func;
+const char *cmd;
+g_autofree char *cmdlinefile = NULL;
+
+if (info->command == MDEVCTL_CMD_STOP) {
+cmd = "stop";
+func = nodeDeviceGetMdevctlStopCommand;
+} else {
+return -1;
+}
+
+cmdlinefile = g_strdup_printf("%s/nodedevmdevctldata/mdevctl-%s.argv",
+  abs_srcdir, cmd);
+
+return testMdevctlUuidCommand(info->uuid, func, cmdlinefile);
+}
+
 static int
 testMdevctlListDefined(const void *data G_GNUC_UNUSED)
 {
@@ -387,8 +412,15 @@ mymain(void)
 #define DO_TEST_DEFINE(filename) \
 DO_TEST_CMD("mdevctl define " filename, "QEMU", CREATE_DEVICE, filename, 
MDEVCTL_CMD_DEFINE)
 
+#define DO_TEST_UUID_COMMAND_FULL(desc, uuid, command) \
+do { \
+struct UuidCommandTestInfo info = { uuid, command }; \
+DO_TEST_FULL(desc, testMdevctlUuidCommandHelper, ); \
+   } \
+while (0)
+
 #define DO_TEST_STOP(uuid) \
-DO_TEST_FULL("mdevctl stop " uuid, testMdevctlStop, uuid)
+DO_TEST_UUID_COMMAND_FULL("mdevctl stop " uuid, uuid, MDEVCTL_CMD_STOP)
 
 #define DO_TEST_LIST_DEFINED() \
 DO_TEST_FULL("mdevctl list --defined", testMdevctlListDefined, NULL)
-- 
2.26.3

[libvirt PATCH v6 16/30] api: add virNodeDeviceDefineXML()

[Jonathon Jongsma]

With mediated devices, we can now define persistent node devices that
can be started and stopped. In order to take advantage of this, we need
an API to define new node devices.

Signed-off-by: Jonathon Jongsma 
---
 include/libvirt/libvirt-nodedev.h|  4 ++
 src/driver-nodedev.h |  6 +++
 src/libvirt-nodedev.c| 42 
 src/libvirt_public.syms  |  1 +
 src/node_device/node_device_driver.c | 71 
 src/node_device/node_device_driver.h |  5 ++
 src/node_device/node_device_udev.c   |  1 +
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 17 ++-
 src/remote_protocol-structs  |  8 
 src/rpc/gendispatch.pl   |  1 +
 11 files changed, 156 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt-nodedev.h 
b/include/libvirt/libvirt-nodedev.h
index 77d814935e..33eb46b3cd 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -131,6 +131,10 @@ virNodeDevicePtrvirNodeDeviceCreateXML  
(virConnectPtr conn,
 
 int virNodeDeviceDestroy(virNodeDevicePtr dev);
 
+virNodeDevicePtr virNodeDeviceDefineXML(virConnectPtr conn,
+const char *xmlDesc,
+unsigned int flags);
+
 /**
  * VIR_NODE_DEVICE_EVENT_CALLBACK:
  *
diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h
index d0fc7f19cf..64a0a7c473 100644
--- a/src/driver-nodedev.h
+++ b/src/driver-nodedev.h
@@ -74,6 +74,11 @@ typedef virNodeDevicePtr
 typedef int
 (*virDrvNodeDeviceDestroy)(virNodeDevicePtr dev);
 
+typedef virNodeDevice*
+(*virDrvNodeDeviceDefineXML)(virConnect *conn,
+ const char *xmlDesc,
+ unsigned int flags);
+
 typedef int
 (*virDrvConnectNodeDeviceEventRegisterAny)(virConnectPtr conn,
virNodeDevicePtr dev,
@@ -113,4 +118,5 @@ struct _virNodeDeviceDriver {
 virDrvNodeDeviceListCaps nodeDeviceListCaps;
 virDrvNodeDeviceCreateXML nodeDeviceCreateXML;
 virDrvNodeDeviceDestroy nodeDeviceDestroy;
+virDrvNodeDeviceDefineXML nodeDeviceDefineXML;
 };
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index fb707b570f..cfc0c9de5b 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -737,6 +737,48 @@ virNodeDeviceDestroy(virNodeDevicePtr dev)
 }
 
 
+/**
+ * virNodeDeviceDefineXML:
+ * @conn: pointer to the hypervisor connection
+ * @xmlDesc: string containing an XML description of the device to be defined
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Define a new device on the VM host machine, for example, a mediated device
+ *
+ * virNodeDeviceFree should be used to free the resources after the
+ * node device object is no longer needed.
+ *
+ * Returns a node device object if successful, NULL in case of failure
+ */
+virNodeDevice*
+virNodeDeviceDefineXML(virConnect *conn,
+   const char *xmlDesc,
+   unsigned int flags)
+{
+VIR_DEBUG("conn=%p, xmlDesc=%s, flags=0x%x", conn, NULLSTR(xmlDesc), 
flags);
+
+virResetLastError();
+
+virCheckConnectReturn(conn, NULL);
+virCheckReadOnlyGoto(conn->flags, error);
+virCheckNonNullArgGoto(xmlDesc, error);
+
+if (conn->nodeDeviceDriver &&
+conn->nodeDeviceDriver->nodeDeviceDefineXML) {
+virNodeDevice *dev = conn->nodeDeviceDriver->nodeDeviceDefineXML(conn, 
xmlDesc, flags);
+if (dev == NULL)
+goto error;
+return dev;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(conn);
+return NULL;
+}
+
+
 /**
  * virConnectNodeDeviceEventRegisterAny:
  * @conn: pointer to the connection
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 51a3d7265a..3d8176351c 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -887,6 +887,7 @@ LIBVIRT_7.1.0 {
 LIBVIRT_7.2.0 {
 global:
 virDomainStartDirtyRateCalc;
+virNodeDeviceDefineXML;
 } LIBVIRT_7.1.0;
 
 #  define new API here using predicted next version number 
diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index abd45a6eab..418faa9fb9 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -776,6 +776,26 @@ virMdevctlStart(virNodeDeviceDefPtr def, char **uuid, char 
**errmsg)
 }
 
 
+static int
+virMdevctlDefine(virNodeDeviceDefPtr def, char **uuid, char **errmsg)
+{
+int status;
+g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlDefineCommand(def, uuid, 
errmsg);
+if (!cmd)
+return -1;
+
+/* an auto-generated uuid is returned via stdout if no uuid is specified in
+ * the mdevctl args */
+if (virCommandRun(cmd, ) < 0 || status != 0)
+return -1;
+
+/* remove newline */
+*uuid = g_strstrip(*uuid);
+

[libvirt PATCH v6 27/30] nodedev: fix hang when destroying an mdev in use

[Jonathon Jongsma]

Calling `mdevctl stop` for a mediated device that is in use by an active
domain will block until that vm exits (or the vm closes the device).
Since the nodedev driver cannot query the hypervisor driver to see
whether any active domains are using the device, we resort to a
workaround that relies on the fact that a vfio group can only be opened
by one user at a time. If we get an EBUSY error when attempting to open
the group file, we assume the device is in use and refuse to try to
destroy that device.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 src/node_device/node_device_driver.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index b1b3217afb..31322e3afb 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1177,8 +1177,27 @@ nodeDeviceDestroy(virNodeDevicePtr device)
 
 ret = 0;
 } else if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) {
+/* If this mediated device is in use by a vm, attempting to stop it
+ * will block until the vm closes the device. The nodedev driver
+ * cannot query the hypervisor driver to determine whether the device
+ * is in use by any active domains, since that would introduce circular
+ * dependencies between daemons and add a risk of deadlocks. So we need
+ * to resort to a workaround.  vfio only allows the group for a device
+ * to be opened by one user at a time. So if we get EBUSY when opening
+ * the group, we infer that the device is in use and therefore we
+ * shouldn't try to remove the device. */
+g_autofree char *vfiogroup =
+virMediatedDeviceGetIOMMUGroupDev(def->caps->data.mdev.uuid);
+VIR_AUTOCLOSE fd = open(vfiogroup, O_RDONLY);
 g_autofree char *errmsg = NULL;
 
+if (fd < 0 && errno == EBUSY) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unable to destroy '%s': device in use"),
+   def->name);
+goto cleanup;
+}
+
 if (virMdevctlStop(def, ) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to destroy '%s': %s"), def->name,
-- 
2.26.3

[libvirt PATCH v6 08/30] nodedev: add ability to list defined mdevs

[Jonathon Jongsma]

This adds an internal API to query for persistent mediated devices
that are defined by mdevctl. Upcoming commits will make use of this
information.

Signed-off-by: Jonathon Jongsma 
---
 src/node_device/node_device_driver.c  | 18 +
 src/node_device/node_device_driver.h  |  3 ++
 .../mdevctl-list-defined.argv |  1 +
 tests/nodedevmdevctltest.c| 39 +++
 4 files changed, 61 insertions(+)
 create mode 100644 tests/nodedevmdevctldata/mdevctl-list-defined.argv

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index a646692870..6911fa36da 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -860,6 +860,24 @@ virMdevctlStop(virNodeDeviceDefPtr def, char **errmsg)
 }
 
 
+virCommand*
+nodeDeviceGetMdevctlListCommand(bool defined,
+char **output)
+{
+virCommand *cmd = virCommandNewArgList(MDEVCTL,
+   "list",
+   "--dumpjson",
+   NULL);
+
+if (defined)
+virCommandAddArg(cmd, "--defined");
+
+virCommandSetOutputBuffer(cmd, output);
+
+return cmd;
+}
+
+
 static void mdevGenerateDeviceName(virNodeDeviceDef *dev)
 {
 nodeDeviceGenerateName(dev, "mdev", dev->caps->data.mdev.uuid, NULL);
diff --git a/src/node_device/node_device_driver.h 
b/src/node_device/node_device_driver.h
index f1c93f4b21..705d83f827 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -121,6 +121,9 @@ virCommandPtr
 nodeDeviceGetMdevctlStopCommand(const char *uuid,
 char **errmsg);
 
+virCommandPtr
+nodeDeviceGetMdevctlListCommand(bool defined, char **output);
+
 int
 nodeDeviceParseMdevctlJSON(const char *jsonstring,
virNodeDeviceDef ***devs);
diff --git a/tests/nodedevmdevctldata/mdevctl-list-defined.argv 
b/tests/nodedevmdevctldata/mdevctl-list-defined.argv
new file mode 100644
index 00..72b5906e9e
--- /dev/null
+++ b/tests/nodedevmdevctldata/mdevctl-list-defined.argv
@@ -0,0 +1 @@
+$MDEVCTL_BINARY$ list --dumpjson --defined
diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index 6c92d839d1..1ab4776a23 100644
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -145,6 +145,40 @@ testMdevctlStop(const void *data)
 return ret;
 }
 
+static int
+testMdevctlListDefined(const void *data G_GNUC_UNUSED)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+const char *actualCmdline = NULL;
+int ret = -1;
+g_autoptr(virCommand) cmd = NULL;
+g_autofree char *output = NULL;
+g_autofree char *cmdlinefile =
+g_strdup_printf("%s/nodedevmdevctldata/mdevctl-list-defined.argv",
+abs_srcdir);
+
+cmd = nodeDeviceGetMdevctlListCommand(true, );
+
+if (!cmd)
+goto cleanup;
+
+virCommandSetDryRun(, NULL, NULL);
+if (virCommandRun(cmd, NULL) < 0)
+goto cleanup;
+
+if (!(actualCmdline = virBufferCurrentContent()))
+goto cleanup;
+
+if (nodedevCompareToFile(actualCmdline, cmdlinefile) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virBufferFreeAndReset();
+virCommandSetDryRun(NULL, NULL, NULL);
+return ret;
+}
 
 static int
 testMdevctlParse(const void *data)
@@ -328,6 +362,9 @@ mymain(void)
 #define DO_TEST_STOP(uuid) \
 DO_TEST_FULL("mdevctl stop " uuid, testMdevctlStop, uuid)
 
+#define DO_TEST_LIST_DEFINED() \
+DO_TEST_FULL("mdevctl list --defined", testMdevctlListDefined, NULL)
+
 #define DO_TEST_PARSE_JSON(filename) \
 DO_TEST_FULL("parse mdevctl json " filename, testMdevctlParse, filename)
 
@@ -339,6 +376,8 @@ mymain(void)
 /* Test mdevctl stop command, pass an arbitrary uuid */
 DO_TEST_STOP("e2451f73-c95b-4124-b900-e008af37c576");
 
+DO_TEST_LIST_DEFINED();
+
 DO_TEST_PARSE_JSON("mdevctl-list-multiple");
 
  done:
-- 
2.26.3

[libvirt PATCH v6 14/30] nodedev: Refresh mdev devices when changes are detected

[Jonathon Jongsma]

We need to query mdevctl for changes to device definitions since an
administrator can define new devices by executing mdevctl outside of
libvirt.

In the future, mdevctl may add a way to signal device add/remove via
events, but for now we resort to a bit of a workaround: monitoring the
mdevctl config directory for changes to files. When a change is
detected, we query mdevctl and update our device list. The mdevctl
querying is handled in a throwaway thread, and these threads are
synchronized with a mutex.

Signed-off-by: Jonathon Jongsma 
---
 src/node_device/node_device_udev.c | 161 +
 1 file changed, 161 insertions(+)

diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 38ebe7b5c5..78144b7762 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -19,6 +19,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -69,6 +70,10 @@ struct _udevEventData {
 
 /* init thread */
 virThread initThread;
+
+GList *mdevctlMonitors;
+virMutex mdevctlLock;
+int mdevctlTimeout;
 };
 
 static virClassPtr udevEventDataClass;
@@ -89,6 +94,11 @@ udevEventDataDispose(void *obj)
 udev_monitor_unref(priv->udev_monitor);
 udev_unref(udev);
 
+virMutexLock(>mdevctlLock);
+g_list_free_full(priv->mdevctlMonitors, g_object_unref);
+virMutexUnlock(>mdevctlLock);
+virMutexDestroy(>mdevctlLock);
+
 virCondDestroy(>threadCond);
 }
 
@@ -120,6 +130,11 @@ udevEventDataNew(void)
 return NULL;
 }
 
+if (virMutexInit(>mdevctlLock) < 0) {
+virObjectUnref(ret);
+return NULL;
+}
+
 ret->watch = -1;
 return ret;
 }
@@ -2004,6 +2019,137 @@ udevPCITranslateInit(bool privileged G_GNUC_UNUSED)
 }
 
 
+static void
+mdevctlHandlerThread(void *opaque G_GNUC_UNUSED)
+{
+udevEventData *priv = driver->privateData;
+
+/* ensure only a single thread can query mdevctl at a time */
+virMutexLock(>mdevctlLock);
+if (nodeDeviceUpdateMediatedDevices() < 0)
+VIR_WARN("mdevctl failed to updated mediated devices");
+virMutexUnlock(>mdevctlLock);
+}
+
+
+static void
+scheduleMdevctlHandler(int timer G_GNUC_UNUSED, void *opaque)
+{
+udevEventData *priv = opaque;
+virThread thread;
+
+if (priv->mdevctlTimeout > 0) {
+virEventRemoveTimeout(priv->mdevctlTimeout);
+priv->mdevctlTimeout = -1;
+}
+
+if (virThreadCreateFull(, false, mdevctlHandlerThread,
+"mdevctl-thread", false, NULL) < 0) {
+virReportSystemError(errno, "%s",
+ _("failed to create mdevctl thread"));
+}
+}
+
+
+static void
+mdevctlEventHandleCallback(GFileMonitor *monitor G_GNUC_UNUSED,
+   GFile *file,
+   GFile *other_file G_GNUC_UNUSED,
+   GFileMonitorEvent event_type,
+   gpointer user_data);
+
+
+/* Recursively monitors a directory and its subdirectories for file changes and
+ * returns a GList of GFileMonitor objects */
+static GList*
+monitorFileRecursively(udevEventData *udev,
+   GFile *file)
+{
+GList *monitors = NULL;
+g_autoptr(GError) error = NULL;
+g_autoptr(GFileEnumerator) children = NULL;
+GFileMonitor *mon;
+
+if (!(children = g_file_enumerate_children(file, "standard::*",
+   G_FILE_QUERY_INFO_NONE, NULL, 
)))
+goto error;
+
+if (!(mon = g_file_monitor(file, G_FILE_MONITOR_NONE, NULL, )))
+goto error;
+
+g_signal_connect(mon, "changed",
+ G_CALLBACK(mdevctlEventHandleCallback), udev);
+
+monitors = g_list_append(monitors, mon);
+
+while (true) {
+GFileInfo *info = NULL;
+GFile *child = NULL;
+GList *child_monitors = NULL;
+
+if (!g_file_enumerator_iterate(children, , , NULL, ))
+goto error;
+
+if (!info)
+break;
+
+if (g_file_query_file_type(child, G_FILE_QUERY_INFO_NONE, NULL) ==
+G_FILE_TYPE_DIRECTORY) {
+
+child_monitors = monitorFileRecursively(udev, child);
+if (child_monitors)
+monitors = g_list_concat(monitors, child_monitors);
+}
+}
+
+return monitors;
+
+ error:
+g_list_free_full(monitors, g_object_unref);
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unable to monitor directory: %s"), error->message);
+g_clear_error();
+return NULL;
+}
+
+
+static void
+mdevctlEventHandleCallback(GFileMonitor *monitor G_GNUC_UNUSED,
+   GFile *file,
+   GFile *other_file G_GNUC_UNUSED,
+   GFileMonitorEvent event_type,
+   gpointer user_data)
+{
+udevEventData *priv = user_data;
+/* if a new directory appears, monitor that directory for changes */
+if 

[libvirt PATCH v6 15/30] nodedev: add function to generate mdevctl define command

[Jonathon Jongsma]

Abstract out the function used to generate the commandline for 'mdevctl
start' since they take the same arguments. Add tests to ensure that
we're generating the command properly.

Signed-off-by: Jonathon Jongsma 
---
 src/node_device/node_device_driver.c  | 34 +++--
 src/node_device/node_device_driver.h  |  6 ++
 ...19_36ea_4111_8f0a_8c9a70e21366-define.argv |  1 +
 ...19_36ea_4111_8f0a_8c9a70e21366-define.json |  1 +
 ...39_495e_4243_ad9f_beb3f14c23d9-define.argv |  1 +
 ...39_495e_4243_ad9f_beb3f14c23d9-define.json |  1 +
 ...16_1ca8_49ac_b176_871d16c13076-define.argv |  1 +
 ...16_1ca8_49ac_b176_871d16c13076-define.json |  1 +
 tests/nodedevmdevctltest.c| 76 +--
 9 files changed, 95 insertions(+), 27 deletions(-)
 create mode 100644 
tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-define.argv
 create mode 100644 
tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-define.json
 create mode 100644 
tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-define.argv
 create mode 100644 
tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-define.json
 create mode 100644 
tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-define.argv
 create mode 100644 
tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-define.json

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 4be650ddef..abd45a6eab 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -698,10 +698,14 @@ nodeDeviceFindAddressByName(const char *name)
 }
 
 
-virCommandPtr
-nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
- char **uuid_out,
- char **errmsg)
+/* the mdevctl 'start' and 'define' commands accept almost the exact same
+ * arguments, so provide a common implementation that can be wrapped by a more
+ * specific function */
+static virCommand*
+nodeDeviceGetMdevctlDefineStartCommand(virNodeDeviceDef *def,
+   const char *subcommand,
+   char **uuid_out,
+   char **errmsg)
 {
 virCommandPtr cmd;
 g_autofree char *json = NULL;
@@ -719,7 +723,7 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
 return NULL;
 }
 
-cmd = virCommandNewArgList(MDEVCTL, "start",
+cmd = virCommandNewArgList(MDEVCTL, subcommand,
"-p", parent_addr,
"--jsonfile", "/dev/stdin",
NULL);
@@ -731,6 +735,26 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
 return cmd;
 }
 
+virCommand*
+nodeDeviceGetMdevctlStartCommand(virNodeDeviceDef *def,
+ char **uuid_out,
+ char **errmsg)
+{
+return nodeDeviceGetMdevctlDefineStartCommand(def, "start", uuid_out,
+  errmsg);
+}
+
+virCommand*
+nodeDeviceGetMdevctlDefineCommand(virNodeDeviceDef *def,
+  char **uuid_out,
+  char **errmsg)
+{
+return nodeDeviceGetMdevctlDefineStartCommand(def, "define", uuid_out,
+  errmsg);
+}
+
+
+
 static int
 virMdevctlStart(virNodeDeviceDefPtr def, char **uuid, char **errmsg)
 {
diff --git a/src/node_device/node_device_driver.h 
b/src/node_device/node_device_driver.h
index 9b54efe4e3..b319990f0f 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -117,6 +117,12 @@ virCommandPtr
 nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
  char **uuid_out,
  char **errmsg);
+
+virCommand*
+nodeDeviceGetMdevctlDefineCommand(virNodeDeviceDef *def,
+  char **uuid_out,
+  char **errmsg);
+
 virCommandPtr
 nodeDeviceGetMdevctlStopCommand(const char *uuid,
 char **errmsg);
diff --git 
a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-define.argv
 
b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-define.argv
new file mode 100644
index 00..773e98b963
--- /dev/null
+++ 
b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-define.argv
@@ -0,0 +1 @@
+$MDEVCTL_BINARY$ define -p :00:02.0 --jsonfile /dev/stdin
diff --git 
a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-define.json
 
b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-define.json
new file mode 100644
index 00..bfc6dcace3
--- /dev/null
+++ 
b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-define.json
@@ -0,0 +1 @@

[libvirt PATCH v6 18/30] virsh: add nodedev-define command

[Jonathon Jongsma]

Add a virsh command that maps to virNodeDeviceDefineXML().

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 tools/virsh-nodedev.c | 58 +++
 1 file changed, 58 insertions(+)

diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 7ab5b264fc..1cbf2e082b 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -1013,6 +1013,58 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd)
 }
 
 
+/*
+ * "nodedev-define" command
+ */
+static const vshCmdInfo info_node_device_define[] = {
+{.name = "help",
+ .data = N_("Define a device by an xml file on a node")
+},
+{.name = "desc",
+ .data = N_("Defines a persistent device on the node that can be "
+"assigned to a domain. The device must be started before "
+"it can be assigned to a domain.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_node_device_define[] = {
+VIRSH_COMMON_OPT_FILE(N_("file containing an XML description "
+ "of the device")),
+{.name = NULL}
+};
+
+static bool
+cmdNodeDeviceDefine(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
+{
+virNodeDevice *dev = NULL;
+const char *from = NULL;
+bool ret = true;
+char *buffer;
+virshControl *priv = ctl->privData;
+
+if (vshCommandOptStringReq(ctl, cmd, "file", ) < 0)
+return false;
+
+if (virFileReadAll(from, VSH_MAX_XML_FILE, ) < 0)
+return false;
+
+dev = virNodeDeviceDefineXML(priv->conn, buffer, 0);
+VIR_FREE(buffer);
+
+if (dev != NULL) {
+vshPrintExtra(ctl, _("Node device '%s' defined from '%s'\n"),
+  virNodeDeviceGetName(dev), from);
+virNodeDeviceFree(dev);
+} else {
+vshError(ctl, _("Failed to define node device from '%s'"), from);
+ret = false;
+}
+
+return ret;
+}
+
+
 const vshCmdDef nodedevCmds[] = {
 {.name = "nodedev-create",
  .handler = cmdNodeDeviceCreate,
@@ -1066,5 +1118,11 @@ const vshCmdDef nodedevCmds[] = {
  .info = info_node_device_event,
  .flags = 0
 },
+{.name = "nodedev-define",
+ .handler = cmdNodeDeviceDefine,
+ .opts = opts_node_device_define,
+ .info = info_node_device_define,
+ .flags = 0
+},
 {.name = NULL}
 };
-- 
2.26.3

[libvirt PATCH v6 06/30] nodedev: expose internal helper for naming devices

[Jonathon Jongsma]

Expose a helper function that can be used by udev and mdevctl to
generate device names for node devices.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 src/node_device/node_device_driver.c | 25 +
 src/node_device/node_device_driver.h |  6 ++
 src/node_device/node_device_udev.c   | 19 +++
 3 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 920fd815f2..bada5bbf00 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -978,3 +978,28 @@ nodedevRegister(void)
 return udevNodeRegister();
 #endif
 }
+
+
+void
+nodeDeviceGenerateName(virNodeDeviceDef *def,
+   const char *subsystem,
+   const char *sysname,
+   const char *s)
+{
+size_t i;
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAsprintf(, "%s_%s",
+  subsystem,
+  sysname);
+
+if (s != NULL)
+virBufferAsprintf(, "_%s", s);
+
+def->name = virBufferContentAndReset();
+
+for (i = 0; i < strlen(def->name); i++) {
+if (!(g_ascii_isalnum(*(def->name + i
+*(def->name + i) = '_';
+}
+}
diff --git a/src/node_device/node_device_driver.h 
b/src/node_device/node_device_driver.h
index 4a40aa51f6..76a40c05ad 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -120,3 +120,9 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
 virCommandPtr
 nodeDeviceGetMdevctlStopCommand(const char *uuid,
 char **errmsg);
+
+void
+nodeDeviceGenerateName(virNodeDeviceDef *def,
+   const char *subsystem,
+   const char *sysname,
+   const char *s);
diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 373f36c41f..eae301cc95 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -311,22 +311,9 @@ udevGenerateDeviceName(struct udev_device *device,
virNodeDeviceDefPtr def,
const char *s)
 {
-size_t i;
-g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-
-virBufferAsprintf(, "%s_%s",
-  udev_device_get_subsystem(device),
-  udev_device_get_sysname(device));
-
-if (s != NULL)
-virBufferAsprintf(, "_%s", s);
-
-def->name = virBufferContentAndReset();
-
-for (i = 0; i < strlen(def->name); i++) {
-if (!(g_ascii_isalnum(*(def->name + i
-*(def->name + i) = '_';
-}
+nodeDeviceGenerateName(def,
+   udev_device_get_subsystem(device),
+   udev_device_get_sysname(device), s);
 
 return 0;
 }
-- 
2.26.3

[libvirt PATCH v6 07/30] nodedev: add ability to parse mdevs from mdevctl

[Jonathon Jongsma]

This function will parse the list of mediated devices that are returned
by mdevctl and convert it into our internal node device representation.

Signed-off-by: Jonathon Jongsma 
---
 src/node_device/node_device_driver.c  | 143 ++
 src/node_device/node_device_driver.h  |   4 +
 .../mdevctl-list-multiple.json|  59 
 .../mdevctl-list-multiple.out.xml |  39 +
 tests/nodedevmdevctltest.c|  56 ++-
 5 files changed, 299 insertions(+), 2 deletions(-)
 create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.json
 create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index bada5bbf00..a646692870 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -860,6 +860,149 @@ virMdevctlStop(virNodeDeviceDefPtr def, char **errmsg)
 }
 
 
+static void mdevGenerateDeviceName(virNodeDeviceDef *dev)
+{
+nodeDeviceGenerateName(dev, "mdev", dev->caps->data.mdev.uuid, NULL);
+}
+
+
+static virNodeDeviceDef*
+nodeDeviceParseMdevctlChildDevice(const char *parent,
+  virJSONValue *json)
+{
+virNodeDevCapMdev *mdev;
+const char *uuid;
+virJSONValue *props;
+virJSONValue *attrs;
+g_autoptr(virNodeDeviceDef) child = g_new0(virNodeDeviceDef, 1);
+
+/* the child object should have a single key equal to its uuid.
+ * The value is an object describing the properties of the mdev */
+if (virJSONValueObjectKeysNumber(json) != 1)
+return NULL;
+
+uuid = virJSONValueObjectGetKey(json, 0);
+props = virJSONValueObjectGetValue(json, 0);
+
+child->parent = g_strdup(parent);
+child->caps = g_new0(virNodeDevCapsDef, 1);
+child->caps->data.type = VIR_NODE_DEV_CAP_MDEV;
+
+mdev = >caps->data.mdev;
+mdev->uuid = g_strdup(uuid);
+mdev->type =
+g_strdup(virJSONValueObjectGetString(props, "mdev_type"));
+
+attrs = virJSONValueObjectGet(props, "attrs");
+
+if (attrs && virJSONValueIsArray(attrs)) {
+size_t i;
+int nattrs = virJSONValueArraySize(attrs);
+
+mdev->attributes = g_new0(virMediatedDeviceAttr*, nattrs);
+mdev->nattributes = nattrs;
+
+for (i = 0; i < nattrs; i++) {
+virJSONValue *attr = virJSONValueArrayGet(attrs, i);
+virMediatedDeviceAttr *attribute;
+virJSONValue *value;
+
+if (!virJSONValueIsObject(attr) ||
+virJSONValueObjectKeysNumber(attr) != 1)
+return NULL;
+
+attribute = g_new0(virMediatedDeviceAttr, 1);
+attribute->name = g_strdup(virJSONValueObjectGetKey(attr, 0));
+value = virJSONValueObjectGetValue(attr, 0);
+attribute->value = g_strdup(virJSONValueGetString(value));
+mdev->attributes[i] = attribute;
+}
+}
+mdevGenerateDeviceName(child);
+
+return g_steal_pointer();
+}
+
+
+int
+nodeDeviceParseMdevctlJSON(const char *jsonstring,
+   virNodeDeviceDef ***devs)
+{
+int n;
+g_autoptr(virJSONValue) json_devicelist = NULL;
+virNodeDeviceDef **outdevs = NULL;
+size_t noutdevs = 0;
+size_t i;
+size_t j;
+
+json_devicelist = virJSONValueFromString(jsonstring);
+
+if (!json_devicelist || !virJSONValueIsArray(json_devicelist)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("mdevctl JSON response contains no devices"));
+goto error;
+}
+
+n = virJSONValueArraySize(json_devicelist);
+
+for (i = 0; i < n; i++) {
+virJSONValue *obj = virJSONValueArrayGet(json_devicelist, i);
+const char *parent;
+virJSONValue *child_array;
+int nchildren;
+
+if (!virJSONValueIsObject(obj)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Parent device is not an object"));
+goto error;
+}
+
+/* mdevctl returns an array of objects.  Each object is a parent device
+ * object containing a single key-value pair which maps from the name
+ * of the parent device to an array of child devices */
+if (virJSONValueObjectKeysNumber(obj) != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Unexpected format for parent device object"));
+goto error;
+}
+
+parent = virJSONValueObjectGetKey(obj, 0);
+child_array = virJSONValueObjectGetValue(obj, 0);
+
+if (!virJSONValueIsArray(child_array)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Parent device's JSON object data is not an 
array"));
+goto error;
+}
+
+nchildren = virJSONValueArraySize(child_array);
+
+for (j = 0; j < nchildren; j++) {
+

[libvirt PATCH v6 13/30] nodedev: handle mdevs that disappear from mdevctl

[Jonathon Jongsma]

mdevctl does not currently provide any events when the list of defined
devices changes, so we will need to poll mdevctl for the list of defined
devices periodically. When a mediated device no longer exists from one
iteration to the next, we need to treat it as an "undefine" event.

When we get such an event, we remove the device from the list if it's
not active. Otherwise, we simply mark it as non-persistent.

Signed-off-by: Jonathon Jongsma 
---
 src/node_device/node_device_driver.c | 67 ++--
 1 file changed, 64 insertions(+), 3 deletions(-)

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index efa524f317..4be650ddef 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1183,20 +1183,81 @@ virMdevctlListDefined(virNodeDeviceDef ***devs)
 }
 
 
+typedef struct _virMdevctlForEachData virMdevctlForEachData;
+struct _virMdevctlForEachData {
+int ndefs;
+virNodeDeviceDef **defs;
+};
+
+
+/* This function keeps the list of persistent mediated devices consistent
+ * between the nodedev driver and mdevctl.
+ * @obj is a device that is currently known by the nodedev driver, and @opaque
+ * contains the most recent list of devices defined by mdevctl. If @obj is no
+ * longer defined in mdevctl, mark it as undefined and possibly remove it from
+ * the driver as well. Returning 'true' from this function indicates that the
+ * device should be removed from the nodedev driver list. */
+static bool
+removeMissingPersistentMdevs(virNodeDeviceObj *obj,
+ const void *opaque)
+{
+bool remove = false;
+const virMdevctlForEachData *data = opaque;
+size_t i;
+virNodeDeviceDef *def = virNodeDeviceObjGetDef(obj);
+virObjectEvent *event;
+
+if (def->caps->data.type != VIR_NODE_DEV_CAP_MDEV)
+return false;
+
+/* transient mdevs are populated via udev, so don't remove them from the
+ * nodedev driver just because they are not reported by by mdevctl */
+if (!virNodeDeviceObjIsPersistent(obj))
+return false;
+
+for (i = 0; i < data->ndefs; i++) {
+/* OK, this mdev is still defined by mdevctl */
+if (STREQ(data->defs[i]->name, def->name))
+return false;
+}
+
+event = virNodeDeviceEventLifecycleNew(def->name,
+   VIR_NODE_DEVICE_EVENT_UNDEFINED,
+   0);
+
+/* The device is active, but no longer defined by mdevctl. Keep the device
+ * in the list, but mark it as non-persistent */
+if (virNodeDeviceObjIsActive(obj))
+virNodeDeviceObjSetPersistent(obj, false);
+else
+remove = true;
+
+virObjectEventStateQueue(driver->nodeDeviceEventState, event);
+
+return remove;
+}
+
+
 int
 nodeDeviceUpdateMediatedDevices(void)
 {
 g_autofree virNodeDeviceDef **defs = NULL;
-int ndefs;
+virMdevctlForEachData data = { 0, };
 size_t i;
 
-if ((ndefs = virMdevctlListDefined()) < 0) {
+if ((data.ndefs = virMdevctlListDefined()) < 0) {
 virReportSystemError(errno, "%s",
  _("failed to query mdevs from mdevctl"));
 return -1;
 }
 
-for (i = 0; i < ndefs; i++) {
+/* Any mdevs that were previously defined but were not returned in the
+ * latest mdevctl query should be removed from the device list */
+data.defs = defs;
+virNodeDeviceObjListForEachRemove(driver->devs,
+  removeMissingPersistentMdevs, );
+
+for (i = 0; i < data.ndefs; i++) {
 virNodeDeviceObj *obj;
 virObjectEvent *event;
 g_autoptr(virNodeDeviceDef) def = defs[i];
-- 
2.26.3

[libvirt PATCH v6 04/30] nodedev: Add ability to filter by active state

[Jonathon Jongsma]

Add two flag values for virConnectListAllNodeDevices() so that we can
list only node devices that are active or inactive.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 include/libvirt/libvirt-nodedev.h|  9 +++--
 src/conf/node_device_conf.h  |  8 
 src/conf/virnodedeviceobj.c  | 57 +---
 src/libvirt-nodedev.c|  2 +
 src/node_device/node_device_driver.c |  2 +-
 5 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/include/libvirt/libvirt-nodedev.h 
b/include/libvirt/libvirt-nodedev.h
index eab8abf6ab..1a0e60b81f 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -61,10 +61,9 @@ int virNodeListDevices  
(virConnectPtr conn,
  * virConnectListAllNodeDevices:
  *
  * Flags used to filter the returned node devices. Flags in each group
- * are exclusive. Currently only one group to filter the devices by cap
- * type.
- */
+ * are exclusive.  */
 typedef enum {
+/* filter the devices by cap type */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_SYSTEM= 1 << 0,  /* System 
capability */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV   = 1 << 1,  /* PCI device */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_DEV   = 1 << 2,  /* USB device */
@@ -86,6 +85,10 @@ typedef enum {
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_CARD   = 1 << 18, /* s390 AP Card 
device */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE  = 1 << 19, /* s390 AP 
Queue */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_MATRIX = 1 << 20, /* s390 AP 
Matrix */
+
+/* filter the devices by active state */
+VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE  = 1 << 30, /* Inactive 
devices */
+VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE= 1U << 31, /* Active 
devices */
 } virConnectListAllNodeDeviceFlags;
 
 int virConnectListAllNodeDevices (virConnectPtr conn,
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index c67b8e2aeb..3d7872fd6e 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -422,6 +422,14 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps);
  VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE  | \
  VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_MATRIX)
 
+#define VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ACTIVE \
+VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE |\
+VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE
+
+#define VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ALL \
+VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_CAP | \
+VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ACTIVE
+
 int
 virNodeDeviceGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host);
 
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index f0ceb61a19..ab663fd5a0 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -848,8 +848,10 @@ virNodeDeviceObjListGetNames(virNodeDeviceObjListPtr devs,
 }
 
 
-#define MATCH(FLAG) ((flags & (VIR_CONNECT_LIST_NODE_DEVICES_CAP_ ## FLAG)) && 
\
- virNodeDeviceObjHasCap(obj, VIR_NODE_DEV_CAP_ ## FLAG))
+#define MATCH_CAP(FLAG) ((flags & (VIR_CONNECT_LIST_NODE_DEVICES_CAP_ ## 
FLAG)) && \
+ virNodeDeviceObjHasCap(obj, VIR_NODE_DEV_CAP_ ## 
FLAG))
+#define MATCH(FLAG) (flags & (FLAG))
+
 static bool
 virNodeDeviceObjMatch(virNodeDeviceObjPtr obj,
   unsigned int flags)
@@ -861,33 +863,42 @@ virNodeDeviceObjMatch(virNodeDeviceObjPtr obj,
 
 /* filter by cap type */
 if (flags & VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_CAP) {
-if (!(MATCH(SYSTEM)||
-  MATCH(PCI_DEV)   ||
-  MATCH(USB_DEV)   ||
-  MATCH(USB_INTERFACE) ||
-  MATCH(NET)   ||
-  MATCH(SCSI_HOST) ||
-  MATCH(SCSI_TARGET)   ||
-  MATCH(SCSI)  ||
-  MATCH(STORAGE)   ||
-  MATCH(FC_HOST)   ||
-  MATCH(VPORTS)||
-  MATCH(SCSI_GENERIC)  ||
-  MATCH(DRM)   ||
-  MATCH(MDEV_TYPES)||
-  MATCH(MDEV)  ||
-  MATCH(CCW_DEV)   ||
-  MATCH(CSS_DEV)   ||
-  MATCH(VDPA)  ||
-  MATCH(AP_CARD)   ||
-  MATCH(AP_QUEUE)  ||
-  MATCH(AP_MATRIX)))
+if (!(MATCH_CAP(SYSTEM)||
+  MATCH_CAP(PCI_DEV)   ||
+  MATCH_CAP(USB_DEV)   ||
+  MATCH_CAP(USB_INTERFACE) ||
+  MATCH_CAP(NET)   ||
+  MATCH_CAP(SCSI_HOST) ||
+  MATCH_CAP(SCSI_TARGET)   ||
+  MATCH_CAP(SCSI)  ||
+  MATCH_CAP(STORAGE)   ||
+  MATCH_CAP(FC_HOST)   ||
+  MATCH_CAP(VPORTS)||
+  MATCH_CAP(SCSI_GENERIC)  ||
+  MATCH_CAP(DRM)   ||
+  

[libvirt PATCH v6 10/30] nodedev: add DEFINED/UNDEFINED lifecycle events

[Jonathon Jongsma]

Since a mediated device can be persistently defined by the mdevctl
backend, we need additional lifecycle events beyond CREATED/DELETED to
indicate that e.g. the device has been stopped but the device definition
still exists.

Signed-off-by: Jonathon Jongsma 
---
 examples/c/misc/event-test.c  | 4 
 include/libvirt/libvirt-nodedev.h | 2 ++
 tools/virsh-nodedev.c | 4 +++-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/examples/c/misc/event-test.c b/examples/c/misc/event-test.c
index 76d4f3f6e8..10c707e66b 100644
--- a/examples/c/misc/event-test.c
+++ b/examples/c/misc/event-test.c
@@ -381,6 +381,10 @@ nodeDeviceEventToString(int event)
 return "Created";
 case VIR_NODE_DEVICE_EVENT_DELETED:
 return "Deleted";
+case VIR_NODE_DEVICE_EVENT_DEFINED:
+return "Defined";
+case VIR_NODE_DEVICE_EVENT_UNDEFINED:
+return "Undefined";
 case VIR_NODE_DEVICE_EVENT_LAST:
 break;
 }
diff --git a/include/libvirt/libvirt-nodedev.h 
b/include/libvirt/libvirt-nodedev.h
index 2deead0791..77d814935e 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -196,6 +196,8 @@ int virConnectNodeDeviceEventDeregisterAny(virConnectPtr 
conn,
 typedef enum {
 VIR_NODE_DEVICE_EVENT_CREATED = 0,
 VIR_NODE_DEVICE_EVENT_DELETED = 1,
+VIR_NODE_DEVICE_EVENT_DEFINED = 2,
+VIR_NODE_DEVICE_EVENT_UNDEFINED = 3,
 
 # ifdef VIR_ENUM_SENTINELS
 VIR_NODE_DEVICE_EVENT_LAST
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index fedc8497f8..b9fe9b8be1 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -775,7 +775,9 @@ VIR_ENUM_DECL(virshNodeDeviceEvent);
 VIR_ENUM_IMPL(virshNodeDeviceEvent,
   VIR_NODE_DEVICE_EVENT_LAST,
   N_("Created"),
-  N_("Deleted"));
+  N_("Deleted"),
+  N_("Defined"),
+  N_("Undefined"));
 
 static const char *
 virshNodeDeviceEventToString(int event)
-- 
2.26.3

[libvirt PATCH v6 05/30] nodedev: fix docs for virConnectListAllNodeDevices()

[Jonathon Jongsma]

It doesn't make sense to list all of the flag values in the function
documentation. This is unnecessary duplication, we already refer to the
enum type.  Also, remove reference to exclusive groups of flags, since
that does not apply to this API.

Signed-off-by: Jonathon Jongsma 
---
 include/libvirt/libvirt-nodedev.h |  3 +--
 src/libvirt-nodedev.c | 30 +-
 2 files changed, 2 insertions(+), 31 deletions(-)

diff --git a/include/libvirt/libvirt-nodedev.h 
b/include/libvirt/libvirt-nodedev.h
index 1a0e60b81f..2deead0791 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -60,8 +60,7 @@ int virNodeListDevices  
(virConnectPtr conn,
 /*
  * virConnectListAllNodeDevices:
  *
- * Flags used to filter the returned node devices. Flags in each group
- * are exclusive.  */
+ * Flags used to filter the returned node devices.  */
 typedef enum {
 /* filter the devices by cap type */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_SYSTEM= 1 << 0,  /* System 
capability */
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index 375b907852..fb707b570f 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -78,35 +78,7 @@ virNodeNumOfDevices(virConnectPtr conn, const char *cap, 
unsigned int flags)
  * objects.
  *
  * Normally, all node devices are returned; however, @flags can be used to
- * filter the results for a smaller list of targeted node devices.  The valid
- * flags are divided into groups, where each group contains bits that
- * describe mutually exclusive attributes of a node device, and where all bits
- * within a group describe all possible node devices.
- *
- * Only one group of the @flags is provided to filter the node devices by
- * capability type, flags include:
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_SYSTEM
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_DEV
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_INTERFACE
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_TARGET
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_DRM
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_CARD
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE
- *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_MATRIX
- *   VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE
- *   VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE
+ * filter the results for a smaller list of targeted node devices.
  *
  * Returns the number of node devices found or -1 and sets @devices to NULL in
  * case of error.  On success, the array stored into @devices is guaranteed to
-- 
2.26.3

[libvirt PATCH v6 03/30] nodedev: introduce concept of 'active' node devices

[Jonathon Jongsma]

we will be able to define mediated devices that can be started or
stopped, so we need to be able to indicate whether the device is active
or not, similar to other resources (storage pools, domains, etc.)

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 src/conf/virnodedeviceobj.c| 16 
 src/conf/virnodedeviceobj.h|  7 +++
 src/libvirt_private.syms   |  2 ++
 src/node_device/node_device_udev.c |  3 +++
 4 files changed, 28 insertions(+)

diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index ecae3d0479..f0ceb61a19 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -39,6 +39,7 @@ struct _virNodeDeviceObj {
 virNodeDeviceDefPtr def;/* device definition */
 bool skipUpdateCaps;/* whether to skip checking host caps,
used by testdriver */
+bool active;
 };
 
 struct _virNodeDeviceObjList {
@@ -976,3 +977,18 @@ virNodeDeviceObjSetSkipUpdateCaps(virNodeDeviceObjPtr obj,
 {
 obj->skipUpdateCaps = skipUpdateCaps;
 }
+
+
+bool
+virNodeDeviceObjIsActive(virNodeDeviceObj *obj)
+{
+return obj->active;
+}
+
+
+void
+virNodeDeviceObjSetActive(virNodeDeviceObj *obj,
+  bool active)
+{
+obj->active = active;
+}
diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h
index 6efdb23d36..e786a70f51 100644
--- a/src/conf/virnodedeviceobj.h
+++ b/src/conf/virnodedeviceobj.h
@@ -121,3 +121,10 @@ virNodeDeviceObjSetSkipUpdateCaps(virNodeDeviceObjPtr obj,
 virNodeDeviceObjPtr
 virNodeDeviceObjListFindMediatedDeviceByUUID(virNodeDeviceObjListPtr devs,
  const char *uuid);
+
+bool
+virNodeDeviceObjIsActive(virNodeDeviceObj *obj);
+
+void
+virNodeDeviceObjSetActive(virNodeDeviceObj *obj,
+  bool active);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cb9fe7c80a..09957c943b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1272,6 +1272,7 @@ virNetworkPortDefSaveStatus;
 # conf/virnodedeviceobj.h
 virNodeDeviceObjEndAPI;
 virNodeDeviceObjGetDef;
+virNodeDeviceObjIsActive;
 virNodeDeviceObjListAssignDef;
 virNodeDeviceObjListExport;
 virNodeDeviceObjListFindByName;
@@ -1284,6 +1285,7 @@ virNodeDeviceObjListGetParentHost;
 virNodeDeviceObjListNew;
 virNodeDeviceObjListNumOfDevices;
 virNodeDeviceObjListRemove;
+virNodeDeviceObjSetActive;
 
 
 # conf/virnwfilterbindingdef.h
diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 3daa5c90ad..373f36c41f 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1544,6 +1544,7 @@ udevAddOneDevice(struct udev_device *device)
 else
 event = virNodeDeviceEventUpdateNew(objdef->name);
 
+virNodeDeviceObjSetActive(obj, true);
 virNodeDeviceObjEndAPI();
 
 ret = 0;
@@ -1930,6 +1931,8 @@ udevSetupSystemDev(void)
 if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def)))
 goto cleanup;
 
+virNodeDeviceObjSetActive(obj, true);
+
 virNodeDeviceObjEndAPI();
 
 ret = 0;
-- 
2.26.3

[libvirt PATCH v6 09/30] nodedev: add persistence to virNodeDeviceObj

[Jonathon Jongsma]

Consistent with other objects (e.g. virDomainObj), add a field to
indicate whether the node device is persistent or transient.

Signed-off-by: Jonathon Jongsma 
---
 src/conf/virnodedeviceobj.c | 16 
 src/conf/virnodedeviceobj.h |  6 ++
 src/libvirt_private.syms|  2 ++
 3 files changed, 24 insertions(+)

diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index ab663fd5a0..ce84e4d8c1 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -40,6 +40,7 @@ struct _virNodeDeviceObj {
 bool skipUpdateCaps;/* whether to skip checking host caps,
used by testdriver */
 bool active;
+bool persistent;
 };
 
 struct _virNodeDeviceObjList {
@@ -1003,3 +1004,18 @@ virNodeDeviceObjSetActive(virNodeDeviceObj *obj,
 {
 obj->active = active;
 }
+
+
+bool
+virNodeDeviceObjIsPersistent(virNodeDeviceObj *obj)
+{
+return obj->persistent;
+}
+
+
+void
+virNodeDeviceObjSetPersistent(virNodeDeviceObj *obj,
+  bool persistent)
+{
+obj->persistent = persistent;
+}
diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h
index e786a70f51..7f682b9dca 100644
--- a/src/conf/virnodedeviceobj.h
+++ b/src/conf/virnodedeviceobj.h
@@ -128,3 +128,9 @@ virNodeDeviceObjIsActive(virNodeDeviceObj *obj);
 void
 virNodeDeviceObjSetActive(virNodeDeviceObj *obj,
   bool active);
+bool
+virNodeDeviceObjIsPersistent(virNodeDeviceObj *obj);
+
+void
+virNodeDeviceObjSetPersistent(virNodeDeviceObj *obj,
+  bool persistent);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 09957c943b..047314ec19 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1273,6 +1273,7 @@ virNetworkPortDefSaveStatus;
 virNodeDeviceObjEndAPI;
 virNodeDeviceObjGetDef;
 virNodeDeviceObjIsActive;
+virNodeDeviceObjIsPersistent;
 virNodeDeviceObjListAssignDef;
 virNodeDeviceObjListExport;
 virNodeDeviceObjListFindByName;
@@ -1286,6 +1287,7 @@ virNodeDeviceObjListNew;
 virNodeDeviceObjListNumOfDevices;
 virNodeDeviceObjListRemove;
 virNodeDeviceObjSetActive;
+virNodeDeviceObjSetPersistent;
 
 
 # conf/virnwfilterbindingdef.h
-- 
2.26.3

[libvirt PATCH v6 01/30] nodedev: capture and report stderror from mdevctl

[Jonathon Jongsma]

When an mdevctl command fails, there is not much information available
to the user about why it failed. This is partly because we were not
making use of the error message that mdevctl itself prints upon failure.

Signed-off-by: Jonathon Jongsma 
---
 src/node_device/node_device_driver.c | 46 +---
 src/node_device/node_device_driver.h |  6 ++--
 tests/nodedevmdevctltest.c   |  6 ++--
 3 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 543e5bb90a..3851a3568f 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -700,7 +700,8 @@ nodeDeviceFindAddressByName(const char *name)
 
 virCommandPtr
 nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
- char **uuid_out)
+ char **uuid_out,
+ char **errmsg)
 {
 virCommandPtr cmd;
 g_autofree char *json = NULL;
@@ -725,15 +726,17 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
 
 virCommandSetInputBuffer(cmd, json);
 virCommandSetOutputBuffer(cmd, uuid_out);
+virCommandSetErrorBuffer(cmd, errmsg);
 
 return cmd;
 }
 
 static int
-virMdevctlStart(virNodeDeviceDefPtr def, char **uuid)
+virMdevctlStart(virNodeDeviceDefPtr def, char **uuid, char **errmsg)
 {
 int status;
-g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlStartCommand(def, uuid);
+g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlStartCommand(def, uuid,
+ errmsg);
 if (!cmd)
 return -1;
 
@@ -754,6 +757,7 @@ nodeDeviceCreateXMLMdev(virConnectPtr conn,
 virNodeDeviceDefPtr def)
 {
 g_autofree char *uuid = NULL;
+g_autofree char *errmsg = NULL;
 
 if (!def->parent) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -761,9 +765,10 @@ nodeDeviceCreateXMLMdev(virConnectPtr conn,
 return NULL;
 }
 
-if (virMdevctlStart(def, ) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Unable to start mediated device"));
+if (virMdevctlStart(def, , ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unable to start mediated device '%s': %s"), 
def->name,
+   errmsg && errmsg[0] ? errmsg : "Unknown Error");
 return NULL;
 }
 
@@ -828,23 +833,25 @@ nodeDeviceCreateXML(virConnectPtr conn,
 
 
 virCommandPtr
-nodeDeviceGetMdevctlStopCommand(const char *uuid)
-{
-return virCommandNewArgList(MDEVCTL,
-"stop",
-"-u",
-uuid,
-NULL);
+nodeDeviceGetMdevctlStopCommand(const char *uuid, char **errmsg)
+{
+virCommandPtr cmd = virCommandNewArgList(MDEVCTL,
+ "stop",
+ "-u",
+ uuid,
+ NULL);
+virCommandSetErrorBuffer(cmd, errmsg);
+return cmd;
 
 }
 
 static int
-virMdevctlStop(virNodeDeviceDefPtr def)
+virMdevctlStop(virNodeDeviceDefPtr def, char **errmsg)
 {
 int status;
 g_autoptr(virCommand) cmd = NULL;
 
-cmd = nodeDeviceGetMdevctlStopCommand(def->caps->data.mdev.uuid);
+cmd = nodeDeviceGetMdevctlStopCommand(def->caps->data.mdev.uuid, errmsg);
 
 if (virCommandRun(cmd, ) < 0 || status != 0)
 return -1;
@@ -901,9 +908,12 @@ nodeDeviceDestroy(virNodeDevicePtr device)
 
 ret = 0;
 } else if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) {
-if (virMdevctlStop(def) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Unable to stop mediated device"));
+g_autofree char *errmsg = NULL;
+
+if (virMdevctlStop(def, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unable to destroy '%s': %s"), def->name,
+   errmsg && errmsg[0] ? errmsg : "Unknown error");
 goto cleanup;
 }
 ret = 0;
diff --git a/src/node_device/node_device_driver.h 
b/src/node_device/node_device_driver.h
index 2113d2b0a5..4a40aa51f6 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -115,6 +115,8 @@ nodeConnectNodeDeviceEventDeregisterAny(virConnectPtr conn,
 
 virCommandPtr
 nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
- char **uuid_out);
+ char **uuid_out,
+ char **errmsg);
 virCommandPtr
-nodeDeviceGetMdevctlStopCommand(const char *uuid);
+nodeDeviceGetMdevctlStopCommand(const char *uuid,
+char **errmsg);
diff --git 

[libvirt PATCH v6 02/30] tests: remove extra trailing semicolon

[Jonathon Jongsma]

The macro should not have a trailing semicolon so that when the macro is
used, the user can add a semicolon themselves.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 tests/nodedevmdevctltest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index c12feaac55..650e46a29f 100644
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -273,7 +273,7 @@ mymain(void)
 struct startTestInfo info = { virt_type, create, filename }; \
 DO_TEST_FULL("mdevctl start " filename, testMdevctlStartHelper, info); 
\
} \
-while (0);
+while (0)
 
 #define DO_TEST_START(filename) \
 DO_TEST_START_FULL("QEMU", CREATE_DEVICE, filename)
-- 
2.26.3

[libvirt PATCH v6 00/30] Add support for persistent mediated devices

[Jonathon Jongsma]

This patch series follows the previously-merged series which added support for
transient mediated devices. This series expands mdev support to include
persistent device definitions. Again, it relies on mdevctl as the backend.

It follows the common libvirt pattern of APIs by adding the following new APIs
for node devices:
- virNodeDeviceDefineXML() - defines a persistent device
- virNodeDeviceUndefine() - undefines a persistent device
- virNodeDeviceCreate() - starts a previously-defined device

It also adds virsh commands mapping to these new APIs: nodedev-define,
nodedev-undefine, and nodedev-start.

Since we rely on mdevctl for the definition of mediated devices, we need a way
to stay up-to-date with devices that are defined by mdevctl (outside of
libvirt).  The method for staying up-to-date is currently a little bit crude
due to the fact that mdevctl does not emit any events when new devices are
added or removed. As a workaround, we create a file monitor for the mdevctl
config directory and re-query mdevctl when we detect changes within that
directory. In the future, mdevctl may introduce a more elegant solution.

Changes in v6:
 - rebase to git master again
 - remove typedefs for various *Ptr types, since they're now discouraged in
   libvirt.

Changes in v5:
 - Rebase to git master
 - updated new API version info to 7.2.0
 - capture and relay stderr message from mdevctl
 - changed to using GHashTable functions directly instead of deprecated
   virHash functions
 - protected mdevctlMonitors with a mutex
 - added a couple patches to fix the 5s delay when defining a new mdev. These
   are currently separate patches added to the end of the series, but could be
   squashed into the earlier commits if that's preferred.
 - various other minor review fixes

Changes in v4:
 - rebase to git master
 - switch to throwaway thread for querying mdevctl
 - fixed a bug when removing devices because I was accidentally using
   virHashForEach() instead of virHashForeachSafe()
 - use DEFINED/UNDEFINED events instead of STARTED/STOPPED events
 - changes related to merging information about mdev devices from both udev a=
nd
   mdevctl:
   - Re-used the same function to copy extra data from mdevctl regardless of
 whether we're processing a udev event or a mdevctl event (recommended by
 Erik). This results in slightly more complex handling of the object
 lifetimes (see patch 9), but it consolidates some code.
   - nodeDeviceDefCopyFromMdevctl() previously only copied the data that was
 unique to mdevctl and didn't exist in udev. It now copies additional data
 (possibly overwriting some udev).  This solves a problem where a device =
is
 defined but not active (i.e.  we have not gotten any data from udev), and
 then changed (e.g. somebody calls 'mdevctl modify' to change the mdev
 type), but libvirt was not updating to the new definition.
 - fix a bug where we were mistakenly emitting 'update' events for devices th=
at
   had not changed
 - Added the ability to specify a uuid for an mdev via device XML.
 - split some commits into multiple patches
 - updated new API version info to 7.1.0
 - Fixed a bug reported by Yan Fu which hangs the client when attempting to
   destroy a nodedev that is in use by an active vm. See
   https://www.redhat.com/archives/libvir-list/2021-February/msg00116.html for
   solution suggested by Alex.
 - numerous smaller fixes from review findings

changes in v3:
 - streamlined tests -- removed some unnecessary duplication
 - split out patch to factor out node device name generation function
 - split nodeDeviceParseMdevctlChildDevice() into a separate function
 - added follow-up patch to remove space-padded alignment in header
 - refactored the mdevctl update handling significantly:
   - no longer a separate persistent thread that gets signaled by a timer
   - now piggybacks onto the existing udev thread and signals the thread in t=
he
 same way that the udev event does.
   - Daniel suggested spawning a throw-away thread to handle mdevctl updates,
 but that introduces the complexity of possibly serializing multiple
 throw-away threads (e.g. if we get an 'created' event followed immediate=
ly
 by a 'deleted' event, two threads may be spawned and we'd need to ensure
 they are properly ordered)
 - added virNodeDeviceObjListForEach() and virNodeDeviceObjListRemoveLocked()
   to simplify removing devices that are removed from mdevctl.
 - coding style fixes
 - NOTE: per Erik's request, I experimented with changing the way that mdevctl
   commands were generated and tested (e.g. introducing something like
   virMdevctlGetCommand(def, MDEVCTL_COMMAND_, ...)), but it was
   too invasive and awkward and didn't seem worthwhile

Changes in v2:
 - rebase to latest git master

Jonathon Jongsma (30):
  nodedev: capture and report stderror from mdevctl
  tests: remove extra trailing semicolon
  nodedev: introduce concept of 'active' node devices
  

[PATCH] qemu: don't reject virtiofs for qemu:///session

[Cole Robinson]

Currently libvirt rejects attempts to use virtiofs with
qemu:///session. Indeed virtiofs does not have a chance of working
with typical session usage, because virtiofsd needs multiple linux
capabilities to perform its job. The only way to do this with out
of the box qemu packaging is to run virtiofsd as root, so libvirtd
must run as root, so qemu:///system is required.

But it's possible that a custom environment could setuid or set
file capabilities on the virtiofsd binary. In this case qemu:///session
would work fine. This may be an option for kubevirt to help them
transition to using qemu:///session everywhere

Drop the two pieces that block virtiofs for qemu:///session. Attempts
to use virtiofs for stock qemu:///session will fail at qemu startup,
though it's a bit opaque:

error: Failed to start domain 'f32'
error: internal error: qemu unexpectedly closed the monitor: 
2021-03-26T16:26:12.459293Z qemu-system-x86_64: -device 
vhost-user-fs-pci,chardev=chr-vu-fs0,tag=/foovirtiofs,bus=pci.10,addr=0x0: 
Failed to write msg. Wrote -1 instead of 12.
2021-03-26T16:26:12.459317Z qemu-system-x86_64: -device 
vhost-user-fs-pci,chardev=chr-vu-fs0,tag=/foovirtiofs,bus=pci.10,addr=0x0: 
vhost_dev_init failed: Operation not permitted

Signed-off-by: Cole Robinson 
---
The SetUID/SetGID bits don't seem to be necessary for qemu:///system
usage AFAICT, but it's a bit tough to decode virSetUIDGIDWithCaps.
virtiofsd is meticulous about managing its capability set though

 src/qemu/qemu_validate.c | 7 +--
 src/qemu/qemu_virtiofs.c | 4 
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 6043f974ce..d4079f6b67 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4053,7 +4053,7 @@ qemuValidateDomainDeviceDefGraphics(const 
virDomainGraphicsDef *graphics,
 static int
 qemuValidateDomainDeviceDefFS(virDomainFSDefPtr fs,
   const virDomainDef *def,
-  virQEMUDriverPtr driver,
+  virQEMUDriverPtr driver G_GNUC_UNUSED,
   virQEMUCapsPtr qemuCaps)
 {
 if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) {
@@ -4107,11 +4107,6 @@ qemuValidateDomainDeviceDefFS(virDomainFSDefPtr fs,
_("virtiofs does not yet support read-only mode"));
 return -1;
 }
-if (!driver->privileged) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("virtiofs is not yet supported in session mode"));
-return -1;
-}
 if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("virtiofs only supports passthrough accessmode"));
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 2e239cad66..0bb4e3c0d1 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -214,10 +214,6 @@ qemuVirtioFSStart(virLogManagerPtr logManager,
 if (!(cmd = qemuVirtioFSBuildCommandLine(cfg, fs, )))
 goto cleanup;
 
-/* so far only running as root is supported */
-virCommandSetUID(cmd, 0);
-virCommandSetGID(cmd, 0);
-
 virCommandSetPidFile(cmd, pidfile);
 virCommandSetOutputFD(cmd, );
 virCommandSetErrorFD(cmd, );
-- 
2.30.2

[PATCH 0/1] vircgroup: Fix nested cgroup on cleanup

[Eric Farman]

Hi Pavel, et al,

Running Fedora 33 KVM/QEMU on s390x, I recently noticed a couple
of oddities when shutting down my guests, which I bisected between
7.0.0 and 7.1.0 to your commit:

  commit 184245f53b94fc84f727eb6e8a2aa52df02d69c0
  Author: Pavel Hrdina 
  Date:   Tue Feb 9 12:33:53 2021 +0100

  vircgroup: introduce nested cgroup to properly work with systemd

The attached patch is based on some of the tweaks you did to use the
parent/nested pointers in a cgroup, rather than a cgroup itself.
It fixes both problems I'm encountering, but as this code is quite
unfamiliar to me, I might well be way off track and would appreciate
your feedback in the matter.

Apologies for hitting this on the same day as freeze; I had not
noticed the messages previously, and started digging yesterday when
I noted the number of file descriptors left open by libvirt.

Thanks,
Eric

Eric Farman (1):
  vircgroup: Cleanup nested cgroups

 src/util/vircgroup.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH 1/1] vircgroup: Cleanup nested cgroups

[Eric Farman]

The introduction of nested cgroups used a little macro
virCgroupGetNested() to retrieve the nested cgroup
pointer, if one exists. But this macro isn't used when
removing cgroups, resulting in some messages:

  Mar 25 20:55:17 fedora33 libvirtd[955]: unable to open 
'/sys/fs/cgroup/machine.slice/machine-qemu\x2d1\x2dguest.scope/': No such file 
or directory
  Mar 25 20:55:17 fedora33 libvirtd[955]: Failed to remove cgroup for guest

That directory exists while the guest is running, as it
was created by systemd/machined, so the code probably meant
to open the libvirt/ subdirectory from that point.

Similarly, there happen to be BPF-related file descriptors
that don't get cleaned up in this process too, because they
are anchored off the nested cgroup location:

  [test@fedora33 ~]# ls /proc/$(pgrep libvirtd)/fd/* | wc -l
  35
  [test@fedora33 ~]# virsh create guest.xml
  Domain 'guest' created from guest.xml

  [test@fedora33 ~]# ls /proc/$(pgrep libvirtd)/fd/* | wc -l
  42
  [test@fedora33 ~]# virsh shutdown guest
  Domain 'guest' is being shutdown

  [test@fedora33 ~]# ls /proc/$(pgrep libvirtd)/fd/* | wc -l
  37
  [test@fedora33 ~]# virsh create guest.xml
  Domain 'guest' created from guest.xml

  [test@fedora33 ~]# ls /proc/$(pgrep libvirtd)/fd/* | wc -l
  44
  [test@fedora33 ~]# virsh shutdown guest
  Domain 'guest' is being shutdown

  [test@fedora33 ~]# ls /proc/$(pgrep libvirtd)/fd/* | wc -l
  39

Let's fix this by using the same macro when removing cgroups,
so that it picks up the right structure and can remove the
associated resources properly.

Fixes: 184245f53b94 ("vircgroup: introduce nested cgroup to properly work with 
systemd")
Signed-off-by: Eric Farman 
---
 src/util/vircgroup.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 5b6097c335..e606cbfe0b 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -2613,11 +2613,12 @@ virCgroupRemoveRecursively(char *grppath)
 int
 virCgroupRemove(virCgroupPtr group)
 {
+virCgroupPtr parent = virCgroupGetNested(group);
 size_t i;
 
 for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) {
-if (group->backends[i]) {
-int rc = group->backends[i]->remove(group);
+if (parent->backends[i]) {
+int rc = parent->backends[i]->remove(parent);
 if (rc < 0)
 return rc;
 }
-- 
2.25.1

Re: [libvirt PATCH 2/2] ci: Call meson consistently

[Andrea Bolognani]

On Fri, 2021-03-26 at 17:00 +0100, Erik Skultety wrote:
> On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
> > We should always pass --werror and display the contents of the
> > log file in case of failure.
> > 
> > Signed-off-by: Andrea Bolognani 
> > ---
> Reviewed-by: Erik Skultety 

Even though you didn't spell that out explicitly, I assume you're
okay with me pushing this series during the freeze?

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH 1/2] ci: Don't use --prefix with meson for Cirrus CI builds

[Erik Skultety]

On Fri, Mar 26, 2021 at 11:35:01AM +0100, Andrea Bolognani wrote:
> It's no longer used.
> 
> Signed-off-by: Andrea Bolognani 
> ---
Reviewed-by: Erik Skultety 

Re: [libvirt PATCH 2/2] ci: Call meson consistently

[Erik Skultety]

On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
> We should always pass --werror and display the contents of the
> log file in case of failure.
> 
> Signed-off-by: Andrea Bolognani 
> ---
Reviewed-by: Erik Skultety 

Re: [PATCH] qemu: virtiofs: support

[Peter Krempa]

On Fri, Mar 26, 2021 at 11:37:48 -0400, Cole Robinson wrote:
> Add a new XML element
> 
> 
>   
> 
>   
> 
> 
> Which maps to `virtiofsd -o sandbox=chroot|namespace`, which was added
> in qemu 5.2.0:
> 
> https://git.qemu.org/?p=qemu.git;a=commit;h=06844584b62a43384642f7243b0fc01c9fff0fc7
> 
> Signed-off-by: Cole Robinson 
> ---
>  docs/formatdomain.rst |  4 
>  docs/schemas/domaincommon.rng | 12 ++
>  src/conf/domain_conf.c| 23 +++
>  src/conf/domain_conf.h| 10 
>  src/libvirt_private.syms  |  1 +
>  src/qemu/qemu_virtiofs.c  |  2 ++
>  .../vhost-user-fs-fd-memory.xml   |  1 +
>  7 files changed, 53 insertions(+)

Please split the commit as it's usual for libvirt patches.

Also a test case modifying any of the .args files in qemuxml2argv test
is missing.

> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 9392c80113..9dda39dbcb 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -3234,6 +3234,7 @@ A directory on the host that can be accessed directly 
> from the guest.
>   
>   
>  
> +
>  
>   
>   
> @@ -3358,6 +3359,9 @@ A directory on the host that can be accessed directly 
> from the guest.
> ``cache`` element, possible ``mode`` values being ``none`` and ``always``.
> Locking can be controlled via the ``lock`` element - attributes ``posix`` 
> and
> ``flock`` both accepting values ``on`` or ``off``. ( :since:`Since 6.2.0` 
> )
> +   The sandboxing method used by virtiofsd can be configured with the 
> ``sandbox``
> +   element, possible ``mode`` values being ``namespace`` and
> +   ``chroot``. ( :since:`Since 7.2.0` )

Is there any reasonable short explanation of differences? Or perhaps
link to virtiofs docs to clarify what that the modes do?


>  ``source``
> The resource on the host that is being accessed in the guest. The ``name``
> attribute must be used with ``type='template'``, and the ``dir`` attribute
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 1dbfc68f18..6404ebf210 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2960,6 +2960,18 @@
>  
>
>  
> +
> +  
> +
> +  
> +
> +  namespace
> +  chroot
> +
> +  
> +
> +  
> +
>  
>
>  
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b0eba9f7bd..70a900ee25 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -538,6 +538,13 @@ VIR_ENUM_IMPL(virDomainFSCacheMode,
>"always",
>  );
>  
> +VIR_ENUM_IMPL(virDomainFSSandboxMode,
> +  VIR_DOMAIN_FS_SANDBOX_MODE_LAST,
> +  "default",
> +  "namespace",
> +  "chroot",
> +);
> +
>  
>  VIR_ENUM_IMPL(virDomainNet,
>VIR_DOMAIN_NET_TYPE_LAST,
> @@ -10373,6 +10380,7 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
>  g_autofree char *binary = virXPathString("string(./binary/@path)", 
> ctxt);
>  g_autofree char *xattr = virXPathString("string(./binary/@xattr)", 
> ctxt);
>  g_autofree char *cache = 
> virXPathString("string(./binary/cache/@mode)", ctxt);
> +g_autofree char *sandbox = 
> virXPathString("string(./binary/sandbox/@mode)", ctxt);
>  g_autofree char *posix_lock = 
> virXPathString("string(./binary/lock/@posix)", ctxt);
>  g_autofree char *flock = 
> virXPathString("string(./binary/lock/@flock)", ctxt);
>  int val;
> @@ -10406,6 +10414,16 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
>  def->cache = val;
>  }
>  
> +if (sandbox) {
> +if ((val = virDomainFSSandboxModeTypeFromString(sandbox)) <= 0) {
> +virReportError(VIR_ERR_XML_ERROR,
> +   _("cannot parse sandbox mode '%s' for 
> virtiofs"),
> +   sandbox);
> +goto error;
> +}
> +def->sandbox = val;
> +}
> +
>  if (posix_lock) {
>  if ((val = virTristateSwitchTypeFromString(posix_lock)) <= 0) {
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -25483,6 +25501,11 @@ virDomainFSDefFormat(virBufferPtr buf,
>virDomainFSCacheModeTypeToString(def->cache));
>  }
>  
> +if (def->sandbox != VIR_DOMAIN_FS_SANDBOX_MODE_DEFAULT) {
> +virBufferAsprintf(, "\n",
> +  
> virDomainFSSandboxModeTypeToString(def->sandbox));
> +}
> +
>  if (def->posix_lock != VIR_TRISTATE_SWITCH_ABSENT) {
>

[PATCH] qemu: virtiofs: support

[Cole Robinson]

Add a new XML element


  

  


Which maps to `virtiofsd -o sandbox=chroot|namespace`, which was added
in qemu 5.2.0:

https://git.qemu.org/?p=qemu.git;a=commit;h=06844584b62a43384642f7243b0fc01c9fff0fc7

Signed-off-by: Cole Robinson 
---
 docs/formatdomain.rst |  4 
 docs/schemas/domaincommon.rng | 12 ++
 src/conf/domain_conf.c| 23 +++
 src/conf/domain_conf.h| 10 
 src/libvirt_private.syms  |  1 +
 src/qemu/qemu_virtiofs.c  |  2 ++
 .../vhost-user-fs-fd-memory.xml   |  1 +
 7 files changed, 53 insertions(+)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 9392c80113..9dda39dbcb 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3234,6 +3234,7 @@ A directory on the host that can be accessed directly 
from the guest.
  
  
 
+
 
  
  
@@ -3358,6 +3359,9 @@ A directory on the host that can be accessed directly 
from the guest.
``cache`` element, possible ``mode`` values being ``none`` and ``always``.
Locking can be controlled via the ``lock`` element - attributes ``posix`` 
and
``flock`` both accepting values ``on`` or ``off``. ( :since:`Since 6.2.0` )
+   The sandboxing method used by virtiofsd can be configured with the 
``sandbox``
+   element, possible ``mode`` values being ``namespace`` and
+   ``chroot``. ( :since:`Since 7.2.0` )
 ``source``
The resource on the host that is being accessed in the guest. The ``name``
attribute must be used with ``type='template'``, and the ``dir`` attribute
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 1dbfc68f18..6404ebf210 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2960,6 +2960,18 @@
 
   
 
+
+  
+
+  
+
+  namespace
+  chroot
+
+  
+
+  
+
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b0eba9f7bd..70a900ee25 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -538,6 +538,13 @@ VIR_ENUM_IMPL(virDomainFSCacheMode,
   "always",
 );
 
+VIR_ENUM_IMPL(virDomainFSSandboxMode,
+  VIR_DOMAIN_FS_SANDBOX_MODE_LAST,
+  "default",
+  "namespace",
+  "chroot",
+);
+
 
 VIR_ENUM_IMPL(virDomainNet,
   VIR_DOMAIN_NET_TYPE_LAST,
@@ -10373,6 +10380,7 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
 g_autofree char *binary = virXPathString("string(./binary/@path)", 
ctxt);
 g_autofree char *xattr = virXPathString("string(./binary/@xattr)", 
ctxt);
 g_autofree char *cache = 
virXPathString("string(./binary/cache/@mode)", ctxt);
+g_autofree char *sandbox = 
virXPathString("string(./binary/sandbox/@mode)", ctxt);
 g_autofree char *posix_lock = 
virXPathString("string(./binary/lock/@posix)", ctxt);
 g_autofree char *flock = 
virXPathString("string(./binary/lock/@flock)", ctxt);
 int val;
@@ -10406,6 +10414,16 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
 def->cache = val;
 }
 
+if (sandbox) {
+if ((val = virDomainFSSandboxModeTypeFromString(sandbox)) <= 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("cannot parse sandbox mode '%s' for 
virtiofs"),
+   sandbox);
+goto error;
+}
+def->sandbox = val;
+}
+
 if (posix_lock) {
 if ((val = virTristateSwitchTypeFromString(posix_lock)) <= 0) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -25483,6 +25501,11 @@ virDomainFSDefFormat(virBufferPtr buf,
   virDomainFSCacheModeTypeToString(def->cache));
 }
 
+if (def->sandbox != VIR_DOMAIN_FS_SANDBOX_MODE_DEFAULT) {
+virBufferAsprintf(, "\n",
+  
virDomainFSSandboxModeTypeToString(def->sandbox));
+}
+
 if (def->posix_lock != VIR_TRISTATE_SWITCH_ABSENT) {
 virBufferAsprintf(, " posix='%s'",
   virTristateSwitchTypeToString(def->posix_lock));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0b8895bbdf..d77b04847b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -846,6 +846,14 @@ typedef enum {
 VIR_DOMAIN_FS_CACHE_MODE_LAST
 } virDomainFSCacheMode;
 
+typedef enum {
+VIR_DOMAIN_FS_SANDBOX_MODE_DEFAULT = 0,
+VIR_DOMAIN_FS_SANDBOX_MODE_NAMESPACE,
+VIR_DOMAIN_FS_SANDBOX_MODE_CHROOT,
+
+VIR_DOMAIN_FS_SANDBOX_MODE_LAST
+} virDomainFSSandboxMode;
+
 struct _virDomainFSDef {

Re: [libvirt PATCH v5 00/30] Add support for persistent mediated devices

[Erik Skultety]

On Fri, Mar 26, 2021 at 09:37:34AM -0500, Jonathon Jongsma wrote:
> On Fri, 26 Mar 2021 07:27:46 +0100
> Erik Skultety  wrote:
> 
> > On Thu, Mar 25, 2021 at 11:49:49AM -0500, Jonathon Jongsma wrote:
> > > Just a friendly ping ;)  
> > 
> > I'm sorry I've been neglecting this for the past 3 weeks or so, I'll
> > dive right into it starting Monday next week, we're entering freeze
> > today anyway. Would you mind resending a rebased version? There were
> > a couple of conflicts wrt to RPC, I fixed them all locally, but it's
> > always better to start the review with fresh content.
> > 
> > Thanks,
> > Erik
> 
> Sure thing. I'll rebase and send a new series before the end of the
> day. Perhaps I should also omit the *Ptr typedefs since they're
> discouraged now?

Sure, I'm more than okay with that.

Erik

Re: [libvirt PATCH v5 00/30] Add support for persistent mediated devices

[Jonathon Jongsma]

On Fri, 26 Mar 2021 07:27:46 +0100
Erik Skultety  wrote:

> On Thu, Mar 25, 2021 at 11:49:49AM -0500, Jonathon Jongsma wrote:
> > Just a friendly ping ;)  
> 
> I'm sorry I've been neglecting this for the past 3 weeks or so, I'll
> dive right into it starting Monday next week, we're entering freeze
> today anyway. Would you mind resending a rebased version? There were
> a couple of conflicts wrt to RPC, I fixed them all locally, but it's
> always better to start the review with fresh content.
> 
> Thanks,
> Erik

Sure thing. I'll rebase and send a new series before the end of the
day. Perhaps I should also omit the *Ptr typedefs since they're
discouraged now?


Jonathon


> > 
> > On Tue,  2 Mar 2021 16:30:35 -0600
> > Jonathon Jongsma  wrote:
> >   
> > > This patch series follows the previously-merged series which added
> > > support for transient mediated devices. This series expands mdev
> > > support to include persistent device definitions. Again, it
> > > relies on mdevctl as the backend.
> > > 
> > > It follows the common libvirt pattern of APIs by adding the
> > > following new APIs for node devices:
> > > - virNodeDeviceDefineXML() - defines a persistent device
> > > - virNodeDeviceUndefine() - undefines a persistent device
> > > - virNodeDeviceCreate() - starts a previously-defined device
> > > 
> > > It also adds virsh commands mapping to these new APIs:
> > > nodedev-define, nodedev-undefine, and nodedev-start.
> > > 
> > > Since we rely on mdevctl for the definition of mediated devices,
> > > we need a way to stay up-to-date with devices that are defined by
> > > mdevctl (outside of libvirt).  The method for staying up-to-date
> > > is currently a little bit crude due to the fact that mdevctl does
> > > not emit any events when new devices are added or removed. As a
> > > workaround, we create a file monitor for the mdevctl config
> > > directory and re-query mdevctl when we detect changes within that
> > > directory. In the future, mdevctl may introduce a more elegant
> > > solution.
> > > 
> > > Changes in v5:
> > >  - Rebase to git master
> > >  - updated new API version info to 7.2.0
> > >  - capture and relay stderr message from mdevctl
> > >  - changed to using GHashTable functions directly instead of
> > > deprecated virHash functions
> > >  - protected mdevctlMonitors with a mutex
> > >  - added a couple patches to fix the 5s delay when defining a new
> > > mdev. These are currently separate patches added to the end of the
> > > series, but could be squashed into the earlier commits if that's
> > > preferred.
> > >  - various other minor review fixes
> > > 
> > > Changes in v4:
> > >  - rebase to git master
> > >  - switch to throwaway thread for querying mdevctl
> > >  - fixed a bug when removing devices because I was accidentally
> > > using virHashForEach() instead of virHashForeachSafe()
> > >  - use DEFINED/UNDEFINED events instead of STARTED/STOPPED events
> > >  - changes related to merging information about mdev devices from
> > > both udev a= nd
> > >mdevctl:
> > >- Re-used the same function to copy extra data from mdevctl
> > > regardless of whether we're processing a udev event or a mdevctl
> > > event (recommended by Erik). This results in slightly more complex
> > > handling of the object lifetimes (see patch 9), but it
> > > consolidates some code.
> > >- nodeDeviceDefCopyFromMdevctl() previously only copied the
> > > data that was unique to mdevctl and didn't exist in udev. It now
> > > copies additional data (possibly overwriting some udev).  This
> > > solves a problem where a device = is
> > >  defined but not active (i.e.  we have not gotten any data
> > > from udev), and then changed (e.g. somebody calls 'mdevctl
> > > modify' to change the mdev type), but libvirt was not updating to
> > > the new definition.
> > >  - fix a bug where we were mistakenly emitting 'update' events for
> > > devices th= at
> > >had not changed
> > >  - Added the ability to specify a uuid for an mdev via device XML.
> > >  - split some commits into multiple patches
> > >  - updated new API version info to 7.1.0
> > >  - Fixed a bug reported by Yan Fu which hangs the client when
> > > attempting to destroy a nodedev that is in use by an active vm.
> > > See
> > > https://www.redhat.com/archives/libvir-list/2021-February/msg00116.html
> > > for solution suggested by Alex.
> > >  - numerous smaller fixes from review findings
> > > 
> > > changes in v3:
> > >  - streamlined tests -- removed some unnecessary duplication
> > >  - split out patch to factor out node device name generation
> > > function
> > >  - split nodeDeviceParseMdevctlChildDevice() into a separate
> > > function
> > >  - added follow-up patch to remove space-padded alignment in
> > > header
> > >  - refactored the mdevctl update handling significantly:
> > >- no longer a separate persistent thread that gets signaled by
> > > a timer
> > >- now piggybacks onto the existing udev thread 

Re: [libvirt PATCH 1/4] meson: Print custom message when GNU grep is not installed

[Andrea Bolognani]

On Fri, 2021-03-26 at 11:15 +0100, Erik Skultety wrote:
> On Wed, Mar 24, 2021 at 07:37:57PM +0100, Andrea Bolognani wrote:
> > Currently, if GNU grep is not installed on a FreeBSD system the
> > configuration step will fail with
> > 
> >   Program grep found: YES (/usr/bin/grep)
> >   Program /usr/local/bin/grep found: NO
> > 
> >   ERROR: Program '/usr/local/bin/grep' not found
> 
> Stupid question, but how does one need to invoke meson to actually hit ^this.
> On my local FreeBSD 12:
> 
> $ ls -l /usr/local/bin/grep
> ls: /usr/local/bin/grep: No such file or directory
> 
> $ pkg info | grep gnu
> gnupg-2.2.26
> gnutls-3.6.15
> 
> $ pwd
> /home/test/libvirt
> 
> $ git describe
> v7.1.0-328-g5f9330e724
> 
> $ meson --version
> 0.56.0
> 
> $ meson build -Dsystem=true | grep grep
> Program grep found: YES (/usr/bin/grep)

This doesn't affect FreeBSD 12, but it can be hit without any
particular setup on -CURRENT, plus of course FreeBSD 13 once that's
released.

See

  commit 7dd7ddac500cd3f453cd19606904ae406f5ffe03
  Author: Roman Bogorodskiy 
  Date:   Tue Mar 2 18:31:36 2021 +0400

build-aux: require GNU grep on FreeBSD

FreeBSD 13.x and newer ship BSD grep which apparently has some
performance issues causing certain syntax check tests to run longer than
the default 30 seconds timeout used by meson.

However, GNU grep is still available through the textproc/gnugrep port,
so require it on FreeBSD if /usr/bin/grep is a BSD grep to make checks
pass in a reasonable time.

Signed-off-by: Roman Bogorodskiy 
Reviewed-by: Ján Tomko 

for more information.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: Re: [PATCH] vm : forbid to start a removing vm

[Hogan Wang]

> On Fri, Mar 05, 2021 at 09:19:52AM +0800, Hogan Wang wrote:
> >From: Zhuang Shengen 
> >
> >When a vm is doing migration phase confirm, and then start it 
> >concurrently, it will lead to the vm out of libvirtd control.
> >
> >Cause Analysis:
> >1. thread1 migrate vm out.
> >2. thread2 start the migrating vm.
> >3. thread1 remove vm from domain list after migrate success.
> >4. thread2 acquired the vm job success and start the vm.
> >5. cannot find the vm any more by 'virsh list' command. Actually,
> >   the started vm is not exist in the domain list.
> >
> >Solution:
> >Check the vm->removing state before start.
> >
> 
> Well, this would only fix starting it, but there could be other ways
> that domain can be started, right?  Like restoring it from a save.
> 
> Anyway, I think the issue here is that the CreateWithFlags is even
> able to get a job started, I think you should look into that.
> 

Yes, it's, a removed vm begin a job may cause unanticipated results.
Therefore, I think qemuDomainObjBeginJobInternal should return error
after a removed vm acquire job success. I will push an new patch to
fix it.

> >
> >Signed-off-by: Zhuang Shengen 
> >Reviewed-by: Hogan Wang 
> >---
> > src/qemu/qemu_driver.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> >diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 
> >d1a3659774..a5dfea94cb 100644
> >--- a/src/qemu/qemu_driver.c
> >+++ b/src/qemu/qemu_driver.c
> >@@ -6637,6 +6637,12 @@ qemuDomainCreateWithFlags(virDomainPtr dom, unsigned 
> >int flags)
> > goto endjob;
> > }
> >
> >+if (vm->removing) {
> >+virReportError(VIR_ERR_OPERATION_INVALID,
> >+   "%s", _("domain is already removing"));
> >+goto endjob;
> >+}
> >+
> > if (qemuDomainObjStart(dom->conn, driver, vm, flags,
> >QEMU_ASYNC_JOB_START) < 0)
> > goto endjob;
> >--
> >2.23.0
> >
> >

Entering freeze for libvirt-7.2.0

[Jiri Denemark]

I have just tagged v7.2.0-rc1 in the repository and pushed signed
tarballs and source RPMs to https://libvirt.org/sources/

Please give the release candidate some testing and in case you find a
serious issue which should have a fix in the upcoming release, feel
free to reply to this thread to make sure the issue is more visible.

If you have not done so yet, please update NEWS.rst to document any
significant change you made since the last release.

Thanks,

Jirka

Re: [libvirt PATCH v3 0/3] qemuProcessUpdateGuestCPU: Check host cpu for forbidden features

[Jiri Denemark]

On Tue, Mar 23, 2021 at 11:01:52 +0100, Tim Wiederhake wrote:
> V1: https://listman.redhat.com/archives/libvir-list/2021-February/msg01275.ht=
> ml
> V2: https://listman.redhat.com/archives/libvir-list/2021-February/msg01289.ht=
> ml
> 
> Changes since V2:
> * Factored out into seperate function in src/cpu/cpu.c
> * Made virCPUDefFindFeature work on const pointers
> 
> Tim Wiederhake (3):
>   virCPUDefFindFeature: Make first argument const ptr
>   cpu: Introduce virCPUCheckForbiddenFeatures
>   qemuProcessUpdateGuestCPU: Check host cpu for forbidden features
> 
>  src/conf/cpu_conf.c  |  2 +-
>  src/conf/cpu_conf.h  |  2 +-
>  src/cpu/cpu.c| 37 +
>  src/cpu/cpu.h|  6 ++
>  src/libvirt_private.syms |  1 +
>  src/qemu/qemu_process.c  | 10 ++
>  6 files changed, 56 insertions(+), 2 deletions(-)
> 
> --=20
> 2.26.2

Reviewed-by: Jiri Denemark 

and pushed, thanks.

[libvirt PATCH 2/2] ci: Call meson consistently

[Andrea Bolognani]

We should always pass --werror and display the contents of the
log file in case of failure.

Signed-off-by: Andrea Bolognani 
---
 .gitlab-ci.yml | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index cbc1292839..8b7df68f47 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -529,7 +529,7 @@ website:
   before_script:
 - *script_variables
   script:
-- meson build --prefix=$(pwd)/vroot || (cat build/meson-logs/meson-log.txt 
&& exit 1)
+- meson build --werror --prefix=$(pwd)/vroot || (cat 
build/meson-logs/meson-log.txt && exit 1)
 - ninja -C build install-web
 - mv vroot/share/doc/libvirt/html/ website
   artifacts:
@@ -549,7 +549,7 @@ codestyle:
   before_script:
 - *script_variables
   script:
-- meson build || (cat build/meson-logs/meson-log.txt && exit 1)
+- meson build --werror || (cat build/meson-logs/meson-log.txt && exit 1)
 - ninja -C build libvirt-pot-dep
 - meson test -C build --suite syntax-check --no-rebuild || (cat 
build/meson-logs/testlog.txt && exit 1)
 
@@ -567,7 +567,7 @@ potfile:
   before_script:
 - *script_variables
   script:
-- meson build || (cat build/meson-logs/meson-log.txt && exit 1)
+- meson build --werror || (cat build/meson-logs/meson-log.txt && exit 1)
 - ninja -C build libvirt-pot-dep
 - ninja -C build libvirt-pot
 - cp po/libvirt.pot libvirt.pot
@@ -605,7 +605,7 @@ coverity:
   script:
 - curl https://scan.coverity.com/download/linux64 --form 
project=$COVERITY_SCAN_PROJECT_NAME --form token=$COVERITY_SCAN_TOKEN -o 
/tmp/cov-analysis-linux64.tgz
 - tar xfz /tmp/cov-analysis-linux64.tgz
-- meson build
+- meson build --werror || (cat build/meson-logs/meson-log.txt && exit 1)
 - cov-analysis-linux64-*/bin/cov-build --dir cov-int ninja -C build
 - tar cfz cov-int.tar.gz cov-int
 - curl 
https://scan.coverity.com/builds?project=$COVERITY_SCAN_PROJECT_NAME --form 
token=$COVERITY_SCAN_TOKEN --form email=$GITLAB_USER_EMAIL --form 
file=@cov-int.tar.gz --form version="$(git describe --tags)" --form 
description="$(git describe --tags) / $CI_COMMIT_TITLE / 
$CI_COMMIT_REF_NAME:$CI_PIPELINE_ID"
-- 
2.26.3

[libvirt PATCH 1/2] ci: Don't use --prefix with meson for Cirrus CI builds

[Andrea Bolognani]

It's no longer used.

Signed-off-by: Andrea Bolognani 
---
 ci/cirrus/build.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ci/cirrus/build.yml b/ci/cirrus/build.yml
index a6b13b947c..2d3c46a77c 100644
--- a/ci/cirrus/build.yml
+++ b/ci/cirrus/build.yml
@@ -20,5 +20,5 @@ build_task:
 - git fetch origin "$CI_COMMIT_REF_NAME"
 - git reset --hard "$CI_COMMIT_SHA"
   build_script:
-- meson build --prefix=$(pwd)/install-root
+- meson build
 - ninja -C build dist
-- 
2.26.3

[libvirt PATCH 0/2] ci: Call meson consistently

[Andrea Bolognani]

Test pipeline:

  https://gitlab.com/abologna/libvirt/-/pipelines/276971193

Andrea Bolognani (2):
  ci: Don't use --prefix with meson for Cirrus CI builds
  ci: Call meson consistently

 .gitlab-ci.yml  | 8 
 ci/cirrus/build.yml | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.26.3

Re: [PATCH] serialize access pci_get_strings function with a mutex

[Michal Privoznik]

On 3/26/21 10:36 AM, wangjian (AN) wrote:

Ok. I agree.



Alright, then:

Reviewed-by: Michal Privoznik 

And congratulations on your first libvirt contribution!

Michal

Re: [libvirt PATCH 1/4] meson: Print custom message when GNU grep is not installed

[Erik Skultety]

On Wed, Mar 24, 2021 at 07:37:57PM +0100, Andrea Bolognani wrote:
> Currently, if GNU grep is not installed on a FreeBSD system the
> configuration step will fail with
> 
>   Program grep found: YES (/usr/bin/grep)
>   Program /usr/local/bin/grep found: NO
> 
>   ERROR: Program '/usr/local/bin/grep' not found

Stupid question, but how does one need to invoke meson to actually hit ^this.
On my local FreeBSD 12:

$ ls -l /usr/local/bin/grep
ls: /usr/local/bin/grep: No such file or directory

$ pkg info | grep gnu
gnupg-2.2.26
gnutls-3.6.15

$ pwd
/home/test/libvirt

$ git describe
v7.1.0-328-g5f9330e724

$ meson --version
0.56.0

$ meson build -Dsystem=true | grep grep
Program grep found: YES (/usr/bin/grep)

Erik

Re: [PATCH] serialize access pci_get_strings function with a mutex

[wangjian (AN)]

Ok. I agree.

On 2021/3/26 17:10, Michal Privoznik wrote:
> There's no need to CC random developers - we are subscribed to the list.
> 
> On 3/26/21 4:21 AM, wangjian wrote:> serialize access pci_get_strings 
> function with a mutex.> > nodedev-init thread:
>> nodeStateInitializeEnumerate ->
>>udevEnumerateDevices->
>>  udevProcessDeviceListEntry ->
>>udevAddOneDevice ->
>>  udevGetDeviceDetails->
>>udevProcessPCI ->
>>  udevTranslatePCIIds ->
>>pci_get_strings -> (libpciaccess)
>>  find_device_name ->
>>populate_vendor ->
>>  d = realloc( vend->devices, (vend->num_devices + 1), * 
>> sizeof( struct pci_device_leaf ) );
>>  vend->num_devices++;
>>
>> udev-event thread:
>> udevEventHandleThread ->
>>udevHandleOneDevice ->
>>  udevAddOneDevice->
>>udevGetDeviceDetails->
>>  udevProcessPCI ->
>>udevTranslatePCIIds ->
>>  pci_get_strings -> (libpciaccess)
>>find_device_name ->
>>  populate_vendor ->
>>d = realloc( vend->devices, (vend->num_devices + 1), * 
>> sizeof( struct pci_device_leaf ) );
>>vend->num_devices++;
>>
>> Since the functions provided by libpciaccess are not thread-safe,
>> when the udev-event and nodedev-init threads of libvirt call
>> the pci_get_strings function provided by libpaciaccess at the same time.
>> It will appear that for the same address, realloc a large space first,
>> and then realloc a small space, which triggers the 'invalid next size' of 
>> realloc,
>> leading to the thread core.
>>
>> Signed-off-by: WangJian 
>> ---
>>   src/node_device/node_device_udev.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/src/node_device/node_device_udev.c 
>> b/src/node_device/node_device_udev.c
>> index 3f28858..cc752ba 100644
>> --- a/src/node_device/node_device_udev.c
>> +++ b/src/node_device/node_device_udev.c
>> @@ -331,6 +331,7 @@ udevGenerateDeviceName(struct udev_device *device,
>>   return 0;
>>   }
>>
>> +static pthread_mutex_t g_pciaccess_mutex = PTHREAD_MUTEX_INITIALIZER;
> 
> We tend to use virMutex (even though it is pthread_mutex under the hood).
> 
>>
>>   static int
>>   udevTranslatePCIIds(unsigned int vendor,
>> @@ -350,11 +351,13 @@ udevTranslatePCIIds(unsigned int vendor,
>>   m.match_data = 0;
>>
>>   /* pci_get_strings returns void */
>> +pthread_mutex_lock(_pciaccess_mutex);
>>   pci_get_strings(,
>>   _name,
>>   _name,
>>   NULL,
>>   NULL);
>> +pthread_mutex_unlock(_pciaccess_mutex);
>>
>>   *vendor_string = g_strdup(vendor_name);
>>   *product_string = g_strdup(device_name);
>>
> 
> Apart from that, the patch is correct. I'd squash this in and push if you 
> agree:
> 
> diff --git i/src/node_device/node_device_udev.c 
> w/src/node_device/node_device_udev.c
> index cc752bad32..3daa5c90ad 100644
> --- i/src/node_device/node_device_udev.c
> +++ w/src/node_device/node_device_udev.c
> @@ -331,7 +331,7 @@ udevGenerateDeviceName(struct udev_device *device,
>  return 0;
>  }
> 
> -static pthread_mutex_t g_pciaccess_mutex = PTHREAD_MUTEX_INITIALIZER;
> +static virMutex pciaccessMutex = VIR_MUTEX_INITIALIZER;
> 
>  static int
>  udevTranslatePCIIds(unsigned int vendor,
> @@ -350,14 +350,14 @@ udevTranslatePCIIds(unsigned int vendor,
>  m.device_class_mask = 0;
>  m.match_data = 0;
> 
> -/* pci_get_strings returns void */
> -pthread_mutex_lock(_pciaccess_mutex);
> +/* pci_get_strings returns void and unfortunately is not thread safe. */
> +virMutexLock();
>  pci_get_strings(,
>  _name,
>  _name,
>  NULL,
>  NULL);
> -pthread_mutex_unlock(_pciaccess_mutex);
> +virMutexUnlock();
> 
>  *vendor_string = g_strdup(vendor_name);
>  *product_string = g_strdup(device_name);
> 
> 
> Michal
> 
> .
> 

Re: [PATCH] serialize access pci_get_strings function with a mutex

[Michal Privoznik]

There's no need to CC random developers - we are subscribed to the list.

On 3/26/21 4:21 AM, wangjian wrote:> serialize access pci_get_strings 
function with a mutex.> > nodedev-init thread:

nodeStateInitializeEnumerate ->
   udevEnumerateDevices->
 udevProcessDeviceListEntry ->
   udevAddOneDevice ->
 udevGetDeviceDetails->
   udevProcessPCI ->
 udevTranslatePCIIds ->
   pci_get_strings -> (libpciaccess)
 find_device_name ->
   populate_vendor ->
 d = realloc( vend->devices, (vend->num_devices + 1), * 
sizeof( struct pci_device_leaf ) );
 vend->num_devices++;

udev-event thread:
udevEventHandleThread ->
   udevHandleOneDevice ->
 udevAddOneDevice->
   udevGetDeviceDetails->
 udevProcessPCI ->
   udevTranslatePCIIds ->
 pci_get_strings -> (libpciaccess)
   find_device_name ->
 populate_vendor ->
   d = realloc( vend->devices, (vend->num_devices + 1), * 
sizeof( struct pci_device_leaf ) );
   vend->num_devices++;

Since the functions provided by libpciaccess are not thread-safe,
when the udev-event and nodedev-init threads of libvirt call
the pci_get_strings function provided by libpaciaccess at the same time.
It will appear that for the same address, realloc a large space first,
and then realloc a small space, which triggers the 'invalid next size' of 
realloc,
leading to the thread core.

Signed-off-by: WangJian 
---
  src/node_device/node_device_udev.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 3f28858..cc752ba 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -331,6 +331,7 @@ udevGenerateDeviceName(struct udev_device *device,
  return 0;
  }

+static pthread_mutex_t g_pciaccess_mutex = PTHREAD_MUTEX_INITIALIZER;


We tend to use virMutex (even though it is pthread_mutex under the hood).



  static int
  udevTranslatePCIIds(unsigned int vendor,
@@ -350,11 +351,13 @@ udevTranslatePCIIds(unsigned int vendor,
  m.match_data = 0;

  /* pci_get_strings returns void */
+pthread_mutex_lock(_pciaccess_mutex);
  pci_get_strings(,
  _name,
  _name,
  NULL,
  NULL);
+pthread_mutex_unlock(_pciaccess_mutex);

  *vendor_string = g_strdup(vendor_name);
  *product_string = g_strdup(device_name);



Apart from that, the patch is correct. I'd squash this in and push if 
you agree:


diff --git i/src/node_device/node_device_udev.c 
w/src/node_device/node_device_udev.c

index cc752bad32..3daa5c90ad 100644
--- i/src/node_device/node_device_udev.c
+++ w/src/node_device/node_device_udev.c
@@ -331,7 +331,7 @@ udevGenerateDeviceName(struct udev_device *device,
 return 0;
 }

-static pthread_mutex_t g_pciaccess_mutex = PTHREAD_MUTEX_INITIALIZER;
+static virMutex pciaccessMutex = VIR_MUTEX_INITIALIZER;

 static int
 udevTranslatePCIIds(unsigned int vendor,
@@ -350,14 +350,14 @@ udevTranslatePCIIds(unsigned int vendor,
 m.device_class_mask = 0;
 m.match_data = 0;

-/* pci_get_strings returns void */
-pthread_mutex_lock(_pciaccess_mutex);
+/* pci_get_strings returns void and unfortunately is not thread 
safe. */

+virMutexLock();
 pci_get_strings(,
 _name,
 _name,
 NULL,
 NULL);
-pthread_mutex_unlock(_pciaccess_mutex);
+virMutexUnlock();

 *vendor_string = g_strdup(vendor_name);
 *product_string = g_strdup(device_name);


Michal

Re: [PATCH 0/2] Revert parts of g_steal_pointer() rewrite

[Peter Krempa]

On Wed, Mar 24, 2021 at 19:01:47 +0100, Michal Privoznik wrote:
> A few hours ago, I've merged a patch that uses g_steal_pointer() instead
> of its opened coded alternative. Well, turns out that some areas are
> more fragile and rely on the open coded version.
> 
> https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/275871589
> 
> Michal Prívozník (2):
>   virnetsocket: Revert part of g_steal_pointer() rewrite
>   lib: Undo some g_steal_pointer() changes

Reviewed-by: Peter Krempa 

but note that I didn't comprehensively check the patch that broke things
that you've fixed all instances.

Re: [libvirt PATCH v2] ci: Refresh information

[Erik Skultety]

On Thu, Mar 25, 2021 at 01:53:27PM +0100, Andrea Bolognani wrote:
> Notable changes:
> 
>   * HAL is no longer installed on FreeBSD;
> 
>   * the native version of libwsman is no longer installed in
> containers intended for cross-compilation;
> 
>   * Meson 0.55 rather than 0.54 is requested when installing
> it from PyPI;
> 
>   * GNU sed and GNU grep are installed explicitly everywhere.
> 
> Signed-off-by: Andrea Bolognani 
> ---

Reviewed-by: Erik Skultety 

Re: [libvirt PATCH v5 00/30] Add support for persistent mediated devices

[Erik Skultety]

On Thu, Mar 25, 2021 at 11:49:49AM -0500, Jonathon Jongsma wrote:
> Just a friendly ping ;)

I'm sorry I've been neglecting this for the past 3 weeks or so, I'll dive right
into it starting Monday next week, we're entering freeze today anyway.
Would you mind resending a rebased version? There were a couple of conflicts
wrt to RPC, I fixed them all locally, but it's always better to start the
review with fresh content.

Thanks,
Erik

> 
> On Tue,  2 Mar 2021 16:30:35 -0600
> Jonathon Jongsma  wrote:
> 
> > This patch series follows the previously-merged series which added
> > support for transient mediated devices. This series expands mdev
> > support to include persistent device definitions. Again, it relies on
> > mdevctl as the backend.
> > 
> > It follows the common libvirt pattern of APIs by adding the following
> > new APIs for node devices:
> > - virNodeDeviceDefineXML() - defines a persistent device
> > - virNodeDeviceUndefine() - undefines a persistent device
> > - virNodeDeviceCreate() - starts a previously-defined device
> > 
> > It also adds virsh commands mapping to these new APIs: nodedev-define,
> > nodedev-undefine, and nodedev-start.
> > 
> > Since we rely on mdevctl for the definition of mediated devices, we
> > need a way to stay up-to-date with devices that are defined by
> > mdevctl (outside of libvirt).  The method for staying up-to-date is
> > currently a little bit crude due to the fact that mdevctl does not
> > emit any events when new devices are added or removed. As a
> > workaround, we create a file monitor for the mdevctl config directory
> > and re-query mdevctl when we detect changes within that directory. In
> > the future, mdevctl may introduce a more elegant solution.
> > 
> > Changes in v5:
> >  - Rebase to git master
> >  - updated new API version info to 7.2.0
> >  - capture and relay stderr message from mdevctl
> >  - changed to using GHashTable functions directly instead of
> > deprecated virHash functions
> >  - protected mdevctlMonitors with a mutex
> >  - added a couple patches to fix the 5s delay when defining a new
> > mdev. These are currently separate patches added to the end of the
> > series, but could be squashed into the earlier commits if that's
> > preferred.
> >  - various other minor review fixes
> > 
> > Changes in v4:
> >  - rebase to git master
> >  - switch to throwaway thread for querying mdevctl
> >  - fixed a bug when removing devices because I was accidentally using
> >virHashForEach() instead of virHashForeachSafe()
> >  - use DEFINED/UNDEFINED events instead of STARTED/STOPPED events
> >  - changes related to merging information about mdev devices from
> > both udev a= nd
> >mdevctl:
> >- Re-used the same function to copy extra data from mdevctl
> > regardless of whether we're processing a udev event or a mdevctl
> > event (recommended by Erik). This results in slightly more complex
> > handling of the object lifetimes (see patch 9), but it consolidates
> > some code.
> >- nodeDeviceDefCopyFromMdevctl() previously only copied the data
> > that was unique to mdevctl and didn't exist in udev. It now copies
> > additional data (possibly overwriting some udev).  This solves a
> > problem where a device = is
> >  defined but not active (i.e.  we have not gotten any data from
> > udev), and then changed (e.g. somebody calls 'mdevctl modify' to
> > change the mdev type), but libvirt was not updating to the new
> > definition.
> >  - fix a bug where we were mistakenly emitting 'update' events for
> > devices th= at
> >had not changed
> >  - Added the ability to specify a uuid for an mdev via device XML.
> >  - split some commits into multiple patches
> >  - updated new API version info to 7.1.0
> >  - Fixed a bug reported by Yan Fu which hangs the client when
> > attempting to destroy a nodedev that is in use by an active vm. See
> >https://www.redhat.com/archives/libvir-list/2021-February/msg00116.html
> > for solution suggested by Alex.
> >  - numerous smaller fixes from review findings
> > 
> > changes in v3:
> >  - streamlined tests -- removed some unnecessary duplication
> >  - split out patch to factor out node device name generation function
> >  - split nodeDeviceParseMdevctlChildDevice() into a separate function
> >  - added follow-up patch to remove space-padded alignment in header
> >  - refactored the mdevctl update handling significantly:
> >- no longer a separate persistent thread that gets signaled by a
> > timer
> >- now piggybacks onto the existing udev thread and signals the
> > thread in t= he
> >  same way that the udev event does.
> >- Daniel suggested spawning a throw-away thread to handle mdevctl
> > updates, but that introduces the complexity of possibly serializing
> > multiple throw-away threads (e.g. if we get an 'created' event
> > followed immediate= ly
> >  by a 'deleted' event, two threads may be spawned and we'd need
> > to ensure they are properly ordered)
> >