On 09.05.25 04:38, Chao Gao wrote:
On Sun, Apr 27, 2025 at 10:26:52AM +0800, Chenyi Qiang wrote:
Hi David,
Any thought on patch 10-12, which is to move the change attribute into a
priority listener. A problem is how to handle the error handling of
private_to_shared failure. Previously, we thought it would never be able
to fail, but right now, it is possible in corner cases (e.g. -ENOMEM) in
set_attribute_private(). At present, I simply raise an assert instead of
adding any rollback work (see patch 11).
I took a look at patches 10-12, and here are my thoughts:
Moving the change attribute into a priority listener seems sensible. It can
ensure the correct order between setting memory attributes and VFIO's DMA
map/unmap operations, and it can also simplify rollbacks. Since
MemoryListener already uses a priority-based list, it should be a good fit
for page conversion listeners.
Regarding error handling, -ENOMEM won't occur during page conversion
because the attribute xarray on the KVM side is populated earlier when QEMU
calls kvm_set_phys_mem() -> kvm_set_memory_attributes_private().
I'll note that, with guest_memfd supporting in-place conversion in the
future, this conversion path will likely change, and we might more
likely in getting more errors on some conversion paths. (e.g., shared ->
private could fail).
But I agree, we should keep complex error handling out of the picture
for now if not required.
--
Cheers,
David / dhildenb
