On Mon, Apr 29, 2013 at 12:28 PM, Thomas Thrainer <[email protected]>wrote:
> > > > On Mon, Apr 29, 2013 at 11:50 AM, Michele Tartara <[email protected]>wrote: > >> On Wed, Apr 24, 2013 at 4:59 PM, Thomas Thrainer <[email protected]>wrote: >> >>> Common functionality between the DRBD 8.3 and DRBD 8.4 parser has been >>> extracted into BaseShowInfo. A test which verifies the behaviour is >>> included, but the DRBD84ShowInfo class is not yet used within the DRBD8 >>> class. >>> >>> Signed-off-by: Thomas Thrainer <[email protected]> >>> --- >>> Makefile.am | 1 + >>> lib/block/drbd.py | 19 ++-- >>> lib/block/drbd_info.py | 194 >>> +++++++++++++++++++++++----------- >>> test/data/bdev-drbd-8.4.txt | 19 ++++ >>> test/py/ganeti.block.bdev_unittest.py | 31 ++++-- >>> 5 files changed, 185 insertions(+), 79 deletions(-) >>> create mode 100644 test/data/bdev-drbd-8.4.txt >>> >>> diff --git a/Makefile.am b/Makefile.am >>> index 23a138f..abf9285 100644 >>> --- a/Makefile.am >>> +++ b/Makefile.am >>> @@ -1035,6 +1035,7 @@ TEST_FILES = \ >>> test/hs/shelltests/htools-mon-collector.test \ >>> test/data/bdev-drbd-8.0.txt \ >>> test/data/bdev-drbd-8.3.txt \ >>> + test/data/bdev-drbd-8.4.txt \ >>> test/data/bdev-drbd-disk.txt \ >>> test/data/bdev-drbd-net-ip4.txt \ >>> test/data/bdev-drbd-net-ip6.txt \ >>> diff --git a/lib/block/drbd.py b/lib/block/drbd.py >>> index 3442199..5889768 100644 >>> --- a/lib/block/drbd.py >>> +++ b/lib/block/drbd.py >>> @@ -33,7 +33,7 @@ from ganeti import netutils >>> from ganeti import objects >>> from ganeti.block import base >>> from ganeti.block.drbd_info import DRBD8Info >>> -from ganeti.block.drbd_info import DRBD8ShowInfo >>> +from ganeti.block.drbd_info import DRBD83ShowInfo >>> >>> >>> # Size of reads in _CanReadDevice >>> @@ -452,7 +452,7 @@ class DRBD8(base.BlockDev): >>> minor, result.fail_reason, result.output) >>> >>> def _CheckNetworkConfig(): >>> - info = DRBD8ShowInfo.GetDevInfo(self._GetShowData(minor)) >>> + info = DRBD83ShowInfo.GetDevInfo(self._GetShowData(minor)) >>> if not "local_addr" in info or not "remote_addr" in info: >>> raise utils.RetryAgain() >>> >>> @@ -474,7 +474,7 @@ class DRBD8(base.BlockDev): >>> self._aminor) >>> if len(devices) != 2: >>> base.ThrowError("drbd%d: need two devices for AddChildren", >>> self.minor) >>> - info = DRBD8ShowInfo.GetDevInfo(self._GetShowData(self.minor)) >>> + info = DRBD83ShowInfo.GetDevInfo(self._GetShowData(self.minor)) >>> if "local_dev" in info: >>> base.ThrowError("drbd%d: already attached to a local disk", >>> self.minor) >>> backend, meta = devices >>> @@ -497,7 +497,7 @@ class DRBD8(base.BlockDev): >>> base.ThrowError("drbd%d: can't attach to drbd8 during >>> RemoveChildren", >>> self._aminor) >>> # early return if we don't actually have backing storage >>> - info = DRBD8ShowInfo.GetDevInfo(self._GetShowData(self.minor)) >>> + info = DRBD83ShowInfo.GetDevInfo(self._GetShowData(self.minor)) >>> if "local_dev" not in info: >>> return >>> if len(self._children) != 2: >>> @@ -849,7 +849,7 @@ class DRBD8(base.BlockDev): >>> # pylint: disable=W0631 >>> net_data = (self._lhost, self._lport, self._rhost, self._rport) >>> for minor in (self._aminor,): >>> - info = DRBD8ShowInfo.GetDevInfo(self._GetShowData(minor)) >>> + info = DRBD83ShowInfo.GetDevInfo(self._GetShowData(minor)) >>> match_l = self._MatchesLocal(info) >>> match_r = self._MatchesNet(info) >>> >>> @@ -861,7 +861,8 @@ class DRBD8(base.BlockDev): >>> # disk matches, but not attached to network, attach and recheck >>> self._AssembleNet(minor, net_data, constants.DRBD_NET_PROTOCOL, >>> hmac=constants.DRBD_HMAC_ALG, >>> secret=self._secret) >>> - if >>> self._MatchesNet(DRBD8ShowInfo.GetDevInfo(self._GetShowData(minor))): >>> + if self._MatchesNet(DRBD83ShowInfo.GetDevInfo( >>> + self._GetShowData(minor))): >>> break >>> else: >>> base.ThrowError("drbd%d: network attach successful, but >>> 'drbdsetup" >>> @@ -871,7 +872,8 @@ class DRBD8(base.BlockDev): >>> # no local disk, but network attached and it matches >>> self._AssembleLocal(minor, self._children[0].dev_path, >>> self._children[1].dev_path, self.size) >>> - if >>> self._MatchesNet(DRBD8ShowInfo.GetDevInfo(self._GetShowData(minor))): >>> + if self._MatchesNet(DRBD83ShowInfo.GetDevInfo( >>> + self._GetShowData(minor))): >>> break >>> else: >>> base.ThrowError("drbd%d: disk attach successful, but >>> 'drbdsetup" >>> @@ -898,7 +900,8 @@ class DRBD8(base.BlockDev): >>> # None) >>> self._AssembleNet(minor, net_data, constants.DRBD_NET_PROTOCOL, >>> hmac=constants.DRBD_HMAC_ALG, >>> secret=self._secret) >>> - if >>> self._MatchesNet(DRBD8ShowInfo.GetDevInfo(self._GetShowData(minor))): >>> + if self._MatchesNet(DRBD83ShowInfo.GetDevInfo( >>> + self._GetShowData(minor))): >>> break >>> else: >>> base.ThrowError("drbd%d: network attach successful, but >>> 'drbdsetup" >>> diff --git a/lib/block/drbd_info.py b/lib/block/drbd_info.py >>> index 4d1e14d..991494c 100644 >>> --- a/lib/block/drbd_info.py >>> +++ b/lib/block/drbd_info.py >>> @@ -255,69 +255,50 @@ class DRBD8Info(object): >>> return DRBD8Info.CreateFromLines(lines) >>> >>> >>> -class DRBD8ShowInfo(object): >>> - """Helper class which parses the output of drbdsetup show >>> +class BaseShowInfo(object): >>> + """Base class for parsing the `drbdsetup show` output. >>> + >>> + Holds various common pyparsing expressions which are used by >>> subclasses. Also >>> + provides caching of the constructed parser. >>> >>> """ >>> _PARSE_SHOW = None >>> >>> - @classmethod >>> - def _GetShowParser(cls): >>> - """Return a parser for `drbd show` output. >>> - >>> - This will either create or return an already-created parser for the >>> - output of the command `drbd show`. >>> - >>> - """ >>> - if cls._PARSE_SHOW is not None: >>> - return cls._PARSE_SHOW >>> - >>> - # pyparsing setup >>> - lbrace = pyp.Literal("{").suppress() >>> - rbrace = pyp.Literal("}").suppress() >>> - lbracket = pyp.Literal("[").suppress() >>> - rbracket = pyp.Literal("]").suppress() >>> - semi = pyp.Literal(";").suppress() >>> - colon = pyp.Literal(":").suppress() >>> - # this also converts the value to an int >>> - number = pyp.Word(pyp.nums).setParseAction(lambda s, l, t: >>> int(t[0])) >>> - >>> - comment = pyp.Literal("#") + pyp.Optional(pyp.restOfLine) >>> - defa = pyp.Literal("_is_default").suppress() >>> - dbl_quote = pyp.Literal('"').suppress() >>> - >>> - keyword = pyp.Word(pyp.alphanums + "-") >>> - >>> - # value types >>> - value = pyp.Word(pyp.alphanums + "_-/.:") >>> - quoted = dbl_quote + pyp.CharsNotIn('"') + dbl_quote >>> - ipv4_addr = (pyp.Optional(pyp.Literal("ipv4")).suppress() + >>> - pyp.Word(pyp.nums + ".") + colon + number) >>> - ipv6_addr = (pyp.Optional(pyp.Literal("ipv6")).suppress() + >>> - pyp.Optional(lbracket) + pyp.Word(pyp.hexnums + ":") + >>> - pyp.Optional(rbracket) + colon + number) >>> - # meta device, extended syntax >>> - meta_value = ((value ^ quoted) + lbracket + number + rbracket) >>> - # device name, extended syntax >>> - device_value = pyp.Literal("minor").suppress() + number >>> - >>> - # a statement >>> - stmt = (~rbrace + keyword + ~lbrace + >>> - pyp.Optional(ipv4_addr ^ ipv6_addr ^ value ^ quoted ^ >>> meta_value ^ >>> - device_value) + >>> - pyp.Optional(defa) + semi + >>> - pyp.Optional(pyp.restOfLine).suppress()) >>> - >>> - # an entire section >>> - section_name = pyp.Word(pyp.alphas + "_") >>> - section = section_name + lbrace + pyp.ZeroOrMore(pyp.Group(stmt)) + >>> rbrace >>> - >>> - bnf = pyp.ZeroOrMore(pyp.Group(section ^ stmt)) >>> - bnf.ignore(comment) >>> - >>> - cls._PARSE_SHOW = bnf >>> - >>> - return bnf >>> + # pyparsing setup >>> + _lbrace = pyp.Literal("{").suppress() >>> + _rbrace = pyp.Literal("}").suppress() >>> + _lbracket = pyp.Literal("[").suppress() >>> + _rbracket = pyp.Literal("]").suppress() >>> + _semi = pyp.Literal(";").suppress() >>> + _colon = pyp.Literal(":").suppress() >>> + # this also converts the value to an int >>> + _number = pyp.Word(pyp.nums).setParseAction(lambda s, l, t: int(t[0])) >>> + >>> + _comment = pyp.Literal("#") + pyp.Optional(pyp.restOfLine) >>> + _defa = pyp.Literal("_is_default").suppress() >>> + _dbl_quote = pyp.Literal('"').suppress() >>> + >>> + _keyword = pyp.Word(pyp.alphanums + "-") >>> + >>> + # value types >>> + _value = pyp.Word(pyp.alphanums + "_-/.:") >>> + _quoted = _dbl_quote + pyp.CharsNotIn('"') + _dbl_quote >>> + _ipv4_addr = (pyp.Optional(pyp.Literal("ipv4")).suppress() + >>> + pyp.Word(pyp.nums + ".") + _colon + _number) >>> + _ipv6_addr = (pyp.Optional(pyp.Literal("ipv6")).suppress() + >>> + pyp.Optional(_lbracket) + pyp.Word(pyp.hexnums + ":") + >>> + pyp.Optional(_rbracket) + _colon + _number) >>> + # meta device, extended syntax >>> + _meta_value = ((_value ^ _quoted) + _lbracket + _number + _rbracket) >>> + # device name, extended syntax >>> + _device_value = pyp.Literal("minor").suppress() + _number >>> + >>> + # a statement >>> + _stmt = (~_rbrace + _keyword + ~_lbrace + >>> + pyp.Optional(_ipv4_addr ^ _ipv6_addr ^ _value ^ _quoted ^ >>> + _meta_value ^ _device_value) + >>> + pyp.Optional(_defa) + _semi + >>> + pyp.Optional(pyp.restOfLine).suppress()) >>> >>> @classmethod >>> def GetDevInfo(cls, show_data): >>> @@ -336,9 +317,8 @@ class DRBD8ShowInfo(object): >>> - remote_addr >>> >>> """ >>> - retval = {} >>> if not show_data: >>> - return retval >>> + return {} >>> >>> try: >>> # run pyparse >>> @@ -346,8 +326,49 @@ class DRBD8ShowInfo(object): >>> except pyp.ParseException, err: >>> base.ThrowError("Can't parse drbdsetup show output: %s", str(err)) >>> >>> - # and massage the results into our desired format >>> - for section in results: >>> + return cls._TransformParseResult(results) >>> + >>> + @classmethod >>> + def _TransformParseResult(cls, parse_result): >>> + raise NotImplementedError >>> + >>> + @classmethod >>> + def _GetShowParser(cls): >>> + """Return a parser for `drbd show` output. >>> + >>> + This will either create or return an already-created parser for the >>> + output of the command `drbd show`. >>> + >>> + """ >>> + if cls._PARSE_SHOW is None: >>> + cls._PARSE_SHOW = cls._ConstructShowParser() >>> + >>> + return cls._PARSE_SHOW >>> + >>> + @classmethod >>> + def _ConstructShowParser(cls): >>> + raise NotImplementedError >>> + >>> + >>> +class DRBD83ShowInfo(BaseShowInfo): >>> + @classmethod >>> + def _ConstructShowParser(cls): >>> + # an entire section >>> + section_name = pyp.Word(pyp.alphas + "_") >>> + section = section_name + \ >>> + cls._lbrace + \ >>> + pyp.ZeroOrMore(pyp.Group(cls._stmt)) + \ >>> + cls._rbrace >>> + >>> + bnf = pyp.ZeroOrMore(pyp.Group(section ^ cls._stmt)) >>> + bnf.ignore(cls._comment) >>> + >>> + return bnf >>> + >>> + @classmethod >>> + def _TransformParseResult(cls, parse_result): >>> + retval = {} >>> + for section in parse_result: >>> sname = section[0] >>> if sname == "_this_host": >>> for lst in section[1:]: >>> @@ -363,3 +384,50 @@ class DRBD8ShowInfo(object): >>> if lst[0] == "address": >>> retval["remote_addr"] = tuple(lst[1:]) >>> return retval >>> + >>> + >>> +class DRBD84ShowInfo(BaseShowInfo): >>> + @classmethod >>> + def _ConstructShowParser(cls): >>> + # an entire section (sections can be nested in DRBD 8.4, and there >>> exist >>> + # sections like "volume 0") >>> + section_name = pyp.Word(pyp.alphas + "_") + \ >>> + pyp.Optional(pyp.Word(pyp.nums)).suppress() # skip >>> volume idx >>> + section = pyp.Forward() >>> + # pylint: disable=W0106 >>> >> >> Why are you disabling this warning? >> > > This expression is indeed assigned to nothing (that's what the warning > checks), but this is still the way to assign an actual parser to a forward > parser (according to the pyparsing docs). > Is there a better way to do this without triggering the warning? > > If that's a pyparsing specific, then it's ok. Just please add a short comment line explaining it. >> >>> + section << (section_name + >>> + cls._lbrace + >>> + pyp.ZeroOrMore(pyp.Group(cls._stmt ^ section)) + >>> + cls._rbrace) >>> + >>> + resource_name = pyp.Word(pyp.alphanums + "_-.") >>> + resource = (pyp.Literal("resource") + resource_name).suppress() + \ >>> + cls._lbrace + \ >>> + pyp.ZeroOrMore(pyp.Group(section)) + \ >>> + cls._rbrace >>> + >>> + resource.ignore(cls._comment) >>> + >>> + return resource >>> + >>> + @classmethod >>> + def _TransformParseResult(cls, parse_result): >>> + retval = {} >>> + for section in parse_result: >>> + sname = section[0] >>> + if sname == "_this_host": >>> + for lst in section[1:]: >>> + if lst[0] == "address": >>> + retval["local_addr"] = tuple(lst[1:]) >>> + elif lst[0] == "volume": >>> + for inner in lst[1:]: >>> + if inner[0] == "disk" and len(inner) == 2: >>> + retval["local_dev"] = inner[1] >>> + elif inner[0] == "meta-disk" and len(inner) == 3: >>> + retval["meta_dev"] = inner[1] >>> + retval["meta_index"] = inner[2] >>> + elif sname == "_remote_host": >>> + for lst in section[1:]: >>> + if lst[0] == "address": >>> + retval["remote_addr"] = tuple(lst[1:]) >>> + return retval >>> diff --git a/test/data/bdev-drbd-8.4.txt b/test/data/bdev-drbd-8.4.txt >>> new file mode 100644 >>> index 0000000..024d5c7 >>> --- /dev/null >>> +++ b/test/data/bdev-drbd-8.4.txt >>> @@ -0,0 +1,19 @@ >>> +resource test0 { >>> + options { >>> + } >>> + net { >>> + } >>> + _remote_host { >>> + address ipv4 192.0.2.2:11000; >>> + } >>> + _this_host { >>> + address ipv4 192.0.2.1:11000; >>> + volume 0 { >>> + device minor 0; >>> + disk "/dev/xenvg/test.data"; >>> + meta-disk "/dev/xenvg/test.meta" [ 0 ]; >>> + disk { >>> + } >>> + } >>> + } >>> +} >>> diff --git a/test/py/ganeti.block.bdev_unittest.py b/test/py/ >>> ganeti.block.bdev_unittest.py >>> index 8f65483..c836432 100755 >>> --- a/test/py/ganeti.block.bdev_unittest.py >>> +++ b/test/py/ganeti.block.bdev_unittest.py >>> @@ -103,14 +103,18 @@ class TestDRBD8Runner(testutils.GanetiTestCase): >>> ) >>> return retval >>> >>> - def testParserCreation(self): >>> + def testParser83Creation(self): >>> """Test drbdsetup show parser creation""" >>> - drbd_info.DRBD8ShowInfo._GetShowParser() >>> + drbd_info.DRBD83ShowInfo._GetShowParser() >>> + >>> + def testParser84Creation(self): >>> + """Test drbdsetup show parser creation""" >>> + drbd_info.DRBD84ShowInfo._GetShowParser() >>> >>> def testParser80(self): >>> """Test drbdsetup show parser for disk and network version 8.0""" >>> data = testutils.ReadTestData("bdev-drbd-8.0.txt") >>> - result = drbd_info.DRBD8ShowInfo.GetDevInfo(data) >>> + result = drbd_info.DRBD83ShowInfo.GetDevInfo(data)pylin >>> self.failUnless(self._has_disk(result, "/dev/xenvg/test.data", >>> "/dev/xenvg/test.meta"), >>> "Wrong local disk info") >>> @@ -121,18 +125,29 @@ class TestDRBD8Runner(testutils.GanetiTestCase): >>> def testParser83(self): >>> """Test drbdsetup show parser for disk and network version 8.3""" >>> data = testutils.ReadTestData("bdev-drbd-8.3.txt") >>> - result = drbd_info.DRBD8ShowInfo.GetDevInfo(data) >>> + result = drbd_info.DRBD83ShowInfo.GetDevInfo(data) >>> self.failUnless(self._has_disk(result, "/dev/xenvg/test.data", >>> "/dev/xenvg/test.meta"), >>> "Wrong local disk info") >>> self.failUnless(self._has_net(result, ("192.0.2.1", 11000), >>> ("192.0.2.2", 11000)), >>> - "Wrong network info (8.0.x)") >>> + "Wrong network info (8.3.x)") >>> + >>> + def testParser84(self): >>> + """Test drbdsetup show parser for disk and network version 8.4""" >>> + data = testutils.ReadTestData("bdev-drbd-8.4.txt") >>> + result = drbd_info.DRBD84ShowInfo.GetDevInfo(data) >>> + self.failUnless(self._has_disk(result, "/dev/xenvg/test.data", >>> + "/dev/xenvg/test.meta"), >>> + "Wrong local disk info") >>> + self.failUnless(self._has_net(result, ("192.0.2.1", 11000), >>> + ("192.0.2.2", 11000)), >>> + "Wrong network info (8.4.x)") >>> >>> def testParserNetIP4(self): >>> """Test drbdsetup show parser for IPv4 network""" >>> data = testutils.ReadTestData("bdev-drbd-net-ip4.txt") >>> - result = drbd_info.DRBD8ShowInfo.GetDevInfo(data) >>> + result = drbd_info.DRBD83ShowInfo.GetDevInfo(data) >>> self.failUnless(("local_dev" not in result and >>> "meta_dev" not in result and >>> "meta_index" not in result), >>> @@ -144,7 +159,7 @@ class TestDRBD8Runner(testutils.GanetiTestCase): >>> def testParserNetIP6(self): >>> """Test drbdsetup show parser for IPv6 network""" >>> data = testutils.ReadTestData("bdev-drbd-net-ip6.txt") >>> - result = drbd_info.DRBD8ShowInfo.GetDevInfo(data) >>> + result = drbd_info.DRBD83ShowInfo.GetDevInfo(data) >>> self.failUnless(("local_dev" not in result and >>> "meta_dev" not in result and >>> "meta_index" not in result), >>> @@ -156,7 +171,7 @@ class TestDRBD8Runner(testutils.GanetiTestCase): >>> def testParserDisk(self): >>> """Test drbdsetup show parser for disk""" >>> data = testutils.ReadTestData("bdev-drbd-disk.txt") >>> - result = drbd_info.DRBD8ShowInfo.GetDevInfo(data) >>> + result = drbd_info.DRBD83ShowInfo.GetDevInfo(data) >>> self.failUnless(self._has_disk(result, "/dev/xenvg/test.data", >>> "/dev/xenvg/test.meta"), >>> "Wrong local disk info") >>> -- >>> 1.8.2.1 >>> >>> >> Rest, LGTM. >> >> Thanks, >> Michele >> > > > > -- > Thomas Thrainer | Software Engineer | [email protected] | > > Google Germany GmbH > Dienerstr. 12 > 80331 München > > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg > Geschäftsführer: Graham Law, Katherine Stephens > Thanks, Michele
