[CRIU] [PATCH 2/3] files-reg: Create ghost files in root directory of a mount

[Kirill Tkhai]

On 21.12.2015 13:49, Pavel Emelyanov wrote:
> 
>>>> @@ -163,6 +175,12 @@ static inline void ghost_path(char *path, int plen,
>>>>  		struct reg_file_info *rfi, RemapFilePathEntry *rfe)
>>>>  {
>>>>  	snprintf(path, plen, "%s.cr.%x.ghost", rfi->path, rfe->remap_id);
>>>> +	/*
>>>> +	 * We create ghost files in root of mount, to avoid (possible)
>>>> +	 * removed intemediate parent directories.
>>>> +	 */
>>>> +	while ((path = strchr(path, '/')) != NULL)
>>>> +		*path = '_';
>>>
>>> This hunk doesn't turn path into a file in the 'root of the mount', it turns 
>>> path into a file in the root of the whole tree.
>>
>> Is it OK for you if I replace the comment with "this function replaces '/' with '_'"?
>>
>> ghost_file() is used for gf->remap.rpath, which is a relative path in some mount, is
>> there exception from this rule?
> 
> It's relative path in mount _namespace_, not mount. Thus you create a file
> in / of mount namespace and subsequent linkat-s may (should) fail.

Ok, thanks

>>>>  }
>>>>  
>>>>  static int open_remap_ghost(struct reg_file_info *rfi,
>>>> @@ -414,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);
>>>> +	sprintf(path + ret, "/%s", gf->remap.rpath);
>>>> +
>>>>  	if (!unlink(path)) {
>>>>  		pr_info(" `- X [%s] ghost\n", path);
>>>>  		return;
>>>> @@ -437,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;
>>>> @@ -449,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;
>>>>  
>>>> @@ -479,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);
>>>
>>> How do you avoid calling this on link and pid remaps?
>>
>> Could you please explain what you mean? We add a file to &ghost_files list only
>> in open_remap_ghost(), which is called for REMAP_TYPE__GHOST only, don't we?
> 
> Oops, that's me misreading the patch. You've replaces remaps with ghost_files,
> so it's OK.
> 
> -- Pavel
>