[CRIU] [PATCH 2/6] mnt: add --ext-mount-map auto option
Tycho Andersen
tycho.andersen at canonical.com
Thu Apr 9 08:26:12 PDT 2015
On Thu, Apr 09, 2015 at 01:02:01PM +0300, Pavel Emelyanov wrote:
> > @@ -641,7 +645,92 @@ static int validate_mounts(struct mount_info *info, bool for_dump)
> > return 0;
> > }
> >
> > -static int collect_shared(struct mount_info *info)
> > +static int resolve_external_mounts(struct mount_info *info)
> > +{
> > + struct mount_info *m;
> > +
> > + for (m = info; m; m = m->next) {
> > + struct mount_info *pm;
> > + bool found = false;
> > + struct ns_id *ns = NULL, *iter;
> > + int ret;
> > +
> > + if (m->parent == NULL || m->is_ns_root)
> > + continue;
>
> Before the patch we only resolved external for !fsroot_mounted() mi-s. Shouldn't
> the same check be here too?
Yes, good catch; I thought I had that in some version of the set, but
apparently not this one. I will make the change.
Tycho
> > + ret = try_resolve_ext_mount(m);
> > + if (ret < 0 && ret != -ENOTSUP) {
> > + return -1;
> > + } else if (ret == -ENOTSUP && !opts.autodetect_ext_mounts) {
> > + continue;
> > + } else if (ret == 0) {
> > + continue;
> > + }
> > +
> > + for (iter = ns_ids; iter->next; iter = iter->next) {
> > + if (iter->pid == getpid() && iter->nd == &mnt_ns_desc) {
> > + ns = iter;
> > + break;
> > + }
> > + }
> > +
> > + if (!ns) {
> > + pr_err("Failed to find criu pid's mount ns!");
> > + return -1;
> > + }
> > +
> > + for (pm = ns->mnt.mntinfo_list; pm; pm = pm->next) {
> > + int size, ret;
> > + char *p;
> > + bool match = false;
> > + struct ext_mount *em;
> > +
> > + if (mounts_equal(pm, m, true))
> > + match = true;
> > +
> > + if (!match)
> > + continue;
>
> The bool match seems to be excessive. The
>
> if (!mounts_equal())
> continue;
>
> would be enough :)
>
> > +
> > + 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;
> > + }
> > +
> > + em = xmalloc(sizeof(struct ext_mount));
> > + if (!em) {
> > + free(p);
> > + return -1;
> > + }
> > +
> > + em->val = "CRIU:AUTOGENERATED";
> > + em->key = p;
> > + m->external = em;
> > +
> > + pr_info("autodetected external mount %s for %s\n", p, m->mountpoint);
> > + xfree(m->source);
> > + m->source = p;
> > +
> > + found = true;
> > + break;
> > + }
> > +
> > + if (!found) {
> > + pr_info("couldn't find external bind mount for %d %s "
> > + "(master_id: %d shared_id: %d)\n",
> > + m->mnt_id, m->mountpoint, m->master_id, m->shared_id);
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int collect_shared(struct mount_info *info, bool for_dump)
> > {
> > struct mount_info *m, *t;
> >
> > @@ -675,10 +764,12 @@ 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);
> > + // If we haven't already determined this mount is external,
> > + // then we don't know where it came from.
> > + if (need_master && m->parent && !m->external) {
> > + pr_err("Mount %d %s (master_id: %d shared_id: %d) "
> > + "has unreachable sharing. Try --enable-external-masters.\n", m->mnt_id,
> > + m->mountpoint, m->master_id, m->shared_id);
> > return -1;
> > }
> >
> > @@ -1594,6 +1685,9 @@ static bool can_mount_now(struct mount_info *mi)
> > if (mi->is_ns_root)
> > return true;
> >
> > + if (mi->external)
> > + return true;
> > +
> > if (mi->master_id && mi->bind == NULL)
> > return false;
> >
> > @@ -1880,6 +1974,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);
> >
> > + pr_debug("\t\tGetting source for %d\n", pm->mnt_id);
> > + pm->source = xstrdup(me->source);
> > + if (!pm->source)
> > + goto err;
> > +
> > /* FIXME: abort unsupported early */
> > pm->fstype = decode_fstype(me->fstype);
> >
> > @@ -1893,8 +1992,29 @@ static int collect_mnt_from_image(struct mount_info **pms, struct ns_id *nsid)
> >
> > em = ext_mount_lookup(me->root);
> > if (!em) {
> > - pr_err("No mapping for %s mountpoint\n", me->mountpoint);
> > - goto err;\
> > + if (!opts.autodetect_ext_mounts) {
> > + pr_err("No mapping for %s mountpoint\n", me->mountpoint);
> > + goto err;
> > + }
> > +
> > + /*
> > + * Make up an external mount entry for this
> > + * mount point, since we couldn't find a user
> > + * supplied one.
> > + */
> > + em = xmalloc(sizeof(struct ext_mount));
> > + if (!em)
> > + goto err;
> > +
> > + em->val = pm->source;
> > +
> > + /*
> > + * Put a : in here since those are invalid on
> > + * the cli, so we know it's autogenerated in
> > + * debugging.
> > + */
> > + em->key = "CRIU:AUTOGENERATED";
> > + list_add_tail(&em->l, &opts.ext_mounts);
> > }
> >
> > pm->external = em;
> > @@ -1926,11 +2046,6 @@ static int collect_mnt_from_image(struct mount_info **pms, struct ns_id *nsid)
> >
> > pr_debug("\t\tGetting mpt for %d %s\n", pm->mnt_id, pm->mountpoint);
> >
> > - pr_debug("\t\tGetting source for %d\n", pm->mnt_id);
> > - pm->source = xstrdup(me->source);
> > - if (!pm->source)
> > - goto err;
> > -
> > pr_debug("\t\tGetting opts for %d\n", pm->mnt_id);
> > pm->options = xstrdup(me->options);
> > if (!pm->options)
> > @@ -2108,7 +2223,7 @@ static int populate_mnt_ns(struct mount_info *mis)
> > if (!pms)
> > return -1;
> >
> > - if (collect_shared(mis))
> > + if (collect_shared(mis, false))
> > return -1;
> >
> > for (nsid = ns_ids; nsid; nsid = nsid->next) {
> > @@ -2346,6 +2461,7 @@ int mntns_get_root_by_mnt_id(int mnt_id)
> > struct collect_mntns_arg {
> > bool need_to_validate;
> > bool for_dump;
> > + bool really_collect_self_mounts;
> > };
> >
> > static int collect_mntns(struct ns_id *ns, void *__arg)
> > @@ -2360,7 +2476,8 @@ static int collect_mntns(struct ns_id *ns, void *__arg)
> > if (arg->for_dump && ns->pid != getpid())
> > arg->need_to_validate = true;
> >
> > - mntinfo_add_list(pms);
> > + if (!opts.autodetect_ext_mounts || ns->pid != getpid() || arg->really_collect_self_mounts)
> > + mntinfo_add_list(pms);
>
> This if and the if for really_collect_self_mounts seems strange to me.
>
> There are two "types" of pms we can get here -- criu's one and non-criu's
> one. The former one should only be put into the global list if it's the only
> one at all, i.e. -- we're having the non-ns case.
>
> So the single if (ns->pid != getpid() || !(root_ns_mask & CLONE_NEWNS)) here
> should give us what we want.
>
> > return 0;
> > }
> >
> > @@ -2371,15 +2488,34 @@ int collect_mnt_namespaces(bool for_dump)
> >
> > arg.for_dump = for_dump;
> > arg.need_to_validate = false;
> > + arg.really_collect_self_mounts = false;
> >
> > - ret = walk_namespaces(&mnt_ns_desc, false, collect_mntns, &need_to_validate);
> > + /*
> > + * XXX: this is a little bit of spaghetti. If we are autodetecting
> > + * external mounts, we want to collect the criu pid's mountinfo (and
> > + * thus the whole mountns) so we can corrolate the external masters.
> > + * So we clear the mountns bit for this call to walk_namespaces.
> > + *
> > + * However, we don't want to add criu's mountns to the dump target
> > + * list if it isn't really going to be dumped, so we set the
> > + * really_collect_self_mounts flag if we really do want to collect
> > + * them above.
> > + */
> > + if (opts.autodetect_ext_mounts && !(root_ns_mask & CLONE_NEWNS)) {
> > + arg.really_collect_self_mounts = true;
> > + }
>
> If I'm correct with the above only the above if can be removed.
>
> > +
> > + ret = walk_namespaces(&mnt_ns_desc, opts.autodetect_ext_mounts, collect_mntns, &arg);
> > if (ret)
> > goto err;
> >
> > + if (resolve_external_mounts(mntinfo))
> > + goto err;
> > +
> > if (arg.need_to_validate) {
> > ret = -1;
> >
> > - if (collect_shared(mntinfo))
> > + if (collect_shared(mntinfo, true))
> > goto err;
> > if (validate_mounts(mntinfo, true))
> > goto err;
> >
>
More information about the CRIU
mailing list