Re: [Xen-devel] [PATCH v4 4/7] libxl: support mapping static shared memory areas during domain creation

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Feb 12, 2018 at 11:08:01PM +0800, Zhongze Liu wrote:
> >> > [...]
> >> >
> >> > > > +
> >> > > > +/*   libxl__sshm_do_map -- map pages into slave's physmap
> >> > > > + *
> >> > > > + *   This functions maps
> >> > > > + *     master gfn: [@msshm->begin + @sshm->offset, @msshm->end +
> >> > > > @sshm->offset)
> >> > > > + *   into
> >> > > > + *     slave gfn: [@sshm->begin, @sshm->end)
> >> > > > + *
> >> > > > + *   The gfns of the pages that are successfully mapped will be 
> >> > > > stored
> >> > > > + *   in @mapped, and the number of the gfns will be stored in 
> >> > > > @nmapped.
> >> > > > + *
> >> > > > + *   The caller have to guarentee that sshm->begin < sshm->end and 
> >> > > > all
> >> > > > the
> >> > >
> >> > >
> >> > > s/have to/has to/ I think.
> >> > > s/guarentee/guarantee/
> >> > >
> >> > > > + *   values are page-aligned.
> >> > >
> >> > >
> >> > > Hmmm, I don't see the alignement check in libxl. So do you rely on the
> >> > > toolstack to do it?
> >> >
> >> > Yes, This was done in libxlu_sshm.c.
> >>
> >> Same remark as above regarding libxlu. Note that I am maintaining the 
> >> tools.
> >> Ian and Wei may have a different opinion here.
> >>
> >
> > Please move the check to libxl.
> 
> I agree that we should move the alignment check to libxl.
> 
> But I still think that re-specification checks could only be done in
> libxlu, because
> this could only be spotted in the parsing phase -- it shouldn't be
> libxl's job. For
> libxl, an *_UNKNOWN values just indicates that the user of libxl
> didn't explicitly
> assign a value to the option upon calling the constructor of the sshm struct,
> and the code in libxl will set the option to its default value.
> However, I do think
> I failed to  handle the possibilities where an option value is not
> not *_UNKNOWN and
> not any valid values, which might occur if the user of libxl didn't
> use the constructor
> to initialize the sshm struct.

So there are two problems under discussion. One is parsing, which should
be done in the parser library (libxlu); the other is checking the
validity of the value in the struct, which should be done in libxl. I
don't think we fundamentally disagree with each other. Sorry for my
terse reply earlier.

Wei.

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