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 :|