Commit 3b559640 authored by Iustin Pop's avatar Iustin Pop
Browse files

Remove checking of DRBD metadata for validity

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

Reviewed-by: ultrotter
parent c723c163
......@@ -847,21 +847,6 @@ class DRBD8(BaseDRBD):
raise errors.BlockDeviceError("Can't find a free DRBD minor")
return highest + 1
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",
if result.failed:
logging.error("Invalid meta device %s: %s", meta_device, result.output)
return False
return True
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):
......@@ -4726,6 +4726,10 @@ class LUReplaceDisks(LogicalUnit):
raise errors.OpPrereqError("Node '%s' not known" %
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
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment