On 21.09.2020 14:22, Oleksandr wrote: > On 14.09.20 16:52, Jan Beulich wrote: >> On 10.09.2020 22:21, Oleksandr Tyshchenko wrote: >>> --- a/xen/arch/x86/hvm/ioreq.c >>> +++ b/xen/arch/x86/hvm/ioreq.c >>> @@ -170,6 +170,29 @@ static bool hvm_wait_for_io(struct hvm_ioreq_vcpu *sv, >>> ioreq_t *p) >>> return true; >>> } >>> >>> +bool arch_handle_hvm_io_completion(enum hvm_io_completion io_completion) >> Do we need "handle" in here? Without it, I'd also not have to ask about >> moving hvm further to the front of the name... > For me without "handle" it will sound a bit confusing because of the > enum type which > has a similar name but without "arch" prefix: > bool arch_hvm_io_completion(enum hvm_io_completion io_completion)
Every function handles something; there's no point including "handle" in every name. Or else we'd have handle_memset() instead of just memset(), for example. > Shall I then move "hvm" to the front of the name here? As per comments on later patches, I think we want to consider dropping hvm prefixes or infixes altogether from the functions involved here. >>> @@ -1215,8 +1233,7 @@ void hvm_destroy_all_ioreq_servers(struct domain *d) >>> struct hvm_ioreq_server *s; >>> unsigned int id; >>> >>> - if ( !relocate_portio_handler(d, 0xcf8, 0xcf8, 4) ) >>> - return; >>> + arch_hvm_ioreq_destroy(d); >> So the call to relocate_portio_handler() simply goes away. No >> replacement, no explanation? > As I understand from the review comment on that for the RFC patch, there > is no > a lot of point of keeping this. Indeed, looking at the code I got the > same opinion. > I should have added an explanation in the commit description at least. > Or shall I return the call back? If there's a reason to drop it (which I can't see, but I also don't recall seeing the discussion you're mentioning), then doing so should be a separate patch with suitable reasoning. In the patch here you definitely should only transform what's already there. Jan
