From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ross Lagerwall Subject: Re: [PATCH 23/29] tools/libxl: Extend datacopier to support reading into a buffer Date: Fri, 19 Sep 2014 08:48:13 +0100 Message-ID: <541BDFBD.60008@citrix.com> References: <1410369067-1330-1-git-send-email-andrew.cooper3@citrix.com> <1410369067-1330-24-git-send-email-andrew.cooper3@citrix.com> <5412B38F.9040804@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5412B38F.9040804@cn.fujitsu.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: Wen Congyang , Andrew Cooper , Xen-devel Cc: Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org On 09/12/2014 09:49 AM, Wen Congyang wrote: > On 09/11/2014 01:11 AM, Andrew Cooper wrote: >> From: Ross Lagerwall >> >> Implement a read-only mode for libxl__datacopier. The mode is invoked >> when readbuf is set and writefd is < 0. On success, the callback passes >> the number of bytes read. >> >> Signed-off-by: Ross Lagerwall >> --- >> tools/libxl/libxl_aoutils.c | 59 +++++++++++++++++++++++++----------------- >> tools/libxl/libxl_internal.h | 4 ++- >> 2 files changed, 38 insertions(+), 25 deletions(-) >> >> diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c >> index 6502325..9183716 100644 >> --- a/tools/libxl/libxl_aoutils.c >> +++ b/tools/libxl/libxl_aoutils.c >> @@ -134,7 +134,7 @@ static void datacopier_check_state(libxl__egc *egc, libxl__datacopier_state *dc) >> STATE_AO_GC(dc->ao); >> int rc; >> >> - if (dc->used) { >> + if (dc->used && !dc->readbuf) { >> if (!libxl__ev_fd_isregistered(&dc->towrite)) { >> rc = libxl__ev_fd_register(gc, &dc->towrite, datacopier_writable, >> dc->writefd, POLLOUT); >> @@ -147,7 +147,7 @@ static void datacopier_check_state(libxl__egc *egc, libxl__datacopier_state *dc) >> } >> } else if (!libxl__ev_fd_isregistered(&dc->toread) || dc->maxread == 0) { >> /* we have had eof */ >> - datacopier_callback(egc, dc, 0, 0); >> + datacopier_callback(egc, dc, 0, dc->readbuf ? dc->used : 0); >> return; >> } else { >> /* nothing buffered, but still reading */ >> @@ -215,26 +215,31 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev, >> } >> assert(revents & POLLIN); >> for (;;) { >> - while (dc->used >= dc->maxsz) { >> - libxl__datacopier_buf *rm = LIBXL_TAILQ_FIRST(&dc->bufs); >> - dc->used -= rm->used; >> - assert(dc->used >= 0); >> - LIBXL_TAILQ_REMOVE(&dc->bufs, rm, entry); >> - free(rm); >> - } >> + libxl__datacopier_buf *buf = NULL; >> + int r; >> + >> + if (dc->readbuf) { >> + r = read(ev->fd, dc->readbuf + dc->used, dc->maxread); >> + } else { >> + while (dc->used >= dc->maxsz) { >> + libxl__datacopier_buf *rm = LIBXL_TAILQ_FIRST(&dc->bufs); >> + dc->used -= rm->used; >> + assert(dc->used >= 0); >> + LIBXL_TAILQ_REMOVE(&dc->bufs, rm, entry); >> + free(rm); >> + } >> >> - libxl__datacopier_buf *buf = >> - LIBXL_TAILQ_LAST(&dc->bufs, libxl__datacopier_bufs); >> - if (!buf || buf->used >= sizeof(buf->buf)) { >> - buf = malloc(sizeof(*buf)); >> - if (!buf) libxl__alloc_failed(CTX, __func__, 1, sizeof(*buf)); >> - buf->used = 0; >> - LIBXL_TAILQ_INSERT_TAIL(&dc->bufs, buf, entry); >> - } >> - int r = read(ev->fd, >> - buf->buf + buf->used, >> + buf = LIBXL_TAILQ_LAST(&dc->bufs, libxl__datacopier_bufs); >> + if (!buf || buf->used >= sizeof(buf->buf)) { >> + buf = malloc(sizeof(*buf)); >> + if (!buf) libxl__alloc_failed(CTX, __func__, 1, sizeof(*buf)); >> + buf->used = 0; >> + LIBXL_TAILQ_INSERT_TAIL(&dc->bufs, buf, entry); >> + } >> + r = read(ev->fd, buf->buf + buf->used, >> (sizeof(buf->buf) - buf->used) < dc->maxread ? >> (sizeof(buf->buf) - buf->used) : dc->maxread); >> + } >> if (r < 0) { >> if (errno == EINTR) continue; >> if (errno == EWOULDBLOCK) break; >> @@ -257,10 +262,12 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev, >> return; >> } >> } >> - buf->used += r; >> + if (!dc->readbuf) { >> + buf->used += r; >> + assert(buf->used <= sizeof(buf->buf)); >> + } >> dc->used += r; >> dc->maxread -= r; >> - assert(buf->used <= sizeof(buf->buf)); >> assert(dc->maxread >= 0); >> if (dc->maxread == 0) >> break; >> @@ -324,9 +331,13 @@ int libxl__datacopier_start(libxl__datacopier_state *dc) >> if (rc) goto out; >> } >> >> - rc = libxl__ev_fd_register(gc, &dc->towrite, datacopier_writable, >> - dc->writefd, POLLOUT); >> - if (rc) goto out; >> + if (dc->writefd >= 0) { >> + rc = libxl__ev_fd_register(gc, &dc->towrite, datacopier_writable, >> + dc->writefd, POLLOUT); >> + if (rc) goto out; >> + } >> + >> + assert(dc->readfd >= 0 || dc->writefd >= 0); > > If readfd and writefd are valid, and readbuf is not NULL, what it the behavior? > I'd say that's an invalid use of the API. There should probably be an assert to check this. Regards -- Ross Lagerwall