[CRIU] [PATCH 3/3] lib: add criu_check() to libcriu

[Pavel Emelyanov]

On 11/22/2013 09:31 PM, Ruslan Kuprieiev wrote:
> On 22.11.2013 14:18, Andrew Vagin wrote:
>> On Fri, Nov 22, 2013 at 01:50:29PM +0400, Pavel Emelyanov wrote:
>>> On 11/22/2013 01:24 PM, Andrew Vagin wrote:
>>>> On Fri, Nov 22, 2013 at 12:50:58PM +0400, Pavel Emelyanov wrote:
>>>>> On 11/22/2013 12:41 PM, Andrew Vagin wrote:
>>>>>> On Fri, Nov 22, 2013 at 05:20:37AM +0400, Ruslan Kuprieiev wrote:
>>>>>>> Signed-off-by: Ruslan Kuprieiev <kupruser at gmail.com>
>>>>>>> ---
>>>>>>>   lib/criu.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>   1 file changed, 128 insertions(+)
>>>>>>>
>>>>>>> diff --git a/lib/criu.c b/lib/criu.c
>>>>>>> index 2bd224e..f154a88 100644
>>>>>>> --- a/lib/criu.c
>>>>>>> +++ b/lib/criu.c
>>>>>>> @@ -1,3 +1,131 @@
>>>>>>>   #include "version.h"
>>>>>>> +#include <sys/socket.h>
>>>>>>> +#include <sys/un.h>
>>>>>>> +#include <limits.h>
>>>>>>> +#include <stdio.h>
>>>>>>> +#include <unistd.h>
>>>>>>> +#include <stdlib.h>
>>>>>>> +
>>>>>>> +#include "rpc.pb-c.h"
>>>>>>> +
>>>>>>> +#define MAX_MSG_SIZE 1024
>>>>>>> +#define CR_DEFAULT_SERVICE_ADDRESS "/var/run/criu_service.socket"
>>>>>>>   
>>>>>>>   const char *criu_lib_version = CRIU_VERSION;
>>>>>>> +
>>>>>>> +static char *service_address = CR_DEFAULT_SERVICE_ADDRESS;
>>>>>>> +
>>>>>>> +void criu_set_service_address(char *path)
>>>>>>> +{
>>>>>>> +	if (path)
>>>>>>> +		service_address = path;
>>>>>>> +	else
>>>>>>> +		service_address = CR_DEFAULT_SERVICE_ADDRESS;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static CriuResp *recv_resp(int socket_fd)
>>>>>>> +{
>>>>>>> +	unsigned char buf[MAX_MSG_SIZE];
>>>>>>> +	int len;
>>>>>>> +	CriuResp *msg = 0;
>>>>>>> +
>>>>>>> +	len = read(socket_fd, buf, MAX_MSG_SIZE);
>>>>>>> +	if (len == -1) {
>>>>>>> +		perror("Can't read response");
>>>>>>> +		return NULL;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	msg = criu_resp__unpack(NULL, len, buf);
>>>>>>> +	if (!msg) {
>>>>>>> +		perror("Failed unpacking response");
>>>>>>> +		return NULL;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	return msg;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static int send_req(int socket_fd, CriuReq *req)
>>>>>>> +{
>>>>>>> +	unsigned char buf[MAX_MSG_SIZE];
>>>>>>> +	int len;
>>>>>>> +
>>>>>>> +	len = criu_req__get_packed_size(req);
>>>>>>> +
>>>>>>> +	if (criu_req__pack(req, buf) != len) {
>>>>>>> +		perror("Failed packing request");
>>>>>>> +		return -1;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	if (write(socket_fd, buf, len)  == -1) {
>>>>>>> +		perror("Can't send request");
>>>>>>> +		return -1;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static int criu_connect(void)
>>>>>>> +{
>>>>>>> +	int fd, ret;
>>>>>>> +	struct sockaddr_un addr;
>>>>>>> +	socklen_t addr_len;
>>>>>>> +
>>>>>>> +	fd = socket(AF_LOCAL, SOCK_SEQPACKET, 0);
>>>>>>> +	if (fd < 0) {
>>>>>>> +		perror("Can't create socket");
>>>>>>> +		return -1;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	memset(&addr, 0, sizeof(addr));
>>>>>>> +	addr.sun_family = AF_LOCAL;
>>>>>>> +
>>>>>>> +	strcpy(addr.sun_path, service_address);
>>>>>>> +
>>>>>>> +	addr_len = strlen(addr.sun_path) + sizeof(addr.sun_family);
>>>>>>> +
>>>>>>> +	ret = connect(fd, (struct sockaddr *) &addr, addr_len);
>>>>>>> +	if (ret < 0) {
>>>>>>> +		perror("Can't connect to socket");
>>>>>>> +		close(fd);
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	return fd;
>>>>>>> +}
>>>>>>> +
>>>>>>> +int criu_check(void)
>>>>>> I'm not sure that a stateless approach is good here. First of all you
>>>>>> will need to connect and disconnect each time.
>>>>> Why is it bad?
>>>> I don't know yet.
>>>>
>>>>>> Can we create interface similar with dynamic linking loader (man
>>>>>> dlopen).
>> The correct question is:
>> Why do you prefer state-less over state-full?
>>
>>>>>> struct criu_handle *h;
>>>>>>
>>>>>> h = criu_open()
>>>>>> criu_check(h);
>>>>>> ...
>>>>>> criu_smth(h);
>>>>>> ...
>>>>>> criu_close(h);
>>>>> We can. What are the benefits of state-full approach?
>>>> * don't need to connect/disconnect each time
>>> Why is it good?
>> It works faster for example.
> 
> I agree with Andrew. It is easy to implement "one connection - multiple 
> requests" logic, and will be much faster for set of requests.  I may 
> just add one more cycle, that will process requests and fork workers.

OK.

>>
>>>> * asynchronous operations and iterative operations
>>> We don't have async operations.
>> yet
>>
>>>> And one more example when state-less approach doesn't work
>>>>
>>>> * criu_check(h) returns true
>>>> * yum update criu; service criu restart
>>>> * criu_dump() returns a error
>>>> .
>>> If updated criu fails it's a bug ;)
>> I want to say, that criu_check() may start to fail after updating criu.
> 
> Should we add libcriu version(!= criu version), so if we make some 
> serious changes lib will tell user to update smth?

I think there's no need in that. We've made RPC service extendable.