On 06/16/2016 02:38 PM, Maxim Nestratov wrote:
From: Derbyshev Dmitry <dderbyshev(a)virtuozzo.com>
To collect all balloon statistics for all guests it was necessary to make
several libvirt requests. Now it's possible to get all balloon statiscs via
single connectGetAllDomainStats call.
Signed-off-by: Derbyshev Dmitry <dderbyshev(a)virtuozzo.com>
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/qemu/qemu_driver.c | 95 +++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 74 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2885936..675ac5a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11073,32 +11073,23 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom,
return ret;
}
+/* This functions assumes that job QEMU_JOB_QUERY is started by a caller */
^
Extra space
+
static int
-qemuDomainMemoryStats(virDomainPtr dom,
- virDomainMemoryStatPtr stats,
- unsigned int nr_stats,
- unsigned int flags)
+qemuDomainMemoryStatsInternal(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainMemoryStatPtr stats,
+ unsigned int nr_stats)
+
{
- virQEMUDriverPtr driver = dom->conn->privateData;
- virDomainObjPtr vm;
int ret = -1;
+ qemuDomainObjPrivatePtr priv;
This fails to compile as 'priv' isn't used.
long rss;
- virCheckFlags(0, -1);
-
- if (!(vm = qemuDomObjFromDomain(dom)))
- goto cleanup;
-
- if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0)
- goto cleanup;
-
- if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
- goto cleanup;
-
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
- goto endjob;
+ goto cleanup;
No need to cleanup, just return -1
}
if (vm->def->memballoon &&
@@ -11109,7 +11100,7 @@ qemuDomainMemoryStats(virDomainPtr dom,
ret = -1;
if (ret < 0 || ret >= nr_stats)
- goto endjob;
+ goto cleanup;
Likewise, return ret
} else {
ret = 0;
}
@@ -11123,7 +11114,33 @@ qemuDomainMemoryStats(virDomainPtr dom,
ret++;
}
- endjob:
+ cleanup:
+ return ret;
Rendering cleanup: unnecessary
+}
+
+static int
+qemuDomainMemoryStats(virDomainPtr dom,
+ virDomainMemoryStatPtr stats,
+ unsigned int nr_stats,
+ unsigned int flags)
+{
+ virQEMUDriverPtr driver = dom->conn->privateData;
+ virDomainObjPtr vm;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = qemuDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+ goto cleanup;
+
+ ret = qemuDomainMemoryStatsInternal(driver, vm, stats, nr_stats);
+
qemuDomainObjEndJob(driver, vm);
cleanup:
@@ -18608,15 +18625,29 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver
ATTRIBUTE_UNUSED,
return 0;
}
+
+#define STORE_MEM_RECORD(TAG, NAME) { \
+ if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ ##TAG) \
+ if (virTypedParamsAddULLong(&record->params, \
+ &record->nparams, \
+ maxparams, \
+ "balloon." NAME, \
+ stats[i].val) < 0) \
+ return -1; \
+}
+
All of the rest feels like a separate patch...
If I understand correctly this is for 'domstats' to display, correct?
If so, then virsh.pod could use an update.
static int
qemuDomainGetStatsBalloon(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
This is now used too.
virDomainObjPtr dom,
virDomainStatsRecordPtr record,
int *maxparams,
- unsigned int privflags ATTRIBUTE_UNUSED)
+ unsigned int privflags)
{
qemuDomainObjPrivatePtr priv = dom->privateData;
+ virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR];
+ int nr_stats;
unsigned long long cur_balloon = 0;
+ size_t i;
int err = 0;
if (!virDomainDefHasMemballoon(dom->def)) {
@@ -18641,8 +18672,30 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver
ATTRIBUTE_UNUSED,
virDomainDefGetMemoryActual(dom->def)) < 0)
return -1;
In order to get "balloon.current" 'err' must be 0, so what would allow
any of the following to be collected if err == -1?
+ if (!HAVE_JOB(privflags))
+ return 0;
+
+ nr_stats = qemuDomainMemoryStatsInternal(driver, dom, stats,
+ VIR_DOMAIN_MEMORY_STAT_NR);
'driver' is now no longer unused it seems...
+ if (nr_stats < 0)
+ return 0;
+
+ for (i = 0; i < nr_stats; i++) {
+ STORE_MEM_RECORD(SWAP_IN, "swap_in")
+ STORE_MEM_RECORD(SWAP_OUT, "swap_out")
+ STORE_MEM_RECORD(MAJOR_FAULT, "major_fault")
+ STORE_MEM_RECORD(MINOR_FAULT, "minor_fault")
+ STORE_MEM_RECORD(UNUSED, "unused")
+ STORE_MEM_RECORD(AVAILABLE, "available")
+ STORE_MEM_RECORD(ACTUAL_BALLOON, "actual")
Doesn't this ^ replicate "balloon.current"
John
+ STORE_MEM_RECORD(RSS, "rss")
+ STORE_MEM_RECORD(LAST_UPDATE, "last-update")
+ STORE_MEM_RECORD(USABLE, "usable")
+ }
+
return 0;
}
+#undef STORE_MEM_RECORD
static int