On Thu, 2016-10-13 at 13:43 -0400, Laine Stump wrote:
> > > > > + if (nbuses > 0 && !addrs->buses[0].model) {
> > > > > + if
(virDomainPCIAddressBusSetModel(&addrs->buses[0],
> > > > > + VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
> > > > >
+ goto error;
> > > > > + }
> > > >
> > > > Shouldn't we use either PCI_ROOT or PCIE_ROOT based on the
> > > > machine type here? And set
hasPCIeRoot *after* doing so?
> > > >
> > > > Sorry for the questions, I guess this is the point in the
> > > > patch where I got a bit lost :(
> >
> > I'm
afraid you missed this question :)
>
> Sorry about the omission. I've tried to limit the code that decides
> whether or not there should be a pci-root or a
pcie-root to the one
> place when default controllers are being added (I forget the name of the
> function right now),
I guess you mean qemuDomainDefAddDefaultDevices()?
That's the function where pci{,e}-root is added, if not
already present in the configuration.
> and after that only decide based on whether a
> pci-root or pcie-root really is there in the config. My subconscious
> reasoning for this is that the extra calisthenics around supporting
> aarch64/virt with PCI(e) vs with mmio has made me wonder if there might
> be machinetypes in the future that could support one type of root bus or
> another (or none) according to what is in the config, rather than having
> a root bus just builtin to the machine. I don't know if that would ever
> happen, but if it did, this code would work without change - only the
> function adding the default controllers would need to be changed.
>
> (Note that I used the same logic when deciding how to right
> qemuDomainMachineHasPCI[e]Root())(still not sure about that name, but it
> can always be changed later to remove the "Machine" if someone doesn't
> like it)
That makes sense.
My point is that the code above, if I'm reading it correctly,
sets the model of bus 0 to PCI_ROOT if it's not already set.
But
1) qemuDomainDefAddDefaultDevices() mentioned above should
already have added pci{,e}-root to @def
2) if that's not the case, we should use either PCI_ROOT
or PCIE_ROOT based on what's appropriate for the machine
type
Looking at the code in qemuDomainDefAddDefaultDevices() it
seems like we would never find ourselves in the situation
where pci{,e}-root is needed but not present in @def by the
time qemuDomainPCIAddressSetCreate() is called, so I think
that chunk of code should just be removed.
> > > > A DMI-to-PCI bridge with index='1' has been added
> > > > automatically here...
> > > >
> > > > ... but it's not being used by any of the devices. So why
> > > > would it be added in the first place?
> > >
> > > That is a *very* good question!
>
> > Can't wait to know the answer! ;)
>
> Oh, now that I've looked in context of the patch ordering, I undestand:
> it's because patch 16/18 hasn't been applied yet. I'd forgotten the
> ordering...
Right you are :)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list