[Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support

[Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support

[Wei Wang]

This is the deivce part implementation to add a new feature,
VIRTIO_BALLOON_F_FREE_PAGE_HINT to the virtio-balloon device. The device
receives the guest free page hints from the driver and clears the
corresponding bits in the dirty bitmap, so that those free pages are
not sent by the migration thread to the destination.

*Tests
1 Test Environment
    Host: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
    Migration setup: migrate_set_speed 100G, migrate_set_downtime 400ms

2 Test Results (results are averaged over several repeated runs)
    2.1 Guest setup: 8G RAM, 4 vCPU
        2.1.1 Idle guest live migration time
            Optimization v.s. Legacy = 620ms vs 2970ms
            --> ~79% reduction
        2.1.2 Guest live migration with Linux compilation workload
          (i.e. make bzImage -j4) running
          1) Live Migration Time:
             Optimization v.s. Legacy = 2273ms v.s. 4502ms
             --> ~50% reduction
          2) Linux Compilation Time:
             Optimization v.s. Legacy = 8min42s v.s. 8min43s
             --> no obvious difference

    2.2 Guest setup: 128G RAM, 4 vCPU
        2.2.1 Idle guest live migration time
            Optimization v.s. Legacy = 5294ms vs 41651ms
            --> ~87% reduction
        2.2.2 Guest live migration with Linux compilation workload
          1) Live Migration Time:
            Optimization v.s. Legacy = 8816ms v.s. 54201ms
            --> 84% reduction
          2) Linux Compilation Time:
             Optimization v.s. Legacy = 8min30s v.s. 8min36s
             --> no obvious difference

ChangeLog:
v8->v9:
bitmap:
    - fix bitmap_count_one to handle the nbits=0 case
migration:
    - replace the ram save notifier chain with a more general precopy
      notifier chain, which is similar to the postcopy notifier chain.
    - Avoid exposing the RAMState struct, and add a function,
      precopy_disable_bulk_stage, to let the virtio-balloon notifier
      callback to disable the bulk stage flag.
virtio-balloon:
    - Remove VIRTIO_BALLOON_F_PAGE_POISON from this series as it is not
      a limit to the free page optimization now. Plan to add the device
      support for VIRTIO_BALLOON_F_PAGE_POISON in another patch later.

Previous changelog:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg01908.html

Wei Wang (8):
  bitmap: fix bitmap_count_one
  bitmap: bitmap_count_one_with_offset
  migration: use bitmap_mutex in migration_bitmap_clear_dirty
  migration: API to clear bits of guest free pages from the dirty bitmap
  migration/ram.c: add a notifier chain for precopy
  migration/ram.c: add a function to disable the bulk stage
  migration: move migrate_postcopy() to include/migration/misc.h
  virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

 hw/virtio/virtio-balloon.c                      | 255 ++++++++++++++++++++++++
 include/hw/virtio/virtio-balloon.h              |  28 ++-
 include/migration/misc.h                        |  16 ++
 include/qemu/bitmap.h                           |  17 ++
 include/standard-headers/linux/virtio_balloon.h |   5 +
 migration/migration.h                           |   2 -
 migration/ram.c                                 |  91 +++++++++
 vl.c                                            |   1 +
 8 files changed, 412 insertions(+), 3 deletions(-)

-- 
1.8.3.1

[virtio-dev] [PATCH v9 0/8] virtio-balloon: free page hint support

[Wei Wang]

This is the deivce part implementation to add a new feature,
VIRTIO_BALLOON_F_FREE_PAGE_HINT to the virtio-balloon device. The device
receives the guest free page hints from the driver and clears the
corresponding bits in the dirty bitmap, so that those free pages are
not sent by the migration thread to the destination.

*Tests
1 Test Environment
    Host: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
    Migration setup: migrate_set_speed 100G, migrate_set_downtime 400ms

2 Test Results (results are averaged over several repeated runs)
    2.1 Guest setup: 8G RAM, 4 vCPU
        2.1.1 Idle guest live migration time
            Optimization v.s. Legacy = 620ms vs 2970ms
            --> ~79% reduction
        2.1.2 Guest live migration with Linux compilation workload
          (i.e. make bzImage -j4) running
          1) Live Migration Time:
             Optimization v.s. Legacy = 2273ms v.s. 4502ms
             --> ~50% reduction
          2) Linux Compilation Time:
             Optimization v.s. Legacy = 8min42s v.s. 8min43s
             --> no obvious difference

    2.2 Guest setup: 128G RAM, 4 vCPU
        2.2.1 Idle guest live migration time
            Optimization v.s. Legacy = 5294ms vs 41651ms
            --> ~87% reduction
        2.2.2 Guest live migration with Linux compilation workload
          1) Live Migration Time:
            Optimization v.s. Legacy = 8816ms v.s. 54201ms
            --> 84% reduction
          2) Linux Compilation Time:
             Optimization v.s. Legacy = 8min30s v.s. 8min36s
             --> no obvious difference

ChangeLog:
v8->v9:
bitmap:
    - fix bitmap_count_one to handle the nbits=0 case
migration:
    - replace the ram save notifier chain with a more general precopy
      notifier chain, which is similar to the postcopy notifier chain.
    - Avoid exposing the RAMState struct, and add a function,
      precopy_disable_bulk_stage, to let the virtio-balloon notifier
      callback to disable the bulk stage flag.
virtio-balloon:
    - Remove VIRTIO_BALLOON_F_PAGE_POISON from this series as it is not
      a limit to the free page optimization now. Plan to add the device
      support for VIRTIO_BALLOON_F_PAGE_POISON in another patch later.

Previous changelog:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg01908.html

Wei Wang (8):
  bitmap: fix bitmap_count_one
  bitmap: bitmap_count_one_with_offset
  migration: use bitmap_mutex in migration_bitmap_clear_dirty
  migration: API to clear bits of guest free pages from the dirty bitmap
  migration/ram.c: add a notifier chain for precopy
  migration/ram.c: add a function to disable the bulk stage
  migration: move migrate_postcopy() to include/migration/misc.h
  virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

 hw/virtio/virtio-balloon.c                      | 255 ++++++++++++++++++++++++
 include/hw/virtio/virtio-balloon.h              |  28 ++-
 include/migration/misc.h                        |  16 ++
 include/qemu/bitmap.h                           |  17 ++
 include/standard-headers/linux/virtio_balloon.h |   5 +
 migration/migration.h                           |   2 -
 migration/ram.c                                 |  91 +++++++++
 vl.c                                            |   1 +
 8 files changed, 412 insertions(+), 3 deletions(-)

-- 
1.8.3.1


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[Qemu-devel] [PATCH v9 1/8] bitmap: fix bitmap_count_one

[Wei Wang]

BITMAP_LAST_WORD_MASK(nbits) returns 0xffffffff when "nbits=0", which
makes bitmap_count_one fail to handle the "nbits=0" case. It appears to be
preferred to remain BITMAP_LAST_WORD_MASK identical to the kernel
implementation that it is ported from.

So this patch fixes bitmap_count_one to handle the nbits=0 case.

