Re: [Xen-devel] [PATCH v3 5/6] tools/libxl: Extend datacopier to support reading into a buffer

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ross Lagerwall writes ("[PATCH v3 5/6] tools/libxl: Extend datacopier to 
support reading into a buffer"):
> Currently a datacopier may source its data from an fd or local buffer, but \
its
> destination must be an fd.  For migration v2, libxl needs to read from the
> migration stream into a local buffer.

Can you please wrap your commit messages to 70 columns or less.

> Implement a "read into local buffer" mode, invoked when readbuf is set and
> writefd is -1.  On success, the callback passes the number of bytes read.
...
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 58ac658..931c00c 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2524,8 +2524,9 @@ typedef struct libxl__datacopier_buf 
> libxl__datacopier_buf;
>  
>  /* onwrite==1 means failure happened when writing, logged, errnoval is valid
>   * onwrite==0 means failure happened when reading
> - *     errnoval==0 means we got eof and all data was written
> - *     errnoval!=0 means we had a read error, logged
> + *     errnoval>=0 means we got eof and all data was written or number of 
> bytes
> + *                 written when in read mode
> + *     errnoval<0 means we had a read error, logged

I don't like this API.  Are you encoding a size in errnoval ?

Also, what does "errnoval<0 means we had a read error, logged" mean
for the actual value of errnoval ?  Is it a negated errno value ?

I think negated errno values should be entirely excluded from userland
code.  They just cause confusion.

Perhaps it would be better to include "bytes read" and "bytes written"
fields in the dc state structure.

Ian.

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