On Thu, Feb 11, 2010 at 04:40:43PM +0000, Daniel P. Berrange wrote:
> Current PCI addresses are allocated at time of VM startup.
> To make them truely persistent, it is neccessary to do this
> at time of virDomainDefine/virDomainCreate. The code in
> qemuStartVMDaemon still remains in order to cope with upgrades
> from older libvirt releases
>
> * src/qemu/qemu_driver.c: Rename existing qemuAssignPCIAddresses
> to qemuDetectPCIAddresses. Add new qemuAssignPCIAddresses which
> does auto-allocation upfront. Call qemuAssignPCIAddresses from
> qemuDomainDefine and qemuDomainCreate to assign PCI addresses that
> can then be persisted. Don't clear PCI addresses at shutdown if
> they are intended to be persistent
> ---
> src/qemu/qemu_driver.c | 84 +++++++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 79 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 03d0f5f..69187fc 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -95,6 +95,7 @@ struct _qemuDomainObjPrivate {
> int *vcpupids;
>
> qemuDomainPCIAddressSetPtr pciaddrs;
> + int persistentAddrs;
> };
>
> static int qemudShutdown(void);
> @@ -857,6 +858,7 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq
> virDomainObjPtr obj = payload;
> struct qemud_driver *driver = opaque;
> qemuDomainObjPrivatePtr priv;
> + unsigned long long qemuCmdFlags;
>
> virDomainObjLock(obj);
>
> @@ -872,6 +874,15 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq
> goto error;
> }
>
> + /* XXX we should be persisting the original flags in the XML
> + * not re-detecting them, since the binary may have changed
> + * since launch time */
But where would we store them ? It sounds a bit strange, it's emulator
properties, not really domain ones, and I think we sound only store in
the domain what specific flags might be needed from the emulator, not
the full set (and this could change over time as a domain is being
modified).
> + if (qemudExtractVersionInfo(obj->def->emulator,
> + NULL,
> + &qemuCmdFlags) >= 0 &&
> + (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE))
> + priv->persistentAddrs = 1;
> +
> if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(obj->def)))
> goto error;
>
> @@ -1976,7 +1987,7 @@ qemuGetPCIWatchdogVendorProduct(virDomainWatchdogDefPtr def,
> * some static addrs on CLI. Have to check that...
> */
> static int
> -qemuAssignPCIAddresses(virDomainObjPtr vm,
> +qemuDetectPCIAddresses(virDomainObjPtr vm,
> qemuMonitorPCIAddress *addrs,
> int naddrs)
> {
> @@ -2098,7 +2109,7 @@ qemuInitPCIAddresses(struct qemud_driver *driver,
> &addrs);
> qemuDomainObjExitMonitorWithDriver(driver, vm);
>
> - ret = qemuAssignPCIAddresses(vm, addrs, naddrs);
> + ret = qemuDetectPCIAddresses(vm, addrs, naddrs);
>
> VIR_FREE(addrs);
>
> @@ -2141,6 +2152,44 @@ static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) {
> return -1;
> }
>
> +
> +static int
> +qemuAssignPCIAddresses(virDomainDefPtr def)
> +{
> + int ret = -1;
> + unsigned long long qemuCmdFlags = 0;
> + qemuDomainPCIAddressSetPtr addrs = NULL;
> + struct stat sb;
> +
> + if (stat(def->emulator, &sb) < 0) {
> + virReportSystemError(errno,
> + _("Cannot find QEMU binary %s"),
> + def->emulator);
> + goto cleanup;
> + }
do we really need to update that every time ? We can't cache forever
but it's not like the emulator is changing every second. Maybe we need
to put a watch on the emulator at the driver level and keep this in
the driver.
> + if (qemudExtractVersionInfo(def->emulator,
> + NULL,
> + &qemuCmdFlags) < 0)
> + goto cleanup;
> +
> + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
> + if (!(addrs = qemuDomainPCIAddressSetCreate(def)))
> + goto cleanup;
> +
> + if (qemuAssignDevicePCISlots(def, addrs) < 0)
> + goto cleanup;
> + }
> +
> + ret = 0;
> +
> +cleanup:
> + qemuDomainPCIAddressSetFree(addrs);
> +
> + return ret;
> +}
> +
> +
> static pciDeviceList *
> qemuGetPciHostDeviceList(virDomainDefPtr def)
> {
> @@ -2662,7 +2711,15 @@ static int qemudStartVMDaemon(virConnectPtr conn,
> goto cleanup;
> }
>
> + /*
> + * Normally PCI addresses are assigned inhe virDomainCreate
> + * or virDomainDefine methods. We might still need to assign
> + * some here to cope with the question of upgrades. Regardless
> + * we also need to populate the PCi address set cache for later
> + * use in hotplug
> + */
> if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
> + /* Populate cache with current addresses */
> if (priv->pciaddrs) {
> qemuDomainPCIAddressSetFree(priv->pciaddrs);
> priv->pciaddrs = NULL;
> @@ -2670,8 +2727,14 @@ static int qemudStartVMDaemon(virConnectPtr conn,
> if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(vm->def)))
> goto cleanup;
>
> +
> + /* Assign any remaining addresses */
> if (qemuAssignDevicePCISlots(vm->def, priv->pciaddrs) < 0)
> goto cleanup;
> +
> + priv->persistentAddrs = 1;
> + } else {
> + priv->persistentAddrs = 0;
> }
>
> vm->def->id = driver->nextvmid++;
> @@ -2903,10 +2966,12 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver,
> VIR_FREE(vm->def->seclabel.imagelabel);
> }
>
> - virDomainDefClearPCIAddresses(vm->def);
> virDomainDefClearDeviceAliases(vm->def);
> - qemuDomainPCIAddressSetFree(priv->pciaddrs);
> - priv->pciaddrs = NULL;
> + if (!priv->persistentAddrs) {
> + virDomainDefClearPCIAddresses(vm->def);
> + qemuDomainPCIAddressSetFree(priv->pciaddrs);
> + priv->pciaddrs = NULL;
> + }
>
> qemuDomainReAttachHostDevices(driver, vm->def);
>
> @@ -3352,6 +3417,12 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
> if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0)
> goto cleanup;
>
> + if (qemudCanonicalizeMachine(driver, def) < 0)
> + goto cleanup;
> +
> + if (qemuAssignPCIAddresses(def) < 0)
> + goto cleanup;
> +
> if (!(vm = virDomainAssignDef(driver->caps,
> &driver->domains,
> def)))
> @@ -5049,6 +5120,9 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
> if (qemudCanonicalizeMachine(driver, def) < 0)
> goto cleanup;
>
> + if (qemuAssignPCIAddresses(def) < 0)
> + goto cleanup;
> +
> if (!(vm = virDomainAssignDef(driver->caps,
> &driver->domains,
> def))) {
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list