[libvirt] [PATCH] Refresh QEMU driver caps in getCapabilities
Daniel P. Berrange
berrange at redhat.com
Wed Apr 29 12:32:30 UTC 2009
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 :|
More information about the libvir-list
mailing list