[CRIU] [PATCH 9/7] Added --mix-pre-dump option for zdtm suite
abhishek dubey
dubeyabhishek777 at gmail.com
Sat Sep 7 15:17:15 MSK 2019
Hi Radostin,
With new changes, pre-dump-mode is dumped in inventory file.
https://github.com/dubeyabhishek/ultimateCRIU/commit/961c08a5cfa4ac59e64677c5e1c14c515ef844da
If the changes are good, I will prepare rev2 of patch-series.
On 05/09/19 9:15 AM, Radostin Stoyanov wrote:
> On 02/09/2019 07:19, abhishek dubey wrote:
>> On 01/09/19 1:07 AM, Radostin Stoyanov wrote:
>>>
>>> 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.
>> Sure. I will remove random to preserve consistency.
>>>
>>>> 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-dhttps://github.com/dubeyabhishek/ultimateCRIU/commit/961c08a5cfa4ac59e64677c5e1c14c515ef844daump
>>>> 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.
>> Random order will be replaced by these 4 cases.
>>>
>>>> + * 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 we don't mix splice and read modes, then we can completely avoid
>> this whole stat-file writing/reading. But, if we mix pre-dump-modes
>> in single run then we need
>>
>> to tell splice-pre-dump, about what has been skipped in parent images
>> through read-pre-dumps.
>>
> I understand that we need to include information about what pre-dump
> mode was used but I think that stats-dump is not the right image for
> this.
>
> Can we use inventory.img instead?
>
> Radostin
-Abhishek
More information about the CRIU
mailing list