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

[Pavel Emelyanov]

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