Skip to content

Commit ebf9b42

Browse files
committed
cr-service: refactor check() RPC options parsing
The check() functionality is very different from dump, pre-dump, and restore. It is used only to check if the kernel supports required features, and does not need the majority of options set via RPC. This patch introduces a new lightweight RPC options parser for check() that only handles logging options. When logging to a file is required (explicit log_file or no log_to_stderr), it also resolves images_dir and work_dir. This avoids parsing options that are not used by the CHECK functionality and reduces side effects while keeping the logging behaviour identical. Signed-off-by: Radostin Stoyanov <[email protected]>
1 parent 26083aa commit ebf9b42

File tree

1 file changed

+103
-16
lines changed

1 file changed

+103
-16
lines changed

criu/cr-service.c

Lines changed: 103 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,8 @@ static int resolve_images_dir_path(char images_dir_path[PATH_MAX],
303303
strncpy(images_dir_path, req->images_dir, PATH_MAX - 1);
304304
images_dir_path[PATH_MAX - 1] = '\0';
305305
} else {
306+
if (opts.mode == CR_CHECK)
307+
return 0; /* we check if work_dir_fd in setup_images_and_workdir() */
306308
pr_err("Neither images_dir_fd nor images_dir was passed by RPC client.\n");
307309
return -1;
308310
}
@@ -317,27 +319,48 @@ static int setup_images_and_workdir(const char *images_dir_path,
317319
{
318320
char work_dir_path[PATH_MAX];
319321

320-
/* Open images dir (mode = -1, same as before) */
321-
if (open_image_dir(images_dir_path, -1) < 0) {
322-
pr_perror("Can't open images directory");
323-
return -1;
324-
}
325-
326-
/* get full path to images_dir to use in process title */
327-
if (!realpath(images_dir_path, images_dir)) {
328-
pr_perror("Can't get realpath %s", images_dir_path);
329-
return -1;
322+
if (opts.mode == CR_CHECK) {
323+
if (images_dir_path[0]) {
324+
/* get full path to images_dir to use in process title */
325+
if (!realpath(images_dir_path, images_dir)) {
326+
pr_perror("Can't get realpath %s", images_dir_path);
327+
return -1;
328+
}
329+
} else if (!work_changed_by_rpc_conf && !req->has_work_dir_fd && !opts.work_dir) {
330+
pr_err("images-dir or work-dir is required when using log file\n");
331+
return -1;
332+
}
333+
} else {
334+
if (open_image_dir(images_dir_path, -1) < 0) {
335+
pr_perror("Can't open images directory");
336+
return -1;
337+
}
330338
}
331339

332-
if (work_changed_by_rpc_conf)
340+
/* Determine work_dir_path */
341+
if (work_changed_by_rpc_conf) {
333342
strncpy(work_dir_path, opts.work_dir, PATH_MAX - 1);
334-
else if (req->has_work_dir_fd)
335-
sprintf(work_dir_path, "/proc/%d/fd/%d", peer_pid, req->work_dir_fd);
336-
else if (opts.work_dir)
343+
344+
} else if (req->has_work_dir_fd) {
345+
char proc_fd_path[PATH_MAX];
346+
ssize_t n;
347+
348+
snprintf(proc_fd_path, sizeof(proc_fd_path), "/proc/%d/fd/%d",
349+
peer_pid, req->work_dir_fd);
350+
351+
n = readlink(proc_fd_path, work_dir_path, sizeof(work_dir_path) - 1);
352+
if (n < 0) {
353+
pr_perror("Can't readlink %s", proc_fd_path);
354+
return -1;
355+
}
356+
work_dir_path[n] = '\0';
357+
} else if (opts.work_dir) {
337358
strncpy(work_dir_path, opts.work_dir, PATH_MAX - 1);
338-
else
359+
} else {
339360
strcpy(work_dir_path, images_dir_path);
361+
}
340362

363+
/* Change to work dir */
341364
if (chdir(work_dir_path)) {
342365
pr_perror("Can't chdir to work_dir");
343366
return -1;
@@ -895,6 +918,70 @@ static int restore_using_req(int sk, CriuOpts *req)
895918
return success ? 0 : 1;
896919
}
897920

921+
/*
922+
* Lightweight parser for options used by the CHECK workflow.
923+
* Only parse log_file, log_to_stderr, and log_level.
924+
*
925+
* If we will write a log file (explicit log_file or default log file),
926+
* resolve images_dir and work_dir the same way as setup_opts_from_req().
927+
*/
928+
static int setup_check_opts_from_req(int sk, CriuOpts *req)
929+
{
930+
struct ucred ids;
931+
socklen_t ids_len = sizeof(ids);
932+
char images_dir_path[PATH_MAX];
933+
934+
if (!req)
935+
return 0; /* nothing to do */
936+
937+
if (getsockopt(sk, SOL_SOCKET, SO_PEERCRED, &ids, &ids_len)) {
938+
pr_perror("Can't get socket options");
939+
return -1;
940+
}
941+
942+
/*
943+
* A log file is needed only if:
944+
* - log_file is explicitly set, or
945+
* - log_to_stderr is NOT requested (so we will default to a file)
946+
*/
947+
if (!req->log_file || (req->has_log_to_stderr && req->log_to_stderr))
948+
return 0;
949+
950+
if (resolve_images_dir_path(images_dir_path, false, req, ids.pid) < 0) {
951+
pr_err("Failed to resolve images dir path\n");
952+
return -1;
953+
}
954+
955+
if (setup_images_and_workdir(images_dir_path, false, req, ids.pid)) {
956+
pr_err("Failed to setup images and work dir\n");
957+
return -1;
958+
}
959+
960+
/* Configure logging destination */
961+
if (req->log_file) {
962+
if (strchr(req->log_file, '/')) {
963+
pr_perror("No subdirs are allowed in log_file name");
964+
return -1;
965+
}
966+
SET_CHAR_OPTS(output, req->log_file);
967+
} else if (req->has_log_to_stderr && req->log_to_stderr) {
968+
xfree(opts.output);
969+
opts.output = NULL;
970+
} else if (!opts.output) {
971+
SET_CHAR_OPTS(output, DEFAULT_LOG_FILENAME);
972+
}
973+
974+
/* Log level + init (log_init(NULL) routes to stderr) */
975+
opts.log_level = req->log_level;
976+
log_set_loglevel(req->log_level);
977+
if (log_init(opts.output) == -1) {
978+
pr_perror("Can't initiate log");
979+
return -1;
980+
}
981+
982+
return 0;
983+
}
984+
898985
static int check(int sk, CriuOpts *req)
899986
{
900987
int pid, status;
@@ -917,7 +1004,7 @@ static int check(int sk, CriuOpts *req)
9171004
__setproctitle("check --rpc");
9181005

9191006
opts.mode = CR_CHECK;
920-
if (setup_opts_from_req(sk, req))
1007+
if (setup_check_opts_from_req(sk, req))
9211008
exit(1);
9221009

9231010
exit(!!cr_check());

0 commit comments

Comments
 (0)