[PATCH] qemu: virtiofs: make fs->sock guard more consistent
When fs->sock is set, virtiofs is externally managed, and most of qemu_virtiofs.c should do nothing. Some qemu_virtiofs.c functions handle this case, but some require the caller to guard against it. Standardize on making this the responsibility of qemu_virtiofs.c Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_extdevice.c | 8 +++----- src/qemu/qemu_hotplug.c | 18 ++++++++---------- src/qemu/qemu_virtiofs.c | 9 +++++++++ 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 28cea52980..6e16c6be29 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -225,7 +225,7 @@ qemuExtDevicesStart(virQEMUDriver *driver, for (i = 0; i < def->nfss; i++) { virDomainFSDef *fs = def->fss[i]; - if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS && !fs->sock) { + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) { if (qemuVirtioFSStart(driver, vm, fs) < 0) return -1; } @@ -315,8 +315,7 @@ qemuExtDevicesStop(virQEMUDriver *driver, for (i = 0; i < def->nfss; i++) { virDomainFSDef *fs = def->fss[i]; - if (!fs->sock && - fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) qemuVirtioFSStop(driver, vm, fs); } @@ -473,8 +472,7 @@ qemuExtDevicesSetupCgroup(virQEMUDriver *driver, for (i = 0; i < def->nfss; i++) { virDomainFSDef *fs = def->fss[i]; - if (!fs->sock && - fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS && + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS && qemuVirtioFSSetupCgroup(vm, fs, cgroup) < 0) return -1; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9439948089..deb02c20be 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3361,17 +3361,15 @@ qemuDomainAttachFSDevice(virQEMUDriver *driver, if (!(devprops = qemuBuildVHostUserFsDevProps(fs, vm->def, charAlias, priv))) goto cleanup; - if (!fs->sock) { - if (qemuVirtioFSPrepareDomain(driver, fs) < 0) - goto cleanup; + if (qemuVirtioFSPrepareDomain(driver, fs) < 0) + goto cleanup; - if (qemuVirtioFSStart(driver, vm, fs) < 0) - goto cleanup; - started = true; + if (qemuVirtioFSStart(driver, vm, fs) < 0) + goto cleanup; + started = true; - if (qemuVirtioFSSetupCgroup(vm, fs, priv->cgroup) < 0) - goto cleanup; - } + if (qemuVirtioFSSetupCgroup(vm, fs, priv->cgroup) < 0) + goto cleanup; qemuDomainObjEnterMonitor(vm); @@ -5506,7 +5504,7 @@ qemuDomainRemoveFSDevice(virQEMUDriver *driver, if (rc < 0) return -1; - if (!fs->sock && fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) qemuVirtioFSStop(driver, vm, fs); if ((idx = virDomainFSDefFind(vm->def, fs)) >= 0) diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c index 91d58703f1..cbc3bc0bc2 100644 --- a/src/qemu/qemu_virtiofs.c +++ b/src/qemu/qemu_virtiofs.c @@ -228,6 +228,9 @@ qemuVirtioFSStart(virQEMUDriver *driver, g_autoptr(domainLogContext) logContext = NULL; int rc; + if (fs->sock) + return 0; + if (!virFileIsExecutable(fs->binary)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("virtiofsd binary '%1$s' is not executable"), @@ -327,6 +330,9 @@ qemuVirtioFSStop(virQEMUDriver *driver G_GNUC_UNUSED, g_autofree char *pidfile = NULL; virErrorPtr orig_err; + if (fs->sock) + return; + virErrorPreserveLast(&orig_err); if (!(pidfile = qemuVirtioFSCreatePidFilename(vm, fs->info.alias))) @@ -361,6 +367,9 @@ qemuVirtioFSSetupCgroup(virDomainObj *vm, pid_t pid = -1; int rc; + if (fs->sock) + return 0; + if (!cgroup) return 0; -- 2.53.0
On Thu, Apr 02, 2026 at 11:58:38 -0400, Cole Robinson via Devel wrote:
When fs->sock is set, virtiofs is externally managed, and most of qemu_virtiofs.c should do nothing.
Some qemu_virtiofs.c functions handle this case, but some require the caller to guard against it.
Standardize on making this the responsibility of qemu_virtiofs.c
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_extdevice.c | 8 +++----- src/qemu/qemu_hotplug.c | 18 ++++++++---------- src/qemu/qemu_virtiofs.c | 9 +++++++++ 3 files changed, 20 insertions(+), 15 deletions(-)
[...]
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9439948089..deb02c20be 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3361,17 +3361,15 @@ qemuDomainAttachFSDevice(virQEMUDriver *driver, if (!(devprops = qemuBuildVHostUserFsDevProps(fs, vm->def, charAlias, priv))) goto cleanup;
- if (!fs->sock) { - if (qemuVirtioFSPrepareDomain(driver, fs) < 0) - goto cleanup; + if (qemuVirtioFSPrepareDomain(driver, fs) < 0) + goto cleanup;
- if (qemuVirtioFSStart(driver, vm, fs) < 0) - goto cleanup; - started = true; + if (qemuVirtioFSStart(driver, vm, fs) < 0) + goto cleanup; + started = true;
While this doesn't change the actual result, the 'started' flag which is present here without any additional documentation will be misleading. Either add a comment stating that it may not be actually started or rename the variable to something more self-explanatory.
- if (qemuVirtioFSSetupCgroup(vm, fs, priv->cgroup) < 0) - goto cleanup; - } + if (qemuVirtioFSSetupCgroup(vm, fs, priv->cgroup) < 0) + goto cleanup;
qemuDomainObjEnterMonitor(vm);
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On 4/7/26 11:06 AM, Peter Krempa wrote:
On Thu, Apr 02, 2026 at 11:58:38 -0400, Cole Robinson via Devel wrote:
When fs->sock is set, virtiofs is externally managed, and most of qemu_virtiofs.c should do nothing.
Some qemu_virtiofs.c functions handle this case, but some require the caller to guard against it.
Standardize on making this the responsibility of qemu_virtiofs.c
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_extdevice.c | 8 +++----- src/qemu/qemu_hotplug.c | 18 ++++++++---------- src/qemu/qemu_virtiofs.c | 9 +++++++++ 3 files changed, 20 insertions(+), 15 deletions(-)
[...]
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9439948089..deb02c20be 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3361,17 +3361,15 @@ qemuDomainAttachFSDevice(virQEMUDriver *driver, if (!(devprops = qemuBuildVHostUserFsDevProps(fs, vm->def, charAlias, priv))) goto cleanup;
- if (!fs->sock) { - if (qemuVirtioFSPrepareDomain(driver, fs) < 0) - goto cleanup; + if (qemuVirtioFSPrepareDomain(driver, fs) < 0) + goto cleanup;
- if (qemuVirtioFSStart(driver, vm, fs) < 0) - goto cleanup; - started = true; + if (qemuVirtioFSStart(driver, vm, fs) < 0) + goto cleanup; + started = true;
While this doesn't change the actual result, the 'started' flag which is present here without any additional documentation will be misleading.
Either add a comment stating that it may not be actually started or rename the variable to something more self-explanatory.
- if (qemuVirtioFSSetupCgroup(vm, fs, priv->cgroup) < 0) - goto cleanup; - } + if (qemuVirtioFSSetupCgroup(vm, fs, priv->cgroup) < 0) + goto cleanup;
qemuDomainObjEnterMonitor(vm);
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
I renamed `started` to `startCalled`, and pushed Thanks, Cole
participants (2)
-
Cole Robinson -
Peter Krempa