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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list