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