[CRIU] [PATCH v2] bfd: bread: read in a loop when running with --remote

[Mike Rapoport]

On Mon, Jul 10, 2017 at 01:15:09PM +0100, Rodrigo Bruno wrote:
> Hi,
> 
> isn't it a good idea to extract the read loop into a separate function (I
> thought we already had one in the past...)?
> 
> I mean, the read syscall is not guaranteed to actually read the number
> requested bytes (not even if we are reading from a local file).

> Besides, if I remember correctly, there are multiple code locations that
> could use that new function.

I think it's the second, the first was in page-read
 
> cheers,
> rodrigo
> 
> 2017-07-10 12:56 GMT+01:00 Omri Kramer <omri.kramer at gmail.com>:
> 
> > When the --remote option is used, the file descriptor
> > used in bread is a socketand it may return after only
> > part of the data is read. Keep going looping until all
> > the data is received when --remote option is on.
> >
> > Signed-off-by: Omri Kramer <omri.kramer at gmail.com>
> > Signed-off-by: Lior Fisch <fischlior at gmail.com>
> > ---
> >  criu/bfd.c | 25 +++++++++++++++++++++++--
> >  1 file changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/criu/bfd.c b/criu/bfd.c
> > index 8269ab1..7c54d9b 100644
> > --- a/criu/bfd.c
> > +++ b/criu/bfd.c
> > @@ -15,6 +15,7 @@
> >  #include "util.h"
> >  #include "xmalloc.h"
> >  #include "page.h"
> > +#include "cr_options.h"
> >
> >  #undef LOG_PREFIX
> >  #define LOG_PREFIX "bfd: "
> > @@ -298,9 +299,29 @@ int bread(struct bfd *bfd, void *buf, int size)
> >  {
> >         struct xbuf *b = &bfd->b;
> >         int more = 1, filled = 0;
> > +       int ret;
> > +       size_t curr = 0;
> >
> > -       if (!bfd_buffered(bfd))
> > -               return read(bfd->fd, buf, size);
> > +       if (!bfd_buffered(bfd)) {
> > +               if(!opts.remote)
> > +                       return read(bfd->fd, buf, size);
> > +               /*
> > +                * Required to keep reading,
> > +                * for succesfully Reading all the data
> > +                * from the socket, when on remote option.
> > +                */
> > +               while (1) {
> > +                       ret = read(bfd->fd, buf + curr, size - curr);
> > +                       if (ret < 0) {
> > +                               pr_perror("Can't read from buffer\n");
> > +                               return -1;
> > +                       }
> > +
> > +                       curr += ret;
> > +                       if (curr == size || ret == 0)
> > +                               return curr;
> > +               }
> > +       }

I've looked at the code once more and it seems that with opts.remote we can
just fall through to chunk reading mode.
Pavel, Andrey, is there anything that prevents it?

> >         while (more > 0) {
> >                 int chunk;
> > --
> > 2.7.4
> >
> >

-- 
Sincerely yours,
Mike.