Inital Discussion Link:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg554316.html
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
 include/qemu/bitmap.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 509eedd..679f1bd 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -221,6 +221,10 @@ static inline int bitmap_intersects(const unsigned long *src1,
 
 static inline long bitmap_count_one(const unsigned long *bitmap, long nbits)
 {
+    if (unlikely(!nbits)) {
+        return 0;
+    }
+
     if (small_nbits(nbits)) {
         return ctpopl(*bitmap & BITMAP_LAST_WORD_MASK(nbits));
     } else {
-- 
1.8.3.1

[virtio-dev] [PATCH v9 1/8] bitmap: fix bitmap_count_one

[Wei Wang]

BITMAP_LAST_WORD_MASK(nbits) returns 0xffffffff when "nbits=0", which
makes bitmap_count_one fail to handle the "nbits=0" case. It appears to be
preferred to remain BITMAP_LAST_WORD_MASK identical to the kernel
implementation that it is ported from.

So this patch fixes bitmap_count_one to handle the nbits=0 case.

Inital Discussion Link:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg554316.html
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
 include/qemu/bitmap.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 509eedd..679f1bd 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -221,6 +221,10 @@ static inline int bitmap_intersects(const unsigned long *src1,
 
 static inline long bitmap_count_one(const unsigned long *bitmap, long nbits)
 {
+    if (unlikely(!nbits)) {
+        return 0;
+    }
+
     if (small_nbits(nbits)) {
         return ctpopl(*bitmap & BITMAP_LAST_WORD_MASK(nbits));
     } else {
-- 
1.8.3.1


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[Qemu-devel] [PATCH v9 2/8] bitmap: bitmap_count_one_with_offset

[Wei Wang]

Count the number of 1s in a bitmap starting from an offset.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/qemu/bitmap.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 679f1bd..5c31334 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -232,6 +232,19 @@ static inline long bitmap_count_one(const unsigned long *bitmap, long nbits)
     }
 }
 
+static inline long bitmap_count_one_with_offset(const unsigned long *bitmap,
+                                                long offset, long nbits)
+{
+    long aligned_offset = QEMU_ALIGN_DOWN(offset, BITS_PER_LONG);
+    long redundant_bits = offset - aligned_offset;
+    long bits_to_count = nbits + redundant_bits;
+    const unsigned long *bitmap_start = bitmap +
+                                        aligned_offset / BITS_PER_LONG;
+
+    return bitmap_count_one(bitmap_start, bits_to_count) -
+           bitmap_count_one(bitmap_start, redundant_bits);
+}
+
 void bitmap_set(unsigned long *map, long i, long len);
 void bitmap_set_atomic(unsigned long *map, long i, long len);
 void bitmap_clear(unsigned long *map, long start, long nr);
-- 
1.8.3.1

[virtio-dev] [PATCH v9 2/8] bitmap: bitmap_count_one_with_offset

[Wei Wang]

Count the number of 1s in a bitmap starting from an offset.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/qemu/bitmap.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 679f1bd..5c31334 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -232,6 +232,19 @@ static inline long bitmap_count_one(const unsigned long *bitmap, long nbits)
     }
 }
 
+static inline long bitmap_count_one_with_offset(const unsigned long *bitmap,
+                                                long offset, long nbits)
+{
+    long aligned_offset = QEMU_ALIGN_DOWN(offset, BITS_PER_LONG);
+    long redundant_bits = offset - aligned_offset;
+    long bits_to_count = nbits + redundant_bits;
+    const unsigned long *bitmap_start = bitmap +
+                                        aligned_offset / BITS_PER_LONG;
+
+    return bitmap_count_one(bitmap_start, bits_to_count) -
+           bitmap_count_one(bitmap_start, redundant_bits);
+}
+
 void bitmap_set(unsigned long *map, long i, long len);
 void bitmap_set_atomic(unsigned long *map, long i, long len);
 void bitmap_clear(unsigned long *map, long start, long nr);
-- 
1.8.3.1


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[Qemu-devel] [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty

[Wei Wang]

The bitmap mutex is used to synchronize threads to update the dirty
bitmap and the migration_dirty_pages counter. For example, the free
page optimization clears bits of free pages from the bitmap in an
iothread context. This patch makes migration_bitmap_clear_dirty update
the bitmap and counter under the mutex.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
 migration/ram.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index 7e7deec..ef69dbe 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1556,11 +1556,14 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
 {
     bool ret;
 
+    qemu_mutex_lock(&rs->bitmap_mutex);
     ret = test_and_clear_bit(page, rb->bmap);
 
     if (ret) {
         rs->migration_dirty_pages--;
     }
+    qemu_mutex_unlock(&rs->bitmap_mutex);
+
     return ret;
 }
 
-- 
1.8.3.1

[virtio-dev] [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty

[Wei Wang]

The bitmap mutex is used to synchronize threads to update the dirty
bitmap and the migration_dirty_pages counter. For example, the free
page optimization clears bits of free pages from the bitmap in an
iothread context. This patch makes migration_bitmap_clear_dirty update
the bitmap and counter under the mutex.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
 migration/ram.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index 7e7deec..ef69dbe 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1556,11 +1556,14 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
 {
     bool ret;
 
+    qemu_mutex_lock(&rs->bitmap_mutex);
     ret = test_and_clear_bit(page, rb->bmap);
 
     if (ret) {
         rs->migration_dirty_pages--;
     }
+    qemu_mutex_unlock(&rs->bitmap_mutex);
+
     return ret;
 }
 
-- 
1.8.3.1


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty

[Peter Xu]

On Thu, Nov 15, 2018 at 06:07:59PM +0800, Wei Wang wrote:
> The bitmap mutex is used to synchronize threads to update the dirty
> bitmap and the migration_dirty_pages counter. For example, the free
> page optimization clears bits of free pages from the bitmap in an
> iothread context. This patch makes migration_bitmap_clear_dirty update
> the bitmap and counter under the mutex.
> 
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Peter Xu <peterx@redhat.com>
> ---
>  migration/ram.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 7e7deec..ef69dbe 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1556,11 +1556,14 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
>  {
>      bool ret;
>  
> +    qemu_mutex_lock(&rs->bitmap_mutex);
>      ret = test_and_clear_bit(page, rb->bmap);
>  
>      if (ret) {
>          rs->migration_dirty_pages--;
>      }
> +    qemu_mutex_unlock(&rs->bitmap_mutex);
> +
>      return ret;
>  }

It seems fine to me, but have you thought about
test_and_clear_bit_atomic()?  Note that we just had
test_and_set_bit_atomic() a few months ago.

And not related to this patch: I'm unclear on why we have had
bitmap_mutex before, since it seems unnecessary.

Regards,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty

[Wei Wang]

On 11/27/2018 01:40 PM, Peter Xu wrote:
> On Thu, Nov 15, 2018 at 06:07:59PM +0800, Wei Wang wrote:
>> The bitmap mutex is used to synchronize threads to update the dirty
>> bitmap and the migration_dirty_pages counter. For example, the free
>> page optimization clears bits of free pages from the bitmap in an
>> iothread context. This patch makes migration_bitmap_clear_dirty update
>> the bitmap and counter under the mutex.
>>
>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> CC: Juan Quintela <quintela@redhat.com>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> CC: Peter Xu <peterx@redhat.com>
>> ---
>>   migration/ram.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 7e7deec..ef69dbe 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -1556,11 +1556,14 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
>>   {
>>       bool ret;
>>   
>> +    qemu_mutex_lock(&rs->bitmap_mutex);
>>       ret = test_and_clear_bit(page, rb->bmap);
>>   
>>       if (ret) {
>>           rs->migration_dirty_pages--;
>>       }
>> +    qemu_mutex_unlock(&rs->bitmap_mutex);
>> +
>>       return ret;
>>   }
> It seems fine to me, but have you thought about
> test_and_clear_bit_atomic()?  Note that we just had
> test_and_set_bit_atomic() a few months ago.

Thanks for sharing. I think we might also need to
mutex migration_dirty_pages.

>
> And not related to this patch: I'm unclear on why we have had
> bitmap_mutex before, since it seems unnecessary.

OK. This is because with the optimization we have a thread
which clears bits (of free pages) from the bitmap and updates
migration_dirty_pages. So we need to synchronization between
the migration thread and the optimization thread.

Best,
Wei

[virtio-dev] Re: [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty

[Wei Wang]

On 11/27/2018 01:40 PM, Peter Xu wrote:
> On Thu, Nov 15, 2018 at 06:07:59PM +0800, Wei Wang wrote:
>> The bitmap mutex is used to synchronize threads to update the dirty
>> bitmap and the migration_dirty_pages counter. For example, the free
>> page optimization clears bits of free pages from the bitmap in an
>> iothread context. This patch makes migration_bitmap_clear_dirty update
>> the bitmap and counter under the mutex.
>>
>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> CC: Juan Quintela <quintela@redhat.com>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> CC: Peter Xu <peterx@redhat.com>
>> ---
>>   migration/ram.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 7e7deec..ef69dbe 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -1556,11 +1556,14 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
>>   {
>>       bool ret;
>>   
>> +    qemu_mutex_lock(&rs->bitmap_mutex);
>>       ret = test_and_clear_bit(page, rb->bmap);
>>   
>>       if (ret) {
>>           rs->migration_dirty_pages--;
>>       }
>> +    qemu_mutex_unlock(&rs->bitmap_mutex);
>> +
>>       return ret;
>>   }
> It seems fine to me, but have you thought about
> test_and_clear_bit_atomic()?  Note that we just had
> test_and_set_bit_atomic() a few months ago.

Thanks for sharing. I think we might also need to
mutex migration_dirty_pages.

>
> And not related to this patch: I'm unclear on why we have had
> bitmap_mutex before, since it seems unnecessary.

OK. This is because with the optimization we have a thread
which clears bits (of free pages) from the bitmap and updates
migration_dirty_pages. So we need to synchronization between
the migration thread and the optimization thread.

Best,
Wei



---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [virtio-dev] Re: [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty

[Wei Wang]

On 11/27/2018 02:02 PM, Wei Wang wrote:
> On 11/27/2018 01:40 PM, Peter Xu wrote:
>> On Thu, Nov 15, 2018 at 06:07:59PM +0800, Wei Wang wrote:
>>> The bitmap mutex is used to synchronize threads to update the dirty
>>> bitmap and the migration_dirty_pages counter. For example, the free
>>> page optimization clears bits of free pages from the bitmap in an
>>> iothread context. This patch makes migration_bitmap_clear_dirty update
>>> the bitmap and counter under the mutex.
>>>
>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>>> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> CC: Juan Quintela <quintela@redhat.com>
>>> CC: Michael S. Tsirkin <mst@redhat.com>
>>> CC: Peter Xu <peterx@redhat.com>
>>> ---
>>>   migration/ram.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/migration/ram.c b/migration/ram.c
>>> index 7e7deec..ef69dbe 100644
>>> --- a/migration/ram.c
>>> +++ b/migration/ram.c
>>> @@ -1556,11 +1556,14 @@ static inline bool 
>>> migration_bitmap_clear_dirty(RAMState *rs,
>>>   {
>>>       bool ret;
>>>   +    qemu_mutex_lock(&rs->bitmap_mutex);
>>>       ret = test_and_clear_bit(page, rb->bmap);
>>>         if (ret) {
>>>           rs->migration_dirty_pages--;
>>>       }
>>> +    qemu_mutex_unlock(&rs->bitmap_mutex);
>>> +
>>>       return ret;
>>>   }
>> It seems fine to me, but have you thought about
>> test_and_clear_bit_atomic()?  Note that we just had
>> test_and_set_bit_atomic() a few months ago.
>
> Thanks for sharing. I think we might also need to
> mutex migration_dirty_pages.
>
>>
>> And not related to this patch: I'm unclear on why we have had
>> bitmap_mutex before, since it seems unnecessary.
>
> OK. This is because with the optimization we have a thread
> which clears bits (of free pages) from the bitmap and updates
> migration_dirty_pages. So we need to synchronization between
> the migration thread and the optimization thread.
>

And before this feature, I think yes, that bitmap_mutex is not needed.
It was left there due to some historical reasons.
I remember Dave previous said he was about to remove it. But the new
feature will need it again.

Best,
Wei

Re: [virtio-dev] Re: [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty

[Wei Wang]

On 11/27/2018 02:02 PM, Wei Wang wrote:
> On 11/27/2018 01:40 PM, Peter Xu wrote:
>> On Thu, Nov 15, 2018 at 06:07:59PM +0800, Wei Wang wrote:
>>> The bitmap mutex is used to synchronize threads to update the dirty
>>> bitmap and the migration_dirty_pages counter. For example, the free
>>> page optimization clears bits of free pages from the bitmap in an
>>> iothread context. This patch makes migration_bitmap_clear_dirty update
>>> the bitmap and counter under the mutex.
>>>
>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>>> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> CC: Juan Quintela <quintela@redhat.com>
>>> CC: Michael S. Tsirkin <mst@redhat.com>
>>> CC: Peter Xu <peterx@redhat.com>
>>> ---
>>>   migration/ram.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/migration/ram.c b/migration/ram.c
>>> index 7e7deec..ef69dbe 100644
>>> --- a/migration/ram.c
>>> +++ b/migration/ram.c
>>> @@ -1556,11 +1556,14 @@ static inline bool 
>>> migration_bitmap_clear_dirty(RAMState *rs,
>>>   {
>>>       bool ret;
>>>   +    qemu_mutex_lock(&rs->bitmap_mutex);
>>>       ret = test_and_clear_bit(page, rb->bmap);
>>>         if (ret) {
>>>           rs->migration_dirty_pages--;
>>>       }
>>> +    qemu_mutex_unlock(&rs->bitmap_mutex);
>>> +
>>>       return ret;
>>>   }
>> It seems fine to me, but have you thought about
>> test_and_clear_bit_atomic()?  Note that we just had
>> test_and_set_bit_atomic() a few months ago.
>
> Thanks for sharing. I think we might also need to
> mutex migration_dirty_pages.
>
>>
>> And not related to this patch: I'm unclear on why we have had
>> bitmap_mutex before, since it seems unnecessary.
>
> OK. This is because with the optimization we have a thread
> which clears bits (of free pages) from the bitmap and updates
> migration_dirty_pages. So we need to synchronization between
> the migration thread and the optimization thread.
>

And before this feature, I think yes, that bitmap_mutex is not needed.
It was left there due to some historical reasons.
I remember Dave previous said he was about to remove it. But the new
feature will need it again.

Best,
Wei

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [virtio-dev] Re: [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty

[Peter Xu]

On Tue, Nov 27, 2018 at 02:12:34PM +0800, Wei Wang wrote:
> On 11/27/2018 02:02 PM, Wei Wang wrote:
> > On 11/27/2018 01:40 PM, Peter Xu wrote:
> > > On Thu, Nov 15, 2018 at 06:07:59PM +0800, Wei Wang wrote:
> > > > The bitmap mutex is used to synchronize threads to update the dirty
> > > > bitmap and the migration_dirty_pages counter. For example, the free
> > > > page optimization clears bits of free pages from the bitmap in an
> > > > iothread context. This patch makes migration_bitmap_clear_dirty update
> > > > the bitmap and counter under the mutex.
> > > > 
> > > > Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> > > > CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > > CC: Juan Quintela <quintela@redhat.com>
> > > > CC: Michael S. Tsirkin <mst@redhat.com>
> > > > CC: Peter Xu <peterx@redhat.com>
> > > > ---
> > > >   migration/ram.c | 3 +++
> > > >   1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/migration/ram.c b/migration/ram.c
> > > > index 7e7deec..ef69dbe 100644
> > > > --- a/migration/ram.c
> > > > +++ b/migration/ram.c
> > > > @@ -1556,11 +1556,14 @@ static inline bool
> > > > migration_bitmap_clear_dirty(RAMState *rs,
> > > >   {
> > > >       bool ret;
> > > >   +    qemu_mutex_lock(&rs->bitmap_mutex);
> > > >       ret = test_and_clear_bit(page, rb->bmap);
> > > >         if (ret) {
> > > >           rs->migration_dirty_pages--;
> > > >       }
> > > > +    qemu_mutex_unlock(&rs->bitmap_mutex);
> > > > +
> > > >       return ret;
> > > >   }
> > > It seems fine to me, but have you thought about
> > > test_and_clear_bit_atomic()?  Note that we just had
> > > test_and_set_bit_atomic() a few months ago.
> > 
> > Thanks for sharing. I think we might also need to
> > mutex migration_dirty_pages.
> > 
> > > 
> > > And not related to this patch: I'm unclear on why we have had
> > > bitmap_mutex before, since it seems unnecessary.
> > 
> > OK. This is because with the optimization we have a thread
> > which clears bits (of free pages) from the bitmap and updates
> > migration_dirty_pages. So we need to synchronization between
> > the migration thread and the optimization thread.
> > 
> 
> And before this feature, I think yes, that bitmap_mutex is not needed.
> It was left there due to some historical reasons.
> I remember Dave previous said he was about to remove it. But the new
> feature will need it again.

Ok then I'm fine with it.  Though you could update the comments too if
you like:

    /* protects modification of the bitmap and migration_dirty_pages */
    QemuMutex bitmap_mutex;

And it's tricky that sometimes we don't take the lock when reading
this variable "migration_dirty_pages".  I don't see obvious issue so
far, hope it's true (at least I skipped the colo ones...).

ram_bytes_remaining[333]       return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
migration_bitmap_clear_dirty[1562] rs->migration_dirty_pages--;
migration_bitmap_sync_range[1570] rs->migration_dirty_pages +=
postcopy_chunk_hostpages_pass[2809] rs->migration_dirty_pages += !test_and_set_bit(page, bitmap);
ram_state_init[3037]           (*rsp)->migration_dirty_pages = ram_bytes_total() >> TARGET_PAGE_BITS;
ram_state_resume_prepare[3112] rs->migration_dirty_pages = pages;
ram_save_pending[3344]         remaining_size = rs->migration_dirty_pages * TARGET_PAGE_SIZE;
ram_save_pending[3353]         remaining_size = rs->migration_dirty_pages * TARGET_PAGE_SIZE;
colo_cache_from_block_offset[3468] ram_state->migration_dirty_pages++;
colo_init_ram_cache[3716]      ram_state->migration_dirty_pages = 0;
colo_flush_ram_cache[3997]     trace_colo_flush_ram_cache_begin(ram_state->migration_dirty_pages);

Regards,

-- 
Peter Xu

Re: [Qemu-devel] [virtio-dev] Re: [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty

[Wei Wang]

On 11/27/2018 03:41 PM, Peter Xu wrote:
>
> Ok then I'm fine with it.  Though you could update the comments too if
> you like:
>
>      /* protects modification of the bitmap and migration_dirty_pages */
>      QemuMutex bitmap_mutex;
>
> And it's tricky that sometimes we don't take the lock when reading
> this variable "migration_dirty_pages".  I don't see obvious issue so
> far, hope it's true (at least I skipped the colo ones...).

The caller reads the value just to estimate the remaining_size, and
it seems fine without a lock, because whether it reads a
value before the update or after the update seem not causing
an issue.

Best,
Wei

Re: [virtio-dev] Re: [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty

[Wei Wang]

On 11/27/2018 03:41 PM, Peter Xu wrote:
>
> Ok then I'm fine with it.  Though you could update the comments too if
> you like:
>
>      /* protects modification of the bitmap and migration_dirty_pages */
>      QemuMutex bitmap_mutex;
>
> And it's tricky that sometimes we don't take the lock when reading
> this variable "migration_dirty_pages".  I don't see obvious issue so
> far, hope it's true (at least I skipped the colo ones...).

The caller reads the value just to estimate the remaining_size, and
it seems fine without a lock, because whether it reads a
value before the update or after the update seem not causing
an issue.

Best,
Wei

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[Qemu-devel] [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap

[Wei Wang]

This patch adds an API to clear bits corresponding to guest free pages
from the dirty bitmap. Spilt the free page block if it crosses the QEMU
RAMBlock boundary.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
 include/migration/misc.h |  2 ++
 migration/ram.c          | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 4ebf24c..113320e 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -14,11 +14,13 @@
 #ifndef MIGRATION_MISC_H
 #define MIGRATION_MISC_H
 
+#include "exec/cpu-common.h"
 #include "qemu/notify.h"
 
 /* migration/ram.c */
 
 void ram_mig_init(void);
+void qemu_guest_free_page_hint(void *addr, size_t len);
 
 /* migration/block.c */
 
diff --git a/migration/ram.c b/migration/ram.c
index ef69dbe..229b791 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3131,6 +3131,54 @@ static void ram_state_resume_prepare(RAMState *rs, QEMUFile *out)
 }
 
 /*
+ * This function clears bits of the free pages reported by the caller from the
+ * migration dirty bitmap. @addr is the host address corresponding to the
+ * start of the continuous guest free pages, and @len is the total bytes of
+ * those pages.
+ */
+void qemu_guest_free_page_hint(void *addr, size_t len)
+{
+    RAMBlock *block;
+    ram_addr_t offset;
+    size_t used_len, start, npages;
+    MigrationState *s = migrate_get_current();
+
+    /* This function is currently expected to be used during live migration */
+    if (!migration_is_setup_or_active(s->state)) {
+        return;
+    }
+
+    for (; len > 0; len -= used_len) {
+        block = qemu_ram_block_from_host(addr, false, &offset);
+        assert(block);
+
+        /*
+         * This handles the case that the RAMBlock is resized after the free
+         * page hint is reported.
+         */
+        if (unlikely(offset > block->used_length)) {
+            return;
+        }
+
+        if (len <= block->used_length - offset) {
+            used_len = len;
+        } else {
+            used_len = block->used_length - offset;
+            addr += used_len;
+        }
+
+        start = offset >> TARGET_PAGE_BITS;
+        npages = used_len >> TARGET_PAGE_BITS;
+
+        qemu_mutex_lock(&ram_state->bitmap_mutex);
+        ram_state->migration_dirty_pages -=
+                      bitmap_count_one_with_offset(block->bmap, start, npages);
+        bitmap_clear(block->bmap, start, npages);
+        qemu_mutex_unlock(&ram_state->bitmap_mutex);
+    }
+}
+
+/*
  * Each of ram_save_setup, ram_save_iterate and ram_save_complete has
  * long-running RCU critical section.  When rcu-reclaims in the code
  * start to become numerous it will be necessary to reduce the
-- 
1.8.3.1

[virtio-dev] [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap

[Wei Wang]

This patch adds an API to clear bits corresponding to guest free pages
from the dirty bitmap. Spilt the free page block if it crosses the QEMU
RAMBlock boundary.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
 include/migration/misc.h |  2 ++
 migration/ram.c          | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 4ebf24c..113320e 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -14,11 +14,13 @@
 #ifndef MIGRATION_MISC_H
 #define MIGRATION_MISC_H
 
+#include "exec/cpu-common.h"
 #include "qemu/notify.h"
 
 /* migration/ram.c */
 
 void ram_mig_init(void);
+void qemu_guest_free_page_hint(void *addr, size_t len);
 
 /* migration/block.c */
 
diff --git a/migration/ram.c b/migration/ram.c
index ef69dbe..229b791 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3131,6 +3131,54 @@ static void ram_state_resume_prepare(RAMState *rs, QEMUFile *out)
 }
 
 /*
+ * This function clears bits of the free pages reported by the caller from the
+ * migration dirty bitmap. @addr is the host address corresponding to the
+ * start of the continuous guest free pages, and @len is the total bytes of
+ * those pages.
+ */
+void qemu_guest_free_page_hint(void *addr, size_t len)
+{
+    RAMBlock *block;
+    ram_addr_t offset;
+    size_t used_len, start, npages;
+    MigrationState *s = migrate_get_current();
+
+    /* This function is currently expected to be used during live migration */
+    if (!migration_is_setup_or_active(s->state)) {
+        return;
+    }
+
+    for (; len > 0; len -= used_len) {
+        block = qemu_ram_block_from_host(addr, false, &offset);
+        assert(block);
+
+        /*
+         * This handles the case that the RAMBlock is resized after the free
+         * page hint is reported.
+         */
+        if (unlikely(offset > block->used_length)) {
+            return;
+        }
+
+        if (len <= block->used_length - offset) {
+            used_len = len;
+        } else {
+            used_len = block->used_length - offset;
+            addr += used_len;
+        }
+
+        start = offset >> TARGET_PAGE_BITS;
+        npages = used_len >> TARGET_PAGE_BITS;
+
+        qemu_mutex_lock(&ram_state->bitmap_mutex);
+        ram_state->migration_dirty_pages -=
+                      bitmap_count_one_with_offset(block->bmap, start, npages);
+        bitmap_clear(block->bmap, start, npages);
+        qemu_mutex_unlock(&ram_state->bitmap_mutex);
+    }
+}
+
+/*
  * Each of ram_save_setup, ram_save_iterate and ram_save_complete has
  * long-running RCU critical section.  When rcu-reclaims in the code
  * start to become numerous it will be necessary to reduce the
-- 
1.8.3.1


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap

[Peter Xu]

On Thu, Nov 15, 2018 at 06:08:00PM +0800, Wei Wang wrote:
> This patch adds an API to clear bits corresponding to guest free pages
> from the dirty bitmap. Spilt the free page block if it crosses the QEMU
> RAMBlock boundary.
> 
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Peter Xu <peterx@redhat.com>
> ---
>  include/migration/misc.h |  2 ++
>  migration/ram.c          | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> index 4ebf24c..113320e 100644
> --- a/include/migration/misc.h
> +++ b/include/migration/misc.h
> @@ -14,11 +14,13 @@
>  #ifndef MIGRATION_MISC_H
>  #define MIGRATION_MISC_H
>  
> +#include "exec/cpu-common.h"
>  #include "qemu/notify.h"
>  
>  /* migration/ram.c */
>  
>  void ram_mig_init(void);
> +void qemu_guest_free_page_hint(void *addr, size_t len);
>  
>  /* migration/block.c */
>  
> diff --git a/migration/ram.c b/migration/ram.c
> index ef69dbe..229b791 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3131,6 +3131,54 @@ static void ram_state_resume_prepare(RAMState *rs, QEMUFile *out)
>  }
>  
>  /*
> + * This function clears bits of the free pages reported by the caller from the
> + * migration dirty bitmap. @addr is the host address corresponding to the
> + * start of the continuous guest free pages, and @len is the total bytes of
> + * those pages.
> + */
> +void qemu_guest_free_page_hint(void *addr, size_t len)
> +{
> +    RAMBlock *block;
> +    ram_addr_t offset;
> +    size_t used_len, start, npages;
> +    MigrationState *s = migrate_get_current();
> +
> +    /* This function is currently expected to be used during live migration */
> +    if (!migration_is_setup_or_active(s->state)) {
> +        return;
> +    }
> +
> +    for (; len > 0; len -= used_len) {
> +        block = qemu_ram_block_from_host(addr, false, &offset);
> +        assert(block);
> +
> +        /*
> +         * This handles the case that the RAMBlock is resized after the free
> +         * page hint is reported.
> +         */
> +        if (unlikely(offset > block->used_length)) {
> +            return;
> +        }

Again, is it possible to resize during migration?

So I think the check is fine, but uncertain about the comment.

And shall we print something if that happened?  We can use
error_report_once(), and squashing the above assert:

  if (!block || offset > block->used_length) {
    /* should never happen, but if it happens we ignore the hints and warn */
    error_report_once("...");
    return;
  }

What do you think?

> +
> +        if (len <= block->used_length - offset) {
> +            used_len = len;
> +        } else {
> +            used_len = block->used_length - offset;
> +            addr += used_len;

Maybe moving this line into the for() could be a bit better?

  for (; len > 0; len -= used_len, addr += used_len) {

> +        }
> +
> +        start = offset >> TARGET_PAGE_BITS;
> +        npages = used_len >> TARGET_PAGE_BITS;
> +
> +        qemu_mutex_lock(&ram_state->bitmap_mutex);
> +        ram_state->migration_dirty_pages -=
> +                      bitmap_count_one_with_offset(block->bmap, start, npages);
> +        bitmap_clear(block->bmap, start, npages);
> +        qemu_mutex_unlock(&ram_state->bitmap_mutex);
> +    }
> +}
> +
> +/*
>   * Each of ram_save_setup, ram_save_iterate and ram_save_complete has
>   * long-running RCU critical section.  When rcu-reclaims in the code
>   * start to become numerous it will be necessary to reduce the
> -- 
> 1.8.3.1
> 

Regards,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap

[Wei Wang]

On 11/27/2018 02:06 PM, Peter Xu wrote:
> On Thu, Nov 15, 2018 at 06:08:00PM +0800, Wei Wang wrote:
> Again, is it possible to resize during migration?
>
> So I think the check is fine, but uncertain about the comment.

Yes, resize would not happen with the current implementation.
But heard it could just be a temporal implementation. Probably
we could improve the comment like this:

"
Though the implementation might not support ram resize currently,
this could happen in theory with future updates. So the check here
handles the case that RAMBLOCK is resized after the free page hint is
reported.
"

>
> And shall we print something if that happened?  We can use
> error_report_once(), and squashing the above assert:
>
>    if (!block || offset > block->used_length) {
>      /* should never happen, but if it happens we ignore the hints and warn */
>      error_report_once("...");
>      return;
>    }
>
> What do you think?

Sounds good.

>
>> +
>> +        if (len <= block->used_length - offset) {
>> +            used_len = len;
>> +        } else {
>> +            used_len = block->used_length - offset;
>> +            addr += used_len;
> Maybe moving this line into the for() could be a bit better?
>
>    for (; len > 0; len -= used_len, addr += used_len) {
>

Yes, I think it looks better, thanks.

Best,
Wei

[virtio-dev] Re: [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap

[Wei Wang]

On 11/27/2018 02:06 PM, Peter Xu wrote:
> On Thu, Nov 15, 2018 at 06:08:00PM +0800, Wei Wang wrote:
> Again, is it possible to resize during migration?
>
> So I think the check is fine, but uncertain about the comment.

Yes, resize would not happen with the current implementation.
But heard it could just be a temporal implementation. Probably
we could improve the comment like this:

"
Though the implementation might not support ram resize currently,
this could happen in theory with future updates. So the check here
handles the case that RAMBLOCK is resized after the free page hint is
reported.
"

>
> And shall we print something if that happened?  We can use
> error_report_once(), and squashing the above assert:
>
>    if (!block || offset > block->used_length) {
>      /* should never happen, but if it happens we ignore the hints and warn */
>      error_report_once("...");
>      return;
>    }
>
> What do you think?

Sounds good.

>
>> +
>> +        if (len <= block->used_length - offset) {
>> +            used_len = len;
>> +        } else {
>> +            used_len = block->used_length - offset;
>> +            addr += used_len;
> Maybe moving this line into the for() could be a bit better?
>
>    for (; len > 0; len -= used_len, addr += used_len) {
>

Yes, I think it looks better, thanks.

Best,
Wei


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap

[Peter Xu]

On Tue, Nov 27, 2018 at 02:52:35PM +0800, Wei Wang wrote:
> On 11/27/2018 02:06 PM, Peter Xu wrote:
> > On Thu, Nov 15, 2018 at 06:08:00PM +0800, Wei Wang wrote:
> > Again, is it possible to resize during migration?
> > 
> > So I think the check is fine, but uncertain about the comment.
> 
> Yes, resize would not happen with the current implementation.
> But heard it could just be a temporal implementation. Probably
> we could improve the comment like this:
> 
> "
> Though the implementation might not support ram resize currently,
> this could happen in theory with future updates. So the check here
> handles the case that RAMBLOCK is resized after the free page hint is
> reported.
> "

I'm not familiar with that part, but this seems ok to me.

Thanks,

-- 
Peter Xu

[Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

This patch adds a notifier chain for the memory precopy. This enables various
precopy optimizations to be invoked at specific places.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
 include/migration/misc.h | 12 ++++++++++++
 migration/ram.c          | 31 +++++++++++++++++++++++++++++++
 vl.c                     |  1 +
 3 files changed, 44 insertions(+)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 113320e..0bac623 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -19,6 +19,18 @@
 
 /* migration/ram.c */
 
+typedef enum PrecopyNotifyReason {
+    PRECOPY_NOTIFY_ERR = 0,
+    PRECOPY_NOTIFY_START_ITERATION = 1,
+    PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2,
+    PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3,
+    PRECOPY_NOTIFY_MAX = 4,
+} PrecopyNotifyReason;
+
+void precopy_infrastructure_init(void);
+void precopy_add_notifier(Notifier *n);
+void precopy_remove_notifier(Notifier *n);
+
 void ram_mig_init(void);
 void qemu_guest_free_page_hint(void *addr, size_t len);
 
diff --git a/migration/ram.c b/migration/ram.c
index 229b791..65b1223 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -292,6 +292,8 @@ struct RAMState {
     bool ram_bulk_stage;
     /* How many times we have dirty too many pages */
     int dirty_rate_high_cnt;
+    /* ram save states used for notifiers */
+    int ram_save_state;
     /* these variables are used for bitmap sync */
     /* last time we did a full bitmap_sync */
     int64_t time_last_bitmap_sync;
@@ -328,6 +330,28 @@ typedef struct RAMState RAMState;
 
 static RAMState *ram_state;
 
+static NotifierList precopy_notifier_list;
+
+void precopy_infrastructure_init(void)
+{
+    notifier_list_init(&precopy_notifier_list);
+}
+
+void precopy_add_notifier(Notifier *n)
+{
+    notifier_list_add(&precopy_notifier_list, n);
+}
+
+void precopy_remove_notifier(Notifier *n)
+{
+    notifier_remove(n);
+}
+
+static void precopy_notify(PrecopyNotifyReason reason)
+{
+    notifier_list_notify(&precopy_notifier_list, &reason);
+}
+
 uint64_t ram_bytes_remaining(void)
 {
     return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
@@ -1642,6 +1666,8 @@ static void migration_bitmap_sync(RAMState *rs)
     int64_t end_time;
     uint64_t bytes_xfer_now;
 
+    precopy_notify(PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP);
+
     ram_counters.dirty_sync_count++;
 
     if (!rs->time_last_bitmap_sync) {
@@ -1699,6 +1725,8 @@ static void migration_bitmap_sync(RAMState *rs)
     if (migrate_use_events()) {
         qapi_event_send_migration_pass(ram_counters.dirty_sync_count);
     }
+
+    precopy_notify(PRECOPY_NOTIFY_AFTER_SYNC_BITMAP);
 }
 
 /**
@@ -2555,6 +2583,8 @@ static void ram_state_reset(RAMState *rs)
     rs->last_page = 0;
     rs->last_version = ram_list.version;
     rs->ram_bulk_stage = true;
+
+    precopy_notify(PRECOPY_NOTIFY_START_ITERATION);
 }
 
 #define MAX_WAIT 50 /* ms, half buffered_file limit */
@@ -3324,6 +3354,7 @@ out:
 
     ret = qemu_file_get_error(f);
     if (ret < 0) {
+        precopy_notify(PRECOPY_NOTIFY_ERR);
         return ret;
     }
 
diff --git a/vl.c b/vl.c
index fa25d1a..48ae0e8 100644
--- a/vl.c
+++ b/vl.c
@@ -3057,6 +3057,7 @@ int main(int argc, char **argv, char **envp)
     module_call_init(MODULE_INIT_OPTS);
 
     runstate_init();
+    precopy_infrastructure_init();
     postcopy_infrastructure_init();
     monitor_init_globals();
 
-- 
1.8.3.1

[virtio-dev] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

This patch adds a notifier chain for the memory precopy. This enables various
precopy optimizations to be invoked at specific places.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
 include/migration/misc.h | 12 ++++++++++++
 migration/ram.c          | 31 +++++++++++++++++++++++++++++++
 vl.c                     |  1 +
 3 files changed, 44 insertions(+)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 113320e..0bac623 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -19,6 +19,18 @@
 
 /* migration/ram.c */
 
+typedef enum PrecopyNotifyReason {
+    PRECOPY_NOTIFY_ERR = 0,
+    PRECOPY_NOTIFY_START_ITERATION = 1,
+    PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2,
+    PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3,
+    PRECOPY_NOTIFY_MAX = 4,
+} PrecopyNotifyReason;
+
+void precopy_infrastructure_init(void);
+void precopy_add_notifier(Notifier *n);
+void precopy_remove_notifier(Notifier *n);
+
 void ram_mig_init(void);
 void qemu_guest_free_page_hint(void *addr, size_t len);
 
diff --git a/migration/ram.c b/migration/ram.c
index 229b791..65b1223 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -292,6 +292,8 @@ struct RAMState {
     bool ram_bulk_stage;
     /* How many times we have dirty too many pages */
     int dirty_rate_high_cnt;
+    /* ram save states used for notifiers */
+    int ram_save_state;
     /* these variables are used for bitmap sync */
     /* last time we did a full bitmap_sync */
     int64_t time_last_bitmap_sync;
@@ -328,6 +330,28 @@ typedef struct RAMState RAMState;
 
 static RAMState *ram_state;
 
+static NotifierList precopy_notifier_list;
+
+void precopy_infrastructure_init(void)
+{
+    notifier_list_init(&precopy_notifier_list);
+}
+
+void precopy_add_notifier(Notifier *n)
+{
+    notifier_list_add(&precopy_notifier_list, n);
+}
+
+void precopy_remove_notifier(Notifier *n)
+{
+    notifier_remove(n);
+}
+
+static void precopy_notify(PrecopyNotifyReason reason)
+{
+    notifier_list_notify(&precopy_notifier_list, &reason);
+}
+
 uint64_t ram_bytes_remaining(void)
 {
     return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
@@ -1642,6 +1666,8 @@ static void migration_bitmap_sync(RAMState *rs)
     int64_t end_time;
     uint64_t bytes_xfer_now;
 
+    precopy_notify(PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP);
+
     ram_counters.dirty_sync_count++;
 
     if (!rs->time_last_bitmap_sync) {
@@ -1699,6 +1725,8 @@ static void migration_bitmap_sync(RAMState *rs)
     if (migrate_use_events()) {
         qapi_event_send_migration_pass(ram_counters.dirty_sync_count);
     }
+
+    precopy_notify(PRECOPY_NOTIFY_AFTER_SYNC_BITMAP);
 }
 
 /**
@@ -2555,6 +2583,8 @@ static void ram_state_reset(RAMState *rs)
     rs->last_page = 0;
     rs->last_version = ram_list.version;
     rs->ram_bulk_stage = true;
+
+    precopy_notify(PRECOPY_NOTIFY_START_ITERATION);
 }
 
 #define MAX_WAIT 50 /* ms, half buffered_file limit */
@@ -3324,6 +3354,7 @@ out:
 
     ret = qemu_file_get_error(f);
     if (ret < 0) {
+        precopy_notify(PRECOPY_NOTIFY_ERR);
         return ret;
     }
 
diff --git a/vl.c b/vl.c
index fa25d1a..48ae0e8 100644
--- a/vl.c
+++ b/vl.c
@@ -3057,6 +3057,7 @@ int main(int argc, char **argv, char **envp)
     module_call_init(MODULE_INIT_OPTS);
 
     runstate_init();
+    precopy_infrastructure_init();
     postcopy_infrastructure_init();
     monitor_init_globals();
 
-- 
1.8.3.1


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Peter Xu]

On Thu, Nov 15, 2018 at 06:08:01PM +0800, Wei Wang wrote:
> This patch adds a notifier chain for the memory precopy. This enables various
> precopy optimizations to be invoked at specific places.
> 
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Peter Xu <peterx@redhat.com>
> ---
>  include/migration/misc.h | 12 ++++++++++++
>  migration/ram.c          | 31 +++++++++++++++++++++++++++++++
>  vl.c                     |  1 +
>  3 files changed, 44 insertions(+)
> 
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> index 113320e..0bac623 100644
> --- a/include/migration/misc.h
> +++ b/include/migration/misc.h
> @@ -19,6 +19,18 @@
>  
>  /* migration/ram.c */
>  
> +typedef enum PrecopyNotifyReason {
> +    PRECOPY_NOTIFY_ERR = 0,
> +    PRECOPY_NOTIFY_START_ITERATION = 1,
> +    PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2,
> +    PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3,
> +    PRECOPY_NOTIFY_MAX = 4,

It would be nice to add some comments for each of the notify reason.
E.g., from the name PRECOPY_NOTIFY_START_ITERATION seems more like a
hook at the start of each iteration but according to [1] it should be
at the start of migration rather than each iteration (or when
migration restarts, though I'm not sure whether we really have this
yet).

> +} PrecopyNotifyReason;
> +
> +void precopy_infrastructure_init(void);
> +void precopy_add_notifier(Notifier *n);
> +void precopy_remove_notifier(Notifier *n);
> +
>  void ram_mig_init(void);
>  void qemu_guest_free_page_hint(void *addr, size_t len);
>  
> diff --git a/migration/ram.c b/migration/ram.c
> index 229b791..65b1223 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -292,6 +292,8 @@ struct RAMState {
>      bool ram_bulk_stage;
>      /* How many times we have dirty too many pages */
>      int dirty_rate_high_cnt;
> +    /* ram save states used for notifiers */
> +    int ram_save_state;

This can be removed?

>      /* these variables are used for bitmap sync */
>      /* last time we did a full bitmap_sync */
>      int64_t time_last_bitmap_sync;
> @@ -328,6 +330,28 @@ typedef struct RAMState RAMState;
>  
>  static RAMState *ram_state;
>  
> +static NotifierList precopy_notifier_list;
> +
> +void precopy_infrastructure_init(void)
> +{
> +    notifier_list_init(&precopy_notifier_list);
> +}
> +
> +void precopy_add_notifier(Notifier *n)
> +{
> +    notifier_list_add(&precopy_notifier_list, n);
> +}
> +
> +void precopy_remove_notifier(Notifier *n)
> +{
> +    notifier_remove(n);
> +}
> +
> +static void precopy_notify(PrecopyNotifyReason reason)
> +{
> +    notifier_list_notify(&precopy_notifier_list, &reason);
> +}
> +
>  uint64_t ram_bytes_remaining(void)
>  {
>      return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
> @@ -1642,6 +1666,8 @@ static void migration_bitmap_sync(RAMState *rs)
>      int64_t end_time;
>      uint64_t bytes_xfer_now;
>  
> +    precopy_notify(PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP);
> +
>      ram_counters.dirty_sync_count++;
>  
>      if (!rs->time_last_bitmap_sync) {
> @@ -1699,6 +1725,8 @@ static void migration_bitmap_sync(RAMState *rs)
>      if (migrate_use_events()) {
>          qapi_event_send_migration_pass(ram_counters.dirty_sync_count);
>      }
> +
> +    precopy_notify(PRECOPY_NOTIFY_AFTER_SYNC_BITMAP);
>  }
>  
>  /**
> @@ -2555,6 +2583,8 @@ static void ram_state_reset(RAMState *rs)
>      rs->last_page = 0;
>      rs->last_version = ram_list.version;
>      rs->ram_bulk_stage = true;
> +
> +    precopy_notify(PRECOPY_NOTIFY_START_ITERATION);

[1]

>  }
>  
>  #define MAX_WAIT 50 /* ms, half buffered_file limit */
> @@ -3324,6 +3354,7 @@ out:
>  
>      ret = qemu_file_get_error(f);
>      if (ret < 0) {
> +        precopy_notify(PRECOPY_NOTIFY_ERR);

Could you show me which function is this line in?

Line 3324 here is ram_save_complete(), but I cannot find this exact
place.

Another thing to mention about the "reasons" (though I see it more
like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
It might help in some cases:

  - then you don't need to trickily export the migrate_postcopy()
    since you'll notify that before postcopy starts

  - you'll have a solid point that you'll 100% guarantee that we'll
    stop the free page hinting and don't need to worry about whether
    there is chance the hinting will be running without an end [2].

Regarding [2] above: now the series only stops the hinting when
PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP is notified.  Could there be a case
that it's missing?  E.g., what if we cancel/fail a migration during
precopy?  Have you tried it?

Regards,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

On 11/27/2018 03:38 PM, Peter Xu wrote:
> On Thu, Nov 15, 2018 at 06:08:01PM +0800, Wei Wang wrote:
>>   
>> +typedef enum PrecopyNotifyReason {
>> +    PRECOPY_NOTIFY_ERR = 0,
>> +    PRECOPY_NOTIFY_START_ITERATION = 1,
>> +    PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2,
>> +    PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3,
>> +    PRECOPY_NOTIFY_MAX = 4,
> It would be nice to add some comments for each of the notify reason.
> E.g., from the name PRECOPY_NOTIFY_START_ITERATION seems more like a
> hook at the start of each iteration but according to [1] it should be
> at the start of migration rather than each iteration (or when
> migration restarts, though I'm not sure whether we really have this
> yet).

OK. I think It would be better if the name itself could be straightforward.
Probably we could change PRECOPY_NOTIFY_START_ITERATION to
PRECOPY_NOTIFY_START_MIGRATION.


>> +} PrecopyNotifyReason;
>> +
>> +void precopy_infrastructure_init(void);
>> +void precopy_add_notifier(Notifier *n);
>> +void precopy_remove_notifier(Notifier *n);
>> +
>>   void ram_mig_init(void);
>>   void qemu_guest_free_page_hint(void *addr, size_t len);
>>   
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 229b791..65b1223 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -292,6 +292,8 @@ struct RAMState {
>>       bool ram_bulk_stage;
>>       /* How many times we have dirty too many pages */
>>       int dirty_rate_high_cnt;
>> +    /* ram save states used for notifiers */
>> +    int ram_save_state;
> This can be removed?

Yes, thanks.

>
>>       /* these variables are used for bitmap sync */
>>       /* last time we did a full bitmap_sync */
>>       int64_t time_last_bitmap_sync;
>> @@ -328,6 +330,28 @@ typedef struct RAMState RAMState;
>>   
>>   static RAMState *ram_state;
>>   
>> +static NotifierList precopy_notifier_list;
>> +
>> +void precopy_infrastructure_init(void)
>> +{
>> +    notifier_list_init(&precopy_notifier_list);
>> +}
>> +
>> +void precopy_add_notifier(Notifier *n)
>> +{
>> +    notifier_list_add(&precopy_notifier_list, n);
>> +}
>> +
>> +void precopy_remove_notifier(Notifier *n)
>> +{
>> +    notifier_remove(n);
>> +}
>> +
>> +static void precopy_notify(PrecopyNotifyReason reason)
>> +{
>> +    notifier_list_notify(&precopy_notifier_list, &reason);
>> +}
>> +
>>   uint64_t ram_bytes_remaining(void)
>>   {
>>       return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
>> @@ -1642,6 +1666,8 @@ static void migration_bitmap_sync(RAMState *rs)
>>       int64_t end_time;
>>       uint64_t bytes_xfer_now;
>>   
>> +    precopy_notify(PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP);
>> +
>>       ram_counters.dirty_sync_count++;
>>   
>>       if (!rs->time_last_bitmap_sync) {
>> @@ -1699,6 +1725,8 @@ static void migration_bitmap_sync(RAMState *rs)
>>       if (migrate_use_events()) {
>>           qapi_event_send_migration_pass(ram_counters.dirty_sync_count);
>>       }
>> +
>> +    precopy_notify(PRECOPY_NOTIFY_AFTER_SYNC_BITMAP);
>>   }
>>   
>>   /**
>> @@ -2555,6 +2583,8 @@ static void ram_state_reset(RAMState *rs)
>>       rs->last_page = 0;
>>       rs->last_version = ram_list.version;
>>       rs->ram_bulk_stage = true;
>> +
>> +    precopy_notify(PRECOPY_NOTIFY_START_ITERATION);
> [1]
>
>>   }
>>   
>>   #define MAX_WAIT 50 /* ms, half buffered_file limit */
>> @@ -3324,6 +3354,7 @@ out:
>>   
>>       ret = qemu_file_get_error(f);
>>       if (ret < 0) {
>> +        precopy_notify(PRECOPY_NOTIFY_ERR);
> Could you show me which function is this line in?
>
> Line 3324 here is ram_save_complete(), but I cannot find this exact
> place.

Sure, it's in ram_save_iterate():
...
out:
     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
     qemu_fflush(f);
     ram_counters.transferred += 8;

     ret = qemu_file_get_error(f);
     if (ret < 0) {
+        precopy_notify(PRECOPY_NOTIFY_ERR);
         return ret;
     }

     return done;
}


>
> Another thing to mention about the "reasons" (though I see it more
> like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
> It might help in some cases:
>
>    - then you don't need to trickily export the migrate_postcopy()
>      since you'll notify that before postcopy starts

I'm thinking probably we don't need to export migrate_postcopy even now.
It's more like a sanity check, and not needed because now we have the
notifier registered to the precopy specific callchain, which has ensured 
that
it is invoked via precopy.

>    - you'll have a solid point that you'll 100% guarantee that we'll
>      stop the free page hinting and don't need to worry about whether
>      there is chance the hinting will be running without an end [2].

Thanks, I think it makes sense. Plan to add PRECOPY_NOTIFY_END in 
ram_save_complete.


>
> Regarding [2] above: now the series only stops the hinting when
> PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP is notified.  Could there be a case
> that it's missing?  E.g., what if we cancel/fail a migration during
> precopy?  Have you tried it?
>

I think it has been handled by the above PRECOPY_NOTIFY_ERR

Best,
Wei

[virtio-dev] Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

On 11/27/2018 03:38 PM, Peter Xu wrote:
> On Thu, Nov 15, 2018 at 06:08:01PM +0800, Wei Wang wrote:
>>   
>> +typedef enum PrecopyNotifyReason {
>> +    PRECOPY_NOTIFY_ERR = 0,
>> +    PRECOPY_NOTIFY_START_ITERATION = 1,
>> +    PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2,
>> +    PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3,
>> +    PRECOPY_NOTIFY_MAX = 4,
> It would be nice to add some comments for each of the notify reason.
> E.g., from the name PRECOPY_NOTIFY_START_ITERATION seems more like a
> hook at the start of each iteration but according to [1] it should be
> at the start of migration rather than each iteration (or when
> migration restarts, though I'm not sure whether we really have this
> yet).

OK. I think It would be better if the name itself could be straightforward.
Probably we could change PRECOPY_NOTIFY_START_ITERATION to
PRECOPY_NOTIFY_START_MIGRATION.


>> +} PrecopyNotifyReason;
>> +
>> +void precopy_infrastructure_init(void);
>> +void precopy_add_notifier(Notifier *n);
>> +void precopy_remove_notifier(Notifier *n);
>> +
>>   void ram_mig_init(void);
>>   void qemu_guest_free_page_hint(void *addr, size_t len);
>>   
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 229b791..65b1223 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -292,6 +292,8 @@ struct RAMState {
>>       bool ram_bulk_stage;
>>       /* How many times we have dirty too many pages */
>>       int dirty_rate_high_cnt;
>> +    /* ram save states used for notifiers */
>> +    int ram_save_state;
> This can be removed?

Yes, thanks.

>
>>       /* these variables are used for bitmap sync */
>>       /* last time we did a full bitmap_sync */
>>       int64_t time_last_bitmap_sync;
>> @@ -328,6 +330,28 @@ typedef struct RAMState RAMState;
>>   
>>   static RAMState *ram_state;
>>   
>> +static NotifierList precopy_notifier_list;
>> +
>> +void precopy_infrastructure_init(void)
>> +{
>> +    notifier_list_init(&precopy_notifier_list);
>> +}
>> +
>> +void precopy_add_notifier(Notifier *n)
>> +{
>> +    notifier_list_add(&precopy_notifier_list, n);
>> +}
>> +
>> +void precopy_remove_notifier(Notifier *n)
>> +{
>> +    notifier_remove(n);
>> +}
>> +
>> +static void precopy_notify(PrecopyNotifyReason reason)
>> +{
>> +    notifier_list_notify(&precopy_notifier_list, &reason);
>> +}
>> +
>>   uint64_t ram_bytes_remaining(void)
>>   {
>>       return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
>> @@ -1642,6 +1666,8 @@ static void migration_bitmap_sync(RAMState *rs)
>>       int64_t end_time;
>>       uint64_t bytes_xfer_now;
>>   
>> +    precopy_notify(PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP);
>> +
>>       ram_counters.dirty_sync_count++;
>>   
>>       if (!rs->time_last_bitmap_sync) {
>> @@ -1699,6 +1725,8 @@ static void migration_bitmap_sync(RAMState *rs)
>>       if (migrate_use_events()) {
>>           qapi_event_send_migration_pass(ram_counters.dirty_sync_count);
>>       }
>> +
>> +    precopy_notify(PRECOPY_NOTIFY_AFTER_SYNC_BITMAP);
>>   }
>>   
>>   /**
>> @@ -2555,6 +2583,8 @@ static void ram_state_reset(RAMState *rs)
>>       rs->last_page = 0;
>>       rs->last_version = ram_list.version;
>>       rs->ram_bulk_stage = true;
>> +
>> +    precopy_notify(PRECOPY_NOTIFY_START_ITERATION);
> [1]
>
>>   }
>>   
>>   #define MAX_WAIT 50 /* ms, half buffered_file limit */
>> @@ -3324,6 +3354,7 @@ out:
>>   
>>       ret = qemu_file_get_error(f);
>>       if (ret < 0) {
>> +        precopy_notify(PRECOPY_NOTIFY_ERR);
> Could you show me which function is this line in?
>
> Line 3324 here is ram_save_complete(), but I cannot find this exact
> place.

Sure, it's in ram_save_iterate():
...
out:
     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
     qemu_fflush(f);
     ram_counters.transferred += 8;

     ret = qemu_file_get_error(f);
     if (ret < 0) {
+        precopy_notify(PRECOPY_NOTIFY_ERR);
         return ret;
     }

     return done;
}


>
> Another thing to mention about the "reasons" (though I see it more
> like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
> It might help in some cases:
>
>    - then you don't need to trickily export the migrate_postcopy()
>      since you'll notify that before postcopy starts

I'm thinking probably we don't need to export migrate_postcopy even now.
It's more like a sanity check, and not needed because now we have the
notifier registered to the precopy specific callchain, which has ensured 
that
it is invoked via precopy.

>    - you'll have a solid point that you'll 100% guarantee that we'll
>      stop the free page hinting and don't need to worry about whether
>      there is chance the hinting will be running without an end [2].

Thanks, I think it makes sense. Plan to add PRECOPY_NOTIFY_END in 
ram_save_complete.


>
> Regarding [2] above: now the series only stops the hinting when
> PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP is notified.  Could there be a case
> that it's missing?  E.g., what if we cancel/fail a migration during
> precopy?  Have you tried it?
>

I think it has been handled by the above PRECOPY_NOTIFY_ERR

Best,
Wei


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Peter Xu]

On Tue, Nov 27, 2018 at 06:25:40PM +0800, Wei Wang wrote:
> On 11/27/2018 03:38 PM, Peter Xu wrote:
> > On Thu, Nov 15, 2018 at 06:08:01PM +0800, Wei Wang wrote:
> > > +typedef enum PrecopyNotifyReason {
> > > +    PRECOPY_NOTIFY_ERR = 0,
> > > +    PRECOPY_NOTIFY_START_ITERATION = 1,
> > > +    PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2,
> > > +    PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3,
> > > +    PRECOPY_NOTIFY_MAX = 4,
> > It would be nice to add some comments for each of the notify reason.
> > E.g., from the name PRECOPY_NOTIFY_START_ITERATION seems more like a
> > hook at the start of each iteration but according to [1] it should be
> > at the start of migration rather than each iteration (or when
> > migration restarts, though I'm not sure whether we really have this
> > yet).
> 
> OK. I think It would be better if the name itself could be straightforward.
> Probably we could change PRECOPY_NOTIFY_START_ITERATION to
> PRECOPY_NOTIFY_START_MIGRATION.

Sounds good.

> 
> 
> > > +} PrecopyNotifyReason;
> > > +
> > > +void precopy_infrastructure_init(void);
> > > +void precopy_add_notifier(Notifier *n);
> > > +void precopy_remove_notifier(Notifier *n);
> > > +
> > >   void ram_mig_init(void);
> > >   void qemu_guest_free_page_hint(void *addr, size_t len);
> > > diff --git a/migration/ram.c b/migration/ram.c
> > > index 229b791..65b1223 100644
> > > --- a/migration/ram.c
> > > +++ b/migration/ram.c
> > > @@ -292,6 +292,8 @@ struct RAMState {
> > >       bool ram_bulk_stage;
> > >       /* How many times we have dirty too many pages */
> > >       int dirty_rate_high_cnt;
> > > +    /* ram save states used for notifiers */
> > > +    int ram_save_state;
> > This can be removed?
> 
> Yes, thanks.
> 
> > 
> > >       /* these variables are used for bitmap sync */
> > >       /* last time we did a full bitmap_sync */
> > >       int64_t time_last_bitmap_sync;
> > > @@ -328,6 +330,28 @@ typedef struct RAMState RAMState;
> > >   static RAMState *ram_state;
> > > +static NotifierList precopy_notifier_list;
> > > +
> > > +void precopy_infrastructure_init(void)
> > > +{
> > > +    notifier_list_init(&precopy_notifier_list);
> > > +}
> > > +
> > > +void precopy_add_notifier(Notifier *n)
> > > +{
> > > +    notifier_list_add(&precopy_notifier_list, n);
> > > +}
> > > +
> > > +void precopy_remove_notifier(Notifier *n)
> > > +{
> > > +    notifier_remove(n);
> > > +}
> > > +
> > > +static void precopy_notify(PrecopyNotifyReason reason)
> > > +{
> > > +    notifier_list_notify(&precopy_notifier_list, &reason);
> > > +}
> > > +
> > >   uint64_t ram_bytes_remaining(void)
> > >   {
> > >       return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
> > > @@ -1642,6 +1666,8 @@ static void migration_bitmap_sync(RAMState *rs)
> > >       int64_t end_time;
> > >       uint64_t bytes_xfer_now;
> > > +    precopy_notify(PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP);
> > > +
> > >       ram_counters.dirty_sync_count++;
> > >       if (!rs->time_last_bitmap_sync) {
> > > @@ -1699,6 +1725,8 @@ static void migration_bitmap_sync(RAMState *rs)
> > >       if (migrate_use_events()) {
> > >           qapi_event_send_migration_pass(ram_counters.dirty_sync_count);
> > >       }
> > > +
> > > +    precopy_notify(PRECOPY_NOTIFY_AFTER_SYNC_BITMAP);
> > >   }
> > >   /**
> > > @@ -2555,6 +2583,8 @@ static void ram_state_reset(RAMState *rs)
> > >       rs->last_page = 0;
> > >       rs->last_version = ram_list.version;
> > >       rs->ram_bulk_stage = true;
> > > +
> > > +    precopy_notify(PRECOPY_NOTIFY_START_ITERATION);
> > [1]
> > 
> > >   }
> > >   #define MAX_WAIT 50 /* ms, half buffered_file limit */
> > > @@ -3324,6 +3354,7 @@ out:
> > >       ret = qemu_file_get_error(f);
> > >       if (ret < 0) {
> > > +        precopy_notify(PRECOPY_NOTIFY_ERR);
> > Could you show me which function is this line in?
> > 
> > Line 3324 here is ram_save_complete(), but I cannot find this exact
> > place.
> 
> Sure, it's in ram_save_iterate():
> ...
> out:
>     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>     qemu_fflush(f);
>     ram_counters.transferred += 8;
> 
>     ret = qemu_file_get_error(f);
>     if (ret < 0) {
> +        precopy_notify(PRECOPY_NOTIFY_ERR);
>         return ret;
>     }
> 
>     return done;
> }

Ok thanks.  Please just make sure you will capture all the error
cases, e.g., I also see path like this (a few lines below):

        if (pages < 0) {
            qemu_file_set_error(f, pages);
            break;
        }

It seems that you missed that one.

I would even suggest that you capture the error with higher level.
E.g., in migration_iteration_run() after qemu_savevm_state_iterate().
Or we can just check the return value of qemu_savevm_state_iterate(),
which we have had ignored so far.

[1]

> 
> 
> > 
> > Another thing to mention about the "reasons" (though I see it more
> > like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
> > It might help in some cases:
> > 
> >    - then you don't need to trickily export the migrate_postcopy()
> >      since you'll notify that before postcopy starts
> 
> I'm thinking probably we don't need to export migrate_postcopy even now.
> It's more like a sanity check, and not needed because now we have the
> notifier registered to the precopy specific callchain, which has ensured
> that
> it is invoked via precopy.

But postcopy will always start with precopy, no?

> 
> >    - you'll have a solid point that you'll 100% guarantee that we'll
> >      stop the free page hinting and don't need to worry about whether
> >      there is chance the hinting will be running without an end [2].
> 
> Thanks, I think it makes sense. Plan to add PRECOPY_NOTIFY_END in
> ram_save_complete.

Yeah you can.

Btw, if you're mostly adding the notifies only in RAM-only codes, then
you can consider add the "RAM" into the names of events too to be
clear.

My suggestion at [1] is precopy general, but you can still capture it
at the end of ram_save_iterate, then they are RAM-only again.  Please
feel free to choose what fits more...

> 
> 
> > 
> > Regarding [2] above: now the series only stops the hinting when
> > PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP is notified.  Could there be a case
> > that it's missing?  E.g., what if we cancel/fail a migration during
> > precopy?  Have you tried it?
> > 
> 
> I think it has been handled by the above PRECOPY_NOTIFY_ERR

Indeed it should, as long as you're covering all the error cases.

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

On 11/28/2018 01:26 PM, Peter Xu wrote:
>
> Ok thanks.  Please just make sure you will capture all the error
> cases, e.g., I also see path like this (a few lines below):
>
>          if (pages < 0) {
>              qemu_file_set_error(f, pages);
>              break;
>          }
>
> It seems that you missed that one.

I think that one should be fine. This notification is actually put at the
bottom of ram_save_iterate. All the above error will bail out to the "out:"
path and then go to call precopy_notify(PRECOPY_NOTIFY_ERR).

>
> I would even suggest that you capture the error with higher level.
> E.g., in migration_iteration_run() after qemu_savevm_state_iterate().
> Or we can just check the return value of qemu_savevm_state_iterate(),
> which we have had ignored so far.

Not very sure about the higher level, because other SaveStateEntry may cause
errors that this feature don't need to care, I think we may only need it 
in ram_save.


> [1]
>
>>
>>> Another thing to mention about the "reasons" (though I see it more
>>> like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
>>> It might help in some cases:
>>>
>>>     - then you don't need to trickily export the migrate_postcopy()
>>>       since you'll notify that before postcopy starts
>> I'm thinking probably we don't need to export migrate_postcopy even now.
>> It's more like a sanity check, and not needed because now we have the
>> notifier registered to the precopy specific callchain, which has ensured
>> that
>> it is invoked via precopy.
> But postcopy will always start with precopy, no?

Yes, but I think we could add the check in precopy_notify()


>
>>>     - you'll have a solid point that you'll 100% guarantee that we'll
>>>       stop the free page hinting and don't need to worry about whether
>>>       there is chance the hinting will be running without an end [2].
>> Thanks, I think it makes sense. Plan to add PRECOPY_NOTIFY_END in
>> ram_save_complete.
> Yeah you can.
>
> Btw, if you're mostly adding the notifies only in RAM-only codes, then
> you can consider add the "RAM" into the names of events too to be
> clear.

Sounds good, will try and see if the name would become too long :)

>
> My suggestion at [1] is precopy general, but you can still capture it
> at the end of ram_save_iterate, then they are RAM-only again.  Please
> feel free to choose what fits more...

OK, thanks.

Best,
Wei

[virtio-dev] Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

On 11/28/2018 01:26 PM, Peter Xu wrote:
>
> Ok thanks.  Please just make sure you will capture all the error
> cases, e.g., I also see path like this (a few lines below):
>
>          if (pages < 0) {
>              qemu_file_set_error(f, pages);
>              break;
>          }
>
> It seems that you missed that one.

I think that one should be fine. This notification is actually put at the
bottom of ram_save_iterate. All the above error will bail out to the "out:"
path and then go to call precopy_notify(PRECOPY_NOTIFY_ERR).

>
> I would even suggest that you capture the error with higher level.
> E.g., in migration_iteration_run() after qemu_savevm_state_iterate().
> Or we can just check the return value of qemu_savevm_state_iterate(),
> which we have had ignored so far.

Not very sure about the higher level, because other SaveStateEntry may cause
errors that this feature don't need to care, I think we may only need it 
in ram_save.


> [1]
>
>>
>>> Another thing to mention about the "reasons" (though I see it more
>>> like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
>>> It might help in some cases:
>>>
>>>     - then you don't need to trickily export the migrate_postcopy()
>>>       since you'll notify that before postcopy starts
>> I'm thinking probably we don't need to export migrate_postcopy even now.
>> It's more like a sanity check, and not needed because now we have the
>> notifier registered to the precopy specific callchain, which has ensured
>> that
>> it is invoked via precopy.
> But postcopy will always start with precopy, no?

Yes, but I think we could add the check in precopy_notify()


>
>>>     - you'll have a solid point that you'll 100% guarantee that we'll
>>>       stop the free page hinting and don't need to worry about whether
>>>       there is chance the hinting will be running without an end [2].
>> Thanks, I think it makes sense. Plan to add PRECOPY_NOTIFY_END in
>> ram_save_complete.
> Yeah you can.
>
> Btw, if you're mostly adding the notifies only in RAM-only codes, then
> you can consider add the "RAM" into the names of events too to be
> clear.

Sounds good, will try and see if the name would become too long :)

>
> My suggestion at [1] is precopy general, but you can still capture it
> at the end of ram_save_iterate, then they are RAM-only again.  Please
> feel free to choose what fits more...

OK, thanks.

Best,
Wei

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Peter Xu]

On Wed, Nov 28, 2018 at 05:01:31PM +0800, Wei Wang wrote:
> On 11/28/2018 01:26 PM, Peter Xu wrote:
> > 
> > Ok thanks.  Please just make sure you will capture all the error
> > cases, e.g., I also see path like this (a few lines below):
> > 
> >          if (pages < 0) {
> >              qemu_file_set_error(f, pages);
> >              break;
> >          }
> > 
> > It seems that you missed that one.
> 
> I think that one should be fine. This notification is actually put at the
> bottom of ram_save_iterate. All the above error will bail out to the "out:"
> path and then go to call precopy_notify(PRECOPY_NOTIFY_ERR).

Ok, maybe I was pointing to a wrong one. :)

> 
> > 
> > I would even suggest that you capture the error with higher level.
> > E.g., in migration_iteration_run() after qemu_savevm_state_iterate().
> > Or we can just check the return value of qemu_savevm_state_iterate(),
> > which we have had ignored so far.
> 
> Not very sure about the higher level, because other SaveStateEntry may cause
> errors that this feature don't need to care, I think we may only need it in
> ram_save.

So what I am worrying here are corner cases where we might forget to
stop the hinting.  I'm fabricating one example sequence of events:

  (start migration)
  START_MIGRATION
  BEFORE_SYNC
  AFTER_SYNC
  ...
  BEFORE_SYNC
  AFTER_SYNC
  (some SaveStateEntry failed rather than RAM, then
   migration_detect_error returned MIG_THR_ERR_FATAL so we need to
   fail the migration, however when running the previous
   ram_save_iterate for RAM's specific SaveStateEntry we didn't see
   any error so no ERROR event detected)

Then it seems the hinting will last forever.  Considering that now I'm
not sure whether this can be done ram-only, since even if you capture
ram_save_complete() and at the same time you introduce PRECOPY_END you
may still miss the PRECOPY_END event since AFAIU ram_save_complete()
won't be called at all in this case.

Could this happen?

> 
> 
> > [1]
> > 
> > > 
> > > > Another thing to mention about the "reasons" (though I see it more
> > > > like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
> > > > It might help in some cases:
> > > > 
> > > >     - then you don't need to trickily export the migrate_postcopy()
> > > >       since you'll notify that before postcopy starts
> > > I'm thinking probably we don't need to export migrate_postcopy even now.
> > > It's more like a sanity check, and not needed because now we have the
> > > notifier registered to the precopy specific callchain, which has ensured
> > > that
> > > it is invoked via precopy.
> > But postcopy will always start with precopy, no?
> 
> Yes, but I think we could add the check in precopy_notify()

I'm not sure that's good.  If the notifier could potentially have
other user, they might still work with postcopy, and they might expect
e.g. BEFORE_SYNC to be called for every sync, even if it's at the
precopy stage of a postcopy.  In that sense I still feel the
PRECOPY_END is better (so contantly call it at the end of precopy, no
matter whether there's another postcopy afterwards).  It sounds like a
cleaner interface.

Or you can check it in the balloon specific callback and ignore the
event if postcopy is on, but then we're going backward to need to
export the API so it seems meaningless.

Regards,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

On 11/28/2018 05:32 PM, Peter Xu wrote:
>
> So what I am worrying here are corner cases where we might forget to
> stop the hinting.  I'm fabricating one example sequence of events:
>
>    (start migration)
>    START_MIGRATION
>    BEFORE_SYNC
>    AFTER_SYNC
>    ...
>    BEFORE_SYNC
>    AFTER_SYNC
>    (some SaveStateEntry failed rather than RAM, then
>     migration_detect_error returned MIG_THR_ERR_FATAL so we need to
>     fail the migration, however when running the previous
>     ram_save_iterate for RAM's specific SaveStateEntry we didn't see
>     any error so no ERROR event detected)
>
> Then it seems the hinting will last forever.  Considering that now I'm
> not sure whether this can be done ram-only, since even if you capture
> ram_save_complete() and at the same time you introduce PRECOPY_END you
> may still miss the PRECOPY_END event since AFAIU ram_save_complete()
> won't be called at all in this case.
>
> Could this happen?

Thanks, indeed this case could happen if we add PRECOPY_END in
ram_save_complete.

How about putting PRECOPY_END in ram_save_cleanup?
I think it would be called in any case.

I'm also thinking probably we don't need PRECOPY_ERR when we have 
PRECOPY_END,
and what do you think of the notifier names below:

+typedef enum PrecopyNotifyReason {
+    PRECOPY_NOTIFY_RAM_SAVE_END = 0,
+    PRECOPY_NOTIFY_RAM_SAVE_START = 1,
+    PRECOPY_NOTIFY_RAM_SAVE_BEFORE_SYNC_BITMAP = 2,
+    PRECOPY_NOTIFY_RAM_SAVE_AFTER_SYNC_BITMAP = 3,
+    PRECOPY_NOTIFY_RAM_SAVE_MAX = 4,
+} PrecopyNotifyReason;


>
>>
>>> [1]
>>>
>>>>> Another thing to mention about the "reasons" (though I see it more
>>>>> like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
>>>>> It might help in some cases:
>>>>>
>>>>>      - then you don't need to trickily export the migrate_postcopy()
>>>>>        since you'll notify that before postcopy starts
>>>> I'm thinking probably we don't need to export migrate_postcopy even now.
>>>> It's more like a sanity check, and not needed because now we have the
>>>> notifier registered to the precopy specific callchain, which has ensured
>>>> that
>>>> it is invoked via precopy.
>>> But postcopy will always start with precopy, no?
>> Yes, but I think we could add the check in precopy_notify()
> I'm not sure that's good.  If the notifier could potentially have
> other user, they might still work with postcopy, and they might expect
> e.g. BEFORE_SYNC to be called for every sync, even if it's at the
> precopy stage of a postcopy.

I think this precopy notifier callchain is expected to be used only for
the precopy mode. Postcopy has its dedicated notifier callchain that
users could use.

How about changing the migrate_postcopy() check to "ms->start_postcopy":

bool migration_postcopy_start(void)
{
     MigrationState *s;

     s = migrate_get_current();

     return atomic_read(&s->start_postcopy);
}


static void precopy_notify(PrecopyNotifyReason reason)
{
     if (migration_postcopy_start())
         return;

     notifier_list_notify(&precopy_notifier_list, &reason);
}

If postcopy started with precopy, the precopy optimization feature
could still be used until it switches to the postcopy mode.



> In that sense I still feel the
> PRECOPY_END is better (so contantly call it at the end of precopy, no
> matter whether there's another postcopy afterwards).  It sounds like a
> cleaner interface.

Probably I still haven't got the point how PRECOPY_END could help above yet.

Best,
Wei

[virtio-dev] Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

On 11/28/2018 05:32 PM, Peter Xu wrote:
>
> So what I am worrying here are corner cases where we might forget to
> stop the hinting.  I'm fabricating one example sequence of events:
>
>    (start migration)
>    START_MIGRATION
>    BEFORE_SYNC
>    AFTER_SYNC
>    ...
>    BEFORE_SYNC
>    AFTER_SYNC
>    (some SaveStateEntry failed rather than RAM, then
>     migration_detect_error returned MIG_THR_ERR_FATAL so we need to
>     fail the migration, however when running the previous
>     ram_save_iterate for RAM's specific SaveStateEntry we didn't see
>     any error so no ERROR event detected)
>
> Then it seems the hinting will last forever.  Considering that now I'm
> not sure whether this can be done ram-only, since even if you capture
> ram_save_complete() and at the same time you introduce PRECOPY_END you
> may still miss the PRECOPY_END event since AFAIU ram_save_complete()
> won't be called at all in this case.
>
> Could this happen?

Thanks, indeed this case could happen if we add PRECOPY_END in
ram_save_complete.

How about putting PRECOPY_END in ram_save_cleanup?
I think it would be called in any case.

I'm also thinking probably we don't need PRECOPY_ERR when we have 
PRECOPY_END,
and what do you think of the notifier names below:

+typedef enum PrecopyNotifyReason {
+    PRECOPY_NOTIFY_RAM_SAVE_END = 0,
+    PRECOPY_NOTIFY_RAM_SAVE_START = 1,
+    PRECOPY_NOTIFY_RAM_SAVE_BEFORE_SYNC_BITMAP = 2,
+    PRECOPY_NOTIFY_RAM_SAVE_AFTER_SYNC_BITMAP = 3,
+    PRECOPY_NOTIFY_RAM_SAVE_MAX = 4,
+} PrecopyNotifyReason;


>
>>
>>> [1]
>>>
>>>>> Another thing to mention about the "reasons" (though I see it more
>>>>> like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
>>>>> It might help in some cases:
>>>>>
>>>>>      - then you don't need to trickily export the migrate_postcopy()
>>>>>        since you'll notify that before postcopy starts
>>>> I'm thinking probably we don't need to export migrate_postcopy even now.
>>>> It's more like a sanity check, and not needed because now we have the
>>>> notifier registered to the precopy specific callchain, which has ensured
>>>> that
>>>> it is invoked via precopy.
>>> But postcopy will always start with precopy, no?
>> Yes, but I think we could add the check in precopy_notify()
> I'm not sure that's good.  If the notifier could potentially have
> other user, they might still work with postcopy, and they might expect
> e.g. BEFORE_SYNC to be called for every sync, even if it's at the
> precopy stage of a postcopy.

I think this precopy notifier callchain is expected to be used only for
the precopy mode. Postcopy has its dedicated notifier callchain that
users could use.

How about changing the migrate_postcopy() check to "ms->start_postcopy":

bool migration_postcopy_start(void)
{
     MigrationState *s;

     s = migrate_get_current();

     return atomic_read(&s->start_postcopy);
}


static void precopy_notify(PrecopyNotifyReason reason)
{
     if (migration_postcopy_start())
         return;

     notifier_list_notify(&precopy_notifier_list, &reason);
}

If postcopy started with precopy, the precopy optimization feature
could still be used until it switches to the postcopy mode.



> In that sense I still feel the
> PRECOPY_END is better (so contantly call it at the end of precopy, no
> matter whether there's another postcopy afterwards).  It sounds like a
> cleaner interface.

Probably I still haven't got the point how PRECOPY_END could help above yet.

Best,
Wei


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Peter Xu]

On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
> On 11/28/2018 05:32 PM, Peter Xu wrote:
> > 
> > So what I am worrying here are corner cases where we might forget to
> > stop the hinting.  I'm fabricating one example sequence of events:
> > 
> >    (start migration)
> >    START_MIGRATION
> >    BEFORE_SYNC
> >    AFTER_SYNC
> >    ...
> >    BEFORE_SYNC
> >    AFTER_SYNC
> >    (some SaveStateEntry failed rather than RAM, then
> >     migration_detect_error returned MIG_THR_ERR_FATAL so we need to
> >     fail the migration, however when running the previous
> >     ram_save_iterate for RAM's specific SaveStateEntry we didn't see
> >     any error so no ERROR event detected)
> > 
> > Then it seems the hinting will last forever.  Considering that now I'm
> > not sure whether this can be done ram-only, since even if you capture
> > ram_save_complete() and at the same time you introduce PRECOPY_END you
> > may still miss the PRECOPY_END event since AFAIU ram_save_complete()
> > won't be called at all in this case.
> > 
> > Could this happen?
> 
> Thanks, indeed this case could happen if we add PRECOPY_END in
> ram_save_complete.
> 
> How about putting PRECOPY_END in ram_save_cleanup?
> I think it would be called in any case.

Sounds good.

> 
> I'm also thinking probably we don't need PRECOPY_ERR when we have
> PRECOPY_END,
> and what do you think of the notifier names below:
> 
> +typedef enum PrecopyNotifyReason {
> +    PRECOPY_NOTIFY_RAM_SAVE_END = 0,
> +    PRECOPY_NOTIFY_RAM_SAVE_START = 1,
> +    PRECOPY_NOTIFY_RAM_SAVE_BEFORE_SYNC_BITMAP = 2,
> +    PRECOPY_NOTIFY_RAM_SAVE_AFTER_SYNC_BITMAP = 3,
> +    PRECOPY_NOTIFY_RAM_SAVE_MAX = 4,
> +} PrecopyNotifyReason;

(please see below [1]...)

> 
> 
> > 
> > > 
> > > > [1]
> > > > 
> > > > > > Another thing to mention about the "reasons" (though I see it more
> > > > > > like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
> > > > > > It might help in some cases:
> > > > > > 
> > > > > >      - then you don't need to trickily export the migrate_postcopy()
> > > > > >        since you'll notify that before postcopy starts
> > > > > I'm thinking probably we don't need to export migrate_postcopy even now.
> > > > > It's more like a sanity check, and not needed because now we have the
> > > > > notifier registered to the precopy specific callchain, which has ensured
> > > > > that
> > > > > it is invoked via precopy.
> > > > But postcopy will always start with precopy, no?
> > > Yes, but I think we could add the check in precopy_notify()
> > I'm not sure that's good.  If the notifier could potentially have
> > other user, they might still work with postcopy, and they might expect
> > e.g. BEFORE_SYNC to be called for every sync, even if it's at the
> > precopy stage of a postcopy.
> 
> I think this precopy notifier callchain is expected to be used only for
> the precopy mode. Postcopy has its dedicated notifier callchain that
> users could use.
> 
> How about changing the migrate_postcopy() check to "ms->start_postcopy":
> 
> bool migration_postcopy_start(void)
> {
>     MigrationState *s;
> 
>     s = migrate_get_current();
> 
>     return atomic_read(&s->start_postcopy);
> }
> 
> 
> static void precopy_notify(PrecopyNotifyReason reason)
> {
>     if (migration_postcopy_start())
>         return;
> 
>     notifier_list_notify(&precopy_notifier_list, &reason);
> }
> 
> If postcopy started with precopy, the precopy optimization feature
> could still be used until it switches to the postcopy mode.

I'm not sure we can use start_postcopy.  It's a variable being set in
the QMP handler but it does not mean postcopy has started.  I'm afraid
there can be race where it's still precopy but the variable is set so
event could be missed...

IMHO the problem is not that complicated.  How about this proposal:

[1]

  typedef enum PrecopyNotifyReason {
    PRECOPY_NOTIFY_RAM_START,
    PRECOPY_NOTIFY_RAM_BEFORE_SYNC,
    PRECOPY_NOTIFY_RAM_AFTER_SYNC,
    PRECOPY_NOTIFY_COMPLETE,
    PRECOPY_NOTIFY_RAM_CLEANUP,
  };

The first three keep the same as your old ones.  Notify RAM_CLEANUP in
ram_save_cleanup() to make sure it'll always be cleaned up (the same
as PRECOPY_END, just another name).  Notify COMPLETE in
qemu_savevm_state_complete_precopy() to show that precopy is
completed.  Meanwhile on balloon side you should stop the hinting for
either RAM_CLEANUP or COMPLETE event.  Then either:

  - precopy is switching to postcopy, or
  - precopy completed, or
  - precopy failed/cancelled

You should always get at least a notification to stop the balloon.
Though you could also get one RAM_CLEANUP after one COMPLETE, but
the balloon should easily handle it (stop the hinting twice).

Here maybe you can even remove the "RAM_" in both RAM_START and
RAM_CLEANUP if we're going to have COMPLETE since after all it'll be
not only limited to RAM.

Another suggestion is that you can add an Error into the notify hooks,
please refer to the postcopy one:

  int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp);

So the hook functions have a way to even stop the migration (though
for balloon hinting it'll be always optional so no error should be
reported...), then the two interfaces are matched.

> 
> 
> 
> > In that sense I still feel the
> > PRECOPY_END is better (so contantly call it at the end of precopy, no
> > matter whether there's another postcopy afterwards).  It sounds like a
> > cleaner interface.
> 
> Probably I still haven't got the point how PRECOPY_END could help above yet.

Please have a look at above proposal.  Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Peter Xu]

On Thu, Nov 29, 2018 at 01:10:14PM +0800, Peter Xu wrote:
> On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
> > On 11/28/2018 05:32 PM, Peter Xu wrote:
> > > 
> > > So what I am worrying here are corner cases where we might forget to
> > > stop the hinting.  I'm fabricating one example sequence of events:
> > > 
> > >    (start migration)
> > >    START_MIGRATION
> > >    BEFORE_SYNC
> > >    AFTER_SYNC
> > >    ...
> > >    BEFORE_SYNC
> > >    AFTER_SYNC
> > >    (some SaveStateEntry failed rather than RAM, then
> > >     migration_detect_error returned MIG_THR_ERR_FATAL so we need to
> > >     fail the migration, however when running the previous
> > >     ram_save_iterate for RAM's specific SaveStateEntry we didn't see
> > >     any error so no ERROR event detected)
> > > 
> > > Then it seems the hinting will last forever.  Considering that now I'm
> > > not sure whether this can be done ram-only, since even if you capture
> > > ram_save_complete() and at the same time you introduce PRECOPY_END you
> > > may still miss the PRECOPY_END event since AFAIU ram_save_complete()
> > > won't be called at all in this case.
> > > 
> > > Could this happen?
> > 
> > Thanks, indeed this case could happen if we add PRECOPY_END in
> > ram_save_complete.
> > 
> > How about putting PRECOPY_END in ram_save_cleanup?
> > I think it would be called in any case.
> 
> Sounds good.
> 
> > 
> > I'm also thinking probably we don't need PRECOPY_ERR when we have
> > PRECOPY_END,
> > and what do you think of the notifier names below:
> > 
> > +typedef enum PrecopyNotifyReason {
> > +    PRECOPY_NOTIFY_RAM_SAVE_END = 0,
> > +    PRECOPY_NOTIFY_RAM_SAVE_START = 1,
> > +    PRECOPY_NOTIFY_RAM_SAVE_BEFORE_SYNC_BITMAP = 2,
> > +    PRECOPY_NOTIFY_RAM_SAVE_AFTER_SYNC_BITMAP = 3,
> > +    PRECOPY_NOTIFY_RAM_SAVE_MAX = 4,
> > +} PrecopyNotifyReason;
> 
> (please see below [1]...)
> 
> > 
> > 
> > > 
> > > > 
> > > > > [1]
> > > > > 
> > > > > > > Another thing to mention about the "reasons" (though I see it more
> > > > > > > like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
> > > > > > > It might help in some cases:
> > > > > > > 
> > > > > > >      - then you don't need to trickily export the migrate_postcopy()
> > > > > > >        since you'll notify that before postcopy starts
> > > > > > I'm thinking probably we don't need to export migrate_postcopy even now.
> > > > > > It's more like a sanity check, and not needed because now we have the
> > > > > > notifier registered to the precopy specific callchain, which has ensured
> > > > > > that
> > > > > > it is invoked via precopy.
> > > > > But postcopy will always start with precopy, no?
> > > > Yes, but I think we could add the check in precopy_notify()
> > > I'm not sure that's good.  If the notifier could potentially have
> > > other user, they might still work with postcopy, and they might expect
> > > e.g. BEFORE_SYNC to be called for every sync, even if it's at the
> > > precopy stage of a postcopy.
> > 
> > I think this precopy notifier callchain is expected to be used only for
> > the precopy mode. Postcopy has its dedicated notifier callchain that
> > users could use.
> > 
> > How about changing the migrate_postcopy() check to "ms->start_postcopy":
> > 
> > bool migration_postcopy_start(void)
> > {
> >     MigrationState *s;
> > 
> >     s = migrate_get_current();
> > 
> >     return atomic_read(&s->start_postcopy);
> > }
> > 
> > 
> > static void precopy_notify(PrecopyNotifyReason reason)
> > {
> >     if (migration_postcopy_start())
> >         return;
> > 
> >     notifier_list_notify(&precopy_notifier_list, &reason);
> > }
> > 
> > If postcopy started with precopy, the precopy optimization feature
> > could still be used until it switches to the postcopy mode.
> 
> I'm not sure we can use start_postcopy.  It's a variable being set in
> the QMP handler but it does not mean postcopy has started.  I'm afraid
> there can be race where it's still precopy but the variable is set so
> event could be missed...
> 
> IMHO the problem is not that complicated.  How about this proposal:
> 
> [1]
> 
>   typedef enum PrecopyNotifyReason {
>     PRECOPY_NOTIFY_RAM_START,
>     PRECOPY_NOTIFY_RAM_BEFORE_SYNC,
>     PRECOPY_NOTIFY_RAM_AFTER_SYNC,
>     PRECOPY_NOTIFY_COMPLETE,
>     PRECOPY_NOTIFY_RAM_CLEANUP,
>   };
> 
> The first three keep the same as your old ones.  Notify RAM_CLEANUP in
> ram_save_cleanup() to make sure it'll always be cleaned up (the same
> as PRECOPY_END, just another name).  Notify COMPLETE in
> qemu_savevm_state_complete_precopy() to show that precopy is
> completed.  Meanwhile on balloon side you should stop the hinting for
> either RAM_CLEANUP or COMPLETE event.  Then either:
> 
>   - precopy is switching to postcopy, or
>   - precopy completed, or
>   - precopy failed/cancelled
> 
> You should always get at least a notification to stop the balloon.
> Though you could also get one RAM_CLEANUP after one COMPLETE, but
> the balloon should easily handle it (stop the hinting twice).
> 
> Here maybe you can even remove the "RAM_" in both RAM_START and
> RAM_CLEANUP if we're going to have COMPLETE since after all it'll be
> not only limited to RAM.

Oh maybe we can remove all the RAM_ prefix to make it precopy
general...

  typedef enum PrecopyNotifyReason {
    PRECOPY_NOTIFY_SETUP,
    PRECOPY_NOTIFY_BEFORE_SYNC,
    PRECOPY_NOTIFY_AFTER_SYNC,
    PRECOPY_NOTIFY_COMPLETE,
    PRECOPY_NOTIFY_CLEANUP,
  };

Then we can just hook everything with the corresponding names:

  SETUP:    hooks with qemu_savevm_state_setup
  COMPLETE: hooks with qemu_savevm_state_complete_precopy
  CLEANUP:  hooks with qemu_savevm_state_cleanup

I'm not sure whether you'll need another hook in ram_state_reset in
the future but for now I don't see it necessary since I don't thnk
ram_list.version would change during migration for now, so
ram_state_reset should only be called during setup.

> 
> Another suggestion is that you can add an Error into the notify hooks,
> please refer to the postcopy one:
> 
>   int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp);
> 
> So the hook functions have a way to even stop the migration (though
> for balloon hinting it'll be always optional so no error should be
> reported...), then the two interfaces are matched.
> 
> > 
> > 
> > 
> > > In that sense I still feel the
> > > PRECOPY_END is better (so contantly call it at the end of precopy, no
> > > matter whether there's another postcopy afterwards).  It sounds like a
> > > cleaner interface.
> > 
> > Probably I still haven't got the point how PRECOPY_END could help above yet.
> 
> Please have a look at above proposal.  Thanks,
> 
> -- 
> Peter Xu
> 

Regards,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

On 11/29/2018 01:10 PM, Peter Xu wrote:
> On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
>> On 11/28/2018 05:32 PM, Peter Xu wrote:
> I'm not sure we can use start_postcopy.  It's a variable being set in
> the QMP handler but it does not mean postcopy has started.  I'm afraid
> there can be race where it's still precopy but the variable is set so
> event could be missed...
>
> IMHO the problem is not that complicated.  How about this proposal:
>
> [1]
>
>    typedef enum PrecopyNotifyReason {
>      PRECOPY_NOTIFY_RAM_START,
>      PRECOPY_NOTIFY_RAM_BEFORE_SYNC,
>      PRECOPY_NOTIFY_RAM_AFTER_SYNC,
>      PRECOPY_NOTIFY_COMPLETE,
>      PRECOPY_NOTIFY_RAM_CLEANUP,
>    };
>
> The first three keep the same as your old ones.  Notify RAM_CLEANUP in
> ram_save_cleanup() to make sure it'll always be cleaned up (the same
> as PRECOPY_END, just another name).  Notify COMPLETE in
> qemu_savevm_state_complete_precopy() to show that precopy is
> completed.  Meanwhile on balloon side you should stop the hinting for
> either RAM_CLEANUP or COMPLETE event.  Then either:
>
>    - precopy is switching to postcopy, or
>    - precopy completed, or
>    - precopy failed/cancelled
>
> You should always get at least a notification to stop the balloon.
> Though you could also get one RAM_CLEANUP after one COMPLETE, but
> the balloon should easily handle it (stop the hinting twice).

Sounds better, thanks.


>
> Here maybe you can even remove the "RAM_" in both RAM_START and
> RAM_CLEANUP if we're going to have COMPLETE since after all it'll be
> not only limited to RAM.
>
> Another suggestion is that you can add an Error into the notify hooks,
> please refer to the postcopy one:
>
>    int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp);
>
> So the hook functions have a way to even stop the migration (though
> for balloon hinting it'll be always optional so no error should be
> reported...), then the two interfaces are matched.

Yes, I removed that "errp" as it's not needed by the balloon usage.
But no problem, I can add it back if no objections from others.

Best,
Wei

[virtio-dev] Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

On 11/29/2018 01:10 PM, Peter Xu wrote:
> On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
>> On 11/28/2018 05:32 PM, Peter Xu wrote:
> I'm not sure we can use start_postcopy.  It's a variable being set in
> the QMP handler but it does not mean postcopy has started.  I'm afraid
> there can be race where it's still precopy but the variable is set so
> event could be missed...
>
> IMHO the problem is not that complicated.  How about this proposal:
>
> [1]
>
>    typedef enum PrecopyNotifyReason {
>      PRECOPY_NOTIFY_RAM_START,
>      PRECOPY_NOTIFY_RAM_BEFORE_SYNC,
>      PRECOPY_NOTIFY_RAM_AFTER_SYNC,
>      PRECOPY_NOTIFY_COMPLETE,
>      PRECOPY_NOTIFY_RAM_CLEANUP,
>    };
>
> The first three keep the same as your old ones.  Notify RAM_CLEANUP in
> ram_save_cleanup() to make sure it'll always be cleaned up (the same
> as PRECOPY_END, just another name).  Notify COMPLETE in
> qemu_savevm_state_complete_precopy() to show that precopy is
> completed.  Meanwhile on balloon side you should stop the hinting for
> either RAM_CLEANUP or COMPLETE event.  Then either:
>
>    - precopy is switching to postcopy, or
>    - precopy completed, or
>    - precopy failed/cancelled
>
> You should always get at least a notification to stop the balloon.
> Though you could also get one RAM_CLEANUP after one COMPLETE, but
> the balloon should easily handle it (stop the hinting twice).

Sounds better, thanks.


>
> Here maybe you can even remove the "RAM_" in both RAM_START and
> RAM_CLEANUP if we're going to have COMPLETE since after all it'll be
> not only limited to RAM.
>
> Another suggestion is that you can add an Error into the notify hooks,
> please refer to the postcopy one:
>
>    int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp);
>
> So the hook functions have a way to even stop the migration (though
> for balloon hinting it'll be always optional so no error should be
> reported...), then the two interfaces are matched.

Yes, I removed that "errp" as it's not needed by the balloon usage.
But no problem, I can add it back if no objections from others.

Best,
Wei



---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

On 11/29/2018 01:10 PM, Peter Xu wrote:
> On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
> I think this precopy notifier callchain is expected to be used only for
> the precopy mode. Postcopy has its dedicated notifier callchain that
> users could use.
>
> How about changing the migrate_postcopy() check to "ms->start_postcopy":
>
> bool migration_postcopy_start(void)
> {
>      MigrationState *s;
>
>      s = migrate_get_current();
>
>      return atomic_read(&s->start_postcopy);
> }
>
>
> static void precopy_notify(PrecopyNotifyReason reason)
> {
>      if (migration_postcopy_start())
>          return;
>
>      notifier_list_notify(&precopy_notifier_list, &reason);
> }
>
> If postcopy started with precopy, the precopy optimization feature
> could still be used until it switches to the postcopy mode.
> I'm not sure we can use start_postcopy.  It's a variable being set in
> the QMP handler but it does not mean postcopy has started.  I'm afraid
> there can be race where it's still precopy but the variable is set so
> event could be missed...

Peter,
I just found that migration_bitmap_sync is also called by 
ram_postcopy_send_discard_bitmap().
But we don't expect notifier_list_notify to be called in the postcopy mode.

So, probably we still need the start_postcopy check in 
notifier_list_notify though we have
the COMPLETE notifier.

Best,
Wei

[virtio-dev] Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

On 11/29/2018 01:10 PM, Peter Xu wrote:
> On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
> I think this precopy notifier callchain is expected to be used only for
> the precopy mode. Postcopy has its dedicated notifier callchain that
> users could use.
>
> How about changing the migrate_postcopy() check to "ms->start_postcopy":
>
> bool migration_postcopy_start(void)
> {
>      MigrationState *s;
>
>      s = migrate_get_current();
>
>      return atomic_read(&s->start_postcopy);
> }
>
>
> static void precopy_notify(PrecopyNotifyReason reason)
> {
>      if (migration_postcopy_start())
>          return;
>
>      notifier_list_notify(&precopy_notifier_list, &reason);
> }
>
> If postcopy started with precopy, the precopy optimization feature
> could still be used until it switches to the postcopy mode.
> I'm not sure we can use start_postcopy.  It's a variable being set in
> the QMP handler but it does not mean postcopy has started.  I'm afraid
> there can be race where it's still precopy but the variable is set so
> event could be missed...

Peter,
I just found that migration_bitmap_sync is also called by 
ram_postcopy_send_discard_bitmap().
But we don't expect notifier_list_notify to be called in the postcopy mode.

So, probably we still need the start_postcopy check in 
notifier_list_notify though we have
the COMPLETE notifier.

Best,
Wei


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Peter Xu]

On Fri, Nov 30, 2018 at 01:05:51PM +0800, Wei Wang wrote:
> On 11/29/2018 01:10 PM, Peter Xu wrote:
> > On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
> > I think this precopy notifier callchain is expected to be used only for
> > the precopy mode. Postcopy has its dedicated notifier callchain that
> > users could use.
> > 
> > How about changing the migrate_postcopy() check to "ms->start_postcopy":
> > 
> > bool migration_postcopy_start(void)
> > {
> >      MigrationState *s;
> > 
> >      s = migrate_get_current();
> > 
> >      return atomic_read(&s->start_postcopy);
> > }
> > 
> > 
> > static void precopy_notify(PrecopyNotifyReason reason)
> > {
> >      if (migration_postcopy_start())
> >          return;
> > 
> >      notifier_list_notify(&precopy_notifier_list, &reason);
> > }
> > 
> > If postcopy started with precopy, the precopy optimization feature
> > could still be used until it switches to the postcopy mode.
> > I'm not sure we can use start_postcopy.  It's a variable being set in
> > the QMP handler but it does not mean postcopy has started.  I'm afraid
> > there can be race where it's still precopy but the variable is set so
> > event could be missed...
> 
> Peter,
> I just found that migration_bitmap_sync is also called by
> ram_postcopy_send_discard_bitmap().
> But we don't expect notifier_list_notify to be called in the postcopy mode.
> 
> So, probably we still need the start_postcopy check in notifier_list_notify
> though we have
> the COMPLETE notifier.

I still don't think using start_postcopy is a good idea, as
explained in my previous reply.

Maybe move the notify()s outside migration_bitmap_sync() but only at
the call sites in precopy?  Say these three:

ram_init_bitmaps[3508]         migration_bitmap_sync(rs);
ram_save_complete[3731]        migration_bitmap_sync(rs);
ram_save_pending[3776]         migration_bitmap_sync(rs);

Or you can introduce migration_bitmap_sync_precopy() and let precopy
code call that one instead.

PS. I'm a bit unsure on why we need to sync bitmap in ram_init_bitmaps.
I feel like it can be removed...

Regards,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

On 11/30/2018 01:57 PM, Peter Xu wrote:
>>
>>
>> Or you can introduce migration_bitmap_sync_precopy() and let precopy
>> code call that one instead.

Agree, thanks.

> PS. I'm a bit unsure on why we need to sync bitmap in ram_init_bitmaps.
> I feel like it can be removed...

Seems it was added in early days to fix some bug:
https://git.virtualopensystems.com/dev/qemu/commit/79536f4f16934d6759a1d67f0342b4e7ceb66671?w=1

Hopefully, Juan could share some details about that.

Best,
Wei

[virtio-dev] Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy

[Wei Wang]

On 11/30/2018 01:57 PM, Peter Xu wrote:
>>
>>
>> Or you can introduce migration_bitmap_sync_precopy() and let precopy
>> code call that one instead.

Agree, thanks.

> PS. I'm a bit unsure on why we need to sync bitmap in ram_init_bitmaps.
> I feel like it can be removed...

Seems it was added in early days to fix some bug:
https://git.virtualopensystems.com/dev/qemu/commit/79536f4f16934d6759a1d67f0342b4e7ceb66671?w=1

Hopefully, Juan could share some details about that.

Best,
Wei





---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[Qemu-devel] [PATCH v9 6/8] migration/ram.c: add a function to disable the bulk stage

[Wei Wang]

This patch adds a function to enable a precopy notifier callback outside
the migration subsystem to disable the bulk stage flag. This is needed by
the free page optimization offered by virtio-balloon.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
 include/migration/misc.h | 1 +
 migration/ram.c          | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 0bac623..67cb275 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -30,6 +30,7 @@ typedef enum PrecopyNotifyReason {
 void precopy_infrastructure_init(void);
 void precopy_add_notifier(Notifier *n);
 void precopy_remove_notifier(Notifier *n);
+void precopy_disable_bulk_stage(void);
 
 void ram_mig_init(void);
 void qemu_guest_free_page_hint(void *addr, size_t len);
diff --git a/migration/ram.c b/migration/ram.c
index 65b1223..8745ca3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -352,6 +352,15 @@ static void precopy_notify(PrecopyNotifyReason reason)
     notifier_list_notify(&precopy_notifier_list, &reason);
 }
 
+void precopy_disable_bulk_stage(void)
+{
+    if (!ram_state) {
+        return;
+    }
+
+    ram_state->ram_bulk_stage = false;
+}
+
 uint64_t ram_bytes_remaining(void)
 {
     return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
-- 
1.8.3.1

[virtio-dev] [PATCH v9 6/8] migration/ram.c: add a function to disable the bulk stage

[Wei Wang]

This patch adds a function to enable a precopy notifier callback outside
the migration subsystem to disable the bulk stage flag. This is needed by
the free page optimization offered by virtio-balloon.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
 include/migration/misc.h | 1 +
 migration/ram.c          | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 0bac623..67cb275 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -30,6 +30,7 @@ typedef enum PrecopyNotifyReason {
 void precopy_infrastructure_init(void);
 void precopy_add_notifier(Notifier *n);
 void precopy_remove_notifier(Notifier *n);
+void precopy_disable_bulk_stage(void);
 
 void ram_mig_init(void);
 void qemu_guest_free_page_hint(void *addr, size_t len);
diff --git a/migration/ram.c b/migration/ram.c
index 65b1223..8745ca3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -352,6 +352,15 @@ static void precopy_notify(PrecopyNotifyReason reason)
     notifier_list_notify(&precopy_notifier_list, &reason);
 }
 
+void precopy_disable_bulk_stage(void)
+{
+    if (!ram_state) {
+        return;
+    }
+
+    ram_state->ram_bulk_stage = false;
+}
+
 uint64_t ram_bytes_remaining(void)
 {
     return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
-- 
1.8.3.1


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[Qemu-devel] [PATCH v9 7/8] migration: move migrate_postcopy() to include/migration/misc.h

[Wei Wang]

The ram save state notifier callback, for example the free page
optimization offerred by virtio-balloon, may need to check if
postcopy is in use, so move migrate_postcopy() to the outside header.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/migration/misc.h | 1 +
 migration/migration.h    | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 67cb275..06b816f 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -70,6 +70,7 @@ bool migration_has_failed(MigrationState *);
 /* ...and after the device transmission */
 bool migration_in_postcopy_after_devices(MigrationState *);
 void migration_global_dump(Monitor *mon);
+bool migrate_postcopy(void);
 
 /* migration/block-dirty-bitmap.c */
 void dirty_bitmap_mig_init(void);
diff --git a/migration/migration.h b/migration/migration.h
index e413d4d..e1e92ff 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -249,8 +249,6 @@ bool migration_is_blocked(Error **errp);
 bool migration_in_postcopy(void);
 MigrationState *migrate_get_current(void);
 
-bool migrate_postcopy(void);
-
 bool migrate_release_ram(void);
 bool migrate_postcopy_ram(void);
 bool migrate_zero_blocks(void);
-- 
1.8.3.1

[virtio-dev] [PATCH v9 7/8] migration: move migrate_postcopy() to include/migration/misc.h

[Wei Wang]

The ram save state notifier callback, for example the free page
optimization offerred by virtio-balloon, may need to check if
postcopy is in use, so move migrate_postcopy() to the outside header.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/migration/misc.h | 1 +
 migration/migration.h    | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 67cb275..06b816f 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -70,6 +70,7 @@ bool migration_has_failed(MigrationState *);
 /* ...and after the device transmission */
 bool migration_in_postcopy_after_devices(MigrationState *);
 void migration_global_dump(Monitor *mon);
+bool migrate_postcopy(void);
 
 /* migration/block-dirty-bitmap.c */
 void dirty_bitmap_mig_init(void);
diff --git a/migration/migration.h b/migration/migration.h
index e413d4d..e1e92ff 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -249,8 +249,6 @@ bool migration_is_blocked(Error **errp);
 bool migration_in_postcopy(void);
 MigrationState *migrate_get_current(void);
 
-bool migrate_postcopy(void);
-
 bool migrate_release_ram(void);
 bool migrate_postcopy_ram(void);
 bool migrate_zero_blocks(void);
-- 
1.8.3.1


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[Qemu-devel] [PATCH v9 8/8] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

[Wei Wang]

The new feature enables the virtio-balloon device to receive hints of
guest free pages from the free page vq.

A notifier is registered to the migration precopy notifier chain. The
notifier calls free_page_start after the migration thread syncs the dirty
bitmap, so that the free page optimization starts to clear bits of free
pages from the bitmap. It calls the free_page_stop before the migration
thread syncs the bitmap, which is the end of the current round of ram
save. The free_page_stop is also called to stop the optimization in the
case when there is an error occurred in the process of ram saving.

Note: balloon will report pages which were free at the time of this call.
As the reporting happens asynchronously, dirty bit logging must be
enabled before this free_page_start call is made. Guest reporting must be
disabled before the migration dirty bitmap is synchronized.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
 hw/virtio/virtio-balloon.c                      | 255 ++++++++++++++++++++++++
 include/hw/virtio/virtio-balloon.h              |  28 ++-
 include/standard-headers/linux/virtio_balloon.h |   5 +
 3 files changed, 287 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 1728e4f..4a3eb30 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -27,6 +27,7 @@
 #include "qapi/visitor.h"
 #include "trace.h"
 #include "qemu/error-report.h"
+#include "migration/misc.h"
 
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
@@ -308,6 +309,176 @@ out:
     }
 }
 
+static void virtio_balloon_handle_free_page_vq(VirtIODevice *vdev,
+                                               VirtQueue *vq)
+{
+    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+    qemu_bh_schedule(s->free_page_bh);
+}
+
+static bool get_free_page_hints(VirtIOBalloon *dev)
+{
+    VirtQueueElement *elem;
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtQueue *vq = dev->free_page_vq;
+
+    while (dev->block_iothread) {
+        qemu_cond_wait(&dev->free_page_cond, &dev->free_page_lock);
+    }
+
+    elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+    if (!elem) {
+        return false;
+    }
+
+    if (elem->out_num) {
+        uint32_t id;
+        size_t size = iov_to_buf(elem->out_sg, elem->out_num, 0,
+                                 &id, sizeof(id));
+        virtqueue_push(vq, elem, size);
+        g_free(elem);
+
+        virtio_tswap32s(vdev, &id);
+        if (unlikely(size != sizeof(id))) {
+            virtio_error(vdev, "received an incorrect cmd id");
+            return false;
+        }
+        if (id == dev->free_page_report_cmd_id) {
+            dev->free_page_report_status = FREE_PAGE_REPORT_S_START;
+        } else {
+            /*
+             * Stop the optimization only when it has started. This
+             * avoids a stale stop sign for the previous command.
+             */
+            if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
+                dev->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+            }
+        }
+    }
+
+    if (elem->in_num) {
+        if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
+            qemu_guest_free_page_hint(elem->in_sg[0].iov_base,
+                                      elem->in_sg[0].iov_len);
+        }
+        virtqueue_push(vq, elem, 1);
+        g_free(elem);
+    }
+
+    return true;
+}
+
+static void virtio_ballloon_get_free_page_hints(void *opaque)
+{
+    VirtIOBalloon *dev = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtQueue *vq = dev->free_page_vq;
+    bool continue_to_get_hints;
+
+    do {
+        qemu_mutex_lock(&dev->free_page_lock);
+        virtio_queue_set_notification(vq, 0);
+        continue_to_get_hints = get_free_page_hints(dev);
+        qemu_mutex_unlock(&dev->free_page_lock);
+        virtio_notify(vdev, vq);
+      /*
+       * Start to poll the vq once the reporting started. Otherwise, continue
+       * only when there are entries on the vq, which need to be given back.
+       */
+    } while (continue_to_get_hints ||
+             dev->free_page_report_status == FREE_PAGE_REPORT_S_START);
+    virtio_queue_set_notification(vq, 1);
+}
+
+static bool virtio_balloon_free_page_support(void *opaque)
+{
+    VirtIOBalloon *s = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+    return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT);
+}
+
+static void virtio_balloon_free_page_start(VirtIOBalloon *s)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+    /* For the stop and copy phase, we don't need to start the optimization */
+    if (!vdev->vm_running) {
+        return;
+    }
+
+    if (s->free_page_report_cmd_id == UINT_MAX) {
+        s->free_page_report_cmd_id =
+                       VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN;
+    } else {
+        s->free_page_report_cmd_id++;
+    }
+
+    s->free_page_report_status = FREE_PAGE_REPORT_S_REQUESTED;
+    virtio_notify_config(vdev);
+}
+
+static void virtio_balloon_free_page_stop(VirtIOBalloon *s)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+    if (s->free_page_report_status != FREE_PAGE_REPORT_S_STOP) {
+        /*
+         * The lock also guarantees us that the
+         * virtio_ballloon_get_free_page_hints exits after the
+         * free_page_report_status is set to S_STOP.
+         */
+        qemu_mutex_lock(&s->free_page_lock);
+        /*
+         * The guest hasn't done the reporting, so host sends a notification
+         * to the guest to actively stop the reporting.
+         */
+        s->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+        qemu_mutex_unlock(&s->free_page_lock);
+        virtio_notify_config(vdev);
+    }
+}
+
+static void virtio_balloon_free_page_done(VirtIOBalloon *s)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+    s->free_page_report_status = FREE_PAGE_REPORT_S_DONE;
+    virtio_notify_config(vdev);
+}
+
+static void virtio_balloon_free_page_report_notify(Notifier *notifier,
+                                                   void *data)
+{
+    VirtIOBalloon *dev = container_of(notifier, VirtIOBalloon,
+                                      free_page_report_notify);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    PrecopyNotifyReason reason = *(PrecopyNotifyReason *)data;
+
+    if (!virtio_balloon_free_page_support(dev) || migrate_postcopy()) {
+        return;
+    }
+
+    switch (reason) {
+    case PRECOPY_NOTIFY_START_ITERATION:
+        precopy_disable_bulk_stage();
+        break;
+    case PRECOPY_NOTIFY_ERR:
+    case PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP:
+        virtio_balloon_free_page_stop(dev);
+        break;
+    case PRECOPY_NOTIFY_AFTER_SYNC_BITMAP:
+        if (vdev->vm_running) {
+            virtio_balloon_free_page_start(dev);
+        } else {
+            virtio_balloon_free_page_done(dev);
+        }
+        break;
+    default:
+        virtio_error(vdev, "%s: %d reason unknown", __func__, reason);
+    }
+}
+
 static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
 {
     VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
@@ -316,6 +487,17 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
     config.num_pages = cpu_to_le32(dev->num_pages);
     config.actual = cpu_to_le32(dev->actual);
 
+    if (dev->free_page_report_status == FREE_PAGE_REPORT_S_REQUESTED) {
+        config.free_page_report_cmd_id =
+                       cpu_to_le32(dev->free_page_report_cmd_id);
+    } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_STOP) {
+        config.free_page_report_cmd_id =
+                       cpu_to_le32(VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID);
+    } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_DONE) {
+        config.free_page_report_cmd_id =
+                       cpu_to_le32(VIRTIO_BALLOON_FREE_PAGE_REPORT_DONE_ID);
+    }
+
     trace_virtio_balloon_get_config(config.num_pages, config.actual);
     memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
 }
@@ -376,6 +558,7 @@ static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f,
     VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
     f |= dev->host_features;
     virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ);
+
     return f;
 }
 
@@ -412,6 +595,18 @@ static int virtio_balloon_post_load_device(void *opaque, int version_id)
     return 0;
 }
 
+static const VMStateDescription vmstate_virtio_balloon_free_page_report = {
+    .name = "virtio-balloon-device/free-page-report",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = virtio_balloon_free_page_support,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(free_page_report_cmd_id, VirtIOBalloon),
+        VMSTATE_UINT32(free_page_report_status, VirtIOBalloon),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_virtio_balloon_device = {
     .name = "virtio-balloon-device",
     .version_id = 1,
@@ -422,6 +617,10 @@ static const VMStateDescription vmstate_virtio_balloon_device = {
         VMSTATE_UINT32(actual, VirtIOBalloon),
         VMSTATE_END_OF_LIST()
     },
+    .subsections = (const VMStateDescription * []) {
+        &vmstate_virtio_balloon_free_page_report,
+        NULL
+    }
 };
 
 static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
@@ -446,6 +645,29 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
     s->dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
     s->svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
 
+    if (virtio_has_feature(s->host_features,
+                           VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+        s->free_page_vq = virtio_add_queue(vdev, VIRTQUEUE_MAX_SIZE,
+                                           virtio_balloon_handle_free_page_vq);
+        s->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+        s->free_page_report_cmd_id =
+                           VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN;
+        s->free_page_report_notify.notify =
+                                       virtio_balloon_free_page_report_notify;
+        precopy_add_notifier(&s->free_page_report_notify);
+        if (s->iothread) {
+            object_ref(OBJECT(s->iothread));
+            s->free_page_bh = aio_bh_new(iothread_get_aio_context(s->iothread),
+                                       virtio_ballloon_get_free_page_hints, s);
+            qemu_mutex_init(&s->free_page_lock);
+            qemu_cond_init(&s->free_page_cond);
+            s->block_iothread = false;
+        } else {
+            /* Simply disable this feature if the iothread wasn't created. */
+            s->host_features &= ~(1 << VIRTIO_BALLOON_F_FREE_PAGE_HINT);
+            virtio_error(vdev, "iothread is missing");
+        }
+    }
     reset_stats(s);
 }
 
@@ -454,6 +676,11 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOBalloon *s = VIRTIO_BALLOON(dev);
 
+    if (virtio_balloon_free_page_support(s)) {
+        qemu_bh_delete(s->free_page_bh);
+        virtio_balloon_free_page_stop(s);
+        precopy_remove_notifier(&s->free_page_report_notify);
+    }
     balloon_stats_destroy_timer(s);
     qemu_remove_balloon_handler(s);
     virtio_cleanup(vdev);
@@ -463,6 +690,10 @@ static void virtio_balloon_device_reset(VirtIODevice *vdev)
 {
     VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
 
+    if (virtio_balloon_free_page_support(s)) {
+        virtio_balloon_free_page_stop(s);
+    }
+
     if (s->stats_vq_elem != NULL) {
         virtqueue_unpop(s->svq, s->stats_vq_elem, 0);
         g_free(s->stats_vq_elem);
@@ -480,6 +711,26 @@ static void virtio_balloon_set_status(VirtIODevice *vdev, uint8_t status)
          * was stopped */
         virtio_balloon_receive_stats(vdev, s->svq);
     }
+
+    if (virtio_balloon_free_page_support(s)) {
+        /*
+         * The VM is woken up and the iothread was blocked, so signal it to
+         * continue.
+         */
+        if (vdev->vm_running && s->block_iothread) {
+            qemu_mutex_lock(&s->free_page_lock);
+            s->block_iothread = false;
+            qemu_cond_signal(&s->free_page_cond);
+            qemu_mutex_unlock(&s->free_page_lock);
+        }
+
+        /* The VM is stopped, block the iothread. */
+        if (!vdev->vm_running) {
+            qemu_mutex_lock(&s->free_page_lock);
+            s->block_iothread = true;
+            qemu_mutex_unlock(&s->free_page_lock);
+        }
+    }
 }
 
 static void virtio_balloon_instance_init(Object *obj)
@@ -508,6 +759,10 @@ static const VMStateDescription vmstate_virtio_balloon = {
 static Property virtio_balloon_properties[] = {
     DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
                     VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
+    DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
+                    VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
+    DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
+                     IOThread *),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index e0df352..df1d632 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -17,11 +17,14 @@
 
 #include "standard-headers/linux/virtio_balloon.h"
 #include "hw/virtio/virtio.h"
+#include "sysemu/iothread.h"
 
 #define TYPE_VIRTIO_BALLOON "virtio-balloon-device"
 #define VIRTIO_BALLOON(obj) \
         OBJECT_CHECK(VirtIOBalloon, (obj), TYPE_VIRTIO_BALLOON)
 
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN 0x80000000
+
 typedef struct virtio_balloon_stat VirtIOBalloonStat;
 
 typedef struct virtio_balloon_stat_modern {
@@ -30,15 +33,38 @@ typedef struct virtio_balloon_stat_modern {
        uint64_t val;
 } VirtIOBalloonStatModern;
 
+enum virtio_balloon_free_page_report_status {
+    FREE_PAGE_REPORT_S_STOP = 0,
+    FREE_PAGE_REPORT_S_REQUESTED = 1,
+    FREE_PAGE_REPORT_S_START = 2,
+    FREE_PAGE_REPORT_S_DONE = 3,
+};
+
 typedef struct VirtIOBalloon {
     VirtIODevice parent_obj;
-    VirtQueue *ivq, *dvq, *svq;
+    VirtQueue *ivq, *dvq, *svq, *free_page_vq;
+    uint32_t free_page_report_status;
     uint32_t num_pages;
     uint32_t actual;
+    uint32_t free_page_report_cmd_id;
     uint64_t stats[VIRTIO_BALLOON_S_NR];
     VirtQueueElement *stats_vq_elem;
     size_t stats_vq_offset;
     QEMUTimer *stats_timer;
+    IOThread *iothread;
+    QEMUBH *free_page_bh;
+    /*
+     * Lock to synchronize threads to access the free page reporting related
+     * fields (e.g. free_page_report_status).
+     */
+    QemuMutex free_page_lock;
+    QemuCond  free_page_cond;
+    /*
+     * Set to block iothread to continue reading free page hints as the VM is
+     * stopped.
+     */
+    bool block_iothread;
+    Notifier free_page_report_notify;
     int64_t stats_last_update;
     int64_t stats_poll_interval;
     uint32_t host_features;
diff --git a/include/standard-headers/linux/virtio_balloon.h b/include/standard-headers/linux/virtio_balloon.h
index 4dbb7dc..9eee1c6 100644
--- a/include/standard-headers/linux/virtio_balloon.h
+++ b/include/standard-headers/linux/virtio_balloon.h
@@ -34,15 +34,20 @@
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST	0 /* Tell before reclaiming pages */
 #define VIRTIO_BALLOON_F_STATS_VQ	1 /* Memory Stats virtqueue */
 #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM	2 /* Deflate balloon on OOM */
+#define VIRTIO_BALLOON_F_FREE_PAGE_HINT 3 /* VQ to report free pages */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
 
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID 0
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_DONE_ID 1
 struct virtio_balloon_config {
 	/* Number of pages host wants Guest to give up. */
 	uint32_t num_pages;
 	/* Number of pages we've actually got in balloon. */
 	uint32_t actual;
+	/* Free page report command id, readonly by guest */
+	uint32_t free_page_report_cmd_id;
 };
 
 #define VIRTIO_BALLOON_S_SWAP_IN  0   /* Amount of memory swapped in */
-- 
1.8.3.1

[virtio-dev] [PATCH v9 8/8] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

[Wei Wang]

The new feature enables the virtio-balloon device to receive hints of
guest free pages from the free page vq.

A notifier is registered to the migration precopy notifier chain. The
notifier calls free_page_start after the migration thread syncs the dirty
bitmap, so that the free page optimization starts to clear bits of free
pages from the bitmap. It calls the free_page_stop before the migration
thread syncs the bitmap, which is the end of the current round of ram
save. The free_page_stop is also called to stop the optimization in the
case when there is an error occurred in the process of ram saving.

Note: balloon will report pages which were free at the time of this call.
As the reporting happens asynchronously, dirty bit logging must be
enabled before this free_page_start call is made. Guest reporting must be
disabled before the migration dirty bitmap is synchronized.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
 hw/virtio/virtio-balloon.c                      | 255 ++++++++++++++++++++++++
 include/hw/virtio/virtio-balloon.h              |  28 ++-
 include/standard-headers/linux/virtio_balloon.h |   5 +
 3 files changed, 287 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 1728e4f..4a3eb30 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -27,6 +27,7 @@
 #include "qapi/visitor.h"
 #include "trace.h"
 #include "qemu/error-report.h"
+#include "migration/misc.h"
 
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
@@ -308,6 +309,176 @@ out:
     }
 }
 
+static void virtio_balloon_handle_free_page_vq(VirtIODevice *vdev,
+                                               VirtQueue *vq)
+{
+    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+    qemu_bh_schedule(s->free_page_bh);
+}
+
+static bool get_free_page_hints(VirtIOBalloon *dev)
+{
+    VirtQueueElement *elem;
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtQueue *vq = dev->free_page_vq;
+
+    while (dev->block_iothread) {
+        qemu_cond_wait(&dev->free_page_cond, &dev->free_page_lock);
+    }
+
+    elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+    if (!elem) {
+        return false;
+    }
+
+    if (elem->out_num) {
+        uint32_t id;
+        size_t size = iov_to_buf(elem->out_sg, elem->out_num, 0,
+                                 &id, sizeof(id));
+        virtqueue_push(vq, elem, size);
+        g_free(elem);
+
+        virtio_tswap32s(vdev, &id);
+        if (unlikely(size != sizeof(id))) {
+            virtio_error(vdev, "received an incorrect cmd id");
+            return false;
+        }
+        if (id == dev->free_page_report_cmd_id) {
+            dev->free_page_report_status = FREE_PAGE_REPORT_S_START;
+        } else {
+            /*
+             * Stop the optimization only when it has started. This
+             * avoids a stale stop sign for the previous command.
+             */
+            if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
+                dev->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+            }
+        }
+    }
+
+    if (elem->in_num) {
+        if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
+            qemu_guest_free_page_hint(elem->in_sg[0].iov_base,
+                                      elem->in_sg[0].iov_len);
+        }
+        virtqueue_push(vq, elem, 1);
+        g_free(elem);
+    }
+
+    return true;
+}
+
+static void virtio_ballloon_get_free_page_hints(void *opaque)
+{
+    VirtIOBalloon *dev = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtQueue *vq = dev->free_page_vq;
+    bool continue_to_get_hints;
+
+    do {
+        qemu_mutex_lock(&dev->free_page_lock);
+        virtio_queue_set_notification(vq, 0);
+        continue_to_get_hints = get_free_page_hints(dev);
+        qemu_mutex_unlock(&dev->free_page_lock);
+        virtio_notify(vdev, vq);
+      /*
+       * Start to poll the vq once the reporting started. Otherwise, continue
+       * only when there are entries on the vq, which need to be given back.
+       */
+    } while (continue_to_get_hints ||
+             dev->free_page_report_status == FREE_PAGE_REPORT_S_START);
+    virtio_queue_set_notification(vq, 1);
+}
+
+static bool virtio_balloon_free_page_support(void *opaque)
+{
+    VirtIOBalloon *s = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+    return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT);
+}
+
+static void virtio_balloon_free_page_start(VirtIOBalloon *s)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+    /* For the stop and copy phase, we don't need to start the optimization */
+    if (!vdev->vm_running) {
+        return;
+    }
+
+    if (s->free_page_report_cmd_id == UINT_MAX) {
+        s->free_page_report_cmd_id =
+                       VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN;
+    } else {
+        s->free_page_report_cmd_id++;
+    }
+
+    s->free_page_report_status = FREE_PAGE_REPORT_S_REQUESTED;
+    virtio_notify_config(vdev);
+}
+
+static void virtio_balloon_free_page_stop(VirtIOBalloon *s)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+    if (s->free_page_report_status != FREE_PAGE_REPORT_S_STOP) {
+        /*
+         * The lock also guarantees us that the
+         * virtio_ballloon_get_free_page_hints exits after the
+         * free_page_report_status is set to S_STOP.
+         */
+        qemu_mutex_lock(&s->free_page_lock);
+        /*
+         * The guest hasn't done the reporting, so host sends a notification
+         * to the guest to actively stop the reporting.
+         */
+        s->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+        qemu_mutex_unlock(&s->free_page_lock);
+        virtio_notify_config(vdev);
+    }
+}
+
+static void virtio_balloon_free_page_done(VirtIOBalloon *s)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+    s->free_page_report_status = FREE_PAGE_REPORT_S_DONE;
+    virtio_notify_config(vdev);
+}
+
+static void virtio_balloon_free_page_report_notify(Notifier *notifier,
+                                                   void *data)
+{
+    VirtIOBalloon *dev = container_of(notifier, VirtIOBalloon,
+                                      free_page_report_notify);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    PrecopyNotifyReason reason = *(PrecopyNotifyReason *)data;
+
+    if (!virtio_balloon_free_page_support(dev) || migrate_postcopy()) {
+        return;
+    }
+
+    switch (reason) {
+    case PRECOPY_NOTIFY_START_ITERATION:
+        precopy_disable_bulk_stage();
+        break;
+    case PRECOPY_NOTIFY_ERR:
+    case PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP:
+        virtio_balloon_free_page_stop(dev);
+        break;
+    case PRECOPY_NOTIFY_AFTER_SYNC_BITMAP:
+        if (vdev->vm_running) {
+            virtio_balloon_free_page_start(dev);
+        } else {
+            virtio_balloon_free_page_done(dev);
+        }
+        break;
+    default:
+        virtio_error(vdev, "%s: %d reason unknown", __func__, reason);
+    }
+}
+
 static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
 {
     VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
@@ -316,6 +487,17 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
     config.num_pages = cpu_to_le32(dev->num_pages);
     config.actual = cpu_to_le32(dev->actual);
 
+    if (dev->free_page_report_status == FREE_PAGE_REPORT_S_REQUESTED) {
+        config.free_page_report_cmd_id =
+                       cpu_to_le32(dev->free_page_report_cmd_id);
+    } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_STOP) {
+        config.free_page_report_cmd_id =
+                       cpu_to_le32(VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID);
+    } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_DONE) {
+        config.free_page_report_cmd_id =
+                       cpu_to_le32(VIRTIO_BALLOON_FREE_PAGE_REPORT_DONE_ID);
+    }
+
     trace_virtio_balloon_get_config(config.num_pages, config.actual);
     memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
 }
@@ -376,6 +558,7 @@ static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f,
     VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
     f |= dev->host_features;
     virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ);
+
     return f;
 }
 
@@ -412,6 +595,18 @@ static int virtio_balloon_post_load_device(void *opaque, int version_id)
     return 0;
 }
 
+static const VMStateDescription vmstate_virtio_balloon_free_page_report = {
+    .name = "virtio-balloon-device/free-page-report",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = virtio_balloon_free_page_support,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(free_page_report_cmd_id, VirtIOBalloon),
+        VMSTATE_UINT32(free_page_report_status, VirtIOBalloon),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_virtio_balloon_device = {
     .name = "virtio-balloon-device",
     .version_id = 1,
@@ -422,6 +617,10 @@ static const VMStateDescription vmstate_virtio_balloon_device = {
         VMSTATE_UINT32(actual, VirtIOBalloon),
         VMSTATE_END_OF_LIST()
     },
+    .subsections = (const VMStateDescription * []) {
+        &vmstate_virtio_balloon_free_page_report,
+        NULL
+    }
 };
 
 static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
@@ -446,6 +645,29 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
     s->dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
     s->svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
 
+    if (virtio_has_feature(s->host_features,
+                           VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+        s->free_page_vq = virtio_add_queue(vdev, VIRTQUEUE_MAX_SIZE,
+                                           virtio_balloon_handle_free_page_vq);
+        s->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+        s->free_page_report_cmd_id =
+                           VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN;
+        s->free_page_report_notify.notify =
+                                       virtio_balloon_free_page_report_notify;
+        precopy_add_notifier(&s->free_page_report_notify);
+        if (s->iothread) {
+            object_ref(OBJECT(s->iothread));
+            s->free_page_bh = aio_bh_new(iothread_get_aio_context(s->iothread),
+                                       virtio_ballloon_get_free_page_hints, s);
+            qemu_mutex_init(&s->free_page_lock);
+            qemu_cond_init(&s->free_page_cond);
+            s->block_iothread = false;
+        } else {
+            /* Simply disable this feature if the iothread wasn't created. */
+            s->host_features &= ~(1 << VIRTIO_BALLOON_F_FREE_PAGE_HINT);
+            virtio_error(vdev, "iothread is missing");
+        }
+    }
     reset_stats(s);
 }
 
@@ -454,6 +676,11 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOBalloon *s = VIRTIO_BALLOON(dev);
 
+    if (virtio_balloon_free_page_support(s)) {
+        qemu_bh_delete(s->free_page_bh);
+        virtio_balloon_free_page_stop(s);
+        precopy_remove_notifier(&s->free_page_report_notify);
+    }
     balloon_stats_destroy_timer(s);
     qemu_remove_balloon_handler(s);
     virtio_cleanup(vdev);
@@ -463,6 +690,10 @@ static void virtio_balloon_device_reset(VirtIODevice *vdev)
 {
     VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
 
+    if (virtio_balloon_free_page_support(s)) {
+        virtio_balloon_free_page_stop(s);
+    }
+
     if (s->stats_vq_elem != NULL) {
         virtqueue_unpop(s->svq, s->stats_vq_elem, 0);
         g_free(s->stats_vq_elem);
@@ -480,6 +711,26 @@ static void virtio_balloon_set_status(VirtIODevice *vdev, uint8_t status)
          * was stopped */
         virtio_balloon_receive_stats(vdev, s->svq);
     }
+
+    if (virtio_balloon_free_page_support(s)) {
+        /*
+         * The VM is woken up and the iothread was blocked, so signal it to
+         * continue.
+         */
+        if (vdev->vm_running && s->block_iothread) {
+            qemu_mutex_lock(&s->free_page_lock);
+            s->block_iothread = false;
+            qemu_cond_signal(&s->free_page_cond);
+            qemu_mutex_unlock(&s->free_page_lock);
+        }
+
+        /* The VM is stopped, block the iothread. */
+        if (!vdev->vm_running) {
+            qemu_mutex_lock(&s->free_page_lock);
+            s->block_iothread = true;
+            qemu_mutex_unlock(&s->free_page_lock);
+        }
+    }
 }
 
 static void virtio_balloon_instance_init(Object *obj)
@@ -508,6 +759,10 @@ static const VMStateDescription vmstate_virtio_balloon = {
 static Property virtio_balloon_properties[] = {
     DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
                     VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
+    DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
+                    VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
+    DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
+                     IOThread *),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index e0df352..df1d632 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -17,11 +17,14 @@
 
 #include "standard-headers/linux/virtio_balloon.h"
 #include "hw/virtio/virtio.h"
+#include "sysemu/iothread.h"
 
 #define TYPE_VIRTIO_BALLOON "virtio-balloon-device"
 #define VIRTIO_BALLOON(obj) \
         OBJECT_CHECK(VirtIOBalloon, (obj), TYPE_VIRTIO_BALLOON)
 
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN 0x80000000
+
 typedef struct virtio_balloon_stat VirtIOBalloonStat;
 
 typedef struct virtio_balloon_stat_modern {
@@ -30,15 +33,38 @@ typedef struct virtio_balloon_stat_modern {
        uint64_t val;
 } VirtIOBalloonStatModern;
 
+enum virtio_balloon_free_page_report_status {
+    FREE_PAGE_REPORT_S_STOP = 0,
+    FREE_PAGE_REPORT_S_REQUESTED = 1,
+    FREE_PAGE_REPORT_S_START = 2,
+    FREE_PAGE_REPORT_S_DONE = 3,
+};
+
 typedef struct VirtIOBalloon {
     VirtIODevice parent_obj;
-    VirtQueue *ivq, *dvq, *svq;
+    VirtQueue *ivq, *dvq, *svq, *free_page_vq;
+    uint32_t free_page_report_status;
     uint32_t num_pages;
     uint32_t actual;
+    uint32_t free_page_report_cmd_id;
     uint64_t stats[VIRTIO_BALLOON_S_NR];
     VirtQueueElement *stats_vq_elem;
     size_t stats_vq_offset;
     QEMUTimer *stats_timer;
+    IOThread *iothread;
+    QEMUBH *free_page_bh;
+    /*
+     * Lock to synchronize threads to access the free page reporting related
+     * fields (e.g. free_page_report_status).
+     */
+    QemuMutex free_page_lock;
+    QemuCond  free_page_cond;
+    /*
+     * Set to block iothread to continue reading free page hints as the VM is
+     * stopped.
+     */
+    bool block_iothread;
+    Notifier free_page_report_notify;
     int64_t stats_last_update;
     int64_t stats_poll_interval;
     uint32_t host_features;
diff --git a/include/standard-headers/linux/virtio_balloon.h b/include/standard-headers/linux/virtio_balloon.h
index 4dbb7dc..9eee1c6 100644
--- a/include/standard-headers/linux/virtio_balloon.h
+++ b/include/standard-headers/linux/virtio_balloon.h
@@ -34,15 +34,20 @@
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST	0 /* Tell before reclaiming pages */
 #define VIRTIO_BALLOON_F_STATS_VQ	1 /* Memory Stats virtqueue */
 #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM	2 /* Deflate balloon on OOM */
+#define VIRTIO_BALLOON_F_FREE_PAGE_HINT 3 /* VQ to report free pages */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
 
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID 0
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_DONE_ID 1
 struct virtio_balloon_config {
 	/* Number of pages host wants Guest to give up. */
 	uint32_t num_pages;
 	/* Number of pages we've actually got in balloon. */
 	uint32_t actual;
+	/* Free page report command id, readonly by guest */
+	uint32_t free_page_report_cmd_id;
 };
 
 #define VIRTIO_BALLOON_S_SWAP_IN  0   /* Amount of memory swapped in */
-- 
1.8.3.1


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support

[no-reply]

Hi,

This series failed docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Message-id: 1542276484-25508-1-git-send-email-wei.w.wang@intel.com
Type: series
Subject: [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-quick@centos7 SHOW_ENV=1 J=8
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20181115183408.1240762-1-eblake@redhat.com -> patchew/20181115183408.1240762-1-eblake@redhat.com
Switched to a new branch 'test'
f36f77c virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
2ed0c1a migration: move migrate_postcopy() to include/migration/misc.h
4261bba migration/ram.c: add a function to disable the bulk stage
56a8004 migration/ram.c: add a notifier chain for precopy
fd731fe migration: API to clear bits of guest free pages from the dirty bitmap
f928ac4 migration: use bitmap_mutex in migration_bitmap_clear_dirty
7671c7e bitmap: bitmap_count_one_with_offset
dd2f05b bitmap: fix bitmap_count_one

=== OUTPUT BEGIN ===
  BUILD   centos7
make[1]: Entering directory `/var/tmp/patchew-tester-tmp-5w43kt65/src'
  GEN     /var/tmp/patchew-tester-tmp-5w43kt65/src/docker-src.2018-11-15-13.47.54.26143/qemu.tar
Cloning into '/var/tmp/patchew-tester-tmp-5w43kt65/src/docker-src.2018-11-15-13.47.54.26143/qemu.tar.vroot'...
done.
Checking out files:   7% (467/6455)   
Checking out files:   8% (517/6455)   
Checking out files:   9% (581/6455)   
Checking out files:  10% (646/6455)   
Checking out files:  11% (711/6455)   
Checking out files:  12% (775/6455)   
Checking out files:  13% (840/6455)   
Checking out files:  14% (904/6455)   
Checking out files:  15% (969/6455)   
Checking out files:  16% (1033/6455)   
Checking out files:  17% (1098/6455)   
Checking out files:  18% (1162/6455)   
Checking out files:  19% (1227/6455)   
Checking out files:  20% (1291/6455)   
Checking out files:  21% (1356/6455)   
Checking out files:  22% (1421/6455)   
Checking out files:  23% (1485/6455)   
Checking out files:  24% (1550/6455)   
Checking out files:  25% (1614/6455)   
Checking out files:  26% (1679/6455)   
Checking out files:  27% (1743/6455)   
Checking out files:  28% (1808/6455)   
Checking out files:  29% (1872/6455)   
Checking out files:  30% (1937/6455)   
Checking out files:  31% (2002/6455)   
Checking out files:  32% (2066/6455)   
Checking out files:  33% (2131/6455)   
Checking out files:  34% (2195/6455)   
Checking out files:  35% (2260/6455)   
Checking out files:  36% (2324/6455)   
Checking out files:  37% (2389/6455)   
Checking out files:  38% (2453/6455)   
Checking out files:  39% (2518/6455)   
Checking out files:  40% (2582/6455)   
Checking out files:  41% (2647/6455)   
Checking out files:  42% (2712/6455)   
Checking out files:  43% (2776/6455)   
Checking out files:  44% (2841/6455)   
Checking out files:  45% (2905/6455)   
Checking out files:  46% (2970/6455)   
Checking out files:  47% (3034/6455)   
Checking out files:  48% (3099/6455)   
Checking out files:  49% (3163/6455)   
Checking out files:  50% (3228/6455)   
Checking out files:  51% (3293/6455)   
Checking out files:  52% (3357/6455)   
Checking out files:  53% (3422/6455)   
Checking out files:  54% (3486/6455)   
Checking out files:  55% (3551/6455)   
Checking out files:  56% (3615/6455)   
Checking out files:  57% (3680/6455)   
Checking out files:  58% (3744/6455)   
Checking out files:  59% (3809/6455)   
Checking out files:  60% (3873/6455)   
Checking out files:  61% (3938/6455)   
Checking out files:  62% (4003/6455)   
Checking out files:  63% (4067/6455)   
Checking out files:  63% (4082/6455)   
Checking out files:  64% (4132/6455)   
Checking out files:  65% (4196/6455)   
Checking out files:  66% (4261/6455)   
Checking out files:  67% (4325/6455)   
Checking out files:  68% (4390/6455)   
Checking out files:  69% (4454/6455)   
Checking out files:  70% (4519/6455)   
Checking out files:  71% (4584/6455)   
Checking out files:  72% (4648/6455)   
Checking out files:  73% (4713/6455)   
Checking out files:  74% (4777/6455)   
Checking out files:  75% (4842/6455)   
Checking out files:  76% (4906/6455)   
Checking out files:  77% (4971/6455)   
Checking out files:  78% (5035/6455)   
Checking out files:  79% (5100/6455)   
Checking out files:  80% (5164/6455)   
Checking out files:  81% (5229/6455)   
Checking out files:  82% (5294/6455)   
Checking out files:  83% (5358/6455)   
Checking out files:  84% (5423/6455)   
Checking out files:  85% (5487/6455)   
Checking out files:  86% (5552/6455)   
Checking out files:  87% (5616/6455)   
Checking out files:  88% (5681/6455)   
Checking out files:  89% (5745/6455)   
Checking out files:  90% (5810/6455)   
Checking out files:  91% (5875/6455)   
Checking out files:  92% (5939/6455)   
Checking out files:  93% (6004/6455)   
Checking out files:  94% (6068/6455)   
Checking out files:  95% (6133/6455)   
Checking out files:  96% (6197/6455)   
Checking out files:  97% (6262/6455)   
Checking out files:  98% (6326/6455)   
Checking out files:  99% (6391/6455)   
Checking out files: 100% (6455/6455)   
Checking out files: 100% (6455/6455), done.
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPY    RUNNER
    RUN test-quick in qemu:centos7 
Packages installed:
SDL-devel-1.2.15-14.el7.x86_64
bison-3.0.4-1.el7.x86_64
bzip2-1.0.6-13.el7.x86_64
bzip2-devel-1.0.6-13.el7.x86_64
ccache-3.3.4-1.el7.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el7.x86_64
flex-2.5.37-3.el7.x86_64
gcc-4.8.5-28.el7_5.1.x86_64
gettext-0.19.8.1-2.el7.x86_64
git-1.8.3.1-14.el7_5.x86_64
glib2-devel-2.54.2-2.el7.x86_64
libaio-devel-0.3.109-13.el7.x86_64
libepoxy-devel-1.3.1-2.el7_5.x86_64
libfdt-devel-1.4.6-1.el7.x86_64
lzo-devel-2.06-8.el7.x86_64
make-3.82-23.el7.x86_64
mesa-libEGL-devel-17.2.3-8.20171019.el7.x86_64
mesa-libgbm-devel-17.2.3-8.20171019.el7.x86_64
nettle-devel-2.7.1-8.el7.x86_64
package g++ is not installed
package librdmacm-devel is not installed
pixman-devel-0.34.0-1.el7.x86_64
spice-glib-devel-0.34-3.el7_5.2.x86_64
spice-server-devel-0.14.0-2.el7_5.5.x86_64
tar-1.26-34.el7.x86_64
vte-devel-0.28.2-10.el7.x86_64
xen-devel-4.8.4.43.ge52ec4b787-1.el7.x86_64
zlib-devel-1.2.7-17.el7.x86_64

Environment variables:
PACKAGES=bison     bzip2     bzip2-devel     ccache     csnappy-devel     flex     g++     gcc     gettext     git     glib2-devel     libaio-devel     libepoxy-devel     libfdt-devel     librdmacm-devel     lzo-devel     make     mesa-libEGL-devel     mesa-libgbm-devel     nettle-devel     pixman-devel     SDL-devel     spice-glib-devel     spice-server-devel     tar     vte-devel     xen-devel     zlib-devel
HOSTNAME=e408575c0370
MAKEFLAGS= -j8
J=8
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
TARGET_LIST=
SHLVL=1
HOME=/home/patchew
TEST_DIR=/tmp/qemu-test
FEATURES= dtc
DEBUG=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/tmp/qemu-test/install
No C++ compiler available; disabling C++ specific optional code
Install prefix    /tmp/qemu-test/install
BIOS directory    /tmp/qemu-test/install/share/qemu
firmware path     /tmp/qemu-test/install/share/qemu-firmware
binary directory  /tmp/qemu-test/install/bin
library directory /tmp/qemu-test/install/lib
module directory  /tmp/qemu-test/install/lib/qemu
libexec directory /tmp/qemu-test/install/libexec
include directory /tmp/qemu-test/install/include
config directory  /tmp/qemu-test/install/etc
local state directory   /tmp/qemu-test/install/var
Manual directory  /tmp/qemu-test/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path       /tmp/qemu-test/src
GIT binary        git
GIT submodules    
C compiler        cc
Host C compiler   cc
C++ compiler      
Objective-C compiler cc
ARFLAGS           rv
CFLAGS            -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS       -I/usr/include/pixman-1    -Werror   -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-strong -Wno-missing-braces   -I/usr/include/libpng15     -pthread -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/spice-1  
LDFLAGS           -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
QEMU_LDFLAGS       
make              make
install           install
python            python -B
smbd              /usr/sbin/smbd
module support    no
host CPU          x86_64
host big endian   no
target list       x86_64-softmmu aarch64-softmmu
gprof enabled     no
sparse enabled    no
strip binaries    yes
profiler          no
static build      no
SDL support       yes (1.2.15)
GTK support       no 
GTK GL support    no
VTE support       no 
TLS priority      NORMAL
GNUTLS support    no
libgcrypt         no
nettle            yes (2.7.1)
libtasn1          no
curses support    yes
virgl support     no 
curl support      no
mingw32 support   no
Audio drivers     oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS support    no
Multipath support no
VNC support       yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   yes
xen support       yes
xen ctrl version  40800
pv dom build      no
brlapi support    no
bluez  support    no
Documentation     no
PIE               yes
vde support       no
netmap support    no
Linux AIO support yes
ATTR/XATTR support yes
Install blobs     yes
KVM support       yes
HAX support       no
HVF support       no
WHPX support      no
TCG support       yes
TCG debug enabled no
TCG interpreter   no
malloc trim support yes
RDMA support      yes
PVRDMA support    yes
fdt support       system
membarrier        no
preadv support    yes
fdatasync         yes
madvise           yes
posix_madvise     yes
posix_memalign    yes
libcap-ng support no
vhost-net support yes
vhost-crypto support yes
vhost-scsi support yes
vhost-vsock support yes
vhost-user support yes
Trace backends    log
spice support     yes (0.12.13/0.14.0)
rbd support       no
xfsctl support    no
smartcard support yes
libusb            no
usb net redir     no
OpenGL support    yes
OpenGL dmabufs    yes
libiscsi support  no
libnfs support    no
build guest agent yes
QGA VSS support   no
QGA w32 disk info no
QGA MSI support   no
seccomp support   no
coroutine backend ucontext
coroutine pool    yes
debug stack usage no
mutex debugging   no
crypto afalg      no
GlusterFS support no
gcov              gcov
gcov enabled      no
TPM support       yes
libssh2 support   no
TPM passthrough   yes
TPM emulator      yes
QOM debugging     yes
Live block migration yes
lzo support       yes
snappy support    no
bzip2 support     yes
NUMA host support no
libxml2           no
tcmalloc support  no
jemalloc support  no
avx2 optimization yes
replication support yes
VxHS block device no
bochs support     yes
cloop support     yes
dmg support       yes
qcow v1 support   yes
vdi support       yes
vvfat support     yes
qed support       yes
parallels support yes
sheepdog support  yes
capstone          no
docker            no
libpmem support   no
libudev           no

WARNING: Use of SDL 1.2 is deprecated and will be removed in
WARNING: future releases. Please switch to using SDL 2.0

NOTE: cross-compilers enabled:  'cc'
  GEN     x86_64-softmmu/config-devices.mak.tmp
  GEN     aarch64-softmmu/config-devices.mak.tmp
  GEN     config-host.h
  GEN     qemu-options.def
  GEN     qapi-gen
  GEN     trace/generated-tcg-tracers.h
  GEN     trace/generated-helpers-wrappers.h
  GEN     trace/generated-helpers.h
  GEN     aarch64-softmmu/config-devices.mak
  GEN     x86_64-softmmu/config-devices.mak
  GEN     trace/generated-helpers.c
  GEN     module_block.h
  GEN     ui/input-keymap-atset1-to-qcode.c
  GEN     ui/input-keymap-linux-to-qcode.c
  GEN     ui/input-keymap-qcode-to-atset1.c
  GEN     ui/input-keymap-qcode-to-atset2.c
  GEN     ui/input-keymap-qcode-to-atset3.c
  GEN     ui/input-keymap-qcode-to-linux.c
  GEN     ui/input-keymap-qcode-to-qnum.c
  GEN     ui/input-keymap-qcode-to-sun.c
  GEN     ui/input-keymap-qnum-to-qcode.c
  GEN     ui/input-keymap-usb-to-qcode.c
  GEN     ui/input-keymap-win32-to-qcode.c
  GEN     ui/input-keymap-x11-to-qcode.c
  GEN     ui/input-keymap-xorgevdev-to-qcode.c
  GEN     ui/input-keymap-xorgkbd-to-qcode.c
  GEN     ui/input-keymap-xorgxquartz-to-qcode.c
  GEN     ui/input-keymap-xorgxwin-to-qcode.c
  GEN     ui/input-keymap-osx-to-qcode.c
  GEN     tests/test-qapi-gen
  GEN     trace-root.h
  GEN     accel/kvm/trace.h
  GEN     accel/tcg/trace.h
  GEN     audio/trace.h
  GEN     block/trace.h
  GEN     chardev/trace.h
  GEN     crypto/trace.h
  GEN     hw/9pfs/trace.h
  GEN     hw/acpi/trace.h
  GEN     hw/alpha/trace.h
  GEN     hw/arm/trace.h
  GEN     hw/audio/trace.h
  GEN     hw/block/trace.h
  GEN     hw/block/dataplane/trace.h
  GEN     hw/char/trace.h
  GEN     hw/display/trace.h
  GEN     hw/dma/trace.h
  GEN     hw/hppa/trace.h
  GEN     hw/i2c/trace.h
  GEN     hw/i386/trace.h
  GEN     hw/i386/xen/trace.h
  GEN     hw/ide/trace.h
  GEN     hw/input/trace.h
  GEN     hw/intc/trace.h
  GEN     hw/isa/trace.h
  GEN     hw/mem/trace.h
  GEN     hw/misc/trace.h
  GEN     hw/misc/macio/trace.h
  GEN     hw/net/trace.h
  GEN     hw/nvram/trace.h
  GEN     hw/pci/trace.h
  GEN     hw/pci-host/trace.h
  GEN     hw/ppc/trace.h
  GEN     hw/rdma/trace.h
  GEN     hw/rdma/vmw/trace.h
  GEN     hw/s390x/trace.h
  GEN     hw/scsi/trace.h
  GEN     hw/sd/trace.h
  GEN     hw/sparc/trace.h
  GEN     hw/sparc64/trace.h
  GEN     hw/timer/trace.h
  GEN     hw/tpm/trace.h
  GEN     hw/usb/trace.h
  GEN     hw/vfio/trace.h
  GEN     hw/virtio/trace.h
  GEN     hw/watchdog/trace.h
  GEN     hw/xen/trace.h
  GEN     io/trace.h
  GEN     linux-user/trace.h
  GEN     migration/trace.h
  GEN     nbd/trace.h
  GEN     net/trace.h
  GEN     qapi/trace.h
  GEN     qom/trace.h
  GEN     scsi/trace.h
  GEN     target/arm/trace.h
  GEN     target/i386/trace.h
  GEN     target/mips/trace.h
  GEN     target/ppc/trace.h
  GEN     target/s390x/trace.h
  GEN     target/sparc/trace.h
  GEN     ui/trace.h
  GEN     util/trace.h
  GEN     trace-root.c
  GEN     accel/kvm/trace.c
  GEN     accel/tcg/trace.c
  GEN     audio/trace.c
  GEN     block/trace.c
  GEN     chardev/trace.c
  GEN     crypto/trace.c
  GEN     hw/9pfs/trace.c
  GEN     hw/acpi/trace.c
  GEN     hw/alpha/trace.c
  GEN     hw/arm/trace.c
  GEN     hw/audio/trace.c
  GEN     hw/block/trace.c
  GEN     hw/block/dataplane/trace.c
  GEN     hw/char/trace.c
  GEN     hw/display/trace.c
  GEN     hw/dma/trace.c
  GEN     hw/hppa/trace.c
  GEN     hw/i2c/trace.c
  GEN     hw/i386/trace.c
  GEN     hw/i386/xen/trace.c
  GEN     hw/ide/trace.c
  GEN     hw/input/trace.c
  GEN     hw/intc/trace.c
  GEN     hw/isa/trace.c
  GEN     hw/mem/trace.c
  GEN     hw/misc/trace.c
  GEN     hw/misc/macio/trace.c
  GEN     hw/net/trace.c
  GEN     hw/nvram/trace.c
  GEN     hw/pci/trace.c
  GEN     hw/pci-host/trace.c
  GEN     hw/ppc/trace.c
  GEN     hw/rdma/trace.c
  GEN     hw/rdma/vmw/trace.c
  GEN     hw/s390x/trace.c
  GEN     hw/scsi/trace.c
  GEN     hw/sd/trace.c
  GEN     hw/sparc/trace.c
  GEN     hw/sparc64/trace.c
  GEN     hw/timer/trace.c
  GEN     hw/tpm/trace.c
  GEN     hw/usb/trace.c
  GEN     hw/vfio/trace.c
  GEN     hw/virtio/trace.c
  GEN     hw/watchdog/trace.c
  GEN     hw/xen/trace.c
  GEN     io/trace.c
  GEN     linux-user/trace.c
  GEN     migration/trace.c
  GEN     nbd/trace.c
  GEN     net/trace.c
  GEN     qapi/trace.c
  GEN     qom/trace.c
  GEN     scsi/trace.c
  GEN     target/arm/trace.c
  GEN     target/i386/trace.c
  GEN     target/mips/trace.c
  GEN     target/ppc/trace.c
  GEN     target/s390x/trace.c
  GEN     target/sparc/trace.c
  GEN     ui/trace.c
  GEN     util/trace.c
  GEN     config-all-devices.mak
  CC      tests/qemu-iotests/socket_scm_helper.o
  GEN     qga/qapi-generated/qapi-gen
  CC      qapi/qapi-builtin-types.o
  CC      qapi/qapi-types.o
  CC      qapi/qapi-types-block-core.o
  CC      qapi/qapi-types-block.o
  CC      qapi/qapi-types-char.o
  CC      qapi/qapi-types-common.o
  CC      qapi/qapi-types-crypto.o
  CC      qapi/qapi-types-introspect.o
  CC      qapi/qapi-types-job.o
  CC      qapi/qapi-types-migration.o
  CC      qapi/qapi-types-misc.o
  CC      qapi/qapi-types-net.o
  CC      qapi/qapi-types-rocker.o
  CC      qapi/qapi-types-run-state.o
  CC      qapi/qapi-types-sockets.o
  CC      qapi/qapi-types-tpm.o
  CC      qapi/qapi-types-trace.o
  CC      qapi/qapi-types-transaction.o
  CC      qapi/qapi-types-ui.o
  CC      qapi/qapi-builtin-visit.o
  CC      qapi/qapi-visit.o
  CC      qapi/qapi-visit-block-core.o
  CC      qapi/qapi-visit-block.o
  CC      qapi/qapi-visit-char.o
  CC      qapi/qapi-visit-common.o
  CC      qapi/qapi-visit-crypto.o
  CC      qapi/qapi-visit-introspect.o
  CC      qapi/qapi-visit-job.o
  CC      qapi/qapi-visit-migration.o
  CC      qapi/qapi-visit-misc.o
  CC      qapi/qapi-visit-rocker.o
  CC      qapi/qapi-visit-net.o
  CC      qapi/qapi-visit-run-state.o
  CC      qapi/qapi-visit-sockets.o
  CC      qapi/qapi-visit-tpm.o
  CC      qapi/qapi-visit-trace.o
  CC      qapi/qapi-visit-transaction.o
  CC      qapi/qapi-visit-ui.o
  CC      qapi/qapi-events.o
  CC      qapi/qapi-events-block-core.o
  CC      qapi/qapi-events-block.o
  CC      qapi/qapi-events-char.o
  CC      qapi/qapi-events-common.o
  CC      qapi/qapi-events-crypto.o
  CC      qapi/qapi-events-introspect.o
  CC      qapi/qapi-events-job.o
  CC      qapi/qapi-events-migration.o
  CC      qapi/qapi-events-misc.o
  CC      qapi/qapi-events-net.o
  CC      qapi/qapi-events-rocker.o
  CC      qapi/qapi-events-run-state.o
  CC      qapi/qapi-events-sockets.o
  CC      qapi/qapi-events-tpm.o
  CC      qapi/qapi-events-transaction.o
  CC      qapi/qapi-events-trace.o
  CC      qapi/qapi-events-ui.o
  CC      qapi/qapi-introspect.o
  CC      qapi/qapi-visit-core.o
  CC      qapi/qapi-dealloc-visitor.o
  CC      qapi/qobject-input-visitor.o
  CC      qapi/qobject-output-visitor.o
  CC      qapi/qmp-registry.o
  CC      qapi/qmp-dispatch.o
  CC      qapi/string-input-visitor.o
  CC      qapi/string-output-visitor.o
  CC      qapi/opts-visitor.o
  CC      qapi/qapi-clone-visitor.o
  CC      qapi/qmp-event.o
  CC      qapi/qapi-util.o
  CC      qobject/qnull.o
  CC      qobject/qnum.o
  CC      qobject/qstring.o
  CC      qobject/qdict.o
  CC      qobject/qlist.o
  CC      qobject/qbool.o
  CC      qobject/qlit.o
  CC      qobject/qjson.o
  CC      qobject/qobject.o
  CC      qobject/json-lexer.o
  CC      qobject/json-streamer.o
  CC      qobject/json-parser.o
  CC      qobject/block-qdict.o
  CC      trace/control.o
  CC      trace/qmp.o
  CC      util/osdep.o
  CC      util/cutils.o
  CC      util/unicode.o
  CC      util/qemu-timer-common.o
  CC      util/bufferiszero.o
  CC      util/lockcnt.o
  CC      util/aiocb.o
  CC      util/async.o
  CC      util/aio-wait.o
  CC      util/thread-pool.o
  CC      util/qemu-timer.o
  CC      util/main-loop.o
  CC      util/iohandler.o
  CC      util/aio-posix.o
  CC      util/compatfd.o
  CC      util/event_notifier-posix.o
  CC      util/mmap-alloc.o
  CC      util/oslib-posix.o
  CC      util/qemu-openpty.o
  CC      util/qemu-thread-posix.o
  CC      util/memfd.o
  CC      util/envlist.o
  CC      util/path.o
  CC      util/module.o
  CC      util/host-utils.o
  CC      util/bitmap.o
  CC      util/bitops.o
  CC      util/hbitmap.o
  CC      util/fifo8.o
  CC      util/acl.o
  CC      util/cacheinfo.o
  CC      util/error.o
  CC      util/qemu-error.o
  CC      util/id.o
  CC      util/iov.o
  CC      util/qemu-config.o
  CC      util/qemu-sockets.o
  CC      util/uri.o
  CC      util/notify.o
  CC      util/qemu-option.o
  CC      util/qemu-progress.o
  CC      util/keyval.o
  CC      util/hexdump.o
  CC      util/crc32c.o
  CC      util/uuid.o
  CC      util/throttle.o
  CC      util/getauxval.o
  CC      util/readline.o
  CC      util/rcu.o
  CC      util/qemu-coroutine.o
  CC      util/qemu-coroutine-lock.o
  CC      util/qemu-coroutine-io.o
  CC      util/qemu-coroutine-sleep.o
  CC      util/coroutine-ucontext.o
  CC      util/buffer.o
  CC      util/timed-average.o
  CC      util/base64.o
  CC      util/log.o
  CC      util/pagesize.o
  CC      util/qdist.o
  CC      util/qht.o
  CC      util/qsp.o
  CC      util/range.o
  CC      util/stats64.o
  CC      util/systemd.o
  CC      util/iova-tree.o
  CC      util/vfio-helpers.o
  CC      util/drm.o
  CC      trace-root.o
  CC      accel/kvm/trace.o
  CC      accel/tcg/trace.o
  CC      audio/trace.o
  CC      block/trace.o
  CC      chardev/trace.o
  CC      crypto/trace.o
  CC      hw/9pfs/trace.o
  CC      hw/acpi/trace.o
  CC      hw/alpha/trace.o
  CC      hw/arm/trace.o
  CC      hw/audio/trace.o
  CC      hw/block/trace.o
  CC      hw/block/dataplane/trace.o
  CC      hw/char/trace.o
  CC      hw/display/trace.o
  CC      hw/dma/trace.o
  CC      hw/hppa/trace.o
  CC      hw/i2c/trace.o
  CC      hw/i386/trace.o
  CC      hw/i386/xen/trace.o
  CC      hw/ide/trace.o
  CC      hw/input/trace.o
  CC      hw/intc/trace.o
  CC      hw/isa/trace.o
  CC      hw/mem/trace.o
  CC      hw/misc/trace.o
  CC      hw/misc/macio/trace.o
  CC      hw/net/trace.o
  CC      hw/nvram/trace.o
  CC      hw/pci/trace.o
  CC      hw/pci-host/trace.o
  CC      hw/ppc/trace.o
  CC      hw/rdma/trace.o
  CC      hw/rdma/vmw/trace.o
  CC      hw/s390x/trace.o
  CC      hw/scsi/trace.o
  CC      hw/sd/trace.o
  CC      hw/sparc/trace.o
  CC      hw/sparc64/trace.o
  CC      hw/timer/trace.o
  CC      hw/tpm/trace.o
  CC      hw/usb/trace.o
  CC      hw/vfio/trace.o
  CC      hw/virtio/trace.o
  CC      hw/watchdog/trace.o
  CC      hw/xen/trace.o
  CC      io/trace.o
  CC      linux-user/trace.o
  CC      migration/trace.o
  CC      nbd/trace.o
  CC      net/trace.o
  CC      qapi/trace.o
  CC      qom/trace.o
  CC      scsi/trace.o
  CC      target/arm/trace.o
  CC      target/i386/trace.o
  CC      target/mips/trace.o
  CC      target/ppc/trace.o
  CC      target/s390x/trace.o
  CC      target/sparc/trace.o
  CC      ui/trace.o
  CC      util/trace.o
  CC      crypto/pbkdf-stub.o
  CC      stubs/arch-query-cpu-def.o
  CC      stubs/arch-query-cpu-model-expansion.o
  CC      stubs/arch-query-cpu-model-comparison.o
  CC      stubs/arch-query-cpu-model-baseline.o
  CC      stubs/bdrv-next-monitor-owned.o
  CC      stubs/blk-commit-all.o
  CC      stubs/blockdev-close-all-bdrv-states.o
  CC      stubs/clock-warp.o
  CC      stubs/cpu-get-clock.o
  CC      stubs/cpu-get-icount.o
  CC      stubs/dump.o
  CC      stubs/error-printf.o
  CC      stubs/fdset.o
  CC      stubs/gdbstub.o
  CC      stubs/get-vm-name.o
  CC      stubs/iothread.o
  CC      stubs/iothread-lock.o
  CC      stubs/is-daemonized.o
  CC      stubs/linux-aio.o
  CC      stubs/migr-blocker.o
  CC      stubs/machine-init-done.o
  CC      stubs/change-state-handler.o
  CC      stubs/monitor.o
  CC      stubs/notify-event.o
  CC      stubs/qtest.o
  CC      stubs/replay.o
  CC      stubs/runstate-check.o
  CC      stubs/set-fd-handler.o
  CC      stubs/slirp.o
  CC      stubs/sysbus.o
  CC      stubs/tpm.o
  CC      stubs/trace-control.o
  CC      stubs/uuid.o
  CC      stubs/vm-stop.o
  CC      stubs/vmstate.o
  CC      stubs/qmp_memory_device.o
  CC      stubs/target-monitor-defs.o
  CC      stubs/target-get-monitor-def.o
  CC      stubs/pc_madt_cpu_entry.o
  CC      stubs/vmgenid.o
  CC      stubs/xen-common.o
  CC      stubs/xen-hvm.o
  CC      stubs/pci-host-piix.o
  CC      stubs/ram-block.o
  CC      stubs/ramfb.o
  CC      contrib/ivshmem-client/ivshmem-client.o
  CC      contrib/ivshmem-client/main.o
  CC      contrib/ivshmem-server/main.o
  CC      contrib/ivshmem-server/ivshmem-server.o
  CC      qemu-nbd.o
  CC      block.o
  CC      blockjob.o
  CC      job.o
  CC      qemu-io-cmds.o
  CC      replication.o
  CC      block/raw-format.o
  CC      block/vmdk.o
  CC      block/vpc.o
  CC      block/qcow.o
  CC      block/vdi.o
  CC      block/cloop.o
  CC      block/bochs.o
  CC      block/vvfat.o
  CC      block/qcow2.o
  CC      block/dmg.o
  CC      block/qcow2-refcount.o
  CC      block/qcow2-cluster.o
  CC      block/qcow2-snapshot.o
  CC      block/qcow2-cache.o
  CC      block/qcow2-bitmap.o
  CC      block/qed.o
  CC      block/qed-l2-cache.o
  CC      block/qed-table.o
  CC      block/qed-cluster.o
  CC      block/qed-check.o
  CC      block/vhdx.o
  CC      block/vhdx-endian.o
  CC      block/vhdx-log.o
  CC      block/quorum.o
  CC      block/blkdebug.o
  CC      block/blkverify.o
  CC      block/blkreplay.o
  CC      block/parallels.o
  CC      block/blklogwrites.o
  CC      block/block-backend.o
  CC      block/snapshot.o
  CC      block/qapi.o
  CC      block/file-posix.o
  CC      block/linux-aio.o
  CC      block/null.o
  CC      block/mirror.o
  CC      block/commit.o
  CC      block/io.o
  CC      block/create.o
  CC      block/throttle-groups.o
  CC      block/nvme.o
  CC      block/nbd.o
  CC      block/nbd-client.o
  CC      block/sheepdog.o
  CC      block/accounting.o
  CC      block/dirty-bitmap.o
  CC      block/write-threshold.o
  CC      block/backup.o
  CC      block/replication.o
  CC      block/throttle.o
  CC      block/copy-on-read.o
  CC      block/crypto.o
  CC      nbd/server.o
  CC      nbd/client.o
  CC      nbd/common.o
  CC      scsi/utils.o
  CC      scsi/pr-manager.o
  CC      scsi/pr-manager-helper.o
  CC      block/dmg-bz2.o
  CC      crypto/init.o
  CC      crypto/hash.o
  CC      crypto/hash-nettle.o
  CC      crypto/hmac.o
  CC      crypto/hmac-nettle.o
  CC      crypto/aes.o
  CC      crypto/desrfb.o
  CC      crypto/cipher.o
  CC      crypto/tlscreds.o
  CC      crypto/tlscredsanon.o
  CC      crypto/tlscredspsk.o
  CC      crypto/tlscredsx509.o
  CC      crypto/tlssession.o
  CC      crypto/secret.o
  CC      crypto/random-platform.o
  CC      crypto/pbkdf.o
  CC      crypto/pbkdf-nettle.o
  CC      crypto/ivgen.o
  CC      crypto/ivgen-essiv.o
  CC      crypto/ivgen-plain.o
  CC      crypto/ivgen-plain64.o
  CC      crypto/afsplit.o
  CC      crypto/xts.o
  CC      crypto/block.o
  CC      crypto/block-qcow.o
  CC      crypto/block-luks.o
  CC      io/channel.o
  CC      io/channel-buffer.o
  CC      io/channel-command.o
  CC      io/channel-file.o
  CC      io/channel-socket.o
  CC      io/channel-tls.o
  CC      io/channel-watch.o
  CC      io/channel-websock.o
  CC      io/channel-util.o
  CC      io/dns-resolver.o
  CC      io/net-listener.o
  CC      io/task.o
  CC      qom/object.o
  CC      qom/container.o
  CC      qom/qom-qobject.o
  CC      qom/object_interfaces.o
  GEN     qemu-img-cmds.h
  CC      qemu-io.o
  CC      qemu-edid.o
  CC      hw/display/edid-generate.o
  CC      scsi/qemu-pr-helper.o
  CC      blockdev.o
  CC      qemu-bridge-helper.o
  CC      blockdev-nbd.o
  CC      bootdevice.o
  CC      iothread.o
  CC      job-qmp.o
  CC      qdev-monitor.o
  CC      device-hotplug.o
  CC      os-posix.o
  CC      bt-host.o
  CC      bt-vhci.o
  CC      dma-helpers.o
  CC      vl.o
  CC      tpm.o
  CC      device_tree.o
  CC      qapi/qapi-commands.o
  CC      qapi/qapi-commands-block-core.o
  CC      qapi/qapi-commands-block.o
  CC      qapi/qapi-commands-char.o
  CC      qapi/qapi-commands-common.o
  CC      qapi/qapi-commands-crypto.o
  CC      qapi/qapi-commands-introspect.o
  CC      qapi/qapi-commands-job.o
  CC      qapi/qapi-commands-migration.o
  CC      qapi/qapi-commands-misc.o
  CC      qapi/qapi-commands-net.o
  CC      qapi/qapi-commands-rocker.o
  CC      qapi/qapi-commands-run-state.o
  CC      qapi/qapi-commands-sockets.o
  CC      qapi/qapi-commands-tpm.o
  CC      qapi/qapi-commands-trace.o
  CC      qapi/qapi-commands-transaction.o
  CC      qapi/qapi-commands-ui.o
  CC      qmp.o
  CC      hmp.o
  CC      cpus-common.o
  CC      audio/audio.o
  CC      audio/noaudio.o
  CC      audio/wavaudio.o
  CC      audio/mixeng.o
  CC      audio/spiceaudio.o
  CC      audio/wavcapture.o
  CC      backends/rng.o
  CC      backends/rng-egd.o
  CC      backends/rng-random.o
  CC      backends/tpm.o
  CC      backends/hostmem.o
  CC      backends/hostmem-ram.o
  CC      backends/hostmem-file.o
  CC      backends/cryptodev.o
  CC      backends/cryptodev-builtin.o
  CC      backends/cryptodev-vhost.o
  CC      backends/cryptodev-vhost-user.o
  CC      backends/hostmem-memfd.o
  CC      block/stream.o
  CC      chardev/msmouse.o
  CC      chardev/wctablet.o
  CC      chardev/testdev.o
  CC      chardev/spice.o
  CC      disas/arm.o
  CC      disas/i386.o
  CC      fsdev/qemu-fsdev-dummy.o
  CC      fsdev/qemu-fsdev-opts.o
  CC      fsdev/qemu-fsdev-throttle.o
  CC      hw/acpi/core.o
  CC      hw/acpi/piix4.o
  CC      hw/acpi/pcihp.o
  CC      hw/acpi/ich9.o
  CC      hw/acpi/tco.o
  CC      hw/acpi/cpu_hotplug.o
  CC      hw/acpi/memory_hotplug.o
  CC      hw/acpi/cpu.o
  CC      hw/acpi/nvdimm.o
  CC      hw/acpi/vmgenid.o
  CC      hw/acpi/acpi_interface.o
  CC      hw/acpi/bios-linker-loader.o
  CC      hw/acpi/aml-build.o
  CC      hw/acpi/ipmi.o
  CC      hw/acpi/acpi-stub.o
  CC      hw/acpi/ipmi-stub.o
  CC      hw/audio/sb16.o
  CC      hw/audio/es1370.o
  CC      hw/audio/ac97.o
  CC      hw/audio/fmopl.o
  CC      hw/audio/adlib.o
  CC      hw/audio/gus.o
  CC      hw/audio/gusemu_hal.o
  CC      hw/audio/gusemu_mixer.o
  CC      hw/audio/cs4231a.o
  CC      hw/audio/intel-hda.o
  CC      hw/audio/hda-codec.o
  CC      hw/audio/pcspk.o
  CC      hw/audio/wm8750.o
  CC      hw/audio/pl041.o
  CC      hw/audio/lm4549.o
  CC      hw/audio/marvell_88w8618.o
  CC      hw/audio/soundhw.o
  CC      hw/block/block.o
  CC      hw/block/cdrom.o
  CC      hw/block/hd-geometry.o
  CC      hw/block/fdc.o
  CC      hw/block/m25p80.o
  CC      hw/block/nand.o
  CC      hw/block/pflash_cfi01.o
  CC      hw/block/pflash_cfi02.o
  CC      hw/block/xen_disk.o
  CC      hw/block/ecc.o
  CC      hw/block/onenand.o
  CC      hw/block/nvme.o
  CC      hw/bt/core.o
  CC      hw/bt/l2cap.o
  CC      hw/bt/sdp.o
  CC      hw/bt/hci.o
  CC      hw/bt/hid.o
  CC      hw/bt/hci-csr.o
  CC      hw/char/ipoctal232.o
  CC      hw/char/nrf51_uart.o
  CC      hw/char/parallel.o
  CC      hw/char/parallel-isa.o
  CC      hw/char/pl011.o
  CC      hw/char/serial.o
  CC      hw/char/serial-isa.o
  CC      hw/char/serial-pci.o
  CC      hw/char/virtio-console.o
  CC      hw/char/xen_console.o
  CC      hw/char/cadence_uart.o
  CC      hw/char/cmsdk-apb-uart.o
  CC      hw/char/debugcon.o
  CC      hw/char/imx_serial.o
  CC      hw/core/qdev.o
  CC      hw/core/qdev-properties.o
  CC      hw/core/bus.o
  CC      hw/core/reset.o
  CC      hw/core/qdev-fw.o
  CC      hw/core/fw-path-provider.o
  CC      hw/core/irq.o
  CC      hw/core/hotplug.o
  CC      hw/core/nmi.o
  CC      hw/core/stream.o
  CC      hw/core/ptimer.o
  CC      hw/core/sysbus.o
  CC      hw/core/machine.o
  CC      hw/core/loader.o
  CC      hw/core/qdev-properties-system.o
  CC      hw/core/register.o
  CC      hw/core/or-irq.o
  CC      hw/core/split-irq.o
  CC      hw/core/platform-bus.o
  CC      hw/core/generic-loader.o
  CC      hw/core/null-machine.o
  CC      hw/cpu/core.o
  CC      hw/display/ramfb.o
  CC      hw/display/ramfb-standalone.o
  CC      hw/display/ads7846.o
  CC      hw/display/cirrus_vga.o
  CC      hw/display/cirrus_vga_isa.o
  CC      hw/display/pl110.o
  CC      hw/display/sii9022.o
  CC      hw/display/ssd0303.o
  CC      hw/display/ssd0323.o
  CC      hw/display/xenfb.o
  CC      hw/display/vga-pci.o
  CC      hw/display/edid-region.o
  CC      hw/display/vga-isa.o
  CC      hw/display/vmware_vga.o
  CC      hw/display/bochs-display.o
  CC      hw/display/blizzard.o
  CC      hw/display/exynos4210_fimd.o
  CC      hw/display/framebuffer.o
  CC      hw/display/tc6393xb.o
  CC      hw/display/qxl.o
  CC      hw/display/qxl-logger.o
  CC      hw/display/qxl-render.o
  CC      hw/dma/pl080.o
  CC      hw/dma/pl330.o
  CC      hw/dma/i8257.o
  CC      hw/dma/xilinx_axidma.o
  CC      hw/dma/xlnx-zynq-devcfg.o
  CC      hw/dma/xlnx-zdma.o
  CC      hw/gpio/max7310.o
  CC      hw/gpio/pl061.o
  CC      hw/gpio/zaurus.o
  CC      hw/gpio/gpio_key.o
  CC      hw/i2c/core.o
  CC      hw/i2c/smbus.o
  CC      hw/i2c/smbus_eeprom.o
  CC      hw/i2c/i2c-ddc.o
  CC      hw/i2c/versatile_i2c.o
  CC      hw/i2c/smbus_ich9.o
  CC      hw/i2c/pm_smbus.o
  CC      hw/i2c/bitbang_i2c.o
  CC      hw/i2c/exynos4210_i2c.o
  CC      hw/i2c/imx_i2c.o
  CC      hw/i2c/aspeed_i2c.o
  CC      hw/ide/core.o
  CC      hw/ide/atapi.o
  CC      hw/ide/qdev.o
  CC      hw/ide/pci.o
  CC      hw/ide/isa.o
  CC      hw/ide/piix.o
  CC      hw/ide/microdrive.o
  CC      hw/ide/ahci.o
  CC      hw/ide/ich.o
  CC      hw/ide/ahci-allwinner.o
  CC      hw/input/hid.o
  CC      hw/input/lm832x.o
  CC      hw/input/pckbd.o
  CC      hw/input/pl050.o
  CC      hw/input/ps2.o
  CC      hw/input/stellaris_input.o
  CC      hw/input/tsc2005.o
  CC      hw/input/virtio-input.o
  CC      hw/input/virtio-input-hid.o
  CC      hw/input/virtio-input-host.o
  CC      hw/intc/i8259_common.o
  CC      hw/intc/i8259.o
  CC      hw/intc/pl190.o
  CC      hw/intc/xlnx-pmu-iomod-intc.o
  CC      hw/intc/xlnx-zynqmp-ipi.o
  CC      hw/intc/imx_avic.o
  CC      hw/intc/imx_gpcv2.o
  CC      hw/intc/realview_gic.o
  CC      hw/intc/ioapic_common.o
  CC      hw/intc/arm_gic_common.o
  CC      hw/intc/arm_gic.o
  CC      hw/intc/arm_gicv2m.o
  CC      hw/intc/arm_gicv3_common.o
  CC      hw/intc/arm_gicv3.o
  CC      hw/intc/arm_gicv3_dist.o
  CC      hw/intc/arm_gicv3_redist.o
  CC      hw/intc/arm_gicv3_its_common.o
  CC      hw/intc/intc.o
  CC      hw/ipack/ipack.o
  CC      hw/ipack/tpci200.o
  CC      hw/ipmi/ipmi.o
  CC      hw/ipmi/ipmi_bmc_sim.o
  CC      hw/ipmi/ipmi_bmc_extern.o
  CC      hw/ipmi/isa_ipmi_kcs.o
  CC      hw/ipmi/isa_ipmi_bt.o
  CC      hw/isa/isa-bus.o
  CC      hw/isa/isa-superio.o
  CC      hw/isa/apm.o
  CC      hw/mem/pc-dimm.o
  CC      hw/mem/memory-device.o
  CC      hw/mem/nvdimm.o
  CC      hw/misc/applesmc.o
  CC      hw/misc/max111x.o
  CC      hw/misc/tmp105.o
  CC      hw/misc/tmp421.o
  CC      hw/misc/debugexit.o
  CC      hw/misc/sga.o
  CC      hw/misc/pc-testdev.o
  CC      hw/misc/pci-testdev.o
  CC      hw/misc/edu.o
  CC      hw/misc/pca9552.o
  CC      hw/misc/unimp.o
  CC      hw/misc/vmcoreinfo.o
  CC      hw/misc/arm_l2x0.o
  CC      hw/misc/arm_integrator_debug.o
  CC      hw/misc/a9scu.o
  CC      hw/misc/arm11scu.o
  CC      hw/net/xen_nic.o
  CC      hw/net/ne2000.o
  CC      hw/net/eepro100.o
  CC      hw/net/pcnet-pci.o
  CC      hw/net/pcnet.o
  CC      hw/net/e1000.o
  CC      hw/net/e1000x_common.o
  CC      hw/net/net_tx_pkt.o
  CC      hw/net/net_rx_pkt.o
  CC      hw/net/e1000e.o
  CC      hw/net/e1000e_core.o
  CC      hw/net/rtl8139.o
  CC      hw/net/vmxnet3.o
  CC      hw/net/smc91c111.o
  CC      hw/net/lan9118.o
  CC      hw/net/ne2000-isa.o
  CC      hw/net/xgmac.o
  CC      hw/net/xilinx_axienet.o
  CC      hw/net/allwinner_emac.o
  CC      hw/net/imx_fec.o
  CC      hw/net/cadence_gem.o
  CC      hw/net/stellaris_enet.o
  CC      hw/net/ftgmac100.o
  CC      hw/net/rocker/rocker.o
  CC      hw/net/rocker/rocker_fp.o
  CC      hw/net/rocker/rocker_desc.o
  CC      hw/net/rocker/rocker_world.o
  CC      hw/net/rocker/rocker_of_dpa.o
  CC      hw/net/can/can_sja1000.o
  CC      hw/net/can/can_kvaser_pci.o
  CC      hw/net/can/can_pcm3680_pci.o
  CC      hw/net/can/can_mioe3680_pci.o
  CC      hw/nvram/eeprom93xx.o
  CC      hw/nvram/fw_cfg.o
  CC      hw/nvram/chrp_nvram.o
  CC      hw/pci-bridge/pci_bridge_dev.o
  CC      hw/pci-bridge/pcie_root_port.o
  CC      hw/pci-bridge/pcie_pci_bridge.o
  CC      hw/pci-bridge/gen_pcie_root_port.o
  CC      hw/pci-bridge/pci_expander_bridge.o
  CC      hw/pci-bridge/xio3130_upstream.o
  CC      hw/pci-bridge/xio3130_downstream.o
  CC      hw/pci-bridge/ioh3420.o
  CC      hw/pci-bridge/i82801b11.o
  CC      hw/pci-host/pam.o
  CC      hw/pci-host/versatile.o
  CC      hw/pci-host/piix.o
  CC      hw/pci-host/q35.o
  CC      hw/pci-host/gpex.o
  CC      hw/pci-host/designware.o
  CC      hw/pci/pci.o
  CC      hw/pci/pci_bridge.o
  CC      hw/pci/msix.o
  CC      hw/pci/msi.o
  CC      hw/pci/shpc.o
  CC      hw/pci/slotid_cap.o
  CC      hw/pci/pci_host.o
  CC      hw/pci/pcie_host.o
  CC      hw/pci/pcie.o
  CC      hw/pci/pcie_aer.o
  CC      hw/pci/pcie_port.o
  CC      hw/pci/pci-stub.o
  CC      hw/pcmcia/pcmcia.o
  CC      hw/scsi/scsi-disk.o
  CC      hw/scsi/emulation.o
  CC      hw/scsi/scsi-generic.o
  CC      hw/scsi/scsi-bus.o
  CC      hw/scsi/lsi53c895a.o
  CC      hw/scsi/mptsas.o
  CC      hw/scsi/mptconfig.o
  CC      hw/scsi/mptendian.o
  CC      hw/scsi/megasas.o
  CC      hw/scsi/vmw_pvscsi.o
  CC      hw/scsi/esp.o
  CC      hw/scsi/esp-pci.o
  CC      hw/sd/pl181.o
  CC      hw/sd/ssi-sd.o
  CC      hw/sd/sd.o
  CC      hw/sd/core.o
  CC      hw/sd/sdmmc-internal.o
  CC      hw/sd/sdhci.o
  CC      hw/smbios/smbios.o
  CC      hw/smbios/smbios_type_38.o
  CC      hw/smbios/smbios-stub.o
  CC      hw/smbios/smbios_type_38-stub.o
  CC      hw/ssi/pl022.o
  CC      hw/ssi/ssi.o
  CC      hw/ssi/xilinx_spips.o
  CC      hw/ssi/aspeed_smc.o
  CC      hw/ssi/stm32f2xx_spi.o
  CC      hw/ssi/mss-spi.o
  CC      hw/timer/arm_timer.o
  CC      hw/timer/arm_mptimer.o
  CC      hw/timer/armv7m_systick.o
  CC      hw/timer/a9gtimer.o
  CC      hw/timer/ds1338.o
  CC      hw/timer/cadence_ttc.o
  CC      hw/timer/hpet.o
  CC      hw/timer/i8254_common.o
  CC      hw/timer/i8254.o
  CC      hw/timer/pl031.o
  CC      hw/timer/twl92230.o
  CC      hw/timer/imx_epit.o
  CC      hw/timer/imx_gpt.o
  CC      hw/timer/xlnx-zynqmp-rtc.o
  CC      hw/timer/stm32f2xx_timer.o
  CC      hw/timer/aspeed_timer.o
  CC      hw/timer/cmsdk-apb-timer.o
  CC      hw/timer/cmsdk-apb-dualtimer.o
  CC      hw/timer/mss-timer.o
  CC      hw/tpm/tpm_util.o
  CC      hw/tpm/tpm_tis.o
  CC      hw/tpm/tpm_crb.o
  CC      hw/tpm/tpm_passthrough.o
  CC      hw/tpm/tpm_emulator.o
  CC      hw/usb/core.o
  CC      hw/usb/combined-packet.o
  CC      hw/usb/bus.o
  CC      hw/usb/libhw.o
  CC      hw/usb/desc.o
  CC      hw/usb/desc-msos.o
  CC      hw/usb/hcd-uhci.o
  CC      hw/usb/hcd-ohci.o
  CC      hw/usb/hcd-ehci.o
  CC      hw/usb/hcd-ehci-pci.o
  CC      hw/usb/hcd-ehci-sysbus.o
  CC      hw/usb/hcd-xhci.o
  CC      hw/usb/hcd-xhci-nec.o
  CC      hw/usb/hcd-musb.o
  CC      hw/usb/dev-hub.o
  CC      hw/usb/dev-hid.o
  CC      hw/usb/dev-wacom.o
  CC      hw/usb/dev-storage.o
  CC      hw/usb/dev-uas.o
  CC      hw/usb/dev-audio.o
  CC      hw/usb/dev-serial.o
  CC      hw/usb/dev-network.o
  CC      hw/usb/dev-bluetooth.o
  CC      hw/usb/dev-smartcard-reader.o
  CC      hw/usb/ccid-card-passthru.o
  CC      hw/usb/ccid-card-emulated.o
  CC      hw/usb/dev-mtp.o
  CC      hw/usb/host-stub.o
  CC      hw/virtio/virtio-bus.o
  CC      hw/virtio/virtio-rng.o
  CC      hw/virtio/virtio-pci.o
  CC      hw/virtio/virtio-mmio.o
  CC      hw/virtio/vhost-stub.o
  CC      hw/watchdog/watchdog.o
  CC      hw/watchdog/cmsdk-apb-watchdog.o
  CC      hw/watchdog/wdt_i6300esb.o
  CC      hw/watchdog/wdt_ib700.o
  CC      hw/watchdog/wdt_aspeed.o
  CC      hw/xen/xen_backend.o
  CC      hw/xen/xen_devconfig.o
  CC      hw/xen/xen_pvdev.o
  CC      hw/xen/xen-common.o
  CC      migration/migration.o
  CC      migration/socket.o
  CC      migration/fd.o
  CC      migration/exec.o
  CC      migration/tls.o
  CC      migration/channel.o
  CC      migration/savevm.o
  CC      migration/colo.o
  CC      migration/colo-failover.o
  CC      migration/vmstate.o
  CC      migration/vmstate-types.o
  CC      migration/qemu-file.o
  CC      migration/page_cache.o
  CC      migration/global_state.o
  CC      migration/qemu-file-channel.o
  CC      migration/xbzrle.o
  CC      migration/postcopy-ram.o
  CC      migration/qjson.o
  CC      migration/block-dirty-bitmap.o
  CC      migration/rdma.o
  CC      migration/block.o
  CC      net/net.o
  CC      net/queue.o
  CC      net/checksum.o
  CC      net/util.o
  CC      net/hub.o
  CC      net/socket.o
  CC      net/dump.o
  CC      net/eth.o
  CC      net/l2tpv3.o
  CC      net/vhost-user.o
  CC      net/slirp.o
  CC      net/filter.o
  CC      net/filter-buffer.o
  CC      net/filter-mirror.o
  CC      net/colo-compare.o
/tmp/qemu-test/src/migration/rdma.c: In function 'qemu_rdma_accept':
/tmp/qemu-test/src/migration/rdma.c:3353:5: error: implicit declaration of function 'migrate_postcopy' [-Werror=implicit-function-declaration]
     if (migrate_postcopy() && !rdma->is_return_path) {
     ^
/tmp/qemu-test/src/migration/rdma.c:3353:5: error: nested extern declaration of 'migrate_postcopy' [-Werror=nested-externs]
cc1: all warnings being treated as errors
make: *** [migration/rdma.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 563, in <module>
    sys.exit(main())
  File "./tests/docker/docker.py", line 560, in main
    return args.cmdobj.run(args, argv)
  File "./tests/docker/docker.py", line 306, in run
    return Docker().run(argv, args.keep, quiet=args.quiet)
  File "./tests/docker/docker.py", line 274, in run
    quiet=quiet)
  File "./tests/docker/docker.py", line 181, in _do_check
    return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib64/python2.7/subprocess.py", line 542, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=033cedace90711e8ba3b68b59973b7d0', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '--net=none', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=8', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-5w43kt65/src/docker-src.2018-11-15-13.47.54.26143:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-5w43kt65/src'
make: *** [docker-run-test-quick@centos7] Error 2

real	2m26.991s
user	0m17.224s
sys	0m3.671s
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support

[Wei Wang]

On 11/16/2018 02:50 AM, no-reply@patchew.org wrote:
> Hi,
>
> This series failed docker-quick@centos7 build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
>
>    CC      net/filter.o
>    CC      net/filter-buffer.o
>    CC      net/filter-mirror.o
>    CC      net/colo-compare.o
> /tmp/qemu-test/src/migration/rdma.c: In function 'qemu_rdma_accept':
> /tmp/qemu-test/src/migration/rdma.c:3353:5: error: implicit declaration of function 'migrate_postcopy' [-Werror=implicit-function-declaration]
>       if (migrate_postcopy() && !rdma->is_return_path) {
>       ^
> /tmp/qemu-test/src/migration/rdma.c:3353:5: error: nested extern declaration of 'migrate_postcopy' [-Werror=nested-externs]
> cc1: all warnings being treated as errors
> make: *** [migration/rdma.o] Error 1
> make: *** Waiting for unfinished jobs....

This is caused by missing "migration/misc.h" in rdma.c, since we moved 
the migrate_postcopy() declaration there. I'll add it.

Best,
Wei

[virtio-dev] Re: [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support

[Wei Wang]

On 11/16/2018 02:50 AM, no-reply@patchew.org wrote:
> Hi,
>
> This series failed docker-quick@centos7 build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
>
>    CC      net/filter.o
>    CC      net/filter-buffer.o
>    CC      net/filter-mirror.o
>    CC      net/colo-compare.o
> /tmp/qemu-test/src/migration/rdma.c: In function 'qemu_rdma_accept':
> /tmp/qemu-test/src/migration/rdma.c:3353:5: error: implicit declaration of function 'migrate_postcopy' [-Werror=implicit-function-declaration]
>       if (migrate_postcopy() && !rdma->is_return_path) {
>       ^
> /tmp/qemu-test/src/migration/rdma.c:3353:5: error: nested extern declaration of 'migrate_postcopy' [-Werror=nested-externs]
> cc1: all warnings being treated as errors
> make: *** [migration/rdma.o] Error 1
> make: *** Waiting for unfinished jobs....

This is caused by missing "migration/misc.h" in rdma.c, since we moved 
the migrate_postcopy() declaration there. I'll add it.

Best,
Wei

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support

[Wei Wang]

On 11/15/2018 06:07 PM, Wei Wang wrote:
> This is the deivce part implementation to add a new feature,
> VIRTIO_BALLOON_F_FREE_PAGE_HINT to the virtio-balloon device. The device
> receives the guest free page hints from the driver and clears the
> corresponding bits in the dirty bitmap, so that those free pages are
> not sent by the migration thread to the destination.
>
> *Tests
> 1 Test Environment
>      Host: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
>      Migration setup: migrate_set_speed 100G, migrate_set_downtime 400ms
>
> 2 Test Results (results are averaged over several repeated runs)
>      2.1 Guest setup: 8G RAM, 4 vCPU
>          2.1.1 Idle guest live migration time
>              Optimization v.s. Legacy = 620ms vs 2970ms
>              --> ~79% reduction
>          2.1.2 Guest live migration with Linux compilation workload
>            (i.e. make bzImage -j4) running
>            1) Live Migration Time:
>               Optimization v.s. Legacy = 2273ms v.s. 4502ms
>               --> ~50% reduction
>            2) Linux Compilation Time:
>               Optimization v.s. Legacy = 8min42s v.s. 8min43s
>               --> no obvious difference
>
>      2.2 Guest setup: 128G RAM, 4 vCPU
>          2.2.1 Idle guest live migration time
>              Optimization v.s. Legacy = 5294ms vs 41651ms
>              --> ~87% reduction
>          2.2.2 Guest live migration with Linux compilation workload
>            1) Live Migration Time:
>              Optimization v.s. Legacy = 8816ms v.s. 54201ms
>              --> 84% reduction
>            2) Linux Compilation Time:
>               Optimization v.s. Legacy = 8min30s v.s. 8min36s
>               --> no obvious difference
>
> ChangeLog:
> v8->v9:
> bitmap:
>      - fix bitmap_count_one to handle the nbits=0 case
> migration:
>      - replace the ram save notifier chain with a more general precopy
>        notifier chain, which is similar to the postcopy notifier chain.
>      - Avoid exposing the RAMState struct, and add a function,
>        precopy_disable_bulk_stage, to let the virtio-balloon notifier
>        callback to disable the bulk stage flag.

Hi Dave and Peter,

Could you continue to review the patches? Thanks!

Best,
Wei

[virtio-dev] Re: [PATCH v9 0/8] virtio-balloon: free page hint support

[Wei Wang]

On 11/15/2018 06:07 PM, Wei Wang wrote:
> This is the deivce part implementation to add a new feature,
> VIRTIO_BALLOON_F_FREE_PAGE_HINT to the virtio-balloon device. The device
> receives the guest free page hints from the driver and clears the
> corresponding bits in the dirty bitmap, so that those free pages are
> not sent by the migration thread to the destination.
>
> *Tests
> 1 Test Environment
>      Host: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
>      Migration setup: migrate_set_speed 100G, migrate_set_downtime 400ms
>
> 2 Test Results (results are averaged over several repeated runs)
>      2.1 Guest setup: 8G RAM, 4 vCPU
>          2.1.1 Idle guest live migration time
>              Optimization v.s. Legacy = 620ms vs 2970ms
>              --> ~79% reduction
>          2.1.2 Guest live migration with Linux compilation workload
>            (i.e. make bzImage -j4) running
>            1) Live Migration Time:
>               Optimization v.s. Legacy = 2273ms v.s. 4502ms
>               --> ~50% reduction
>            2) Linux Compilation Time:
>               Optimization v.s. Legacy = 8min42s v.s. 8min43s
>               --> no obvious difference
>
>      2.2 Guest setup: 128G RAM, 4 vCPU
>          2.2.1 Idle guest live migration time
>              Optimization v.s. Legacy = 5294ms vs 41651ms
>              --> ~87% reduction
>          2.2.2 Guest live migration with Linux compilation workload
>            1) Live Migration Time:
>              Optimization v.s. Legacy = 8816ms v.s. 54201ms
>              --> 84% reduction
>            2) Linux Compilation Time:
>               Optimization v.s. Legacy = 8min30s v.s. 8min36s
>               --> no obvious difference
>
> ChangeLog:
> v8->v9:
> bitmap:
>      - fix bitmap_count_one to handle the nbits=0 case
> migration:
>      - replace the ram save notifier chain with a more general precopy
>        notifier chain, which is similar to the postcopy notifier chain.
>      - Avoid exposing the RAMState struct, and add a function,
>        precopy_disable_bulk_stage, to let the virtio-balloon notifier
>        callback to disable the bulk stage flag.

Hi Dave and Peter,

Could you continue to review the patches? Thanks!

Best,
Wei

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org