[libvirt] [PATCH] qemu: forbid user aliases for implicit pci-root controllers

[Michal Privoznik]

On 11/29/2017 03:15 PM, Jiri Denemark wrote:
> On Wed, Nov 29, 2017 at 11:57:30 +0100, Michal Privoznik wrote:
>> On 11/28/2017 02:51 PM, Ján Tomko wrote:
>>> For implicit controllers, we do not format user aliases on the command
>>> line so we provide a translation between the user alias and the qemu
>>> alias.
>>>
>>> However for pci-root controllers, the logic determining whether to use
>>> 'pci' or 'pci.0' depends on:
>>> * domain architecture
>>> * machine type
>>> * QEMU version (for PPC)
>>>
>>> Since we do not store the QEMU version in status XML and as of
>>> commit 5b783379 we no longer have the QEMU_CAPS_PCI_MULTIBUS
>>> capability formatted there either, we can only rely on the alias
>>> of the controller formatted there.
>>>
>>> Forbid user aliases for the implicit pci-root controllers so that
>>> we retain this information.
>>>
>>> This allows us to drop the call to virQEMUCapsHasPCIMultiBus,
>>> and fixing hotplug after daemon restart, since virQEMUCapsHasPCIMultiBus
>>> relies on qemuCaps->arch which is not filled out by reading
>>> the status XML either.
>>>
>>> Partially reverts commit 937f3195 which added the user alias ->
>>> qemu alias mapping for implicit PCI controllers.
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1518148
>>> ---
>>>
>>> While we cannot reliably map user aliases to qemu aliases in
>>> every corner case, I am not sure where to draw the line.
>>>
>>> For x86_64, HasPCIMultiBus could be fixed by using def->os.arch instead
>>> of qemuCaps->arch. So only PPC is unfixable, but this looked like the option
>>> with the least amount of exceptions.
>>>
>>>  src/conf/domain_conf.c                             |  3 +--
>>>  src/conf/domain_conf.h                             |  2 ++
>>>  src/qemu/qemu_command.c                            | 16 +++++--------
>>>  src/qemu/qemu_domain.c                             | 26 +++++++++++++++++++++-
>>>  .../qemuxml2argvdata/qemuxml2argv-user-aliases.xml |  4 +---
>>>  5 files changed, 35 insertions(+), 16 deletions(-)
>>
>> I'm not a fan of this. If virQEMUCapsHasPCIMultiBus() returns different
>> results before and after libvirtd is restarted than it will bite us
>> again in the future. While you're fixing one instance of the bug, the
>> bug itself is still present. I suggest we extend our status XML to
>> contain all the info needed so that we can make right decision
>> regardless of how many times libvirtd is restarted.
> 
> Which would break any running domains, because they won't have the right
> info there...

Well, there's no good solution here that'd work 100%, is there. What we
might do though is to forbid setting user aliases on those ancient qemu
which lack the multibus capability.

Michal