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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux