[libvirt] [PATCH] qemu: forbid user aliases for implicit pci-root controllers
Jiri Denemark
jdenemar at redhat.com
Wed Nov 29 14:15:31 UTC 2017
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
More information about the libvir-list
mailing list