LGTM, thanks

On Thu, Oct 8, 2015 at 5:40 AM, Dimitris Aragiorgis <
[email protected]> wrote:

> In order to support the theoretical maximum of devices (16 disks and
> 8 NICs) we introduce kvm_pci_reservations hvparam that denotes the
> number of PCI slots that QEMU will implicitly use, e.g. if
> kvm_pci_reservations is set to 5, QEMU will manage PCI slots 0, 1,
> 2, 3, 4. By default this will be 12 and Ganeti will start adding
> disks and NICs from the 13rd slot (or whatever the next slot is)
> onwards.
>
> Signed-off-by: Dimitris Aragiorgis <[email protected]>
> ---
>  lib/hypervisor/hv_kvm/__init__.py            |   24
> +++++++++++++++++++++---
>  lib/hypervisor/hv_kvm/monitor.py             |    4 ++--
>  man/gnt-instance.rst                         |    8 ++++++++
>  src/Ganeti/Constants.hs                      |   11 +++++++++++
>  test/py/ganeti.hypervisor.hv_kvm_unittest.py |    5 ++++-
>  5 files changed, 46 insertions(+), 6 deletions(-)
>
> diff --git a/lib/hypervisor/hv_kvm/__init__.py
> b/lib/hypervisor/hv_kvm/__init__.py
> index bac72c0..d50a9a0 100644
> --- a/lib/hypervisor/hv_kvm/__init__.py
> +++ b/lib/hypervisor/hv_kvm/__init__.py
> @@ -515,6 +515,11 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>      constants.HV_KVM_EXTRA: hv_base.NO_CHECK,
>      constants.HV_KVM_MACHINE_VERSION: hv_base.NO_CHECK,
>      constants.HV_KVM_MIGRATION_CAPS: hv_base.NO_CHECK,
> +    constants.HV_KVM_PCI_RESERVATIONS:
> +      (False, lambda x: (x >= 0 and x <= constants.QEMU_PCI_SLOTS),
> +       "The number of PCI slots managed by QEMU (max: %s)" %
> +       constants.QEMU_PCI_SLOTS,
> +       None, None),
>      constants.HV_VNET_HDR: hv_base.NO_CHECK,
>      }
>
> @@ -570,6 +575,11 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>    # NOTE: This maps to the default PCI bus created by pc machine type
>    # by default (pci.0). The q35 creates a PCIe bus that is not
> hotpluggable
>    # and should be handled differently (pcie.0).
> +  # NOTE: This bitarray here is defined for more fine-grained control.
> +  # Currently the number of slots is QEMU_PCI_SLOTS and the reserved
> +  # ones are the first QEMU_DEFAULT_PCI_RESERVATIONS.
> +  # If the above constants change without updating
> _DEFAULT_PCI_RESERVATIONS
> +  # properly, TestGenerateDeviceHVInfo() will probably break.
>    _DEFAULT_PCI_RESERVATIONS = "11111111111100000000000000000000"
>    # The SCSI bus is created on demand or automatically and is empty.
>    # For simplicity we decide to use a different target (scsi-id)
> @@ -1225,7 +1235,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>
>      kvm_cmd.extend(["-pidfile", pidfile])
>
> -    bus_slots = self._GetBusSlots()
> +    bus_slots = self._GetBusSlots(hvp)
>
>      # As requested by music lovers
>      if hvp[constants.HV_SOUNDHW]:
> @@ -2034,7 +2044,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>      if (int(v_major), int(v_min)) < (1, 7):
>        raise errors.HotplugError("Hotplug not supported for qemu versions
> < 1.7")
>
> -  def _GetBusSlots(self, runtime=None):
> +  def _GetBusSlots(self, hvp=None, runtime=None):
>      """Helper function to get the slots of PCI and SCSI QEMU buses.
>
>      This will return the status of the first PCI and SCSI buses. By
> default
> @@ -2059,6 +2069,14 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>        _SCSI_BUS: bitarray(self._DEFAULT_SCSI_RESERVATIONS),
>        }
>
> +    # Adjust the empty slots depending of the corresponding hvparam
> +    if hvp and constants.HV_KVM_PCI_RESERVATIONS in hvp:
> +      res = hvp[constants.HV_KVM_PCI_RESERVATIONS]
> +      pci = bitarray(constants.QEMU_PCI_SLOTS)
> +      pci.setall(False) # pylint: disable=E1101
> +      pci[0:res:1] = True
> +      bus_slots[_PCI_BUS] = pci
> +
>      # This is during hot-add
>      if runtime:
>        _, nics, _, disks = runtime
> @@ -2118,7 +2136,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>      runtime = self._LoadKVMRuntime(instance)
>      up_hvp = runtime[2]
>      device_type = _DEVICE_TYPE[dev_type](up_hvp)
> -    bus_state = self._GetBusSlots(runtime)
> +    bus_state = self._GetBusSlots(up_hvp, runtime)
>      # in case of hot-mod this is given
>      if not device.hvinfo:
>        device.hvinfo = _GenerateDeviceHVInfo(dev_type, kvm_devid,
> diff --git a/lib/hypervisor/hv_kvm/monitor.py
> b/lib/hypervisor/hv_kvm/monitor.py
> index 74317f6..658f20c 100644
> --- a/lib/hypervisor/hv_kvm/monitor.py
> +++ b/lib/hypervisor/hv_kvm/monitor.py
> @@ -48,6 +48,7 @@ from bitarray import bitarray
>
>  from ganeti import errors
>  from ganeti import utils
> +from ganeti import constants
>  from ganeti import serializer
>
>
> @@ -255,7 +256,6 @@ class QmpConnection(MonitorSocket):
>    _CAPABILITIES_COMMAND = "qmp_capabilities"
>    _QUERY_COMMANDS = "query-commands"
>    _MESSAGE_END_TOKEN = "\r\n"
> -  _QEMU_PCI_SLOTS = 32 # The number of PCI slots QEMU exposes by default
>    # List of valid attributes for the device_add QMP command.
>    # Extra attributes found in device's hvinfo will be ignored.
>    _DEVICE_ATTRIBUTES = [
> @@ -669,7 +669,7 @@ class QmpConnection(MonitorSocket):
>      """Get the first available PCI slot of a running instance.
>
>      """
> -    slots = bitarray(self._QEMU_PCI_SLOTS)
> +    slots = bitarray(constants.QEMU_PCI_SLOTS)
>      slots.setall(False) # pylint: disable=E1101
>      for d in self._GetPCIDevices():
>        slot = d["slot"]
> diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst
> index 03d8fc9..caad1d0 100644
> --- a/man/gnt-instance.rst
> +++ b/man/gnt-instance.rst
> @@ -343,6 +343,14 @@ scsi\_controller\_type
>      - megasas
>      - virtio-scsi-pci
>
> +kvm\_pci\_reservations
> +    Valid for the KVM hypervisor.
> +
> +    The nubmer of PCI slots that QEMU will manage implicitly. By default
> Ganeti
> +    will let QEMU use the first 12 slots (i.e. PCI slots 0-11) on its own
> and
> +    will start adding disks and NICs from the 13rd slot (i.e. PCI slot 12)
> +    onwards. So by default one can add 20 PCI devices (32 - 12). To
> support more
> +    than that, this hypervisor parameter should be set accordingly (e.g.
> to 8).
>
>  disk\_type
>      Valid for the Xen HVM and KVM hypervisors.
> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
> index 98d6805..28efc16 100644
> --- a/src/Ganeti/Constants.hs
> +++ b/src/Ganeti/Constants.hs
> @@ -1700,6 +1700,9 @@ hvKvmDiskAio = "disk_aio"
>  hvKvmScsiControllerType :: String
>  hvKvmScsiControllerType = "scsi_controller_type"
>
> +hvKvmPciReservations :: String
> +hvKvmPciReservations = "kvm_pci_reservations"
> +
>  hvKvmSpiceAudioCompr :: String
>  hvKvmSpiceAudioCompr = "spice_playback_compression"
>
> @@ -1907,6 +1910,7 @@ hvsParameterTypes = Map.fromList
>    , (hvKvmPath,                         VTypeString)
>    , (hvKvmDiskAio,                      VTypeString)
>    , (hvKvmScsiControllerType,           VTypeString)
> +  , (hvKvmPciReservations,              VTypeInt)
>    , (hvKvmSpiceAudioCompr,              VTypeBool)
>    , (hvKvmSpiceBind,                    VTypeString)
>    , (hvKvmSpiceIpVersion,               VTypeInt)
> @@ -2647,6 +2651,12 @@ vncBasePort = 5900
>  vncDefaultBindAddress :: String
>  vncDefaultBindAddress = ip4AddressAny
>
> +qemuPciSlots :: Int
> +qemuPciSlots = 32
> +
> +qemuDefaultPciReservations :: Int
> +qemuDefaultPciReservations = 12
> +
>  -- * NIC types
>
>  htNicE1000 :: String
> @@ -4058,6 +4068,7 @@ hvcDefaults =
>            , (hvVncX509Verify,                   PyValueEx False)
>            , (hvVncPasswordFile,                 PyValueEx "")
>            , (hvKvmScsiControllerType,           PyValueEx
> htScsiControllerLsi)
> +          , (hvKvmPciReservations,         PyValueEx
> qemuDefaultPciReservations)
>            , (hvKvmSpiceBind,                    PyValueEx "")
>            , (hvKvmSpiceIpVersion,           PyValueEx
> ifaceNoIpVersionSpecified)
>            , (hvKvmSpicePasswordFile,            PyValueEx "")
> diff --git a/test/py/ganeti.hypervisor.hv_kvm_unittest.py b/test/py/
> ganeti.hypervisor.hv_kvm_unittest.py
> index 5ea7ebb..16b1e0a 100755
> --- a/test/py/ganeti.hypervisor.hv_kvm_unittest.py
> +++ b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
> @@ -468,6 +468,7 @@ class TestGenerateDeviceKVMId(unittest.TestCase):
>
>  class TestGenerateDeviceHVInfo(testutils.GanetiTestCase):
>    def testPCI(self):
> +    """Test the placement of the first PCI device during startup."""
>      self.MockOut(mock.patch('ganeti.utils.EnsureDirs'))
>      hypervisor = hv_kvm.KVMHypervisor()
>      dev_type = constants.HOTPLUG_TARGET_NIC
> @@ -478,16 +479,18 @@ class
> TestGenerateDeviceHVInfo(testutils.GanetiTestCase):
>                                            kvm_devid,
>                                            hv_dev_type,
>                                            bus_slots)
> +    # NOTE: The PCI slot is zero-based, i.e. 13th slot has addr hex(12)
>      expected_hvinfo = {
>        "driver": "virtio-net-pci",
>        "id": kvm_devid,
>        "bus": "pci.0",
> -      "addr": hex(16),
> +      "addr": hex(constants.QEMU_DEFAULT_PCI_RESERVATIONS),
>        }
>
>      self.assertTrue(hvinfo == expected_hvinfo)
>
>    def testSCSI(self):
> +    """Test the placement of the first SCSI device during startup."""
>      self.MockOut(mock.patch('ganeti.utils.EnsureDirs'))
>      hypervisor = hv_kvm.KVMHypervisor()
>      dev_type = constants.HOTPLUG_TARGET_DISK
> --
> 1.7.10.4
>
>
Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.

This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.

Reply via email to