Re: [libvirt] [PATCH 16/34] Clear assigned PCI devices at shutdown

On Fri, Jan 08, 2010 at 05:23:12PM +0000, Daniel P. Berrange wrote: > The PCI device addresses are only valid while the VM is running, > since they are auto-assigned by QEMU. After shutdown they must > all be cleared. Future QEMU driver enhancement will allow for > persistent PCI address assignment > > * src/conf/domain_conf.h, src/conf/domain_conf.c, src/libvirt_private.syms > Add virDomainDefClearPCIAddresses() method for wiping out auto assigned > PCI addresses > * src/qemu/qemu_driver.c: Clear PCI addresses at VM shutdown > --- > src/conf/domain_conf.c | 36 ++++++++++++++++++++++++++++++++++++ > src/conf/domain_conf.h | 2 ++ > src/libvirt_private.syms | 1 + > src/qemu/qemu_driver.c | 2 ++ > 4 files changed, 41 insertions(+), 0 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 5edd060..ef5dbe9 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -816,6 +816,42 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) > } > > > +static void virDomainDeviceInfoClearField(virDomainDeviceInfoPtr info) > +{ > + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { > + memset(&info->addr, 0, sizeof(info->addr)); > + info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE; > + } > +} > + > + > +static void virDomainDefClearDeviceInfo(virDomainDefPtr def) > +{ > + int i; > + > + for (i = 0; i < def->ndisks ; i++) > + virDomainDeviceInfoClearField(&def->disks[i]->info); > + for (i = 0; i < def->nnets ; i++) > + virDomainDeviceInfoClearField(&def->nets[i]->info); > + for (i = 0; i < def->nsounds ; i++) > + virDomainDeviceInfoClearField(&def->sounds[i]->info); > + for (i = 0; i < def->nhostdevs ; i++) > + virDomainDeviceInfoClearField(&def->hostdevs[i]->info); > + for (i = 0; i < def->nvideos ; i++) > + virDomainDeviceInfoClearField(&def->videos[i]->info); > + for (i = 0; i < def->ncontrollers ; i++) > + virDomainDeviceInfoClearField(&def->controllers[i]->info); > + if (def->watchdog) > + virDomainDeviceInfoClearField(&def->watchdog->info); > +} > + > + > +void virDomainDefClearPCIAddresses(virDomainDefPtr def) > +{ > + virDomainDefClearDeviceInfo(def); > +} > + > + > /* Generate a string representation of a device address > * @param address Device address to stringify > */ > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 1831d17..a6f7ab2 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -736,6 +736,8 @@ int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr); > int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr addr); > int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info); > void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info); > +void virDomainDefClearPCIAddresses(virDomainDefPtr def); > + > void virDomainDefFree(virDomainDefPtr vm); > void virDomainObjRef(virDomainObjPtr vm); > /* Returns 1 if the object was freed, 0 if more refs exist */ > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index a4a02e7..678d610 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -187,6 +187,7 @@ virDomainControllerTypeToString; > virDomainControllerDefFree; > virDomainDeviceAddressTypeToString; > virDomainDefAddDiskControllers; > +virDomainDefClearPCIAddresses; > > > # domain_event.h > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index cd406ec..6a3af61 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -3032,6 +3032,8 @@ static void qemudShutdownVMDaemon(virConnectPtr conn, > VIR_FREE(vm->def->seclabel.imagelabel); > } > > + virDomainDefClearPCIAddresses(vm->def); > + > if (qemuDomainSetAllDeviceOwnership(conn, driver, vm->def, 1) < 0) > VIR_WARN("Failed to restore all device ownership for %s", > vm->def->name); > -- > 1.6.5.2 > Hum, I don't see in the patch what's makes the difference between an auto-assigned PCI address and one which might be set by the user. Since it shows up in the XML construct it has to be setable by the user so I'm a bit confused here,