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.
> + * 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
> + 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) {
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list