On Wed, Nov 29, 2017 at 04:27:16PM +0100, Michal Privoznik wrote:
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.
Either way, commit 937f3195 needs to be completely or partially reverted before the release, since it breaks hotplug for domains that don't even use aliases. I do not particularly care whether the partial revert will be accompanied by breaking user aliases for implicit pci-root (as proposed here) or breaking user aliases for old QEMUs where HasPCIMultiBus is unfixable (proposed in: [PATCH 0/4] Fix hotplug after daemon restart) https://www.redhat.com/archives/libvir-list/2017-November/msg01151.html Jan
Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
