[libvirt PATCHv3 10/12] qemu: add code for handling virtiofsd
Ján Tomko
jtomko at redhat.com
Mon Feb 3 18:36:09 UTC 2020
[adding virtiofs list]
On Mon, Feb 03, 2020 at 04:43:51PM +0000, Daniel P. Berrangé wrote:
>On Thu, Jan 30, 2020 at 06:06:26PM +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.
>
>So we're not able to use virtiofsd with the session libvirtd
>which runs completely unprivileged ?
>
Not with the version of virtiofsd currently merged in the QEMU tree.
>I can understand the need to run as root if we want to support
>chown() of files, or DAC_OVERRIDE, but I'm surprised it isn't
>possible to run virtiofsd unprivileged & simply have a reduced
>featureset where it can only create files as that one user.
>
Apart from the possibly missing features (I don't know how well
virtiofsd internals are ready for those), current version of the
daemon sets up namespaces and the seccomp sandbox.
>
>> +int
>> +qemuVirtioFSStart(virLogManagerPtr logManager,
>> + virQEMUDriverPtr driver,
>> + virDomainObjPtr vm,
>> + virDomainFSDefPtr fs)
>> +{
>> + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
>> + g_autoptr(virCommand) cmd = NULL;
>> + g_autofree char *socket_path = NULL;
>> + g_autofree char *pidfile = NULL;
>> + g_autofree char *logpath = NULL;
>> + pid_t pid = (pid_t) -1;
>> + VIR_AUTOCLOSE fd = -1;
>> + VIR_AUTOCLOSE logfd = -1;
>> + int ret = -1;
>> + int rc;
>
>> +
>> + if (!(pidfile = qemuVirtioFSCreatePidFilename(cfg, vm->def, fs->info.alias)))
>> + goto cleanup;
>> +
>> + if (!(socket_path = qemuVirtioFSCreateSocketFilename(vm, fs->info.alias)))
>> + goto cleanup;
>> +
>> + if ((fd = qemuVirtioFSOpenChardev(driver, vm, socket_path)) < 0)
>> + goto cleanup;
>> +
>> + 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)
>> + goto cleanup;
>> + } else {
>> + if ((logfd = open(logpath, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) < 0) {
>> + virReportSystemError(errno, _("failed to create logfile %s"),
>> + logpath);
>> + goto cleanup;
>> + }
>> + if (virSetCloseExec(logfd) < 0) {
>> + virReportSystemError(errno, _("failed to set close-on-exec flag on %s"),
>> + logpath);
>> + goto error;
>> + }
>> + }
>> +
>> + if (!(cmd = qemuVirtioFSBuildCommandLine(cfg, fs, &fd)))
>> + goto cleanup;
>> +
>> + virCommandSetPidFile(cmd, pidfile);
>> + virCommandSetOutputFD(cmd, &logfd);
>> + virCommandSetErrorFD(cmd, &logfd);
>> + virCommandNonblockingFDs(cmd);
>> + virCommandDaemonize(cmd);
>
>We're not mandating "root" here, it is just inheriting the user that
>libvirtd runs as. So IIUC ,this will run virtofsd as non-root when
>used with session libvirtd, unless there's a check somewhere else
>that prevents this scenario ?
I'll add a check.
>
>I'm also wondering about cgroups placement in this method, and
>any use of SELinux
>
Placing it into a cgroup should be easy, AFAIK it does not need to
access any devices.
As for SELinux, I don't think there's anything to be done other than
updating the selinux-policy. Recursively relabeling the whole directory
feels intrusive.
Jano
>> +
>> + if (qemuExtDeviceLogCommand(driver, vm, cmd, "virtiofsd") < 0)
>> + goto cleanup;
>> +
>> + rc = virCommandRun(cmd, NULL);
>> + logfd = -1;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200203/6dab00aa/attachment-0001.sig>
More information about the libvir-list
mailing list