* Apollon Oikonomopoulos <[email protected]> [2015-10-02 14:01:04 +0300]:
> 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. > I prefer we do not add that many special cases to the code right now. A patch adding such functionallity can be added afterwards since this is orthogonal to the current patch. > > 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 > >
signature.asc
Description: Digital signature
