[virt-tools-list] [virt-manager] [PATCH 0/7] Add a checkbox in memory details to control memory access mode

[Cole Robinson]

On 1/25/22 12:50 PM, Cole Robinson wrote:
> On 8/1/21 8:36 AM, Lin Ma wrote:
>> Future patches about virtiofs addhw support relies on shared memory access.
>>
>> Lin Ma (7):
>>   domcapabilities: Get filesystem devices
>>   domcapabilities: Add supports_filesystem_virtiofs()
>>   domcapabilities: Add supports_memorybacking_memfd()
>>   domain: memorybacking: Add function is_shared_access()
>>   domain: cpu: Add function has_private_memAccess_cells()
>>   domain: cpu: Add function all_shared_memAccess_cells()
>>   details: Add new checkbox to control shared memory access
>>
>>  ui/details.ui                    | 14 +++++++++
>>  virtManager/details/details.py   | 52 ++++++++++++++++++++++++++++++++
>>  virtManager/object/domain.py     | 17 ++++++++++-
>>  virtinst/domain/cpu.py           | 14 +++++++++
>>  virtinst/domain/memorybacking.py |  3 ++
>>  virtinst/domcapabilities.py      | 20 ++++++++++++
>>  6 files changed, 119 insertions(+), 1 deletion(-)
>>
> 
> Sorry for the reaaally long delay. This came in right when I went on
> paternity leave and I'm still digging out of the backlog.
> 
> I've pushed this series now, but with some bits removed. I stripped the
> logic back to only handle enabling+disabling source_type=memfd and
> access_mode=shared. If the guest has numa cells configured, we leave the
> checkbox unchecked and disable it with a warning. Same thing if the
> domcaps don't advertise both virtiofs and memfd.
> 
> I think that logic covers what most users will need, going forward. If a
> user has <numa> in their XML, I think they can be expected to make the
> shared memory change themselves. It makes me a little nervous getting
> into the business of processing <numa> XML that virt-manager otherwise
> doesn't have any UI for. So my preference is to not do that.
> 
> If you have a good argument for why it should be readded, I'm open to
> it. But resubmitting it should come with uitests, since this adds some
> annoying corner cases that I will want to see tested. FWIW I tried to
> keep the code revert to a single commit, so hopefully it's easy to
> revive, if you are interested.
> 
> The other big thing I did was move the logic from details.py to
> domain.py, which will make it easier to share the interesting bits with
> the Add Hardware UI like I mentioned in this mail:
> https://listman.redhat.com/archives/virt-tools-list/2021-July/msg00016.html
> 
> I'm working towards a release, so unless you have patches with that work
> kicking around in a git branch, I'll make the addhw change in the next
> few weeks.

I updated the filesystem UI in git to show 9p vs virtiofs option. If
user selects virtiofs and they don't have shared memory enabled (per our
less than exhaustive logic), a UI label appears with: "You may need to
'Enable shared memory' on the 'Memory' screen". It would take more work
to wire up the 'Enable shared memory' checkbox into the addhardware
wizard, so I saved that for another day.

Thanks,
Cole