Re: [Xen-devel] [PATCH XEN v5 13/23] tools: Refactor foreign memory mapping into libxenforeignmemory

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Wed, 2015-11-11 at 15:13 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH XEN v5 13/23] tools: Refactor foreign memory
> mapping into libxenforeignmemory"):
> > libxenforeignmemory will provide a stable API and ABI for mapping
> > foreign domain memory (subject to appropriate privileges).
> > 
> > The new library exposes an interface equivalent to
> > xc_map_foreign_memory_bulk, which all the other
> > xc_map_foreign_memory_* functions (which remain in libxc) are
> > implemented in terms of.
> ...
> > diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h
> > b/tools/libs/foreignmemory/include/xenforeignmemory.h
> 
> > +/*
> > + * Return a handle onto the hypercall driver.ÂÂLogs errors.
> > + */
> > +xenforeignmemory_handle *xenforeignmemory_open(xentoollog_logger
> > *logger,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂunsigned open_flags);
> 
> `onto the foreign memory driver'.
> 
> This file should probably say something about fork().ÂÂPerhaps just a
> refernce to the rules in the grant map driver.

Yes, same rules I think.

> 
> > +/*
> > + * Maps a range within one domain to a local address range.ÂÂMappings
> > + * should be unmapped with munmap and should follow the same rules as
> > mmap
> > + * regarding page alignment.
> > + *
> > + * prot is as for mmap(2).
> > + *
> > + * Can partially succeed. When a page cannot be mapped, its respective
> > + * field in @err is set to the corresponding errno value.
> > + *
> > + * Returns NULL if no pages can be mapped.
> ...
> > +void *xenforeignmemory_map(xenforeignmemory_handle *fmem, uint32_t
> > dom,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂint prot, const xen_pfn_t *arr, int *err,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂunsigned int num);
> 
> If it returns NULL, will all of *err be filled with errno values ?
> Ie is all of err[] always written ?
> 
> Does this function ever set errno ?

I'llÂfigure all these out and say.

> I would write `int err_out[num]' in the prototype which is a bit
> clearer about the semantics.

Are you suggesting:

void *xenforeignmemory_map(xenforeignmemory_handle *fmem, uint32_t dom,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂint prot, const xen_pfn_t *arr,
                           int err_out[num],
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂunsigned int num);

Is that (a var sized array based on another param) really allowed?

Or did you mean as comment like:

void *xenforeignmemory_map(xenforeignmemory_handle *fmem, uint32_t dom,
ÂÂÂÂ
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂint prot, const xen_pfn_t *arr,
                     
     int *err_out /*int err_out[num]*/,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂunsigned
int num);

> This API invites callers to fail to notice partial failures.
> 
> How about permitting err==NULL to mean `on partial failure, unmap
> everything and return NULL setting errno' ?

Not a bad idea.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel