[CRIU] [PATCH v3 04/16] autofs: dump stage introduced

[Stanislav Kinsburskiy]

14.12.2015 13:14, Pavel Emelyanov пишет:
> On 12/14/2015 03:07 PM, Stanislav Kinsburskiy wrote:
>>
>> 14.12.2015 12:56, Pavel Emelyanov пишет:
>>> On 12/14/2015 02:52 PM, Stanislav Kinsburskiy wrote:
>>>> 14.12.2015 12:45, Pavel Emelyanov пишет:
>>>>> On 12/14/2015 02:12 PM, Stanislav Kinsburskiy wrote:
>>>>>
>>>>>>>> +
>>>>>>>> +	return ret;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static int autofs_fixup_options(struct mount_info *pm, int pgrp, bool catatonic,
>>>>>>>> +				int kernel_fd, bool kernel_fd_alive,
>>>>>>>> +				int read_fd)
>>>>>>>> +{
>>>>>>>> +	char **options;
>>>>>>>> +	int nr_opts, i, err = -1;
>>>>>>>> +	char *new_opt;
>>>>>>>> +	struct mount_info *t;
>>>>>>>> +
>>>>>>>> +	new_opt = construct_string("pgrp=%d,fd=%d", pgrp, kernel_fd);
>>>>>>>> +	if (!new_opt)
>>>>>>>> +		return -1;
>>>>>>>> +
>>>>>>>> +	if (!catatonic && !kernel_fd_alive) {
>>>>>>>> +		/* Write end is closed or invalid
>>>>>>>> +		 * Let's introduce new options only for us to carry pipe
>>>>>>>> +		 * information. */
>>>>>>> So, do I get it right, that the only reason for doing the complex write-end
>>>>>>> discovery is to decide whether or not to keep read_fd=%d in the image?
>>>>>> I don't understand. Write end is known and just checked for presence and
>>>>>> validity.
>>>>>> Discovery is done for read end. And we do it for two purposes:
>>>>>> 1) Check, that it's empty
>>>>>> 2) Dump, if write end is closed.
>>>>> I'm talking about the kernel_fd_alive variable. It's set to true after calling
>>>>> the autofs_kernel_pipe_alive() and is only used here, to check whether or not
>>>>> to keep the read_fd=%d option in the image or not. Also, the only thing for
>>>>> which the autofs_kernel_pipe_alive() is used is to set the kernel_fd_alive bool.
>>>>>
>>>>> So my question is -- why not keep the read_fd=%d option always and remove the
>>>>> whole autofs_kernel_pipe_alive() routine?
>>>> Because "read_fd" option is used on restore stage to device whether or
>>>> not temporary write end must be created.
>>>> In case of this option is absent, only artificial file is created to be
>>>> used to fix up mount point.
>>>> Otherwise there must be added write end of the pipe to file descriptors,
>>>> which must be closed after mount is fixed.
>>> Hm... OK, let's consider two autofs mount points and master tasks.
>>> The first -- with only read pipe end opened at task and write end opened at
>>> mountpoint (superblock). The second mountpoint has write end at superblock
>>> and task has both -- read and write ends. What would be the difference in
>>> option set and restore procedure?
>> For the first one:
>> 1) Create temporary write pipe end and add it to process fds, pipe_info,
>> etc.
>> 2) Create artificial fd for pipe write end, which will be used to fixup
>> the mount (and also close temporary write_fd).
>>
>> For the second one:
>> 1) Create artificial fd for pipe write end, which will be used to fixup
>> the mount.
> OK, and in the 2nd case the artificial fd will be "attached" to some existing
> pipe-object that is the one sitting the the tasks fds, right?

Yes and no. It won't be attached physically to a pipe-object by itself.
Artificial fd is "attached" (by desc) _only_ to  Autofs mount info 
object, which has information about write pipe fd _number_.

>>>>>>>> +		new_opt = add_to_string(new_opt, ",read_fd=%d", read_fd);
>>>>>>> If you need to keep some fs-specific data in the image that is not going to
>>>>>>> be passed as-is into the mount() call, just introduce the necessary optional
>>>>>>> fields in the mnt proto file.
>>>>>> This mnt proto file is generic.
>>>>>> Do you want me to add some autofs-specific fild to it (which can be
>>>>>> unset, btw, thus I need another boolean flag)?
>>>>> Well, this can be done in two ways. The first is to add optional field into the
>>>>> mnt entry. The second is to introduce one more image file (like for tmpfs).
>>>>>
>>>>> Keeping this in options string is also an option, but if this particular
>>>>> option is never passed into mount() system call and you parse this string
>>>>> anyway, I propose to save the cpu cycles and keep the desired data out of
>>>>> this string.
>>>> It will make code more complex, and, frankly, I don't see any
>>>> significant reason for this.
>>> Right now you keep read_fd value as sub-string inside the options one and parse
>>> one from there on restore. In my proposal the read_fd value will sit as integer
>>> right in the mnt_entry. Will it?
>> Sure, it will. But mnt_entry is generic.
>> I suppose it's nice to put there some fs-private information in explicit
>> way, don't you think?
> This is what I propose, but protobuf doesn't allow for void *field :) You would
> have to introduce the 'optional autofs_private autofs' field in the mnt_entry itself.
>
>

Ok, will do so.