Add DRBD barriers disk parameters
authorAndrea Spadaccini <spadaccio@google.com>
Mon, 28 Nov 2011 18:17:04 +0000 (18:17 +0000)
committerAndrea Spadaccini <spadaccio@google.com>
Tue, 6 Dec 2011 16:34:42 +0000 (16:34 +0000)
Add the disk-barriers and meta-barriers parameters described in the
design doc.

constants.py:
* add the needed LD and DT-level parameters, use the defaults provided
  at ./configure time;
* add constants representing which barriers should be disabled and the
  set of valid options.

lib/bdev.py:
* factor the barriers handling code to a class method, for testing
  purposes;
* implement the more granular version checking logic;
* use the LD level parameters;
* add stricter check on DRBD version (8.0, 8.2 or 8.3), as we do not
  support 8.4 yet.

lib/cmdlib.py:
* translate DT level parameters to LD level ones.

configure.ac, Makefile.am:
* set both disk and meta barriers parameters depending on the value of
  --enable-drbd-barriers.

test/ganeti.bdev_unittest.py:
* unit tests for the code that sets DRBD barrier parameters depending on
  the version.

doc/design-resource-model.rst:
* reword the description of meta-barriers;
* change all disk parameters names to use dashes instead of underscores.

Signed-off-by: Andrea Spadaccini <spadaccio@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Makefile.am
configure.ac
doc/design-resource-model.rst
lib/bdev.py
lib/cmdlib.py
lib/constants.py
test/ganeti.bdev_unittest.py

index 05a3152..80e4808 100644 (file)
@@ -985,7 +985,8 @@ lib/_autoconf.py: Makefile | lib/.dir
          echo "TOOLSDIR = '$(toolsdir)'"; \
          echo "GNT_SCRIPTS = [$(foreach i,$(notdir $(gnt_scripts)),'$(i)',)]"; \
          echo "PKGLIBDIR = '$(pkglibdir)'"; \
-         echo "DRBD_BARRIERS = $(DRBD_BARRIERS)"; \
+         echo "DRBD_BARRIERS = '$(DRBD_BARRIERS)'"; \
+         echo "DRBD_NO_META_FLUSH = $(DRBD_NO_META_FLUSH)"; \
          echo "SYSLOG_USAGE = '$(SYSLOG_USAGE)'"; \
          echo "DAEMONS_GROUP = '$(DAEMONS_GROUP)'"; \
          echo "ADMIN_GROUP = '$(ADMIN_GROUP)'"; \
index 7c4b148..b672d95 100644 (file)
@@ -208,15 +208,20 @@ AC_MSG_NOTICE([Group for clients is $group_admin])
 # --enable-drbd-barriers
 AC_ARG_ENABLE([drbd-barriers],
   [AS_HELP_STRING([--enable-drbd-barriers],
-    [enable the DRBD barrier functionality (>= 8.0.12) (default: enabled)])],
+    [enable by default the DRBD barriers functionality (>= 8.0.12) (default: enabled)])],
   [[if test "$enableval" != no; then
-      DRBD_BARRIERS=True
+      DRBD_BARRIERS=n
+      DRBD_NO_META_FLUSH=False
     else
-      DRBD_BARRIERS=False
+      DRBD_BARRIERS=bfd
+      DRBD_NO_META_FLUSH=True
     fi
   ]],
-  [DRBD_BARRIERS=True])
+  [DRBD_BARRIERS=n
+   DRBD_NO_META_FLUSH=False
+  ])
 AC_SUBST(DRBD_BARRIERS, $DRBD_BARRIERS)
