[CRIU] [PATCH] mnt: add support for external shared mounts
Tycho Andersen
tycho.andersen at canonical.com
Fri Apr 3 09:20:17 PDT 2015
Hi Andrey,
On Fri, Apr 03, 2015 at 07:00:43PM +0300, Andrew Vagin wrote:
> On Thu, Apr 02, 2015 at 08:22:33PM +0000, Tycho Andersen wrote:
> > Add support for external shared mounts via the --enable-external-masters flag.
> > This flag assumes that the mount is visible to criu via /proc/self/mountinfo
> > and that it will be present at the same location on restore. No attempt to dump
> > or restore the content is made; the external master is bind mounted into the
> > mount ns and whatever is there is what the container sees.
>
> I need time to think about the idea. At the first glance it should work.
>
> But we (I and Pavel) are not sure that we understand what you do:).
>
> Could you show /proc/PID/mountinfo for a container? Why can you not use
> external mounts?
Because criu dies with "couldn't find sharing" before the external
mount map is consulted. I could use ext-mount-map, but we'd still need
some patch to skip this check, and since we can resolve these
automatically anyway it seems like we should just do that instead.
(This is the first issue I mentioned in the "Dealing with other mount
types" thread a week ago.)
I'll make the rest of your changes, they look good.
Tycho
> A few comments are inline.
>
> Thanks.
>
> >
> > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > ---
> > crtools.c | 6 +++
> > include/cr_options.h | 1 +
> > include/proc_parse.h | 1 +
> > mount.c | 115 +++++++++++++++++++++++++++++++++++++++++++++------
> > protobuf/mnt.proto | 2 +
> > 5 files changed, 112 insertions(+), 13 deletions(-)
> >
> > diff --git a/crtools.c b/crtools.c
> > index 0b3c497..7986beb 100644
> > --- a/crtools.c
> > +++ b/crtools.c
> > @@ -203,6 +203,7 @@ int main(int argc, char *argv[], char *envp[])
> > { "cgroup-root", required_argument, 0, 1061},
> > { "inherit-fd", required_argument, 0, 1062},
> > { "feature", required_argument, 0, 1063},
> > + { "enable-external-masters", no_argument, 0, 1064},
> > { },
> > };
> >
> > @@ -416,6 +417,9 @@ int main(int argc, char *argv[], char *envp[])
> > if (check_add_feature(optarg) < 0)
> > return 1;
> > break;
> > + case 1064:
> > + opts.enable_external_masters = true;
> > + break;
> > case 'M':
> > {
> > char *aux;
> > @@ -637,6 +641,8 @@ usage:
> > " --force-irmap force resolving names for inotify/fsnotify watches\n"
> > " -M|--ext-mount-map KEY:VALUE\n"
> > " add external mount mapping\n"
> > +" --enable-external-masters\n"
> > +" Allow mounts with external masters\n"
>
> Could you add a full description for this option into
> Documentation/criu.txt
>
> > " --manage-cgroups dump or restore cgroups the process is in\n"
> > " --cgroup-root [controller:]/newroot\n"
> > " change the root cgroup the controller will be\n"
> > diff --git a/include/cr_options.h b/include/cr_options.h
> > index d6ce2ae..846f03c 100644
> > --- a/include/cr_options.h
> > +++ b/include/cr_options.h
> > @@ -59,6 +59,7 @@ struct cr_options {
> > bool manage_cgroups;
> > char *new_global_cg_root;
> > struct list_head new_cgroup_roots;
> > + bool enable_external_masters;
> > bool aufs; /* auto-deteced, not via cli */
> > };
> >
> > diff --git a/include/proc_parse.h b/include/proc_parse.h
> > index 792cf06..46ae393 100644
> > --- a/include/proc_parse.h
> > +++ b/include/proc_parse.h
> > @@ -130,6 +130,7 @@ struct mount_info {
> > struct ns_id *nsid;
> >
> > struct ext_mount *external;
> > + bool external_master;
> >
> > /* tree linkage */
> > struct mount_info *parent;
> > diff --git a/mount.c b/mount.c
> > index 021c6e3..cbdad87 100644
> > --- a/mount.c
> > +++ b/mount.c
> > @@ -566,6 +566,9 @@ static struct mount_info *find_fsroot_mount_for(struct mount_info *bm)
> > return NULL;
> > }
> >
> > +struct mount_info *external_mounts = NULL;
> > +static int try_resolve_external_sharing(struct mount_info *m);
> > +
> > static int validate_mounts(struct mount_info *info, bool for_dump)
> > {
> > struct mount_info *m, *t;
> > @@ -578,6 +581,11 @@ static int validate_mounts(struct mount_info *info, bool for_dump)
> > if (m->shared_id && validate_shared(m))
> > return -1;
> >
> > + if (m->master_id && opts.enable_external_masters) {
> > + if (try_resolve_external_sharing(m) == 0)
> > + continue;
> > + }
> > +
> > /*
> > * Mountpoint can point to / of an FS. In that case this FS
> > * should be of some known type so that we can just mount one.
> > @@ -646,10 +654,11 @@ static int collect_shared(struct mount_info *info)
> > struct mount_info *m, *t;
> >
> > /*
> > - * If we have a shared mounts, both master
> > - * slave targets are to be present in mount
> > - * list, otherwise we can't be sure if we can
> > - * recreate the scheme later on restore.
> > + * If we have a shared mounts, both master slave targets are to be
> > + * present in mount list, otherwise we can't be sure if we can
> > + * recreate the scheme later on restore. If --enable-external-masters
> > + * is supplied, we assume that the scheme will be present on restore
> > + * and allow the mounts to be dumped.
> > */
> > for (m = info; m; m = m->next) {
> > bool need_share, need_master;
> > @@ -676,10 +685,28 @@ static int collect_shared(struct mount_info *info)
> > }
> >
> > if (need_master && m->parent) {
> > - pr_err("Mount %d (master_id: %d shared_id: %d) "
> > - "has unreachable sharing\n", m->mnt_id,
> > - m->master_id, m->shared_id);
> > - return -1;
> > + if (opts.enable_external_masters) {
> > + struct mount_info *pm;
> > + bool found = false;
> > +
> > + for (pm = external_mounts; pm->next != NULL; pm = pm->next) {
> > + if (pm->shared_id == m->master_id) {
> > + found = true;
> > + break;
> > + }
> > + }
>
> if (collect_shared(mntinfo))
> goto err;
> if (validate_mounts(mntinfo, true))
> goto err;
>
> try_resolve_external_sharing() is called from validate_mounts(), so we
> enumerate external mounts twice. Can we do this only once?
>
> > +
> > + if (!found) {
> > + pr_err("couldn't find sharing for %d "
> > + "(master_id: %d shared_id: %d)",
> > + m->mnt_id, m->master_id, m->shared_id);
> > + }
> > + } else {
> > + pr_err("Mount %d (master_id: %d shared_id: %d) "
> > + "has unreachable sharing. Try --enable-external-masters.\n", m->mnt_id,
> > + m->master_id, m->shared_id);
> > + return -1;
> > + }
> > }
> >
> > /* Search bind-mounts */
> > @@ -1159,6 +1186,12 @@ static int dump_one_mountpoint(struct mount_info *pm, struct cr_img *img)
> > me.has_shared_id = true;
> > me.master_id = pm->master_id;
> > me.has_master_id = true;
> > +
> > + if (pm->external_master) {
> > + me.external_master = true;
> > + me.has_external_master = true;
> > + }
> > +
> > if (pm->need_plugin) {
> > me.has_with_plugin = true;
> > me.with_plugin = true;
> > @@ -1302,7 +1335,7 @@ again:
> > if (!progress) {
> > struct mount_info *m;
> >
> > - pr_err("A few mount points can't be mounted");
> > + pr_err("A few mount points can't be mounted\n");
> > list_for_each_entry(m, &postpone2, postpone) {
> > pr_err("%d:%d %s %s %s\n", m->mnt_id,
> > m->parent_mnt_id, m->root,
> > @@ -1475,17 +1508,19 @@ static int do_new_mount(struct mount_info *mi)
> > return -1;
> >
> > if (mount(src, mi->mountpoint, tp->name,
> > - mi->flags & (~MS_SHARED), mi->options) < 0) {
> > + mi->flags & ~(MS_SLAVE | MS_SHARED), mi->options) < 0) {
>
> Maybe it's better to mount such mounts in do_bind_mount?
>
> > pr_perror("Can't mount at %s", mi->mountpoint);
> > return -1;
> > }
> >
> > - if (restore_shared_options(mi, 0, mi->shared_id, 0))
> > + if (restore_shared_options(mi, 0, mi->shared_id, mi->flags & MS_SLAVE || mi->external_master))
> > return -1;
> >
> > mi->mounted = true;
> >
> > - if (tp->restore && tp->restore(mi))
> > + // If there is an external master, the content of the mount comes from
> > + // there; otherwise, we restore it as usual.
> > + if (!mi->external_master && tp->restore && tp->restore(mi))
> > return -1;
> >
> > return 0;
> > @@ -1617,7 +1652,7 @@ static int do_mount_one(struct mount_info *mi)
> > if (mi->mounted)
> > return 0;
> >
> > - if (!can_mount_now(mi)) {
> > + if (!mi->external_master && !can_mount_now(mi)) {
> > pr_debug("Postpone slave %s\n", mi->mountpoint);
> > return 1;
> > }
> > @@ -1858,6 +1893,11 @@ static int collect_mnt_from_image(struct mount_info **pms, struct ns_id *nsid)
> > pm->need_plugin = me->with_plugin;
> > pm->is_ns_root = is_root(me->mountpoint);
> >
> > + if (me->has_external_master)
> > + pm->external_master = pm->external_master;
> > + else
> > + pm->external_master = false;
> > +
> > /* FIXME: abort unsupported early */
> > pm->fstype = decode_fstype(me->fstype);
> >
> > @@ -2065,6 +2105,8 @@ static int prepare_roots_yard(void)
> > return 0;
> > }
> >
> > +static int collect_external_mounts();
> > +
> > static int populate_mnt_ns(struct mount_info *mis)
> > {
> > struct mount_info *pms;
> > @@ -2077,6 +2119,9 @@ static int populate_mnt_ns(struct mount_info *mis)
> > if (!pms)
> > return -1;
> >
> > + if (collect_external_mounts() < 0)
> > + return -1;
> > +
> > if (collect_shared(mis))
> > return -1;
> >
> > @@ -2312,6 +2357,48 @@ int mntns_get_root_by_mnt_id(int mnt_id)
> > return mntns_get_root_fd(mntns);
> > }
> >
> > +// We collect the mounts in /proc/self in order to find the master node for
> > +// shared mounts.
>
> We usually use this style for comments:
> /*
> *
> */
>
> > +static int collect_external_mounts()
> > +{
> > + external_mounts = parse_mountinfo(getpid(), NULL);
> > + if (!external_mounts)
> > + return -1;
> > +
> > + return 0;
> > +}
> > +
> > +static int try_resolve_external_sharing(struct mount_info *m)
> > +{
> > + struct mount_info *pm;
> > +
> > + for (pm = external_mounts; pm->next != NULL; pm = pm->next) {
> > + if (pm->shared_id == m->master_id) {
>
> if (pm->shared_id != m->master_id)
> continue
>
> > + int size, ret;
> > + char *p;
> > +
> > + size = strlen(pm->mountpoint + 1) + strlen(m->root) + 1;
> > + p = xmalloc(sizeof(char) * size);
> > + if (!p)
> > + return -1;
> > +
> > + ret = snprintf(p, size+1, "%s%s", pm->mountpoint + 1, m->root);
> > + if (ret < 0 || ret >= size) {
> > + free(p);
> > + return -1;
> > + }
> > +
> > + xfree(m->source);
> > + m->source = p;
> > + m->external_master = true;
> > + m->flags |= MS_BIND;
> > + return 0;
> > + }
> > + }
> > +
> > + return -1;
> > +}
> > +
> > static int collect_mntns(struct ns_id *ns, void *oarg)
> > {
> > struct mount_info *pms;
> > @@ -2338,6 +2425,8 @@ int collect_mnt_namespaces(bool for_dump)
> > if (for_dump && need_to_validate) {
> > ret = -1;
> >
> > + if (collect_external_mounts())
> > + goto err;
> > if (collect_shared(mntinfo))
> > goto err;
> > if (validate_mounts(mntinfo, true))
> > diff --git a/protobuf/mnt.proto b/protobuf/mnt.proto
> > index 343bd6d..6f52010 100644
> > --- a/protobuf/mnt.proto
> > +++ b/protobuf/mnt.proto
> > @@ -35,4 +35,6 @@ message mnt_entry {
> >
> > optional bool with_plugin = 12;
> > optional bool ext_mount = 13;
> > +
> > + optional bool external_master = 14;
> > }
> > --
> > 2.1.4
> >
> > _______________________________________________
> > CRIU mailing list
> > CRIU at openvz.org
> > https://lists.openvz.org/mailman/listinfo/criu
More information about the CRIU
mailing list