[CRIU] [PATCH] fs: Add new argument to fstype::parse() and use it in binfmt_misc
Andrei Vagin
avagin at virtuozzo.com
Wed Jun 21 03:52:28 MSK 2017
On Mon, May 15, 2017 at 03:40:57PM +0300, Kirill Tkhai wrote:
> On 13.05.2017 02:54, Andrei Vagin wrote:
> > 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.
>
> It's not saved in kdat, but in opts. So, the description confuses user.
> Please, see fixed below:
>
> [PATCH]fs: Add new argument to fstype::parse() and use it in binfmt_misc
>
> opts.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
> opts.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 opts.has_binfmt_misc = true,
> only if parse is called on dump, and don't do that on restore.
>
> Signed-off-by: Kirill Tkhai <ktkhai at 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;
I don't like this patch. I think it shows that we do something wrong.
binfmt_misc is connected with ve_struct and opts.has_binfmt_misc = true
should be set if we found that we restore a vz container.
We should not try to write code for the upstream kernel, which doesn't
have a required functionality. If binfmt_misc will be virtuallized in
the upstream kernel, it will be attached to mntns, userns, or something
else and we will have to rework this code.
> + 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;
>
More information about the CRIU
mailing list