On Wed, Feb 26, 2020 at 11:34:06AM +0100, Ján Tomko wrote: > On Wed, Feb 26, 2020 at 09:42:04AM +0100, Peter Krempa wrote: > > Explicitly CCing danpb to clarify usage of the logging daemon. > > > > On Thu, Feb 20, 2020 at 15:32:48 +0100, Ján Tomko wrote: > > > Start virtiofsd for each <filesystem> device using it. > > > > > > Pre-create the socket for communication with QEMU and pass it > > > to virtiofsd. > > > > > > Note that virtiofsd needs to run as root. > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1694166 > > > > > > Introduced by QEMU commit a43efa34c7d7b628cbf1ec0fe60043e5c91043ea > > > > > > Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx> > > > --- > > > > [...] > > > > > diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c > > > new file mode 100644 > > > index 0000000000..600a3644bd > > > --- /dev/null > > > +++ b/src/qemu/qemu_virtiofs.c > > > @@ -0,0 +1,300 @@ > > > > [...] > > > > > +char * > > > +qemuVirtioFSCreatePidFilename(virQEMUDriverConfigPtr cfg, > > > + const virDomainDef *def, > > > + const char *alias) > > > +{ > > > + g_autofree char *shortName = NULL; > > > + g_autofree char *name = NULL; > > > + > > > + if (!(shortName = virDomainDefGetShortName(def))) > > > + return NULL; > > > + > > > + name = g_strdup_printf("%s-%s-virtiofsd", shortName, alias); > > > + > > > + return virPidFileBuildPath(cfg->stateDir, name); > > > +} > > > > Why do we want to store the pid file here? > > > > > + > > > + > > > +char * > > > +qemuVirtioFSCreateSocketFilename(virDomainObjPtr vm, > > > + const char *alias) > > > +{ > > > + qemuDomainObjPrivatePtr priv = vm->privateData; > > > + > > > + return virFileBuildPath(priv->libDir, alias, "-virtiofsd.sock"); > > > > Shouldn't we put it here as well? It's a sub-process of the domain > > itself and doesn't make much sense to treat it as the qemu pid file. > > > > Right, libDir sounds like a better location. > > > [...] > > > > + > > > + logpath = qemuVirtioFSCreateLogFilename(cfg, vm->def, fs->info.alias); > > > + > > > + if (cfg->stdioLogD) { > > > + if ((logfd = virLogManagerDomainOpenLogFile(logManager, > > > + "qemu", > > > + vm->def->uuid, > > > + vm->def->name, > > > + logpath, > > > + 0, > > > + NULL, NULL)) < 0) > > > > Hmm, I'm not sure now what the intented usage of uuid/domname was and > > there's no docs. > > > > Daniel, please review this. > > > > Grepping for uuid in src/logging, this seems to be only used when saving > and loading virtlockd's state. But virtiofsd's lifecycle is tied to the > domain, so I don't think it should get its own uuid. Essentially the UUID/name are just a way to associate the log file(s) with what "thing" is owning them. We already have virtlogd handling multiple logs from the same QEMU - the main QEMU stderr, and the file base logs for character devices. Having another log file open for virtiofs, slirp, swtpm, etc, all with the same UUID+name makes total sense. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
