[CRIU] [PATCH v2] Add inherit fd support

[Pavel Emelyanov]

On 12/10/2014 08:39 PM, Saied Kazemi wrote:
> On Wed, Dec 10, 2014 at 1:52 AM, Pavel Emelyanov <xemul at parallels.com> wrote:
>> On 12/09/2014 11:20 PM, Saied Kazemi wrote:
>>> There are cases where a process's file descriptor cannot be restored
>>> from the checkpoint images.  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 will be broken.
>>>
>>> There are also cases where the user wants to use a new file during
>>> restore instead of the original file at checkpoint time.  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 the file descriptor 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 descriptors
>>> to use for restoring the 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, for example, as an easy way 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>
>>
>> Patch applied, thanks a lot! I have only one question, would you clarify
>> it please:
>>
>>
>>> +int inherit_fd_fini()
>>> +{
>>> +     int reused;
>>> +     struct inherit_fd *inh;
>>> +
>>> +     list_for_each_entry(inh, &opts.inherit_fds, inh_list) {
>>> +             if (inh->inh_fd < 0) {
>>> +                     pr_err("File %s in inherit fd list has invalid fd %d\n",
>>> +                             inh->inh_id, inh->inh_fd);
>>> +                     return -1;
>>> +             }
>>> +
>>> +             reused = inherit_fd_reused(inh);
>>
>> How can the fd become reused? I've found no places where the fd sitting
>> in the inh object may become such.
> 
> Sorry that it's not obvious, Andrew had the same question.  Please let
> me know if we should add a comment to explain.
> 
> Some parts of the file restore engine can close an inherit fd
> explicitly by close() or implicitly by dup2() to reuse that
> descriptor.  For example, here is a code snippet from
> send_fd_to_self():
> 
>         if (move_img_fd(sock, dfd))
>                 return -1;
> 
>         if (dup2(fd, dfd) != dfd) {
>                 pr_perror("Can't dup local fd %d -> %d", fd, dfd);
>                 return -1;
>         }
> 
> dfd may be an inherit fd.  In the specific case of send_fd_to_self(),
> we check for clashes at the beginning of the function and, therefore,
> this function will not reuse an inherit fd.  However, to avoid adding
> a ton of clash detect and resolve code everywhere we close() and/or
> dup2(), we just make sure that when we're dup()ing or close()ing our
> inherit fd we're still dealing with the same fd that we inherited.

Ah, I see. Would you send a patch with the comment describing these 
precautions?

> I hope it's more clear now.
> 
>>> +             if (reused < 0)
>>> +                     return -1;
>>> +
>>> +             if (!reused) {
>>> +                     pr_debug("Closing inherit fd %d -> %s\n", inh->inh_fd,
>>> +                             inh->inh_id);
>>> +                     if (close_safe(&inh->inh_fd) < 0)
>>> +                             return -1;
>>> +             }
>>> +     }
>>> +     return 0;
>>> +}
>>
> 
> Thank you for applying this patch.
> 
> --Saied
> .
>