于 2011年08月16日 00:40, Adam Litke 写道:
Hi Osier,
Just to be clear, this is a cleanup not a bugfix right? The current
code should be working properly as written.
No, "virDomainMemoryStats" will return empty result if libvirt
communicates to qemu monitor in text mode. Actually this
is reported by a upstream Debian user, It seems to me packager
of Debian compiles libvirt without QMP support.
On 08/15/2011 08:58 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".
---
src/qemu/qemu_monitor_text.c | 47 +++++++++++++++++++++++++----------------
1 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 7bf733d..d17d863 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;
+ break;
+ /* Convert bytes to kilobytes for libvirt */
case VIR_DOMAIN_MEMORY_STAT_SWAP_IN:
case VIR_DOMAIN_MEMORY_STAT_SWAP_OUT:
case VIR_DOMAIN_MEMORY_STAT_UNUSED:
@@ -565,15 +569,17 @@ static int parseMemoryStat(char **text, unsigned int tag,
/* The reply from the 'info balloon' command may contain additional memory
* statistics in the form: '[,<tag>=<val>]*'
*/
-static int qemuMonitorParseExtraBalloonInfo(char *text,
- virDomainMemoryStatPtr stats,
- unsigned int nr_stats)
+static int qemuMonitorParseBalloonInfo(char *text,
+ virDomainMemoryStatPtr stats,
+ unsigned int nr_stats)
{
char *p = text;
unsigned int nr_stats_found = 0;
while (*p&& nr_stats_found< nr_stats) {
- if (parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN,
+ if (parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON,
+ "actual=",&stats[nr_stats_found]) ||
+ parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN,
",mem_swapped_in=",&stats[nr_stats_found]) ||
parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_SWAP_OUT,
",mem_swapped_out=",&stats[nr_stats_found]) ||
According to this code (and actual monitor behavior) 'actual' always
appears and has to come first. Therefore, I would still parse the
'actual' stat outside of the while loop.
@@ -584,9 +590,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text,
parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_UNUSED,
",free_mem=",&stats[nr_stats_found]) ||
parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_AVAILABLE,
- ",total_mem=",&stats[nr_stats_found]) ||
- parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON,
- ",actual=",&stats[nr_stats_found]))
+ ",total_mem=",&stats[nr_stats_found]))
nr_stats_found++;
/* Skip to the next label. When *p is ',' the last match attempt
@@ -602,7 +606,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text,
/* The reply from QEMU contains 'ballon: actual=421' where value is in MB */
-#define BALLOON_PREFIX "balloon: actual="
+#define BALLOON_PREFIX "balloon: "
/*
* Returns: 0 if balloon not supported, +1 if balloon query worked
@@ -625,13 +629,22 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon,
unsigned int memMB;
char *end;
offset += strlen(BALLOON_PREFIX);
- if (virStrToLong_ui(offset,&end, 10,&memMB)< 0) {
- qemuReportError(VIR_ERR_OPERATION_FAILED,
- _("could not parse memory balloon allocation from '%s'"), reply);
- goto cleanup;
+
+ if (STRPREFIX(offset, "actual=")) {
+ offset += strlen("actual=");
+
+ if (virStrToLong_ui(offset,&end, 10,&memMB)< 0) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED,
+ _("could not parse memory balloon allocation from '%s'"), reply);
+ goto cleanup;
+ }
+ *currmem = memMB * 1024;
+ ret = 1;
+ } else {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected balloon information '%s'"), reply);
+ ret = -1;
}
- *currmem = memMB * 1024;
- ret = 1;
} else {
/* We don't raise an error here, since its to be expected that
* many QEMU's don't support ballooning
Why not just use qemuMonitorParseBalloonInfo() for the logic above? You
only need to request one stat since actual must be first.
Not sure if I get your meaning correctly.
If parsing "actual" in qemuMonitorParseBalloonInfo, then yes, agree
using qemuMonitorParseballoonInfo here is more simpler.
But it looks to me you object to parse "actual=" in
qemuMonitorParseBalloonInfo. :)
I want to explain more about this, all the purpose of this patch
is to prevent "actual=" being stripped before the parsing. And it's
fix a problem, not cleanup.
Osier
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list