[CRIU] [PATCH v4 2/6] files-reg: Make try_clean_ghost() use struct ghost_file ptr

[Kirill Tkhai]

On 07.02.2016 03:32, Andrew Vagin wrote:
> On Sat, Feb 06, 2016 at 05:34:38PM +0300, Kirill Tkhai wrote:
>> On 06.02.2016 04:03, Andrew Vagin wrote:
>>> Kirill, here is one more issue for you. It was introduced in this patch.
>>>
>>> [root at fc22-vm criu]# ./test/zdtm.py run -t zdtm/live/static/unlink_fstat00 --fault 2 --report report -f h 
>>> === Run 1/1 ================
>>>
>>> =================== Run zdtm/live/static/unlink_fstat00 in h ===================
>>> Start test
>>> ./unlink_fstat00 --pidfile=unlink_fstat00.pid --outfile=unlink_fstat00.out --filename=unlink_fstat00.test
>>> Dangling ghost file
>>> Run criu dump
>>> Forcing 2 fault
>>> Running zdtm/live/static/unlink_fstat00.hook(--pre-restore)
>>> Run criu restore
>>> Forcing 2 fault
>>> =[log]=> dump/zdtm/live/static/unlink_fstat00/72/1/restore.log
>>> ------------------------ grep Error ------------------------
>>> (00.014193)     72: Error (cr-restore.c:1606): BUG at cr-restore.c:1606
>>> (00.021403) Error (cr-restore.c:1305): 72 killed by signal 11
>>> (00.021431) Error (cr-restore.c:2127): Restoring FAILED.
>>> ------------------------ ERROR OVER ------------------------
>>> Running zdtm/live/static/unlink_fstat00.hook(--fault, restore)
>>> ###### Test zdtm/live/static/unlink_fstat00 FAIL at hook --fault restore #######
>>>
>>> ##################################### FAIL #####################################
>>
>> ghost_files are changed in other process, and the changes are not visible to parent.
> 
> How did it work before you changes?

It used to do not to try to recreate a ghost file under n+1 index in case of ghost.xxx.n
exists. It just used to fail in this case.
 
>>
>> This patch is to be able to create ghost files under names with a digit suffix,
>> if the standard  "%s.cr.%x.ghost" (rfi->path, rfe->remap_id)  is not available.
>> So, it's need a different solution.
>>
>> We can't check disk state from parent, because mount points are restored only in
>> restore_task_with_children() from child. So, the child should report parent,
>> which ghosts names it has changed.
>>
>> shmem is not infinity; sending the information about changing of names using
>> socket is ugly. Do you know better solution?
>>  
>>> On Mon, Jan 25, 2016 at 07:39:44PM +0300, Kirill Tkhai wrote:
>>>> This cleanup will be used in next patch.
>>>>
>>>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>>>> ---
>>>>  files-reg.c |   15 +++++++--------
>>>>  1 file changed, 7 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/files-reg.c b/files-reg.c
>>>> index 735a15a..648c8d2 100644
>>>> --- a/files-reg.c
>>>> +++ b/files-reg.c
>>>> @@ -432,17 +432,18 @@ int prepare_remaps(void)
>>>>  	return ret;
>>>>  }
>>>>  
>>>> -static void try_clean_ghost(struct remap_info *ri)
>>>> +static void try_clean_ghost(struct ghost_file *gf)
>>>>  {
>>>>  	char path[PATH_MAX];
>>>>  	int mnt_id, ret;
>>>>  
>>>> -	mnt_id = ri->rfi->rfe->mnt_id; /* rirfirfe %) */
>>>> +	mnt_id = gf->remap.rmnt_id;
>>>>  	ret = rst_get_mnt_root(mnt_id, path, sizeof(path));
>>>>  	if (ret < 0)
>>>>  		return;
>>>>  
>>>> -	ghost_path(path + ret, sizeof(path) - 1, ri->rfi, ri->rfe);
>>>> +	snprintf(path + ret, PATH_MAX - ret, "/%s", gf->remap.rpath);
>>>> +
>>>>  	if (!unlink(path)) {
>>>>  		pr_info(" `- X [%s] ghost\n", path);
>>>>  		return;
>>>> @@ -455,7 +456,6 @@ static void try_clean_ghost(struct remap_info *ri)
>>>>  	 */
>>>>  
>>>>  	if ((errno == EISDIR)) {
>>>> -		strncpy(path + ret, ri->rfi->path, sizeof(path) - 1);
>>>>  		if (!rmdir(path)) {
>>>>  			pr_info(" `- Xd [%s] ghost\n", path);
>>>>  			return;
>>>> @@ -467,7 +467,7 @@ static void try_clean_ghost(struct remap_info *ri)
>>>>  
>>>>  void try_clean_remaps(int ns_fd)
>>>>  {
>>>> -	struct remap_info *ri;
>>>> +	struct ghost_file *gf;
>>>>  	int old_ns = -1;
>>>>  	int cwd_fd = -1;
>>>>  
>>>> @@ -497,9 +497,8 @@ void try_clean_remaps(int ns_fd)
>>>>  		}
>>>>  	}
>>>>  
>>>> -	list_for_each_entry(ri, &remaps, list)
>>>> -		if (ri->rfe->remap_type == REMAP_TYPE__GHOST)
>>>> -			try_clean_ghost(ri);
>>>> +	list_for_each_entry(gf, &ghost_files, list)
>>>> +		try_clean_ghost(gf);
>>>>  
>>>>  	if (old_ns >= 0) {
>>>>  		if (setns(old_ns, CLONE_NEWNS) < 0)
>>>>