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

[Cole Robinson]

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

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

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

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?

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.

Thanks,
Cole