On Thu, Feb 20, 2020 at 15:32:49 +0100, Ján Tomko wrote:
Wire up the code to put virtiofsd in the emulator cgroup on domain
startup.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_extdevice.c | 15 +++++++++++++++
src/qemu/qemu_virtiofs.c | 28 ++++++++++++++++++++++++++++
src/qemu/qemu_virtiofs.h | 6 ++++++
3 files changed, 49 insertions(+)
[...]
@@ -271,5 +278,13 @@ qemuExtDevicesSetupCgroup(virQEMUDriverPtr
driver,
qemuExtTPMSetupCgroup(driver, def, cgroup) < 0)
return -1;
+ for (i = 0; i < def->nfss; i++) {
+ virDomainFSDefPtr fs = def->fss[i];
+
+ if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS &&
+ qemuVirtioFSSetupCgroup(driver, def, fs, cgroup) < 0)
While this is consistent with what we do with other cases of 'externa'
devices. I'm worried though that that stashing everything into a single
cgroup will not scale.
+ return -1;
+ }
+
return 0;
}
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 600a3644bd..9e354a30c6 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -298,3 +298,31 @@ qemuVirtioFSStop(virQEMUDriverPtr driver,
cleanup:
virErrorRestore(&orig_err);
}
+
+
+int
+qemuVirtioFSSetupCgroup(virQEMUDriverPtr driver,
+ virDomainDefPtr def,
+ virDomainFSDefPtr fs,
+ virCgroupPtr cgroup)
+{
+ g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+ g_autofree char *pidfile = NULL;
+ pid_t pid = -1;
+ int rc;
+
+ if (!(pidfile = qemuVirtioFSCreatePidFilename(cfg, def, fs->info.alias)))
+ return -1;
+
+ rc = virPidFileReadPathIfAlive(pidfile, &pid, NULL);
+ if (rc < 0 || pid == (pid_t) -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("virtiofsd died unexpectedly"));
+ return -1;
+ }
Why don't we store the pid in the private data? Seems to be a waste of
cycles to read it here.
+
+ if (virCgroupAddProcess(cgroup, pid) < 0)
+ return -1;
+
+ return 0;
+}
Please consider storing the pid somewhere rather than looking it up.
With that:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
We can worry about the cgroup congestion later since that's
pre-existing.