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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list