[libvirt PATCHv3 10/12] qemu: add code for handling virtiofsd

Daniel P. Berrangé berrange at redhat.com
Mon Feb 3 16:43:51 UTC 2020


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 ?

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.


> +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'm also wondering about cgroups placement in this method, and
any use of SELinux

> +
> +    if (qemuExtDeviceLogCommand(driver, vm, cmd, "virtiofsd") < 0)
> +        goto cleanup;
> +
> +    rc = virCommandRun(cmd, NULL);
> +    logfd = -1;
> +
> +    if (rc < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not start 'virtiofsd'"));
> +        goto error;
> +    }
> +
> +    rc = virPidFileReadPath(pidfile, &pid);
> +    if (rc < 0) {
> +        virReportSystemError(-rc,
> +                             _("Unable to read virtiofsd pidfile '%s'"),
> +                             pidfile);
> +        goto error;
> +    }
> +
> +    if (virProcessKill(pid, 0) != 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("virtiofsd died unexpectedly"));
> +        goto error;
> +    }
> +
> +    QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock = g_steal_pointer(&socket_path);
> +    ret = 0;
> +
> + cleanup:
> +    if (socket_path)
> +        unlink(socket_path);
> +    return ret;
> +
> + error:
> +    if (pid != -1)
> +        virProcessKillPainfully(pid, true);
> +    if (pidfile)
> +        unlink(pidfile);
> +    goto cleanup;
> +}

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 :|




More information about the libvir-list mailing list