在 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
