Rework the DRBD8 device status computation

Currently, compute the status of a drbd8 device in GetSyncStatus and
return only the values that we need (and fit in the framework of
GetSyncStatus). However, the full status details are useful (and needed)
in other places, so the patch attempts to improve this situation.

We abstract the status of a device outside in a separate class, that
knows how to parse contents from /proc/drbd and set easily accessible
attributes. We then simplify the GetSyncStatus to use this and return
the values that it needs, and add a separate method that returns the
full status object.

The move to a separate class cleans up a little bit the old
sync-progress computation from GetSyncStatus, but it's still many
regexes.

The patch also adds unittests for a few statuses, and modifies one
BaseDRBD call to accept a custom filename instead of '/proc/drbd' to
ease unittests.

Reviewed-by: imsnah

diff --git a/lib/bdev.py b/lib/bdev.py
index f3b182f..698f02e 100644 (file)
--- a/lib/bdev.py
+++ b/lib/bdev.py
@@ -1005,6 +1005,56 @@ class MDRaid1(BlockDev):
     pass
 
 
+class DRBD8Status(object):
+  """A DRBD status representation class.
+
+  Note that this doesn't support unconfigured devices (cs:Unconfigured).
+
+  """
+  LINE_RE = re.compile(r"\s*[0-9]+:\s*cs:(\S+)\s+st:([^/]+)/(\S+)"
+                       "\s+ds:([^/]+)/(\S+)\s+.*$")
+  SYNC_RE = re.compile(r"^.*\ssync'ed:\s*([0-9.]+)%.*"
+                       "\sfinish: ([0-9]+):([0-9]+):([0-9]+)\s.*$")
+
+  def __init__(self, procline):
+    m = self.LINE_RE.match(procline)
+    if not m:
+      raise errors.BlockDeviceError("Can't parse input data '%s'" % procline)
+    self.cstatus = m.group(1)
+    self.lrole = m.group(2)
+    self.rrole = m.group(3)
+    self.ldisk = m.group(4)
+    self.rdisk = m.group(5)
+
+    self.is_standalone = self.cstatus == "StandAlone"
+    self.is_wfconn = self.cstatus == "WFConnection"
+    self.is_connected = self.cstatus == "Connected"
+    self.is_primary = self.lrole == "Primary"
+    self.is_secondary = self.lrole == "Secondary"
+    self.peer_primary = self.rrole == "Primary"
+    self.peer_secondary = self.rrole == "Secondary"
+    self.both_primary = self.is_primary and self.peer_primary
+    self.both_secondary = self.is_secondary and self.peer_secondary
+
+    self.is_diskless = self.ldisk == "Diskless"
+    self.is_disk_uptodate = self.ldisk == "UpToDate"
+
+    m = self.SYNC_RE.match(procline)
+    if m:
+      self.sync_percent = float(m.group(1))
+      hours = int(m.group(2))
+      minutes = int(m.group(3))
+      seconds = int(m.group(4))
+      self.est_time = hours * 3600 + minutes * 60 + seconds
+    else:
+      self.sync_percent = None
+      self.est_time = None
+
+    self.is_sync_target = self.peer_sync_source = self.cstatus == "SyncTarget"
+    self.peer_sync_target = self.is_sync_source = self.cstatus == "SyncSource"
+    self.is_resync = self.is_sync_target or self.is_sync_source
+
+
 class BaseDRBD(BlockDev):
   """Base DRBD class.
 
@@ -1020,18 +1070,20 @@ class BaseDRBD(BlockDev):
   _ST_WFCONNECTION = "WFConnection"
   _ST_CONNECTED = "Connected"
 
+  _STATUS_FILE = "/proc/drbd"
+
   @staticmethod
-  def _GetProcData():
+  def _GetProcData(filename=_STATUS_FILE):
     """Return data from /proc/drbd.
 
     """
-    stat = open("/proc/drbd", "r")
+    stat = open(filename, "r")
     try:
       data = stat.read().splitlines()
     finally:
       stat.close()
     if not data:
-      raise errors.BlockDeviceError("Can't read any data from /proc/drbd")
+      raise errors.BlockDeviceError("Can't read any data from %s" % filename)
     return data
 
   @staticmethod
@@ -2043,6 +2095,18 @@ class DRBD8(BaseDRBD):
                    (result.fail_reason, result.output))
     return not result.failed and children_result
 
+  def GetProcStatus(self):
+    """Return device data from /proc.
+
+    """
+    if self.minor is None:
+      raise errors.BlockDeviceError("GetStats() called while not attached")
+    proc_info = self._MassageProcData(self._GetProcData())
+    if self.minor not in proc_info:
+      raise errors.BlockDeviceError("Can't find myself in /proc (minor %d)" %
+                                    self.minor)
+    return DRBD8Status(proc_info[self.minor])
+
   def GetSyncStatus(self):
     """Returns the sync status of the device.
 
