From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v2 5/6] tools/libxl: Extend datacopier to support reading into a buffer Date: Wed, 11 Mar 2015 12:19:58 +0000 Message-ID: <1426076398.21353.220.camel@citrix.com> References: <1425668741-28003-1-git-send-email-andrew.cooper3@citrix.com> <1425668741-28003-6-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1425668741-28003-6-git-send-email-andrew.cooper3@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper Cc: Ross Lagerwall , Ian Jackson , Wei Liu , Xen-devel List-Id: xen-devel@lists.xenproject.org On Fri, 2015-03-06 at 19:05 +0000, Andrew Cooper wrote: > From: Ross Lagerwall > > 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. > > 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. > + if (dc->readbuf) { > + r = read(ev->fd, dc->readbuf + dc->used, dc->maxread); Should the final parameter not be maxread - used? I assume the caller is expected to make maxread and the readbuf size consistent (which could BTW be mentioned in an update to the comments relating to this interface). > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 22921c7..3e1d78a 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -2524,7 +2524,8 @@ _hidden void libxl__device_disk_local_initiate_detach(libxl__egc *egc, > > /* 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 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 Nit: the last should be errnoval < 0. I'm not sure how I feel about using a field called errnoval to store something other than an errno though. I suppose I can live with it. > * onwrite==-1 means some other internal failure, errnoval not valid, logged > * If we get POLLHUP, we call callback_pollhup(..., onwrite, -1); > @@ -2553,6 +2554,7 @@ struct libxl__datacopier_state { > /* remaining fields are private to datacopier */ > libxl__ev_fd toread, towrite; > ssize_t used; > + void *readbuf; > LIBXL_TAILQ_HEAD(libxl__datacopier_bufs, libxl__datacopier_buf) bufs; > }; >