+AC_SUBST(DRBD_NO_META_FLUSH, $DRBD_NO_META_FLUSH)
 
 # --enable-syslog[=no/yes/only]
 AC_ARG_ENABLE([syslog],
index e9e179f..212fdc7 100644 (file)
@@ -724,28 +724,28 @@ encoded via a separator (e.g. slash), or by having two dict levels.
 |        |             |                         |as the meta LVs are  |      |
 |        |             |                         |small                |      |
 +--------+-------------+-------------------------+---------------------+------+
-|drbd    |disk_barriers|What kind of barriers to |Either all enabled or|string|
+|drbd    |disk-barriers|What kind of barriers to |Either all enabled or|string|
 |        |             |*disable* for disks;     |all disabled, per    |      |
 |        |             |either "n" or a string   |./configure time     |      |
 |        |             |containing a subset of   |option               |      |
 |        |             |"bfd"                    |                     |      |
 +--------+-------------+-------------------------+---------------------+------+
-|drbd    |meta_barriers|Whether barriers are     |Handled together with|bool  |
-|        |             |enabled or not for the   |disk_barriers        |      |
-|        |             |meta volume              |                     |      |
+|drbd    |meta-barriers|Whether to disable or not|Handled together with|bool  |
+|        |             |the barriers for the meta|disk-barriers        |      |
+|        |             |volume                   |                     |      |
 +--------+-------------+-------------------------+---------------------+------+
-|drbd    |resync_rate  |The (static) resync rate |Hardcoded in         |int   |
+|drbd    |resync-rate  |The (static) resync rate |Hardcoded in         |int   |
 |        |             |for drbd, when using the |constants.py, not    |      |
 |        |             |static syncer, in MiB/s  |changeable via Ganeti|      |
 +--------+-------------+-------------------------+---------------------+------+
-|drbd    |disk_custom  |Free-form string that    |Not supported        |string|
+|drbd    |disk-custom  |Free-form string that    |Not supported        |string|
 |        |             |will be appended to the  |                     |      |
 |        |             |drbdsetup disk command   |                     |      |
 |        |             |line, for custom options |                     |      |
 |        |             |not supported by Ganeti  |                     |      |
 |        |             |itself                   |                     |      |
 +--------+-------------+-------------------------+---------------------+------+
-|drbd    |net_custom   |Free-form string for     |Not supported        |string|
+|drbd    |net-custom   |Free-form string for     |Not supported        |string|
 |        |             |custom net setup options |                     |      |
 +--------+-------------+-------------------------+---------------------+------+
 
index 230a4a4..fabba9f 100644 (file)
@@ -1103,6 +1103,12 @@ class DRBD8(BaseDRBD):
   # timeout constants
   _NET_RECONFIG_TIMEOUT = 60
 
+  # command line options for barriers
+  _DISABLE_DISK_OPTION = "--no-disk-barrier"  # -a
+  _DISABLE_DRAIN_OPTION = "--no-disk-drain"   # -D
+  _DISABLE_FLUSH_OPTION = "--no-disk-flushes" # -i
+  _DISABLE_META_FLUSH_OPTION = "--no-md-flushes"  # -m
+
   def __init__(self, unique_id, children, size, params):
     if children and children.count(None) > 0:
       children = []
@@ -1344,39 +1350,106 @@ class DRBD8(BaseDRBD):
               info["remote_addr"] == (self._rhost, self._rport))
     return retval
 
-  @classmethod
-  def _AssembleLocal(cls, minor, backend, meta, size):
+  def _AssembleLocal(self, minor, backend, meta, size):
     """Configure the local part of a DRBD device.
 
     """
-    args = ["drbdsetup", cls._DevPath(minor), "disk",
+    args = ["drbdsetup", self._DevPath(minor), "disk",
             backend, meta, "0",
             "-e", "detach",
             "--create-device"]
     if size:
       args.extend(["-d", "%sm" % size])
-    if not constants.DRBD_BARRIERS: # disable barriers, if configured so
-      version = cls._GetVersion(cls._GetProcData())
-      # various DRBD versions support different disk barrier options;
-      # what we aim here is to revert back to the 'drain' method of
-      # disk flushes and to disable metadata barriers, in effect going
-      # back to pre-8.0.7 behaviour
-      vmaj = version["k_major"]
-      vmin = version["k_minor"]
-      vrel = version["k_point"]
-      assert vmaj == 8
-      if vmin == 0: # 8.0.x
-        if vrel >= 12:
-          args.extend(["-i", "-m"])
-      elif vmin == 2: # 8.2.x
-        if vrel >= 7:
-          args.extend(["-i", "-m"])
-      elif vmaj >= 3: # 8.3.x or newer
-        args.extend(["-i", "-a", "m"])
+
+    version = self._GetVersion(self._GetProcData())
+    vmaj = version["k_major"]
+    vmin = version["k_minor"]
+    vrel = version["k_point"]
+
+    barrier_args = \
+      self._ComputeDiskBarrierArgs(vmaj, vmin, vrel,
+                                   self.params[constants.BARRIERS],
+                                   self.params[constants.NO_META_FLUSH])
+    args.extend(barrier_args)
+
     result = utils.RunCmd(args)
     if result.failed:
       _ThrowError("drbd%d: can't attach local disk: %s", minor, result.output)
 
+  @classmethod
+  def _ComputeDiskBarrierArgs(cls, vmaj, vmin, vrel, disabled_barriers,
+      disable_meta_flush):
+    """Compute the DRBD command line parameters for disk barriers
+
+    Returns a list of the disk barrier parameters as requested via the
+    disabled_barriers and disable_meta_flush arguments, and according to the
+    supported ones in the DRBD version vmaj.vmin.vrel
+
+    If the desired option is unsupported, raises errors.BlockDeviceError.
+
+    """
+    disabled_barriers_set = frozenset(disabled_barriers)
+    if not disabled_barriers_set in constants.DRBD_VALID_BARRIER_OPT:
+      raise errors.BlockDeviceError("%s is not a valid option set for DRBD"
+                                    " barriers" % disabled_barriers)
+
+    args = []
+
+    # The following code assumes DRBD 8.x, with x < 4 and x != 1 (DRBD 8.1.x
+    # does not exist)
+    if not vmaj == 8 and vmin in (0, 2, 3):
+      raise errors.BlockDeviceError("Unsupported DRBD version: %d.%d.%d" %
+                                    (vmaj, vmin, vrel))
+
+    def _AppendOrRaise(option, min_version):
+      """Helper for DRBD options"""
+      if min_version is not None and vrel >= min_version:
+        args.append(option)
+      else:
+        raise errors.BlockDeviceError("Could not use the option %s as the"
+                                      " DRBD version %d.%d.%d does not support"
+                                      " it." % (option, vmaj, vmin, vrel))
+
+    # the minimum version for each feature is encoded via pairs of (minor
+    # version -> x) where x is version in which support for the option was
+    # introduced.
+    meta_flush_supported = disk_flush_supported = {
+      0: 12,
+      2: 7,
+      3: 0,
+      }
+
+    disk_drain_supported = {
+      2: 7,
+      3: 0,
+      }
+
+    disk_barriers_supported = {
+      3: 0,
+      }
+
+    # meta flushes
+    if disable_meta_flush:
+      _AppendOrRaise(cls._DISABLE_META_FLUSH_OPTION,
+                     meta_flush_supported.get(vmin, None))
+
+    # disk flushes
+    if constants.DRBD_B_DISK_FLUSH in disabled_barriers_set:
+      _AppendOrRaise(cls._DISABLE_FLUSH_OPTION,
+                     disk_flush_supported.get(vmin, None))
+
+    # disk drain
+    if constants.DRBD_B_DISK_DRAIN in disabled_barriers_set:
+      _AppendOrRaise(cls._DISABLE_DRAIN_OPTION,
+                     disk_drain_supported.get(vmin, None))
+
+    # disk barriers
+    if constants.DRBD_B_DISK_BARRIERS in disabled_barriers_set:
+      _AppendOrRaise(cls._DISABLE_DISK_OPTION,
+                     disk_barriers_supported.get(vmin, None))
+
+    return args
+
   def _AssembleNet(self, minor, net_info, protocol,
                    dual_pri=False, hmac=None, secret=None):
     """Configure the network part of the device.
index e04f46c..561a9bf 100644 (file)
@@ -8058,7 +8058,9 @@ def _ComputeLDParams(disk_template, disk_params):
   dt_params = disk_params[disk_template]
   if disk_template == constants.DT_DRBD8:
     drbd_params = {
-      constants.RESYNC_RATE: dt_params[constants.DRBD_RESYNC_RATE]
+      constants.RESYNC_RATE: dt_params[constants.DRBD_RESYNC_RATE],
+      constants.BARRIERS: dt_params[constants.DRBD_DISK_BARRIERS],
+      constants.NO_META_FLUSH: dt_params[constants.DRBD_META_BARRIERS],
       }
 
     drbd_params = \
index c7aa397..c91f5f5 100644 (file)
@@ -472,7 +472,25 @@ LDS_BLOCK = frozenset([LD_LV, LD_DRBD8, LD_BLOCKDEV])
 # drbd constants
 DRBD_HMAC_ALG = "md5"
 DRBD_NET_PROTOCOL = "C"
-DRBD_BARRIERS = _autoconf.DRBD_BARRIERS
+
+# drbd barrier types
+DRBD_B_NONE = "n"
+DRBD_B_DISK_BARRIERS = "b"
+DRBD_B_DISK_DRAIN = "d"
+DRBD_B_DISK_FLUSH = "f"
+
+# Valid barrier combinations: "n" or any non-null subset of "bfd"
+DRBD_VALID_BARRIER_OPT = frozenset([
+  frozenset([DRBD_B_NONE]),
+  frozenset([DRBD_B_DISK_BARRIERS]),
+  frozenset([DRBD_B_DISK_DRAIN]),
+  frozenset([DRBD_B_DISK_FLUSH]),
+  frozenset([DRBD_B_DISK_DRAIN, DRBD_B_DISK_FLUSH]),
+  frozenset([DRBD_B_DISK_DRAIN, DRBD_B_DISK_FLUSH]),
+  frozenset([DRBD_B_DISK_BARRIERS, DRBD_B_DISK_DRAIN]),
+  frozenset([DRBD_B_DISK_BARRIERS, DRBD_B_DISK_FLUSH]),
+  frozenset([DRBD_B_DISK_BARRIERS, DRBD_B_DISK_FLUSH, DRBD_B_DISK_DRAIN]),
+  ])
 
 # file backend driver
 FD_LOOP = "loop"
@@ -898,9 +916,13 @@ NDS_PARAMETERS = frozenset(NDS_PARAMETER_TYPES.keys())
 # Logical Disks parameters
 RESYNC_RATE = "resync-rate"
 STRIPES = "stripes"
+BARRIERS = "disabled-barriers"
+NO_META_FLUSH = "disable-meta-flush"
 DISK_LD_TYPES = {
   RESYNC_RATE: VTYPE_INT,
   STRIPES: VTYPE_INT,
+  BARRIERS: VTYPE_STRING,
+  NO_META_FLUSH: VTYPE_BOOL,
   }
 DISK_LD_PARAMETERS = frozenset(DISK_LD_TYPES.keys())
 
@@ -908,11 +930,15 @@ DISK_LD_PARAMETERS = frozenset(DISK_LD_TYPES.keys())
 DRBD_RESYNC_RATE = "resync-rate"
 DRBD_DATA_STRIPES = "data-stripes"
 DRBD_META_STRIPES = "meta-stripes"
+DRBD_DISK_BARRIERS = "disk-barriers"
+DRBD_META_BARRIERS = "meta-barriers"
 LV_STRIPES = "stripes"
 DISK_DT_TYPES = {
   DRBD_RESYNC_RATE: VTYPE_INT,
   DRBD_DATA_STRIPES: VTYPE_INT,
   DRBD_META_STRIPES: VTYPE_INT,
+  DRBD_DISK_BARRIERS: VTYPE_STRING,
+  DRBD_META_BARRIERS: VTYPE_BOOL,
   LV_STRIPES: VTYPE_INT,
   }
 
@@ -1683,6 +1709,8 @@ NDC_DEFAULTS = {
 DISK_LD_DEFAULTS = {
   LD_DRBD8: {
     RESYNC_RATE: CLASSIC_DRBD_SYNC_SPEED,
+    BARRIERS: _autoconf.DRBD_BARRIERS,
+    NO_META_FLUSH: _autoconf.DRBD_NO_META_FLUSH,
     },
   LD_LV: {
     STRIPES: _autoconf.LVM_STRIPECOUNT
@@ -1701,6 +1729,8 @@ DISK_DT_DEFAULTS = {
     DRBD_RESYNC_RATE: DISK_LD_DEFAULTS[LD_DRBD8][RESYNC_RATE],
     DRBD_DATA_STRIPES: DISK_LD_DEFAULTS[LD_LV][STRIPES],
     DRBD_META_STRIPES: DISK_LD_DEFAULTS[LD_LV][STRIPES],
+    DRBD_DISK_BARRIERS: DISK_LD_DEFAULTS[LD_DRBD8][BARRIERS],
+    DRBD_META_BARRIERS: DISK_LD_DEFAULTS[LD_DRBD8][NO_META_FLUSH],
     },
   DT_DISKLESS: {
     },
index 6f8e3d4..2d418a9 100755 (executable)
@@ -27,6 +27,7 @@ import unittest
 
 from ganeti import bdev
 from ganeti import errors
+from ganeti import constants
 
 import testutils
 
@@ -156,6 +157,75 @@ class TestDRBD8Runner(testutils.GanetiTestCase):
                      "remote_addr" not in result),
                     "Should not find network info")
 
+  def testBarriersOptions(self):
+    """Test class method that generates drbdsetup options for disk barriers"""
+    # Tests that should fail because of wrong version/options combinations
+    should_fail = [
+      (8, 0, 12, "bfd", True),
+      (8, 0, 12, "fd", False),
+      (8, 0, 12, "b", True),
+      (8, 2, 7, "bfd", True),
+      (8, 2, 7, "b", True)
+    ]
+
+    for vmaj, vmin, vrel, opts, meta in should_fail:
+      self.assertRaises(errors.BlockDeviceError,
+                        bdev.DRBD8._ComputeDiskBarrierArgs,
+                        vmaj, vmin, vrel, opts, meta)
+
+    # get the valid options from the frozenset(frozenset()) in constants.
+    valid_options = [list(x)[0] for x in constants.DRBD_VALID_BARRIER_OPT]
+
+    # Versions that do not support anything
+    for vmaj, vmin, vrel in ((8, 0, 0), (8, 0, 11), (8, 2, 6)):
+      for opts in valid_options:
+        self.assertRaises(errors.BlockDeviceError,
+                          bdev.DRBD8._ComputeDiskBarrierArgs,
+                          vmaj, vmin, vrel, opts, True)
+
+    # Versions with partial support (testing only options that are supported)
+    tests = [
+      (8, 0, 12, "n", False, []),
+      (8, 0, 12, "n", True, ["--no-md-flushes"]),
+      (8, 2, 7, "n", False, []),
+      (8, 2, 7, "fd", False, ["--no-disk-flushes", "--no-disk-drain"]),
+      (8, 0, 12, "n", True, ["--no-md-flushes"]),
+      ]
+
+    # Versions that support everything
+    for vmaj, vmin, vrel in ((8, 3, 0), (8, 3, 12)):
+      tests.append((vmaj, vmin, vrel, "bfd", True,
+                    ["--no-disk-barrier", "--no-disk-drain",
+                     "--no-disk-flushes", "--no-md-flushes"]))
+      tests.append((vmaj, vmin, vrel, "n", False, []))
+      tests.append((vmaj, vmin, vrel, "b", True,
+                    ["--no-disk-barrier", "--no-md-flushes"]))
+      tests.append((vmaj, vmin, vrel, "fd", False,
+                    ["--no-disk-flushes", "--no-disk-drain"]))
+      tests.append((vmaj, vmin, vrel, "n", True, ["--no-md-flushes"]))
+
+    # Test execution
+    for test in tests:
+      vmaj, vmin, vrel, disabled_barriers, disable_meta_flush, expected = test
+      args = \
+        bdev.DRBD8._ComputeDiskBarrierArgs(vmaj, vmin, vrel,
+                                           disabled_barriers,
+                                           disable_meta_flush)
+      self.failUnless(set(args) == set(expected),
+                      "For test %s, got wrong results %s" % (test, args))
+
+    # Unsupported or invalid versions
+    for vmaj, vmin, vrel in ((0, 7, 25), (9, 0, 0), (7, 0, 0), (8, 4, 0)):
+      self.assertRaises(errors.BlockDeviceError,
+                        bdev.DRBD8._ComputeDiskBarrierArgs,
+                        vmaj, vmin, vrel, "n", True)
+
+    # Invalid options
+    for option in ("", "c", "whatever", "nbdfc", "nf"):
+      self.assertRaises(errors.BlockDeviceError,
+                        bdev.DRBD8._ComputeDiskBarrierArgs,
+                        8, 3, 11, option, True)
+
 
 class TestDRBD8Status(testutils.GanetiTestCase):
   """Testing case for DRBD8 /proc status"""