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

[lma]

在 2021-07-01 08:39，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.

Because vhost-user needs shared memory, virtio-fs can't be enabled in 
isolation.

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

You know much about domcaps, I lean towards to you, So, please.

Thanks for your input!
Lin