[CRIU] files-reg.c dump_ghost_file()

Radostin Stoyanov rstoyanov1 at gmail.com
Tue May 28 16:23:52 MSK 2019


Hi Adrian,

You are right! copy_file() should return 0 on success. We could fix it 
with something like:

diff --git a/criu/util.c b/criu/util.c
index 31cdee1f..0b3520f4 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -463,6 +463,7 @@ int copy_file(int fd_in, int fd_out, size_t bytes)

                 written += ret;
         }
+       ret = 0;
  err:
         xfree(buffer);
         return ret;

In addition, the copy_file() function is used only in files-reg.c and 
maybe we could/should move it there?

On 28/05/2019 10:21, Adrian Reber wrote:
> Going through the review of https://github.com/checkpoint-restore/criu/pull/697
> I had a closer look at dump_ghost_file() in files-reg.c and it seems
> wrong:
>
> 	if (gfe.chunks)
> 		ret = copy_file_to_chunks(fd, img, st->st_size);
> 	else
> 		ret = copy_file(fd, img_raw_fd(img), st->st_size);
> 	close(fd);
> 	if (ret)
> 		return -1;
>
> copy_file_to_chunks() returns -1 if something failed and else 0
> copy_file() returns -1 if something failed and else the number of bytes
> written (never 0).
>
> copy_file() seems to have changed its return value with:
>
> commit 3315681b4d9fb494ed35d5c78056e6199aef3ad1
> Author: rbruno at gsd.inesc-id.pt <rbruno at gsd.inesc-id.pt>
> Date:   Sat Feb 11 04:34:43 2017 +0100
>
>      util: Copy file w/o sendfile
>      
>      This is the case when the in/out files are image cache/proxy sockets.
>      
>      Signed-off-by: Rodrigo Bruno <rbruno at gsd.inesc-id.pt>
>      Signed-off-by: Katerina Koukiou <k.koukiou at gmail.com>
>      Signed-off-by: Pavel Emelyanov <xemul at virtuozzo.com>
>
>
> I think the right thing to do is to fix copy_file() to again return 0 on
> success.
>
> cc-ing Radostin as this seems to be image proxy related.
>
> 		Adrian



More information about the CRIU mailing list