From cfca1546f65f9e8c4126171c7cf71083e008cdc7 Mon Sep 17 00:00:00 2001 From: Petr Pudlak Date: Mon, 24 Nov 2014 15:12:01 +0100 Subject: [PATCH 01/34] Update install docs - DRBD module parameters In the example ensure that the DRBD options are always passed to the module even if it's loaded before /etc/modules is processed. Signed-off-by: Petr Pudlak Reviewed-by: Hrvoje Ribicic --- doc/install.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/install.rst b/doc/install.rst index 107dbee4d6..cda719cb6e 100644 --- a/doc/install.rst +++ b/doc/install.rst @@ -286,9 +286,11 @@ instances on a node. Then to configure it for Ganeti:: - $ echo drbd minor_count=128 usermode_helper=/bin/true >> /etc/modules + $ echo "options drbd minor_count=128 usermode_helper=/bin/true" \ + >> /etc/modprobe.d/drbd.conf + $ echo "drbd" >> /etc/modules $ depmod -a - $ modprobe drbd minor_count=128 usermode_helper=/bin/true + $ modprobe drbd It is also recommended that you comment out the default resources (if any) in the ``/etc/drbd.conf`` file, so that the init script doesn't try to From 4da68dfe57593e7b6522629e4a2ff03797a381da Mon Sep 17 00:00:00 2001 From: Petr Pudlak Date: Fri, 28 Nov 2014 10:25:47 +0100 Subject: [PATCH 02/34] Fix the installation instructions for the DRBD module The options for the DRBD module should not be duplicated in the case the procedure is run multiple times. Signed-off-by: Petr Pudlak Reviewed-by: Hrvoje Ribicic --- doc/install.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/install.rst b/doc/install.rst index cda719cb6e..b828f993ba 100644 --- a/doc/install.rst +++ b/doc/install.rst @@ -287,7 +287,7 @@ instances on a node. Then to configure it for Ganeti:: $ echo "options drbd minor_count=128 usermode_helper=/bin/true" \ - >> /etc/modprobe.d/drbd.conf + > /etc/modprobe.d/drbd.conf $ echo "drbd" >> /etc/modules $ depmod -a $ modprobe drbd From f11d939684d2153ec5031707362bc3398d8cf5f8 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:10 +0200 Subject: [PATCH 03/34] kvm: Minor refactor of MonitorSocket Create socket upon connect(), keep the status of connection (self._connected = False) in close(), make existing connect() method protected and let connect() act as a reconnect method. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/monitor.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py index 0fc7fc1a37..3e2213581a 100644 --- a/lib/hypervisor/hv_kvm/monitor.py +++ b/lib/hypervisor/hv_kvm/monitor.py @@ -140,10 +140,6 @@ def __init__(self, monitor_filename): """ self.monitor_filename = monitor_filename - self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) - # We want to fail if the server doesn't send a complete message - # in a reasonable amount of time - self.sock.settimeout(self._SOCKET_TIMEOUT) self._connected = False def _check_socket(self): @@ -168,6 +164,13 @@ def _check_connection(self): " invoke connect() on it") def connect(self): + """Connect to the monitor socket if not already connected. + + """ + if not self._connected: + self._connect() + + def _connect(self): """Connects to the monitor. Connects to the UNIX socket @@ -182,6 +185,10 @@ def connect(self): # Check file existance/stuff try: + self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + # We want to fail if the server doesn't send a complete message + # in a reasonable amount of time + self.sock.settimeout(self._SOCKET_TIMEOUT) self.sock.connect(self.monitor_filename) except EnvironmentError: raise errors.HypervisorError("Can't connect to qmp socket") @@ -194,6 +201,7 @@ def close(self): """ self.sock.close() + self._connected = False def GetFd(self, fds, kvm_devid): """Pass file descriptor to kvm process via monitor socket using SCM_RIGHTS From a813462d79f36546f12e688cda306e4e1ca90da7 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:11 +0200 Subject: [PATCH 04/34] utils: Introduce GetFreeSlot() function Since this is a generic function that implements bitarray logic move it from kvm to utils so that it can be easily used across all modules. Make it raise errors.GenericError if it cannot find a free slot in the given bitarray. We add this function in a separate utils module (bitarrays) that can be extended in the future. Currently only the network module and the KVM hypervisor make use of bitarrays. This patch is a step forward unifying these pieces of code and make both use the same bitarray utility functions. Add unittest for bitarrays utils module. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- Makefile.am | 4 +- lib/hypervisor/hv_kvm/__init__.py | 50 +++------------ lib/utils/__init__.py | 1 + lib/utils/bitarrays.py | 73 ++++++++++++++++++++++ test/py/ganeti.utils.bitarrays_unittest.py | 67 ++++++++++++++++++++ 5 files changed, 151 insertions(+), 44 deletions(-) create mode 100644 lib/utils/bitarrays.py create mode 100755 test/py/ganeti.utils.bitarrays_unittest.py diff --git a/Makefile.am b/Makefile.am index c3da1058bb..3fa41d8f92 100644 --- a/Makefile.am +++ b/Makefile.am @@ -596,7 +596,8 @@ utils_PYTHON = \ lib/utils/text.py \ lib/utils/version.py \ lib/utils/wrapper.py \ - lib/utils/x509.py + lib/utils/x509.py \ + lib/utils/bitarrays.py docinput = \ doc/admin.rst \ @@ -1801,6 +1802,7 @@ python_tests = \ test/py/ganeti.utils.version_unittest.py \ test/py/ganeti.utils.wrapper_unittest.py \ test/py/ganeti.utils.x509_unittest.py \ + test/py/ganeti.utils.bitarrays_unittest.py \ test/py/ganeti.utils_unittest.py \ test/py/ganeti.vcluster_unittest.py \ test/py/ganeti.workerpool_unittest.py \ diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index 747faf1526..91fa15aa72 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -83,10 +83,6 @@ constants.HV_KVM_SPICE_USE_TLS, ]) -# Constant bitarray that reflects to a free pci slot -# Use it with bitarray.search() -_AVAILABLE_PCI_SLOT = bitarray("0") - # below constants show the format of runtime file # the nics are in second possition, while the disks in 4th (last) # moreover disk entries are stored as a list of in tuples @@ -186,38 +182,6 @@ def _GenerateDeviceKVMId(dev_type, dev): return "%s-%s-pci-%d" % (dev_type.lower(), dev.uuid.split("-")[0], dev.pci) -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 slot is not None: - assert slot < len(slots) - if slots[slot]: - raise errors.HypervisorError("Slots %d occupied" % slot) - - else: - avail = slots.search(_AVAILABLE_PCI_SLOT, 1) - if not avail: - raise errors.HypervisorError("All slots occupied") - - slot = int(avail[0]) - - if reserve: - slots[slot] = True - - return slot - - def _GetExistingDeviceInfo(dev_type, device, runtime): """Helper function to get an existing device inside the runtime file @@ -1123,15 +1087,15 @@ def _GenerateKVMRuntime(self, instance, block_devices, startup_paused, # while the Audio controller *must* be in slot 3. # That's why we bridge this option early in command line if soundhw in self._SOUNDHW_WITH_PCI_SLOT: - _ = _GetFreeSlot(pci_reservations, reserve=True) + _ = utils.GetFreeSlot(pci_reservations, reserve=True) kvm_cmd.extend(["-soundhw", soundhw]) if hvp[constants.HV_DISK_TYPE] == constants.HT_DISK_SCSI: # The SCSI controller requires another PCI slot. - _ = _GetFreeSlot(pci_reservations, reserve=True) + _ = utils.GetFreeSlot(pci_reservations, reserve=True) # Add id to ballon and place to the first available slot (3 or 4) - addr = _GetFreeSlot(pci_reservations, reserve=True) + addr = utils.GetFreeSlot(pci_reservations, reserve=True) pci_info = ",bus=pci.0,addr=%s" % hex(addr) kvm_cmd.extend(["-balloon", "virtio,id=balloon%s" % pci_info]) kvm_cmd.extend(["-daemonize"]) @@ -1369,7 +1333,7 @@ def _GenerateKVMRuntime(self, instance, block_devices, startup_paused, else: # Enable the spice agent communication channel between the host and the # agent. - addr = _GetFreeSlot(pci_reservations, reserve=True) + addr = utils.GetFreeSlot(pci_reservations, reserve=True) pci_info = ",bus=pci.0,addr=%s" % hex(addr) kvm_cmd.extend(["-device", "virtio-serial-pci,id=spice%s" % pci_info]) kvm_cmd.extend([ @@ -1420,12 +1384,12 @@ def _GenerateKVMRuntime(self, instance, block_devices, startup_paused, kvm_disks = [] for disk, link_name, uri in block_devices: - disk.pci = _GetFreeSlot(pci_reservations, disk.pci, True) + disk.pci = utils.GetFreeSlot(pci_reservations, disk.pci, True) kvm_disks.append((disk, link_name, uri)) kvm_nics = [] for nic in instance.nics: - nic.pci = _GetFreeSlot(pci_reservations, nic.pci, True) + nic.pci = utils.GetFreeSlot(pci_reservations, nic.pci, True) kvm_nics.append(nic) hvparams = hvp @@ -1857,7 +1821,7 @@ def _GetFreePCISlot(self, instance, dev): slot = int(match.group(1)) slots[slot] = True - dev.pci = _GetFreeSlot(slots) + dev.pci = utils.GetFreeSlot(slots) def VerifyHotplugSupport(self, instance, action, dev_type): """Verifies that hotplug is supported. diff --git a/lib/utils/__init__.py b/lib/utils/__init__.py index 17d517c560..7e07bcf075 100644 --- a/lib/utils/__init__.py +++ b/lib/utils/__init__.py @@ -69,6 +69,7 @@ from ganeti.utils.wrapper import * from ganeti.utils.version import * from ganeti.utils.x509 import * +from ganeti.utils.bitarrays import * _VALID_SERVICE_NAME_RE = re.compile("^[-_.a-zA-Z0-9]{1,128}$") diff --git a/lib/utils/bitarrays.py b/lib/utils/bitarrays.py new file mode 100644 index 0000000000..a635eef197 --- /dev/null +++ b/lib/utils/bitarrays.py @@ -0,0 +1,73 @@ +# +# + +# Copyright (C) 2014 Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS +# IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED +# TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR +# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +# EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR +# PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +# LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +# NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Utility functions for managing bitarrays. + +""" + + +from bitarray import bitarray + +from ganeti import errors + +# Constant bitarray that reflects to a free slot +# Use it with bitarray.search() +_AVAILABLE_SLOT = bitarray("0") + + +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.OpPrereqError: If all slots in a bitarray are occupied + or the given slot is not free. + + """ + if slot is not None: + assert slot < len(slots) + if slots[slot]: + raise errors.GenericError("Slot %d occupied" % slot) + + else: + avail = slots.search(_AVAILABLE_SLOT, 1) + if not avail: + raise errors.GenericError("All slots occupied") + + slot = int(avail[0]) + + if reserve: + slots[slot] = True + + return slot diff --git a/test/py/ganeti.utils.bitarrays_unittest.py b/test/py/ganeti.utils.bitarrays_unittest.py new file mode 100755 index 0000000000..b95d3111a0 --- /dev/null +++ b/test/py/ganeti.utils.bitarrays_unittest.py @@ -0,0 +1,67 @@ +#!/usr/bin/python +# + +# Copyright (C) 2014 Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS +# IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED +# TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR +# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +# EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR +# PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +# LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +# NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + + +"""Script for unittesting the bitarray utility functions""" + +import unittest +import testutils +from bitarray import bitarray + +from ganeti import errors +from ganeti.utils import bitarrays + + +_FREE = bitarray("11100010") +_FULL = bitarray("11111111") + + +class GetFreeSlotTest(unittest.TestCase): + """Test function that finds a free slot in a bitarray""" + + def testFreeSlot(self): + self.assertEquals(bitarrays.GetFreeSlot(_FREE), 3) + + def testReservedSlot(self): + self.assertRaises(errors.GenericError, + bitarrays.GetFreeSlot, + _FREE, slot=1) + + def testNoFreeSlot(self): + self.assertRaises(errors.GenericError, + bitarrays.GetFreeSlot, + _FULL) + + def testGetAndReserveSlot(self): + self.assertEquals(bitarrays.GetFreeSlot(_FREE, slot=5, reserve=True), 5) + self.assertEquals(_FREE, bitarray("11100110")) + + +if __name__ == "__main__": + testutils.GanetiTestProgram() From d6056fc7062984ae783891a31ef29942959ba479 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:12 +0200 Subject: [PATCH 05/34] qmp: Refactor of add-fd and remove-fd commands Instead of sending the qmp command along with the SCM_RIGHTS control message we send just a blank (idea taken from qemu iotests). Then we invoke the corresponding command (add-fd, remove-fd) with the regular way (qmp.Execute). Since the qmp connection does not close in between the fds will be added properly. AddFd() wrapper will be used for disk hot-add except for the case where we have userspace access mode. Make it raise HypervisorError if the command fails for some reason so that any hotplug action that takes place will abort. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/monitor.py | 52 +++++++++++++++----------------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py index 3e2213581a..fe35b2651c 100644 --- a/lib/hypervisor/hv_kvm/monitor.py +++ b/lib/hypervisor/hv_kvm/monitor.py @@ -432,51 +432,47 @@ def _GetResponse(self, command): return response[self._RETURN_KEY] - def AddFd(self, fds): - """Pass file descriptor to kvm process via qmp socket using SCM_RIGHTS + def AddFd(self, fd): + """Wrapper around add-fd qmp command - Add the fds to an fdset so that they can be used later by hot-add commands + Use fdsend to send fd to a running process via SCM_RIGHTS and then add-fd + qmp command to add it to an fdset so that it can be used later by + disk hotplugging. - @type fds: list - @param fds: The list of file descriptors to pass + @type fd: int + @param fd: The file descriptor to pass - @return: The fdset ID that the fds have been added to - (None if operation fails) + @return: The fdset ID that the fd has been added to + @raise errors.HypervisorError: If add-fd fails for some reason """ self._check_connection() - - if not fdsend or "add-fd" not in self.supported_commands: - return None - try: + fdsend.sendfds(self.sock, " ", fds=[fd]) # Omit fdset-id and let qemu create a new one (see qmp-commands.hx) - command = {"execute": "add-fd"} - fdsend.sendfds(self.sock, serializer.Dump(command), fds=fds) - # Get the response out of the buffer - response = self._GetResponse("add-fd") + response = self.Execute("add-fd") fdset = response["fdset-id"] - logging.info("Sent fds %s and added to fdset %s", fds, fdset) except errors.HypervisorError, err: - # In case _GetResponse() fails - fdset = None - logging.info("Sending fds %s failed: %s", fds, err) + logging.info("Passing fd %s via SCM_RIGHTS failed: %s", fd, err) + raise return fdset def RemoveFdset(self, fdset): - """Remove the file descriptor previously passed + """Wrapper around remove-fd qmp command - After qemu has dup'd the fd (e.g. during disk hotplug), - it can be safely removed. + Remove the file descriptor previously passed. After qemu has dup'd the fd + (e.g. during disk hotplug), it can be safely removed. """ self._check_connection() - # Omit the fd to cleanup all fds in the fdset (see qmp-commands.hx) - command = "remove-fd" - arguments = {"fdset-id": fdset} - logging.info("Removing fdset %s", fdset) + # Omit the fd to cleanup all fds in the fdset (see qemu/qmp-commands.hx) try: - self.Execute(command, arguments=arguments) + self.Execute("remove-fd", {"fdset-id": fdset}) except errors.HypervisorError, err: - logging.info("Removing %s fdset failed: %s", fdset, err) + # There is no big deal if we cannot remove an fdset. This cleanup here is + # done on a best effort basis. Upon next hot-add a new fdset will be + # created. If we raise an exception here, that is after drive_add has + # succeeded, the whole hot-add action will fail and the runtime file will + # not be updated which will make the instance non migrate-able + logging.info("Removing fdset with id %s failed: %s", fdset, err) From 6a4289167c8a4882a26f690e51b523f016a4803e Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:13 +0200 Subject: [PATCH 06/34] qmp: Add GetFd() wrapper around getfd command getfd qmp command will be used to pass an fd using SCM_RIGHTS and name it properly so that NIC hot-add can take place. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/monitor.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py index fe35b2651c..704cd2757a 100644 --- a/lib/hypervisor/hv_kvm/monitor.py +++ b/lib/hypervisor/hv_kvm/monitor.py @@ -432,6 +432,29 @@ def _GetResponse(self, command): return response[self._RETURN_KEY] + def GetFd(self, fd, fdname): + """Wrapper around the getfd qmp command + + Use fdsend to send an fd to a running process via SCM_RIGHTS and then use + the getfd qmp command to name it properly so that it can be used + later by NIC hotplugging. + + @type fd: int + @param fd: The file descriptor to pass + @raise errors.HypervisorError: If getfd fails for some reason + + """ + self._check_connection() + try: + fdsend.sendfds(self.sock, " ", fds=[fd]) + arguments = { + "fdname": fdname, + } + self.Execute("getfd", arguments) + except errors.HypervisorError, err: + logging.info("Passing fd %s via SCM_RIGHTS failed: %s", fd, err) + raise + def AddFd(self, fd): """Wrapper around add-fd qmp command From b8c9f8c18b954dd7fdc64b0655eda2831c27c26e Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:14 +0200 Subject: [PATCH 07/34] qmp: Add helper methods to verify hotplug support Introduce CheckDiskHotAddSupport() and CheckNicHotAddSupport() helper methods that will be used to verify hotplug support. Both check for fdsend python module and if the required qmp commands are supported. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/monitor.py | 40 ++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py index 704cd2757a..1a8a64d89a 100644 --- a/lib/hypervisor/hv_kvm/monitor.py +++ b/lib/hypervisor/hv_kvm/monitor.py @@ -432,6 +432,46 @@ def _GetResponse(self, command): return response[self._RETURN_KEY] + def CheckDiskHotAddSupport(self): + """Check if disk hotplug is possible + + Hotplug is *not* supported in case: + - fdsend module is missing + - add-fd and blockdev-add qmp commands are not supported + + """ + def _raise(reason): + raise errors.HotplugError("Cannot hot-add disk: %s." % reason) + + if not fdsend: + _raise("fdsend python module is missing") + + if "add-fd" not in self.supported_commands: + _raise("add-fd qmp command is not supported") + + if "blockdev-add" not in self.supported_commands: + _raise("blockdev-add qmp command is not supported") + + def CheckNicHotAddSupport(self): + """Check if NIC hotplug is possible + + Hotplug is *not* supported in case: + - fdsend module is missing + - getfd and netdev_add qmp commands are not supported + + """ + def _raise(reason): + raise errors.HotplugError("Cannot hot-add NIC: %s." % reason) + + if not fdsend: + _raise("fdsend python module is missing") + + if "getfd" not in self.supported_commands: + _raise("getfd qmp command is not supported") + + if "netdev_add" not in self.supported_commands: + _raise("netdev_add qmp command is not supported") + def GetFd(self, fd, fdname): """Wrapper around the getfd qmp command From 9abb1e38c9b53946a5e6555b954d9d43b29290b8 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:15 +0200 Subject: [PATCH 08/34] qmp: Helper methods for parsing query-pci output Introduce GetPCIDevices() that gets the devices of the first PCI bus of a running instance, HasPCIDevice() that will be used to verify if a specific device is correctly hotplugged or not, and GetFreePCISlot() that will be used during hot-add to find the first available PCI slot of a running instance. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/monitor.py | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py index 1a8a64d89a..9ff0018521 100644 --- a/lib/hypervisor/hv_kvm/monitor.py +++ b/lib/hypervisor/hv_kvm/monitor.py @@ -44,6 +44,8 @@ except ImportError: fdsend = None +from bitarray import bitarray + from ganeti import errors from ganeti import utils from ganeti import serializer @@ -233,6 +235,7 @@ 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 def __init__(self, monitor_filename): super(QmpConnection, self).__init__(monitor_filename) @@ -432,6 +435,41 @@ def _GetResponse(self, command): return response[self._RETURN_KEY] + def GetPCIDevices(self): + """Get the devices of the first PCI bus of a running instance. + + """ + self._check_connection() + pci = self.Execute("query-pci") + bus = pci[0] + devices = bus["devices"] + return devices + + def HasPCIDevice(self, device, devid): + """Check if a specific device exists or not on a running instance. + + It will match the PCI slot of the device and the id currently + obtained by _GenerateDeviceKVMId(). + + """ + for d in self.GetPCIDevices(): + if d["qdev_id"] == devid and d["slot"] == device.pci: + return True + + return False + + def GetFreePCISlot(self): + """Get the first available PCI slot of a running instance. + + """ + slots = bitarray(self._QEMU_PCI_SLOTS) + slots.setall(False) # pylint: disable=E1101 + for d in self.GetPCIDevices(): + slot = d["slot"] + slots[slot] = True + + return utils.GetFreeSlot(slots) + def CheckDiskHotAddSupport(self): """Check if disk hotplug is possible From d5dc810fa57cad1acd4818d1e0d2f8b487ecb45c Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:16 +0200 Subject: [PATCH 09/34] qmp: Add NIC hotplugging related methods HotAddNic() uses netdev_add and device_add after passing the tapfd to the running process with the GetFd() helper method. HotDelNic() uses device_del and netdev_del qmp commands. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/monitor.py | 35 ++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py index 9ff0018521..5d9b57b27c 100644 --- a/lib/hypervisor/hv_kvm/monitor.py +++ b/lib/hypervisor/hv_kvm/monitor.py @@ -435,6 +435,41 @@ def _GetResponse(self, command): return response[self._RETURN_KEY] + def HotAddNic(self, nic, tapfd, devid): + """Hot-add a NIC + + First pass the tapfd, then netdev_add and then device_add + + """ + self._check_connection() + + self.GetFd(tapfd, devid) + + arguments = { + "type": "tap", + "id": devid, + "fd": devid, + } + self.Execute("netdev_add", arguments) + + arguments = { + "driver": "virtio-net-pci", + "id": devid, + "bus": "pci.0", + "addr": hex(nic.pci), + "netdev": devid, + "mac": nic.mac, + } + self.Execute("device_add", arguments) + + def HotDelNic(self, devid): + """Hot-del a NIC + + """ + self._check_connection() + self.Execute("device_del", {"id": devid}) + self.Execute("netdev_del", {"id": devid}) + def GetPCIDevices(self): """Get the devices of the first PCI bus of a running instance. From 5a79d857e3c7baa489ecf2f2b9b1c646bd9f07e3 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:17 +0200 Subject: [PATCH 10/34] qmp: Add Disk hotplugging related methods HotAddDisk() uses blockdev-add and device_add after getting the drive fd and passing it with the AddFd() helper method. Please note that in case of userspace access mode this step will be omitted. HotDelDisk() uses device_del. Please note that drive_del is currently not supported in qmp and thus hmp should be used instead. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/monitor.py | 57 ++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py index 5d9b57b27c..b86788590d 100644 --- a/lib/hypervisor/hv_kvm/monitor.py +++ b/lib/hypervisor/hv_kvm/monitor.py @@ -470,6 +470,63 @@ def HotDelNic(self, devid): self.Execute("device_del", {"id": devid}) self.Execute("netdev_del", {"id": devid}) + def HotAddDisk(self, disk, devid, uri): + """Hot-add a disk + + Try opening the device to obtain a fd and pass it with SCM_RIGHTS. This + will be omitted in case of userspace access mode (open will fail). + Then use blockdev-add and then device_add. + + """ + self._check_connection() + + if os.path.exists(uri): + fd = os.open(uri, os.O_RDWR) + fdset = self.AddFd(fd) + os.close(fd) + filename = "/dev/fdset/%s" % fdset + else: + # The uri is not a file. + # This can happen if a userspace uri is provided. + filename = uri + fdset = None + + arguments = { + "options": { + "driver": "raw", + "id": devid, + "file": { + "driver": "file", + "filename": filename, + } + } + } + self.Execute("blockdev-add", arguments) + + if fdset is not None: + self.RemoveFdset(fdset) + + arguments = { + "driver": "virtio-blk-pci", + "id": devid, + "bus": "pci.0", + "addr": hex(disk.pci), + "drive": devid, + } + self.Execute("device_add", arguments) + + def HotDelDisk(self, devid): + """Hot-del a Disk + + Note that drive_del is not supported yet in qmp and thus should + be invoked from HMP. + + """ + self._check_connection() + self.Execute("device_del", {"id": devid}) + #TODO: uncomment when drive_del gets implemented in upstream qemu + # self.Execute("drive_del", {"id": devid}) + def GetPCIDevices(self): """Get the devices of the first PCI bus of a running instance. From c5297eceaa322341f41c863c0b5664da29f63fcf Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:18 +0200 Subject: [PATCH 11/34] kvm: New _with_qmp decorator The decorator takes the hypervisor and the instance object as passed to all hotplug related commands and ensures a valid QmpConnection() object is found in hypervisor's qmp attribute. This decorator is responsible for all actions related to the QmpConnection (e.g. init, connect, close) and for proper exception propagation. In order to be safe against nested invocations we close the connection only if we create it. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/__init__.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index 91fa15aa72..ff36f9efe4 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -112,6 +112,26 @@ _MIGRATION_CAPS_DELIM = ":" +def _with_qmp(fn): + """Wrapper used on hotplug related methods""" + def wrapper(self, instance, *args, **kwargs): + """Open a QmpConnection, run the wrapped method and then close it""" + conn_created = False + if not getattr(self, "qmp", None): + filename = self._InstanceQmpMonitor(instance.name)# pylint: disable=W0212 + self.qmp = QmpConnection(filename) + conn_created = True + + self.qmp.connect() + try: + ret = fn(self, instance, *args, **kwargs) + finally: + if conn_created = True: + self.qmp.close() + return ret + return wrapper + + def _GetDriveURI(disk, link, uri, qmp=None): """Helper function to get the drive uri to be used in --drive kvm option @@ -462,6 +482,7 @@ def __init__(self): # in a tmpfs filesystem or has been otherwise wiped out. dirs = [(dname, constants.RUN_DIRS_MODE) for dname in self._DIRS] utils.EnsureDirs(dirs) + self.qmp = None @staticmethod def VersionsSafeForMigration(src, target): From 16f4bee6b722918fdd7bf924b2a6e177137274ad Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:19 +0200 Subject: [PATCH 12/34] monitor: Close socket fd if already connected We close the socket fd only if we have an active connection. We hide this logic in the public close() method, and all the actions are moved to a private method. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/monitor.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py index b86788590d..461293e526 100644 --- a/lib/hypervisor/hv_kvm/monitor.py +++ b/lib/hypervisor/hv_kvm/monitor.py @@ -202,6 +202,10 @@ def close(self): It cannot be used after this call. """ + if self._connected: + self._close() + + def _close(self): self.sock.close() self._connected = False From 4c513aab72ea05ac64cc2f83311533356054b001 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:20 +0200 Subject: [PATCH 13/34] hotplug: Use QMP in VerifyHotplugSupport This used to check whether a specific hotplug action is supported. Since everything is about to be done via qmp commands delegate these checks to QmpConnection class. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/__init__.py | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index ff36f9efe4..9b1ddba38e 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -1844,36 +1844,19 @@ def _GetFreePCISlot(self, instance, dev): dev.pci = utils.GetFreeSlot(slots) + @_with_qmp def VerifyHotplugSupport(self, instance, action, dev_type): """Verifies that hotplug is supported. - Hotplug is *not* supported in case of: - - fdsend module is missing (hot-add) - - add-fd qmp command is not supported and - chroot or some security model is used - @raise errors.HypervisorError: in one of the previous cases """ - with QmpConnection(self._InstanceQmpMonitor(instance.name)) as qmp: - qmp_supports_add_fd = fdsend and "add-fd" in qmp.supported_commands - if dev_type == constants.HOTPLUG_TARGET_DISK: - hvp = instance.hvparams - security_model = hvp[constants.HV_SECURITY_MODEL] - use_chroot = hvp[constants.HV_KVM_USE_CHROOT] - if action == constants.HOTPLUG_ACTION_ADD and not qmp_supports_add_fd: - if use_chroot: - raise errors.HotplugError("Disk hotplug is not supported" - " in case of chroot.") - if security_model != constants.HT_SM_NONE: - raise errors.HotplugError("Disk Hotplug is not supported in case" - " security models are used.") - - if (dev_type == constants.HOTPLUG_TARGET_NIC and - action == constants.HOTPLUG_ACTION_ADD and not fdsend): - raise errors.HotplugError("Cannot hot-add NIC." - " fdsend python module is missing.") + if action == constants.HOTPLUG_ACTION_ADD: + self.qmp.CheckDiskHotAddSupport() + if dev_type == constants.HOTPLUG_TARGET_NIC: + if action == constants.HOTPLUG_ACTION_ADD: + self.qmp.CheckNicHotAddSupport() def HotplugSupported(self, instance): """Checks if hotplug is generally supported. From 154455532bea1353c4a0089dc3836aa602913114 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:21 +0200 Subject: [PATCH 14/34] hotplug: Use QMP in VerifyHotplugCommand This used to check whether a specific hotplug action has succeeded. Use HasPCIDevice() qmp method to verify if the requested device exists or not. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/__init__.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index 9b1ddba38e..214e70cd24 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -1888,25 +1888,24 @@ def _CallHotplugCommands(cls, instance_name, cmds): cls._CallMonitorCommand(instance_name, c) time.sleep(1) - def _VerifyHotplugCommand(self, instance_name, device, dev_type, - should_exist): + @_with_qmp + def _VerifyHotplugCommand(self, _instance, + device, kvm_devid, should_exist): """Checks if a previous hotplug command has succeeded. - It issues info pci monitor command and checks depending on should_exist - value if an entry with PCI slot and device ID is found or not. + Depending on the should_exist value, verifies that an entry identified by + the PCI slot and device ID is present or not. @raise errors.HypervisorError: if result is not the expected one """ - output = self._CallMonitorCommand(instance_name, self._INFO_PCI_CMD) - kvm_devid = _GenerateDeviceKVMId(dev_type, device) - match = \ - self._FIND_PCI_DEVICE_RE(device.pci, kvm_devid).search(output.stdout) - if match and not should_exist: + found = self.qmp.HasPCIDevice(device, kvm_devid) + + if found and not should_exist: msg = "Device %s should have been removed but is still there" % kvm_devid raise errors.HypervisorError(msg) - if not match and should_exist: + if not found and should_exist: msg = "Device %s should have been added but is missing" % kvm_devid raise errors.HypervisorError(msg) @@ -1962,7 +1961,7 @@ def HotAddDevice(self, instance, dev_type, device, extra, seq): utils.WriteFile(self._InstanceNICFile(instance.name, seq), data=tap) self._CallHotplugCommands(instance.name, cmds) - self._VerifyHotplugCommand(instance.name, device, dev_type, True) + self._VerifyHotplugCommand(instance, device, kvm_devid, True) # update relevant entries in runtime file index = _DEVICE_RUNTIME_INDEX[dev_type] entry = _RUNTIME_ENTRY[dev_type](device, extra) @@ -1988,7 +1987,7 @@ def HotDelDevice(self, instance, dev_type, device, _, seq): cmds += ["netdev_del %s" % kvm_devid] utils.RemoveFile(self._InstanceNICFile(instance.name, seq)) self._CallHotplugCommands(instance.name, cmds) - self._VerifyHotplugCommand(instance.name, kvm_device, dev_type, False) + self._VerifyHotplugCommand(instance, kvm_device, kvm_devid, False) index = _DEVICE_RUNTIME_INDEX[dev_type] runtime[index].remove(entry) self._SaveKVMRuntime(instance, runtime) From 79050c53e475cd0e7c00017fe01905630fa67e50 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:22 +0200 Subject: [PATCH 15/34] netdev: Refactor OpenTap for future use Make OpenTap() helper method to take a dictionary with the features that should be enabled or not. Currently these features are: vhost, vnet_hdr, multiqueue. In case vhost is enabled, along with the tapfds we return a list of vhostfds after opening /dev/vhost-net. This regularly is done by the qemu process if vhost=on is passed in --netdev option, but in case of hotplug and if the qemu process does not run with root privileges we must get the fds and pass them via SCM_RIGHTS prior qemu uses them. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/netdev.py | 35 ++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/lib/hypervisor/hv_kvm/netdev.py b/lib/hypervisor/hv_kvm/netdev.py index e35930589a..852d48f7cf 100644 --- a/lib/hypervisor/hv_kvm/netdev.py +++ b/lib/hypervisor/hv_kvm/netdev.py @@ -124,29 +124,31 @@ def _ProbeTapMqVirtioNet(fd, _features_fn=_GetTunFeatures): return result -def OpenTap(vnet_hdr=True, virtio_net_queues=1, name=""): +def OpenTap(name="", features=None): """Open a new tap device and return its file descriptor. This is intended to be used by a qemu-type hypervisor together with the -net tap,fd= or -net tap,fds=x:y:...:z command line parameter. - @type vnet_hdr: boolean - @param vnet_hdr: Enable the VNET Header - - @type virtio_net_queues: int - @param virtio_net_queues: Set number of tap queues but not more than 8, - queues only work with virtio-net device; - disabled by default (one queue). - @type name: string @param name: name for the TAP interface being created; if an empty string is passed, the OS will generate a unique name - @return: (ifname, [tapfds]) + @type features: dict + @param features: A dict denoting whether vhost, vnet_hdr, mq + netdev features are enabled or not. + + @return: (ifname, [tapfds], [vhostfds]) @rtype: tuple """ tapfds = [] + vhostfds = [] + if features is None: + features = {} + vhost = features.get("vhost", False) + vnet_hdr = features.get("vnet_hdr", True) + _, virtio_net_queues = features.get("mq", (False, 1)) for _ in range(virtio_net_queues): try: @@ -174,9 +176,20 @@ def OpenTap(vnet_hdr=True, virtio_net_queues=1, name=""): raise errors.HypervisorError("Failed to allocate a new TAP device: %s" % err) + if vhost: + # This is done regularly by the qemu process if vhost=on was passed with + # --netdev option. Still, in case of hotplug and if the process does not + # run with root privileges, we have to get the fds and pass them via + # SCM_RIGHTS prior to qemu using them. + try: + vhostfd = os.open("/dev/vhost-net", os.O_RDWR) + vhostfds.append(vhostfd) + except EnvironmentError: + raise errors.HypervisorError("Failed to open /dev/vhost-net") + tapfds.append(tapfd) # Get the interface name from the ioctl ifname = struct.unpack("16sh", res)[0].strip("\x00") - return (ifname, tapfds) + return (ifname, tapfds, vhostfds) From 9e26a355496e75ab657813929293669c208f9873 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:23 +0200 Subject: [PATCH 16/34] kvm: Refactor _GetNetworkDeviceFeatures() helper Make _GetNetworkDeviceFeatures() return a tuple of (dict, str, str). The first item should be a dictionary including the enabled netdev/device features (vhost, vnet_hdr, mq). The other two are strings to be appended to --netdev and --device qemu options. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/__init__.py | 43 +++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index 214e70cd24..23debeb681 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -1508,24 +1508,36 @@ def _GenerateKvmTapName(nic): def _GetNetworkDeviceFeatures(self, up_hvp, devlist, kvmhelp): """Get network device options to properly enable supported features. - Return tuple of supported and enabled tap features with nic_model. + Return a dict of supported and enabled tap features with nic_model along + with the extra strings to be appended to the --netdev and --device options. This function is called before opening a new tap device. - @return: (nic_model, vnet_hdr, virtio_net_queues, tap_extra, nic_extra) - @rtype: tuple + Currently the features_dict includes the following attributes: + - vhost (boolean) + - vnet_hdr (boolean) + - mq (boolean, int) + + @rtype: (dict, str, str) tuple + @return: The supported features, + the string to be appended to the --netdev option, + the string to be appended to the --device option """ - virtio_net_queues = 1 - nic_extra = "" nic_type = up_hvp[constants.HV_NIC_TYPE] - tap_extra = "" - vnet_hdr = False + nic_extra_str = "" + tap_extra_str = "" + features = { + "vhost": False, + "vnet_hdr": False, + "mq": (False, 1) + } + update_features = {} if nic_type == constants.HT_NIC_PARAVIRTUAL: nic_model = self._VIRTIO try: if self._VIRTIO_NET_RE.search(devlist): nic_model = self._VIRTIO_NET_PCI - vnet_hdr = up_hvp[constants.HV_VNET_HDR] + update_features["vnet_hdr"] = up_hvp[constants.HV_VNET_HDR] except errors.HypervisorError, _: # Older versions of kvm don't support DEVICE_LIST, but they don't # have new virtio syntax either. @@ -1534,25 +1546,30 @@ def _GetNetworkDeviceFeatures(self, up_hvp, devlist, kvmhelp): if up_hvp[constants.HV_VHOST_NET]: # Check for vhost_net support. if self._VHOST_RE.search(kvmhelp): - tap_extra = ",vhost=on" + update_features["vhost"] = True + tap_extra_str = ",vhost=on" else: raise errors.HypervisorError("vhost_net is configured" " but it is not available") - if up_hvp[constants.HV_VIRTIO_NET_QUEUES] > 1: + virtio_net_queues = up_hvp.get(constants.HV_VIRTIO_NET_QUEUES, 1) + if virtio_net_queues > 1: # Check for multiqueue virtio-net support. if self._VIRTIO_NET_QUEUES_RE.search(kvmhelp): - virtio_net_queues = up_hvp[constants.HV_VIRTIO_NET_QUEUES] # As advised at http://www.linux-kvm.org/page/Multiqueue formula # for calculating vector size is: vectors=2*N+1 where N is the # number of queues (HV_VIRTIO_NET_QUEUES). - nic_extra = ",mq=on,vectors=%d" % (2 * virtio_net_queues + 1) + nic_extra_str = ",mq=on,vectors=%d" % (2 * virtio_net_queues + 1) + update_features["mq"] = (True, virtio_net_queues) else: raise errors.HypervisorError("virtio_net_queues is configured" " but it is not available") else: nic_model = nic_type - return (nic_model, vnet_hdr, virtio_net_queues, tap_extra, nic_extra) + update_features["driver"] = nic_model + features.update(update_features) + + return features, tap_extra_str, nic_extra_str # too many local variables # pylint: disable=R0914 From 35c3b67b0b63445cc85862e18c7fec733b72bcae Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:24 +0200 Subject: [PATCH 17/34] monitor: Extend HotAddNic() for advanced features In case vhost is enabled we have to pass the vhostfds as returned by OpenTap() via SCM_RIGHTS prior issuing netdev_add command with vhost and vhostfds extra arguments. In case multiqueue is enabled we add the mq and vectors extra arguments to netdev_add command. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/monitor.py | 38 ++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py index 461293e526..9650c1aa82 100644 --- a/lib/hypervisor/hv_kvm/monitor.py +++ b/lib/hypervisor/hv_kvm/monitor.py @@ -439,21 +439,46 @@ def _GetResponse(self, command): return response[self._RETURN_KEY] - def HotAddNic(self, nic, tapfd, devid): + def HotAddNic(self, nic, devid, tapfds=None, vhostfds=None, features=None): """Hot-add a NIC - First pass the tapfd, then netdev_add and then device_add + First pass the tapfds, then netdev_add and then device_add """ self._check_connection() - self.GetFd(tapfd, devid) + if tapfds is None: + tapfds = [] + if vhostfds is None: + vhostfds = [] + if features is None: + features = {} + + enable_vhost = features.get("vhost", False) + enable_mq, virtio_net_queues = features.get("mq", (False, 1)) + + fdnames = [] + for i, fd in enumerate(tapfds): + fdname = "%s-%d" % (devid, i) + self.GetFd(fd, fdname) + fdnames.append(fdname) arguments = { "type": "tap", "id": devid, - "fd": devid, + "fds": ":".join(fdnames), } + if enable_vhost: + fdnames = [] + for i, fd in enumerate(vhostfds): + fdname = "%s-vhost-%d" % (devid, i) + self.GetFd(fd, fdname) + fdnames.append(fdname) + + arguments.update({ + "vhost": "on", + "vhostfds": ":".join(fdnames), + }) self.Execute("netdev_add", arguments) arguments = { @@ -464,6 +489,11 @@ def HotAddNic(self, nic, tapfd, devid): "netdev": devid, "mac": nic.mac, } + if enable_mq: + arguments.update({ + "mq": "on", + "vectors": (2 * virtio_net_queues + 1), + }) self.Execute("device_add", arguments) def HotDelNic(self, devid): From c776bef5ff253d84f9e2ff29e8f97f3c14ce3f97 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:25 +0200 Subject: [PATCH 18/34] kvm: Use the new interface during instance start Use new OpenTap() and _GetNetworkDeviceFeatures() interfaces during _ExecuteKVMRuntime(). Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/__init__.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index 23debeb681..a87dbe79a2 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -1629,14 +1629,13 @@ def _ExecuteKVMRuntime(self, instance, kvm_runtime, kvmhelp, incoming=None): if not kvm_nics: kvm_cmd.extend(["-net", "none"]) else: - (nic_model, vnet_hdr, - virtio_net_queues, tap_extra, - nic_extra) = self._GetNetworkDeviceFeatures(up_hvp, devlist, kvmhelp) + features, tap_extra, nic_extra = \ + self._GetNetworkDeviceFeatures(up_hvp, devlist, kvmhelp) + nic_model = features["driver"] kvm_supports_netdev = self._NETDEV_RE.search(kvmhelp) for nic_seq, nic in enumerate(kvm_nics): - tapname, nic_tapfds = OpenTap(vnet_hdr=vnet_hdr, - virtio_net_queues=virtio_net_queues, - name=self._GenerateKvmTapName(nic)) + tapname, nic_tapfds, _ = OpenTap(features=features, + name=self._GenerateKvmTapName(nic)) tapfds.extend(nic_tapfds) taps.append(tapname) tapfd = "%s%s" % ("fds=" if len(nic_tapfds) > 1 else "fd=", From a80a860577dd9f14115869977ffb01802e8f426b Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:26 +0200 Subject: [PATCH 19/34] hotplug: Use QMP during HotAddDevice During device hot-add use new QMP helper methods: GetFreePCISlot() for allocating a free PCI slot, HotAddDisk() for disk hotplug and HotAddNic() for NIC hotplug. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/__init__.py | 37 ++++++------------------------- 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index a87dbe79a2..3358e558b2 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -1927,6 +1927,7 @@ def _VerifyHotplugCommand(self, _instance, logging.info("Device %s has been correctly hot-plugged", kvm_devid) + @_with_qmp def HotAddDevice(self, instance, dev_type, device, extra, seq): """ Helper method to hot-add a new device @@ -1936,46 +1937,22 @@ def HotAddDevice(self, instance, dev_type, device, extra, seq): """ # in case of hot-mod this is given if device.pci is None: - self._GetFreePCISlot(instance, device) + device.pci = self.qmp.GetFreePCISlot() kvm_devid = _GenerateDeviceKVMId(dev_type, device) runtime = self._LoadKVMRuntime(instance) - fdset = None if dev_type == constants.HOTPLUG_TARGET_DISK: - # Create a shared qmp connection because - # fdsets get cleaned up on monitor disconnect - # See qemu commit efb87c1 - qmp = QmpConnection(self._InstanceQmpMonitor(instance.name)) - qmp.connect() - drive_uri, fdset = _GetDriveURI(device, extra[0], extra[1], qmp) - cmds = ["drive_add dummy file=%s,if=none,id=%s,format=raw" % - (drive_uri, kvm_devid)] - cmds += ["device_add virtio-blk-pci,bus=pci.0,addr=%s,drive=%s,id=%s" % - (hex(device.pci), kvm_devid, kvm_devid)] - self._CallHotplugCommands(instance.name, cmds) - if fdset is not None: - qmp.RemoveFdset(fdset) - qmp.close() + uri = _GetDriveURI(device, extra[0], extra[1]) + self.qmp.HotAddDisk(device, kvm_devid, uri) elif dev_type == constants.HOTPLUG_TARGET_NIC: kvmpath = instance.hvparams[constants.HV_KVM_PATH] kvmhelp = self._GetKVMOutput(kvmpath, self._KVMOPT_HELP) devlist = self._GetKVMOutput(kvmpath, self._KVMOPT_DEVICELIST) up_hvp = runtime[2] - (_, vnet_hdr, - virtio_net_queues, tap_extra, - nic_extra) = self._GetNetworkDeviceFeatures(up_hvp, devlist, kvmhelp) - (tap, fds) = OpenTap(vnet_hdr=vnet_hdr, - virtio_net_queues=virtio_net_queues) - # netdev_add don't support "fds=" when multiple fds are - # requested, generate separate "fd=" string for every fd - tapfd = ",".join(["fd=%s" % fd for fd in fds]) + features, _, _ = self._GetNetworkDeviceFeatures(up_hvp, devlist, kvmhelp) + (tap, tapfds, vhostfds) = OpenTap(features=features) self._ConfigureNIC(instance, seq, device, tap) - self._HMPPassFd(instance.name, fds, kvm_devid) - cmds = ["netdev_add tap,id=%s,%s%s" % (kvm_devid, tapfd, tap_extra)] - args = "virtio-net-pci,bus=pci.0,addr=%s,mac=%s,netdev=%s,id=%s%s" % \ - (hex(device.pci), device.mac, kvm_devid, kvm_devid, nic_extra) - cmds += ["device_add %s" % args] + self.qmp.HotAddNic(device, kvm_devid, tapfds, vhostfds, features) utils.WriteFile(self._InstanceNICFile(instance.name, seq), data=tap) - self._CallHotplugCommands(instance.name, cmds) self._VerifyHotplugCommand(instance, device, kvm_devid, True) # update relevant entries in runtime file From 1fb45c21ba66bfd1d1ce2b654937213723f542fe Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:27 +0200 Subject: [PATCH 20/34] hotplug: Use QMP during HotDelDevice During device hot-del use new QMP helper methods: HotDelDisk() for disk hotplug and HotDelNic() for NIC hotplug. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/__init__.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index 3358e558b2..e737788666 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -1961,6 +1961,7 @@ def HotAddDevice(self, instance, dev_type, device, extra, seq): runtime[index].append(entry) self._SaveKVMRuntime(instance, runtime) + @_with_qmp def HotDelDevice(self, instance, dev_type, device, _, seq): """ Helper method for hot-del device @@ -1973,13 +1974,13 @@ def HotDelDevice(self, instance, dev_type, device, _, seq): kvm_device = _RUNTIME_DEVICE[dev_type](entry) kvm_devid = _GenerateDeviceKVMId(dev_type, kvm_device) if dev_type == constants.HOTPLUG_TARGET_DISK: - cmds = ["device_del %s" % kvm_devid] - cmds += ["drive_del %s" % kvm_devid] + self.qmp.HotDelDisk(kvm_devid) + # drive_del is not implemented yet in qmp + command = "drive_del %s\n" % kvm_devid + self._CallMonitorCommand(instance.name, command) elif dev_type == constants.HOTPLUG_TARGET_NIC: - cmds = ["device_del %s" % kvm_devid] - cmds += ["netdev_del %s" % kvm_devid] + self.qmp.HotDelNic(kvm_devid) utils.RemoveFile(self._InstanceNICFile(instance.name, seq)) - self._CallHotplugCommands(instance.name, cmds) self._VerifyHotplugCommand(instance, kvm_device, kvm_devid, False) index = _DEVICE_RUNTIME_INDEX[dev_type] runtime[index].remove(entry) From 5c501ba18a27c5a94e4ad58e5122a10294cec045 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:28 +0200 Subject: [PATCH 21/34] hotplug: Remove unused code after refacoring All HMP related code gets removed since everything is done via QMP. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/__init__.py | 75 +++---------------------------- lib/hypervisor/hv_kvm/monitor.py | 10 ----- 2 files changed, 7 insertions(+), 78 deletions(-) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index e737788666..855d6a50bc 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -132,12 +132,12 @@ def wrapper(self, instance, *args, **kwargs): return wrapper -def _GetDriveURI(disk, link, uri, qmp=None): +def _GetDriveURI(disk, link, uri): """Helper function to get the drive uri to be used in --drive kvm option - Invoked during startup and hot-add. In latter case if kernelspace is used - we get the fd of the disk, pass it to the instance via SCM rights and qmp - monitor, and return the proper path (/dev/fdset/) + Invoked during startup and disk hot-add. In latter case and if no userspace + access mode is used it will be overriden with /dev/fdset/ (see + HotAddDisk() and AddFd() of QmpConnection). @type disk: L{objects.Disk} @param disk: A disk configuration object @@ -145,39 +145,20 @@ def _GetDriveURI(disk, link, uri, qmp=None): @param link: The device link as returned by _SymlinkBlockDev() @type uri: string @param uri: The drive uri as returned by _CalculateDeviceURI() - @type qmp: L{QmpConnection} - @param qmp: The qmp connection used to pass the drive's fd - @return: (the drive uri to use, the corresponing fdset if any) tuple + @return: The drive uri to use in kvm option """ - fdset = None access_mode = disk.params.get(constants.LDP_ACCESS, constants.DISK_KERNELSPACE) # If uri is available, use it during startup/hot-add if (uri and access_mode == constants.DISK_USERSPACE): drive_uri = uri - # During hot-add get the disk's fd and pass it to qemu via SCM rights - elif qmp: - try: - fd = os.open(link, os.O_RDWR) - fdset = qmp.AddFd([fd]) - os.close(fd) - except OSError: - logging.warning("Cannot open disk with link %s in order to" - " pass its fd to qmp monitor", link) - fdset = None - - # fd passing succeeded - if fdset is not None: - drive_uri = "/dev/fdset/%s" % fdset - else: - drive_uri = link # Otherwise use the link previously created else: drive_uri = link - return drive_uri, fdset + return drive_uri def _GenerateDeviceKVMId(dev_type, dev): @@ -441,13 +422,6 @@ class KVMHypervisor(hv_base.BaseHypervisor): _BOOT_RE = re.compile(r"^-drive\s([^-]|(? Date: Thu, 27 Nov 2014 18:00:29 +0200 Subject: [PATCH 22/34] hotplug: Is not supported for QEMU < 1.7 Change the bulk check of whether hotplug is supported or not. Only versions >= 1.7 support the required qmp commands. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index 855d6a50bc..8df13e084d 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -1837,7 +1837,7 @@ def HotplugSupported(self, instance): """Checks if hotplug is generally supported. Hotplug is *not* supported in case of: - - qemu versions < 1.0 + - qemu versions < 1.7 (where all qmp related commands are supported) - for stopped instances @raise errors.HypervisorError: in one of the previous cases @@ -1848,14 +1848,14 @@ def HotplugSupported(self, instance): except errors.HypervisorError: raise errors.HotplugError("Instance is probably down") - # TODO: search for netdev_add, drive_add, device_add..... match = self._INFO_VERSION_RE.search(output.stdout) if not match: raise errors.HotplugError("Cannot parse qemu version via monitor") + #TODO: delegate more fine-grained checks to VerifyHotplugSupport v_major, v_min, _, _ = match.groups() - if (int(v_major), int(v_min)) < (1, 0): - raise errors.HotplugError("Hotplug not supported for qemu versions < 1.0") + if (int(v_major), int(v_min)) < (1, 7): + raise errors.HotplugError("Hotplug not supported for qemu versions < 1.7") @_with_qmp def _VerifyHotplugCommand(self, _instance, From 0e22b420ccf588386d9ff571621f76989ba90cbf Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:30 +0200 Subject: [PATCH 23/34] qmp: Log qmp commands and responses Add debug logging for all qmp commands and responses, except for query-commands, and qmp_capabilities, that are executed upon initialization of QmpConnection (and their output is not really useful). Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/monitor.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py index 69bcd9559e..d884096d70 100644 --- a/lib/hypervisor/hv_kvm/monitor.py +++ b/lib/hypervisor/hv_kvm/monitor.py @@ -398,7 +398,11 @@ def Execute(self, command, arguments=None): message[self._ARGUMENTS_KEY] = arguments self._Send(message) - return self._GetResponse(command) + ret = self._GetResponse(command) + # log important qmp commands.. + if command not in [self._QUERY_COMMANDS, self._CAPABILITIES_COMMAND]: + logging.debug("QMP %s %s: %s\n", command, arguments, ret) + return ret def _GetResponse(self, command): """Parse the QMP response From e63f1e7430964663231ba13526551fd061fd6e9c Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:31 +0200 Subject: [PATCH 24/34] qa: Re-enable NIC hotplug tests This was disabled due to Issue 885. This issue states that NIC hotplugging sometimes makes an instance unresponsive. This cannot be the case, since NIC hotplug takes place on an extra NIC and does not mess up with instance's primary NIC. Still this sometimes might be related with --online fix (see commit b0a383a). Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- qa/qa_instance.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/qa/qa_instance.py b/qa/qa_instance.py index cecec18f60..757b372275 100644 --- a/qa/qa_instance.py +++ b/qa/qa_instance.py @@ -633,12 +633,11 @@ def TestInstanceModify(instance): ]) elif default_hv == constants.HT_KVM and \ qa_config.TestEnabled("instance-device-hotplug"): - # FIXME: Fix issue 885 and then re-enable the tests below - #args.extend([ - # ["--net", "-1:add", "--hotplug"], - # ["--net", "-1:modify,mac=aa:bb:cc:dd:ee:ff", "--hotplug", "--force"], - # ["--net", "-1:remove", "--hotplug"], - # ]) + args.extend([ + ["--net", "-1:add", "--hotplug"], + ["--net", "-1:modify,mac=aa:bb:cc:dd:ee:ff", "--hotplug", "--force"], + ["--net", "-1:remove", "--hotplug"], + ]) args.extend([ ["--disk", "-1:add,size=1G", "--hotplug"], ["--disk", "-1:remove", "--hotplug"], From 88bef78240d2b181cdbb48b2ffe9d4cf8054918e Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:32 +0200 Subject: [PATCH 25/34] Update hotplug design doc ..to reflect the transition from HMP to QMP during hotplug actions. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- doc/design-hotplug.rst | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/doc/design-hotplug.rst b/doc/design-hotplug.rst index 45f0862654..a15381d8dc 100644 --- a/doc/design-hotplug.rst +++ b/doc/design-hotplug.rst @@ -54,7 +54,7 @@ To keep track where each device is plugged into, we add the files, since it is hypervisor specific info. This is added for easy object manipulation and is ensured not to be written back to the config. -We propose to make use of QEMU 1.0 monitor commands so that +We propose to make use of QEMU 1.7 QMP commands so that modifications to devices take effect instantly without the need for hard reboot. The only change exposed to the end-user will be the addition of a ``--hotplug`` option to the ``gnt-instance modify`` command. @@ -81,7 +81,7 @@ Who decides where to hotplug each device? As long as this is a hypervisor specific matter, there is no point for the master node to decide such a thing. Master node just has to request noded to hotplug a device. To this end, hypervisor specific code should parse the current -PCI configuration (i.e. ``info pci`` QEMU monitor command), find the first +PCI configuration (i.e. ``query-pci`` QMP command), find the first available slot and hotplug the device. Having noded to decide where to hotplug a device we ensure that no error will occur due to duplicate slot assignment (if masterd keeps track of PCI reservations and noded @@ -149,18 +149,19 @@ Hypervisor changes ------------------ We implement hotplug on top of the KVM hypervisor. We take advantage of -QEMU 1.0 monitor commands (``device_add``, ``device_del``, -``drive_add``, ``drive_del``, ``netdev_add``,`` netdev_del``). QEMU +QEMU 1.7 QMP commands (``device_add``, ``device_del``, +``blockdev-add``, ``netdev_add``, ``netdev_del``). Since ``drive_del`` +is not yet implemented in QMP we use the one of HMP. QEMU refers to devices based on their id. We use ``uuid`` to name them properly. If a device is about to be hotplugged we parse the output of -``info pci`` and find the occupied PCI slots. We choose the first +``query-pci`` and find the occupied PCI slots. We choose the first available and the whole device object is appended to the corresponding entry in the runtime file. Concerning NIC handling, we build on the top of the existing logic (first create a tap with _OpenTap() and then pass its file descriptor to the KVM process). To this end we need to pass access rights to the -corresponding file descriptor over the monitor socket (UNIX domain +corresponding file descriptor over the QMP socket (UNIX domain socket). The open file is passed as a socket-level control message (SCM), using the ``fdsend`` python library. @@ -220,8 +221,8 @@ support only disk addition/deletion. gnt-instance modify --net 1:remove --hotplug test -Dealing with chroot and uid pool --------------------------------- +Dealing with chroot and uid pool (and disks in general) +------------------------------------------------------- The design so far covers all issues that arise without addressing the case where the kvm process will not run with root privileges. @@ -232,18 +233,18 @@ Specifically: - in case of uid pool security model, the kvm process is not allowed to access the device -For NIC hotplug we address this problem by using the ``getfd`` monitor +For NIC hotplug we address this problem by using the ``getfd`` QMP command and passing the file descriptor to the kvm process over the monitor socket using SCM_RIGHTS. For disk hotplug and in case of uid pool we can let the hypervisor code temporarily ``chown()`` the device before the actual hotplug. Still this is insufficient in case of chroot. In this case, we need to ``mknod()`` the device inside the chroot. Both -workarounds can be avoided, if we make use of the ``add-fd`` qemu -monitor command, that was introduced in version 1.7. This command is the +workarounds can be avoided, if we make use of the ``add-fd`` +QMP command, that was introduced in version 1.7. This command is the equivalent of NICs' `get-fd`` for disks and will allow disk hotplug in -every case. So, if the qemu monitor does not support the ``add-fd`` -command, we will not allow disk hotplug for chroot and uid security -model and notify the user with the corresponding warning. +every case. So, if the QMP does not support the ``add-fd`` +command, we will not allow disk hotplug +and notify the user with the corresponding warning. .. vim: set textwidth=72 : .. Local Variables: From b4f2f739457afb517400be655d507bcde9b5808f Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:33 +0200 Subject: [PATCH 26/34] hotplug: Retry VerifyHotplugCommand up to 5 times It seems that QMP upon device_del returns without QEMU having completely removed the device from the PCI bus. This probably has to do PCI bus hotplug handlers (DeviceState->BusState->hotplug_handler). So we invoke HasPCIDevice up to 5 times with 1 sec sleep in between to get the proper response. Tests point out that usually only one retry is needed. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/__init__.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index 8df13e084d..424365ba07 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -1868,7 +1868,14 @@ def _VerifyHotplugCommand(self, _instance, @raise errors.HypervisorError: if result is not the expected one """ - found = self.qmp.HasPCIDevice(device, kvm_devid) + for i in range(5): + found = self.qmp.HasPCIDevice(device, kvm_devid) + logging.info("Verifying hotplug command (retry %s): %s", i, found) + if found and should_exist: + break + if not found and not should_exist: + break + time.sleep(1) if found and not should_exist: msg = "Device %s should have been removed but is still there" % kvm_devid From cb5e23ec030acbfcbb4db2b8fd1de20247f70b25 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:34 +0200 Subject: [PATCH 27/34] kvm: Use vhostfds obtained by OpenTap During instance startup, if vhost_net is True, OpenTap() opens /dev/vhost-net device to obtain some fds. These fds were never used. This patch adds the vhostsfd option to the --netdev option. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/__init__.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index 424365ba07..0db226f356 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -1608,12 +1608,22 @@ def _ExecuteKVMRuntime(self, instance, kvm_runtime, kvmhelp, incoming=None): nic_model = features["driver"] kvm_supports_netdev = self._NETDEV_RE.search(kvmhelp) for nic_seq, nic in enumerate(kvm_nics): - tapname, nic_tapfds, _ = OpenTap(features=features, - name=self._GenerateKvmTapName(nic)) + tapname, nic_tapfds, nic_vhostfds = \ + OpenTap(features=features, name=self._GenerateKvmTapName(nic)) + tapfds.extend(nic_tapfds) + tapfds.extend(nic_vhostfds) taps.append(tapname) tapfd = "%s%s" % ("fds=" if len(nic_tapfds) > 1 else "fd=", ":".join(str(fd) for fd in nic_tapfds)) + + if nic_vhostfds: + vhostfd = "%s%s" % (",vhostfds=" + if len(nic_vhostfds) > 1 else ",vhostfd=", + ":".join(str(fd) for fd in nic_vhostfds)) + else: + vhostfd = "" + if kvm_supports_netdev: nic_val = "%s,mac=%s" % (nic_model, nic.mac) try: @@ -1625,8 +1635,8 @@ def _ExecuteKVMRuntime(self, instance, kvm_runtime, kvmhelp, incoming=None): except errors.HotplugError: netdev = "netdev%d" % nic_seq nic_val += (",netdev=%s%s" % (netdev, nic_extra)) - tap_val = ("type=tap,id=%s,%s%s" % - (netdev, tapfd, tap_extra)) + tap_val = ("type=tap,id=%s,%s%s%s" % + (netdev, tapfd, vhostfd, tap_extra)) kvm_cmd.extend(["-netdev", tap_val, "-device", nic_val]) else: nic_val = "nic,vlan=%s,macaddr=%s,model=%s" % (nic_seq, From 1e25ff0a5b5f4f710ac6df8fa39bd31156409926 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Thu, 27 Nov 2014 18:00:35 +0200 Subject: [PATCH 28/34] kvm: Delegate socket handling to monitor module Introduce @_ensure_connection decorator that properly handles the socket connection (i.e. connect, close) and propagates any exceptions raised in the decorated method. In general @_ensure_connection wraps external methods. Here we close the connection only if we initiated it before, to protect us from using the socket after closing it in case we invoke a decorated method internally by accident. Rename all private methods of QmpConnection properly. Signed-off-by: Dimitris Aragiorgis Reviewed-by: Hrvoje Ribicic --- lib/hypervisor/hv_kvm/__init__.py | 13 ++----- lib/hypervisor/hv_kvm/monitor.py | 60 ++++++++++++++++++++++--------- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index 0db226f356..53c473d280 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -115,20 +115,11 @@ def _with_qmp(fn): """Wrapper used on hotplug related methods""" def wrapper(self, instance, *args, **kwargs): - """Open a QmpConnection, run the wrapped method and then close it""" - conn_created = False + """Create a QmpConnection and run the wrapped method""" if not getattr(self, "qmp", None): filename = self._InstanceQmpMonitor(instance.name)# pylint: disable=W0212 self.qmp = QmpConnection(filename) - conn_created = True - - self.qmp.connect() - try: - ret = fn(self, instance, *args, **kwargs) - finally: - if conn_created = True: - self.qmp.close() - return ret + return fn(self, instance, *args, **kwargs) return wrapper diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py index d884096d70..f2b7d025eb 100644 --- a/lib/hypervisor/hv_kvm/monitor.py +++ b/lib/hypervisor/hv_kvm/monitor.py @@ -172,6 +172,12 @@ def connect(self): if not self._connected: self._connect() + def is_connected(self): + """Return whether there is a connection to the socket or not. + + """ + return self._connected + def _connect(self): """Connects to the monitor. @@ -210,6 +216,26 @@ def _close(self): self._connected = False +def _ensure_connection(fn): + """Decorator that wraps MonitorSocket external methods""" + def wrapper(*args, **kwargs): + """Ensure proper connect/close and exception propagation""" + mon = args[0] + already_connected = mon.is_connected() + mon.connect() + try: + ret = fn(*args, **kwargs) + finally: + # In general this decorator wraps external methods. + # Here we close the connection only if we initiated it before, + # to protect us from using the socket after closing it + # in case we invoke a decorated method internally by accident. + if not already_connected: + mon.close() + return ret + return wrapper + + class QmpConnection(MonitorSocket): """Connection to the QEMU Monitor using the QEMU Monitor Protocol (QMP). @@ -433,14 +459,13 @@ def _GetResponse(self, command): return response[self._RETURN_KEY] + @_ensure_connection def HotAddNic(self, nic, devid, tapfds=None, vhostfds=None, features=None): """Hot-add a NIC First pass the tapfds, then netdev_add and then device_add """ - self._check_connection() - if tapfds is None: tapfds = [] if vhostfds is None: @@ -454,7 +479,7 @@ def HotAddNic(self, nic, devid, tapfds=None, vhostfds=None, features=None): fdnames = [] for i, fd in enumerate(tapfds): fdname = "%s-%d" % (devid, i) - self.GetFd(fd, fdname) + self._GetFd(fd, fdname) fdnames.append(fdname) arguments = { @@ -466,7 +491,7 @@ def HotAddNic(self, nic, devid, tapfds=None, vhostfds=None, features=None): fdnames = [] for i, fd in enumerate(vhostfds): fdname = "%s-vhost-%d" % (devid, i) - self.GetFd(fd, fdname) + self._GetFd(fd, fdname) fdnames.append(fdname) arguments.update({ @@ -490,14 +515,15 @@ def HotAddNic(self, nic, devid, tapfds=None, vhostfds=None, features=None): }) self.Execute("device_add", arguments) + @_ensure_connection def HotDelNic(self, devid): """Hot-del a NIC """ - self._check_connection() self.Execute("device_del", {"id": devid}) self.Execute("netdev_del", {"id": devid}) + @_ensure_connection def HotAddDisk(self, disk, devid, uri): """Hot-add a disk @@ -506,11 +532,9 @@ def HotAddDisk(self, disk, devid, uri): Then use blockdev-add and then device_add. """ - self._check_connection() - if os.path.exists(uri): fd = os.open(uri, os.O_RDWR) - fdset = self.AddFd(fd) + fdset = self._AddFd(fd) os.close(fd) filename = "/dev/fdset/%s" % fdset else: @@ -532,7 +556,7 @@ def HotAddDisk(self, disk, devid, uri): self.Execute("blockdev-add", arguments) if fdset is not None: - self.RemoveFdset(fdset) + self._RemoveFdset(fdset) arguments = { "driver": "virtio-blk-pci", @@ -543,6 +567,7 @@ def HotAddDisk(self, disk, devid, uri): } self.Execute("device_add", arguments) + @_ensure_connection def HotDelDisk(self, devid): """Hot-del a Disk @@ -550,12 +575,11 @@ def HotDelDisk(self, devid): be invoked from HMP. """ - self._check_connection() self.Execute("device_del", {"id": devid}) #TODO: uncomment when drive_del gets implemented in upstream qemu # self.Execute("drive_del", {"id": devid}) - def GetPCIDevices(self): + def _GetPCIDevices(self): """Get the devices of the first PCI bus of a running instance. """ @@ -565,6 +589,7 @@ def GetPCIDevices(self): devices = bus["devices"] return devices + @_ensure_connection def HasPCIDevice(self, device, devid): """Check if a specific device exists or not on a running instance. @@ -572,24 +597,26 @@ def HasPCIDevice(self, device, devid): obtained by _GenerateDeviceKVMId(). """ - for d in self.GetPCIDevices(): + for d in self._GetPCIDevices(): if d["qdev_id"] == devid and d["slot"] == device.pci: return True return False + @_ensure_connection def GetFreePCISlot(self): """Get the first available PCI slot of a running instance. """ slots = bitarray(self._QEMU_PCI_SLOTS) slots.setall(False) # pylint: disable=E1101 - for d in self.GetPCIDevices(): + for d in self._GetPCIDevices(): slot = d["slot"] slots[slot] = True return utils.GetFreeSlot(slots) + @_ensure_connection def CheckDiskHotAddSupport(self): """Check if disk hotplug is possible @@ -610,6 +637,7 @@ def _raise(reason): if "blockdev-add" not in self.supported_commands: _raise("blockdev-add qmp command is not supported") + @_ensure_connection def CheckNicHotAddSupport(self): """Check if NIC hotplug is possible @@ -630,7 +658,7 @@ def _raise(reason): if "netdev_add" not in self.supported_commands: _raise("netdev_add qmp command is not supported") - def GetFd(self, fd, fdname): + def _GetFd(self, fd, fdname): """Wrapper around the getfd qmp command Use fdsend to send an fd to a running process via SCM_RIGHTS and then use @@ -653,7 +681,7 @@ def GetFd(self, fd, fdname): logging.info("Passing fd %s via SCM_RIGHTS failed: %s", fd, err) raise - def AddFd(self, fd): + def _AddFd(self, fd): """Wrapper around add-fd qmp command Use fdsend to send fd to a running process via SCM_RIGHTS and then add-fd @@ -679,7 +707,7 @@ def AddFd(self, fd): return fdset - def RemoveFdset(self, fdset): + def _RemoveFdset(self, fdset): """Wrapper around remove-fd qmp command Remove the file descriptor previously passed. After qemu has dup'd the fd From 7c58bd7e0f9fffb19faeb0dd1521922aaf223938 Mon Sep 17 00:00:00 2001 From: Klaus Aehlig Date: Mon, 1 Dec 2014 19:19:00 +0100 Subject: [PATCH 29/34] Only check header for non-generated files For generated files, it is enough to check the headers of the files they are generated from. Moreover, the generated files have the shebang-line of the target system, which might be different from the generic one checked for. Signed-off-by: Klaus Aehlig Reviewed-by: Niklas Hambuechen --- Makefile.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index dbe867ece7..44ba84fd1f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2048,7 +2048,8 @@ check-news: .PHONY: check-local check-local: check-dirs check-news $(GENERATED_FILES) $(CHECK_PYTHON_CODE) $(check_python_code) - PYTHONPATH=. $(CHECK_HEADER) $(check_python_code) + PYTHONPATH=. $(CHECK_HEADER) \ + $(filter-out $(GENERATED_FILES),$(check_python_code)) $(CHECK_VERSION) $(VERSION) $(top_srcdir)/NEWS PYTHONPATH=. $(RUN_IN_TEMPDIR) $(CURDIR)/$(CHECK_IMPORTS) . $(standalone_python_modules) error= ; \ From 845868232610febc857960c09d2d1bc7e1f858df Mon Sep 17 00:00:00 2001 From: Niklas Hambuechen Date: Sat, 8 Nov 2014 00:09:23 +0100 Subject: [PATCH 30/34] Fix hlint warnings found by hlint 1.9.11 Our current hlint version cannot find them yet. Signed-off-by: Niklas Hambuechen Reviewed-by: Klaus Aehlig Cherry-picked from: d05f1c86fcca Conflicts: src/Ganeti/Utils.hs (trivial) Signed-off-by: Klaus Aehlig Reviewed-by: Niklas Hambuechen --- src/Ganeti/HTools/Program/Hroller.hs | 2 +- src/Ganeti/Hypervisor/Xen/Types.hs | 6 +++--- src/Ganeti/Utils.hs | 4 ++-- test/hs/Test/Ganeti/Hypervisor/Xen/XmParser.hs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Ganeti/HTools/Program/Hroller.hs b/src/Ganeti/HTools/Program/Hroller.hs index b4cdde382b..74730ed89d 100644 --- a/src/Ganeti/HTools/Program/Hroller.hs +++ b/src/Ganeti/HTools/Program/Hroller.hs @@ -409,7 +409,7 @@ main opts args = do sortBy (flip compare `on` length . fst) $ nodesRebootGroups confToMoveNames = - map (Instance.name *** (Node.name *** flip (>>=) (return . Node.name))) + map (Instance.name *** (Node.name *** (=<<) (return . Node.name))) . getMoves (nlf, ilf) namesAndMoves = map (map Node.name *** confToMoveNames) outputRebootGroups diff --git a/src/Ganeti/Hypervisor/Xen/Types.hs b/src/Ganeti/Hypervisor/Xen/Types.hs index 7026a1d3ec..704eea284d 100644 --- a/src/Ganeti/Hypervisor/Xen/Types.hs +++ b/src/Ganeti/Hypervisor/Xen/Types.hs @@ -66,7 +66,7 @@ class FromLispConfig a where -- | Instance of FromLispConfig for Int. instance FromLispConfig Int where fromLispConfig (LCDouble d) = Ok $ floor d - fromLispConfig (LCList (LCString _:LCDouble d:[])) = Ok $ floor d + fromLispConfig (LCList [LCString _, LCDouble d]) = Ok $ floor d fromLispConfig c = Bad $ "Unable to extract a Int from this configuration: " ++ show c @@ -74,7 +74,7 @@ instance FromLispConfig Int where -- | Instance of FromLispConfig for Double. instance FromLispConfig Double where fromLispConfig (LCDouble d) = Ok d - fromLispConfig (LCList (LCString _:LCDouble d:[])) = Ok d + fromLispConfig (LCList [LCString _, LCDouble d]) = Ok d fromLispConfig c = Bad $ "Unable to extract a Double from this configuration: " ++ show c @@ -82,7 +82,7 @@ instance FromLispConfig Double where -- | Instance of FromLispConfig for String instance FromLispConfig String where fromLispConfig (LCString s) = Ok s - fromLispConfig (LCList (LCString _:LCString s:[])) = Ok s + fromLispConfig (LCList [LCString _, LCString s]) = Ok s fromLispConfig c = Bad $ "Unable to extract a String from this configuration: " ++ show c diff --git a/src/Ganeti/Utils.hs b/src/Ganeti/Utils.hs index 31f490694f..bf0708e6e4 100644 --- a/src/Ganeti/Utils.hs +++ b/src/Ganeti/Utils.hs @@ -221,8 +221,8 @@ if' _ _ y = y -- | Parse results from readsPrec. parseChoices :: (Monad m, Read a) => String -> String -> [(a, String)] -> m a -parseChoices _ _ ((v, ""):[]) = return v -parseChoices name s ((_, e):[]) = +parseChoices _ _ [(v, "")] = return v +parseChoices name s [(_, e)] = fail $ name ++ ": leftover characters when parsing '" ++ s ++ "': '" ++ e ++ "'" parseChoices name s _ = fail $ name ++ ": cannot parse string '" ++ s ++ "'" diff --git a/test/hs/Test/Ganeti/Hypervisor/Xen/XmParser.hs b/test/hs/Test/Ganeti/Hypervisor/Xen/XmParser.hs index 602e2dad9b..6eb20cc182 100644 --- a/test/hs/Test/Ganeti/Hypervisor/Xen/XmParser.hs +++ b/test/hs/Test/Ganeti/Hypervisor/Xen/XmParser.hs @@ -86,7 +86,7 @@ instance Arbitrary LispConfig where -- | Determines conservatively whether a string could be a number. canBeNumber :: String -> Bool canBeNumber [] = False -canBeNumber (c:[]) = canBeNumberChar c +canBeNumber [c] = canBeNumberChar c canBeNumber (c:xs) = canBeNumberChar c && canBeNumber xs -- | Determines whether a char can be part of the string representation of a From 128f067936d0c8ae8667111466abfa564fd611d0 Mon Sep 17 00:00:00 2001 From: Petr Pudlak Date: Thu, 27 Nov 2014 12:08:53 +0100 Subject: [PATCH 31/34] Add "ignore-ipolicy" option to gnt-instance grow-disk Unless the option is given, growing disks outside the ipolicy limits will fail. Signed-off-by: Petr Pudlak Reviewed-by: Klaus Aehlig --- lib/client/gnt_instance.py | 7 +++++-- lib/tools/burnin.py | 3 ++- src/Ganeti/OpCodes.hs | 1 + test/hs/Test/Ganeti/OpCodes.hs | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py index 5e92927dd8..1dd14156c1 100644 --- a/lib/client/gnt_instance.py +++ b/lib/client/gnt_instance.py @@ -608,7 +608,9 @@ def GrowDisk(opts, args): op = opcodes.OpInstanceGrowDisk(instance_name=instance, disk=disk, amount=amount, wait_for_sync=opts.wait_for_sync, - absolute=opts.absolute) + absolute=opts.absolute, + ignore_ipolicy=opts.ignore_ipolicy + ) SubmitOrSend(op, opts) return 0 @@ -1641,7 +1643,8 @@ def ChangeGroup(opts, args): GrowDisk, [ArgInstance(min=1, max=1), ArgUnknown(min=1, max=1), ArgUnknown(min=1, max=1)], - SUBMIT_OPTS + [NWSYNC_OPT, DRY_RUN_OPT, PRIORITY_OPT, ABSOLUTE_OPT], + SUBMIT_OPTS + + [NWSYNC_OPT, DRY_RUN_OPT, PRIORITY_OPT, ABSOLUTE_OPT, IGNORE_IPOLICY_OPT], " ", "Grow an instance's disk"), "change-group": ( ChangeGroup, ARGS_ONE_INSTANCE, diff --git a/lib/tools/burnin.py b/lib/tools/burnin.py index 3ec14dd0c7..6271701e43 100755 --- a/lib/tools/burnin.py +++ b/lib/tools/burnin.py @@ -667,7 +667,8 @@ def BurnGrowDisks(self): for idx, growth in enumerate(self.disk_growth): if growth > 0: op = opcodes.OpInstanceGrowDisk(instance_name=instance, disk=idx, - amount=growth, wait_for_sync=True) + amount=growth, wait_for_sync=True, + ignore_ipolicy=True) Log("increase disk/%s by %s MB", idx, growth, indent=2) self.ExecOrQueue(instance, [op]) diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs index c6740c26fd..1238f97785 100644 --- a/src/Ganeti/OpCodes.hs +++ b/src/Ganeti/OpCodes.hs @@ -692,6 +692,7 @@ $(genOpCode "OpCode" , pDiskIndex , pDiskChgAmount , pDiskChgAbsolute + , pIgnoreIpolicy ], "instance_name") , ("OpInstanceChangeGroup", diff --git a/test/hs/Test/Ganeti/OpCodes.hs b/test/hs/Test/Ganeti/OpCodes.hs index 9dd8da00ce..20517a8910 100644 --- a/test/hs/Test/Ganeti/OpCodes.hs +++ b/test/hs/Test/Ganeti/OpCodes.hs @@ -390,7 +390,7 @@ instance Arbitrary OpCodes.OpCode where <*> arbitrary -- instance_communication "OP_INSTANCE_GROW_DISK" -> OpCodes.OpInstanceGrowDisk <$> genFQDN <*> return Nothing <*> - arbitrary <*> arbitrary <*> arbitrary <*> arbitrary + arbitrary <*> arbitrary <*> arbitrary <*> arbitrary <*> arbitrary "OP_INSTANCE_CHANGE_GROUP" -> OpCodes.OpInstanceChangeGroup <$> genFQDN <*> return Nothing <*> arbitrary <*> genMaybe genNameNE <*> From 17daebc30ddb75bdf80fe4d992a3f6cb473ca080 Mon Sep 17 00:00:00 2001 From: Petr Pudlak Date: Fri, 28 Nov 2014 13:24:54 +0100 Subject: [PATCH 32/34] Add a helper function for checking the disk size ipolicy The function only checks the disk-related parameters: The disk sizes (and their count) and the disk template. Signed-off-by: Petr Pudlak Reviewed-by: Klaus Aehlig --- lib/cmdlib/common.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/cmdlib/common.py b/lib/cmdlib/common.py index 8415a92733..e2eb909654 100644 --- a/lib/cmdlib/common.py +++ b/lib/cmdlib/common.py @@ -602,6 +602,33 @@ def ComputeIPolicySpecViolation(ipolicy, mem_size, cpu_count, disk_count, return ret + min_errs +def ComputeIPolicyDiskSizesViolation(ipolicy, disk_sizes, + disk_template, + _compute_fn=_ComputeMinMaxSpec): + """Verifies ipolicy against provided disk sizes. + + No other specs except the disk sizes, the number of disks and the disk + template are checked. + + @type ipolicy: dict + @param ipolicy: The ipolicy + @type disk_sizes: list of ints + @param disk_sizes: Disk sizes of used disk (len must match C{disk_count}) + @type disk_template: string + @param disk_template: The disk template of the instance + @param _compute_fn: The compute function (unittest only) + @return: A list of violations, or an empty list of no violations are found + + """ + return ComputeIPolicySpecViolation(ipolicy, + # mem_size, cpu_count, disk_count + None, None, len(disk_sizes), + None, disk_sizes, # nic_count, disk_sizes + None, # spindle_use + disk_template, + _compute_fn=_compute_fn) + + def ComputeIPolicyInstanceViolation(ipolicy, instance, cfg, _compute_fn=ComputeIPolicySpecViolation): """Compute if instance meets the specs of ipolicy. From 91f2d02ca44c1d361b76a63f7be4ed0c6e9001b9 Mon Sep 17 00:00:00 2001 From: Petr Pudlak Date: Tue, 2 Dec 2014 14:40:54 +0100 Subject: [PATCH 33/34] Test function ComputeIPolicyDiskSizesViolation The class tests basic properties of the function. Signed-off-by: Petr Pudlak Reviewed-by: Klaus Aehlig --- test/py/cmdlib/cmdlib_unittest.py | 74 +++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/test/py/cmdlib/cmdlib_unittest.py b/test/py/cmdlib/cmdlib_unittest.py index baa2b1786e..30cbd839d6 100755 --- a/test/py/cmdlib/cmdlib_unittest.py +++ b/test/py/cmdlib/cmdlib_unittest.py @@ -557,6 +557,80 @@ def AssertComputeViolation(ipolicy, violations): AssertComputeViolation(ipolicy1, 1) +class TestComputeIPolicyDiskSizesViolation(unittest.TestCase): + # Minimal policy accepted by _ComputeIPolicyDiskSizesViolation() + _MICRO_IPOL = { + constants.IPOLICY_DTS: [constants.DT_PLAIN, constants.DT_DISKLESS], + constants.ISPECS_MINMAX: [NotImplemented], + } + + def test(self): + compute_fn = _ValidateComputeMinMaxSpec + ret = common.ComputeIPolicyDiskSizesViolation(self._MICRO_IPOL, [1024], + constants.DT_PLAIN, + _compute_fn=compute_fn) + self.assertEqual(ret, []) + + def testDiskFull(self): + compute_fn = _NoDiskComputeMinMaxSpec + ret = common.ComputeIPolicyDiskSizesViolation(self._MICRO_IPOL, [1024], + constants.DT_PLAIN, + _compute_fn=compute_fn) + self.assertEqual(ret, [constants.ISPEC_DISK_COUNT]) + + def testDiskLess(self): + compute_fn = _NoDiskComputeMinMaxSpec + ret = common.ComputeIPolicyDiskSizesViolation(self._MICRO_IPOL, [1024], + constants.DT_DISKLESS, + _compute_fn=compute_fn) + self.assertEqual(ret, []) + + def testWrongTemplates(self): + compute_fn = _ValidateComputeMinMaxSpec + ret = common.ComputeIPolicyDiskSizesViolation(self._MICRO_IPOL, [1024], + constants.DT_DRBD8, + _compute_fn=compute_fn) + self.assertEqual(len(ret), 1) + self.assertTrue("Disk template" in ret[0]) + + def _AssertComputeViolation(self, ipolicy, disk_sizes, disk_template, + violations): + ret = common.ComputeIPolicyDiskSizesViolation(ipolicy, disk_sizes, + disk_template) + self.assertEqual(len(ret), violations) + + def testWithIPolicy(self): + mem_size = 2048 + cpu_count = 2 + disk_count = 1 + disk_sizes = [512] + nic_count = 1 + spindle_use = 4 + disk_template = "mytemplate" + ispec = { + constants.ISPEC_MEM_SIZE: mem_size, + constants.ISPEC_CPU_COUNT: cpu_count, + constants.ISPEC_DISK_COUNT: disk_count, + constants.ISPEC_DISK_SIZE: disk_sizes[0], + constants.ISPEC_NIC_COUNT: nic_count, + constants.ISPEC_SPINDLE_USE: spindle_use, + } + + ipolicy = { + constants.ISPECS_MINMAX: [{ + constants.ISPECS_MIN: ispec, + constants.ISPECS_MAX: ispec, + }], + constants.IPOLICY_DTS: [disk_template], + } + + self._AssertComputeViolation(ipolicy, [512], disk_template, 0) + self._AssertComputeViolation(ipolicy, [], disk_template, 1) + self._AssertComputeViolation(ipolicy, [512, 512], disk_template, 1) + self._AssertComputeViolation(ipolicy, [511], disk_template, 1) + self._AssertComputeViolation(ipolicy, [513], disk_template, 1) + + class _StubComputeIPolicySpecViolation: def __init__(self, mem_size, cpu_count, disk_count, nic_count, disk_sizes, spindle_use, disk_template): From f1e2d4f4f74a6ba4fbb44a831fc0553bcabe8980 Mon Sep 17 00:00:00 2001 From: Petr Pudlak Date: Fri, 28 Nov 2014 13:26:44 +0100 Subject: [PATCH 34/34] Check disk size ipolicy during "gnt-instance grow-disk" If a new disk size doesn't fit the ipolicy and --ignore-ipolicy is given, just log warnings. If it's not, fail the operation. Signed-off-by: Petr Pudlak Reviewed-by: Klaus Aehlig --- lib/cmdlib/instance_storage.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/lib/cmdlib/instance_storage.py b/lib/cmdlib/instance_storage.py index 35fba76cd9..9028e9bfc8 100644 --- a/lib/cmdlib/instance_storage.py +++ b/lib/cmdlib/instance_storage.py @@ -47,6 +47,7 @@ from ganeti.cmdlib.base import LogicalUnit, NoHooksLU, Tasklet from ganeti.cmdlib.common import INSTANCE_DOWN, INSTANCE_NOT_RUNNING, \ AnnotateDiskParams, CheckIAllocatorOrNode, ExpandNodeUuidAndName, \ + ComputeIPolicyDiskSizesViolation, \ CheckNodeOnline, CheckInstanceNodeGroups, CheckInstanceState, \ IsExclusiveStorageEnabledNode, FindFaultyInstanceDisks, GetWantedNodes, \ CheckDiskTemplateEnabled @@ -1558,6 +1559,8 @@ def CheckPrereq(self): self._CheckDiskSpace(node_uuids, self.disk.ComputeGrowth(self.delta)) + self._CheckIPolicy(self.target) + def _CheckDiskSpace(self, node_uuids, req_vgspace): template = self.instance.disk_template if (template not in (constants.DTS_NO_FREE_SPACE_CHECK) and @@ -1569,6 +1572,29 @@ def _CheckDiskSpace(self, node_uuids, req_vgspace): # the dry run performed in Exec() instead. CheckNodesFreeDiskPerVG(self, node_uuids, req_vgspace) + def _CheckIPolicy(self, target_size): + cluster = self.cfg.GetClusterInfo() + group_uuid = list(self.cfg.GetInstanceNodeGroups(self.op.instance_uuid, + primary_only=True))[0] + group_info = self.cfg.GetNodeGroup(group_uuid) + ipolicy = ganeti.masterd.instance.CalculateGroupIPolicy(cluster, + group_info) + + disk_sizes = [disk.size if disk.uuid != self.disk.uuid else target_size + for disk in self.cfg.GetInstanceDisks(self.op.instance_uuid)] + + # The ipolicy checker below ignores None, so we only give it the disk size + res = ComputeIPolicyDiskSizesViolation(ipolicy, disk_sizes, + self.instance.disk_template) + if res: + msg = ("Growing disk %s violates policy: %s" % + (self.op.disk, + utils.CommaJoin(res))) + if self.op.ignore_ipolicy: + self.LogWarning(msg) + else: + raise errors.OpPrereqError(msg, errors.ECODE_INVAL) + def Exec(self, feedback_fn): """Execute disk grow.