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 <[email protected]> >> --- >> 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 <[email protected]> >
I renamed `started` to `startCalled`, and pushed Thanks, Cole
