Re: [PATCH v1 05/17] virtio-balloon: Rip out qemu_balloon_inhibit()

[David Hildenbrand <david@redhat.com>]

On 15.05.20 14:09, Dr. David Alan Gilbert wrote:
> * David Hildenbrand (david@redhat.com) wrote:
>> The only remaining special case is postcopy. It cannot handle
>> concurrent discards yet, which would result in requesting already sent
>> pages from the source. Special-case it in virtio-balloon instead.
>>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Juan Quintela <quintela@redhat.com>
>> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  balloon.c                  | 18 ------------------
>>  hw/virtio/virtio-balloon.c | 12 +++++++++++-
>>  include/sysemu/balloon.h   |  2 --
>>  migration/postcopy-ram.c   | 23 -----------------------
>>  4 files changed, 11 insertions(+), 44 deletions(-)
>>
>> diff --git a/balloon.c b/balloon.c
>> index c49f57c27b..354408c6ea 100644
>> --- a/balloon.c
>> +++ b/balloon.c
>> @@ -36,24 +36,6 @@
>>  static QEMUBalloonEvent *balloon_event_fn;
>>  static QEMUBalloonStatus *balloon_stat_fn;
>>  static void *balloon_opaque;
>> -static int balloon_inhibit_count;
>> -
>> -bool qemu_balloon_is_inhibited(void)
>> -{
>> -    return atomic_read(&balloon_inhibit_count) > 0 ||
>> -           ram_block_discard_is_broken();
>> -}
>> -
>> -void qemu_balloon_inhibit(bool state)
>> -{
>> -    if (state) {
>> -        atomic_inc(&balloon_inhibit_count);
>> -    } else {
>> -        atomic_dec(&balloon_inhibit_count);
>> -    }
>> -
>> -    assert(atomic_read(&balloon_inhibit_count) >= 0);
>> -}
>>  
>>  static bool have_balloon(Error **errp)
>>  {
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index a4729f7fc9..aa5b89fb47 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -29,6 +29,7 @@
>>  #include "trace.h"
>>  #include "qemu/error-report.h"
>>  #include "migration/misc.h"
>> +#include "migration/postcopy-ram.h"
>>  
>>  #include "hw/virtio/virtio-bus.h"
>>  #include "hw/virtio/virtio-access.h"
>> @@ -63,6 +64,15 @@ static bool virtio_balloon_pbp_matches(PartiallyBalloonedPage *pbp,
>>      return pbp->base_gpa == base_gpa;
>>  }
>>  
>> +static bool virtio_balloon_inhibited(void)
>> +{
>> +    PostcopyState ps = postcopy_state_get();
>> +
>> +    /* Postcopy cannot deal with concurrent discards (yet), so it's special. */
>> +    return ram_block_discard_is_broken() ||
>> +           (ps >= POSTCOPY_INCOMING_DISCARD && ps < POSTCOPY_INCOMING_END);
> 
> It's a shame this is open-coded here; it would be better to have
> something in migration.c ; we have a migration_in_postcopy but that's
> really the sending side; a 'migration_in_incoming_postcopy' would
> perhaps be good.

Yes, makes sense - then I can also reuse it in patch #10.

Thanks!

-- 
Thanks,

David / dhildenb