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? > > >> + 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) >> 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