@@ -2063,31 +2127,10 @@ class DRBD8(BaseDRBD):
     """
     if self.minor is None and not self.Attach():
       raise errors.BlockDeviceError("Can't attach to device in GetSyncStatus")
-    proc_info = self._MassageProcData(self._GetProcData())
-    if self.minor not in proc_info:
-      raise errors.BlockDeviceError("Can't find myself in /proc (minor %d)" %
-                                    self.minor)
-    line = proc_info[self.minor]
-    match = re.match("^.*sync'ed: *([0-9.]+)%.*"
-                     " finish: ([0-9]+):([0-9]+):([0-9]+) .*$", line)
-    if match:
-      sync_percent = float(match.group(1))
-      hours = int(match.group(2))
-      minutes = int(match.group(3))
-      seconds = int(match.group(4))
-      est_time = hours * 3600 + minutes * 60 + seconds
-    else:
-      sync_percent = None
-      est_time = None
-    match = re.match("^ *\d+: cs:(\w+).*ds:(\w+)/(\w+).*$", line)
-    if not match:
-      raise errors.BlockDeviceError("Can't find my data in /proc (minor %d)" %
-                                    self.minor)
-    client_state = match.group(1)
-    local_disk_state = match.group(2)
-    ldisk = local_disk_state != "UpToDate"
-    is_degraded = client_state != "Connected"
-    return sync_percent, est_time, is_degraded or ldisk, ldisk
+    stats = self.GetProcStatus()
+    ldisk = not stats.is_disk_uptodate
+    is_degraded = not stats.is_connected
+    return stats.sync_percent, stats.est_time, is_degraded or ldisk, ldisk
 
   def GetStatus(self):
     """Compute the status of the DRBD device

diff --git a/test/data/proc_drbd8.txt b/test/data/proc_drbd8.txt
new file mode 100644 (file)
index 0000000..22ce59a
--- /dev/null
+++ b/test/data/proc_drbd8.txt
@@ -0,0 +1,29 @@
+version: 8.0.12 (api:86/proto:86)
+GIT-hash: 5c9f89594553e32adb87d9638dce591782f947e3 build by XXX
+ 0: cs:Connected st:Primary/Secondary ds:UpToDate/UpToDate C r---
+    ns:4375577 nr:0 dw:4446279 dr:674 al:1067 bm:69 lo:0 pe:0 ua:0 ap:0
+       resync: used:0/61 hits:0 misses:0 starving:0 dirty:0 changed:0
+       act_log: used:0/257 hits:793749 misses:1067 starving:0 dirty:0 changed:1067
+ 1: cs:Connected st:Secondary/Primary ds:UpToDate/UpToDate C r---
+    ns:738320 nr:0 dw:738320 dr:554400 al:67 bm:0 lo:0 pe:0 ua:0 ap:0
+       resync: used:0/61 hits:0 misses:0 starving:0 dirty:0 changed:0
+       act_log: used:0/257 hits:92464 misses:67 starving:0 dirty:0 changed:67
+ 2: cs:Unconfigured
+ 4: cs:WFConnection st:Primary/Unknown ds:UpToDate/DUnknown C r---
+    ns:738320 nr:0 dw:738320 dr:554400 al:67 bm:0 lo:0 pe:0 ua:0 ap:0
+        resync: used:0/61 hits:0 misses:0 starving:0 dirty:0 changed:0
+        act_log: used:0/257 hits:92464 misses:67 starving:0 dirty:0 changed:67
+ 5: cs:Connected st:Primary/Secondary ds:UpToDate/Diskless C r---
+    ns:4375581 nr:0 dw:4446283 dr:674 al:1069 bm:69 lo:0 pe:0 ua:0 ap:0
+        resync: used:0/61 hits:0 misses:0 starving:0 dirty:0 changed:0
+        act_log: used:0/257 hits:793750 misses:1069 starving:0 dirty:0 changed:1069
+ 6: cs:Connected st:Secondary/Primary ds:Diskless/UpToDate C r---
+    ns:0 nr:4375581 dw:5186925 dr:327 al:75 bm:214 lo:0 pe:0 ua:0 ap:0
+ 7: cs:WFConnection st:Secondary/Unknown ds:UpToDate/DUnknown C r---
+    ns:0 nr:0 dw:0 dr:0 al:0 bm:0 lo:0 pe:0 ua:0 ap:0
+        resync: used:0/61 hits:0 misses:0 starving:0 dirty:0 changed:0
+        act_log: used:0/257 hits:0 misses:0 starving:0 dirty:0 changed:0
+ 8: cs:StandAlone st:Secondary/Unknown ds:UpToDate/DUnknown   r---
+    ns:0 nr:0 dw:0 dr:0 al:0 bm:0 lo:0 pe:0 ua:0 ap:0
+        resync: used:0/61 hits:0 misses:0 starving:0 dirty:0 changed:0
+        act_log: used:0/257 hits:0 misses:0 starving:0 dirty:0 changed:0

