[CRIU] [PATCH] Introducing the --check-only option
Pavel Emelyanov
xemul at virtuozzo.com
Mon Oct 10 03:52:19 PDT 2016
On 10/06/2016 06:32 PM, Adrian Reber wrote:
> From: Adrian Reber <areber at redhat.com>
Please, see my comments inline.
> Talking about criu a common question is, if it is possible to know if a
> checkpoint and restore will actually work. Running 'criu dump' with
> --leave-running to see if the checkpointing will work and then running
> 'criu restore' is always an option. If one of those operations (either
> 'dump' or 'restore') will fail the chances are high that there are
> problems with checkpointing or restoring. But a lot of memory might have
> already been dumped to disk and transferred to the destination system
> which is not necessary to test for a restore failure. If the restore,
> however, works the problem exists that the source process has been told
> to keep on running (--leave-running) which might be an undesired
> situation to have the process now running on the source and destination
> system. To avoid a situation like this and to give an easier option to
> test if 'criu dump' and 'criu restore' will work, this patch introduces
> the '--check-only' option:
>
> source system:
> # criu dump --check-only -D /tmp/cp -t <PID>
> Only checking if requested operation will succeed
> # rsync -a /tmp/cp dest-system:/tmp
>
> destination system:
> # criu restore -D /tmp/cp
> Checking mode enabled
>
> criu will detect if a checkpoint is a 'check-only' checkpoint and the
> restore will automatically run in '--check-only' mode.
>
> It is also possible to use the '--check-only' switch on a full
> checkpoint to see if the restore will succeed and making sure at the
> same time that the process will not start running:
>
> destination system:
> # criu restore --check-only -D /tmp/cp
> Only checking if requested operation will succeed
> Checking mode enabled
>
> Right now only the existing checks (e.g., check binary size) are run in
> 'check-only' mode but additional checks could be added like:
>
> * checksums of binaries
> * checksums of used libraries
> * available memory
>
> Signed-off-by: Adrian Reber <areber at redhat.com>
> ---
> Documentation/criu.txt | 21 +++++++++++++++++++++
> criu/cr-restore.c | 14 +++++++++-----
> criu/crtools.c | 9 +++++++++
> criu/image.c | 10 ++++++++++
> criu/include/cr_options.h | 1 +
> criu/include/restorer.h | 2 ++
> criu/mem.c | 9 +++++++++
> criu/pie/restorer.c | 9 +++++++++
> images/inventory.proto | 1 +
> 9 files changed, 71 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/criu.txt b/Documentation/criu.txt
> index 48331d7..86d6f58 100644
> --- a/Documentation/criu.txt
> +++ b/Documentation/criu.txt
> @@ -262,6 +262,15 @@ For example, the command line for the above example should look like this:
> engine's default directory for tasks, permissions will not be preserved on
> the upper directories with no tasks in them, which may cause problems.
>
> +*--check-only*::
> + Check if the checkpoint operation will actually succeed. An almost
> + complete checkpoint of the specified processes will be performed, except
> + that memory pages will not be dumped to disk. The checkpoint will be
> + marked as a *check-only* checkpoint and *criu* will detect during restore
> + that not a full restore should be performed but only a *check-only*
> + restore. In addition to not dumping memory pages, the checkpointed process
> + will continue running.
> +
> *restore*
> ~~~~~~~~~
> Restores previously checkpointed processes.
> @@ -366,6 +375,18 @@ The 'mode' may be one of the following:
> *none*::: Ignore capabilities. Most dangerous mode. The behaviour is
> implementation dependent. Try to not use it until really
> required.
> +
> +*--check-only*::
> + Check if the restore operation will actually succeed. If a checkpoint
> + is a *check-only* checkpoint will be automatically detected by *criu*.
> + A *check-only* checkpoint does not include any memory pages in the dump.
> + This option is therefore only required if a normal (not *check-only*)
> + checkpoint should be used to run in *check-only* mode.
> + A restore in *check-only* mode skips copying the memory pages to the
> + restored process but all other steps to verify that the restore would
> + succeed are performed as if it would be a real restore. Just before
> + giving control to the restored process the restore is aborted as the
> + restored process will crash without its memory pages.
> +
> For example, this option can be used in case *--cpu-cap=cpu* was used
> during *dump*, and images are migrated to a less capable CPU and are
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index a323df9..4459b4c 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -1939,14 +1939,14 @@ static int restore_root_task(struct pstree_item *init)
> pr_info("Restore finished successfully. Resuming tasks.\n");
> futex_set_and_wake(&task_entries->start, CR_STATE_COMPLETE);
>
> - if (ret == 0)
> + if (!opts.check_only && ret == 0)
> ret = parasite_stop_on_syscall(task_entries->nr_threads,
> __NR(rt_sigreturn, 0), __NR(rt_sigreturn, 1), flag);
>
> - if (clear_breakpoints())
> + if (!opts.check_only && clear_breakpoints())
> pr_err("Unable to flush breakpoints\n");
>
> - if (ret == 0)
> + if (!opts.check_only && ret == 0)
> finalize_restore();
Does it worth adding a single
if (opts.check_only)
goto somewhere;
statement here?
>
> if (restore_freezer_state())
> @@ -1954,8 +1954,9 @@ static int restore_root_task(struct pstree_item *init)
>
> fini_cgroup();
>
> - /* Detaches from processes and they continue run through sigreturn. */
> - finalize_restore_detach(ret);
> + if (!opts.check_only)
> + /* Detaches from processes and they continue run through sigreturn. */
> + finalize_restore_detach(ret);
>
> write_stats(RESTORE_STATS);
>
> @@ -3033,6 +3034,9 @@ static int sigreturn_restore(pid_t pid, struct task_restore_args *task_args, uns
> task_args->seccomp_mode = core->tc->seccomp_mode;
>
> task_args->compatible_mode = core_is_compat(core);
> +
> + if (opts.check_only)
> + task_args->check_only = true;
> /*
> * Arguments for task restoration.
> */
> diff --git a/criu/crtools.c b/criu/crtools.c
> index 0e853ee..73d152a 100644
> --- a/criu/crtools.c
> +++ b/criu/crtools.c
> @@ -279,6 +279,7 @@ int main(int argc, char *argv[], char *envp[])
> { "cgroup-dump-controller", required_argument, 0, 1082 },
> { SK_INFLIGHT_PARAM, no_argument, 0, 1083 },
> { "deprecated", no_argument, 0, 1084 },
> + { "check-only", no_argument, 0, 1085 },
> { },
> };
>
> @@ -594,6 +595,11 @@ int main(int argc, char *argv[], char *envp[])
> pr_msg("Turn deprecated stuff ON\n");
> opts.deprecated_ok = true;
> break;
> + case 1085:
> + pr_msg("Only checking if requested operation will succeed\n");
> + opts.check_only = true;
> + opts.final_state = TASK_ALIVE;
> + break;
> case 'V':
> pr_msg("Version: %s\n", CRIU_VERSION);
> if (strcmp(CRIU_GITID, "0"))
> @@ -827,6 +833,9 @@ usage:
> " this requires running a second instance of criu\n"
> " in lazy-pages mode: 'criu lazy-pages -D DIR'\n"
> " --lazy-pages and lazy-pages mode require userfaultfd\n"
> +" --check-only check if checkpointing/restoring will actually work\n"
> +" the process will keep on running and memory pages\n"
> +" will not be dumped\n"
> "\n"
> "* External resources support:\n"
> " --external RES dump objects from this list as external resources:\n"
> diff --git a/criu/image.c b/criu/image.c
> index a3bb285..7cee4ce 100644
> --- a/criu/image.c
> +++ b/criu/image.c
> @@ -72,6 +72,12 @@ int check_img_inventory(void)
> goto out_err;
> }
>
> + if (!opts.check_only && he->has_check_only)
> + opts.check_only = he->check_only;
> +
> + if (opts.check_only)
> + pr_msg("Checking mode enabled\n");
> +
> ret = 0;
>
> out_err:
> @@ -114,6 +120,10 @@ int prepare_inventory(InventoryEntry *he)
> he->ns_per_id = true;
> he->has_ns_per_id = true;
> he->lsmtype = host_lsm_type();
> + if (opts.check_only) {
> + he->has_check_only = true;
> + he->check_only = true;
> + }
>
> crt.i.pid.state = TASK_ALIVE;
> crt.i.pid.real = getpid();
> diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
> index 8a9427b..51aee66 100644
> --- a/criu/include/cr_options.h
> +++ b/criu/include/cr_options.h
> @@ -114,6 +114,7 @@ struct cr_options {
> * to turn one ON while the code is in.
> */
> bool deprecated_ok;
> + bool check_only;
> };
>
> extern struct cr_options opts;
> diff --git a/criu/include/restorer.h b/criu/include/restorer.h
> index c74d25f..bfa3e8f 100644
> --- a/criu/include/restorer.h
> +++ b/criu/include/restorer.h
> @@ -177,6 +177,8 @@ struct task_restore_args {
>
> bool compatible_mode;
>
> + bool check_only;
> +
> #ifdef CONFIG_VDSO
> unsigned long vdso_rt_size;
> struct vdso_symtable vdso_sym_rt; /* runtime vdso symbols */
> diff --git a/criu/mem.c b/criu/mem.c
> index 6e6fcc2..2436229 100644
> --- a/criu/mem.c
> +++ b/criu/mem.c
> @@ -269,6 +269,9 @@ static int xfer_pages(struct page_pipe *pp, struct page_xfer *xfer, bool lazy)
> {
> int ret;
>
> + if (opts.check_only)
> + return 0;
> +
Why is this check here? It's present in the top-level routine __parasite_dump_pages_seized(),
which looks to be enough.
> /*
> * Step 3 -- write pages into image (or delay writing for
> * pre-dump action (see pre_dump_one_task)
> @@ -294,6 +297,9 @@ static int __parasite_dump_pages_seized(struct pstree_item *item,
> unsigned cpp_flags = 0;
> unsigned long pmc_size;
>
> + if (opts.check_only)
> + return 0;
> +
> pr_info("\n");
> pr_info("Dumping pages (type: %d pid: %d)\n", CR_FD_PAGES, item->pid.real);
> pr_info("----------------------------------------\n");
> @@ -717,6 +723,9 @@ static int restore_priv_vma_content(struct pstree_item *t)
> unsigned long va;
> struct page_read pr;
>
> + if (opts.check_only)
> + return 0;
> +
> vma = list_first_entry(vmas, struct vma_area, list);
>
> ret = open_page_read(t->pid.virt, &pr, PR_TASK);
> diff --git a/criu/pie/restorer.c b/criu/pie/restorer.c
> index 37e9500..b7741f6 100644
> --- a/criu/pie/restorer.c
> +++ b/criu/pie/restorer.c
> @@ -1237,6 +1237,8 @@ long __export_restore_task(struct task_restore_args *args)
> * Proxify vDSO.
> */
> for (i = 0; i < args->vmas_n; i++) {
> + if (args->check_only)
> + break;
Can we pull this check out of the for () loop?
> if (vma_entry_is(&args->vmas[i], VMA_AREA_VDSO) ||
> vma_entry_is(&args->vmas[i], VMA_AREA_VVAR)) {
> if (vdso_proxify("dumpee", &args->vdso_sym_rt,
> @@ -1525,6 +1527,13 @@ long __export_restore_task(struct task_restore_args *args)
>
> restore_finish_stage(task_entries_local, CR_STATE_RESTORE_CREDS);
>
> + if (args->check_only) {
> + pr_info("Restore check was successful.\n");
> + futex_abort_and_wake(&task_entries_local->nr_in_progress);
> + return 0;
> + }
> +
> +
> if (ret)
> BUG();
>
> diff --git a/images/inventory.proto b/images/inventory.proto
> index f1b01f7..5bc3db2 100644
> --- a/images/inventory.proto
> +++ b/images/inventory.proto
> @@ -15,4 +15,5 @@ message inventory_entry {
> optional bool ns_per_id = 4;
> optional uint32 root_cg_set = 5;
> optional lsmtype lsmtype = 6;
> + optional bool check_only = 7;
> }
>
More information about the CRIU
mailing list