Re: [PATCH 1/2] xen/granttable: Support sub-page grants

[ANNIE LI <annie.li@oracle.com>]

On 2011-12-7 17:56, Ian Campbell wrote:
> On Wed, 2011-12-07 at 03:36 +0000, ANNIE LI wrote:
>> Thanks for your reviewing, Ian.
>>>>    EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access);
>>>>
>>>> +int gnttab_grant_foreign_access_subpage_v2(domid_t domid, unsigned long frame,
>>>> +					   int flags, unsigned page_off,
>>>> +					   unsigned length)
>>> Please drop the v2 suffixes on the public functions.
>> OK, the initial interface is without v2 suffixes. It was added in order
>> to reminder user the interfaces are only available for grant table v2.
>> But I am fine to remove it, and following ops fn pointers are better.
>>> Any reason not to route these via the ops table for consistency with all
>>> the other ops? Then your availability check becomes a test for NULL fn
>>> pointer rather than a specific version.
>> Ok, it is good.
>> How about following implements?
> Looks to be along the right lines. Thanks.
>
>> gnttab_v1_ops = {
>>    ...
>> .access_subpage = NULL;
>> .access_ref_subpage = NULL;
>> .access_trans = NULL;
>> .access_ref_trans = NULL;
>> }
> I think you can omit these since NULL is the default but perhaps
> explicitly listing them is useful in a self documenting type way.
>
> [...]
OK, I can delete those.
>> Same operations for access_ref_subpage, access_trans and access_ref_trans.
>>
>> bool gnttab_subpage_available()
>> {
>>        return (gnttab_interface->access_subpage != NULL);
>> }
>>
>> bool gnttab_subpage_available()
> Typo:       ..trans..
Thanks for pointing out this.

Thanks
Annie
>> {
>>        return (gnttab_interface->access_trans != NULL);
>> }
> Ian.
>
>> Thanks
>> Annie
>