Re: [PATCH] xen/grant-table: fix suspend for non-PV guests

[David Vrabel <david.vrabel@citrix.com>]

On 16/06/14 15:20, Konrad Rzeszutek Wilk wrote:
> On Mon, Jun 16, 2014 at 12:49:18PM +0100, David Vrabel wrote:
>> Commit aa8532c32216ae07c3813b9aeb774517878a7573 (xen: refactor suspend
>> pre/post hooks) broke resuming PVHVM (auto-translated physmap) guests.
>>
>> The gnttab_suspend() would clear the mapping for the grant table
>> frames, but the ->unmap_frames() call is only applicable to PV guests.
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> 
> You forgot:
> 
> Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

I'm more interested in a Tested-by, any chance of one?

> However, if the code is structured like this, should we just
> make the 'unmap_frames' be a NOP for HVM and instead have:
> 
> if (gnttab_interface->unmap_frames)
> 	gnttab_interface->unmap_frames();
> 
> And the HVM path will make sure that 'unmap_frames' is
> set to NULL?
> 
> Maybe even change the name to 'unmap_frames_pv' ?

That may be useful as part of a wider refactor.  But the use of the
feature test mirrors the use in gnttab_setup() (as called by
gnttab_resume()) so I think it is clearer.

David