Peter Xu <pet...@redhat.com> writes:
>>
>> * pc - refers to the page_size/mr->addr members, so newly added members
>> begin from "bitmap_size".
>
> Could you elaborate more on what's the pc?
>
> I also didn't see this *pc in below migration.rst update.
>
Yeah, you need to be looking at the code to figure that one out. That
was intended to reference some postcopy data that is (already) inserted
into the stream. Literally this:
if (migrate_postcopy_ram() && block->page_size !=
qemu_host_page_size) {
qemu_put_be64(f, block->page_size);
}
if (migrate_ignore_shared()) {
qemu_put_be64(f, block->mr->addr);
}
It has nothing to do with this patch. I need to rewrite that part of the
commit message a bit.
>>
>> This layout is initialized during ram_save_setup so instead of having a
>> sequential stream of pages that follow the ramblock headers the dirty
>> pages for a ramblock follow its header. Since all pages have a fixed
>> location RAM_SAVE_FLAG_EOS is no longer generated on every migration
>> iteration but there is effectively a single RAM_SAVE_FLAG_EOS right at
>> the end.
>>
>> Signed-off-by: Nikolay Borisov <nbori...@suse.com>
>> Signed-off-by: Fabiano Rosas <faro...@suse.de>
...
>> @@ -4390,6 +4432,12 @@ void migrate_fd_connect(MigrationState *s, Error
>> *error_in)
>> }
>> }
>>
>> + if (migrate_check_fixed_ram(s, &local_err) < 0) {
>
> This check might be too late afaict, QMP cmd "migrate" could have already
> succeeded.
>
> Can we do an early check in / close to qmp_migrate()? The idea is we fail
> at the QMP migrate command there.
>
Yes, some of it depends on the QEMUFile being known but I can at least
move part of the verification earlier.
>> + migrate_fd_cleanup(s);
>> + migrate_fd_error(s, local_err);
>> + return;
>> + }
>> +
>> if (resume) {
>> /* Wakeup the main migration thread to do the recovery */
>> migrate_set_state(&s->state, MIGRATION_STATUS_POSTCOPY_PAUSED,
>> @@ -4519,6 +4567,7 @@ static Property migration_properties[] = {
>> DEFINE_PROP_STRING("tls-authz", MigrationState, parameters.tls_authz),
>>
>> /* Migration capabilities */
>> + DEFINE_PROP_MIG_CAP("x-fixed-ram", MIGRATION_CAPABILITY_FIXED_RAM),
>> DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
>> DEFINE_PROP_MIG_CAP("x-rdma-pin-all",
>> MIGRATION_CAPABILITY_RDMA_PIN_ALL),
>> DEFINE_PROP_MIG_CAP("x-auto-converge",
>> MIGRATION_CAPABILITY_AUTO_CONVERGE),
>> diff --git a/migration/migration.h b/migration/migration.h
>> index 2da2f8a164..8cf3caecfe 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -416,6 +416,7 @@ bool migrate_zero_blocks(void);
>> bool migrate_dirty_bitmaps(void);
>> bool migrate_ignore_shared(void);
>> bool migrate_validate_uuid(void);
>> +int migrate_fixed_ram(void);
>>
>> bool migrate_auto_converge(void);
>> bool migrate_use_multifd(void);
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 96e8a19a58..56f0f782c8 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -1310,9 +1310,14 @@ static int save_zero_page_to_file(PageSearchStatus
>> *pss,
>> int len = 0;
>>
>> if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
>> - len += save_page_header(pss, block, offset | RAM_SAVE_FLAG_ZERO);
>> - qemu_put_byte(file, 0);
>> - len += 1;
>> + if (migrate_fixed_ram()) {
>> + /* for zero pages we don't need to do anything */
>> + len = 1;
>
> I think you wanted to increase the "duplicated" counter, but this will also
> increase ram-transferred even though only 1 byte.
>
Ah, well spotted, that is indeed incorrect.
> Perhaps just pass a pointer to keep the bytes, and return true/false to
> increase the counter (to make everything accurate)?
>
Ok
>> + } else {
>> + len += save_page_header(pss, block, offset |
>> RAM_SAVE_FLAG_ZERO);
>> + qemu_put_byte(file, 0);
>> + len += 1;
>> + }
>> ram_release_page(block->idstr, offset);
>> }
>> return len;
