From cb999543e337e13bb2ab839d424839d575330c89 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Wed, 9 Jul 2008 10:41:30 +0000
Subject: [PATCH] Reduce duplicate Attach() calls in bdev

Currently, the 'public' functions of bdev (FindDevice and
AttachOrAssemble) will call the Attach() method right after class
instantiation.

But the constructor itself calls this function, and therefore we have
duplicate Attach() calls (which are not cheap at all).

The patch introduces a new 'attached' instance attribute that tells if
the last Attach() was successful. The public functions reuse this so
that we only do the minimum required number of calls.

Reviewed-by: imsnah
---
 lib/bdev.py | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lib/bdev.py b/lib/bdev.py
index 213ecbc27..7ba3470ea 100644
--- a/lib/bdev.py
+++ b/lib/bdev.py
@@ -82,6 +82,7 @@ class BlockDev(object):
     self.unique_id = unique_id
     self.major = None
     self.minor = None
+    self.attached = False
 
   def Assemble(self):
     """Assemble the device from its components.
@@ -394,6 +395,7 @@ class LogicalVolume(BlockDev):
     recorded.
 
     """
+    self.attached = False
     result = utils.RunCmd(["lvs", "--noheadings", "--separator=,",
                            "-olv_attr,lv_kernel_major,lv_kernel_minor",
                            self.dev_path])
@@ -422,6 +424,7 @@ class LogicalVolume(BlockDev):
     self.minor = minor
     self._degraded = status[0] == 'v' # virtual volume, i.e. doesn't backing
                                       # storage
+    self.attached = True
     return True
 
   def Assemble(self):
@@ -435,7 +438,8 @@ class LogicalVolume(BlockDev):
     result = utils.RunCmd(["lvchange", "-ay", self.dev_path])
     if result.failed:
       logging.error("Can't activate lv %s: %s", self.dev_path, result.output)
-    return not result.failed
+      return False
+    return self.Attach()
 
   def Shutdown(self):
     """Shutdown the device.
@@ -728,9 +732,11 @@ class BaseDRBD(BlockDev):
     """
     if minor is None:
       self.minor = self.dev_path = None
+      self.attached = False
     else:
       self.minor = minor
       self.dev_path = self._DevPath(minor)
+      self.attached = True
 
   @staticmethod
   def _CheckMetaSize(meta_device):
@@ -1550,7 +1556,7 @@ def FindDevice(dev_type, unique_id, children):
   if dev_type not in DEV_MAP:
     raise errors.ProgrammerError("Invalid block device type '%s'" % dev_type)
   device = DEV_MAP[dev_type](unique_id, children)
-  if not device.Attach():
+  if not device.attached:
     return None
   return  device
 
@@ -1565,9 +1571,9 @@ def AttachOrAssemble(dev_type, unique_id, children):
   if dev_type not in DEV_MAP:
     raise errors.ProgrammerError("Invalid block device type '%s'" % dev_type)
   device = DEV_MAP[dev_type](unique_id, children)
-  if not device.Attach():
+  if not device.attached:
     device.Assemble()
-    if not device.Attach():
+    if not device.attached:
       raise errors.BlockDeviceError("Can't find a valid block device for"
                                     " %s/%s/%s" %
                                     (dev_type, unique_id, children))
-- 
GitLab