[CRIU] [PATCH 2/2]v2 service: add support for check request

[Ruslan Kuprieiev]

On 19.11.2013 21:37, Pavel Emelyanov wrote:
> On 11/19/2013 06:03 PM, Ruslan Kuprieiev wrote:
>> On 19.11.2013 12:10, Pavel Emelyanov wrote:
>>> On 11/19/2013 03:44 PM, Ruslan Kuprieiev wrote:
>>>> On 19.11.2013 11:36, Pavel Emelyanov wrote:
>>>>> On 11/18/2013 06:25 PM, Ruslan Kuprieiev wrote:
>>>>>> Signed-off-by: Ruslan Kuprieiev <kupruser at gmail.com>
>>>>>> ---
>>>>>>     cr-service.c | 37 +++++++++++++++++++++++++++++++++++++
>>>>>>     1 file changed, 37 insertions(+)
>>>>>>
>>>>>> diff --git a/cr-service.c b/cr-service.c
>>>>>> index 65710fd..c337fce 100644
>>>>>> --- a/cr-service.c
>>>>>> +++ b/cr-service.c
>>>>>> @@ -217,6 +217,41 @@ exit:
>>>>>>     	return success ? 0 : 1;
>>>>>>     }
>>>>>>     
>>>>>> +/*
>>>>>> + * Only checks criu ability to work in this enviroment.
>>>>>> + */
>>>>>> +static int check_using_req(int sk, CriuOpts *req)
>>>>>> +{
>>>>>> +	bool success = false;
>>>>>> +	CriuResp msg = CRIU_RESP__INIT;
>>>>>> +	CriuCheckResp resp = CRIU_CHECK_RESP__INIT;
>>>>>> +
>>>>>> +	if (setup_opts_from_req(sk, req) == -1) {
>>>>>> +		pr_perror("Arguments treating fail");
>>>>>> +		goto exit;
>>>>>> +	}
>>>>>> +
>>>>>> +	if (cr_check() < 0) {
>>>>>> +		pr_perror("The kernel support isn't up-to-date");
>>>>>> +		goto exit;
>>>>>> +	}

Here, if cr_check() hasn't returned 0, it means that resp.kernel = 
false, and it is by default false. If cr_check has returned 0, than we 
set resp.kernel to true.

>>>>>> +
>>>>>> +	resp.kernel = true;
>>>>> Always true argument is pointless.
>>>> But resp.kernel = false after init. If cr_check() does not fail, we set
>>>> resp.kernel to true, and if cr_check() does fail - resp.kernel = false.
>>> So we have two types of responses:
>>>
>>> * kernel = true,  success = true  for OK
>>> * kernel = false, success = false for not-OK
>>>
>>> Why is just success = <success-or-not> not enough?
>> Here is what we've got:
>>
>> -- resp.success == false  -- options are wrong or something bad happened
>> and check wasn't done
>>
>> -- resp.success == true and resp.check.kernel == true -- check was done
>> and kernel features are ok
>>
>> -- resp.success == true and resp.check.kernel == false -- check was done
>> and kernel features are not ok
> I don't see this in the code -- resp.success = true and resp.kernel = true
> are set together in one place.
>
>> And also we may easily add some more fields for more complete system
>> state analysis.
>>
>>>>>> +
>>>>>> +	success = true;
>>>>>> +exit:
>>>>>> +	msg.type = CRIU_REQ_TYPE__CHECK;
>>>>>> +	msg.success = success;
>>>>>> +	msg.check = &resp;
>>>>>> +
>>>>>> +	if (send_criu_msg(sk, &msg) == -1) {
>>>>>> +		pr_perror("Can't send response");
>>>>>> +		success = false;
>>>>>> +	}
>>>>>> +
>>>>>> +	return success ? 0 : 1;
>>>>>> +}
>>>>>> +
>>>>>>     static int cr_service_work(int sk)
>>>>>>     {
>>>>>>     	CriuReq *msg = 0;
>>>>>> @@ -233,6 +268,8 @@ static int cr_service_work(int sk)
>>>>>>     		return dump_using_req(sk, msg->opts);
>>>>>>     	case CRIU_REQ_TYPE__RESTORE:
>>>>>>     		return restore_using_req(sk, msg->opts);
>>>>>> +	case CRIU_REQ_TYPE__CHECK:
>>>>>> +		return check_using_req(sk, msg->opts);
>>>>>>     
>>>>>>     	default: {
>>>>>>     		CriuResp resp = CRIU_RESP__INIT;
>>>>>>
>>>> .
>>>>
>> .
>>
>