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

[Andrei Vagin]

On Mon, Nov 28, 2016 at 01:34:07PM -0800, 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. Could you look at the third patch, there is an example how it
> works.

BTW: the second patch fixes the bug, which was found with this patch.
> 
> > 
> > -- 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)
> > > 
> >