[CRIU] [PATCH] stats/pid-reuse: put stats to image directory instead of cwd

[Pavel Tikhomirov]

On 03/26/2018 11:54 AM, Pavel Emelyanov wrote:
> On 03/26/2018 11:12 AM, Pavel Tikhomirov wrote:
>> Statistics for a special dump/pre-dump/restore action should be
>> collected in the images directory of these action. We need these for
>> pid-reuse detection as we need to read the stats of parent predump.
> 
> What's wrong with checking pid-reuse form stats sitting in workdir?

We can read previous dump statistics from workdir before 
cr_*dump_finish, I agree with that, it is easy.

But Kirill says that "writing image outside of image directory is 
architectural problem", actually I think these way too for at least two 
reasons:

1) When stats image is in images dir we can latter analize stats for 
each iteration if we want it. Now doing migration in VZ7 we have only 
stats of last dump in the end.

2) If for instance stats image was not created, we will see for which 
iteration it had happened (there will be no file in directory). Now we 
wrongly and silently reuse stats of irrelevant older iteration, and will 
do wrong assumptions based on wrong data.

> 
>> Now then work-dir option is set statistics image is put to the work-dir
>> and on each iteration these image will replace previos image.
>>
>> For backward compatibility with older versions of p.haul create a
>> symlink in cwd so that p.haul still has access to the latest stats
>> image. Yet I plan to make p.haul work with stats through images dir.
>>
>> https://jira.sw.ru/browse/PSBM-82864
>> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
>> ---
>>   criu/stats.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/criu/stats.c b/criu/stats.c
>> index d344ad336..2c0f8bda4 100644
>> --- a/criu/stats.c
>> +++ b/criu/stats.c
>> @@ -1,6 +1,7 @@
>>   #include <unistd.h>
>>   #include <fcntl.h>
>>   #include <sys/time.h>
>> +#include <sys/stat.h>
>>   #include "int.h"
>>   #include "atomic.h"
>>   #include "cr_options.h"
>> @@ -157,6 +158,53 @@ static void display_stats(int what, StatsEntry *stats)
>>   		return;
>>   }
>>   
>> +static int fd_to_path(int fd, char *buf, int size) {
>> +	char proc_self_fd_path[PATH_MAX];
>> +	int ret;
>> +
>> +	if (snprintf(proc_self_fd_path, PATH_MAX, "/proc/self/fd/%d", fd) < 0) {
>> +		pr_perror("Failed to format /proc/self/fd/* path");
>> +		return -1;
>> +	}
>> +
>> +	ret = readlink(proc_self_fd_path, buf, size-1);
>> +	if (ret < 0) {
>> +		pr_perror("Failed to readlink %s path", proc_self_fd_path);
>> +		return -1;
>> +	}
>> +	buf[ret] = '\0';
>> +
>> +	return 0;
>> +}
>> +
>> +static void symlink_stats(char *target, char *name)
>> +{
>> +	char link_path[PATH_MAX];
>> +	struct stat st;
>> +	int ret;
>> +
>> +	if (snprintf(link_path, PATH_MAX, imgset_template[CR_FD_STATS].fmt, name) < 0)
>> +		goto exit;
>> +
>> +	ret = lstat(link_path, &st);
>> +	if (ret == -1 && errno != ENOENT)
>> +		goto exit;
>> +	else if (ret == 0) {
>> +		/* Real image occupies these location, no symlink needed */
>> +		if (!S_ISLNK(st.st_mode))
>> +			return;
>> +
>> +		/* Unlink symlink of previous iteration */
>> +		if (unlink(link_path) == -1)
>> +			goto exit;
>> +	}
>> +
>> +	if(!symlink(target, link_path))
>> +		return;
>> +exit:
>> +	pr_warn("Error when trying to create stats symlink");
>> +}
>> +
>>   void write_stats(int what)
>>   {
>>   	StatsEntry stats = STATS_ENTRY__INIT;
>> @@ -203,9 +251,20 @@ void write_stats(int what)
>>   	} else
>>   		return;
>>   
>> -	img = open_image_at(AT_FDCWD, CR_FD_STATS, O_DUMP, name);
>> +	img = open_image(CR_FD_STATS, O_DUMP, name);
>>   	if (img) {
>> +		char image_path[PATH_MAX];
>> +
>>   		pb_write_one(img, &stats, PB_STATS);
>> +
>> +		/*
>> +		 * Backward compatibility: old p.haul whants to read stats
>> +		 * from working directory, but not from images directory, so
>> +		 * create a symlink for it.
>> +		 */
>> +		if (!fd_to_path(img->fd, image_path, PATH_MAX))
>> +			symlink_stats(image_path, name);
>> +
>>   		close_image(img);
>>   	}
>>   
>>
> 

-- 
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.