Hi

This are the changes for migration that are already reviewed.

Please, apply.

Later, Juan.


The following changes since commit 281f327487c9c9b1599f93c589a408bbf4a651b8:

  Merge remote-tracking branch 'remotes/vivier/tags/m68k-for-2.12-pull-request' into staging (2017-12-22 00:11:36 +0000)

are available in the Git repository at:

  git://github.com/juanquintela/qemu.git tags/migration/20180103

for you to fetch changes up to 8a18afdcd8d2d6ab31f9de89d2f20fdadb89beb8:

  migration: finalize current_migration object (2018-01-03 09:28:56 +0100)

----------------------------------------------------------------
migration/next for 20180103

----------------------------------------------------------------
Alexey Perevalov (6):
      migration: introduce postcopy-blocktime capability
      migration: add postcopy blocktime ctx into MigrationIncomingState
      migration: calculate vCPU blocktime on dst side
      migration: postcopy_blocktime documentation
      migration: add blocktime calculation into migration-test
      migration: add postcopy total blocktime into query-migrate

Dr. David Alan Gilbert (2):
      docs: Convert migration.txt to rst
      migration: Guard ram_bytes_remaining against early call

Juan Quintela (4):
      migration: Use proper types in json
      migration: print features as on off
      migration: free addr in the same function that we created it
      migration: free result string

Laurent Vivier (1):
      migration: fix analyze-migration.py script with radix table

Vladimir Sementsov-Ogievskiy (1):
      migration: finalize current_migration object

 docs/devel/{migration.txt => migration.rst} | 484 +++++++++++++++-------------
 hmp.c                                       |  37 ++-
 include/migration/misc.h                    |   1 +
 migration/migration.c                       | 118 ++++---
 migration/migration.h                       |  13 +
 migration/postcopy-ram.c                    | 258 ++++++++++++++-
 migration/ram.c                             |   3 +-
 migration/socket.c                          |   4 +-
 migration/trace-events                      |   6 +-
 qapi/migration.json                         |  37 ++-
 scripts/analyze-migration.py                |   4 +
 tests/migration-test.c                      |  19 +-
 vl.c                                        |   1 +
 13 files changed, 696 insertions(+), 289 deletions(-)
 rename docs/devel/{migration.txt => migration.rst} (58%)

On 01/03/2018 03:38 AM, Juan Quintela wrote:
> Hi
> 
> This are the changes for migration that are already reviewed.
> 
> Please, apply.
> 

> Alexey Perevalov (6):
>       migration: introduce postcopy-blocktime capability
>       migration: add postcopy blocktime ctx into MigrationIncomingState
>       migration: calculate vCPU blocktime on dst side
>       migration: postcopy_blocktime documentation
>       migration: add blocktime calculation into migration-test
>       migration: add postcopy total blocktime into query-migrate

I had unanswered questions about these patches in the v12 series, where
I'm not sure if the interface is still quite right.  We're still early
enough that we could adjust the interface after the fact depending on
how the questions are answered; but we're also early enough that it may
be smarter to get the interface right before including it in a pull
request.  I'll leave it to Peter and Juan to sort out whether this means
an updated pull request is needed, or to take this as-is.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

* Eric Blake (eblake@redhat.com) wrote:
> On 01/03/2018 03:38 AM, Juan Quintela wrote:
> > Hi
> > 
> > This are the changes for migration that are already reviewed.
> > 
> > Please, apply.
> > 
> 
> > Alexey Perevalov (6):
> >       migration: introduce postcopy-blocktime capability
> >       migration: add postcopy blocktime ctx into MigrationIncomingState
> >       migration: calculate vCPU blocktime on dst side
> >       migration: postcopy_blocktime documentation
> >       migration: add blocktime calculation into migration-test
> >       migration: add postcopy total blocktime into query-migrate
> 
> I had unanswered questions about these patches in the v12 series, where
> I'm not sure if the interface is still quite right.  We're still early
> enough that we could adjust the interface after the fact depending on
> how the questions are answered; but we're also early enough that it may
> be smarter to get the interface right before including it in a pull
> request.  I'll leave it to Peter and Juan to sort out whether this means
> an updated pull request is needed, or to take this as-is.

