Re: [Qemu-devel] [PATCH v1 2/2] migration: add bitmap for copied page

[Juan Quintela <quintela@redhat.com>]

Alexey Perevalov <a.perevalov@samsung.com> wrote:
> On 06/13/2017 02:42 PM, Juan Quintela wrote:
>> Alexey Perevalov <a.perevalov@samsung.com> wrote:
>>
>> Hi
>>
>> I think that it would make things clearer if we do a s/copied/received/
>> As what we are tracking here are the pages that have already been
>> received.
>>
>>
>>> This patch adds ability to track down already copied
>>> pages, it's necessary for calculation vCPU block time in
>>> postcopy migration feature, maybe for restore after
>>> postcopy migration failure.
>>> Also it's necessary to solve shared memory issue in
>>> postcopy livemigration. Information about copied pages
>>> will be transferred to the software virtual bridge
>>> (e.g. OVS-VSWITCHD), to avoid fallocate (unmap) for
>>> already copied pages. fallocate syscall is required for
>>> remmaped shared memory, due to remmaping itself blocks
>>> ioctl(UFFDIO_COPY, ioctl in this case will end with EEXIT
>>> error (struct page is exists after remmap).
>>>
>>> Bitmap is placed into RAMBlock as another postcopy/precopy
>>> related bitmaps.
>> Why are we not using the TARGET_PAGE_SIZE as units of the bitmap?
>>
>>
>>> copied bitmap is not releasing due to ramblocks is not releasing
>>> too.
>> RAMBlocks are used for other reasons, not only migration.  This bitmaps
>> can be released on the ram_load_cleanup() function.  See my patches on
>> list about how to use it.
> I saw patch "migration: Convert ram to use new load_setup()/load_cleanup()",
> second version
>
> ram_load_cleanup - IIUC, calls as load_cleanup callback, from 2 use cases:
> 	1. QEMU_OPTION_loadvm
> 	2. hmp_loadvm
>
> in both cases: load_snapshot, qemu_loadvm_state,
> qemu_loadvm_state_cleanup, load_cleanup callback

Hi

Are you looking at v2?

On my tree:

int qemu_loadvm_state(QEMUFile *f)
{

....

    qemu_loadvm_state_cleanup();
    cpu_synchronize_all_post_init();

    return ret;
}

And everything that counts call qemu_loadvm_state() to load the state,
no?

Later, Juan.