On Tue, Apr 28, 2009 at 12:25:41PM -0400, Cole Robinson wrote:
Hi all,
The attached patch fixes QEMU getCapabilities to refresh caps before returning them to the user. Currently the capabilities are only refreshed once (at daemon startup), which means libvirtd needs to be restarted to pick up changes if QEMU or KVM are installed while the daemon is running. See: https://bugzilla.redhat.com/show_bug.cgi?id=460649
There are several things 'wrong' with this change:
- We reset/rescan fields that won't change (host arch, mac address prefix). This should be fixed at some point, but isn't a big deal since total performance impact is negligible (see below).
- We only refresh the capabilities when the user calls getCapabilities, which means we are still carrying around stale caps prior to that, which is what libvirt validates against. In practice, virt-manager and virt-install both call getCapabilities often so this isn't an issue, though the caps internal API should probably address this at some point.
To test the performance impact, I used a simple python script:
import libvirt conn = libvirt.open("qemu:///system") for i in range(0, 30): conn.getCapabilities()
The time difference was on average .02 seconds slower, which I think is negligible.
More importantly I don't see this API call as being a performance critical one. It is not something applicaitions will be calling inside a hot loop.
diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 79ee072..6b5c17f 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -1872,10 +1872,12 @@ static int qemudGetNodeInfo(virConnectPtr conn,
static char *qemudGetCapabilities(virConnectPtr conn) { struct qemud_driver *driver = conn->privateData; - char *xml; + char *xml = NULL;
qemuDriverLock(driver); - if ((xml = virCapabilitiesFormatXML(driver->caps)) == NULL) + virCapabilitiesFree(qemu_driver->caps); + if ((qemu_driver->caps = qemudCapsInit()) == NULL || + (xml = virCapabilitiesFormatXML(driver->caps)) == NULL) virReportOOMError(conn); qemuDriverUnlock(driver);
The thing to be wary of now, is that all use of driver->caps needs to be protected by the driver mutex. Most usages are OK, but I spotted a couple that are not. 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 :|