Hadn't that discussion been done to death about 10 times during the
(long) life of this series?

Dave

> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 



--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Eric Blake <eblake@redhat.com> wrote:
> On 01/03/2018 03:38 AM, Juan Quintela wrote:
>> Hi
>> 
>> This are the changes for migration that are already reviewed.
>> 
>> Please, apply.
>> 
>
>> Alexey Perevalov (6):
>>       migration: introduce postcopy-blocktime capability
>>       migration: add postcopy blocktime ctx into MigrationIncomingState
>>       migration: calculate vCPU blocktime on dst side
>>       migration: postcopy_blocktime documentation
>>       migration: add blocktime calculation into migration-test
>>       migration: add postcopy total blocktime into query-migrate
>
> I had unanswered questions about these patches in the v12 series, where
> I'm not sure if the interface is still quite right.

To be fair, I had alroady integrated the patches before I saw your questions.

> We're still early
> enough that we could adjust the interface after the fact depending on
> how the questions are answered;

I think this is the best approach, so far I can see two questions:

- do we want to make it conditional?  it requires some locking, but I
  haven't meassured it to see how slow/fast is it.

- the other was documentation.

I will like Alexey to answer.  Depending of how slow it is, I can agree
to make it non-optional.

> but we're also early enough that it may
> be smarter to get the interface right before including it in a pull
> request.  I'll leave it to Peter and Juan to sort out whether this means
> an updated pull request is needed, or to take this as-is.

Thanks, Juan.

On 5 January 2018 at 09:59, Juan Quintela <quintela@redhat.com> wrote:
> Eric Blake <eblake@redhat.com> wrote:
>> On 01/03/2018 03:38 AM, Juan Quintela wrote:
>>> Hi
>>>
>>> This are the changes for migration that are already reviewed.
>>>
>>> Please, apply.
>>>
>>
>>> Alexey Perevalov (6):
>>>       migration: introduce postcopy-blocktime capability
>>>       migration: add postcopy blocktime ctx into MigrationIncomingState
>>>       migration: calculate vCPU blocktime on dst side
>>>       migration: postcopy_blocktime documentation
>>>       migration: add blocktime calculation into migration-test
>>>       migration: add postcopy total blocktime into query-migrate
>>
>> I had unanswered questions about these patches in the v12 series, where
>> I'm not sure if the interface is still quite right.
>
> To be fair, I had alroady integrated the patches before I saw your questions.
>
>> We're still early
>> enough that we could adjust the interface after the fact depending on
>> how the questions are answered;
>
> I think this is the best approach, so far I can see two questions:
>
> - do we want to make it conditional?  it requires some locking, but I
>   haven't meassured it to see how slow/fast is it.
>
> - the other was documentation.
>
> I will like Alexey to answer.  Depending of how slow it is, I can agree
> to make it non-optional.

So have you come to a consensus on whether you'd like me to apply
this pull request or not ?

thanks
-- PMM

Peter Maydell <peter.maydell@linaro.org> wrote:
> On 5 January 2018 at 09:59, Juan Quintela <quintela@redhat.com> wrote:
>> Eric Blake <eblake@redhat.com> wrote:
>>> On 01/03/2018 03:38 AM, Juan Quintela wrote:
>>>> Hi
>>>>
>>>> This are the changes for migration that are already reviewed.
>>>>
>>>> Please, apply.
>>>>
>>>
>>>> Alexey Perevalov (6):
>>>>       migration: introduce postcopy-blocktime capability
>>>>       migration: add postcopy blocktime ctx into MigrationIncomingState
>>>>       migration: calculate vCPU blocktime on dst side
>>>>       migration: postcopy_blocktime documentation
>>>>       migration: add blocktime calculation into migration-test
>>>>       migration: add postcopy total blocktime into query-migrate
>>>
>>> I had unanswered questions about these patches in the v12 series, where
>>> I'm not sure if the interface is still quite right.
>>
>> To be fair, I had alroady integrated the patches before I saw your questions.
>>
>>> We're still early
>>> enough that we could adjust the interface after the fact depending on
>>> how the questions are answered;
>>
>> I think this is the best approach, so far I can see two questions:
>>
>> - do we want to make it conditional?  it requires some locking, but I
>>   haven't meassured it to see how slow/fast is it.
>>
>> - the other was documentation.
>>
>> I will like Alexey to answer.  Depending of how slow it is, I can agree
>> to make it non-optional.
>
> So have you come to a consensus on whether you'd like me to apply
> this pull request or not ?

