On 07/11/2013 10:33 AM, Daniel P. Berrange wrote:
On Mon, Jul 08, 2013 at 03:20:31PM -0400, John Ferlan wrote:
At vm startup, reconnect, and attach - check for the presence of the balloon driver and save the path in the private area of the driver. This path will remain constant throughout the life of the domain and can then be used rather than attempting to find the path each time balloon driver statistics are fetched or the collection period changes. The qom object model model requires setting object properties after device startup. That is, it's not possible to pass the period along via the startup code as it won't be recognized. If a balloon driver path is found a check of the existing collection period will be made against the saved domain value in order to determine if an adjustment needs to be made to the period to start or stop collecting stats --- src/qemu/qemu_monitor.c | 130 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 42 ++++++++++++++ src/qemu/qemu_monitor_json.h | 3 + src/qemu/qemu_process.c | 21 ++++++- 5 files changed, 196 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fe5ab0a..038c9e8 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -83,6 +83,9 @@ struct _qemuMonitor {
/* cache of query-command-line-options results */ virJSONValuePtr options; + + /* If found, path to the virtio memballoon driver */ + char *balloonpath;
Hmm, we can't distinguish between "not searched yet" and "searched, but didn't find one". This means in the latter case we'll repeatedly search every time. Not too bad, but a little wasteful. Perhaps add a 'bool ballooninit' to track whether we've searched yet.
Oh yeah - added that, thanks.
+ * Returns: + * + * 1 - Found + * 0 - Not found still looking + * -1 - Error bail out + * + * NOTE: This assumes we have already called qemuDomainObjEnterMonitor() + */ +static int +qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon, + virDomainObjPtr vm, + const char *curpath) +{ + int i,j;
size_t now
Yep
+ int npaths = 0; + int nprops = 0; + int ret = 0; + char *nextpath = NULL; + qemuMonitorJSONListPathPtr *paths = NULL; + qemuMonitorJSONListPathPtr *bprops = NULL; + + /* Not supported */ + if (vm->def->memballoon && + vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) { + VIR_DEBUG("Model must be virtio to get memballoon path"); + return -1; + }
What about if 'vm->def->memballoon == NULL' ? Shouldn't we return -1 in that case too ? eg this condition should actually be
if (vm->def->memballoon == NULL || vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
Ah - right! Must've been sleeping when I used the && and not a if (!vm->def->memballoon...) John