From cad0723b646fccab4b3e880f5bbaf26f4fba2324 Mon Sep 17 00:00:00 2001 From: Iustin Pop <iustin@google.com> Date: Thu, 5 Jul 2012 12:08:44 +0200 Subject: [PATCH] Fix DRBD resize code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are two bugs in the current resize code, affecting mostly DRBD. First, due to bugs in old DRBD versions (pre 8.0.14), the code currently calls `drbdsetup resize' on both the primary or secondary. However, this is actually wrong per current DRBD (from drbdsetup(8)): resize This causes DRBD to reexamine the size of the device's backing storage device. To actually do online growing you need to extend the backing storages on both devices and call the resize command on one of your nodes. So calling it just on the primary node should be enough. However, we can't simply remove the calls to the secondary nodes, since that would break the growth of the underlying storage (LVM) on the secondary. Which leads to the second existing bug: we call resize on each node, even before finish the growth of the underlying storage. This can leads to all kind of issues if DRDB is not well behaved. So to fix both these bugs, we have to extend the current RPC call with another parameter, which denotes whether to extend the actual backing storage or just the "logical" one (DRBD being the only one; MD would be another, if implemented). This allows us to do the growth in two steps, first the backing store on all nodes, then the logical storage on just the primary node. Signed-off-by: Iustin Pop <iustin@google.com> Reviewed-by: RenΓ© Nussbaumer <rn@google.com> --- lib/backend.py | 7 +++++-- lib/bdev.py | 28 +++++++++++++++++++--------- lib/cmdlib.py | 18 +++++++++--------- lib/rpc_defs.py | 6 ++++-- lib/server/noded.py | 8 ++++++-- 5 files changed, 43 insertions(+), 24 deletions(-) diff --git a/lib/backend.py b/lib/backend.py index 51a06bbfa..b18906738 100644 --- a/lib/backend.py +++ b/lib/backend.py @@ -2443,7 +2443,7 @@ def OSEnvironment(instance, inst_os, debug=0): return result -def BlockdevGrow(disk, amount, dryrun): +def BlockdevGrow(disk, amount, dryrun, backingstore): """Grow a stack of block devices. This function is called recursively, with the childrens being the @@ -2456,6 +2456,9 @@ def BlockdevGrow(disk, amount, dryrun): @type dryrun: boolean @param dryrun: whether to execute the operation in simulation mode only, without actually increasing the size + @param backingstore: whether to execute the operation on backing storage + only, or on "logical" storage only; e.g. DRBD is logical storage, + whereas LVM, file, RBD are backing storage @rtype: (status, result) @return: a tuple with the status of the operation (True/False), and the errors message if status is False @@ -2466,7 +2469,7 @@ def BlockdevGrow(disk, amount, dryrun): _Fail("Cannot find block device %s", disk) try: - r_dev.Grow(amount, dryrun) + r_dev.Grow(amount, dryrun, backingstore) except errors.BlockDeviceError, err: _Fail("Failed to grow block device: %s", err, exc=True) diff --git a/lib/bdev.py b/lib/bdev.py index 6c16da425..3bb3f115d 100644 --- a/lib/bdev.py +++ b/lib/bdev.py @@ -338,7 +338,7 @@ class BlockDev(object): for child in self._children: child.SetInfo(text) - def Grow(self, amount, dryrun): + def Grow(self, amount, dryrun, backingstore): """Grow the block device. @type amount: integer @@ -346,6 +346,9 @@ class BlockDev(object): @type dryrun: boolean @param dryrun: whether to execute the operation in simulation mode only, without actually increasing the size + @param backingstore: whether to execute the operation on backing storage + only, or on "logical" storage only; e.g. DRBD is logical storage, + whereas LVM, file, RBD are backing storage """ raise NotImplementedError @@ -768,10 +771,12 @@ class LogicalVolume(BlockDev): _ThrowError("Command: %s error: %s - %s", result.cmd, result.fail_reason, result.output) - def Grow(self, amount, dryrun): + def Grow(self, amount, dryrun, backingstore): """Grow the logical volume. """ + if not backingstore: + return if self.pe_size is None or self.stripe_count is None: if not self.Attach(): _ThrowError("Can't attach to LV during Grow()") @@ -2070,7 +2075,7 @@ class DRBD8(BaseDRBD): cls._InitMeta(aminor, meta.dev_path) return cls(unique_id, children, size, params) - def Grow(self, amount, dryrun): + def Grow(self, amount, dryrun, backingstore): """Resize the DRBD device and its backing storage. """ @@ -2078,9 +2083,10 @@ class DRBD8(BaseDRBD): _ThrowError("drbd%d: Grow called while not attached", self._aminor) if len(self._children) != 2 or None in self._children: _ThrowError("drbd%d: cannot grow diskless device", self.minor) - self._children[0].Grow(amount, dryrun) - if dryrun: - # DRBD does not support dry-run mode, so we'll return here + self._children[0].Grow(amount, dryrun, backingstore) + if dryrun or backingstore: + # DRBD does not support dry-run mode and is not backing storage, + # so we'll return here return result = utils.RunCmd(["drbdsetup", self.dev_path, "resize", "-s", "%dm" % (self.size + amount)]) @@ -2163,12 +2169,14 @@ class FileStorage(BlockDev): # TODO: implement rename for file-based storage _ThrowError("Rename is not supported for file-based storage") - def Grow(self, amount, dryrun): + def Grow(self, amount, dryrun, backingstore): """Grow the file @param amount: the amount (in mebibytes) to grow with """ + if not backingstore: + return # Check that the file exists self.Assemble() current_size = self.GetActualSize() @@ -2339,7 +2347,7 @@ class PersistentBlockDevice(BlockDev): """ pass - def Grow(self, amount, dryrun): + def Grow(self, amount, dryrun, backingstore): """Grow the logical volume. """ @@ -2605,7 +2613,7 @@ class RADOSBlockDevice(BlockDev): """ pass - def Grow(self, amount, dryrun): + def Grow(self, amount, dryrun, backingstore): """Grow the Volume. @type amount: integer @@ -2615,6 +2623,8 @@ class RADOSBlockDevice(BlockDev): only, without actually increasing the size """ + if not backingstore: + return if not self.Attach(): _ThrowError("Can't attach to rbd device during Grow()") diff --git a/lib/cmdlib.py b/lib/cmdlib.py index 3a22ef53a..e5f0451f7 100644 --- a/lib/cmdlib.py +++ b/lib/cmdlib.py @@ -11683,23 +11683,23 @@ class LUInstanceGrowDisk(LogicalUnit): for node in instance.all_nodes: self.cfg.SetDiskID(disk, node) result = self.rpc.call_blockdev_grow(node, (disk, instance), self.delta, - True) + True, True) result.Raise("Grow request failed to node %s" % node) # We know that (as far as we can test) operations across different - # nodes will succeed, time to run it for real + # nodes will succeed, time to run it for real on the backing storage for node in instance.all_nodes: self.cfg.SetDiskID(disk, node) result = self.rpc.call_blockdev_grow(node, (disk, instance), self.delta, - False) + False, True) result.Raise("Grow request failed to node %s" % node) - # TODO: Rewrite code to work properly - # DRBD goes into sync mode for a short amount of time after executing the - # "resize" command. DRBD 8.x below version 8.0.13 contains a bug whereby - # calling "resize" in sync mode fails. Sleeping for a short amount of - # time is a work-around. - time.sleep(5) + # And now execute it for logical storage, on the primary node + node = instance.primary_node + self.cfg.SetDiskID(disk, node) + result = self.rpc.call_blockdev_grow(node, (disk, instance), self.delta, + False, False) + result.Raise("Grow request failed to node %s" % node) disk.RecordGrow(self.delta) self.cfg.Update(instance, feedback_fn) diff --git a/lib/rpc_defs.py b/lib/rpc_defs.py index 35c5a490c..2d2fcaca4 100644 --- a/lib/rpc_defs.py +++ b/lib/rpc_defs.py @@ -1,7 +1,7 @@ # # -# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Google Inc. +# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Google Inc. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -393,7 +393,9 @@ _BLOCKDEV_CALLS = [ ("cf_bdev", ED_SINGLE_DISK_DICT_DP, None), ("amount", None, None), ("dryrun", None, None), - ], None, None, "Request a snapshot of the given block device"), + ("backingstore", None, None), + ], None, None, "Request growing of the given block device by a" + " given amount"), ("blockdev_export", SINGLE, None, TMO_1DAY, [ ("cf_bdev", ED_SINGLE_DISK_DICT_DP, None), ("dest_node", None, None), diff --git a/lib/server/noded.py b/lib/server/noded.py index d95680a55..ae7be3216 100644 --- a/lib/server/noded.py +++ b/lib/server/noded.py @@ -1,7 +1,7 @@ # # -# Copyright (C) 2006, 2007, 2010, 2011 Google Inc. +# Copyright (C) 2006, 2007, 2010, 2011, 2012 Google Inc. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -332,10 +332,14 @@ class NodeRequestHandler(http.server.HttpServerHandler): """Grow a stack of devices. """ + if len(params) < 4: + raise ValueError("Received only 3 parameters in blockdev_grow," + " old master?") cfbd = objects.Disk.FromDict(params[0]) amount = params[1] dryrun = params[2] - return backend.BlockdevGrow(cfbd, amount, dryrun) + backingstore = params[3] + return backend.BlockdevGrow(cfbd, amount, dryrun, backingstore) @staticmethod def perspective_blockdev_close(params): -- GitLab