Re: [Qemu-devel] [PATCH v9 10/12] migration: Transfer pages over new channels

[Juan Quintela <quintela@redhat.com>]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> We switch for sending the page number to send real pages.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> I think this is OK if squashed with the 'test' patch to remove
> the test stuff.

Done.

>
> Some minor comments below.
>
>> --
>> 
>> Remove the HACK bit, now we have the function that calculates the size
>> of a page exported.
>> Rename multifd_pages{_now}, to sent pages
>> Remove multifd pages field, it is the same than normal pages
>> ---
>>  migration/migration.c |  7 ++++++-
>>  migration/ram.c       | 39 +++++++++++----------------------------
>>  2 files changed, 17 insertions(+), 29 deletions(-)
>> 
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 54ef095d82..1bd87a4e44 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -2085,6 +2085,7 @@ static void *migration_thread(void *opaque)
>>       */
>>      int64_t threshold_size = 0;
>>      int64_t qemu_file_bytes = 0;
>> +    int64_t sent_pages = 0;
>>      int64_t start_time = initial_time;
>>      int64_t end_time;
>>      bool old_vm_running = false;
>> @@ -2173,8 +2174,11 @@ static void *migration_thread(void *opaque)
>>          current_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>>          if (current_time >= initial_time + BUFFER_DELAY) {
>>              uint64_t qemu_file_bytes_now = qemu_ftell(s->to_dst_file);
>> +            uint64_t sent_pages_now = ram_counters.normal;
>>              uint64_t transferred_bytes =
>> -                qemu_file_bytes_now - qemu_file_bytes;
>> +                (qemu_file_bytes_now - qemu_file_bytes) +
>> +                (sent_pages_now - sent_pages) *
>> +                qemu_target_page_size();
>
> This could do with commenting to explain the difference between the
> two sets of counts.

Rework it to make clear that multifd data is not sent through qemu file.

>> @@ -1288,8 +1270,10 @@ static int ram_multifd_page(RAMState *rs, PageSearchStatus *pss,
>>                               offset | RAM_SAVE_FLAG_MULTIFD_PAGE);
>>          fd_num = multifd_send_page(p, rs->migration_dirty_pages == 1);
>>          qemu_put_be16(rs->f, fd_num);
>> +        if (fd_num != MULTIFD_CONTINUE) {
>> +            qemu_fflush(rs->f);
>> +        }
>
> Could do with a comment.

Done.

Later, Juan.