Re: [libvirt PATCH 16/16] qemu: implement virtiofs hotunplug

[Peter Krempa]

On Wed, Oct 06, 2021 at 09:15:22 +0200, Ján Tomko wrote:
> Signed-off-by: Ján Tomko 
> ---
>  src/conf/domain_conf.c   | 24 
>  src/conf/domain_conf.h   |  2 +
>  src/libvirt_private.syms |  1 +
>  src/qemu/qemu_hotplug.c  | 81 +++-
>  4 files changed, 106 insertions(+), 2 deletions(-)

[...]

> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index f5dad0b829..713da5ebfc 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -5180,6 +5180,45 @@ qemuDomainRemoveRedirdevDevice(virQEMUDriver *driver,
>  }
>  
>  
> +static int
> +qemuDomainRemoveFSDevice(virQEMUDriver *driver,
> + virDomainObj *vm,
> + virDomainFSDef *fs)
> +{
> +g_autofree char *charAlias = NULL;
> +qemuDomainObjPrivate *priv = vm->privateData;
> +ssize_t idx;
> +int rc = 0;
> +
> +VIR_DEBUG("Removing FS device %s from domain %p %s",
> +  fs->info.alias, vm, vm->def->name);
> +
> +charAlias = qemuDomainGetVhostUserChrAlias(fs->info.alias);
> +
> +qemuDomainObjEnterMonitor(driver, vm);
> +
> +if (qemuMonitorDetachCharDev(priv->mon, charAlias) < 0)
> +rc = -1;

Note that you must not assume here that the check below [1] that allows
only virtio-fs device unplug guards this code too.

The *Remove* code is executed always when we get a 'DEVICE_DELETED'
event from qemu thus also for guest-initiated unplug, which
theoretically can happen also for p9fs device.

> +
> +if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +return -1;
> +
> +virDomainAuditFS(vm, fs, NULL, "detach", rc == 0);
> +
> +if (rc < 0)
> +return -1;
> +
> +if (!fs->sock)
> +qemuVirtioFSStop(driver, vm, fs);
> +
> +if ((idx = virDomainFSDefFind(vm->def, fs)) >= 0)
> +virDomainFSRemove(vm->def, idx);
> +qemuDomainReleaseDeviceAddress(vm, >info);
> +virDomainFSDefFree(fs);
> +return 0;
> +}
> +
> +
>  static void
>  qemuDomainRemoveAuditDevice(virDomainObj *vm,
>  virDomainDeviceDef *detach,

[...]

> @@ -6020,6 +6066,31 @@ qemuDomainDetachPrepVsock(virDomainObj *vm,
>  }
>  
>  
> +static int
> +qemuDomainDetachPrepFS(virDomainObj *vm,
> +   virDomainFSDef *match,
> +   virDomainFSDef **detach)
> +{
> +ssize_t idx;
> +
> +if ((idx = virDomainFSDefFind(vm->def, match)) < 0) {
> +virReportError(VIR_ERR_DEVICE_MISSING, "%s",
> +   _("matching filesystem not found"));
> +return -1;
> +}
> +
> +if (vm->def->fss[idx]->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) {
> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +   _("only virtiofs filesystems can be hotplugged"));
> +return -1;
> +}

[1]. This check only guards host-initiated delete.

> +
> +*detach = vm->def->fss[idx];
> +
> +return 0;
> +}
> +
> +
>  static int
>  qemuDomainDetachDeviceLease(virQEMUDriver *driver,
>  virDomainObj *vm,

[libvirt PATCH 16/16] qemu: implement virtiofs hotunplug

[Ján Tomko]

Signed-off-by: Ján Tomko 
---
 src/conf/domain_conf.c   | 24 
 src/conf/domain_conf.h   |  2 +
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_hotplug.c  | 81 +++-
 4 files changed, 106 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b8370f6950..057dbf91a5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28829,6 +28829,30 @@ virDomainFSRemove(virDomainDef *def, size_t i)
 return fs;
 }
 
+ssize_t
+virDomainFSDefFind(virDomainDef *def,
+   virDomainFSDef *fs)
+{
+size_t i = 0;
+
+for (i = 0; i < def->nfss; i++) {
+virDomainFSDef *tmp = def->fss[i];
+
+if (fs->dst && STRNEQ_NULLABLE(fs->dst, tmp->dst))
+continue;
+
+if (fs->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+!virDomainDeviceInfoAddressIsEqual(>info, >info))
+continue;
+
+if (fs->info.alias && STRNEQ_NULLABLE(fs->info.alias, tmp->info.alias))
+continue;
+
+return i;
+}
+return -1;
+}
+
 virDomainFSDef *
 virDomainGetFilesystemForTarget(virDomainDef *def,
 const char *target)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c23c233184..1fcef7b0e1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3757,6 +3757,8 @@ virDomainFSDef 
