LGTM. Thanks, Jose
On Mar 17 14:00, Dimitris Aragiorgis wrote: > Introduce new method `_GetFreeSlot()` responsible only for bitarray > operations. It fixes search in case of bitarray is either '0000..' > or '1111..'. > > Use it instead of `_UpdatePCISlots()` and in `_GetFreePCISlot()`. > > Signed-off-by: Dimitris Aragiorgis <[email protected]> > --- > lib/hypervisor/hv_kvm.py | 55 > ++++++++++++++++++++++++---------------------- > 1 file changed, 29 insertions(+), 26 deletions(-) > > diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py > index dcb4329..796e2a8 100644 > --- a/lib/hypervisor/hv_kvm.py > +++ b/lib/hypervisor/hv_kvm.py > @@ -137,29 +137,36 @@ def _GenerateDeviceKVMId(dev_type, dev): > return "%s-%s-pci-%d" % (dev_type.lower(), dev.uuid.split("-")[0], dev.pci) > > > -def _UpdatePCISlots(dev, pci_reservations): > - """Update pci configuration for a stopped instance > +def _GetFreeSlot(slots, slot=None, reserve=False): > + """Helper method to get first available slot in a bitarray > + > + @type slots: bitarray > + @param slots: the bitarray to operate on > + @type slot: integer > + @param slot: if given we check whether the slot is free > + @type reserve: boolean > + @param reserve: whether to reserve the first available slot or not > + @return: the idx of the (first) available slot > + @raise errors.HotplugError: If all slots in a bitarray are occupied > + or the given slot is not free. > > - If dev has a pci slot then reserve it, else find first available > - in pci_reservations bitarray. It acts on the same objects passed > - as params so there is no need to return anything. > + """ > + if slot is not None: > + assert slot < len(slots) > + if slots[slot]: > + raise errors.HypervisorError("Slots %d occupied" % slot) > > - @type dev: L{objects.Disk} or L{objects.NIC} > - @param dev: the device object for which we update its pci slot > - @type pci_reservations: bitarray > - @param pci_reservations: existing pci reservations for an instance > - @raise errors.HotplugError: in case an instance has all its slot occupied > + else: > + avail = slots.search(_AVAILABLE_PCI_SLOT, 1) > + if not avail: > + raise errors.HypervisorError("All slots occupied") > > - """ > - if dev.pci: > - free = dev.pci > - else: # pylint: disable=E1103 > - [free] = pci_reservations.search(_AVAILABLE_PCI_SLOT, 1) > - if not free: > - raise errors.HypervisorError("All PCI slots occupied") > - dev.pci = int(free) > + slot = int(avail[0]) > + > + if reserve: > + slots[slot] = True > > - pci_reservations[free] = True > + return slot > > > def _GetExistingDeviceInfo(dev_type, device, runtime): > @@ -1670,12 +1677,12 @@ class KVMHypervisor(hv_base.BaseHypervisor): > pci_reservations = bitarray(self._DEFAULT_PCI_RESERVATIONS) > kvm_disks = [] > for disk, link_name, uri in block_devices: > - _UpdatePCISlots(disk, pci_reservations) > + disk.pci = _GetFreeSlot(pci_reservations, disk.pci, True) > kvm_disks.append((disk, link_name, uri)) > > kvm_nics = [] > for nic in instance.nics: > - _UpdatePCISlots(nic, pci_reservations) > + nic.pci = _GetFreeSlot(pci_reservations, nic.pci, True) > kvm_nics.append(nic) > > hvparams = hvp > @@ -2029,11 +2036,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): > slot = int(match.group(1)) > slots[slot] = True > > - [free] = slots.search(_AVAILABLE_PCI_SLOT, 1) # pylint: disable=E1101 > - if not free: > - raise errors.HypervisorError("All PCI slots occupied") > - > - dev.pci = int(free) > + dev.pci = _GetFreeSlot(slots) > > def VerifyHotplugSupport(self, instance, action, dev_type): > """Verifies that hotplug is supported. > -- > 1.7.10.4 > -- Jose Antonio Lopes Ganeti Engineering Google Germany GmbH Dienerstr. 12, 80331, München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores Steuernummer: 48/725/00206 Umsatzsteueridentifikationsnummer: DE813741370
