[CRIU] [PATCH 9/7] Added --mix-pre-dump option for zdtm suite

Radostin Stoyanov rstoyanov1 at gmail.com
Sat Aug 31 22:37:28 MSK 2019


On 31/08/2019 13:12, Abhishek Dubey wrote:
> READ and SPLICE mode pre-dumps can happen
> in any order. New option --mix-pre-dump is
> added for zdtm suite only, to test integrity
> of random order of pre-dump modes.
It is important when a test fails to be able to reproduce the error 
consistently. The use of random order for testing might make debugging 
difficult.

> Signed-off-by: Abhishek Dubey <dubeyabhishek777 at gmail.com>
> ---
>   criu/cr-dump.c       |  4 ++--
>   criu/cr-restore.c    |  4 ++--
>   criu/include/stats.h |  4 ++--
>   criu/mem.c           | 65 ++++++++++++++++++++++++++++++++++++++++------------
>   criu/stats.c         | 42 ++++++++++++++++++++++++++++++++-
>   images/stats.proto   |  1 +
>   test/zdtm.py         | 13 ++++++++++-
>   7 files changed, 110 insertions(+), 23 deletions(-)
>
> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
> index b916e0a..f62362f 100644
> --- a/criu/cr-dump.c
> +++ b/criu/cr-dump.c
> @@ -1554,7 +1554,7 @@ err:
>   	if (ret)
>   		pr_err("Pre-dumping FAILED.\n");
>   	else {
> -		write_stats(DUMP_STATS);
> +		write_stats(DUMP_STATS, 1);
>   		pr_info("Pre-dumping finished successfully\n");
>   	}
>   	return ret;
> @@ -1760,7 +1760,7 @@ static int cr_dump_finish(int ret)
>   	if (ret) {
>   		pr_err("Dumping FAILED.\n");
>   	} else {
> -		write_stats(DUMP_STATS);
> +		write_stats(DUMP_STATS, 0);
>   		pr_info("Dumping finished successfully\n");
>   	}
>   	return post_dump_ret ? : (ret != 0);
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index de0b2cb..8ca46e4 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -2226,7 +2226,7 @@ skip_ns_bouncing:
>   	if (ret != 0) {
>   		pr_err("Aborting restore due to post-restore script ret code %d\n", ret);
>   		timing_stop(TIME_RESTORE);
> -		write_stats(RESTORE_STATS);
> +		write_stats(RESTORE_STATS, 0);
>   		goto out_kill;
>   	}
>   
> @@ -2292,7 +2292,7 @@ skip_ns_bouncing:
>   	/* Detaches from processes and they continue run through sigreturn. */
>   	finalize_restore_detach(ret);
>   
> -	write_stats(RESTORE_STATS);
> +	write_stats(RESTORE_STATS, 0);
>   
>   	ret = run_scripts(ACT_POST_RESUME);
>   	if (ret != 0)
> diff --git a/criu/include/stats.h b/criu/include/stats.h
> index 5d408b7..e6f86cb 100644
> --- a/criu/include/stats.h
> +++ b/criu/include/stats.h
> @@ -51,6 +51,6 @@ extern void cnt_sub(int c, unsigned long val);
>   #define RESTORE_STATS	2
>   
>   extern int init_stats(int what);
> -extern void write_stats(int what);
> -
> +extern void write_stats(int what, int pre_dump);
> +extern int get_parent_pre_dump_type();
>   #endif /* __CR_STATS_H__ */
> diff --git a/criu/mem.c b/criu/mem.c
> index 15094f7..0cfa4c2 100644
> --- a/criu/mem.c
> +++ b/criu/mem.c
> @@ -351,7 +351,8 @@ static int generate_vma_iovs(struct pstree_item *item, struct vma_area *vma,
>   			     struct page_pipe *pp, struct page_xfer *xfer,
>   			     struct parasite_dump_pages_args *args,
>   			     struct parasite_ctl *ctl, pmc_t *pmc,
> -			     bool has_parent, bool pre_dump)
> +			     bool has_parent, bool pre_dump,
> +			     int parent_predump_mode)
Should the patch be applied after "[PATCH 2/7] Skip generating iov for 
non PROT_READ region" ?

