Re: [libvirt] [PATCH 3/6] qemu-driver: Enable domainMemStats in the qemu driver
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 :|