[PATCH] qemu: don't reject virtiofs for qemu:///session

[Ján Tomko]

On a Friday in 2021, Cole Robinson wrote:
>Currently libvirt rejects attempts to use virtiofs with
>qemu:///session. Indeed virtiofs does not have a chance of working
>with typical session usage, because virtiofsd needs multiple linux
>capabilities to perform its job. The only way to do this with out
>of the box qemu packaging is to run virtiofsd as root, so libvirtd
>must run as root, so qemu:///system is required.
>
>But it's possible that a custom environment could setuid or set
>file capabilities on the virtiofsd binary. In this case qemu:///session
>would work fine. This may be an option for kubevirt to help them
>transition to using qemu:///session everywhere
>
>Drop the two pieces that block virtiofs for qemu:///session. Attempts
>to use virtiofs for stock qemu:///session will fail at qemu startup,
>though it's a bit opaque:
>
>error: Failed to start domain 'f32'
>error: internal error: qemu unexpectedly closed the monitor: 2021-03-26T16:26:12.459293Z qemu-system-x86_64: -device vhost-user-fs-pci,chardev=chr-vu-fs0,tag=/foovirtiofs,bus=pci.10,addr=0x0: Failed to write msg. Wrote -1 instead of 12.
>2021-03-26T16:26:12.459317Z qemu-system-x86_64: -device vhost-user-fs-pci,chardev=chr-vu-fs0,tag=/foovirtiofs,bus=pci.10,addr=0x0: vhost_dev_init failed: Operation not permitted
>

That is not a friendly error message for regular users.

Some alternatives come to mind:
* XML element telling libvirt to ignore the check/do not set the UID.
   The downside is, that we usually do this via:
     <seclabel relabel='no'/>
   and the seclabel code makes my head hurt.
* Introduce a QEMU capability for this, that kubevirt could set via
   <qemu:capabilities>
   https://libvirt.org/drvqemu.html#xmlnsfeatures
* Introduce the capability to 50-qemu-virtiofsd.json
* Wait until the lockdown eases up and I finally post the patches
   for externally launched virtiofsd
   https://bugzilla.redhat.com/show_bug.cgi?id=1855789

>Signed-off-by: Cole Robinson <crobinso at redhat.com>
>---
>The SetUID/SetGID bits don't seem to be necessary for qemu:///system
>usage AFAICT, but it's a bit tough to decode virSetUIDGIDWithCaps.

I *think* the only difference without the two virCommandSet?ID calls
is that we error out if UID/GID 0 can't be set. But yes it's tough
to read.

Jano

>virtiofsd is meticulous about managing its capability set though
>
> src/qemu/qemu_validate.c | 7 +------
> src/qemu/qemu_virtiofs.c | 4 ----
> 2 files changed, 1 insertion(+), 10 deletions(-)
>
>diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>index 6043f974ce..d4079f6b67 100644
>--- a/src/qemu/qemu_validate.c
>+++ b/src/qemu/qemu_validate.c
>@@ -4053,7 +4053,7 @@ qemuValidateDomainDeviceDefGraphics(const virDomainGraphicsDef *graphics,
> static int
> qemuValidateDomainDeviceDefFS(virDomainFSDefPtr fs,
>                               const virDomainDef *def,
>-                              virQEMUDriverPtr driver,
>+                              virQEMUDriverPtr driver G_GNUC_UNUSED,
>                               virQEMUCapsPtr qemuCaps)
> {
>     if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) {
>@@ -4107,11 +4107,6 @@ qemuValidateDomainDeviceDefFS(virDomainFSDefPtr fs,
>                            _("virtiofs does not yet support read-only mode"));
>             return -1;
>         }
>-        if (!driver->privileged) {
>-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>-                           _("virtiofs is not yet supported in session mode"));
>-            return -1;
>-        }
>         if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) {
>             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                            _("virtiofs only supports passthrough accessmode"));
>diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
>index 2e239cad66..0bb4e3c0d1 100644
>--- a/src/qemu/qemu_virtiofs.c
>+++ b/src/qemu/qemu_virtiofs.c
>@@ -214,10 +214,6 @@ qemuVirtioFSStart(virLogManagerPtr logManager,
>     if (!(cmd = qemuVirtioFSBuildCommandLine(cfg, fs, &fd)))
>         goto cleanup;
>
>-    /* so far only running as root is supported */
>-    virCommandSetUID(cmd, 0);
>-    virCommandSetGID(cmd, 0);
>-
>     virCommandSetPidFile(cmd, pidfile);
>     virCommandSetOutputFD(cmd, &logfd);
>     virCommandSetErrorFD(cmd, &logfd);
>-- 
>2.30.2
>
-------------- 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/20210329/c0651cc0/attachment-0001.sig>