[virt-tools-list] [virt-manager 0/8] filesystem: Add support for virtiofs

[Michal Prívozník]

On 7/1/21 12:52 AM, Cole Robinson wrote:
> (ccing mprivozn with a domaincapabilities design question below)
> 
> On 6/30/21 8:10 AM, Lin Ma wrote:
>> So far, virt-manager only supports virtio-9p, The patchset adds virtiofs
>> which offering better performance.
>>
>> We know that the virtiofs needs 'shared' access mode of memory backing
>> or 'shared' access mode of virtual numa node, But virt-manager doesn't
>> provide UI to configure memory backing or virtual numa node because they
>> are advanced features and can be configured by raw XML editor.
>>
>> This patchset introduces basic virtiofs support and offers an easier way
>> to configure virtiofs by adjusting access mode to 'shared' if necessary.
>>
>> I don't intend to introduce memory backing UI or numa UI, That means I
>> need to modify the access mode attribue which belongs memorybacking or
>> numa in filesystem code, This perhaps looks not good, Any comments are
>> appreciated.
>>
> 
> Thanks for the patches. Regarding virtio-fs I've recorded my thoughts in
> this issue: https://github.com/virt-manager/virt-manager/issues/127
> 
> Basically I don't want to add this to virt-manager until we can make it
> closer to 'just work' without pitfalls. IMO that means adjusting libvirt
> to report via domcapabilities when it is safe and supported to
> unconditionally specify shared memory, without hugepages or numa config.
> Then we set that by default for new VMs, and _maybe_ do something like
> what your patches do (set it automatically when user requests virtiofs
> via addhw).
> 
> Until that's done, it's a pain in the ass to try and figure out, outside
> of libvirt, whether the domain XML has suitable setup to make virtio-fs
> work, and what is the simplest memory XML adjustment to make virtiofs
> work. We basically have to reimplement the libvirt
> qemuValidateDomainDefVhostUserRequireSharedMemory function from here
> https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_validate.c#L1427
> 
> Your code attempts to implement the numa_nodes check, but it doesn't
> account for the defaultRAMID bit.

Right. IIRC the shared memory is needed for DAX. I wonder if there's a
way to turn off DAX in virtiofsd. Then the <filesystem/> could be added
just like any other device.

> 
> The specific <memoryBacking><access mode='shared></memoryBacking> config
> is only accepted on libvirt 7.0.0+ AFAICT:
> https://gitlab.com/libvirt/libvirt/-/commit/bff2ad5d6b1f25da02802273934d2a519159fec7
> 
> And even then we probably want libvirt 7.1.0 at least before we set it
> unconditionally for new VMs:
> https://gitlab.com/libvirt/libvirt/-/commit/677c90cc1d1fcb3aba09b5d4f0f8f83099911775
> 

This could be avoided if domcapabilities were checked for before adding
virtiofsd. I mean, support for virtiofsd was added in 6.2.0; later, some
requirements were refined (e.g. NUMA nodes no longer needed in
v6.9.0-rc1~161). yada yada yada and only recently (v7.4.0-rc1~117)
virtiofs is announced in domcapabilities.

> So if you want to help move this forward in a sustainable way, please
> look into extending libvirt domcapabilities. One related bit would be
> reporting valid memory source type values, so that we know if memfd is
> an option (it can be compiled out of qemu). We may prefer to use that
> over type='file' memory, if it simplifies things. I think the schema
> would be:
> 
> <domainCapabilities>
>   <memoryBacking supported='yes'>
>     <enum name='sourceType'>
>       <value>file</value>
>       <value>memfd</value>
>       ...
> 

Yes, this looks sane and could be valuable for other use cases too.

> The 7.1.0 check, when access mode=shared can be used without numa or
> hugepages, we probably need some arbitrary boolean to report. It could be:
> 
> <domainCapabilities>
>   <memoryBacking>
>     <bareAccessMode supported='yes'>
> 
> Or maybe something under <features>. There isn't a clear precedent for
> exposing something like this in the XML. CCing mprivozn, any suggestions?

I think we can rely on <filesystem/> from domcaps AND newly added
<memoryBacking/> as described above. Yes, this will leave behind some
versions where virtiofs would work and yet virt-manager won't be able to
configure it, but I think that's acceptable.

> 
> Lin if you get those into libvirt I will be happy to help you land
> virtio-fs support in virt-manager, writing code coverage tests etc.

And I can help with domcaps, let me know if you want to post patches
yourself or whether I should do it.

> 
> Thanks,
> Cole
> 

Michal