diff --git a/test/ganeti.bdev_unittest.py b/test/ganeti.bdev_unittest.py
index 7876f3d..6f0b098 100755 (executable)
--- a/test/ganeti.bdev_unittest.py
+++ b/test/ganeti.bdev_unittest.py
@@ -26,6 +26,7 @@ import os
 import unittest
 
 from ganeti import bdev
+from ganeti import errors
 
 
 class TestDRBD8Runner(unittest.TestCase):
@@ -108,5 +109,54 @@ class TestDRBD8Runner(unittest.TestCase):
                      "remote_addr" not in result),
                     "Should not find network info")
 
+
+class TestDRBD8Status(unittest.TestCase):
+  """Testing case for DRBD8 /proc status"""
+
+  def setUp(self):
+    """Read in txt data"""
+    self.proc_data = bdev.DRBD8._GetProcData(filename="data/proc_drbd8.txt")
+    self.mass_data = bdev.DRBD8._MassageProcData(self.proc_data)
+
+  def testMinorNotFound(self):
+    """Test not-found-minor in /proc"""
+    self.failUnless(9 not in self.mass_data)
+
+  def testLineNotMatch(self):
+    """Test wrong line passed to DRBD8Status"""
+    self.assertRaises(errors.BlockDeviceError, bdev.DRBD8Status, "foo")
+
+  def testMinor0(self):
+    """Test connected, primary device"""
+    stats = bdev.DRBD8Status(self.mass_data[0])
+    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"""
+    stats = bdev.DRBD8Status(self.mass_data[1])
+    self.failUnless(stats.is_connected and stats.is_secondary and
+                    stats.peer_primary and stats.is_disk_uptodate)
+
+  def testMinor4(self):
+    """Test WFconn device"""
+    stats = bdev.DRBD8Status(self.mass_data[4])
+    self.failUnless(stats.is_wfconn and stats.is_primary and
+                    stats.rrole == 'Unknown' and
+                    stats.is_disk_uptodate)
+
+  def testMinor6(self):
+    """Test diskless device"""
+    stats = bdev.DRBD8Status(self.mass_data[6])
+    self.failUnless(stats.is_connected and stats.is_secondary and
+                    stats.peer_primary and stats.is_diskless)
+
+  def testMinor8(self):
+    """Test standalone device"""
+    stats = bdev.DRBD8Status(self.mass_data[8])
+    self.failUnless(stats.is_standalone and
+                    stats.rrole == 'Unknown' and
+                    stats.is_disk_uptodate)
+
 if __name__ == '__main__':
   unittest.main()