Re: [libvirt] [PATCH v2] qemu: Get memory balloon info correctly for text monitor

On 08/19/2011 04:17 AM, Osier Yang wrote: > * src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as > "balloon: actual=", which cause "actual=" is stripped early before > the real parsing. This patch changes BALLOON_PREFIX into "balloon: ", > and modifies related functions, also renames > "qemuMonitorParseExtraBalloonInfo" to "qemuMonitorParseBalloonInfo", > as after the changing, it parses all the info returned by "info > balloon". > > v2: > > Adopted Adam's suggestion, parse "actual=" outside of the loop > of qemuMonitorParseBalloonInfo, and use qemuMonitorParseBalloonInfo > for qemuMonitorTextGetBalloonInfo. > --- > src/qemu/qemu_monitor_text.c | 53 > +++++++++++++++++++++++++++-------------- > 1 files changed, 35 insertions(+), 18 deletions(-) Looks nicer. > > diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c > index 335e39e..a661626 100644 > --- a/src/qemu/qemu_monitor_text.c > +++ b/src/qemu/qemu_monitor_text.c > @@ -547,8 +547,12 @@ static int parseMemoryStat(char **text, unsigned > int tag, > return 0; > } > > - /* Convert bytes to kilobytes for libvirt */ > switch (tag) { > + /* Convert megabytes to kilobytes for libvirt */ > + case VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON: > + value = value<< 10; Why not use 'value <<= 10;' for fewer characters? > char *p = text; > unsigned int nr_stats_found = 0; > > + /* Since "actual=" always comes first in the returned string, > + * and sometime we only care about the value of "actual", such s/sometime/sometimes/ > + * as qemuMonitorGetBalloonInfo, so parse it outside of the Either s/so/we/, or s/Since// - leaving both 'since' and 'so' sounds awkward in the same sentence. ACK with the nits fixed.