Re: [Xen-devel] [PATCH] common/gnttab: Explicitly default to gnttab v1 during domain creation

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper
> Sent: 09 August 2018 12:22
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Xen-devel <xen-
> devel@xxxxxxxxxxxxx>
> Cc: Jan Beulich <JBeulich@xxxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Julien Grall <julien.grall@xxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>; Roger Pau Monne <roger.pau@xxxxxxxxxx>; George
> Dunlap <George.Dunlap@xxxxxxxxxx>
> Subject: Re: [PATCH] common/gnttab: Explicitly default to gnttab v1 during
> domain creation
> 
> On 09/08/18 12:17, Paul Durrant wrote:
> >>
> >>>> which rearranges large chunks of DOMCTL_createdomain
> >>>> ---
> >>>>  xen/common/grant_table.c | 40 +++++++---------------------------------
> >>>>  1 file changed, 7 insertions(+), 33 deletions(-)
> >>>>
> >>>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> >>>> index d9ec711..8bae656 100644
> >>>> --- a/xen/common/grant_table.c
> >>>> +++ b/xen/common/grant_table.c
> >>>> @@ -49,10 +49,7 @@ struct grant_table {
> >>>>      percpu_rwlock_t       lock;
> >>>>      /* Lock protecting the maptrack limit */
> >>>>      spinlock_t            maptrack_lock;
> >>>> -    /*
> >>>> -     * The defined versions are 1 and 2.  Set to 0 if we don't know
> >>>> -     * what version to use yet.
> >>>> -     */
> >>> Given that only the guest can set the version by hypercall, it might be
> >> worth comment somewhere saying that grant table always start in v1
> format
> >> and then get morphed if the guest sets v2. I guess it might be inferred
> that a
> >> v1 table is immutable (for those who don't look too hard).
> >>
> >> Really?  I don't think that's a reasonable inference to make, given
> >> GNTTABOP_set_version as a public interface.
> >>
> > If you think it is sufficiently evident then fine, but the comment you're
> removing implies that 0 -> 1 and 0 -> 2 are valid transitions but maybe 1 -> 
> 2 is
> not (otherwise why would you have 0, right?)
> 
> /* Defaults to v1.  May be changed with GNTTABOP_set_version.  All other
> values are invalid. */
> 
> ?

LGTM.

  Paul

> 
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel