[CRIU] GSoC: Memory mapping collection
abhishek dubey
dubeyabhishek777 at gmail.com
Thu Jun 20 01:00:51 MSK 2019
Hi,
In the past week, I have been working on latest code to incorporate
Andrei's attempt for using userspace buffer.
Additional changes:
- skipping iovs generation for READ protected areas
- skipping parasite injection (working on it)
Debugging:
After implementing Andrei's changes and running zdtm suite, 28 test
cases are failing. I am resolving this mismatch issue.
Error log of failed test:
=== Run 324/362 ==============-- zdtm/static/different_creds
===================== Run zdtm/static/different_creds in h
=====================
Start test
Test is SUID
./different_creds --pidfile=different_creds.pid
--outfile=different_creds.out
Run criu dump
=[log]=> dump/zdtm/static/different_creds/38/1/dump.log
------------------------ grep Error ------------------------
(00.007650) Fetched ack: 66 66 0
(00.007655) Transferring pages:
*(00.007658) buf 2/1**
**(00.007669) Error (criu/page-xfer.c:498): Can't splice (4096(8192)/2)*
(00.007687) page-pipe: Killing page pipe
(00.007727) ----------------------------------------
(00.007732) Error (criu/mem.c:547): Can't dump page with parasite
(00.008371) 38 was stopped
(00.008487) Unlock network
(00.008493) Unfreezing tasks into 1
(00.008497) Unseizing 38 into 1
(00.008510) Error (criu/cr-dump.c:1753): Dumping FAILED.
------------------------ ERROR OVER ------------------------
############## Test zdtm/static/different_creds FAIL at CRIU dump
##############
Send the 9 signal to 38
Wait for zdtm/static/different_creds(38) to die for 0.100000
Queries:
1) Whether zdtm test suite cover test cases to check functionality of
pre-dump? Is there a particular test case which
needs to be passed for pre-dump functionality verification?
2) Currently I am following steps below to check if pre-dump works fine
(since some test cases which fails in zdtm suite passes when run this way):
- make <testcase_name>.pid
- predump <testcase>
- dump <testcase> pointing to predump images
- restore <testcase>
- make <testcase_name>.out
look for PASS message in last step.
-Abhishek
On 12/06/19 12:19 AM, Andrei Vagin wrote:
> I think transition tests have not been compiled. zdtm.py searches all
> binaries and run them.
>
> Could you try to run make -C test/zdtm and then try to run zdtm.py run -a again.
>
> Thanks,
> Andrei
>
> On Tue, Jun 11, 2019 at 11:24 AM Abhishek Dubey
> <dubeyabhishek777 at gmail.com> wrote:
>> Please see attachment for Complete Console output.
>>
>> -Abhishek
>>
>> On Tue, Jun 11, 2019 at 11:52 PM Abhishek Dubey <dubeyabhishek777 at gmail.com> wrote:
>>> Summary of test/zdtm.py run -a --ignore-taint --keep-going is
>>> ################## ALL TEST(S) PASSED (TOTAL 204/SKIPPED 21) ###################
>>>
>>> Sending complete console output separately.
>>>
>>> On Mon, Jun 10, 2019 at 10:50 PM Andrei Vagin <avagin at gmail.com> wrote:
>>>> On Mon, Jun 10, 2019 at 2:50 AM abhishek dubey
>>>> <dubeyabhishek777 at gmail.com> wrote:
>>>>>
>>>>> On 10/06/19 12:59 PM, Andrei Vagin (C) wrote:
>>>>>> On Wed, Jun 05, 2019 at 12:49:03PM +0000, Pavel Emelianov wrote:
>>>>>>> On 6/4/19 7:50 PM, Abhishek Dubey wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On running "zdtm.py run -a", testcases of zdtm/static directory are checked and that of zdtm/transition are skipped. Do all testcases of transition directory need manual run?
>>>>>>> Yes.
>>>>>> I don't understand what you mean here.
>>>>>>
>>>>>> https://ci.openvz.org/job/CRIU/job/CRIU-x86_64/job/criu-dev/5439/console
>>>>>>
>>>>>> Here we can see that all trasition tests were checked...
>>>>> Here I refer to locally running zdtm tests as given on
>>>>> https://www.criu.org/ZDTM_test_suite
>>>> Could you show output for zdtm.py run -a?
>>>>
>>>>>>>> What is specific about transition testcases?
>>>>>>> They take time to accomplish, so the default run is limited.
>>>>>>>
>>>>>>>> For the first step of fetching VMAs between freeze and unfreeze of pre-dump:
>>>>>>>> - To maintain correspondence between PID and collected VMA, added new entry in struct pstree_item
>>>>>>>> - Moved out unfreeze code from cr_pre_dump_finish()
>>>>>>>> - Moved out collect_mappings() from pre_dump_one_task
>>>>>>>>
>>>>>>>> I have tested these changes with env00-basic example and zdtm.py test-suite. All test-cases under zdtm/static directory are Passing. Remaining transition test cases are skipped.
>>>>>>>>
>>>>>>>> Share your thoughts on this approach. I am attaching patch-file for reference.
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On running "zdtm.py run -a", testcases of zdtm/static directory are checked and that of zdtm/transition are skipped. Do all testcases of transition directory need manual run?
>>>>>>>> What is specific about transition testcases?
>>>>>>>>
>>>>>>>> For the first step of fetching VMAs between freeze and unfreeze of pre-dump:
>>>>>>>> - To maintain correspondence between PID and collected VMA, added new entry in struct pstree_item
>>>>>>>> - Moved out unfreeze code from cr_pre_dump_finish()
>>>>>>>> - Moved out collect_mappings() from pre_dump_one_task
>>>>>>> Please, describe why you need this. Current code does "fetching vmas between freeze and unfreeze",
>>>>>>> what's wrong with it so you need this change?
>>>>>>>
>>>>>>>> I have tested these changes with env00-basic example and zdtm.py test-suite. All test-cases under zdtm/static directory are Passing. Remaining transition test cases are skipped.
>>>>>>>>
>>>>>>>> Share your thoughts on this approach. I am attaching patch-file for reference.
>>>>>>>>
>>>>>>>> -Abhishek
>>>>>>>>
>>>>>>>> 0001-freezing-target-task-to-collect-memory-mappings-foll.patch
>>>>>>>>
>>>>>>>> From 5b131559ce20ebbdb8a04be2cced321b55d864f4 Mon Sep 17 00:00:00 2001
>>>>>>> Please mind the https://criu.org/How_to_submit_patches article. Patch should go inline, not attached,
>>>>>>> and properly formatted. Also comments are inline.
>>>>>>>
>>>>>>>> From: abhidubey4178 <abhishekdubey4178940 at gmail.com>
>>>>>>>> Date: Tue, 4 Jun 2019 19:59:21 +0530
>>>>>>>> Subject: [PATCH] freezing target task to collect memory mappings, followed by
>>>>>>>> unfreeze
>>>>>>>>
>>>>>>>> ---
>>>>>>>> criu/cr-dump.c | 103 +++++++++++++++++++++++++++++++-----------
>>>>>>>> criu/include/pstree.h | 1 +
>>>>>>>> 2 files changed, 78 insertions(+), 26 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
>>>>>>>> index 7f2e5edf..40bc7594 100644
>>>>>>>> --- a/criu/cr-dump.c
>>>>>>>> +++ b/criu/cr-dump.c
>>>>>>>> @@ -1154,20 +1154,19 @@ static int pre_dump_one_task(struct pstree_item *item, InventoryEntry *parent_ie
>>>>>>>> if (item->pid->state == TASK_DEAD)
>>>>>>>> return 0;
>>>>>>>>
>>>>>>>> - ret = collect_mappings(pid, &vmas, NULL);
>>>>>>> Up above there have been left the now unused vmas variable.
>>>>>>>
>>>>>>>> - if (ret) {
>>>>>>>> + if (!item->vmas) {
>>>>>>>> pr_err("Collect mappings (pid: %d) failed with %d\n", pid, ret);
>>>>>>>> goto err;
>>>>>>>> }
>>>>>>>>
>>>>>>>> ret = -1;
>>>>>>>> - parasite_ctl = parasite_infect_seized(pid, item, &vmas);
>>>>>>>> + parasite_ctl = parasite_infect_seized(pid, item, item->vmas);
>>>>>>>> if (!parasite_ctl) {
>>>>>>>> pr_err("Can't infect (pid: %d) with parasite\n", pid);
>>>>>>>> goto err_free;
>>>>>>>> }
>>>>>>>>
>>>>>>>> - ret = parasite_fixup_vdso(parasite_ctl, pid, &vmas);
>>>>>>>> + ret = parasite_fixup_vdso(parasite_ctl, pid, item->vmas);
>>>>>>>> if (ret) {
>>>>>>>> pr_err("Can't fixup vdso VMAs (pid: %d)\n", pid);
>>>>>>>> goto err_cure;
>>>>>>>> @@ -1192,14 +1191,14 @@ static int pre_dump_one_task(struct pstree_item *item, InventoryEntry *parent_ie
>>>>>>>> mdc.stat = NULL;
>>>>>>>> mdc.parent_ie = parent_ie;
>>>>>>>>
>>>>>>>> - ret = parasite_dump_pages_seized(item, &vmas, &mdc, parasite_ctl);
>>>>>>>> + ret = parasite_dump_pages_seized(item, item->vmas, &mdc, parasite_ctl);
>>>>>>>> if (ret)
>>>>>>>> goto err_cure;
>>>>>>>>
>>>>>>>> if (compel_cure_remote(parasite_ctl))
>>>>>>>> pr_err("Can't cure (pid: %d) from parasite\n", pid);
>>>>>>>> err_free:
>>>>>>>> - free_mappings(&vmas);
>>>>>>>> + free_mappings(item->vmas);
>>>>>>>> err:
>>>>>>>> return ret;
>>>>>>>>
>>>>>>>> @@ -1471,26 +1470,9 @@ static int setup_alarm_handler()
>>>>>>>>
>>>>>>>> static int cr_pre_dump_finish(int status)
>>>>>>>> {
>>>>>>>> - InventoryEntry he = INVENTORY_ENTRY__INIT;
>>>>>>>> struct pstree_item *item;
>>>>>>>> int ret;
>>>>>>>>
>>>>>>>> - /*
>>>>>>>> - * Restore registers for tasks only. The threads have not been
>>>>>>>> - * infected. Therefore, the thread register sets have not been changed.
>>>>>>>> - */
>>>>>>>> - ret = arch_set_thread_regs(root_item, false);
>>>>>>>> - if (ret)
>>>>>>>> - goto err;
>>>>>>>> -
>>>>>>>> - ret = inventory_save_uptime(&he);
>>>>>>>> - if (ret)
>>>>>>>> - goto err;
>>>>>>>> -
>>>>>>>> - pstree_switch_state(root_item, TASK_ALIVE);
>>>>>>>> -
>>>>>>>> - timing_stop(TIME_FROZEN);
>>>>>>>> -
>>>>>>>> if (status < 0) {
>>>>>>>> ret = status;
>>>>>>>> goto err;
>>>>>>>> @@ -1540,9 +1522,6 @@ err:
>>>>>>>> if (bfd_flush_images())
>>>>>>>> ret = -1;
>>>>>>>>
>>>>>>>> - if (write_img_inventory(&he))
>>>>>>>> - ret = -1;
>>>>>>>> -
>>>>>>> You write inventory too early, I'm not 100% sure this is correct.
>>>>>>>
>>>>>>>> if (ret)
>>>>>>>> pr_err("Pre-dumping FAILED.\n");
>>>>>>>> else {
>>>>>>>> @@ -1552,6 +1531,68 @@ err:
>>>>>>>> return ret;
>>>>>>>> }
>>>>>>>>
>>>>>>>> +
>>>>>>>> +static int pre_dump_collect_vmas(struct pstree_item *item)
>>>>>>>> +{
>>>>>>>> + pid_t pid = item->pid->real;
>>>>>>>> + struct vm_area_list *vmas;
>>>>>>>> + int ret = -1;
>>>>>>>> +
>>>>>>>> + vmas = xmalloc(sizeof(struct vm_area_list));
>>>>>>> This memory is never free-d.
>>>>>>>
>>>>>>>> + if(!vmas)
>>>>>>>> + return -1;
>>>>>>>> + INIT_LIST_HEAD(&vmas->h);
>>>>>>>> + vmas->nr = 0;
>>>>>>>> +
>>>>>>>> + pr_info("========================================\n");
>>>>>>>> + pr_info("Collecting VMAs of (pid: %d)\n", pid);
>>>>>>>> + pr_info("========================================\n");
>>>>>>>> +
>>>>>>>> + if (item->pid->state == TASK_STOPPED) {
>>>>>>>> + pr_warn("Stopped tasks are not supported\n");
>>>>>>>> + return 0;
>>>>>>>> + }
>>>>>>> Formatting.
>>>>>>>
>>>>>>>> +
>>>>>>>> + if (item->pid->state == TASK_DEAD)
>>>>>>>> + return 0;
>>>>>>> Vmas allocation can happen here.
>>>>>>>
>>>>>>>> +
>>>>>>>> + ret = collect_mappings(pid, vmas, NULL);
>>>>>>>> + if (ret) {
>>>>>>>> + pr_err("Collect mappings (pid: %d) failed with %d\n", pid, ret);
>>>>>>>> + return -1;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + item->vmas = vmas;
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +
>>>>>>>> +static int unfreeze_pstree()
>>>>>>>> +{
>>>>>>>> + InventoryEntry he = INVENTORY_ENTRY__INIT;
>>>>>>>> + int ret;
>>>>>>>> +
>>>>>>>> + /*
>>>>>>>> + * Restore registers for tasks only. The threads have not been
>>>>>>>> + * infected. Therefore, the thread register sets have not been changed.
>>>>>>>> + */
>>>>>>>> + ret = arch_set_thread_regs(root_item, false);
>>>>>>>> + if (ret)
>>>>>>>> + return -1;
>>>>>>>> +
>>>>>>>> + ret = inventory_save_uptime(&he);
>>>>>>>> + if (ret)
>>>>>>>> + return -1;
>>>>>>>> +
>>>>>>>> + if (write_img_inventory(&he))
>>>>>>>> + return -1;
>>>>>>>> +
>>>>>>>> + pstree_switch_state(root_item, TASK_ALIVE);
>>>>>>>> + timing_stop(TIME_FROZEN);
>>>>>>>> +
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> int cr_pre_dump_tasks(pid_t pid)
>>>>>>>> {
>>>>>>>> InventoryEntry *parent_ie = NULL;
>>>>>>>> @@ -1619,6 +1660,16 @@ int cr_pre_dump_tasks(pid_t pid)
>>>>>>>> /* Errors handled later in detect_pid_reuse */
>>>>>>>> parent_ie = get_parent_inventory();
>>>>>>>>
>>>>>>>> + /* Collecting VMAs of all processes in pstree */
>>>>>>>> + for_each_pstree_item(item)
>>>>>>>> + if (pre_dump_collect_vmas(item))
>>>>>>>> + goto err;
>>>>>>>> +
>>>>>>>> + /* Unfreeze pstree */
>>>>>>> This is pointless comment as it 100% repeats the subsequent function name :)
>>>>>>>
>>>>>>>> + ret = unfreeze_pstree();
>>>>>>>> + if(ret)
>>>>>>>> + goto err;
>>>>>>>> +
>>>>>>>> for_each_pstree_item(item)
>>>>>>>> if (pre_dump_one_task(item, parent_ie))
>>>>>>>> goto err;
>>>>>>>> diff --git a/criu/include/pstree.h b/criu/include/pstree.h
>>>>>>>> index 7303c1fe..69562608 100644
>>>>>>>> --- a/criu/include/pstree.h
>>>>>>>> +++ b/criu/include/pstree.h
>>>>>>>> @@ -30,6 +30,7 @@ struct pstree_item {
>>>>>>>> futex_t task_st;
>>>>>>>> unsigned long task_st_le_bits;
>>>>>>>> };
>>>>>>>> + struct vm_area_list *vmas;
>>>>>>> This is dump-only field, it should be placed on dmp_info instead.
>>>>>>>
>>>>>>>> };
>>>>>>>>
>>>>>>>> static inline pid_t vpid(const struct pstree_item *i)
>>>>>>>> --
>>>>>>>> 2.17.1
>>>>>>>>
>>>>>>> -- Pavel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/criu/attachments/20190620/c1ac08d6/attachment-0001.html>
More information about the CRIU
mailing list