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
