Re: [libvirt] [PATCH v5 09/13] conf: Allocate/release 'uid' and 'fid' in PCI address

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.