My understanding is that you should apply it.  We will add a
documentation patch later.

Later, Juan.

On 01/05/2018 12:59 PM, Juan Quintela wrote:
> Eric Blake <eblake@redhat.com> wrote:
>> On 01/03/2018 03:38 AM, Juan Quintela wrote:
>>> Hi
>>>
>>> This are the changes for migration that are already reviewed.
>>>
>>> Please, apply.
>>>
>>> Alexey Perevalov (6):
>>>        migration: introduce postcopy-blocktime capability
>>>        migration: add postcopy blocktime ctx into MigrationIncomingState
>>>        migration: calculate vCPU blocktime on dst side
>>>        migration: postcopy_blocktime documentation
>>>        migration: add blocktime calculation into migration-test
>>>        migration: add postcopy total blocktime into query-migrate
>> I had unanswered questions about these patches in the v12 series, where
>> I'm not sure if the interface is still quite right.
> To be fair, I had alroady integrated the patches before I saw your questions.
>
>> We're still early
>> enough that we could adjust the interface after the fact depending on
>> how the questions are answered;
> I think this is the best approach, so far I can see two questions:
>
> - do we want to make it conditional?  it requires some locking, but I
>    haven't meassured it to see how slow/fast is it.
>
> - the other was documentation.
>
> I will like Alexey to answer.  Depending of how slow it is, I can agree
> to make it non-optional.
Ok, I'll give a logs with traces, maybe gprof result, today
or tomorrow.
>
>> but we're also early enough that it may
>> be smarter to get the interface right before including it in a pull
>> request.  I'll leave it to Peter and Juan to sort out whether this means
>> an updated pull request is needed, or to take this as-is.
> Thanks, Juan.
>
>
>

-- 
Best regards,
Alexey Perevalov

On 3 January 2018 at 09:38, Juan Quintela <quintela@redhat.com> wrote:
> Hi
>
> This are the changes for migration that are already reviewed.
>
> Please, apply.
>
> Later, Juan.
>
>
> The following changes since commit 281f327487c9c9b1599f93c589a408bbf4a651b8:
>
>   Merge remote-tracking branch 'remotes/vivier/tags/m68k-for-2.12-pull-request' into staging (2017-12-22 00:11:36 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/juanquintela/qemu.git tags/migration/20180103
>
> for you to fetch changes up to 8a18afdcd8d2d6ab31f9de89d2f20fdadb89beb8:
>
>   migration: finalize current_migration object (2018-01-03 09:28:56 +0100)
>
> ----------------------------------------------------------------
> migration/next for 20180103
>

This fails to build on 32-bit systems:

/home/peter.maydell/qemu/migration/postcopy-ram.c: In function
'mark_postcopy_blocktime_begin':
/home/peter.maydell/qemu/migration/postcopy-ram.c:658:54: error: cast
to pointer from integer of different size
[-Werror=int-to-pointer-cast]
     already_received = ramblock_recv_bitmap_test(rb, (void *)addr);
                                                      ^

Keeping pointers in uint64_t and casting all over the place
looks like the wrong thing -- using the uintptr_t type will
likely make things cleaner.

This also looks suspicious:

    atomic_xchg__nocheck(&dc->last_begin, now_ms);
    atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], now_ms);
    atomic_xchg__nocheck(&dc->vcpu_addr[cpu], addr);

because atomic operations on types larger than the host pointer
size are not portable to all architectures. This should probably
use the atomic_cmpxchg(), not __nocheck, because the latter
bypasses the build time assert for this problem and turns a
"fails on any 32-bit host" into "fails obscurely on some
architectures only".

thanks
-- PMM