*virDomainGetFilesystemForTarget(virDomainDef *def,
 int virDomainFSInsert(virDomainDef *def, virDomainFSDef *fs);
 int virDomainFSIndexByName(virDomainDef *def, const char *name);
 virDomainFSDef *virDomainFSRemove(virDomainDef *def, size_t i);
+ssize_t virDomainFSDefFind(virDomainDef *def,
+   virDomainFSDef *fs);
 
 unsigned int virDomainVideoDefaultRAM(const virDomainDef *def,
   const virDomainVideoType type);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fd0eea0777..687a3bbc4c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -410,6 +410,7 @@ virDomainFeatureTypeFromString;
 virDomainFeatureTypeToString;
 virDomainFSAccessModeTypeToString;
 virDomainFSCacheModeTypeToString;
+virDomainFSDefFind;
 virDomainFSDefFree;
 virDomainFSDefNew;
 virDomainFSDriverTypeToString;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f5dad0b829..713da5ebfc 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5180,6 +5180,45 @@ qemuDomainRemoveRedirdevDevice(virQEMUDriver *driver,
 }
 
 
+static int
+qemuDomainRemoveFSDevice(virQEMUDriver *driver,
+ virDomainObj *vm,
+ virDomainFSDef *fs)
+{
+g_autofree char *charAlias = NULL;
+qemuDomainObjPrivate *priv = vm->privateData;
+ssize_t idx;
+int rc = 0;
+
+VIR_DEBUG("Removing FS device %s from domain %p %s",
+  fs->info.alias, vm, vm->def->name);
+
+charAlias = qemuDomainGetVhostUserChrAlias(fs->info.alias);
+
+qemuDomainObjEnterMonitor(driver, vm);
+
+if (qemuMonitorDetachCharDev(priv->mon, charAlias) < 0)
+rc = -1;
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+return -1;
+
+virDomainAuditFS(vm, fs, NULL, "detach", rc == 0);
+
+if (rc < 0)
+return -1;
+
+if (!fs->sock)
+qemuVirtioFSStop(driver, vm, fs);
+
+if ((idx = virDomainFSDefFind(vm->def, fs)) >= 0)
+virDomainFSRemove(vm->def, idx);
+qemuDomainReleaseDeviceAddress(vm, >info);
+virDomainFSDefFree(fs);
+return 0;
+}
+
+
 static void
 qemuDomainRemoveAuditDevice(virDomainObj *vm,
 virDomainDeviceDef *detach,
@@ -5218,6 +5257,10 @@ qemuDomainRemoveAuditDevice(virDomainObj *vm,
 virDomainAuditRedirdev(vm, detach->data.redirdev, "detach", success);
 break;
 
+case VIR_DOMAIN_DEVICE_FS:
+virDomainAuditFS(vm, detach->data.fs, NULL, "detach", success);
+break;
+
 case VIR_DOMAIN_DEVICE_LEASE:
 case VIR_DOMAIN_DEVICE_CONTROLLER:
 case VIR_DOMAIN_DEVICE_WATCHDOG:
@@ -5225,7 +5268,6 @@ qemuDomainRemoveAuditDevice(virDomainObj *vm,
 /* These devices don't have associated audit logs */
 break;
 
-case VIR_DOMAIN_DEVICE_FS:
 case VIR_DOMAIN_DEVICE_SOUND:
 case VIR_DOMAIN_DEVICE_VIDEO:
 case VIR_DOMAIN_DEVICE_GRAPHICS:
@@ -5323,9 +5365,13 @@ qemuDomainRemoveDevice(virQEMUDriver *driver,
 return -1;
 break;
 
+case VIR_DOMAIN_DEVICE_FS:
+if (qemuDomainRemoveFSDevice(driver, vm, dev->data.fs) < 0)
+return -1;
+break;
+
 case VIR_DOMAIN_DEVICE_NONE:
 case VIR_DOMAIN_DEVICE_LEASE:
-case VIR_DOMAIN_DEVICE_FS:
 case VIR_DOMAIN_DEVICE_SOUND:
 case VIR_DOMAIN_DEVICE_VIDEO:
 case VIR_DOMAIN_DEVICE_GRAPHICS:
@@ -6020,6 +6066,31 @@ qemuDomainDetachPrepVsock(virDomainObj *vm,
 }
 
 
+static int
+qemuDomainDetachPrepFS(virDomainObj *vm,
+