Re: [Qemu-devel] [PATCH v1] migration: skip sending ram pages released by virtio-balloon driver.

[Jitendra Kolhe]

On 3/7/2016 10:35 PM, Eric Blake wrote:
> On 03/04/2016 02:02 AM, Jitendra Kolhe wrote:
>> While measuring live migration performance for qemu/kvm guest, it
>> was observed that the qemu doesn’t maintain any intelligence for the
>> guest ram pages which are release by the guest balloon driver and
>> treat such pages as any other normal guest ram pages. This has direct
>> impact on overall migration time for the guest which has released
>> (ballooned out) memory to the host.
>>
>
>> Signed-off-by: Jitendra Kolhe <address@hidden>
>> ---
>>  balloon.c                          | 253 
>> ++++++++++++++++++++++++++++++++++++-
>>  exec.c                             |   3 +
>>  hw/virtio/virtio-balloon.c         |  35 ++++-
>>  include/hw/virtio/virtio-balloon.h |   1 +
>>  include/migration/migration.h      |   1 +
>>  include/sysemu/balloon.h           |  15 ++-
>>  migration/migration.c              |   9 ++
>>  migration/ram.c                    |  23 +++-
>>  qapi-schema.json                   |   5 +-
>>  9 files changed, 337 insertions(+), 8 deletions(-)
>>
>
>> +++ b/qapi-schema.json
>> @@ -544,11 +544,14 @@
>>  #          been migrated, pulling the remaining pages along as needed. 
>> NOTE: If
>>  #          the migration fails during postcopy the VM will fail.  (since 
>> 2.5)
>>  #
>> +# @skip-balloon: Skip scaning ram pages released by virtio-balloon driver.
>
> s/scaning/scanning/
>
>> +#          (since 2.5)
>
> You've missed 2.5.  In fact, this is borderline between new feature and
> bug fix, so you may have even missed 2.6 since soft freeze has already
> passed, in which case this should read 2.7.

Thanks for sharing the timeline was not aware of it. I think making changes to 
version 2.7 should be fine. However the version string "(since 2.5)" is
part of existing code. I have added a new option "skip-balloon" below it.

>
> Does this need to be an option, or should it be unconditionally enabled?
>

That's a good point, ideally we should be able to enable the option 
unconditionally.
In which case, we may not even need to have the new capability. Currently we 
have
disabled the option by default just to make sure that we don't break any 
migration
compatibility. Although I have not seen any such breakage during my testing, I 
am not
sure whether we are taking care of all subcomponents, expecting some comments 
this part.

Thanks,
- Jitendra