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

[Cole Robinson]

On 3/29/21 6:52 AM, Ján Tomko wrote:
> 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
> 

I don't have much of an opinion. The latter will be useful for CNV
someday but it will take some rearchitechting on their part

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

I just tested with the qemu_validate.c removal only, so SetUID still in
place. The error:

error: Failed to start domain 'f32'
error: internal error: process exited while connecting to monitor:
2021-03-29T19:11:41.217583Z qemu-system-x86_64: -chardev
socket,id=chr-vu-fs0,path=/home/crobinso/.config/libvirt/qemu/lib/domain-3-f32/fs0-fs.sock:
Failed to connect to
'/home/crobinso/.config/libvirt/qemu/lib/domain-3-f32/fs0-fs.sock':
Connection refused

virtiofsd log has:
libvirt:  error : internal error: cannot apply process capabilities -5

So seems like libvirt error reporting here needs some tweaks regardless.
Maybe if catching virtiofsd startup error is improved it will improve
the first error case as well.

- Cole