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

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.