[CRIU] [PATCH 1/2] [v4] mount: dump a file system only if a mount point isn't overmounted

[Pavel Emelyanov]

On 05/12/2016 07:59 PM, Andrew Vagin wrote:
> On Thu, May 12, 2016 at 05:44:09PM +0300, Pavel Emelyanov wrote:
>> On 05/12/2016 08:52 AM, Andrey Vagin wrote:
>>> From: Andrew Vagin <avagin at virtuozzo.com>
>>>
>>> Something else may be mounted into the same folder and in this case
>>> we can't get access to the required file system.
>>>
>>> $ cat /proc/61693/root/etc/redhat-release
>>> Fedora release 23 (Twenty Three)
>>>
>>> $ cat /proc/61692/mountinfo  | grep '\s/tmp'
>>> 234 199 0:57 / /tmp rw shared:97 master:76 - tmpfs tmpfs rw,size=131072k,nr_inodes=32768
>>> 235 234 0:57 /systemd-private-dd74de99e1104383aa7cd6e27d3d0b8a-httpd.service-uFqNHk/tmp /tmp rw,relatime shared:98 master:76 - tmpfs tmpfs rw,size=131072k,nr_inodes=32768
>>>
>>> v2: return an error if we can't dump a file system
>>> v3: try to find a mount point which allows to dump a file system
>>> v4: check that children are not overmounted a target mount instead
>>> of getting a mnt_id for a file descriptor.
>>>
>>> Signed-off-by: Andrew Vagin <avagin at virtuozzo.com>
>>> ---
>>>  criu/mount.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++-------------
>>>  1 file changed, 48 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/criu/mount.c b/criu/mount.c
>>> index e08f46e..83692a8 100644
>>> --- a/criu/mount.c
>>> +++ b/criu/mount.c
>>> @@ -1155,6 +1155,7 @@ static char *get_clean_mnt(struct mount_info *mi, char *mnt_path_tmp, char *mnt_
>>>  
>>>  static int open_mountpoint(struct mount_info *pm)
>>>  {
>>> +	struct mount_info *c;
>>>  	int fd = -1, ns_old = -1;
>>>  	char mnt_path_tmp[] = "/tmp/cr-tmpfs.XXXXXX";
>>>  	char mnt_path_root[] = "/cr-tmpfs.XXXXXX";
>>> @@ -1170,6 +1171,13 @@ static int open_mountpoint(struct mount_info *pm)
>>>  
>>>  	pr_info("Something is mounted on top of %s\n", pm->mountpoint);
>>>  
>>> +	list_for_each_entry(c, &pm->children, siblings) {
>>> +		if (!strcmp(c->mountpoint, pm->mountpoint)) {
>>
>> A-ha! So we only don't open this thing if there's some children
>> mounted at the same path. Don't we have c->fd for such beasts?
>> From your previous set.
> 
> We c->fd on restore, but this patch is about dump
> 
>>
>>> +			pr_err("%d:%s is overmounted\n", pm->mnt_id, pm->mountpoint);
>>> +			return -1;
>>> +		}
>>> +	}
>>> +
>>>  	/*
>>>  	 * To create a "private" copy, the target mount is bind-mounted
>>>  	 * in a temporary place w/o MS_REC (non-recursively).
>>> @@ -1252,7 +1260,7 @@ static int tmpfs_dump(struct mount_info *pm)
>>>  
>>>  	fd = open_mountpoint(pm);
>>>  	if (fd < 0)
>>> -		return -1;
>>> +		return 1;
>>
>> No, no, don't propagate every error from open_mountpoint() into "I'm overmounted"
>> return code.
> 
> Why? criu enumirates all mount points to find one which is not
> overmounted (can be opened).

Because if an fs cannot be opened for any reason other that being
overmounted, you'd still try to find a dumpable mount. But it's
better to abort the dump.

Make clear return code from open_mountpoint() saying -- it's overmounted.
And propagate it from here.

> We can't check that a mount point isn't overmounted before calling
> fstype->dump(), because for example autofs is in many cases is
> overmounted, but autofs->dump() can dump this mount point.

Yes, that's clear.

-- Pavel