[CRIU] [PATCH 3/9] mount: link dependent mounts

[Andrew Vagin]

On Tue, Jul 09, 2013 at 08:38:53PM +0400, Pavel Emelyanov wrote:
> On 07/09/2013 03:05 PM, Andrey Vagin wrote:
> > All shared mounts from one group are connected to circular list.
> > All slave are added into the proper master list.
> > 
> > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > ---
> >  include/proc_parse.h |  5 ++++
> >  mount.c              | 81 ++++++++++++++++++++++++++++++++--------------------
> >  2 files changed, 55 insertions(+), 31 deletions(-)
> > 
> > diff --git a/include/proc_parse.h b/include/proc_parse.h
> > index 8c2c36e..99255c4 100644
> > --- a/include/proc_parse.h
> > +++ b/include/proc_parse.h
> > @@ -116,6 +116,11 @@ struct mount_info {
> >  	struct mount_info *parent;
> >  	struct list_head children;
> >  	struct list_head siblings;
> > +
> > +	struct list_head mnt_share;	/* circular list of shared mounts */
> > +	struct list_head mnt_slave_list;/* list of slave mounts */
> > +	struct list_head mnt_slave;	/* slave list entry */
> > +	struct mount_info *mnt_master;	/* slave is on master->mnt_slave_list */
> >  };
> >  
> >  struct proc_posix_timer {
> > diff --git a/mount.c b/mount.c
> > index fbc9f7d..1971c26 100644
> > --- a/mount.c
> > +++ b/mount.c
> > @@ -178,6 +178,51 @@ static void mnt_tree_show(struct mount_info *tree, int off)
> >  	pr_info("%*s<--\n", off, "");
> >  }
> >  
> > +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.
> > +	 */
> > +	for (m = info; m; m = m->next) {
> > +		bool share, master;
> > +
> > +		share = m->shared_id && list_empty(&m->mnt_share);
> > +		master = m->master_id;
> 
> master is bool, master_id is int. What's going on here?
Probably I should rename this variables.

share says that we should to collect shared mappings (they can be more
than one, so we should enumirate all mounts).

master says that we should to find a master mount for this slave

> 
> > +
> > +		for (t = info; t && (share || master); t = t->next) {
> > +			if (t == m)
> > +				continue;
> > +			if (master && t->shared_id == m->master_id) {
> > +				pr_debug("The mount %d is slave for %d\n", m->mnt_id, t->mnt_id);
> > +				list_add(&m->mnt_slave, &t->mnt_slave_list);
> > +				m->mnt_master = t;
> > +				master = false;
> > +			}
> > +
> > +			/* Collect all mounts for this group */
> > +			if (share && t->shared_id == m->shared_id) {
> > +				pr_debug("Mount %d is shared with %d group %d\n",
> > +						m->mnt_id, t->mnt_id, m->shared_id);
> > +				list_add(&m->mnt_share, &t->mnt_share);
> 
> Shouldn't "share = false" be here? Otherwise we may add m to several mnt_share lists.
No, all shared mapping from m->shared_id collects into circular list
m->mnt_share

And yes, here is a bug, this line should be:
list_add(&t->mnt_share, &m->mnt_share);

Thanks
> 
> > +			}
> > +		}
> > +		if (!master)
> > +			continue;
> > +
>