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

[Cyrill Gorcunov]

On Wed, Jun 27, 2012 at 09:24:58PM +0400, Pavel Emelyanov wrote:
> > +
> > +	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.
> 

well, I fear no, we can't. look, O_RDWR guarantees that we
do not block, while O_NONBLOCK doesn't.

	case FMODE_WRITE:
	/*
	 *  O_WRONLY
	 *  POSIX.1 says that O_NONBLOCK means return -1 with
	 *  errno=ENXIO when there is no process reading the FIFO.
	 */
		ret = -ENXIO;
		if ((filp->f_flags & O_NONBLOCK) && !pipe->readers)
			goto err;

		filp->f_op = &write_pipefifo_fops;
		pipe->w_counter++;
		if (!pipe->writers++)
			wake_up_partner(inode);

		if (!pipe->readers) {
			wait_for_partner(inode, &pipe->r_counter);
			if (signal_pending(current))
				goto err_wr;
		}
		break;

thus I O_RDWR is most simplier approach. Still I could try to reorder
creation of fifos with O_RDONLY | O_NONBLOCK first

	case FMODE_READ:
	/*
	 *  O_RDONLY
	 *  POSIX.1 says that O_NONBLOCK means return with the FIFO
	 *  opened, even when there is no process writing the FIFO.
	 */
		filp->f_op = &read_pipefifo_fops;
		pipe->r_counter++;
		if (pipe->readers++ == 0)
			wake_up_partner(inode);

		if (!pipe->writers) {
			if ((filp->f_flags & O_NONBLOCK)) {
				/* suppress POLLHUP until we have
				 * seen a writer */
				filp->f_version = pipe->w_counter;
			} else {
				wait_for_partner(inode, &pipe->w_counter);
				if(signal_pending(current))
					goto err_rd;
			}
		}
		break;

... but still if there only one fifo end with O_WRONLY I don't see
a way to restore it without fake O_RDWR pipe end.

> > +
> > +	if (restore_pipe_data(CR_FD_FIFO_DATA, fake_fifo, info->fe.id,
> > +			      info->bytes, info->off))
> > +		goto err_unlink;
> 
> Multiple fifo data restoration?

Hmm, will re-check but at moment of dumping we do test to not dump
same data twise.

> 
> > +	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 :\

Yeah, I know, i tried to escape code uglification. but will retry again.

	Cyrill