From 3b5596402bbe33d34a52dc7db8e3636527fc25cd Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Fri, 23 Jan 2009 12:36:28 +0000
Subject: [PATCH] Remove checking of DRBD metadata for validity
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently the DRBD code checks that the metadata devices are valid
before creation, initial disk attachment and add children.

However, the process for checking validity requires a free DRBD minor,
and this conflict with parallel checking.

There are at least three possible solutions:
  - serialize all checks, which means we reduce parallelism and need
    extra locks
  - don't pass a valid minor number, but one like β€œ/dev/drbd256” (which
    is invalid); this works for current version of DRBD, but since it's
    not guaranteed to remain so it doesn't look nice
  - don't do the checking at all, and rely on β€œdrbdsetup ... disk ...”
    to fail by itself

The reason for checking metadata was that in 1.2, this was much cheaper
than trying to activate devices (and the subsequent iteration over the
minors). However, in 2.0, they have the same cost, so we can choose
option 3: just remove the explicit checking and rely on drbdsetup and
the kernel to fail.

Since DRBD8._InitMeta still requires a minor number, the two places
where this is run are handled as follows:
  - Create: we just use our own (unused currently) minor number
  - AddChildren: we keep using FindUnusedMinor, with the caveat that
    this function (used by replace-disks -n ...) cannot be yet
    parallelized

Reviewed-by: ultrotter
---
 lib/bdev.py   | 26 +-------------------------
 lib/cmdlib.py |  4 ++++
 2 files changed, 5 insertions(+), 25 deletions(-)

diff --git a/lib/bdev.py b/lib/bdev.py
index 08551b2d8..834d7b3ea 100644
--- a/lib/bdev.py
+++ b/lib/bdev.py
@@ -847,21 +847,6 @@ class DRBD8(BaseDRBD):
       raise errors.BlockDeviceError("Can't find a free DRBD minor")
     return highest + 1
 
-  @classmethod
-  def _IsValidMeta(cls, meta_device):
-    """Check if the given meta device looks like a valid one.
-
-    """
-    minor = cls._FindUnusedMinor()
-    minor_path = cls._DevPath(minor)
-    result = utils.RunCmd(["drbdmeta", minor_path,
-                           "v08", meta_device, "0",
-                           "dstate"])
-    if result.failed:
-      logging.error("Invalid meta device %s: %s", meta_device, result.output)
-      return False
-    return True
-
   @classmethod
   def _GetShowParser(cls):
     """Return a parser for `drbd show` output.
@@ -1023,12 +1008,7 @@ class DRBD8(BaseDRBD):
   def _AssembleLocal(cls, minor, backend, meta):
     """Configure the local part of a DRBD device.
 
-    This is the first thing that must be done on an unconfigured DRBD
-    device. And it must be done only once.
-
     """
-    if not cls._IsValidMeta(meta):
-      return False
     args = ["drbdsetup", cls._DevPath(minor), "disk",
             backend, meta, "0", "-e", "detach", "--create-device"]
     result = utils.RunCmd(args)
@@ -1109,8 +1089,6 @@ class DRBD8(BaseDRBD):
     if not self._CheckMetaSize(meta.dev_path):
       raise errors.BlockDeviceError("Invalid meta device size")
     self._InitMeta(self._FindUnusedMinor(), meta.dev_path)
-    if not self._IsValidMeta(meta.dev_path):
-      raise errors.BlockDeviceError("Cannot initalize meta device")
 
     if not self._AssembleLocal(self.minor, backend.dev_path, meta.dev_path):
       raise errors.BlockDeviceError("Can't attach to local storage")
@@ -1551,9 +1529,7 @@ class DRBD8(BaseDRBD):
       raise errors.BlockDeviceError("Can't attach to meta device")
     if not cls._CheckMetaSize(meta.dev_path):
       raise errors.BlockDeviceError("Invalid meta device size")
-    cls._InitMeta(cls._FindUnusedMinor(), meta.dev_path)
-    if not cls._IsValidMeta(meta.dev_path):
-      raise errors.BlockDeviceError("Cannot initalize meta device")
+    cls._InitMeta(aminor, meta.dev_path)
     return cls(unique_id, children)
 
   def Grow(self, amount):
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 7da0f87c6..e337b686a 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -4726,6 +4726,10 @@ class LUReplaceDisks(LogicalUnit):
         raise errors.OpPrereqError("Node '%s' not known" %
                                    self.op.remote_node)
       self.op.remote_node = remote_node
+      # Warning: do not remove the locking of the new secondary here
+      # unless DRBD8.AddChildren is changed to work in parallel;
+      # currently it doesn't since parallel invocations of
+      # FindUnusedMinor will conflict
       self.needed_locks[locking.LEVEL_NODE] = [remote_node]
       self.recalculate_locks[locking.LEVEL_NODE] = constants.LOCKS_APPEND
     else:
-- 
GitLab