[CRIU] [PATCH] mnt: relax constraints on fuse mounts

[Pavel Emelyanov]

On 04/09/2015 05:48 PM, Tycho Andersen wrote:
> Hi Pavel,
> 
> On Thu, Apr 09, 2015 at 08:26:07AM -0600, Tycho Andersen wrote:
>> On Thu, Apr 09, 2015 at 12:37:06PM +0300, Pavel Emelyanov wrote:
>>> On 04/08/2015 09:13 PM, Tycho Andersen wrote:
>>>> 1. If a fuse connection is present, but there are no fuse mounts of that type
>>>>    in the mount namespace, don't refuse to dump.
>>>> 2. If there are mounts of that type in the container but they are external,
>>>>    we're going to bind them anyway, so there's no fuse-specific things that
>>>>    need to be done, so it is safe to dump.
>>>>
>>>> Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
>>>> ---
>>>>  mount.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>>>>  1 file changed, 42 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/mount.c b/mount.c
>>>> index 71c39bf..700c303 100644
>>>> --- a/mount.c
>>>> +++ b/mount.c
>>>> @@ -1024,6 +1024,47 @@ out:
>>>>  	return ret;
>>>>  }
>>>>  
>>>> +static int fusectl_dump(struct mount_info *pm)
>>>> +{
>>>> +	int fd, ret = -1;
>>>> +	struct dirent *de;
>>>> +	DIR *fdir = NULL;
>>>> +
>>>> +	fd = open_mountpoint(pm);
>>>> +	if (fd < 0)
>>>> +		return -1;
>>>> +
>>>> +	fdir = fdopendir(fd);
>>>> +	if (fdir == NULL) {
>>>> +		close(fd);
>>>> +		return -1;
>>>> +	}
>>>> +
>>>> +	while ((de = readdir(fdir))) {
>>>> +		int id;
>>>> +		struct mount_info *it;
>>>> +
>>>> +		if (dir_dots(de))
>>>> +			continue;
>>>> +
>>>> +		if (sscanf(de->d_name, "%d", &id) != 1) {
>>>> +			pr_err("wrong number of items scanned in fusectl dump\n");
>>>> +			goto out;
>>>> +		}
>>>> +
>>>> +		for (it = mntinfo; it; it = it->next) {
>>>> +			if (id == minor(it->s_dev) && !it->external) {
>>>
>>> I'm not sure that minor number match is enough. Should we also check the
>>> it->type being fuse?
>>
>> I think it should be safe in this case because we're excluding more
>> things than we should, rather than allowing stuff through. It is
>> better to compare against it->type, though, so I'll resend.
> 
> After looking into this a bit more, I don't think we save the fsname
> directly, since we resolve it to something in fstypes. The mountinfo
> entries for fuse look like:
> 
> 46 38 0:40 / /run/user/1000/gvfs rw,nosuid,nodev,relatime - fuse.gvfsd-fuse gvfsd-fuse rw,user_id=1000,group_id=1000
> 49 21 0:43 / /home/tycho/smitten rw,nosuid,nodev,relatime - fuse.sshfs tycho at 192.168.0.69:/home/tycho rw,user_id=1000,group_id=1000
> 
> i.e. they have a string like "fuse.foo" for their fstype, which gets resolved
> to unsupported. 

:( Indeed. Kernel reports "subtypes" sometimes and the valid type-vs-subtype
delimiter is the dot symbol. I think we should do the same thing kernel does --
trim the part of fstype after '.' when resolving the fstype.

> We could either save the raw fstype too, add a special
> catch-all type for fuse (with failing dump/restore functions, since we can't
> really dump/restore these fs types) or leave the patch as is. Let me know what
> changes you want and I can make them.

I would fix the find_fstype_by_name() to carefully handle the subtypes and
then your proposal with fuse type having always failing ->dump callback.

-- Pavel