On 19:52 Thu 01 Oct , Dimitris Aragiorgis wrote: > In order to support more than 16 devices (disks and NICs) we > introduce kvm_pci_reservations hvparam that denotes the number of > PCI slots that QEMU will use implicitly. Ganeti will start adding > disks and NICs from the 17th (or whatever the next slot is) onwards. >
I have already expressed my concern for this approach, so I won't state it again here. Regardless of the approach, I feel that exposing this to the users via an HV param is not worth it and I wonder if it makes more sense to instead calculate a "safe" and documented limit based on the instance configuration: - If disk_type == scsi, then we only need 8 slots for up to MAX_NICS and a single slot for the SCSI controller. - If disk_type == ide, we only need 8 slots for up to MAX_NICS - If disk_type == paravirtual, we need 16 + 8 slots for MAX_DISKS + MAX_NICS IOW, if/when users eventually move to virtio-scsi, the need for reserved PCI slots will be greatly reduced. > Signed-off-by: Dimitris Aragiorgis <[email protected]> > --- > lib/hypervisor/hv_kvm/__init__.py | 29 > ++++++++++++++++++++------ > 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, 48 insertions(+), 9 deletions(-) > > diff --git a/lib/hypervisor/hv_kvm/__init__.py > b/lib/hypervisor/hv_kvm/__init__.py > index a78e103..12e24e9 100644 > --- a/lib/hypervisor/hv_kvm/__init__.py > +++ b/lib/hypervisor/hv_kvm/__init__.py > @@ -503,6 +503,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, > } > > @@ -553,9 +558,13 @@ class KVMHypervisor(hv_base.BaseHypervisor): > _INFO_VERSION_CMD = "info version" > > # Slot 0 for Host bridge, Slot 1 for ISA bridge, Slot 2 for VGA controller > - # and the rest up to slot 16 will be used by QEMU implicitly. > - # Ganeti will add disks and NICs from the 17th slot onwards. > - _DEFAULT_PCI_RESERVATIONS = "11111111111111110000000000000000" > + # and the rest up to slot 12 will be used by QEMU implicitly. > + # Ganeti will add disks and NICs from the 13th slot onwards. > + # 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. > + # NOTE: If this changes, 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) > # for each SCSI disk. > @@ -1206,7 +1215,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]: > @@ -2015,7 +2024,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 > @@ -2040,6 +2049,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 > @@ -2099,7 +2116,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..efb9f25 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 16 slots on its own and will > + start adding disks and NICs from the 17th slot onwards. If one wants > + to support more than 16 devices, this value 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 bec3177..056f1bb 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 >