diff --git a/criu/mem.c b/criu/mem.c
index e19688d..740992d 100644
--- a/criu/mem.c
+++ b/criu/mem.c
@@ -351,7 +351,7 @@ static int generate_vma_iovs(struct pstree_item 
*item, struct vma_area *vma,
                   struct page_pipe *pp, struct page_xfer *xfer,
                   struct parasite_dump_pages_args *args,
                   struct parasite_ctl *ctl, pmc_t *pmc,
-                 bool has_parent, bool pre_dump)
+                 bool has_parent, struct mem_dump_ctl *mdc)


>   {
>   	u64 off = 0;
>   	u64 *map;
> @@ -362,17 +363,47 @@ static int generate_vma_iovs(struct pstree_item *item, struct vma_area *vma,
>   		return 0;
>   
>   	/*
> -	 * process_vm_readv syscall can't copy memory regions lacking
> -	 * PROT_READ flag. Therefore, avoid generating iovs for such
> -	 * regions in "read" mode pre-dump. Regions skipped by pre-dumps
> -	 * can't be referred as parent by following dump stage. So, mark
> -	 * "has_parent=false" for such regions.
> +	 * To facilitate any combination of pre-dump modes to run after
> +	 * one another, we need to take extra care as discussed below.
> +	 *
> +	 * The SPLICE mode pre-dump, processes all type of memory regions,
> +	 * whereas READ mode pre-dump skips processing those memory regions
> +	 * which lacks PROT_READ flag.
> +	 *
> +	 * Now on mixing pre-dump modes:
> +	 * 	If SPLICE mode follows SPLICE mode	: no issue
> +	 *		-> everything dumped both the times
> +	 *
> +	 * 	If READ mode follows READ mode		: no issue
> +	 *		-> non-PROT_READ skipped both the time
> +	 *
> +	 * 	If READ mode follows SPLICE mode   	: no issue
> +	 *		-> everything dumped at first,
> +	 *		   the non-PROT_READ skipped later
> +	 *
> +	 * 	If SPLICE mode follows READ mode   	: Need special care
> +	 *
It would be good to cover these 4 cases with a test.

> +	 * If READ pre-dump happens first then it has skipped processing
> +	 * non-PROT_READ regions. Following SPLICE pre-dump expects pagemap
> +	 * entries for all mappings in parent pagemap, but last READ mode
> +	 * pre-dump has skipped processing and pagemap generation for
> +	 * non-PROT_READ regions. So SPLICE mode throws error of missing
> +	 * pagemap entry for non-PROT_READ mapping.
> +	 *
> +	 * To resolve this, the mode of pre-dump is stored in pre-dump's
> +	 * stats file. This mode is read back from stats file during next
> +	 * pre-dump.
The purpose of the stats file is to contain statistics about how 
dump/restore went. IMHO the data in this file should not be required for 
pre-dump.

> +	 * If parent-pre-dump and next pre-dump turns out in READ-mode -->
> +	 * SPLICE-mode order, then SPLICE mode doesn't expect mappings for
> +	 * non-PROT_READ regions in parent-image and marks "has_parent=false".
>   	 */
> -	if (opts.pre_dump_mode == PRE_DUMP_READ &&
> -	                          !(vma->e->prot & PROT_READ)) {
> -		if (pre_dump)
> +
> +	if (!(vma->e->prot & PROT_READ)) {
> +		if (opts.pre_dump_mode == PRE_DUMP_READ && pre_dump)
>   			return 0;
> -		has_parent = false;
> +		if ((parent_predump_mode == PRE_DUMP_READ &&
> +			opts.pre_dump_mode == PRE_DUMP_SPLICE) || !pre_dump)
> +			has_parent = false;
>   	}
>   
This becomes complicated because of the fix mentioned in 
https://lists.openvz.org/pipermail/criu/2019-August/044404.html

Would it be possible to send version 2 for the whole series?

You can use git rebase to apply necessary changes in previous commits.

Radostin




More information about the CRIU mailing list