Extract /proc/drbd parsing code into DRBD8Info
authorThomas Thrainer <thomasth@google.com>
Mon, 29 Apr 2013 11:51:19 +0000 (13:51 +0200)
committerMichele Tartara <mtartara@google.com>
Mon, 29 Apr 2013 16:05:20 +0000 (16:05 +0000)
As the DRBD8 class got bigger due to the previous merge of BaseDRBD, now
parts of it are ripped out into DRBD8Info. This new class parses
/proc/drbd and exposes the information in an easily accessible way. This
allowed to simplify some methods in DRBD8 and do make the tests more
concise.

Signed-off-by: Thomas Thrainer <thomasth@google.com>
Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

lib/backend.py
lib/block/drbd.py
lib/watcher/nodemaint.py
test/data/proc_drbd80-emptyline.txt
test/hs/Test/Ganeti/Block/Drbd/Parser.hs
test/py/ganeti.block.bdev_unittest.py

index 5c83471..d7ed407 100644 (file)
@@ -832,7 +832,7 @@ def VerifyNode(what, cluster_name):
 
   if constants.NV_DRBDLIST in what and vm_capable:
     try:
-      used_minors = drbd.DRBD8.GetUsedDevs().keys()
+      used_minors = drbd.DRBD8.GetUsedDevs()
     except errors.BlockDeviceError, err:
       logging.warning("Can't get used minors list", exc_info=True)
       used_minors = str(err)
index 92e5b81..f7667a1 100644 (file)
@@ -42,11 +42,11 @@ from ganeti.block import base
 _DEVICE_READ_SIZE = 128 * 1024
 
 
-class DRBD8Status(object):
+class DRBD8Status(object): # pylint: disable=R0902
   """A DRBD status representation class.
 
   Note that this class is meant to be used to parse one of the entries returned
-  from L{DRBD8._JoinProcDataPerMinor}.
+  from L{DRBD8Info._JoinProcDataPerMinor}.
 
   """
   UNCONF_RE = re.compile(r"\s*[0-9]+:\s*cs:Unconfigured$")
@@ -119,6 +119,7 @@ class DRBD8Status(object):
     self.is_standalone = self.cstatus == self.CS_STANDALONE
     self.is_wfconn = self.cstatus == self.CS_WFCONNECTION
     self.is_connected = self.cstatus == self.CS_CONNECTED
+    self.is_unconfigured = self.cstatus == self.CS_UNCONFIGURED
     self.is_primary = self.lrole == self.RO_PRIMARY
     self.is_secondary = self.lrole == self.RO_SECONDARY
     self.peer_primary = self.rrole == self.RO_PRIMARY
@@ -151,6 +152,119 @@ class DRBD8Status(object):
       self.est_time = None
 
 
