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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list