On 08/17/2010 09:18 AM, Daniel P. Berrange wrote:
On Tue, Aug 17, 2010 at 09:11:30AM -0600, Eric Blake wrote:
> On 08/16/2010 11:08 AM, Jiri Denemark wrote:
>> +void
>> +qemuDomainPCIAddressSetUpdate(qemuDomainPCIAddressSetPtr addrs)
>> +{
>> +
>> + if (!addrs)
>> + return;
>> +
>> + virHashForEach(addrs->used, qemuDomainPCIAddressSetUpdateIter, addrs);
>
> Does virHashForEach visit the hash table in the same order in which
> entries were inserted, or does it visit in random order based on how the
> hashes map to buckets? If the latter, then how are you guaranteeing
> that you are setting nextslot to the right value, if you don't know if
> you are visiting the correct table entry last?
The hash iterator visits in an unpredictable order. It shouldn't matter
because for this usage, all that is important is that 'nextslot' eventually
ends up with the largest slot ID + 1.
We don't want the first unused slot, rather the last slot reservation
that was in place before the reconnect (often the first unused slot if
you haven't done a lot of hot-unplugging, but not necessarily the case).
The only way I can see to get at this is to revisit the reservations in
the same order as they were reserved, rather than in the random order
returned by the hash table, or at least keep track of whether each pci
reservation has had subsequent reservations, vs. being the last
reservation in the chain.
qemuDomainPCIAddressSetUpdateIter currently sets nextslot to pci->slot+1
for each pci, without regards to what order the various pci reservations
are visited in; but it seems to me that it should _only_ call
qemuDomainPCIAddressSetUpdateNextSlot once, on just the particular pci
that is provably the last reservation in the original chain of reservations.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org