Re: [Xen-devel] [PATCH 22/29] tools/libxl: Allow limiting amount copied by datacopier

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

On 09/12/2014 09:36 AM, Wen Congyang wrote:
> On 09/11/2014 01:11 AM, Andrew Cooper wrote:
> > From: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> >
> > Add a parameter, maxread, to limit the amount of data read from the
> > source fd of a datacopier.
> >
> > Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> > ---
> >   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

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