[CRIU] [PATCH] Add inherit fd support

[Saied Kazemi]

Andrew and Pavel,

Thank you both for your feedback.  I sent a new patch that addresses
all the issues that you had mentioned.  I will also send a (rather
comprehensive) test program in a separate email that tests the inherit
fd functionality including clashing fds, multiple fds for the same
pipe, passing the wrong end of a pipe (e.g., read instead of write),
etc.

The only case that doesn't work is when a process opens a pipe through
/proc/<pid>/fd/<fd>.  As this is not a common case (i.e., most
applications dup a pipe fd as opposed to opening it via /proc/<pid>),
I am OK with criu restore failing to restore.  Criu would have failed
without --inherit-fd, so we're not breaking something that was
working.  As the fix seems to require changes to the heart of file
restore engine (including open_transport_fd), I refrained from making
such changes.  Again, I am perfectly OK with saying that this is one
of those cases that criu restore doesn't work.

I have rebased my patch to the latest head.  I think it's pretty safe
now to merge but I don't want to rush things.

Thanks,

--Saied


On Thu, Dec 4, 2014 at 2:46 AM, Andrew Vagin <avagin at parallels.com> wrote:
> On Thu, Dec 04, 2014 at 01:29:53PM +0300, Andrew Vagin wrote:
>> On Wed, Dec 03, 2014 at 01:04:18PM -0800, Saied Kazemi wrote:
>> > There are cases where a process's file descriptor cannot be restored
>> > from the checkpointed image.  For example, a pipe file descriptor with
>> > one end in the checkpointed process and the other end in a separate
>> > process (that was not part of the checkpointed process tree) cannot be
>> > restored because after checkpoint the pipe would be broken and removed.
>> >
>> > There are also cases where the user wants to use a new file during
>> > restore instead of the original file in the checkpointed image.  For
>> > example, the user wants to change the log file of a process from
>> > /path/to/oldlog to /path/to/newlog.
>> >
>> > In these cases, criu's caller should set up a new file descriptor to be
>> > inherited by the restored process and specify it with the --inherit-fd
>> > command line option.  The argument of --inherit-fd has the format
>> > fd[%d]:%s, where %d tells criu which of its own file descriptor to use
>> > for restoring file identified by %s.
>> >
>> > As a debugging aid, if the argument has the format debug[%d]:%s, it tells
>> > criu to write out the string after colon to the file descriptor %d.  This
>> > can be used to leave a "restore marker" in the output stream of the process.
>> >
>> > It's important to note that inherit fd support breaks applications
>> > that depend on the state of the file descriptor being inherited.  So,
>> > consider inherit fd only for specific use cases that you know for sure
>> > won't break the application.
>> >
>> > For examples please visit http://criu.org/Category:HOWTO.
>> >
>> > Signed-off-by: Saied Kazemi <saied at google.com>
>> ...
>> > @@ -302,6 +314,20 @@ static int open_pipe(struct file_desc *d)
>> >
>> >     pi = container_of(d, struct pipe_info, d);
>> >
>> > +   /*
>> > +    * If the pipe is in the inherit fd list,
>> > +    * it should be inherited rather than created.
>> > +    */
>> > +   pipe_name = pipe_id_string(pi->pe->pipe_id);
>> > +   if (pipe_name == NULL)
>> > +           return -1;
>> > +   tmp = inherit_fd_lookup_id(pipe_name);
>> > +   if (tmp >= 0) {
>> > +           pr_info("Pipe %s will be restored from inherit fd %d\n",
>> > +                   pipe_name, tmp);
>> > +           return tmp;
>>
>> A process can have more than one descriptor associated with a pipe. This
>> code doesn't handle this case.
>>
>> I think inherit_fd_lookup_id must return dup(inh->inh_fd).
>
> Actually Pavel already said about this problem.
>
>>
>> _______________________________________________
>> CRIU mailing list
>> CRIU at openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu