On 06/21/2016 10:08 PM, Laine Stump wrote:
On 06/20/2016 10:26 PM, Tomasz Flendrich wrote:
>> Apologies if I'm missing something, I didn't look too closely at this
series,
>> however have you seen this thread?
>>
>>
http://www.redhat.com/archives/libvir-list/2016-May/msg01071.html
> I haven’t noticed that some work has been done on that, thank you!
>
>> My understanding of the current code is that the cached vioserial/ccw/pciaddrs
>> lists in qemu aren't actually required…they were at one point to handle
>> older qemu, but we dropped that support. Maybe you can pick up my patches and
>> finish off dropping of pciaddrs and ccwaddrs? I suspect the pciaddrs cache in
>> bhyve can be dropped as well, I don't think it was ever strictly required,
the
>> code just followed the qemu example
> If we could do without the caching, it would make the current code simpler.
> There wouldn’t be those booleans in qemu_hotplug.c that remember whether
> an address has to be deleted or not in case something fails. We could
> delete qemuDomainReleaseDeviceAddress() and a few more functions.
I'm completely ignorant about vioserial and ccw. As far as the "cache" for
pci
addresses, I guess whether or not we want the cache depends completely on how
long it takes to reconstruct vs. how often a device is added.
Constructing the cache likely takes less time than parsing a single XML
document... it's just iterating over the parsed XML in memory and performing
the collision checks.
There is also
the issue of the mismatch between live and config devices'
address use, and
the inconsistency that can be caused by that (if you hotplug a device with
--live, then hotplug another with --live --config, then the 2nd device will
have the same address in config as the first has in the live state of the
guest (more importantly, the address of the 2nd device will change the next
time the domain is shutdown and restarted, which all of this address
assignment stuff is intended to avoid) - I don't know if that problem would be
more easily solved by a cache that is used for assigning addresses for both
--live and --config, or if, as Cole suggests, it would be better just to
remove the cache and rebuild the allocation table each time a new device is
added (this would mean that we would need to scan through all the live devices
*and* persistent devices to re-populate it)
For 'config' updates we don't even use the address cache presently, at least
for vioserialaddr, I didn't confirm for the other ones. It's only used for
hotplug. For 'config' updates we basically just insert the device into the XML
and effectively 'redefine' it, and the generic XML parsing machinery and
domain callbacks assign addresses.
The address list is only needed for runtime hotplug because we need to
generate an address on demand, for passing to the qemu monitor commands.
Thanks,
Cole