[CRIU] [PATCH v4 5/6] files-reg: Recreate deleted parent directories during restore of ghost file

[Kirill Tkhai]

On 27.01.2016 17:10, Pavel Emelyanov wrote:
> On 01/25/2016 07:40 PM, Kirill Tkhai wrote:
>> This patch makes ghost files to restore with the right path.
>>
>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>> ---
>>  files-reg.c |   97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 92 insertions(+), 5 deletions(-)
>>
>> diff --git a/files-reg.c b/files-reg.c
>> index b6c348a..dbe5e37 100644
>> --- a/files-reg.c
>> +++ b/files-reg.c
>> @@ -1217,13 +1217,90 @@ static int linkat_hard(int odir, char *opath, int ndir, char *npath, uid_t owner
>>  	return ret;
>>  }
>>  
>> +/* Prints parent and returns place where last delimiter was */
>> +static char *sprintf_parent_dir(char *dst, const char *path)
>> +{
>> +	char *p;
>> +
>> +	strcpy(dst, path);
>> +
>> +	p = strrchr(dst, '/');
>> +	if (!p) {
>> +		pr_err("Path %s has no parent dir", path);
>> +		return NULL;
>> +	}
>> +	*p = '\0';
>> +
>> +	return p;
>> +}
>> +
>> +static void rm_parent_dirs(int mntns_root, char *path, int count)
>> +{
>> +	char pdir[PATH_MAX], *p;
>> +
>> +	if (!count || !sprintf_parent_dir(pdir, path))
>> +		return;
>> +
>> +	while (count--) {
>> +		if (unlinkat(mntns_root, pdir, AT_REMOVEDIR))
>> +			pr_perror("Can't remove %s AT %d", pdir, mntns_root);
>> +		else
>> +			pr_debug("Unlinked parent dir: %s AT %d\n", pdir, mntns_root);
>> +
>> +		p = strrchr(pdir, '/');
>> +		if (p)
>> +			*p = '\0';
>> +	}
>> +}
>> +
>> +/* Construct parent dir name and mkdir parent/grandparents if they're not exist */
>> +static int make_parent_dirs_if_need(int mntns_root, char *path)
>> +{
>> +	char pdir[PATH_MAX], *p;
> 
> Too many PATH_MAX strings on the stack. Can we work directly on the path variable?

Ok, will do in v5.

>> +	int err, count = 0;
>> +	struct stat st;
>> +
>> +	sprintf_parent_dir(pdir, path);
>> +
>> +	if (fstatat(mntns_root, pdir, &st, AT_EMPTY_PATH) == 0)
> 
> Is this fstatat() required? The below mkdir() would return EEXISTS anyway.

fstatat() checks parent path, and it's a likely case when the whole hierarhy exists.

mkdir() creates directories from grand..grand parent to parent:

/dir
/dir/subdir
/dir/subdir/childsubdir
...

I don't think it's good to use mkdir for the whole hierarhy for every file.

Maybe you mean something else?

>> +		return 0;
>> +	if (errno != ENOENT) {
>> +		pr_perror("Can't stat %s", pdir);
>> +		return -1;
>> +	}
>> +
>> +	p = pdir;
>> +	do {
>> +		p = strchr(p, '/');
>> +		if (p)
>> +			*p = '\0';
>> +
>> +		err = mkdirat(mntns_root, pdir, 0777);
>> +		if (err && errno != EEXIST) {
>> +			pr_perror("Can't create dir: %s AT %d", pdir, mntns_root);
>> +			goto cleanup;
>> +		} else if (!err) {
>> +			pr_debug("Created parent dir: %s AT %d\n", pdir, mntns_root);
>> +			count++;
>> +		}
>> +
>> +		if (p)
>> +			*p++ = '/';
>> +	} while (p);
>> +
>> +	return count;
>> +cleanup:
>> +	rm_parent_dirs(mntns_root, pdir, count);
>> +	return -1;
>> +}
>> +
>>  /*
>>   * This routine properly resolves d's path handling ghost/link-remaps.
>>   * The open_cb is a routine that does actual open, it differs for
>>   * files, directories, fifos, etc.
>>   */
>>  
>> -static int rfi_remap(struct reg_file_info *rfi)
>> +static int rfi_remap(struct reg_file_info *rfi, int *level)
>>  {
>>  	struct mount_info *mi, *rmi, *tmi;
>>  	char _path[PATH_MAX], *path = _path;
>> @@ -1269,14 +1346,23 @@ static int rfi_remap(struct reg_file_info *rfi)
>>  	mntns_root = mntns_get_root_fd(tmi->nsid);
>>  
>>  out_root:
>> -	return linkat_hard(mntns_root, rpath, mntns_root, path, rfi->remap->owner);
>> +	*level = make_parent_dirs_if_need(mntns_root, path);
>> +	if (*level < 0)
>> +		return -1;
>> +
>> +	if (linkat_hard(mntns_root, rpath, mntns_root, path, rfi->remap->owner) < 0) {
>> +		rm_parent_dirs(mntns_root, path, *level);
>> +		return -1;
>> +	}
>> +
>> +	return 0;
>>  }
>>  
>>  int open_path(struct file_desc *d,
>>  		int(*open_cb)(int mntns_root, struct reg_file_info *, void *), void *arg)
>>  {
>> +	int tmp, mntns_root, level;
>>  	struct reg_file_info *rfi;
>> -	int tmp, mntns_root;
>>  	char *orig_path = NULL;
>>  
>>  	if (inherited_fd(d, &tmp))
>> @@ -1292,7 +1378,7 @@ int open_path(struct file_desc *d,
>>  			 */
>>  			orig_path = rfi->path;
>>  			rfi->path = rfi->remap->rpath;
>> -		} else if (rfi_remap(rfi) < 0) {
>> +		} else if (rfi_remap(rfi, &level) < 0) {
>>  			static char tmp_path[PATH_MAX];
>>  
>>  			if (errno != EEXIST) {
>> @@ -1316,7 +1402,7 @@ int open_path(struct file_desc *d,
>>  			snprintf(tmp_path, sizeof(tmp_path), "%s.cr_link", orig_path);
>>  			pr_debug("Fake %s -> %s link\n", rfi->path, rfi->remap->rpath);
>>  
>> -			if (rfi_remap(rfi) < 0) {
>> +			if (rfi_remap(rfi, &level) < 0) {
>>  				pr_perror("Can't create even fake link!");
>>  				return -1;
>>  			}
>> @@ -1356,6 +1442,7 @@ int open_path(struct file_desc *d,
>>  	if (rfi->remap) {
>>  		if (!rfi->remap->is_dir) {
>>  			unlinkat(mntns_root, rfi->path, 0);
>> +			rm_parent_dirs(mntns_root, rfi->path, level);
>>  		}
>>  
>>  		BUG_ON(!rfi->remap->users);
>>
>> .
>>
>