[CRIU] [PATCH 1/3] [v3] crtools: close a signal descriptor after passing a preparation stage

[Pavel Emelyanov]

On 11/29/2016 12:34 AM, Andrei Vagin wrote:
> On Wed, Nov 23, 2016 at 11:24:07AM +0300, Pavel Emelyanov wrote:
>> On 11/18/2016 10:33 AM, Andrei Vagin wrote:
>>> From: Andrei Vagin <avagin at virtuozzo.com>
>>>
>>> This patch adds the --siganl-fd FD option to specify a file descriptor.
>>> CRIU will write '\0' to this descriptor and close it after passing
>>> a preparation stage.
>>>
>>> It is alternative way to demonizing a criu process after a preparation
>>> stage. It's imposiable to get exit code, if a process has daemonized.
>>>
>>> The introduced way allows to wait a preparation stage and to get an exit
>>> code. It can be easy used from shell and other script languages.
>>
>> Wait a second, if the whole effort is to get error from __preparation__
>> stage, then it should be fixed the other way -- once preparation stage
>> fails the call to cr_daemon() shouldn't happen and "criu <whatever>"
>> should just exit with an error. Doesn't it work like this now, btw?
> 
> It works, but this patch set is about the final exit code, when a work
> is done.

Done? The close_status_fd() is called right after cr_daemon() in all the
daemons we have. Also, the patch comment says: "CRIU will write '\0' to 
this descriptor and close it after passing a _preparation_ stage."

-- Pavel

> Could you look at the third patch, there is an example how it works.
> 
>>
>> -- Pavel
>>
>>> v3: fix a help message
>>>
>>> Cc: Mike Rapoport <mike.rapoport at gmail.com>
>>> Cc: Kir Kolyshkin <kir at openvz.org>
>>> Signed-off-by: Andrei Vagin <avagin at virtuozzo.com>
>>> ---
>>>  criu/crtools.c            | 12 +++++++++++-
>>>  criu/include/cr_options.h |  1 +
>>>  criu/include/util.h       |  1 +
>>>  criu/uffd.c               |  3 +++
>>>  criu/util.c               | 18 ++++++++++++++++++
>>>  5 files changed, 34 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/criu/crtools.c b/criu/crtools.c
>>> index 710059f..eedb8c0 100644
>>> --- a/criu/crtools.c
>>> +++ b/criu/crtools.c
>>> @@ -55,7 +55,9 @@
>>>  
>>>  #include "../soccr/soccr.h"
>>>  
>>> -struct cr_options opts;
>>> +struct cr_options opts = {
>>> +	.status_fd = -1,
>>> +};
>>>  
>>>  void init_opts(void)
>>>  {
>>> @@ -285,6 +287,7 @@ int main(int argc, char *argv[], char *envp[])
>>>  		{ "deprecated",			no_argument,		0, 1084 },
>>>  		{ "check-only",			no_argument,		0, 1085 },
>>>  		{ "display-stats",		no_argument,		0, 1086 },
>>> +		{ "status-fd",			required_argument,	0, 1087 },
>>>  		{ },
>>>  	};
>>>  
>>> @@ -608,6 +611,12 @@ int main(int argc, char *argv[], char *envp[])
>>>  		case 1086:
>>>  			opts.display_stats = true;
>>>  			break;
>>> +		case 1087:
>>> +			if (sscanf(optarg, "%d", &opts.status_fd) != 1) {
>>> +				pr_err("Unable to parse a value of --status-fd\n");
>>> +				return 1;
>>> +			}
>>> +			break;
>>>  		case 'V':
>>>  			pr_msg("Version: %s\n", CRIU_VERSION);
>>>  			if (strcmp(CRIU_GITID, "0"))
>>> @@ -945,6 +954,7 @@ usage:
>>>  "  --address ADDR        address of server or service\n"
>>>  "  --port PORT           port of page server\n"
>>>  "  -d|--daemon           run in the background after creating socket\n"
>>> +"  --status-fd FD        write \\0 to the FD and close it once daemon is ready\n"
>>>  "\n"
>>>  "Other options:\n"
>>>  "  -h|--help             show this text\n"
>>> diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
>>> index e017236..abebe05 100644
>>> --- a/criu/include/cr_options.h
>>> +++ b/criu/include/cr_options.h
>>> @@ -118,6 +118,7 @@ struct cr_options {
>>>  	bool			deprecated_ok;
>>>  	bool			display_stats;
>>>  	bool			check_only;
>>> +	int			status_fd;
>>>  };
>>>  
>>>  extern struct cr_options opts;
>>> diff --git a/criu/include/util.h b/criu/include/util.h
>>> index 8eaee33..ea1053b 100644
>>> --- a/criu/include/util.h
>>> +++ b/criu/include/util.h
>>> @@ -170,6 +170,7 @@ extern int cr_system(int in, int out, int err, char *cmd, char *const argv[], un
>>>  extern int cr_system_userns(int in, int out, int err, char *cmd,
>>>  				char *const argv[], unsigned flags, int userns_pid);
>>>  extern int cr_daemon(int nochdir, int noclose, int *keep_fd, int close_fd);
>>> +extern int close_status_fd(void);
>>>  extern int is_root_user(void);
>>>  
>>>  static inline bool dir_dots(const struct dirent *de)
>>> diff --git a/criu/uffd.c b/criu/uffd.c
>>> index cd370aa..e26a077 100644
>>> --- a/criu/uffd.c
>>> +++ b/criu/uffd.c
>>> @@ -967,6 +967,9 @@ int cr_lazy_pages(bool daemon)
>>>  		}
>>>  	}
>>>  
>>> +	if (close_status_fd())
>>> +		return -1;
>>> +
>>>  	nr_fds = epoll_nr_fds(task_entries->nr_tasks);
>>>  	epollfd = prepare_epoll(nr_fds, &events);
>>>  	if (epollfd < 0)
>>> diff --git a/criu/util.c b/criu/util.c
>>> index 2cf0355..ba54fc7 100644
>>> --- a/criu/util.c
>>> +++ b/criu/util.c
>>> @@ -687,6 +687,21 @@ out:
>>>  	return ret;
>>>  }
>>>  
>>> +int close_status_fd(void)
>>> +{
>>> +	char c = 0;
>>> +
>>> +	if (opts.status_fd < 0)
>>> +		return 0;
>>> +
>>> +	if (write(opts.status_fd, &c, 1) != 1) {
>>> +		pr_perror("Unable to write into the status fd");
>>> +		return -1;
>>> +	}
>>> +
>>> +	return close_safe(&opts.status_fd);
>>> +}
>>> +
>>>  int cr_daemon(int nochdir, int noclose, int *keep_fd, int close_fd)
>>>  {
>>>  	int pid;
>>> @@ -1144,6 +1159,9 @@ int run_tcp_server(bool daemon_mode, int *ask, int cfd, int sk)
>>>  		}
>>>  	}
>>>  
>>> +	if (close_status_fd())
>>> +		return -1;
>>> +
>>>  	if (sk >= 0) {
>>>  		ret = *ask = accept(sk, (struct sockaddr *)&caddr, &clen);
>>>  		if (*ask < 0)
>>>
>>
> .
>