[CRIU] [PATCH 3/3] fsnotify: inotify -- Add flushing of criu generated events

[Andrew Vagin]

On Tue, Sep 02, 2014 at 11:27:21PM +0400, Cyrill Gorcunov wrote:
> When we restore inotify events we generate own events. So flush them before
> restored application get control back, otherwise the application may get
> confused because these events has nothing to do with original application
> logic.
> 
> Note at the moment we call the inotify_flush_events without testing
> if the descriptor is created with O_NONBLOCK, that's because we know
> that we're generating events.

but you call read() in a look, so you will block or I don't understand
smth

> But this might be not always true so
> be prepared.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  fsnotify.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 88 insertions(+)
> 
> diff --git a/fsnotify.c b/fsnotify.c
> index c14ecb4d6225..08d39e5f24f9 100644
> --- a/fsnotify.c
> +++ b/fsnotify.c
> @@ -403,6 +403,86 @@ err:
>  	return path;
>  }
>  
> +static void decode_event_mask(char *buf, size_t size, u32 mask)
> +{
> +	static const char *names[32] = {
> +		[ 0]	= "IN_ACCESS",
> +		[ 1]	= "IN_MODIFY",
> +		[ 2]	= "IN_ATTRIB",
> +		[ 3]	= "IN_CLOSE_WRITE",
> +		[ 4]	= "IN_CLOSE_NOWRITE",
> +		[ 5]	= "IN_OPEN",
> +		[ 6]	= "IN_MOVED_FROM",
> +		[ 7]	= "IN_MOVED_TO",
> +		[ 8]	= "IN_CREATE",
> +		[ 9]	= "IN_DELETE",
> +		[10]	= "IN_DELETE_SELF",
> +		[11]	= "IN_MOVE_SELF",
> +
> +		[13]	= "IN_UNMOUNT",
> +		[14]	= "IN_Q_OVERFLOW",
> +		[15]	= "IN_IGNORED",
> +
> +		[24]	= "IN_ONLYDIR",
> +		[25]	= "IN_DONT_FOLLOW",
> +		[26]	= "IN_EXCL_UNLINK",
> +
> +		[29]	= "IN_MASK_ADD",
> +		[30]	= "IN_ISDIR",
> +		[31]	= "IN_ONESHOT",
> +	};
> +
> +	size_t i, j;
> +
> +	memzero(buf, size);
> +	for (i = 0, j = 0; i < 32 && j < size; i++) {
> +		if (!(mask & (1u << i)))
> +			continue;
> +		if (j)
> +			j += snprintf(&buf[j], size - j, " | %s", names[i]);
> +		else
> +			j += snprintf(&buf[j], size - j, "%s", names[i]);
> +	}
> +}
> +
> +static int inotify_flush_events(int inotify_fd, struct fsnotify_file_info *info)
> +{
> +	struct inotify_event *event;
> +	char buf[sizeof(*event) * 8];
> +	int ret, off;
> +
> +	/*
> +	 * NOTE: It's up to a caller to make @inotify_fd being
> +	 * created with O_NONBLOCK if needed.

I don't understand this comment. I think we need to say why we can't
block here.

> +	 */
> +	while (1) {
> +		ret = read(inotify_fd, buf, sizeof(buf));

You will block here, if inotify_fd isn't in non-block state.

> +		if (ret < 0) {
> +			if (errno != EAGAIN) {
> +				pr_perror("Can't read inotify queue for 0x%08x",
> +					  info->ife->id);
> +				return -1;
> +			} else
> +				return 0;
> +		} else if (ret <= 0)
> +			break;

if (ret < 0)
	pr_perror(...);

> +
> +		if (pr_quelled(LOG_DEBUG))
> +			continue;
> +
> +		for (off = 0; off < ret; off += sizeof(*event) + event->len) {
> +			char emask[128];
> +
> +			event = (void *)(buf + off);
> +			decode_event_mask(emask, sizeof(emask), event->mask);
> +			pr_debug("\t0x%08x: flushing event %#10x -> %s\n",
> +				 info->ife->id, event->mask, emask);
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>  static int restore_one_inotify(int inotify_fd, struct fsnotify_mark_info *info)
>  {
>  	InotifyWdEntry *iwe = info->iwe;
> @@ -539,6 +619,14 @@ static int open_inotify_fd(struct file_desc *d)
>  	if (restore_fown(tmp, info->ife->fown))
>  		close_safe(&tmp);
>  
> +	if (tmp >= 0) {

I don't like this game with close_safe(&tmp). Can we add a error label
and use goto on error paths?

> +		if (!list_empty(&info->marks)) {
> +			if (inotify_flush_events(tmp, info)) {
> +				pr_err("Can't flush event queue for 0x%08x", info->ife->id);
> +				close_safe(&tmp);
> +			}
> +		}
> +	}
>  	return tmp;
>  }
>  
> -- 
> 1.9.3
>