[CRIU] Re: [PATCH 3/3] fifo: Add checkpoint restore for fifos

[Pavel Emelyanov]

On 06/26/2012 06:49 PM, Cyrill Gorcunov wrote:
> 
> Checpoint and restore of fifo is similar to
> pipes c/r except the pipe end-points are named
> file.
> 
> Because the fifo has a name we use regular files
> facility for fifo path c/r.
> 
> Still there is a trick used to "open" fifo:
> the opening procedure migh sleep if a fifo's peer
> is not yet opened, so before doing a real open
> we yield a fake open procedure (with O_RDWR flag)
> which prevents us from sleeping even if peer
> is not yet ready.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  Makefile            |    1 +
>  cr-dump.c           |    9 ++-
>  cr-restore.c        |    4 +
>  cr-show.c           |   40 +++++++---
>  crtools.c           |    2 +
>  fifo.c              |  212 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  files-reg.c         |    8 --
>  include/crtools.h   |    4 +
>  include/fifo.h      |   10 +++
>  include/files-reg.h |   10 +++
>  include/image.h     |    8 ++
>  11 files changed, 288 insertions(+), 20 deletions(-)
>  create mode 100644 fifo.c
>  create mode 100644 include/fifo.h
> 

> +static int dump_one_fifo(int lfd, u32 id, const struct fd_parms *p)
> +{
> +	int img = fdset_fd(glob_fdset, CR_FD_FIFO);
> +	struct fifo_entry e;
> +
> +	/*
> +	 * It's a trick here, we use regular files dumping
> +	 * code to save path to a fifo, then we reuse it
> +	 * on restore.
> +	 */
> +	if (dump_one_reg_file(lfd, id, p))
> +		return -1;
> +
> +	pr_info("Dumping fifo %d with id %#x pipe_id %#x\n",
> +		lfd, id, (u32)p->stat.st_ino);
> +
> +	e.id		= id;
> +	e.pipe_id	= p->stat.st_ino;

Why do we need two IDs for fifo? Isn't one enough?

> +	if (write_img(img, &e) < 0)
> +		return -1;
> +
> +	return dump_one_pipe_data(CR_FD_FIFO_DATA, lfd, id, p);
> +}

> +static int open_fifo_fd(struct file_desc *d)
> +{
> +	struct fifo_info *info = container_of(d, struct fifo_info, d);
> +	int new_fifo, fake_fifo = -1;
> +
> +	struct reg_file_info *rfi;
> +	struct file_desc *rd;
> +
> +	pr_info("\t\tCreating fifo pipe_id=%#x id=%#x\n",
> +		info->fe.pipe_id, info->fe.id);
> +
> +	rd = find_file_desc_raw(FDINFO_REG, info->fe.id);
> +	if (!rd) {
> +		pr_perror("Can't find regfile for fifo %#x\n", info->fe.id);
> +		return -1;
> +	}
> +
> +	rfi = container_of(rd, struct reg_file_info, d);
> +	if (rfi->remap_path) {
> +		if (link(rfi->remap_path, rfi->path) < 0) {
> +			pr_perror("Can't link %s -> %s\n",
> +				  rfi->remap_path, rfi->path);
> +			return -1;
> +		}
> +	} else {
> +		if (!rfi->path) {
> +			pr_err("Nil path found on regfile %#x for fifo %#x\n",
> +			       rfi->rfe.id, info->fe.id);
> +			return -1;
> +		}
> +	}
> +
> +	/*
> +	 * The fifos (except read-write fifos) do wait until
> +	 * another pipe-end get connected, so to be able to
> +	 * proceed the restoration procedure we open a fake
> +	 * fifo here.
> +	 */
> +	fake_fifo = open(rfi->path, O_RDWR);
> +	if (fake_fifo < 0) {
> +		pr_perror("Can't open fake fifo %#08x [%s]",
> +			  info->fe.id, rfi->path);
> +		return -1;
> +	}
> +
> +	new_fifo = open(rfi->path, rfi->rfe.flags);
> +	if (new_fifo < 0) {
> +		pr_perror("Can't open fifo %#08x [%s]",
> +			  info->fe.id, rfi->path);
> +		goto err_close_fake;
> +	}

Can the
  open(O_RDWR) + open(O_xxx)
be replaced with 
  open(O_xxx | O_NONBLOCK) + fcntl(~O_NONBLOCK);
?

This will allow to gracefully fix the comment labeled A: below.

> +
> +	if (restore_pipe_data(CR_FD_FIFO_DATA, fake_fifo, info->fe.id,
> +			      info->bytes, info->off))
> +		goto err_unlink;

Multiple fifo data restoration?

> +	if (rfi->remap_path)
> +		unlink(rfi->path);
> +
> +	if (restore_fown(new_fifo, &rfi->rfe.fown) < 0) {
> +		pr_perror("Can't restore owner on fifo %#08x",
> +			  info->fe.id);
> +		goto err_close_new;
> +	}

A: You didn't merge it with the open_fe_fd :\

> +err_close_fake:
> +	close_safe(&fake_fifo);
> +	return new_fifo;
> +
> +err_close_new:
> +	close_safe(&new_fifo);
> +	goto err_close_fake;
> +
> +err_unlink:
> +	if (rfi->remap_path)
> +		unlink(rfi->path);
> +	goto err_close_new;
> +}