On 30/05/2019 07:18, Petre Pircalabu wrote: > Modified xc_mem_paging_enable to use directly xc_vm_event_enable and > moved the ring_page handling from client to libxc (xenpaging). > > Restricted vm_event_control usage only to simplest domctls which do > not expect any return values and change xc_vm_event_enable to call do_domctl > directly. > > Removed xc_memshr_ring_enable/disable and xc_memshr_domain_resume. > > Signed-off-by: Petre Pircalabu <[email protected]> > --- > tools/libxc/include/xenctrl.h | 49 +-------------------------------- > tools/libxc/xc_mem_paging.c | 23 +++++----------- > tools/libxc/xc_memshr.c | 34 ----------------------- > tools/libxc/xc_monitor.c | 31 +++++++++++++++++---- > tools/libxc/xc_private.h | 2 +- > tools/libxc/xc_vm_event.c | 64 > ++++++++++++++++--------------------------- > tools/xenpaging/xenpaging.c | 42 +++-------------------------
So, the diffstat here is very impressive, and judging by the delta, its all about not opencoding the use of the HVM_PARAM interface. Overall, this is clearly a good thing. However, it is quite difficult to follow exactly what is going on. AFAICT, this wants splitting into $N patches. One patch should refactor xc_mem_paging_enable() to use xc_vm_event_enable(), with the associated xenpaging cleanup. One patch should be the deletion of xc_memshr_* on its own, because AFAICT it is an isolated change. It also needs some justification, even if it is "interface is unused and/or redundant with $X". One patch (alone) should be the repositioning of the domain_pause() calls. This does certainly look like a vast improvement WRT error handling in xc_vm_event_enable(), but you've definitely changed the API (insofar as the expectation that the caller has paused the domain) goes, and its not at all obvious that this change is safe. It may very well be, but you need to convince people as to why in the commit message. I still haven't figured out what the purpose behind dropping the port parameter from xc_vm_event_control() is. ~Andrew _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
