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

[Cyrill Gorcunov]

On Mon, Oct 19, 2015 at 01:23:21PM +0300, Pavel Emelyanov wrote:
> > 
> > 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().

Well, I'll try to split it but I fear this would be more messy. Simply take
a look not on the diff but the complete function instead

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;

	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;

		fd = userns_call(open_by_handle, UNS_FDOUT, &handle,
				 sizeof(handle), mntfd);
		close(mntfd);
		if (fd < 0)
			goto cant_open;

		/*
		 * 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);

		/*
		 * Convert into a relative path.
		 */
		__path = (buf[1] != '\0') ? buf + 1 : ".";
		pr_debug("\t\t\tlink as %s\n", __path);

		mntfd = mntns_get_root_by_mnt_id(m->mnt_id);
		if (mntfd < 0)
			goto err;

		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 = m->mnt_id;
			goto out;
		} 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");
		return -1;
	}
out:
	pr_debug("\tDumping %s as path for handle\n", path);
	f_handle->path = path;
out_nopath:
	close_safe(&fd);
	return 0;
err:
	close_safe(&fd);
	return -1;
}

As to path check -- we don't care which path we obtain,
the inode referenced by the path is the same.