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

[Andrew Vagin]

On Wed, Oct 14, 2015 at 03:23:05PM +0300, Cyrill Gorcunov wrote:
> On Wed, Oct 14, 2015 at 01:01:44AM +0300, Cyrill Gorcunov wrote:
> > 
> > Update attached (I've added also more comments into a code).
> > Take a look please (I tested inotify tests and containers
> > c/r)
> 
> Here is a version 3 attached, as Andrew pointed we don't have
> to fetch paths for fs which have persistent inodes. Please take
> a look.

> From 87dbc133b815ee9624745838f03966f11db9a0b8 Mon Sep 17 00:00:00 2001
> From: Cyrill Gorcunov <gorcunov at openvz.org>
> Date: Tue, 13 Oct 2015 19:58:22 +0300
> Subject: [PATCH] fsnotify: Always provide the path for inotify watchees
> 
> In debian-8 container we faced the problem -- systemd creates nested
> mount namespaces and inotify watchee are resolved into a path which
> is inaccessbile on restore, the same happens when pathes where
> watchees are living are bind-overmounted. Thus when we try to
> restore such watchees we can't open the paths.
> 
> Lets do a trick here (thanks a huge to Andrew Vagin for idea and
> overall help) -- walk over all mount points which device match
> the handle's device and open handle first and test if the path
> provided is openable as well for tmpfs and devtmps which do not
> save inodes between remounts. After all the inotify objects are
> bound to inode so it's irrelevean via which path it's assigned.
> 
> https://jira.sw.ru/browse/PSBM-39957
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  fsnotify.c | 99 +++++++++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 69 insertions(+), 30 deletions(-)
> 
> diff --git a/fsnotify.c b/fsnotify.c
> index 931e76785b41..d3e1865c4056 100644
> --- a/fsnotify.c
> +++ b/fsnotify.c
> @@ -139,56 +139,95 @@ out:
>  int check_open_handle(unsigned int s_dev, unsigned long i_ino,
>  		FhEntry *f_handle)
>  {
> +	struct mount_info *m;
> +	fh_t handle;
>  	int fd = -1;
>  	char *path;
>  
> -	fd = open_handle(s_dev, i_ino, f_handle);
> -	if (fd >= 0) {
> -		struct mount_info *mi;
> +	decode_handle(&handle, f_handle);
> +
> +	/*
> +	 * We gonna try to open the handle and then
> +	 * depending on command line options and type
> +	 * of the filesystem (tmpfs/devtmpfs do not
> +	 * preserve their inodes between mounts) we
> +	 * might need to find out an openable path
> +	 * get used on restore as a watch destination.
> +	 */
> +	for (m = mntinfo; m; m = m->next) {
> +		char buf[PATH_MAX], *__path;
> +		int mntfd, openable_fd;
> +
> +		if (m->s_dev != s_dev)
> +			continue;
> +
> +		mntfd = __open_mountpoint(m, -1);
> +		pr_debug("\t\tTrying via mntid %d root %s ns_mountpoint @%s (%d)\n",
> +			 m->mnt_id, m->root, m->ns_mountpoint, mntfd);
> +		if (mntfd < 0)
> +			goto cant_open;
>  
> -		pr_debug("\tHandle 0x%x:0x%lx is openable\n", s_dev, i_ino);
> +		fd = userns_call(open_by_handle, UNS_FDOUT, &handle,
> +				 sizeof(handle), mntfd);
> +		close(mntfd);
> +		if (fd < 0)
> +			goto cant_open;
>  
> -		mi = lookup_mnt_sdev(s_dev);
> -		if (mi == NULL) {
> -			pr_err("Unable to lookup a mount by dev 0x%x\n", s_dev);
> +		/*
> +		 * On tmpfs/devtmps we have to always fetch
> +		 * openable path, in turn on all others
> +		 * it depends on command line option: if
> +		 * we're requested to use irmap lets fetch
> +		 * the path, otherwise simply save the bare
> +		 * handler and that's it.
> +		 */
> +		if ((m->fstype->code != FSTYPE__TMPFS) &&
> +		    (m->fstype->code != FSTYPE__DEVTMPFS)) {
> +			if (!opts.force_irmap)
> +				goto out_nopath;
> +			else
> +				goto force_irmap;
> +		}
> +
> +		if (read_fd_link(fd, buf, sizeof(buf)) < 0) {
> +			close_safe(&fd);
>  			goto err;
>  		}
> +		close_safe(&fd);
>  
>  		/*
> -		 * Inode numbers are not restored for tmpfs content, but we can
> -		 * get file names, becasue tmpfs cache is not pruned.
> +		 * Convert into a relative path.
>  		 */
> -		if ((mi->fstype->code == FSTYPE__TMPFS) ||
> -				(mi->fstype->code == FSTYPE__DEVTMPFS)) {
> -			char p[PATH_MAX];
> +		__path = (buf[1] != '\0') ? buf + 1 : buf;

it (buf[1] == 0)
		__path = ".";

> +		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);
> +		if (mntfd < 0)
> +			goto err;
>  
> -			path = xstrdup(p);
> +		openable_fd = openat(mntfd, __path, O_PATH);
> +		/*
> +		 * Close it early, we only interested
> +		 * in openability.
> +		 */
> +		close(openable_fd);

don't need to call close for -1

> +
> +		if (openable_fd >= 0) {
> +			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);
>  	}
>  
> +cant_open:
>  	pr_warn("\tHandle 0x%x:0x%lx cannot be opened\n", s_dev, i_ino);
> +force_irmap:
>  	path = irmap_lookup(s_dev, i_ino);
>  	if (!path) {
>  		pr_err("\tCan't dump that handle\n");
> -- 
> 2.4.3
>