From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ross Lagerwall Subject: Re: [PATCH 22/29] tools/libxl: Allow limiting amount copied by datacopier Date: Fri, 19 Sep 2014 08:45:15 +0100 Message-ID: <541BDF0B.10502@citrix.com> References: <1410369067-1330-1-git-send-email-andrew.cooper3@citrix.com> <1410369067-1330-23-git-send-email-andrew.cooper3@citrix.com> <5412B095.4020805@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: <5412B095.4020805@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:36 AM, Wen Congyang wrote: > On 09/11/2014 01:11 AM, Andrew Cooper wrote: >> From: Ross Lagerwall >> >> Add a parameter, maxread, to limit the amount of data read from the >> source fd of a datacopier. >> >> Signed-off-by: Ross Lagerwall >> --- >> tools/libxl/libxl_aoutils.c | 9 +++++++-- >> tools/libxl/libxl_bootloader.c | 2 ++ >> tools/libxl/libxl_dom.c | 1 + >> tools/libxl/libxl_internal.h | 1 + >> 4 files changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c >> index caba637..6502325 100644 >> --- a/tools/libxl/libxl_aoutils.c >> +++ b/tools/libxl/libxl_aoutils.c >> @@ -145,7 +145,7 @@ static void datacopier_check_state(libxl__egc *egc, libxl__datacopier_state *dc) >> return; >> } >> } >> - } else if (!libxl__ev_fd_isregistered(&dc->toread)) { >> + } else if (!libxl__ev_fd_isregistered(&dc->toread) || dc->maxread == 0) { >> /* we have had eof */ >> datacopier_callback(egc, dc, 0, 0); >> return; >> @@ -233,7 +233,8 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev, >> } >> int r = read(ev->fd, >> buf->buf + buf->used, >> - sizeof(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; >> @@ -258,7 +259,11 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev, >> } >> buf->used += r; >> dc->used += r; >> + dc->maxread -= r; >> assert(buf->used <= sizeof(buf->buf)); >> + assert(dc->maxread >= 0); >> + if (dc->maxread == 0) >> + break; > > We can call libxl__ev_fd_deregister() here, and no need to touch datacopier_check_state(). > Otherwise, datacopier_readable() may be called again, and read() returns 0. And then > libxl__ev_fd_deregister() is called. > Well datacopier_check_state() is called immediately after the break and that would deregister the fd so datacopier_readable() would not be called again. What you suggest is definitely better though. Thanks -- Ross Lagerwall