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

[Pavel Emelyanov]

On 06/29/2012 02:37 AM, Cyrill Gorcunov wrote:
> On Thu, Jun 28, 2012 at 10:20:27PM +0400, Cyrill Gorcunov wrote:
>>
>> I find it inconvenient, open_reg_by_id is already used
>>
>> [cyrill at moon crtools]$ git grep open_reg_by_id
>> cr-restore.c:   exe_fd = open_reg_by_id(args->mm.exe_file_id);
>> fifo.c: open_reg_by_id
>> files-reg.c:int open_reg_by_id(u32 id)
>> files.c:        cwd = open_reg_by_id(fe.cwd_id);
>> files.c:        return open_reg_by_id(vma_entry->shmid);
>> include/files-reg.h:extern int open_reg_by_id(u32 id);
>>
>> should I pass real_open for all callers? Actually I've had idea
>> of reworking this all completely to make more flexible design
>> on path remapping, so Pavel will you be fine if I
>>
>>  - add explicit comment on 1)
>>  - add fixme for reg-files engine and will redesign this on
>>    weekend (i'm about to switch to tty task at moment)
>>
>> Hm?
> 
> Anyway, here is a patch updated (comments fix). I'll redesign
> it later but at moment i would like to stick with this patch,
> since it brings minimal changes to files-reg.c.
> 
> 	Cyrill

> +static int __open_fifo_fd(struct reg_file_info *rfi)
> +{
> +	struct fifo_info *info = rfi->priv;
> +	int new_fifo, fake_fifo = -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 %#x [%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 %#x [%s]", info->fe.id, rfi->path);
> +		goto out;
> +	}
> +
> +	if (restore_pipe_data(CR_FD_FIFO_DATA, fake_fifo, info->fe.id,
> +			      info->bytes, info->off)) {
> +		close(new_fifo);
> +		new_fifo = -1;

I still don't understand why we restore the fifo contents for each fifo end.
Let alone the fact, that restoring fifo's data is not a part of its "opening",
but "restoring".

> +	}
> +
> +out:
> +	close(fake_fifo);
> +	return new_fifo;
> +}