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

[Andrew Vagin]

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.

> 
> > * 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.