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

[lma]

在 2021-07-01 14:05，Cole Robinson 写道：
> On 7/1/21 4:39 AM, Michal Prívozník wrote:
>> 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.
>> 
> 
> IIRC it's a generic vhost-user requirement. Libvirt enforces shared
> memory for vhost-user-blk as well. And this blog post suggests it's a
> requirement for the older vhostuser net support too, though libvirt
> doesn't seem to enforce it:
> https://www.redhat.com/en/blog/hands-vhost-user-warm-welcome-dpdk

Yes, indeed.

>>> 
>>> 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.
>> 
> 
> Good point, we can use domcaps virtiofs reporting as a proxy for
> 'libvirt is new enough to allow bare access mode=shared'. Is it
> sufficient for qemu support? Are there qemu versions that support
> virtiofs but aren't new enough for libvirt's defaultRAMid 
> determination?
> If we want to set this by default for new VMs I want to be as safe as
> possible here.
> 
> If we want to start setting shared memory, possibly memfd, by default
> though we will still want the memoryBacking sourceType reporting as 
> well.

I'd like to wait for the domcaps adjusting by Michal, then to see what I 
can do
in virt-manager side for next.

Thanks for your comments & suggestion!
Lin