[CRIU] [PATCHv6 3/5] crtools: cr_service meat

[Pavel Emelyanov]

On 09/12/2013 01:00 AM, Ruslan Kuprieiev wrote:
> Signed-off-by: Ruslan Kuprieiev <kupruser at gmail.com>
> 


> @@ -1,6 +1,250 @@
> -int cr_service()
> +static int treat_req(CriuDumpReq *req)

Function name is bad.

> +{
> +	struct ucred ids;
> +	struct stat st;
> +	socklen_t ids_len = sizeof(struct ucred);
> +	char images_dir_path[PATH_MAX];
> +
> +	if (getsockopt(cr_service_client->sk_fd, SOL_SOCKET, SO_PEERCRED,
> +							  &ids, &ids_len)) {
> +		pr_perror("Can't get socket options.");
> +		return -1;
> +	}
> +
> +	cr_service_client->pid = ids.pid;
> +	cr_service_client->uid = ids.uid;
> +
> +	if (req->pid == 0)
> +		req->pid = ids.pid;
> +
> +	if (fstat(cr_service_client->sk_fd, &st)) {
> +		pr_perror("Can't get socket stat");
> +		return -1;
> +	}
> +
> +	cr_service_client->sk_ino = st.st_ino;
> +
> +	/* going to dir, where to place images*/
> +	sprintf(images_dir_path, "/proc/%d/fd/%d",
> +		cr_service_client->pid, req->images_dir_fd);
> +
> +	if (chdir(images_dir_path)) {
> +		pr_perror("Can't chdir to images directory");
> +		return -1;
> +	}
> +
> +	if (open_image_dir() < 0)
> +		return -1;
> +
> +	/* initiate log file in imgs dir */
> +	opts.output = xmalloc(PATH_MAX);
> +	strcpy(opts.output, images_dir_path);
> +	strcat(opts.output, "/dump.log");

Use relative path for log file name.

> +
> +	/* setting loglevel */
> +	/*
> +	 * FIXME why relative path does not work?
> +	 * Maybe child uses env of parent?
> +	 */
> +	log_set_loglevel(req->log_level);
> +	if (log_init(opts.output) == -1) {
> +		pr_perror("Can't initiate log.");
> +		return -1;
> +	}
> +
> +	/* checking dump flags from client */
> +	if (req->leave_running)
> +		opts.final_state = TASK_ALIVE;
> +
> +	opts.ext_unix_sk	= req->ext_unix_sk;
> +	opts.tcp_established_ok	= req->tcp_established;
> +	opts.evasive_devices	= req->evasive_devices;
> +	opts.shell_job		= req->shell_job;
> +	opts.handle_file_locks	= req->file_locks;
> +
> +	return 0;
> +}
> +
> +int cr_service(bool daemon_mode)
>  {
> +	int server_fd;
> +	int child_pid;
> +
> +	struct sockaddr_un server_addr;
> +	struct sockaddr_un client_addr;
> +
> +	socklen_t server_addr_len;
> +	socklen_t client_addr_len;
> +
> +	CriuDumpReq *req = 0;
> +	CriuDumpResp *resp;
> +
> +	cr_service_client = malloc(sizeof(struct _cr_service_client));
> +	resp = xmalloc(sizeof(CriuDumpResp));

The xmalloc can fail, but if you make resp a struct variable, not a pointer,
you wouldn't have to allocate it.

> +	criu_dump_resp__init(resp);
> +
> +	server_fd = socket(AF_LOCAL, SOCK_SEQPACKET, 0);

I have a subtle feeling that you didn't test this. Criu cannot dump
seq-packet sockets due to check in can_dump_unix_sk().

> +	if (server_fd == -1) {
> +		pr_perror("Can't initialize service socket.");
> +		return -1;
> +	}
> +
> +	memset(&server_addr, 0, sizeof(server_addr));
> +	memset(&client_addr, 0, sizeof(client_addr));
> +	server_addr.sun_family = AF_LOCAL;
> +
> +	if (opts.addr == NULL)
> +		opts.addr = CR_DEFAULT_SERVICE_ADDRESS;
> +
> +	strcpy(server_addr.sun_path, opts.addr);
> +
> +	server_addr_len = strlen(server_addr.sun_path)
> +			+ sizeof(server_addr.sun_family);
> +	client_addr_len = sizeof(client_addr);
> +
> +	unlink(server_addr.sun_path);
> +
> +	if (bind(server_fd, (struct sockaddr *) &server_addr,
> +					server_addr_len) == -1) {
> +		pr_perror("Can't bind.");
> +		return -1;
> +	}
> +
> +	pr_info("The service socket was bound to %s\n", server_addr.sun_path);
> +
> +	/* change service socket permissions, so anyone can connect to it */
> +	if (chmod(server_addr.sun_path, 0666)) {
> +		pr_perror("Can't change permissions of the service socket.");
> +		return -1;
> +	}
> +
> +	if (listen(server_fd, 16) == -1) {
> +		pr_perror("Can't listen for socket connections.");
> +		return -1;
> +	}
> +
> +	if (daemon_mode) {
> +		if (daemon(0, 0) == -1) {
> +			pr_perror("Can't run service server in the background");
> +			return -errno;
> +		}
> +	}
> +
> +	/* FIXME Do not ignore children's return values */
> +	signal(SIGCHLD, SIG_IGN);
> +
> +	while (1) {
> +		pr_info("Waiting for connection...\n");
> +
> +		cr_service_client->sk_fd = accept(server_fd,
> +						  &client_addr,
> +						  &client_addr_len);
> +		if (cr_service_client->sk_fd == -1) {
> +			pr_perror("Can't accept connection.");
> +			continue;
> +		}
> +
> +		pr_info("Connected.\n");
> +
> +		switch (child_pid = fork()) {
> +		case -1:
> +			pr_perror("Can't fork a child.");
> +			continue;
> +
> +		case 0:
> +			if (recv_req(cr_service_client->sk_fd, &req) == -1) {
> +				pr_perror("Can't recv request");
> +				goto exit;
> +			}
> +
> +			if (treat_req(req) == -1) {
> +				pr_perror("Arguments treating fail");
> +				goto exit;
> +			}
> +
> +			if (cr_dump_tasks(req->pid) == -1) {
> +				pr_perror("Dump fail");
> +				goto exit;
> +			}
> +
> +			resp->success = true;
> +
> +exit:
> +			if (req->leave_running) {
> +				if (send_resp(cr_service_client->sk_fd,
> +								resp) == -1) {
> +					pr_perror("Can't send response");
> +					resp->success = false;
> +				}
> +			}
> +
> +			close(cr_service_client->sk_fd);
> +			exit(resp->success ? 0 : 1);
> +
> +		default:
> +			close(cr_service_client->sk_fd);
> +		}
> +	}
> +
>  	return 0;
>  }

> @@ -1,6 +1,22 @@
>  #ifndef __CR_SERVICE_H__
>  #define __CR_SERVICE_H__
>  
> -int cr_service(void);
> +#include "protobuf/rpc.pb-c.h"
> +
> +#define CR_DEFAULT_SERVICE_ADDRESS "/tmp/criu_service.socket"
> +#define MAX_MSG_SIZE 1024
> +
> +int cr_service(bool deamon_mode);
> +
> +int send_resp(int socket_fd, CriuDumpResp *resp);
> +
> +struct _cr_service_client {
> +	int sk_ino;
> +	int uid;
> +	int pid;
> +	int sk_fd;
> +};
> +
> +struct _cr_service_client *cr_service_client;

A variable cannot be declared in a header file -- linker would object
its presence in several places.

>  
>  #endif
> 
>