[CRIU] Re: [PATCH 2/4] tty: Add checkpoint/restore for unix terminals v3

[Pavel Emelyanov]

On 09/07/2012 06:25 PM, Cyrill Gorcunov wrote:
> On Fri, Sep 07, 2012 at 05:55:50PM +0400, Pavel Emelyanov wrote:
>> On 09/07/2012 05:44 PM, Cyrill Gorcunov wrote:
>>> On Fri, Sep 07, 2012 at 05:41:00PM +0400, Cyrill Gorcunov wrote:
>>>>  
>>>> -static int dump_chrdev(struct fd_parms *p, int lfd, const struct cr_fdset *set)
>>>> +static int dump_chrdev(pid_t pid, struct fd_parms *p, int lfd, const struct cr_fdset *set)
>>>
>>> sigh, i missed this redundant pid parameter. please ignore it, if anything else
>>> will be fine for you i'll update it on top.
>>
>> You haven't fixed all the comments from the previous attempt. Please do it.
> 
> OK, this one should fit the request. Note that I've not addressed comments on
> crontol terminals code with purpose -- they are will be addressed in furter
> patches which implement ctl tty functionality.
> 
> The "char *" from pty private no longer needed, so I dropped it.
> 
> As to "I don't see the tty contents dumping." -- at moment we can't
> dump ttys buffers yet.
> 
> But one comment left untouched:
> 
>> +             winsize_conv(&w, info­>tfe­>winsize);
>> +             if (ioctl(fd, TIOCSWINSZ, &w) < 0)
>> +                     goto err;
>> +     }
>> Need checks for n_foo is enough.
> 
> I don;t understand what exactly you mean here. If you're
> about c_cc array in termios structure -- I've added a check
> in "./crtools check" for it. Or maybe you mean something else?

You assign the n_c_cc = TERMIOS_NCC, thus you should check in on restore.

OK, this patch looks sane. Where's the rest with control terminals?

> 	Cyrill