On 06/16/2016 02:38 PM, Maxim Nestratov wrote:
From: Derbyshev Dmitry <dderbyshev@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@virtuozzo.com> Signed-off-by: Maxim Nestratov <mnestratov@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