<div dir="ltr">Hi,<div><br></div><div>the patch looks good. It avoids using 'pread' which is not supported for remote images.</div><div><br></div><div>However, the patch that I sent for the mailing list introducing remote images used 'read' instead of 'pread'. </div><div><br></div><div>If more code locations were reverted to 'pread', it might be necessary to protect them as well.</div><div><br></div><div>Is there any reason for using 'pread' instead of 'read'? It seems to me that the code always reads files forwards. </div><div>Additionally, I also made some experiments (including zdtm tests) back in February using 'read' instead of 'pread' and it worked.</div><div><br></div><div>Best,</div><div>rodrigo</div><div><br><div><div class="gmail_extra"><br><div class="gmail_quote">2017-04-22 4:42 GMT+01:00 Andrei Vagin <span dir="ltr"><<a href="mailto:avagin@virtuozzo.com" target="_blank">avagin@virtuozzo.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Rodrigo, could you review this patch?<br>
<br>
On Fri, Apr 21, 2017 at 06:25:54PM +0300, Mike Rapoport wrote:<br>
> When --remote option is specified, read_local_page tries to pread from a<br>
> socket, and fails with "Illegal seek" error.<br>
> Restore single pread call for regular image files case and use read syscall<br>
> instead of pread for the --remote case.<br>
><br>
> Signed-off-by: Mike Rapoport <<a href="mailto:rppt@linux.vnet.ibm.com" target="_blank">rppt@linux.vnet.ibm.com</a>><br>
> ---<br>
> criu/pagemap.c | 18 +++++++++++++-----<br>
> 1 file changed, 13 insertions(+), 5 deletions(-)<br>
><br>
> diff --git a/criu/pagemap.c b/criu/pagemap.c<br>
> index 6c741b4..7d25c6f 100644<br>
> --- a/criu/pagemap.c<br>
> +++ b/criu/pagemap.c<br>
> @@ -265,15 +265,23 @@ static int read_local_page(struct page_read *pr, unsigned long vaddr,<br>
> return -1;<br>
><br>
> pr_debug("\tpr%d-%u Read page from self %lx/%"PRIx64"\n", pr->pid, pr->id, pr->cvaddr, pr->pi_off);<br>
> - while (1) {<br>
> + if (!opts.remote) {<br>
> ret = pread(fd, buf + curr, len - curr, pr->pi_off + curr);<br>
> - if (ret < 1) {<br>
> + if (ret != len) {<br>
> pr_perror("Can't read mapping page %d", ret);<br>
> return -1;<br>
> }<br>
> - curr += ret;<br>
> - if (curr == len)<br>
> - break;<br>
> + } else {<br>
> + while (1) {<br>
> + ret = read(fd, buf + curr, len - curr);<br>
> + if (ret < 0) {<br>
> + pr_perror("Can't read mapping page %d", ret);<br>
> + return -1;<br>
> + }<br>
> + curr += ret;<br>
> + if (curr == len)<br>
> + break;<br>
> + }<br>
> }<br>
><br>
> if (opts.auto_dedup) {<br>
<span class="gmail-m_7209670845697349871m_8986669502199782117HOEnZb"><font color="#888888">> --<br>
> 1.9.1<br>
><br>
</font></span></blockquote></div><br></div></div></div></div>