On Mon, Apr 29, 2013 at 1:51 PM, Thomas Thrainer <[email protected]>wrote:
> The DRBD84CmdGenerator class has been added which generates commands > suited for DRBD 8.4. A common baseclass from DRBD83CmdGenerator and > DRBD84CmdGenerator has been introduced as well. > Usually we use the present and not the past to describe the commit. "The DRBD84CmdGenerator class is added [...] is introduced as well". > > Signed-off-by: Thomas Thrainer <[email protected]> > --- > lib/block/drbd.py | 61 ++++---- > lib/block/drbd_cmdgen.py | 254 > +++++++++++++++++++++++++++++++--- > test/py/ganeti.block.drbd_unittest.py | 3 +- > 3 files changed, 273 insertions(+), 45 deletions(-) > > diff --git a/lib/block/drbd.py b/lib/block/drbd.py > index 3df7db7..b69a821 100644 > --- a/lib/block/drbd.py > +++ b/lib/block/drbd.py > @@ -90,11 +90,10 @@ class DRBD8(base.BlockDev): > > if version["k_minor"] <= 3: > self._show_info_cls = DRBD83ShowInfo > - self._cmd_gen = drbd_cmdgen.DRBD83CmdGenerator(drbd_info) > else: > self._show_info_cls = DRBD84ShowInfo > - # FIXME: use proper command generator! > - self._cmd_gen = None > + > + self._cmd_gen = self._GetCmdGenerator(drbd_info) > > if (self._lhost is not None and self._lhost == self._rhost and > self._lport == self._rport): > @@ -102,6 +101,14 @@ class DRBD8(base.BlockDev): > (unique_id,)) > self.Attach() > > + @classmethod > + def _GetCmdGenerator(cls, drbd_info): > + version = drbd_info.GetVersion() > + if version["k_minor"] <= 3: > + return drbd_cmdgen.DRBD83CmdGenerator(version) > + else: > + return drbd_cmdgen.DRBD84CmdGenerator(version) > + > @staticmethod > def GetUsermodeHelper(filename=_USERMODE_HELPER_FILE): > """Returns DRBD usermode_helper currently set. > @@ -198,8 +205,11 @@ class DRBD8(base.BlockDev): > if result.failed: > base.ThrowError("Can't wipe the meta device: %s", result.output) > > - result = utils.RunCmd(["drbdmeta", "--force", cls._DevPath(minor), > - "v08", dev_path, "0", "create-md"]) > + drbd_info = DRBD8Info.CreateFromFile() > + cmd_gen = cls._GetCmdGenerator(drbd_info) > + cmd = cmd_gen.GenInitMetaCmd(minor, dev_path) > + > + result = utils.RunCmd(cmd) > if result.failed: > base.ThrowError("Can't initialize meta device: %s", result.output) > > @@ -331,18 +341,7 @@ class DRBD8(base.BlockDev): > base.ThrowError("drbd%d: can't set the synchronization parameters: > %s" % > (minor, utils.CommaJoin(sync_errors))) > > - if netutils.IP6Address.IsValid(lhost): > - if not netutils.IP6Address.IsValid(rhost): > - base.ThrowError("drbd%d: can't connect ip %s to ip %s" % > - (minor, lhost, rhost)) > - family = "ipv6" > - elif netutils.IP4Address.IsValid(lhost): > - if not netutils.IP4Address.IsValid(rhost): > - base.ThrowError("drbd%d: can't connect ip %s to ip %s" % > - (minor, lhost, rhost)) > - family = "ipv4" > - else: > - base.ThrowError("drbd%d: Invalid ip %s" % (minor, lhost)) > + family = self._GetNetFamily(minor, lhost, rhost) > > cmd = self._cmd_gen.GenNetInitCmd(minor, family, lhost, lport, > rhost, rport, protocol, > @@ -367,6 +366,21 @@ class DRBD8(base.BlockDev): > except utils.RetryTimeout: > base.ThrowError("drbd%d: timeout while configuring network", minor) > > + @staticmethod > + def _GetNetFamily(minor, lhost, rhost): > + if netutils.IP6Address.IsValid(lhost): > + if not netutils.IP6Address.IsValid(rhost): > + base.ThrowError("drbd%d: can't connect ip %s to ip %s" % > + (minor, lhost, rhost)) > + return "ipv6" > + elif netutils.IP4Address.IsValid(lhost): > + if not netutils.IP4Address.IsValid(rhost): > + base.ThrowError("drbd%d: can't connect ip %s to ip %s" % > + (minor, lhost, rhost)) > + return "ipv4" > + else: > + base.ThrowError("drbd%d: Invalid ip %s" % (minor, lhost)) > + > def AddChildren(self, devices): > """Add a disk to the DRBD device. > > @@ -817,7 +831,10 @@ class DRBD8(base.BlockDev): > This fails if we don't have a local device. > > """ > - cmd = self._cmd_gen.GenDisconnectCmd(minor) > + family = self._GetNetFamily(minor, self._lhost, self._rhost) > + cmd = self._cmd_gen.GenDisconnectCmd(minor, family, > + self._lhost, self._lport, > + self._rhost, self._rport) > result = utils.RunCmd(cmd) > if result.failed: > base.ThrowError("drbd%d: can't shutdown network: %s", > @@ -832,13 +849,9 @@ class DRBD8(base.BlockDev): > """ > # FIXME: _ShutdownAll, despite being private, is used in nodemaint.py. > # That's why we can't make it an instance method, which in turn > requires > - # us to duplicate code here (from __init__). This should be proberly > fixed. > + # us to duplicate code here (from __init__). This should be properly > fixed. > drbd_info = DRBD8Info.CreateFromFile() > - if drbd_info.GetVersion()["k_minor"] <= 3: > - cmd_gen = drbd_cmdgen.DRBD83CmdGenerator(drbd_info) > - else: > - # FIXME: use proper command generator! > - cmd_gen = None > + cmd_gen = cls._GetCmdGenerator(drbd_info) > > cmd = cmd_gen.GenDownCmd(minor) > result = utils.RunCmd(cmd) > diff --git a/lib/block/drbd_cmdgen.py b/lib/block/drbd_cmdgen.py > index 0877279..35ee875 100644 > --- a/lib/block/drbd_cmdgen.py > +++ b/lib/block/drbd_cmdgen.py > @@ -28,8 +28,66 @@ from ganeti import constants > from ganeti import errors > > > -class DRBD83CmdGenerator(object): > - """Generates drbdsetup commands suited for the DRBD <= 8.3 syntax > +class BaseDRBDCmdGenerator(object): > + """Base class for DRBD command generators. > + > + This class defines the interface for the command generators and holds > shared > + code. > + > + """ > + def __init__(self, version): > + self._version = version > + > + def GenShowCmd(self, minor): > + raise NotImplementedError > + > + def GenInitMetaCmd(self, minor, meta_dev): > + raise NotImplementedError > + > + def GenLocalInitCmds(self, minor, data_dev, meta_dev, size_mb, params): > + raise NotImplementedError > + > + def GenNetInitCmd(self, minor, family, lhost, lport, rhost, rport, > protocol, > + dual_pri, hmac, secret, params): > + raise NotImplementedError > + > + def GenSyncParamsCmd(self, minor, params): > + raise NotImplementedError > + > + def GenPauseSyncCmd(self, minor): > + raise NotImplementedError > + > + def GenResumeSyncCmd(self, minor): > + raise NotImplementedError > + > + def GenPrimaryCmd(self, minor, force): > + raise NotImplementedError > + > + def GenSecondaryCmd(self, minor): > + raise NotImplementedError > + > + def GenDetachCmd(self, minor): > + raise NotImplementedError > + > + def GenDisconnectCmd(self, minor, family, lhost, lport, rhost, rport): > + raise NotImplementedError > + > + def GenDownCmd(self, minor): > + raise NotImplementedError > + > + def GenResizeCmd(self, minor, size_mb): > + raise NotImplementedError > + > + @staticmethod > + def _DevPath(minor): > + """Return the path to a drbd device for a given minor. > + > + """ > + return "/dev/drbd%d" % minor > + > + > +class DRBD83CmdGenerator(BaseDRBDCmdGenerator): > + """Generates drbdsetup commands suited for the DRBD <= 8.3 syntax. > > """ > # command line options for barriers > @@ -38,12 +96,16 @@ class DRBD83CmdGenerator(object): > _DISABLE_FLUSH_OPTION = "--no-disk-flushes" # -i > _DISABLE_META_FLUSH_OPTION = "--no-md-flushes" # -m > > - def __init__(self, drbd_info): > - self._drbd_info = drbd_info > + def __init__(self, version): > + super(DRBD83CmdGenerator, self).__init__(version) > > def GenShowCmd(self, minor): > return ["drbdsetup", self._DevPath(minor), "show"] > > + def GenInitMetaCmd(self, minor, meta_dev): > + return ["drbdmeta", "--force", self._DevPath(minor), > + "v08", meta_dev, "0", "create-md"] > + > def GenLocalInitCmds(self, minor, data_dev, meta_dev, size_mb, params): > args = ["drbdsetup", self._DevPath(minor), "disk", > data_dev, meta_dev, "0", > @@ -52,10 +114,9 @@ class DRBD83CmdGenerator(object): > if size_mb: > args.extend(["-d", "%sm" % size_mb]) > > - version = self._drbd_info.GetVersion() > - vmaj = version["k_major"] > - vmin = version["k_minor"] > - vrel = version["k_point"] > + vmaj = self._version["k_major"] > + vmin = self._version["k_minor"] > + vrel = self._version["k_point"] > > barrier_args = \ > self._ComputeDiskBarrierArgs(vmaj, vmin, vrel, > @@ -90,9 +151,8 @@ class DRBD83CmdGenerator(object): > def GenSyncParamsCmd(self, minor, params): > args = ["drbdsetup", self._DevPath(minor), "syncer"] > if params[constants.LDP_DYNAMIC_RESYNC]: > - version = self._drbd_info.GetVersion() > - vmin = version["k_minor"] > - vrel = version["k_point"] > + vmin = self._version["k_minor"] > + vrel = self._version["k_point"] > > # By definition we are using 8.x, so just check the rest of the > version > # number > @@ -144,7 +204,7 @@ class DRBD83CmdGenerator(object): > def GenDetachCmd(self, minor): > return ["drbdsetup", self._DevPath(minor), "detach"] > > - def GenDisconnectCmd(self, minor): > + def GenDisconnectCmd(self, minor, family, lhost, lport, rhost, rport): > return ["drbdsetup", self._DevPath(minor), "disconnect"] > > def GenDownCmd(self, minor): > @@ -153,13 +213,6 @@ class DRBD83CmdGenerator(object): > def GenResizeCmd(self, minor, size_mb): > return ["drbdsetup", self._DevPath(minor), "resize", "-s", "%dm" % > size_mb] > > - @staticmethod > - def _DevPath(minor): > - """Return the path to a drbd device for a given minor. > - > - """ > - return "/dev/drbd%d" % minor > - > @classmethod > def _ComputeDiskBarrierArgs(cls, vmaj, vmin, vrel, disabled_barriers, > disable_meta_flush): > @@ -233,3 +286,166 @@ class DRBD83CmdGenerator(object): > disk_barriers_supported.get(vmin, None)) > > return args > + > + > +class DRBD84CmdGenerator(BaseDRBDCmdGenerator): > + """Generates drbdsetup commands suited for the DRBD >= 8.4 syntax. > + > + """ > + # command line options for barriers > + _DISABLE_DISK_OPTION = "--disk-barrier=no" > + _DISABLE_DRAIN_OPTION = "--disk-drain=no" > + _DISABLE_FLUSH_OPTION = "--disk-flushes=no" > + _DISABLE_META_FLUSH_OPTION = "--md-flushes=no" > + > + def __init__(self, version): > + super(DRBD84CmdGenerator, self).__init__(version) > + > + def GenShowCmd(self, minor): > + return ["drbdsetup", "show", minor] > + > + def GenInitMetaCmd(self, minor, meta_dev): > + return ["drbdmeta", "--force", self._DevPath(minor), > + "v08", meta_dev, "0", "create-md"] > + > + def GenLocalInitCmds(self, minor, data_dev, meta_dev, size_mb, params): > + cmds = [] > + > + cmds.append(["drbdsetup", "new-resource", self._GetResource(minor)]) > + cmds.append(["drbdsetup", "new-minor", self._GetResource(minor), > + str(minor), "0"]) > + # We need to apply the activity log before attaching the disk else > drbdsetup > + # will fail. > + cmds.append(["drbdmeta", self._DevPath(minor), > + "v08", meta_dev, "0", "apply-al"]) > + > + attach_cmd = ["drbdsetup", "attach", minor, data_dev, meta_dev, "0", > + "--on-io-error=detach"] > + if size_mb: > + attach_cmd.extend(["--size", "%sm" % size_mb]) > + > + barrier_args = \ > + self._ComputeDiskBarrierArgs(params[constants.LDP_BARRIERS], > + params[constants.LDP_NO_META_FLUSH]) > + attach_cmd.extend(barrier_args) > + > + if params[constants.LDP_DISK_CUSTOM]: > + attach_cmd.extend(shlex.split(params[constants.LDP_DISK_CUSTOM])) > + > + cmds.append(attach_cmd) > + > + return cmds > + > + def GenNetInitCmd(self, minor, family, lhost, lport, rhost, rport, > protocol, > + dual_pri, hmac, secret, params): > + args = ["drbdsetup", "connect", self._GetResource(minor), > + "%s:%s:%s" % (family, lhost, lport), > + "%s:%s:%s" % (family, rhost, rport), > + "--protocol", protocol, > + "--after-sb-0pri", "discard-zero-changes", > + "--after-sb-1pri", "consensus" > + ] > + if dual_pri: > + args.append("--allow-two-primaries") > + if hmac and secret: > + args.extend(["--cram-hmac-alg", hmac, "--shared-secret", secret]) > + > + if params[constants.LDP_NET_CUSTOM]: > + args.extend(shlex.split(params[constants.LDP_NET_CUSTOM])) > + > + return args > + > + def GenSyncParamsCmd(self, minor, params): > + args = ["drbdsetup", "disk-options", minor] > + if params[constants.LDP_DYNAMIC_RESYNC]: > + if params[constants.LDP_PLAN_AHEAD] == 0: > + msg = ("A value of 0 for c-plan-ahead disables the dynamic sync > speed" > + " controller at DRBD level. If you want to disable it, > please" > + " set the dynamic-resync disk parameter to False.") > + logging.error(msg) > + return [msg] > + > + # add the c-* parameters to args > + args.extend(["--c-plan-ahead", params[constants.LDP_PLAN_AHEAD], > + "--c-fill-target", params[constants.LDP_FILL_TARGET], > + "--c-delay-target", params[constants.LDP_DELAY_TARGET], > + "--c-max-rate", params[constants.LDP_MAX_RATE], > + "--c-min-rate", params[constants.LDP_MIN_RATE], > + ]) > + > + else: > + args.extend(["--resync-rate", "%d" % > params[constants.LDP_RESYNC_RATE]]) > + > + return args > + > + def GenPauseSyncCmd(self, minor): > + return ["drbdsetup", "pause-sync", minor] > + > + def GenResumeSyncCmd(self, minor): > + return ["drbdsetup", "resume-sync", minor] > + > + def GenPrimaryCmd(self, minor, force): > + cmd = ["drbdsetup", "primary", minor] > + > + if force: > + cmd.append("--force") > + > + return cmd > + > + def GenSecondaryCmd(self, minor): > + return ["drbdsetup", "secondary", minor] > + > + def GenDetachCmd(self, minor): > + return ["drbdsetup", "detach", minor] > + > + def GenDisconnectCmd(self, minor, family, lhost, lport, rhost, rport): > + return ["drbdsetup", "disconnect", > + "%s:%s:%s" % (family, lhost, lport), > + "%s:%s:%s" % (family, rhost, rport)] > + > + def GenDownCmd(self, minor): > + return ["drbdsetup", "down", self._GetResource(minor)] > + > + def GenResizeCmd(self, minor, size_mb): > + return ["drbdsetup", "resize", minor, "--size", "%dm" % size_mb] > + > + @staticmethod > + def _GetResource(minor): > + """Return the resource name for a given minor. > + > + Currently we don't support DRBD volumes which share a resource, so we > + generate the resource name based on the minor the resulting volumes is > + assigned to. > + > + """ > + return "resource%d" % minor > + > + @classmethod > + def _ComputeDiskBarrierArgs(cls, disabled_barriers, disable_meta_flush): > + """Compute the DRBD command line parameters for disk barriers > + > + """ > + 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 = [] > + > + # meta flushes > + if disable_meta_flush: > + args.append(cls._DISABLE_META_FLUSH_OPTION) > + > + # disk flushes > + if constants.DRBD_B_DISK_FLUSH in disabled_barriers_set: > + args.append(cls._DISABLE_FLUSH_OPTION) > + > + # disk drain > + if constants.DRBD_B_DISK_DRAIN in disabled_barriers_set: > + args.append(cls._DISABLE_DRAIN_OPTION) > + > + # disk barriers > + if constants.DRBD_B_DISK_BARRIERS in disabled_barriers_set: > + args.append(cls._DISABLE_DISK_OPTION) > + > + return args > diff --git a/test/py/ganeti.block.drbd_unittest.py b/test/py/ > ganeti.block.drbd_unittest.py > index 87b3d03..112f5b1 100755 > --- a/test/py/ganeti.block.drbd_unittest.py > +++ b/test/py/ganeti.block.drbd_unittest.py > @@ -413,8 +413,7 @@ class TestDRBD8Construction(testutils.GanetiTestCase): > > inst = drbd.DRBD8(self.test_unique_id, [], 123, {}) > self.assertEqual(inst._show_info_cls, drbd_info.DRBD84ShowInfo) > - # FIXME: add assertion for right class here! > - self.assertEqual(inst._cmd_gen, None) > + self.assertTrue(isinstance(inst._cmd_gen, > drbd_cmdgen.DRBD84CmdGenerator)) > > > if __name__ == "__main__": > -- > 1.8.2.1 > > LGTM, thanks. Michele
