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

[Pavel Emelyanov]

On 11/20/2013 02:32 AM, Ruslan Kuprieiev wrote:
> 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.

And success to true as well. Show me how resp.success can be != resp.kernel.

>>>>>>> +
>>>>>>> +	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;
>>>>>>>
>>>>> .
>>>>>
>>> .
>>>
>>
> 
> .
>