fs: Add new argument to fstype::parse() and use it in binfmt_misc

Submitted by Kirill Tkhai on May 11, 2017, 4:27 p.m.

Details

Message ID 149452000664.27503.16060484841221881470.stgit@localhost.localdomain
State New
Series "fs: Add new argument to fstype::parse() and use it in binfmt_misc"
Headers show

Commit Message

Kirill Tkhai May 11, 2017, 4:27 p.m.
kdat.has_binfmt_misc has two meaning. On dump it means, that
there is a binfmt_misc partition during the dumped list, and
it's not need to mount temporary binfmt_misc partition if so.

On restore it means that there is a binfmt_misc partition
in mount image (and it's not need to add a tmp partition
if there is registered entries in binfmt-misc.img).

But parse is called on dump and restore both, so it sets
kdat.has_binfmt_misc = true, when there is a binfmt_misc
partition on host.

The patch solves this problem and adds for_dump argument
to parse method. Now we will set kdat.has_binfmt_misc = true,
only if parse is called on dump, and don't do that on restore.

https://travis-ci.org/tkhai/criu/builds/231219579

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 criu/autofs.c              |    2 +-
 criu/filesystems.c         |   25 +++++++++++++++++--------
 criu/include/autofs.h      |    2 +-
 criu/include/filesystems.h |    6 +++---
 criu/proc_parse.c          |    6 +++---
 5 files changed, 25 insertions(+), 16 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/autofs.c b/criu/autofs.c
index 15b0fc4b3..41687674e 100644
--- a/criu/autofs.c
+++ b/criu/autofs.c
@@ -58,7 +58,7 @@  static int autofs_gather_pipe(unsigned long inode)
 	return 0;
 }
 
