[CRIU] [PATCH] pagemap: fix reading pages from socket for --remote case

[Rodrigo Bruno]

Hi,

the patch looks good. It avoids using 'pread' which is not supported for
remote images.

However, the patch that I sent for the mailing list introducing remote
images used 'read' instead of 'pread'.

If more code locations were reverted to 'pread', it might be necessary to
protect them as well.

Is there any reason for using 'pread' instead of 'read'? It seems to me
that the code always reads files forwards.
Additionally, I also made some experiments (including zdtm tests) back in
February using 'read' instead of 'pread' and it worked.

Best,
rodrigo


2017-04-22 4:42 GMT+01:00 Andrei Vagin <avagin at virtuozzo.com>:

> Rodrigo, could you review this patch?
>
> On Fri, Apr 21, 2017 at 06:25:54PM +0300, Mike Rapoport wrote:
> > When --remote option is specified, read_local_page tries to pread from a
> > socket, and fails with "Illegal seek" error.
> > Restore single pread call for regular image files case and use read
> syscall
> > instead of pread for the --remote case.
> >
> > Signed-off-by: Mike Rapoport <rppt at linux.vnet.ibm.com>
> > ---
> >  criu/pagemap.c | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/criu/pagemap.c b/criu/pagemap.c
> > index 6c741b4..7d25c6f 100644
> > --- a/criu/pagemap.c
> > +++ b/criu/pagemap.c
> > @@ -265,15 +265,23 @@ static int read_local_page(struct page_read *pr,
> unsigned long vaddr,
> >               return -1;
> >
> >       pr_debug("\tpr%d-%u Read page from self %lx/%"PRIx64"\n", pr->pid,
> pr->id, pr->cvaddr, pr->pi_off);
> > -     while (1) {
> > +     if (!opts.remote) {
> >               ret = pread(fd, buf + curr, len - curr, pr->pi_off + curr);
> > -             if (ret < 1) {
> > +             if (ret != len) {
> >                       pr_perror("Can't read mapping page %d", ret);
> >                       return -1;
> >               }
> > -             curr += ret;
> > -             if (curr == len)
> > -                     break;
> > +     } else {
> > +             while (1) {
> > +                     ret = read(fd, buf + curr, len - curr);
> > +                     if (ret < 0) {
> > +                             pr_perror("Can't read mapping page %d",
> ret);
> > +                             return -1;
> > +                     }
> > +                     curr += ret;
> > +                     if (curr == len)
> > +                             break;
> > +             }
> >       }
> >
> >       if (opts.auto_dedup) {
> > --
> > 1.9.1
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/criu/attachments/20170423/4a540e6b/attachment.html>