[CRIU] [PATCH] link_remap: do not create excessive links for a single file

[Pavel Emelyanov]

On 07/13/2016 05:29 PM, Stanislav Kinsburskiy wrote:
> 
> 
> 13.07.2016 16:30, Pavel Emelyanov пишет:
>> On 07/13/2016 04:10 PM, Stanislav Kinsburskiy wrote:
>>>
>>> 13.07.2016 14:48, Pavel Emelyanov пишет:
>>>> On 06/30/2016 08:11 PM, Stanislav Kinsburskiy wrote:
>>>>> Currently criu always creates link for each file, considered as "silly-renamed".
>>>>> Even in those cases, when file is opened multiple times, it creates one new
>>>>> link for each occurrence.
>>>>> On restore only one link file is used for all the file descriptors, sharing
>>>>> this path. And only this link is removed.
>>>> Would you cook a zdtm test showing it?
>>> Sorry, I don't understand. Showing what exactly?
>>> I caught by using zdtm tests. Test "static/unlink_mmap00" should expose it.
>>> Two links created, only one used and unlinked. Another one stays on the
>>> disk.
>> And test passes :) Then modify it (or it's cleanup hook) to catch this error.
> 
> Checking for no "link_remap*" files left suits your purpose?

Yup :) IIRC we already have some test hook doing checks for ghost/remaps files
hanging around.

>>>>> The rest of the links are forgotten and abandoned on file system.
>>>>> This patch fixed this mess by creating the link only iff it's unique. Or
>>>>> skipping this job otherwise.
>>>>>
>>>>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
>>>>> ---
>>>>>    criu/files-reg.c |   50 ++++++++++++++++++++++++++++++++++++++++++++------
>>>>>    1 file changed, 44 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/criu/files-reg.c b/criu/files-reg.c
>>>>> index fd36ae9..b1ad271 100644
>>>>> --- a/criu/files-reg.c
>>>>> +++ b/criu/files-reg.c
>>>>> @@ -70,9 +70,11 @@ struct link_remap_rlb {
>>>>>    	struct list_head	list;
>>>>>    	struct ns_id		*mnt_ns;
>>>>>    	char			*path;
>>>>> +	char			*orig;
>>>>> +	u32			id;
>>>>>    };
>>>>>    
>>>>> -static int note_link_remap(char *path, struct ns_id *nsid)
>>>>> +static int note_link_remap(char *path, char *orig, struct ns_id *nsid, u32 id)
>>>>>    {
>>>>>    	struct link_remap_rlb *rlb;
>>>>>    
>>>>> @@ -84,11 +86,18 @@ static int note_link_remap(char *path, struct ns_id *nsid)
>>>>>    	if (!rlb->path)
>>>>>    		goto err2;
>>>>>    
>>>>> +	rlb->orig = strdup(orig);
>>>>> +	if (!rlb->orig)
>>>>> +		goto err3;
>>>>> +
>>>>>    	rlb->mnt_ns = nsid;
>>>>> +	rlb->id = id;
>>>>>    	list_add(&rlb->list, &remaps);
>>>>>    
>>>>>    	return 0;
>>>>>    
>>>>> +err3:
>>>>> +	xfree(rlb->path);
>>>>>    err2:
>>>>>    	xfree(rlb);
>>>>>    err:
>>>>> @@ -96,6 +105,22 @@ err:
>>>>>    	return -1;
>>>>>    }
>>>>>    
>>>>> +static int find_link_remap(char *path, struct ns_id *nsid, u32 *id)
>>>>> +{
>>>>> +	struct link_remap_rlb *rlb;
>>>>> +
>>>>> +	list_for_each_entry(rlb, &remaps, list) {
>>>>> +		if (rlb->mnt_ns != nsid)
>>>>> +			continue;
>>>>> +		if (strcmp(rlb->orig, path))
>>>>> +			continue;
>>>>> +
>>>>> +		*id = rlb->id;
>>>>> +		return 0;
>>>>> +	}
>>>>> +	return -ENOENT;
>>>>> +}
>>>>> +
>>>>>    /* Trim "a/b/c/d" to "a/b/d" */
>>>>>    static int trim_last_parent(char *path)
>>>>>    {
>>>>> @@ -725,6 +750,7 @@ static void __rollback_link_remaps(bool do_unlink)
>>>>>    		}
>>>>>    
>>>>>    		list_del(&rlb->list);
>>>>> +		xfree(rlb->orig);
>>>>>    		xfree(rlb->path);
>>>>>    		xfree(rlb);
>>>>>    	}
>>>>> @@ -781,28 +807,40 @@ static int create_link_remap(char *path, int len, int lfd,
>>>>>    		return -1;
>>>>>    	}
>>>>>    
>>>>> -	if (note_link_remap(link_name, nsid))
>>>>> +	if (note_link_remap(link_name, path, nsid, *idp))
>>>>>    		return -1;
>>>>>    
>>>>>    	return pb_write_one(img_from_set(glob_imgset, CR_FD_REG_FILES), &rfe, PB_REG_FILE);
>>>>>    }
>>>>>    
>>>>> -static int dump_linked_remap(char *path, int len, const struct stat *ost,
>>>>> -				int lfd, u32 id, struct ns_id *nsid)
>>>>> +static int dump_linked_remap_type(char *path, int len, int lfd, u32 id, struct ns_id *nsid, RemapType remap_type, bool unique)
>>>>>    {
>>>>>    	u32 lid;
>>>>>    	RemapFilePathEntry rpe = REMAP_FILE_PATH_ENTRY__INIT;
>>>>>    
>>>>> -	if (create_link_remap(path, len, lfd, &lid, nsid))
>>>>> -		return -1;
>>>>> +	if (!find_link_remap(path, nsid, &lid)) {
>>>>> +		pr_debug("Link remap for %s already exists with id %x\n",
>>>>> +				path, lid);
>>>>> +		if (unique)
>>>>> +			return 0;
>>>>> +	} else if (create_link_remap(path, len, lfd, &lid, nsid))
>>>>> +			return -1;
>>>>>    
>>>>>    	rpe.orig_id = id;
>>>>>    	rpe.remap_id = lid;
>>>>> +	rpe.has_remap_type = true;
>>>>> +	rpe.remap_type = remap_type;
>>>>>    
>>>>>    	return pb_write_one(img_from_set(glob_imgset, CR_FD_REMAP_FPATH),
>>>>>    			&rpe, PB_REMAP_FPATH);
>>>>>    }
>>>>>    
>>>>> +static int dump_linked_remap(char *path, int len, const struct stat *ost,
>>>>> +				int lfd, u32 id, struct ns_id *nsid)
>>>>> +{
>>>>> +	return dump_linked_remap_type(path, len, lfd, id, nsid, REMAP_TYPE__LINKED, false);
>>>>> +}
>>>>> +
>>>>>    static pid_t *dead_pids;
>>>>>    static int n_dead_pids;
>>>>>    
>>>>>
>>>>> _______________________________________________
>>>>> CRIU mailing list
>>>>> CRIU at openvz.org
>>>>> https://lists.openvz.org/mailman/listinfo/criu
>>>>> .
>>>>>
>>> .
>>>
> 
> .
>