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

[Andrei Vagin]

On Tue, Apr 25, 2017 at 09:55:50PM +0300, Pavel Emelyanov wrote:
> On 04/25/2017 01:38 PM, Mike Rapoport wrote:
> > On Tue, Apr 25, 2017 at 08:56:19AM +0100, Rodrigo Bruno wrote:
> >>From 645c2f8e5afe090ee362d3225b97e2b5f1bc95cb Mon Sep 17 00:00:00 2001
> > From: Mike Rapoport <rppt at linux.vnet.ibm.com>
> > Date: Fri, 21 Apr 2017 17:29:10 +0300
> > Subject: [CRIU][PATCH v3] pagemap: fix reading pages from socket for --remote case
> > 
> > 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 introduce
> > maybe_read_page_img_cache version of maybe_read_page method.
> > 
> > Generally-approved-by: Rodrigo Bruno <rbruno at gsd.inesc-id.pt>
> > Signed-off-by: Mike Rapoport <rppt at linux.vnet.ibm.com>
> 
> Acked-by: Pavel Emelyanov <xemul at virtuozzo.com>

Applied, thanks
> 
> And Rodrigo was right telling that the posision in the page-read seem to
> be not-needed, as we always access pages images linearly. But that's another
> story, I guess we'll need to revisit this a bit later.
> 
> > ---
> > v3: use different ->maybe_read_page method for --remote case
> > v2: simplify call to pread in non-remote case, it does not need 'curr'
> > 
> >  criu/pagemap.c | 52 +++++++++++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 41 insertions(+), 11 deletions(-)
> > 
> > diff --git a/criu/pagemap.c b/criu/pagemap.c
> > index 6c741b4..353c469 100644
> > --- a/criu/pagemap.c
> > +++ b/criu/pagemap.c
> > @@ -255,7 +255,6 @@ static int read_local_page(struct page_read *pr, unsigned long vaddr,
> >  {
> >  	int fd = img_raw_fd(pr->pi);
> >  	int ret;
> > -	size_t curr = 0;
> >  
> >  	/*
> >  	 * Flush any pending async requests if any not to break the
> > @@ -265,15 +264,10 @@ 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) {
> > -		ret = pread(fd, buf + curr, len - curr, pr->pi_off + curr);
> > -		if (ret < 1) {
> > -			pr_perror("Can't read mapping page %d", ret);
> > -			return -1;
> > -		}
> > -		curr += ret;
> > -		if (curr == len)
> > -			break;
> > +	ret = pread(fd, buf, len, pr->pi_off);
> > +	if (ret != len) {
> > +		pr_perror("Can't read mapping page %d", ret);
> > +		return -1;
> >  	}
> >  
> >  	if (opts.auto_dedup) {
> > @@ -390,6 +384,40 @@ static int maybe_read_page_local(struct page_read *pr, unsigned long vaddr,
> >  	return ret;
> >  }
> >  
> > +static int maybe_read_page_img_cache(struct page_read *pr, unsigned long vaddr,
> > +				     int nr, void *buf, unsigned flags)
> > +{
> > +	unsigned long len = nr * PAGE_SIZE;
> > +	int fd = img_raw_fd(pr->pi);
> > +	int ret;
> > +	size_t curr = 0;
> > +
> > +	pr_debug("\tpr%d-%u Read page from self %lx/%"PRIx64"\n", pr->pid, pr->id, pr->cvaddr, pr->pi_off);
> > +	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) {
> > +		ret = punch_hole(pr, pr->pi_off, len, false);
> > +		if (ret == -1)
> > +			return -1;
> > +	}
> > +
> > +	if (ret == 0 && pr->io_complete)
> > +		ret = pr->io_complete(pr, vaddr, nr);
> > +
> > +	pr->pi_off += len;
> > +
> > +	return ret;
> > +}
> > +
> >  static int read_page_complete(int pid, unsigned long vaddr, int nr_pages, void *priv)
> >  {
> >  	int ret = 0;
> > @@ -803,7 +831,9 @@ int open_page_read_at(int dfd, int pid, struct page_read *pr, int pr_flags)
> >  	pr->id = ids++;
> >  	pr->pid = pid;
> >  
> > -	if (remote)
> > +	if (opts.remote)
> > +		pr->maybe_read_page = maybe_read_page_img_cache;
> > +	else if (remote)
> >  		pr->maybe_read_page = maybe_read_page_remote;
> >  	else
> >  		pr->maybe_read_page = maybe_read_page_local;
> > 
>