[CRIU] [PATCH 4/5] lib: add check/dump/restore

[Pavel Emelyanov]

On 12/10/2013 10:11 PM, Ruslan Kuprieiev wrote:

More comments inline.

> Signed-off-by: Ruslan Kuprieiev <kupruser at gmail.com>
> ---

> +static int send_req_and_recv_resp(CriuReq *req, CriuResp **resp)
> +{
> +	int fd;
> +	int ret	= 0;
> +
> +	fd = criu_connect();
> +	if (fd < 0) {
> +		perror("Can't connect to criu");
> +		ret = ECONNREFUSED;

The crui_connect can fail for numerous reasons. And all of them
will result in errno = ECONNREFUSED. Try to preserve the errno
from system calls if they fail. And set some sane one _if_ they
succeed, but we want to fail the call anyway.

> +		goto exit;
> +	}
> +
> +	if (send_req(fd, req) < 0) {
> +		ret = ECOMM;

Same here.

> +		goto exit;
> +	}
> +
> +	*resp = recv_resp(fd);
> +	if (!*resp) {
> +		perror("Can't receive response");
> +		ret = ECOMM;

And here.

> +		goto exit;
> +	}
> +
> +	if ((*resp)->type != req->type) {
> +		if ((*resp)->type == CRIU_REQ_TYPE__EMPTY &&
> +		    (*resp)->success == false)
> +			ret = EINVAL;
> +		else {
> +			perror("Unexpected response type");
> +			ret = EBADMSG;
> +		}
> +	}
> +
> +exit:
> +	if (fd > 0)
> +		close(fd);
> +
> +	return ret;
> +}
> +
> +int criu_check(void)
> +{
> +	int ret = -1;
> +	int err = 0;
> +	CriuReq req	= CRIU_REQ__INIT;
> +	CriuResp *resp	= NULL;
> +
> +	req.type	= CRIU_REQ_TYPE__CHECK;
> +
> +	err = send_req_and_recv_resp(&req, &resp);
> +	if (err)
> +		goto exit;
> +
> +	ret = resp->success ? 0 : -1;
> +
> +exit:
> +	criu_resp__free_unpacked(resp, NULL);
> +
> +	if (err)
> +		errno = err;

In case send_req_and_recv_resp succeeded, but the resp->success
is false, the errno is not updated. Set it to some sane value, we'll
provide the detailed errno on the rpc-resp later.

> +
> +	return ret;
> +}
> +
> +int criu_dump(void)
> +{
> +	int ret = -1;
> +	int err = 0;
> +	CriuReq req	= CRIU_REQ__INIT;
> +	CriuResp *resp	= NULL;
> +
> +	req.type	= CRIU_REQ_TYPE__DUMP;
> +	req.opts	= opts;
> +
> +	err = send_req_and_recv_resp(&req, &resp);
> +	if (err)
> +		goto exit;
> +
> +	if (resp->success) {
> +		if (resp->dump->has_restored && resp->dump->restored)
> +			ret = 1;
> +		else
> +			ret = 0;
> +	}
> +
> +exit:
> +	criu_resp__free_unpacked(resp, NULL);
> +
> +	if (err)
> +		errno = err;
> +
> +	return ret;
> +}
> +
> +int criu_restore(void)
> +{
> +	int ret = -1;
> +	int err = 0;
> +	CriuReq req	= CRIU_REQ__INIT;
> +	CriuResp *resp	= NULL;
> +
> +	req.type	= CRIU_REQ_TYPE__RESTORE;
> +	req.opts	= opts;
> +
> +	err = send_req_and_recv_resp(&req, &resp);
> +	if (err)
> +		goto exit;
> +
> +	if (resp->success)
> +		ret = resp->restore->pid;
> +
> +exit:
> +	criu_resp__free_unpacked(resp, NULL);
> +
> +	if (err)
> +		errno = err;
> +
> +	return ret;
> +}
> diff --git a/lib/criu.h b/lib/criu.h
> new file mode 100644
> index 0000000..37f6f32
> --- /dev/null
> +++ b/lib/criu.h
> @@ -0,0 +1,50 @@
> +/*
> + * (C) Copyright 2013 Parallels, Inc. (www.parallels.com).
> + *
> + * All rights reserved. This program and the accompanying materials
> + * are made available under the terms of the GNU Lesser General Public License
> + * (LGPL) version 2.1 which accompanies this distribution, and is available at
> + * http://www.gnu.org/licenses/lgpl-2.1.html
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, you can find it here:
> + * www.gnu.org/licenses/lgpl.html
> + */
> +
> +#ifndef __CRIU_LIB_H__
> +#define __CRIU_LIB_H__
> +
> +#include <stdbool.h>
> +
> +void criu_set_service_address(char *path);
> +
> +int criu_init_opts(void); /* must be called before using any criu_set* functions down below */

This one can fail, this should be described in the comment.

> +void criu_set_pid(int pid);
> +void criu_set_images_dir_fd(int fd); /* must be set for dump/restore */
> +void criu_set_leave_running(bool leave_running);
> +void criu_set_ext_unix_sk(bool ext_unix_sk);
> +void criu_set_tcp_established(bool tcp_established);
> +void criu_set_evasive_devices(bool evasive_devices);
> +void criu_set_shell_job(bool shell_job);
> +void criu_set_file_locks(bool file_locks);
> +void criu_set_log_level(int log_level);
> +void criu_set_log_file(char *log_file);
> +
> +/* On error functions down below return -1 and errno set approprietly:

This comment is not enough. You should also mention, that e.g. criu_restore()
may return positive value (others can not).

> + * ECONNREFUSED		-- Unable to connect to criu
> + * ECOMM		-- Unable to send/recv msg to/from criu
> + * EINVAL		-- CRIU doesn't support this type of request.
> + *			   You should probably update criu.
> + * EBADMSG		-- Unexpected response from criu.
> + *		           You should probably update criu.
> + */
> +int criu_check(void);
> +int criu_dump(void);
> +int criu_restore(void);
> +
> +#endif /* __CRIU_LIB_H__ */
>