[Devel] [PATCH rh7] ve/fs/sync: fix CT's mountpoints traversal

[Vladimir Davydov]

On Wed, Feb 24, 2016 at 06:36:22PM +0300, Andrey Ryabinin wrote:
> Currently sync reads 've->root_path.mnt' mount and iterate over it childs.
> This doesn't work, as not all in-container mounts could be reached that way.
> 
> This patch slightly rework's mounts traversal. Now sync just iterates over all
> mounts of current mount namespace (actually we iterate over all mounts that
> seen in container's /proc/mounts). This approach seems much more correct and
> also simpler.
> 
> I think we could get rid of 've->root_path' now.

It's still used in follow_dotdot to prevent chroot escape.

> 
> https://jira.sw.ru/browse/PSBM-44125
> 
> Signed-off-by: Andrey Ryabinin <aryabinin at virtuozzo.com>
...
> diff --git a/fs/sync.c b/fs/sync.c
> index 8119ab4..9242a60 100644
> --- a/fs/sync.c
> +++ b/fs/sync.c
> @@ -127,17 +127,17 @@ static int sync_filesystem_collected(struct list_head *sync_list, struct super_b
>  	return 0;
>  }
>  
> -static int sync_collect_filesystems(struct ve_struct *ve, struct list_head *sync_list)
> +static int sync_collect_filesystems(struct list_head *sync_list)
>  {
> -	struct mount *root = real_mount(ve->root_path.mnt);
>  	struct mount *mnt;
> +	struct mnt_namespace *mnt_ns = current->nsproxy->mnt_ns;

Will it work as expected if there are mount namespaces inside a
container?

>  	struct sync_sb *ss;
>  	int ret = 0;
>  
>  	BUG_ON(!list_empty(sync_list));
>  
>  	down_read(&namespace_sem);
> -	for (mnt = root; mnt; mnt = next_mnt(mnt, root)) {
> +	list_for_each_entry(mnt, &mnt_ns->list, mnt_list) {
>  		if (sync_filesystem_collected(sync_list, mnt->mnt.mnt_sb))
>  			continue;
>  
> @@ -161,20 +161,18 @@ static int sync_collect_filesystems(struct ve_struct *ve, struct list_head *sync
>  	return ret;
>  }
>  
> -static void sync_filesystems_ve(struct ve_struct *ve, struct user_beancounter *ub, int wait)
> +static void sync_filesystems_ub(struct user_beancounter *ub, int wait)
>  {
>  	struct super_block *sb;
>  	LIST_HEAD(sync_list);
>  	struct sync_sb *ss;
>  
> -	mutex_lock(&ve->sync_mutex);		/* Could be down_interruptible */
> -

This mutex isn't here to protect ve->root_path. I don't know why we need
it or whether we need it at all, but anyway this should go in a separate
patch with a proper comment.