Re: [libvirt] [PATCH 0/6] auto-assign addresses when <address type='pci'/> is specified
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list