[CRIU] [PATCH 2/5] util: clone service descriptors, if fd tables are shared for tasks

Andrew Vagin avagin at parallels.com
Sat Dec 29 05:24:01 EST 2012


On Wed, Dec 26, 2012 at 01:16:39PM +0400, Pavel Emelyanov wrote:
> On 12/25/2012 05:10 PM, Andrey Vagin wrote:
> > It looks like a namespace for service descriptors.
> > It will be used for restoring tasks with shared fd tables.
> > Service descriptors should be its for each process.
> 
> The implementation looks horrible.
I don't like this too...
> Why do we need per service fd type explicit helper to clone it?

due to proc_dir_fd, current_logfd, ...

These variables are internal logic and they should be changed on this
stage.

> 
> > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > ---
> >  image.c           | 22 +++++++++++++++++
> >  include/crtools.h |  2 ++
> >  include/log.h     |  1 +
> >  log.c             | 11 +++++++++
> >  util.c            | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  5 files changed, 109 insertions(+), 1 deletion(-)
> > 
> > diff --git a/image.c b/image.c
> > index f1f93a0..70ababe 100644
> > --- a/image.c
> > +++ b/image.c
> > @@ -276,6 +276,28 @@ err:
> >  	return -1;
> >  }
> >  
> > +int clone_image_dir(void)
> > +{
> > +	int fd;
> > +
> > +	if (image_dir_fd < 0)
> > +		return 0;
> > +
> > +	fd = get_service_fd(IMG_FD_OFF);
> > +	if (fd < 0) {
> > +		pr_perror("Can't get image fd");
> > +		return -1;
> > +	}
> > +
> > +	image_dir_fd = dup2(image_dir_fd, fd);
> > +	if (image_dir_fd < 0) {
> > +		pr_perror("Unable to duplicate an image directory descriptor");
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  int open_image_dir(void)
> >  {
> >  	int fd;
> > diff --git a/include/crtools.h b/include/crtools.h
> > index 1155341..9d94fd7 100644
> > --- a/include/crtools.h
> > +++ b/include/crtools.h
> > @@ -121,6 +121,7 @@ enum sfd_type {
> >  	SERVICE_FD_MAX
> >  };
> >  
> > +extern int clone_service_fd(bool to_init);
> >  extern int init_service_fd(void);
> >  extern int get_service_fd(enum sfd_type type);
> >  extern bool is_service_fd(int fd, enum sfd_type type);
> > @@ -163,6 +164,7 @@ extern void print_image_data(int fd, unsigned int length, int show);
> >  extern struct cr_fd_desc_tmpl fdset_template[CR_FD_MAX];
> >  
> >  extern int open_image_dir(void);
> > +extern int clone_image_dir(void);
> >  extern void close_image_dir(void);
> >  
> >  int open_image(int type, unsigned long flags, ...);
> > diff --git a/include/log.h b/include/log.h
> > index 8fa7dc6..148359e 100644
> > --- a/include/log.h
> > +++ b/include/log.h
> > @@ -10,6 +10,7 @@ extern void log_closedir(void);
> >  
> >  extern void log_set_fd(int fd);
> >  extern int log_get_fd(void);
> > +extern int log_clone_fd(void);
> >  
> >  extern void log_set_loglevel(unsigned int loglevel);
> >  extern unsigned int log_get_loglevel(void);
> > diff --git a/log.c b/log.c
> > index 5fa1c6b..24c3ff5 100644
> > --- a/log.c
> > +++ b/log.c
> > @@ -55,7 +55,18 @@ static void print_ts(void)
> >  	buffer[TS_BUF_OFF - 1] = ' '; /* kill the '\0' produced by snprintf */
> >  }
> >  
> > +int log_clone_fd(void)
> > +{
> > +	current_logfd = dup2(current_logfd, get_service_fd(LOG_FD_OFF));
> > +	if (current_logfd < 0)
> > +		return -1;
> > +
> > +	logdir = dup2(logdir, get_service_fd(LOG_DIR_FD_OFF));
> > +	if (logdir < 0)
> > +		return -1;
> >  
> > +	return 0;
> > +}
> >  
> >  int log_get_fd(void)
> >  {
> > diff --git a/util.c b/util.c
> > index 59ed831..f4339de 100644
> > --- a/util.c
> > +++ b/util.c
> > @@ -33,6 +33,7 @@
> >  #include "types.h"
> >  #include "list.h"
> >  #include "util.h"
> > +#include "lock.h"
> >  
> >  #include "crtools.h"
> >  
> > @@ -188,6 +189,22 @@ int close_pid_proc(void)
> >  	return ret;
> >  }
> >  
> > +static int clone_proc(void)
> > +{
> > +	close_pid_proc();
> > +
> > +	if (proc_dir_fd <= 0)
> > +		return 0;
> > +
> > +	proc_dir_fd = dup2(proc_dir_fd, get_service_fd(PROC_FD_OFF));
> > +	if (proc_dir_fd < 0) {
> > +		pr_perror("Unable to duplicate a proc descriptor");
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  void close_proc()
> >  {
> >  	close_pid_proc();
> > @@ -280,6 +297,8 @@ int do_open_proc(pid_t pid, int flags, const char *fmt, ...)
> >  }
> >  
> >  static int service_fd_rlim_cur;
> > +static int service_fd_id = 0;
> > +static mutex_t *service_fd_lock;
> >  
> >  int init_service_fd(void)
> >  {
> > @@ -298,12 +317,65 @@ int init_service_fd(void)
> >  	service_fd_rlim_cur = (int)rlimit.rlim_cur;
> >  	BUG_ON(service_fd_rlim_cur < SERVICE_FD_MAX);
> >  
> > +	service_fd_lock = shmalloc(sizeof(mutex_t));
> > +	if (service_fd_lock == NULL)
> > +		return -1;
> > +
> > +	mutex_init(service_fd_lock);
> > +
> >  	return 0;
> >  }
> >  
> > +int clone_service_fd(bool to_init)
> > +{
> > +	int ret = -1;
> > +
> > +	if (to_init && service_fd_id == 0)
> > +		return 0;
> > +
> > +	mutex_lock(service_fd_lock);
> > +
> > +	if (to_init)
> > +		service_fd_id = 0;
> > +	else {
> > +		/*
> > +		 * Find a free service fd namespace. We suppose,
> > +		 * that LOG_FD_OFF should be opened all time, so
> > +		 * if LOG_FD_OFF isn't busy, a service fd namespace is free.
> > +		 */
> > +		while (1) {
> > +			service_fd_id++;
> > +
> > +			if (fcntl(get_service_fd(LOG_FD_OFF), F_GETFD) != -1)
> > +				continue;
> > +
> > +			if (errno == EBADF)
> > +				break;
> > +
> > +			pr_perror("fcntl failed\n");
> > +			goto out;
> > +		}
> > +	}
> > +
> > +	if (log_clone_fd())
> > +		goto out;
> > +
> > +	if (clone_proc())
> > +		goto out;
> > +
> > +	if (clone_image_dir())
> > +		goto out;
> > +
> > +	ret = 0;
> > +out:
> > +	mutex_unlock(service_fd_lock);
> > +
> > +	return ret;
> > +}
> > +
> >  static int __get_service_fd(enum sfd_type type)
> >  {
> > -	return service_fd_rlim_cur - type;
> > +	return service_fd_rlim_cur - type - SERVICE_FD_MAX * service_fd_id;
> >  }
> >  
> >  int get_service_fd(enum sfd_type type)
> > 
> 
> 


More information about the CRIU mailing list