On Fri, Dec 11, 2009 at 03:26:14PM -0500, Adam Litke wrote:
> Support for memory statistics reporting is accepted for qemu inclusion.
>
> static int
> +qemudDomainMemoryStats (virDomainPtr dom,
> + struct _virDomainMemoryStat *stats,
> + unsigned int nr_stats)
> +{
> + struct qemud_driver *driver = dom->conn->privateData;
> + virDomainObjPtr vm;
> + unsigned int nr_stats_ret = -1;
> +
> + qemuDriverLock(driver);
> + vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> + qemuDriverUnlock(driver);
> +
> + if (!vm) {
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + virUUIDFormat(dom->uuid, uuidstr);
> + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
> + _("no domain with matching uuid '%s'"), uuidstr);
> + goto cleanup;
> + }
> +
> + if (virDomainIsActive(vm))
> + nr_stats_ret = qemuMonitorGetMemoryStats(vm, stats, nr_stats);
The thread locking rules require that you realize the lock on 'vm'
before invoking any monitor command and re-acquire it afterwards.
So this line of code should look like
qemuDomainObjPrivatePtr priv = vm->privateData;
qemuDomainObjEnterMonitor(vm);
nr_stats_ret = qemuMonitorGetMemoryStats(vm, stats, nr_stats);
qemuDomainObjExitMonitor(vm);
> +
> +cleanup:
> + if (vm)
> + virDomainObjUnlock(vm);
> + return nr_stats_ret;
> +}
In this method, if the domain is not active, it returns -1, without
atually reporting what the error was. There needs to be code like
if (!virDomainIsActive(vm)) {
qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list