On Tue, 2018-09-18 at 13:51 +0800, Yi Min Zhao wrote:
> > > > +int
> > > >
+virDomainPCIAddressExtensionReserveNextAddr(virDomainPCIAddressSetPtr addrs,
> > > > + virPCIDeviceAddressPtr
dev,
> > > > +
virDomainPCIAddressExtensionFlags extFlags)
> > > > +{
> > > > + if ((extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) &&
> > > > + virZPCIDeviceAddressIsEmpty(&dev->zpci)) {
> > >
> > > You shouldn't need the second check: just go ahead and reserve the
> > > next address regardless of what's currently stored in the device
> > > info, no?
> >
> > I think it's hard to do it as what you said. We process assigned zpci
> > addresses
> > firstly. And then reserve next address for empty zpci addresses. If we
don't
> > check this, we might reserve an address for a reserved one.
>
> Shouldn't the EnsureAddr() function take care of avoiding that?
> It will call ReserveAddr() or ReserveNextAddr() based on whether
> or not an address has already been provided by the user.
IIUC, EnsureAddr() is handling hotplug case, and ReserveNextAddr() is also
called in startup stage. After reserve defined addresses, RerserveNextAddr()
is called to allocate addresses. Here, we should check if it's reserved.
You could see virDomainDeviceInfoIterate(def,
qemuDomainAssignPCIAddressExtension, addrs)
in qemuDomainAssignPCIAddresses().
Okay, qemuDomainAssignPCIAddresses() doesn't actually call
virDomainPCIAddressEnsureAddr() but it's basically doing the same
thing: after the first dry-run used to figure out how many PCI buses
are necessary, it will call qemuDomainPCIAddressSetCreate() which
internally calls qemuDomainCollectPCIAddress() on all devices, thus
picking up all PCI addresses that were already provided by the user;
then it calls qemuDomainAssignDevicePCISlots(), which calls
qemuDomainPCIAddressReserveNextAddr() on all devices, but *only*
after making sure with virDeviceInfoPCIAddressIsWanted() that they
hadn't already been assigned an address.
The end result is that qemuDomainPCIAddressReserveNextAddr() will
only ever be called on devices that were not already assigned a PCI
address, and thus virDomainPCIAddressReserveNextAddr() can afford
to simply pick an address and reserve it without checking first
whether the device in question had one already.
To keep things easy to understand, your functions should follow the
same semantics.
--
Andrea Bolognani / Red Hat / Virtualization