image: do_open_remote_image: Move the unix sockets to workdir

Submitted by Omri Kramer on July 17, 2017, 6:49 p.m.

Details

Message ID 1500317353-2846-1-git-send-email-omri.kramer@gmail.com
State New
Series "image: do_open_remote_image: Move the unix sockets to workdir"
Headers show

Commit Message

Omri Kramer July 17, 2017, 6:49 p.m.
It's better to open the unix sockets for dump and restore
inside the workdir. See TODO item 1 in:
https://github.com/xemul/criu/commit/037b2e3c95e098d63a3500834681543da544840e

Signed-off-by: Omri Kramer <omri.kramer@gmail.com>
Signed-off-by: Lior Fisch <fischlior@gmail.com>
---
 criu/image.c             | 32 +++++++++++++++++++++++++++++++-
 criu/include/servicefd.h |  1 +
 2 files changed, 32 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/criu/image.c b/criu/image.c
index 62e8e71..7c21178 100644
--- a/criu/image.c
+++ b/criu/image.c
@@ -338,7 +338,7 @@  int do_open_remote_image(int dfd, char *path, int flags)
 		return -1;
 	}
 
-	if (fchdir(get_service_fd(IMG_FD_OFF)) < 0) {
+	if (fchdir(get_service_fd(WRK_FD_OFF)) < 0) {
 		pr_perror("fchdir to dfd failed!");
 		close(save);
 		return -1;
@@ -465,6 +465,32 @@  struct cr_img *img_from_fd(int fd)
 	return img;
 }
 
+static void close_work_dir(void)
+{
+	close_service_fd(WRK_FD_OFF);
+}
+
+static int open_work_dir(char *dir)
+{
+	int fd;
+
+	fd = open(dir, O_RDONLY);
+	if (fd < 0) {
+		pr_perror("Can't open dir %s", dir);
+		return -1;
+	}
+
+	if (install_service_fd(WRK_FD_OFF, fd) < 0) {
+		pr_perror("Can't install work_fd\n");
+		close(fd);
+		return -1;
+	}
+
+	close(fd);
+	
+	return 0;
+}
+
 int open_image_dir(char *dir)
 {
 	int fd, ret;
@@ -481,6 +507,8 @@  int open_image_dir(char *dir)
 
 	if (opts.remote) {
 		init_snapshot_id(dir);
+		if (open_work_dir(opts.work_dir) < 0)
+			goto err;
 	} else if (opts.img_parent) {
 		ret = symlinkat(opts.img_parent, fd, CR_PARENT_LINK);
 		if (ret < 0 && errno != EEXIST) {
@@ -503,6 +531,8 @@  err:
 void close_image_dir(void)
 {
 	close_service_fd(IMG_FD_OFF);
+	if(opts.remote)
+		close_work_dir();
 }
 
 static unsigned long page_ids = 1;
diff --git a/criu/include/servicefd.h b/criu/include/servicefd.h
index b779223..66e4aeb 100644
--- a/criu/include/servicefd.h
+++ b/criu/include/servicefd.h
@@ -8,6 +8,7 @@  enum sfd_type {
 
 	LOG_FD_OFF,
 	IMG_FD_OFF,
+	WRK_FD_OFF,
 	PROC_FD_OFF,	/* fd with /proc for all proc_ calls */
 	PROC_PID_FD_OFF,
 	CTL_TTY_OFF,

Comments

Pavel Emelyanov July 18, 2017, 6:08 a.m.
On 07/17/2017 09:49 PM, Omri Kramer wrote:
> It's better to open the unix sockets for dump and restore
> inside the workdir. See TODO item 1 in:
> https://github.com/xemul/criu/commit/037b2e3c95e098d63a3500834681543da544840e
> 
> Signed-off-by: Omri Kramer <omri.kramer@gmail.com>
> Signed-off-by: Lior Fisch <fischlior@gmail.com>
> ---
>  criu/image.c             | 32 +++++++++++++++++++++++++++++++-
>  criu/include/servicefd.h |  1 +
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/criu/image.c b/criu/image.c
> index 62e8e71..7c21178 100644
> --- a/criu/image.c
> +++ b/criu/image.c
> @@ -338,7 +338,7 @@ int do_open_remote_image(int dfd, char *path, int flags)
>  		return -1;
>  	}
>  
> -	if (fchdir(get_service_fd(IMG_FD_OFF)) < 0) {
> +	if (fchdir(get_service_fd(WRK_FD_OFF)) < 0) {
>  		pr_perror("fchdir to dfd failed!");
>  		close(save);
>  		return -1;
> @@ -465,6 +465,32 @@ struct cr_img *img_from_fd(int fd)
>  	return img;
>  }
>  
> +static void close_work_dir(void)
> +{
> +	close_service_fd(WRK_FD_OFF);
> +}
> +
> +static int open_work_dir(char *dir)
> +{
> +	int fd;
> +
> +	fd = open(dir, O_RDONLY);
> +	if (fd < 0) {
> +		pr_perror("Can't open dir %s", dir);
> +		return -1;
> +	}
> +
> +	if (install_service_fd(WRK_FD_OFF, fd) < 0) {
> +		pr_perror("Can't install work_fd\n");
> +		close(fd);
> +		return -1;
> +	}

Right now all the code in criu assumes, that workdir is cwd. Why do we
need to introduce a service descriptor for that?

> +
> +	close(fd);
> +	
> +	return 0;
> +}
> +
>  int open_image_dir(char *dir)
>  {
>  	int fd, ret;
> @@ -481,6 +507,8 @@ int open_image_dir(char *dir)
>  
>  	if (opts.remote) {
>  		init_snapshot_id(dir);
> +		if (open_work_dir(opts.work_dir) < 0)
> +			goto err;
>  	} else if (opts.img_parent) {
>  		ret = symlinkat(opts.img_parent, fd, CR_PARENT_LINK);
>  		if (ret < 0 && errno != EEXIST) {
> @@ -503,6 +531,8 @@ err:
>  void close_image_dir(void)
>  {
>  	close_service_fd(IMG_FD_OFF);
> +	if(opts.remote)
> +		close_work_dir();
>  }
>  
>  static unsigned long page_ids = 1;
> diff --git a/criu/include/servicefd.h b/criu/include/servicefd.h
> index b779223..66e4aeb 100644
> --- a/criu/include/servicefd.h
> +++ b/criu/include/servicefd.h
> @@ -8,6 +8,7 @@ enum sfd_type {
>  
>  	LOG_FD_OFF,
>  	IMG_FD_OFF,
> +	WRK_FD_OFF,
>  	PROC_FD_OFF,	/* fd with /proc for all proc_ calls */
>  	PROC_PID_FD_OFF,
>  	CTL_TTY_OFF,
>
Mike Rapoport July 20, 2017, 7:19 a.m.
On Tue, Jul 18, 2017 at 09:08:01AM +0300, Pavel Emelyanov wrote:
> On 07/17/2017 09:49 PM, Omri Kramer wrote:
> > It's better to open the unix sockets for dump and restore
> > inside the workdir. See TODO item 1 in:
> > https://github.com/xemul/criu/commit/037b2e3c95e098d63a3500834681543da544840e
> > 
> > Signed-off-by: Omri Kramer <omri.kramer@gmail.com>
> > Signed-off-by: Lior Fisch <fischlior@gmail.com>
> > ---
> >  criu/image.c             | 32 +++++++++++++++++++++++++++++++-
> >  criu/include/servicefd.h |  1 +
> >  2 files changed, 32 insertions(+), 1 deletion(-)
> > 
> > diff --git a/criu/image.c b/criu/image.c
> > index 62e8e71..7c21178 100644
> > --- a/criu/image.c
> > +++ b/criu/image.c
> > @@ -338,7 +338,7 @@ int do_open_remote_image(int dfd, char *path, int flags)
> >  		return -1;
> >  	}
> >  
> > -	if (fchdir(get_service_fd(IMG_FD_OFF)) < 0) {
> > +	if (fchdir(get_service_fd(WRK_FD_OFF)) < 0) {
> >  		pr_perror("fchdir to dfd failed!");
> >  		close(save);
> >  		return -1;
> > @@ -465,6 +465,32 @@ struct cr_img *img_from_fd(int fd)
> >  	return img;
> >  }
> >  
> > +static void close_work_dir(void)
> > +{
> > +	close_service_fd(WRK_FD_OFF);
> > +}
> > +
> > +static int open_work_dir(char *dir)
> > +{
> > +	int fd;
> > +
> > +	fd = open(dir, O_RDONLY);
> > +	if (fd < 0) {
> > +		pr_perror("Can't open dir %s", dir);
> > +		return -1;
> > +	}
> > +
> > +	if (install_service_fd(WRK_FD_OFF, fd) < 0) {
> > +		pr_perror("Can't install work_fd\n");
> > +		close(fd);
> > +		return -1;
> > +	}
> 
> Right now all the code in criu assumes, that workdir is cwd. Why do we
> need to introduce a service descriptor for that?

I haven't really dig into this, but I believe it's related to re-mounting
of the directories and namespace changing.
The img-{proxy,cache} are started before all the mount/namespace changes
and the UNIX socket they create in the default namespace is not visible
afterwards.
 
> > +
> > +	close(fd);
> > +	
> > +	return 0;
> > +}
> > +
> >  int open_image_dir(char *dir)
> >  {
> >  	int fd, ret;
> > @@ -481,6 +507,8 @@ int open_image_dir(char *dir)
> >  
> >  	if (opts.remote) {
> >  		init_snapshot_id(dir);
> > +		if (open_work_dir(opts.work_dir) < 0)
> > +			goto err;
> >  	} else if (opts.img_parent) {
> >  		ret = symlinkat(opts.img_parent, fd, CR_PARENT_LINK);
> >  		if (ret < 0 && errno != EEXIST) {
> > @@ -503,6 +531,8 @@ err:
> >  void close_image_dir(void)
> >  {
> >  	close_service_fd(IMG_FD_OFF);
> > +	if(opts.remote)
> > +		close_work_dir();
> >  }
> >  
> >  static unsigned long page_ids = 1;
> > diff --git a/criu/include/servicefd.h b/criu/include/servicefd.h
> > index b779223..66e4aeb 100644
> > --- a/criu/include/servicefd.h
> > +++ b/criu/include/servicefd.h
> > @@ -8,6 +8,7 @@ enum sfd_type {
> >  
> >  	LOG_FD_OFF,
> >  	IMG_FD_OFF,
> > +	WRK_FD_OFF,
> >  	PROC_FD_OFF,	/* fd with /proc for all proc_ calls */
> >  	PROC_PID_FD_OFF,
> >  	CTL_TTY_OFF,
> > 
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
>