[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]