[libvirt] [PATCH v3 06/10] Add capability to fetch balloon stats

[John Ferlan]

On 07/12/2013 08:42 AM, Daniel P. Berrange wrote:
> On Thu, Jul 11, 2013 at 07:55:56PM -0400, John Ferlan wrote:
>> This patch will add the qemuMonitorJSONGetMemoryStats() to execute a
>> "guest-stats" on the balloonpath using "get-qom" replacing the former
>> mechanism which looked through the "query-ballon" returned data for
>> the fields.  The "query-balloon" code only returns 'actual' memory.
>> Rather than duplicating the existing code, have the JSON API use the
>> GetBalloonInfo API.
>>
>> A check in the qemuMonitorGetMemoryStats() will be made to ensure the
>> balloon driver path has been set.  Since the underlying JSON code can
>> return data not associated with the balloon driver, we don't fail on
>> a failure to get the balloonpath.  Of course since we've made the check,
>> we can then set the ballooninit flag.  Getting the path here is primarily
>> due to the process reconnect path which doesn't attempt to set the
>> collection period.
>> ---
>>  src/qemu/qemu_monitor.c      |  10 ++-
>>  src/qemu/qemu_monitor_json.c | 190 ++++++++++++++++++++-----------------------
>>  src/qemu/qemu_monitor_json.h |   1 +
>>  3 files changed, 95 insertions(+), 106 deletions(-)
> 
> Can you also extend qemumonitorjsontest.c to cover the new
> code in  qemuMonitorJSONGetMemoryStats.
> 
>>
>> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
>> index a3e250c..14b80e3 100644
>> --- a/src/qemu/qemu_monitor.c
>> +++ b/src/qemu/qemu_monitor.c
>> @@ -1483,10 +1483,14 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
>>          return -1;
>>      }
>>  
>> -    if (mon->json)
>> -        ret = qemuMonitorJSONGetMemoryStats(mon, stats, nr_stats);
>> -    else
>> +    if (mon->json) {
>> +        ignore_value(qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/"));
>> +        mon->ballooninit = true;
> 
> Hmm, I think that qemuMonitorFindBalloonObjectPath ought to be
> setting 'mon->ballooninit = true' itself, rather than expecting
> all the callers to do it.
> 
> Actually I should have mentioned this against the previous patch.
> 

Was the previous explanation "good enough"?  Since it's recursive I see
no where in the function that one could safely set the value.

John

>> +        ret = qemuMonitorJSONGetMemoryStats(mon, mon->balloonpath,
>> +                                            stats, nr_stats);
>> +    } else {
>>          ret = qemuMonitorTextGetMemoryStats(mon, stats, nr_stats);
>> +    }
>>      return ret;
>>  }
>>  
> 
> Daniel
>