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

[Jiri Denemark]

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...

Jirka