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

David Hildenbrand
2020-05-15T12:12:11Z

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