+class DRBD8Info(object):
+  """Represents information DRBD exports (usually via /proc/drbd).
+
+  An instance of this class is created by one of the CreateFrom... methods.
+
+  """
+
+  _VERSION_RE = re.compile(r"^version: (\d+)\.(\d+)\.(\d+)(?:\.\d+)?"
+                           r" \(api:(\d+)/proto:(\d+)(?:-(\d+))?\)")
+  _VALID_LINE_RE = re.compile("^ *([0-9]+): cs:([^ ]+).*$")
+
+  def __init__(self, lines):
+    self._version = self._ParseVersion(lines)
+    self._minors, self._line_per_minor = self._JoinProcDataPerMinor(lines)
+
+  def GetVersion(self):
+    """Return the DRBD version.
+
+    This will return a dict with keys:
+      - k_major
+      - k_minor
+      - k_point
+      - api
+      - proto
+      - proto2 (only on drbd > 8.2.X)
+
+    """
+    return self._version
+
+  def GetMinors(self):
+    """Return a list of minor for which information is available.
+
+    This list is ordered in exactly the order which was found in the underlying
+    data.
+
+    """
+    return self._minors
+
+  def HasMinorStatus(self, minor):
+    return minor in self._line_per_minor
+
+  def GetMinorStatus(self, minor):
+    return DRBD8Status(self._line_per_minor[minor])
+
+  def _ParseVersion(self, lines):
+    first_line = lines[0].strip()
+    version = self._VERSION_RE.match(first_line)
+    if not version:
+      raise errors.BlockDeviceError("Can't parse DRBD version from '%s'" %
+                                    first_line)
+
+    values = version.groups()
+    retval = {
+      "k_major": int(values[0]),
+      "k_minor": int(values[1]),
+      "k_point": int(values[2]),
+      "api": int(values[3]),
+      "proto": int(values[4]),
+      }
+    if values[5] is not None:
+      retval["proto2"] = values[5]
+
+    return retval
+
+  def _JoinProcDataPerMinor(self, lines):
+    """Transform the raw lines into a dictionary based on the minor.
+
+    @return: a dictionary of minor: joined lines from /proc/drbd
+        for that minor
+
+    """
+    minors = []
+    results = {}
+    old_minor = old_line = None
+    for line in lines:
+      if not line: # completely empty lines, as can be returned by drbd8.0+
+        continue
+      lresult = self._VALID_LINE_RE.match(line)
+      if lresult is not None:
+        if old_minor is not None:
+          minors.append(old_minor)
+          results[old_minor] = old_line
+        old_minor = int(lresult.group(1))
+        old_line = line
+      else:
+        if old_minor is not None:
+          old_line += " " + line.strip()
+    # add last line
+    if old_minor is not None:
+      minors.append(old_minor)
+      results[old_minor] = old_line
+    return minors, results
+
+  @staticmethod
+  def CreateFromLines(lines):
+    return DRBD8Info(lines)
+
+  @staticmethod
+  def CreateFromFile(filename=constants.DRBD_STATUS_FILE):
+    try:
+      lines = utils.ReadFile(filename).splitlines()
+    except EnvironmentError, err:
+      if err.errno == errno.ENOENT:
+        base.ThrowError("The file %s cannot be opened, check if the module"
+                        " is loaded (%s)", filename, str(err))
+      else:
+        base.ThrowError("Can't read the DRBD proc file %s: %s",
+                        filename, str(err))
+    if not lines:
+      base.ThrowError("Can't read any data from %s", filename)
+    return DRBD8Info.CreateFromLines(lines)
+
+
 class DRBD8(base.BlockDev):
   """DRBD v8.x block device.
 
@@ -164,17 +278,8 @@ class DRBD8(base.BlockDev):
   is checked for valid size and is zeroed on create.
 
   """
-  _VERSION_RE = re.compile(r"^version: (\d+)\.(\d+)\.(\d+)(?:\.\d+)?"
-                           r" \(api:(\d+)/proto:(\d+)(?:-(\d+))?\)")
-  _VALID_LINE_RE = re.compile("^ *([0-9]+): cs:([^ ]+).*$")
-  _UNUSED_LINE_RE = re.compile("^ *([0-9]+): cs:Unconfigured$")
-
   _DRBD_MAJOR = 147
-  _ST_UNCONFIGURED = DRBD8Status.CS_UNCONFIGURED
-  _ST_WFCONNECTION = DRBD8Status.CS_WFCONNECTION
-  _ST_CONNECTED = DRBD8Status.CS_CONNECTED
 
-  _STATUS_FILE = constants.DRBD_STATUS_FILE
   _USERMODE_HELPER_FILE = "/sys/module/drbd/parameters/usermode_helper"
 
   _MAX_MINORS = 255
@@ -205,7 +310,9 @@ class DRBD8(base.BlockDev):
         children = []
     super(DRBD8, self).__init__(unique_id, children, size, params)
     self.major = self._DRBD_MAJOR
