[CRIU] [PATCH 2/2 v4] fsnotify: Always provide the path for inotify watchees

[Pavel Emelyanov]

On 10/19/2015 01:14 PM, Cyrill Gorcunov wrote:
> On Mon, Oct 19, 2015 at 12:57:35PM +0300, Pavel Emelyanov wrote:
>>> +
>>> +		fd = userns_call(open_by_handle, UNS_FDOUT, &handle,
>>> +				 sizeof(handle), mntfd);
>>
>> Turn from open_by_handle into userns_call deserves separate patch and explanation.
> 
> It was there all the time, not introduced by me, I simply reuse this call.

Oh, indeed :)

>>> +		if ((m->fstype->code != FSTYPE__TMPFS) &&
>>> +		    (m->fstype->code != FSTYPE__DEVTMPFS)) {
>>> +			if (!opts.force_irmap)
>>> +				goto out_nopath;
>>> +			else
>>> +				goto force_irmap;
>>> +		}
>>
>> So this WHOLE loop is for a single thing -- handle path resolve for tmpfs mounts. Right?
> 
> Yeah, exactly!

OK. Can we then split common path from tmpfs-path then? Like -- do regular
open_handle(), then (if opened) get the fstype and IFF it's tmpfs go on a
loop to find the accessible path?

Another issue I've found while re-reading the patch:

> +		openable_fd = openat(mntfd, __path, O_PATH);
> +		if (openable_fd >= 0) {
> +			close(openable_fd);
> +
> +			pr_debug("\t\t\topenable as %s\n", __path);
> +			path = xstrdup(buf);
>  			if (path == NULL)
>  				goto err;
>  
>  			f_handle->has_mnt_id = true;
> -			f_handle->mnt_id = mi->mnt_id;
> -
> +			f_handle->mnt_id = m->mnt_id;
>  			goto out;
> -		}
> -
> -		if (!opts.force_irmap)
> -			/*
> -			 * If we're not forced to do irmap, then
> -			 * say we have no path for watch. Otherwise
> -			 * do irmap scan even if the handle is
> -			 * working.
> -			 *
> -			 * FIXME -- no need to open-by-handle if
> -			 * we are in force-irmap and not on tempfs
> -			 */
> -			goto out_nopath;
> +		} else
> +			pr_debug("\t\t\tnot openable as %s (%m)\n", __path);
>  	}

After you've managed to open something by some name you should check that
the file opened is the same as referenced by fd from open_by_handle().

>>> +		pr_debug("\t\t\tlink as %s\n", __path);
>>>  
>>> -			if (read_fd_link(fd, p, sizeof(p)) < 0)
>>> -				goto err;
>>> +		mntfd = mntns_get_root_by_mnt_id(m->mnt_id);
>>
>> Why not just "m->nsid" dereference?
> 
> Good point. But this is just an optimisatio, may I do it on top?
> 
> 	Cyrill
> .
>