On Thu, May 19, 2016 at 05:14:33PM -0400, Laine Stump wrote: > On 05/19/2016 01:23 PM, Ján Tomko wrote: > > Yes, qemu_hotplug.c has a few of those places using == DEVICE_ADDRESS_TYPE_PCI > > untouched by this series. > > Since they happen after parse is finished, it should be safe. > > And anyway, looking at those uses, I think what most of them are doing > (calling virDomainPCIAddressEnsureAddr()) is 100% unnecessary, since > it's now already done when addresses are assigned in > qemuDomainDefAssignAddresses(), which has already been called. Actually, virDomainPCIAddressEnsureAddr is the place where the address gets assigned. But only when address == NONE or PCI, which is OK because virDomainPCIAddressEnsureAddr calls PCIAddressIsPresent to decide whether it needs to allocate a new one. qemuDomainDefAssignAddresses is only called after parsing the whole domain, not just one device. > (Back in > Jan 2010 when the calls to qemuDomainPCIAddressEnsureAddr() were added > (commit d8acc4), this was not the case - they were needed in order for > the new devices to get an address assigned, but a lot has changed since > then - even before Cole put in the postparse callback address assignment > stuff and called it when attaching a device, we had already been > assigning addresses during the higher level qemuDomainAttachDeviceConfig > for a long time (since commit f5dd58a in June 2012; qemuDomainAttachDeviceConfig is for changing the persistent definition. Hotplug in qemuDomainAttachDeviceLive does not call any other address assignment function. Jan > long enough that the > calls to qemuDomainCCWAddressAssign() that are also sprinkled throughout > qemu_hotplug.c in the same vicinity as the calls to > qemuDomainPCIAddressEnsureAddr() weren't needed *even at the time they > were added!* (commit f94646, March 2013). This was purely cargo-cult > coding, caused by commit f5dd58a failing to remove the calls to > ...EnsureAddr(). The interesting bit is that both of these commits were > put in in support of s390 virtio devices.). > > (Well, *that* was a nice trip down git "memory lane" (aka "blame") > > So, the result is that most of the code in qemu_hotplug that requires > checking for address type is unneeded, and I'm going to send a patch to > remove it. > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list