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

[Kirill Tkhai]

On 21.12.2015 11:55, Pavel Emelyanov wrote:
> On 12/15/2015 04:29 PM, Kirill Tkhai wrote:
>> Since a deleted file may belong to a deleted directory
>> (which, in turn, may belong to another deleted, etc),
>> it's necessary to recreate all missing dentries in the path.
>>
>> Doing this in mkreg_ghost() is not a good idea, because
>> another ghost file may need some of the dentries in the path,
>> and this requires to do refcouting of recreated directories.
>>
>> To avoid this, we restore a ghost file in the root directory
>> of the mount instead.
>>
>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>> ---
>>  files-reg.c |   37 +++++++++++++++++++++++++++----------
>>  1 file changed, 27 insertions(+), 10 deletions(-)
>>
>> diff --git a/files-reg.c b/files-reg.c
>> index 8469417..e38a2e8 100644
>> --- a/files-reg.c
>> +++ b/files-reg.c
>> @@ -96,11 +96,23 @@ static int note_link_remap(char *path, struct ns_id *nsid)
>>  
>>  static int mkreg_ghost(char *path, u32 mode, struct ghost_file *gf, struct cr_img *img)
>>  {
>> -	int gfd, ret;
>> +	int gfd, ret, len, i = 0;
>> +
>> +	len = strlen(path);
>> +again:
>> +	sprintf(path + len, ".%x", i);
>>  
>>  	gfd = open(path, O_WRONLY | O_CREAT | O_EXCL, mode);
>> -	if (gfd < 0)
>> +	if (gfd < 0) {
>> +		if (errno != EEXIST)
>> +			return -1;
>> +		if (++i < INT_MAX)
>> +			goto again;
>>  		return -1;
>> +	}
>> +
>> +	len = strlen(gf->remap.rpath);
>> +	sprintf(gf->remap.rpath + len, ".%x", i);
>>  
>>  	ret = copy_file(img_raw_fd(img), gfd, 0);
>>  	close(gfd);
>> @@ -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?
 
>>  }
>>  
>>  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?
 
>>  	if (old_ns >= 0) {
>>  		if (setns(old_ns, CLONE_NEWNS) < 0)
>>
>> .
>>
> 

Thanks