-    version = self._GetVersion(self._GetProcData())
+
+    drbd_info = DRBD8Info.CreateFromFile()
+    version = drbd_info.GetVersion()
     if version["k_major"] != 8:
       base.ThrowError("Mismatch in DRBD kernel version and requested ganeti"
                       " usage: kernel is %s.%s, ganeti wants 8.x",
@@ -218,83 +325,6 @@ class DRBD8(base.BlockDev):
     self.Attach()
 
   @staticmethod
-  def _GetProcData(filename=_STATUS_FILE):
-    """Return data from /proc/drbd.
-
-    """
-    try:
-      data = utils.ReadFile(filename).splitlines()
-    except EnvironmentError, err:
-      if err.errno == errno.ENOENT:
-        base.ThrowError("The file %s cannot be opened, check if the module"
-                        " is loaded (%s)", filename, str(err))
-      else:
-        base.ThrowError("Can't read the DRBD proc file %s: %s",
-                        filename, str(err))
-    if not data:
-      base.ThrowError("Can't read any data from %s", filename)
-    return data
-
-  @classmethod
-  def _JoinProcDataPerMinor(cls, data):
-    """Transform the output of _GetProdData into a nicer form.
-
-    @return: a dictionary of minor: joined lines from /proc/drbd
-        for that minor
-
-    """
-    results = {}
-    old_minor = old_line = None
-    for line in data:
-      if not line: # completely empty lines, as can be returned by drbd8.0+
-        continue
-      lresult = cls._VALID_LINE_RE.match(line)
-      if lresult is not None:
-        if old_minor is not None:
-          results[old_minor] = old_line
-        old_minor = int(lresult.group(1))
-        old_line = line
-      else:
-        if old_minor is not None:
-          old_line += " " + line.strip()
-    # add last line
-    if old_minor is not None:
-      results[old_minor] = old_line
-    return results
-
-  @classmethod
-  def _GetVersion(cls, proc_data):
-    """Return the DRBD version.
-
-    This will return a dict with keys:
-      - k_major
-      - k_minor
-      - k_point
-      - api
-      - proto
-      - proto2 (only on drbd > 8.2.X)
-
-    """
-    first_line = proc_data[0].strip()
-    version = cls._VERSION_RE.match(first_line)
-    if not version:
-      raise errors.BlockDeviceError("Can't parse DRBD version from '%s'" %
-                                    first_line)
-
-    values = version.groups()
-    retval = {
-      "k_major": int(values[0]),
-      "k_minor": int(values[1]),
-      "k_point": int(values[2]),
-      "api": int(values[3]),
-      "proto": int(values[4]),
-      }
-    if values[5] is not None:
-      retval["proto2"] = values[5]
-
-    return retval
-
-  @staticmethod
   def GetUsermodeHelper(filename=_USERMODE_HELPER_FILE):
     """Returns DRBD usermode_helper currently set.
 
@@ -324,20 +354,9 @@ class DRBD8(base.BlockDev):
     """Compute the list of used DRBD devices.
 
     """
-    data = cls._GetProcData()
-
-    used_devs = {}
-    for line in data:
-      match = cls._VALID_LINE_RE.match(line)
-      if not match:
-        continue
-      minor = int(match.group(1))
-      state = match.group(2)
-      if state == cls._ST_UNCONFIGURED:
-        continue
-      used_devs[minor] = state, line
-
-    return used_devs
+    drbd_info = DRBD8Info.CreateFromFile()
+    return filter(lambda m: not drbd_info.GetMinorStatus(m).is_unconfigured,
+                  drbd_info.GetMinors())
 
   def _SetFromMinor(self, minor):
     """Set our parameters based on the given minor.
@@ -406,30 +425,28 @@ class DRBD8(base.BlockDev):
     if result.failed:
       base.ThrowError("Can't initialize meta device: %s", result.output)
 
-  @classmethod
-  def _FindUnusedMinor(cls):
+  def _FindUnusedMinor(self):
     """Find an unused DRBD device.
 
     This is specific to 8.x as the minors are allocated dynamically,
     so non-existing numbers up to a max minor count are actually free.
 
     """
-    data = cls._GetProcData()
 
     highest = None
-    for line in data:
-      match = cls._UNUSED_LINE_RE.match(line)
-      if match:
-        return int(match.group(1))
-      match = cls._VALID_LINE_RE.match(line)
-      if match:
-        minor = int(match.group(1))
-        highest = max(highest, minor)
+    drbd_info = DRBD8Info.CreateFromFile()
+    for minor in drbd_info.GetMinors():
+      status = drbd_info.GetMinorStatus(minor)
+      if not status.is_in_use:
+        return minor
+      highest = max(highest, minor)
+
     if highest is None: # there are no minors in use at all
       return 0
-    if highest >= cls._MAX_MINORS:
+    if highest >= self._MAX_MINORS:
       logging.error("Error: no free drbd minors!")
       raise errors.BlockDeviceError("Can't find a free DRBD minor")
+
     return highest + 1
 
   @classmethod
@@ -607,7 +624,8 @@ class DRBD8(base.BlockDev):
     if size:
       args.extend(["-d", "%sm" % size])
 
-    version = self._GetVersion(self._GetProcData())
+    drbd_info = DRBD8Info.CreateFromFile()
+    version = drbd_info.GetVersion()
     vmaj = version["k_major"]
     vmin = version["k_minor"]
     vrel = version["k_point"]
@@ -822,8 +840,7 @@ class DRBD8(base.BlockDev):
     self._ShutdownLocal(self.minor)
     self._children = []
 
-  @classmethod
-  def _SetMinorSyncParams(cls, minor, params):
+  def _SetMinorSyncParams(self, minor, params):
     """Set the parameters of the DRBD syncer.
 
     This is the low-level implementation.
@@ -837,9 +854,10 @@ class DRBD8(base.BlockDev):
 
     """
 
-    args = ["drbdsetup", cls._DevPath(minor), "syncer"]
+    args = ["drbdsetup", self._DevPath(minor), "syncer"]
     if params[constants.LDP_DYNAMIC_RESYNC]:
-      version = cls._GetVersion(cls._GetProcData())
+      drbd_info = DRBD8Info.CreateFromFile()
+      version = drbd_info.GetVersion()
       vmin = version["k_minor"]
       vrel = version["k_point"]
 
@@ -929,10 +947,10 @@ class DRBD8(base.BlockDev):
     if self.minor is None:
       base.ThrowError("drbd%d: GetStats() called while not attached",
                       self._aminor)
-    proc_info = self._JoinProcDataPerMinor(self._GetProcData())
-    if self.minor not in proc_info:
+    drbd_info = DRBD8Info.CreateFromFile()
+    if not drbd_info.HasMinorStatus(self.minor):
       base.ThrowError("drbd%d: can't find myself in /proc", self.minor)
-    return DRBD8Status(proc_info[self.minor])
+    return drbd_info.GetMinorStatus(self.minor)
 
   def GetSyncStatus(self):
     """Returns the sync status of the device.
@@ -1312,9 +1330,10 @@ class DRBD8(base.BlockDev):
                                    " exclusive_storage")
     # check that the minor is unused
     aminor = unique_id[4]
-    proc_info = cls._JoinProcDataPerMinor(cls._GetProcData())
-    if aminor in proc_info:
-      status = DRBD8Status(proc_info[aminor])
+
+    drbd_info = DRBD8Info.CreateFromFile()
+    if drbd_info.HasMinorStatus(aminor):
+      status = drbd_info.GetMinorStatus(aminor)
       in_use = status.is_in_use
     else:
       in_use = False
index bfe761e..da8f095 100644 (file)
@@ -80,7 +80,7 @@ class NodeMaintenance(object):
     """Get list of used DRBD minors.
 
     """
-    return drbd.DRBD8.GetUsedDevs().keys()
+    return drbd.DRBD8.GetUsedDevs()
 
   @classmethod
   def DoMaintenance(cls, role):
index d7c3d58..03fafae 100644 (file)
@@ -1,3 +1,4 @@
+version: 8.0.12 (api:86/proto:86)
 GIT-hash: 5c9f89594553e32adb87d9638dce591782f947e3 build by root@node1.example.com, 2009-05-22 12:47:52
  0: cs:Connected st:Primary/Secondary ds:UpToDate/UpToDate C r---
     ns:78728316 nr:0 dw:77675644 dr:1277039 al:254 bm:270 lo:0 pe:0 ua:0 ap:0
index 099f52e..f9e57cf 100644 (file)
@@ -52,7 +52,7 @@ testFile fileName expectedContent = do
 case_drbd80_emptyline :: Assertion
 case_drbd80_emptyline = testFile "proc_drbd80-emptyline.txt" $
   DRBDStatus
-    ( VersionInfo Nothing Nothing Nothing Nothing
+    ( VersionInfo (Just "8.0.12") (Just "86") (Just "86") Nothing
         (Just "5c9f89594553e32adb87d9638dce591782f947e3")
         (Just "root@node1.example.com, 2009-05-22 12:47:52")
     )
index 1ec838d..4867683 100755 (executable)
@@ -71,7 +71,8 @@ class TestDRBD8(testutils.GanetiTestCase):
       }
     ]
     for d,r in zip(data, result):
-      self.assertEqual(drbd.DRBD8._GetVersion(d), r)
+      info = drbd.DRBD8Info.CreateFromLines(d)
+      self.assertEqual(info.GetVersion(), r)
 
 
 class TestDRBD8Runner(testutils.GanetiTestCase):
@@ -244,26 +245,21 @@ class TestDRBD8Status(testutils.GanetiTestCase):
     proc83_sync_data = testutils.TestDataFilename("proc_drbd83_sync.txt")
     proc83_sync_krnl_data = \
       testutils.TestDataFilename("proc_drbd83_sync_krnl2.6.39.txt")
-    self.proc_data = drbd.DRBD8._GetProcData(filename=proc_data)
-    self.proc80e_data = drbd.DRBD8._GetProcData(filename=proc80e_data)
-    self.proc83_data = drbd.DRBD8._GetProcData(filename=proc83_data)
-    self.proc83_sync_data = drbd.DRBD8._GetProcData(filename=proc83_sync_data)
-    self.proc83_sync_krnl_data = \
-      drbd.DRBD8._GetProcData(filename=proc83_sync_krnl_data)
-    self.mass_data = drbd.DRBD8._JoinProcDataPerMinor(self.proc_data)
-    self.mass80e_data = drbd.DRBD8._JoinProcDataPerMinor(self.proc80e_data)
-    self.mass83_data = drbd.DRBD8._JoinProcDataPerMinor(self.proc83_data)
-    self.mass83_sync_data = \
-      drbd.DRBD8._JoinProcDataPerMinor(self.proc83_sync_data)
-    self.mass83_sync_krnl_data = \
-      drbd.DRBD8._JoinProcDataPerMinor(self.proc83_sync_krnl_data)
+
+    self.drbd_info = drbd.DRBD8Info.CreateFromFile(filename=proc_data)
+    self.drbd_info80e = drbd.DRBD8Info.CreateFromFile(filename=proc80e_data)
+    self.drbd_info83 = drbd.DRBD8Info.CreateFromFile(filename=proc83_data)
+    self.drbd_info83_sync = \
+      drbd.DRBD8Info.CreateFromFile(filename=proc83_sync_data)
+    self.drbd_info83_sync_krnl = \
+      drbd.DRBD8Info.CreateFromFile(filename=proc83_sync_krnl_data)
 
   def testIOErrors(self):
     """Test handling of errors while reading the proc file."""
     temp_file = self._CreateTempFile()
     os.unlink(temp_file)
     self.failUnlessRaises(errors.BlockDeviceError,
-                          drbd.DRBD8._GetProcData, filename=temp_file)
+                          drbd.DRBD8Info.CreateFromFile, filename=temp_file)
 
   def testHelper(self):
     """Test reading usermode_helper in /sys."""
@@ -280,9 +276,9 @@ class TestDRBD8Status(testutils.GanetiTestCase):
 
   def testMinorNotFound(self):
     """Test not-found-minor in /proc"""
-    self.failUnless(9 not in self.mass_data)
-    self.failUnless(9 not in self.mass83_data)
-    self.failUnless(3 not in self.mass80e_data)
+    self.failUnless(not self.drbd_info.HasMinorStatus(9))
+    self.failUnless(not self.drbd_info83.HasMinorStatus(9))
+    self.failUnless(not self.drbd_info80e.HasMinorStatus(3))
 
   def testLineNotMatch(self):
     """Test wrong line passed to drbd.DRBD8Status"""
@@ -290,30 +286,30 @@ class TestDRBD8Status(testutils.GanetiTestCase):
 
   def testMinor0(self):
     """Test connected, primary device"""
-    for data in [self.mass_data, self.mass83_data]:
-      stats = drbd.DRBD8Status(data[0])
+    for info in [self.drbd_info, self.drbd_info83]:
+      stats = info.GetMinorStatus(0)
       self.failUnless(stats.is_in_use)
       self.failUnless(stats.is_connected and stats.is_primary and
                       stats.peer_secondary and stats.is_disk_uptodate)
 
   def testMinor1(self):
     """Test connected, secondary device"""
-    for data in [self.mass_data, self.mass83_data]:
-      stats = drbd.DRBD8Status(data[1])
+    for info in [self.drbd_info, self.drbd_info83]:
+      stats = info.GetMinorStatus(1)
       self.failUnless(stats.is_in_use)
       self.failUnless(stats.is_connected and stats.is_secondary and
                       stats.peer_primary and stats.is_disk_uptodate)
 
   def testMinor2(self):
     """Test unconfigured device"""
-    for data in [self.mass_data, self.mass83_data, self.mass80e_data]:
-      stats = drbd.DRBD8Status(data[2])
+    for info in [self.drbd_info, self.drbd_info83, self.drbd_info80e]:
+      stats = info.GetMinorStatus(2)
       self.failIf(stats.is_in_use)
 
   def testMinor4(self):
     """Test WFconn device"""
-    for data in [self.mass_data, self.mass83_data]:
-      stats = drbd.DRBD8Status(data[4])
+    for info in [self.drbd_info, self.drbd_info83]:
+      stats = info.GetMinorStatus(4)
       self.failUnless(stats.is_in_use)
       self.failUnless(stats.is_wfconn and stats.is_primary and
                       stats.rrole == "Unknown" and
@@ -321,28 +317,28 @@ class TestDRBD8Status(testutils.GanetiTestCase):
 
   def testMinor6(self):
     """Test diskless device"""
-    for data in [self.mass_data, self.mass83_data]:
-      stats = drbd.DRBD8Status(data[6])
+    for info in [self.drbd_info, self.drbd_info83]:
+      stats = info.GetMinorStatus(6)
       self.failUnless(stats.is_in_use)
       self.failUnless(stats.is_connected and stats.is_secondary and
                       stats.peer_primary and stats.is_diskless)
 
   def testMinor8(self):
     """Test standalone device"""
-    for data in [self.mass_data, self.mass83_data]:
-      stats = drbd.DRBD8Status(data[8])
+    for info in [self.drbd_info, self.drbd_info83]:
+      stats = info.GetMinorStatus(8)
       self.failUnless(stats.is_in_use)
       self.failUnless(stats.is_standalone and
                       stats.rrole == "Unknown" and
                       stats.is_disk_uptodate)
 
   def testDRBD83SyncFine(self):
-    stats = drbd.DRBD8Status(self.mass83_sync_data[3])
+    stats = self.drbd_info83_sync.GetMinorStatus(3)
     self.failUnless(stats.is_in_resync)
     self.failUnless(stats.sync_percent is not None)
 
   def testDRBD83SyncBroken(self):
-    stats = drbd.DRBD8Status(self.mass83_sync_krnl_data[3])
+    stats = self.drbd_info83_sync_krnl.GetMinorStatus(3)
     self.failUnless(stats.is_in_resync)
     self.failUnless(stats.sync_percent is not None)