-int autofs_parse(struct mount_info *pm)
+int autofs_parse(struct mount_info *pm, bool for_dump)
 {
 	long pipe_ino = AUTOFS_OPT_UNKNOWN;
 	char **opts;
diff --git a/criu/filesystems.c b/criu/filesystems.c
index 8211e396a..b1884db3f 100644
--- a/criu/filesystems.c
+++ b/criu/filesystems.c
@@ -40,7 +40,15 @@  struct binfmt_misc_info {
 
 LIST_HEAD(binfmt_misc_list);
 
-static int binfmt_misc_parse_or_collect(struct mount_info *pm)
+static int binfmt_misc_parse(struct mount_info *pm, bool for_dump)
+{
+	if (for_dump)
+		opts.has_binfmt_misc = true;
+	return 0;
+
+}
+
+static int binfmt_misc_collect(struct mount_info *pm)
 {
 	opts.has_binfmt_misc = true;
 	return 0;
@@ -376,7 +384,8 @@  int collect_binfmt_misc(void)
 #else
 #define binfmt_misc_dump	NULL
 #define binfmt_misc_restore	NULL
-#define binfmt_misc_parse_or_collect NULL
+#define binfmt_misc_parse NULL
+#define binfmt_misc_collect NULL
 #endif
 
 static int tmpfs_dump(struct mount_info *pm)
@@ -500,7 +509,7 @@  static int devtmpfs_restore(struct mount_info *pm)
 }
 
 /* Is it mounted w or w/o the newinstance option */
-static int devpts_parse(struct mount_info *pm)
+static int devpts_parse(struct mount_info *pm, bool for_dump)
 {
 	int ret;
 
@@ -559,7 +568,7 @@  static int fusectl_dump(struct mount_info *pm)
 	return ret;
 }
 
-static int debugfs_parse(struct mount_info *pm)
+static int debugfs_parse(struct mount_info *pm, bool for_dump)
 {
 	/* tracefs is automounted underneath debugfs sometimes, and the
 	 * kernel's overmounting protection prevents us from mounting debugfs
@@ -570,7 +579,7 @@  static int debugfs_parse(struct mount_info *pm)
 	return 0;
 }
 
-static int tracefs_parse(struct mount_info *pm)
+static int tracefs_parse(struct mount_info *pm, bool for_dump)
 {
 	return 1;
 }
@@ -586,7 +595,7 @@  static bool cgroup_sb_equal(struct mount_info *a, struct mount_info *b)
 	return true;
 }
 
-static int cgroup_parse(struct mount_info *pm)
+static int cgroup_parse(struct mount_info *pm, bool for_dump)
 {
 	if (!(root_ns_mask & CLONE_NEWCGROUP))
 		return 0;
@@ -685,8 +694,8 @@  static struct fstype fstypes[] = {
 		.restore = devtmpfs_restore,
 	}, {
 		.name = "binfmt_misc",
-		.parse = binfmt_misc_parse_or_collect,
-		.collect = binfmt_misc_parse_or_collect,
+		.parse = binfmt_misc_parse,
+		.collect = binfmt_misc_collect,
 		.code = FSTYPE__BINFMT_MISC,
 		.dump = binfmt_misc_dump,
 		.restore = binfmt_misc_restore,
diff --git a/criu/include/autofs.h b/criu/include/autofs.h
index d294277f6..f0f2b17bb 100644
--- a/criu/include/autofs.h
+++ b/criu/include/autofs.h
@@ -10,7 +10,7 @@ 
 bool is_autofs_pipe(unsigned long inode);
 
 struct mount_info;
-int autofs_parse(struct mount_info *pm);
+int autofs_parse(struct mount_info *pm, bool for_dump);
 int autofs_dump(struct mount_info *pm);
 int autofs_mount(struct mount_info *mi, const char *source, const
 		 char *filesystemtype, unsigned long mountflags);
diff --git a/criu/include/filesystems.h b/criu/include/filesystems.h
index bd798062d..51f4ab10b 100644
--- a/criu/include/filesystems.h
+++ b/criu/include/filesystems.h
@@ -14,7 +14,7 @@  struct fstype {
 	int (*dump)(struct mount_info *pm);
 	int (*restore)(struct mount_info *pm);
 	int (*check_bindmount)(struct mount_info *pm);
-	int (*parse)(struct mount_info *pm);
+	int (*parse)(struct mount_info *pm, bool for_dump);
 	int (*collect)(struct mount_info *pm);
 	bool (*sb_equal)(struct mount_info *a, struct mount_info *b);
 	mount_fn_t mount;
@@ -23,10 +23,10 @@  struct fstype {
 extern struct fstype *fstype_auto(void);
 
 /* callback for AUFS support */
-extern int aufs_parse(struct mount_info *mi);
+extern int aufs_parse(struct mount_info *mi, bool for_dump);
 
 /* callback for OverlayFS support */
-extern int overlayfs_parse(struct mount_info *mi);
+extern int overlayfs_parse(struct mount_info *mi, bool for_dump);
 
 /* FIXME -- remove */
 extern struct list_head binfmt_misc_list;
diff --git a/criu/proc_parse.c b/criu/proc_parse.c
index 041d45124..901016b00 100644
--- a/criu/proc_parse.c
+++ b/criu/proc_parse.c
@@ -1538,7 +1538,7 @@  struct mount_info *parse_mountinfo(pid_t pid, struct ns_id *nsid, bool for_dump)
 				new->flags, new->options);
 
 		if (new->fstype->parse) {
-			ret = new->fstype->parse(new);
+			ret = new->fstype->parse(new, for_dump);
 			if (ret < 0) {
 				pr_err("Failed to parse FS specific data on %s\n",
 						new->mountpoint);
@@ -2578,7 +2578,7 @@  int collect_controllers(struct list_head *cgroups, unsigned int *n_cgroups)
  *
  * See fixup_overlayfs for details.
  */
-int overlayfs_parse(struct mount_info *new)
+int overlayfs_parse(struct mount_info *new, bool for_dump)
 {
 	opts.overlayfs = true;
 	return 0;
@@ -2588,7 +2588,7 @@  int overlayfs_parse(struct mount_info *new)
  * AUFS callback function to "fix up" the root pathname.
  * See sysfs_parse.c for details.
  */
-int aufs_parse(struct mount_info *new)
+int aufs_parse(struct mount_info *new, bool for_dump)
 {
 	int ret = 0;
 

Comments

Kirill Tkhai May 11, 2017, 6:36 p.m.
On 11.05.2017 21:27, Patchwork wrote:
> == Series Details ==
> 
> Series: fs: Add new argument to fstype::parse() and use it in binfmt_misc
> URL   : https://patchwork.criu.org/series/1536/
> State : failure
> 
> == Logs ==
> 
> For more details see: https://travis-ci.org/criupatchwork/criu/builds/231243578?utm_source=github_status&utm_medium=notification
> 
It's not related to patch.
Andrey Vagin May 12, 2017, 11:54 p.m.
On Thu, May 11, 2017 at 07:27:00PM +0300, Kirill Tkhai wrote:
> kdat.has_binfmt_misc has two meaning. On dump it means, that
> there is a binfmt_misc partition during the dumped list, and
> it's not need to mount temporary binfmt_misc partition if so.
> 
> On restore it means that there is a binfmt_misc partition
> in mount image (and it's not need to add a tmp partition
> if there is registered entries in binfmt-misc.img).

I think it is a wrong using of kdat. kdat describes what features
the current system are supported. It is not about images.

Have you seen that now we save kdat (kerndat_save_cache())? I think it
doesn't work for this case.

> 
> But parse is called on dump and restore both, so it sets
> kdat.has_binfmt_misc = true, when there is a binfmt_misc
> partition on host.
> 
> The patch solves this problem and adds for_dump argument
> to parse method. Now we will set kdat.has_binfmt_misc = true,
> only if parse is called on dump, and don't do that on restore.
> 
> https://travis-ci.org/tkhai/criu/builds/231219579
> 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  criu/autofs.c              |    2 +-
>  criu/filesystems.c         |   25 +++++++++++++++++--------
>  criu/include/autofs.h      |    2 +-
>  criu/include/filesystems.h |    6 +++---
>  criu/proc_parse.c          |    6 +++---
>  5 files changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/criu/autofs.c b/criu/autofs.c
> index 15b0fc4b3..41687674e 100644
> --- a/criu/autofs.c
> +++ b/criu/autofs.c
> @@ -58,7 +58,7 @@ static int autofs_gather_pipe(unsigned long inode)
>  	return 0;
>  }
>  
> -int autofs_parse(struct mount_info *pm)
> +int autofs_parse(struct mount_info *pm, bool for_dump)
>  {
>  	long pipe_ino = AUTOFS_OPT_UNKNOWN;
>  	char **opts;
> diff --git a/criu/filesystems.c b/criu/filesystems.c
> index 8211e396a..b1884db3f 100644
> --- a/criu/filesystems.c
> +++ b/criu/filesystems.c
> @@ -40,7 +40,15 @@ struct binfmt_misc_info {
>  
>  LIST_HEAD(binfmt_misc_list);
>  
> -static int binfmt_misc_parse_or_collect(struct mount_info *pm)
> +static int binfmt_misc_parse(struct mount_info *pm, bool for_dump)
> +{
> +	if (for_dump)
> +		opts.has_binfmt_misc = true;
> +	return 0;
> +
> +}
> +
> +static int binfmt_misc_collect(struct mount_info *pm)
>  {
>  	opts.has_binfmt_misc = true;
>  	return 0;
> @@ -376,7 +384,8 @@ int collect_binfmt_misc(void)
>  #else
>  #define binfmt_misc_dump	NULL
>  #define binfmt_misc_restore	NULL
> -#define binfmt_misc_parse_or_collect NULL
> +#define binfmt_misc_parse NULL
> +#define binfmt_misc_collect NULL
>  #endif
>  
>  static int tmpfs_dump(struct mount_info *pm)
> @@ -500,7 +509,7 @@ static int devtmpfs_restore(struct mount_info *pm)
>  }
>  
>  /* Is it mounted w or w/o the newinstance option */
> -static int devpts_parse(struct mount_info *pm)
> +static int devpts_parse(struct mount_info *pm, bool for_dump)
>  {
>  	int ret;
>  
> @@ -559,7 +568,7 @@ static int fusectl_dump(struct mount_info *pm)
>  	return ret;
>  }
>  
> -static int debugfs_parse(struct mount_info *pm)
> +static int debugfs_parse(struct mount_info *pm, bool for_dump)
>  {
>  	/* tracefs is automounted underneath debugfs sometimes, and the
>  	 * kernel's overmounting protection prevents us from mounting debugfs
> @@ -570,7 +579,7 @@ static int debugfs_parse(struct mount_info *pm)
>  	return 0;
>  }
>  
> -static int tracefs_parse(struct mount_info *pm)
> +static int tracefs_parse(struct mount_info *pm, bool for_dump)
>  {
>  	return 1;
>  }
> @@ -586,7 +595,7 @@ static bool cgroup_sb_equal(struct mount_info *a, struct mount_info *b)
>  	return true;
>  }
>  
> -static int cgroup_parse(struct mount_info *pm)
> +static int cgroup_parse(struct mount_info *pm, bool for_dump)
>  {
>  	if (!(root_ns_mask & CLONE_NEWCGROUP))
>  		return 0;
> @@ -685,8 +694,8 @@ static struct fstype fstypes[] = {
>  		.restore = devtmpfs_restore,
>  	}, {
>  		.name = "binfmt_misc",
> -		.parse = binfmt_misc_parse_or_collect,
> -		.collect = binfmt_misc_parse_or_collect,
> +		.parse = binfmt_misc_parse,
> +		.collect = binfmt_misc_collect,
>  		.code = FSTYPE__BINFMT_MISC,
>  		.dump = binfmt_misc_dump,
>  		.restore = binfmt_misc_restore,
> diff --git a/criu/include/autofs.h b/criu/include/autofs.h
> index d294277f6..f0f2b17bb 100644
> --- a/criu/include/autofs.h
> +++ b/criu/include/autofs.h
> @@ -10,7 +10,7 @@
>  bool is_autofs_pipe(unsigned long inode);
>  
>  struct mount_info;
> -int autofs_parse(struct mount_info *pm);
> +int autofs_parse(struct mount_info *pm, bool for_dump);
>  int autofs_dump(struct mount_info *pm);
>  int autofs_mount(struct mount_info *mi, const char *source, const
>  		 char *filesystemtype, unsigned long mountflags);
> diff --git a/criu/include/filesystems.h b/criu/include/filesystems.h
> index bd798062d..51f4ab10b 100644
> --- a/criu/include/filesystems.h
> +++ b/criu/include/filesystems.h
> @@ -14,7 +14,7 @@ struct fstype {
>  	int (*dump)(struct mount_info *pm);
>  	int (*restore)(struct mount_info *pm);
>  	int (*check_bindmount)(struct mount_info *pm);
> -	int (*parse)(struct mount_info *pm);
> +	int (*parse)(struct mount_info *pm, bool for_dump);
>  	int (*collect)(struct mount_info *pm);
>  	bool (*sb_equal)(struct mount_info *a, struct mount_info *b);
>  	mount_fn_t mount;
> @@ -23,10 +23,10 @@ struct fstype {
>  extern struct fstype *fstype_auto(void);
>  
>  /* callback for AUFS support */
> -extern int aufs_parse(struct mount_info *mi);
> +extern int aufs_parse(struct mount_info *mi, bool for_dump);
>  
>  /* callback for OverlayFS support */
> -extern int overlayfs_parse(struct mount_info *mi);
> +extern int overlayfs_parse(struct mount_info *mi, bool for_dump);
>  
>  /* FIXME -- remove */
>  extern struct list_head binfmt_misc_list;
> diff --git a/criu/proc_parse.c b/criu/proc_parse.c
> index 041d45124..901016b00 100644
> --- a/criu/proc_parse.c
> +++ b/criu/proc_parse.c
> @@ -1538,7 +1538,7 @@ struct mount_info *parse_mountinfo(pid_t pid, struct ns_id *nsid, bool for_dump)
>  				new->flags, new->options);
>  
>  		if (new->fstype->parse) {
> -			ret = new->fstype->parse(new);
> +			ret = new->fstype->parse(new, for_dump);
>  			if (ret < 0) {
>  				pr_err("Failed to parse FS specific data on %s\n",
>  						new->mountpoint);
> @@ -2578,7 +2578,7 @@ int collect_controllers(struct list_head *cgroups, unsigned int *n_cgroups)
>   *
>   * See fixup_overlayfs for details.
>   */
> -int overlayfs_parse(struct mount_info *new)
> +int overlayfs_parse(struct mount_info *new, bool for_dump)
>  {
>  	opts.overlayfs = true;
>  	return 0;
> @@ -2588,7 +2588,7 @@ int overlayfs_parse(struct mount_info *new)
>   * AUFS callback function to "fix up" the root pathname.
>   * See sysfs_parse.c for details.
>   */
> -int aufs_parse(struct mount_info *new)
> +int aufs_parse(struct mount_info *new, bool for_dump)
>  {
>  	int ret = 0;
>  
>