Skip to content

Commit

Permalink
Merge branch 'stable-2.13' into master
Browse files Browse the repository at this point in the history
* stable-2.13
  kvm: Delegate socket handling to monitor module
  kvm: Use vhostfds obtained by OpenTap
  hotplug: Retry VerifyHotplugCommand up to 5 times
  Update hotplug design doc
  qa: Re-enable NIC hotplug tests
  qmp: Log qmp commands and responses
  hotplug: Is not supported for QEMU < 1.7
  hotplug: Remove unused code after refacoring
  hotplug: Use QMP during HotDelDevice
  hotplug: Use QMP during HotAddDevice
  kvm: Use the new interface during instance start
  monitor: Extend HotAddNic() for advanced features
  kvm: Refactor _GetNetworkDeviceFeatures() helper
  netdev: Refactor OpenTap for future use
  hotplug: Use QMP in VerifyHotplugCommand
  hotplug: Use QMP in VerifyHotplugSupport
  monitor: Close socket fd if already connected
  kvm: New _with_qmp decorator
  qmp: Add Disk hotplugging related method
  qmp: Add NIC hotplugging related methods
  qmp: Helper methods for parsing query-pci outpu
  qmp: Add helper methods to verify hotplug support
  qmp: Add GetFd() wrapper around getfd command
  qmp: Refactor of add-fd and remove-fd commands
  utils: Introduce GetFreeSlot() function
  kvm: Minor refactor of MonitorSocket

* stable-2.12
  Check disk size ipolicy during "gnt-instance grow-disk"
  Test function ComputeIPolicyDiskSizesViolation
  Add a helper function for checking the disk size ipolicy
  Add "ignore-ipolicy" option to gnt-instance grow-disk
  Fix the installation instructions for the DRBD module
  Update install docs - DRBD module parameters

* stable-2.11
  (no changes)

* stable-2.10
  Fix hlint warnings found by hlint 1.9.11
  Only check header for non-generated files

Signed-off-by: Klaus Aehlig <[email protected]>
Reviewed-by: Niklas Hambuechen <[email protected]>
  • Loading branch information
aehlig committed Dec 4, 2014
2 parents dbe4681 + 51620a2 commit cdda271
Show file tree
Hide file tree
Showing 17 changed files with 761 additions and 284 deletions.
7 changes: 5 additions & 2 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,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 \
Expand Down Expand Up @@ -1905,6 +1906,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 \
Expand Down Expand Up @@ -2510,7 +2512,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= ; \
Expand Down
29 changes: 15 additions & 14 deletions doc/design-hotplug.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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.
Expand All @@ -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:
Expand Down
6 changes: 4 additions & 2 deletions doc/install.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions lib/client/gnt_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -1690,7 +1692,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],
"<instance> <disk> <size>", "Grow an instance's disk"),
"change-group": (
ChangeGroup, ARGS_ONE_INSTANCE,
Expand Down
27 changes: 27 additions & 0 deletions lib/cmdlib/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,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.
Expand Down
26 changes: 26 additions & 0 deletions lib/cmdlib/instance_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1729,6 +1730,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.disk.dev_type
if (template not in constants.DTS_NO_FREE_SPACE_CHECK and
Expand All @@ -1740,6 +1743,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.
Expand Down
Loading

0 comments on commit cdda271

Please sign in to comment.