Change in vdsm[master]: VolumeMetadata: Add from_lines factory method

alitke at redhat.com alitke at redhat.com
Mon Jan 25 08:48:26 UTC 2016


Adam Litke has uploaded a new change for review.

Change subject: VolumeMetadata: Add from_lines factory method
......................................................................

VolumeMetadata: Add from_lines factory method

Add support for creating a VolumeMetadata instance from a list of lines.
This is the format that can be read from either block or file based
volume metadata and enables us to consolidate the logic into one place.

Change-Id: Id7f63dce8e42e99d3240f4e916a9bd6ee5ee4b61
Signed-off-by: Adam Litke <alitke at redhat.com>
---
M tests/volume_metadata_test.py
M vdsm/storage/blockVolume.py
M vdsm/storage/fileVolume.py
M vdsm/storage/volume.py
4 files changed, 71 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/72/52672/1

diff --git a/tests/volume_metadata_test.py b/tests/volume_metadata_test.py
index e5f716f..8143e2c 100644
--- a/tests/volume_metadata_test.py
+++ b/tests/volume_metadata_test.py
@@ -23,6 +23,7 @@
 from testlib import VdsmTestCase
 
 from storage import image, sd, volume
+from storage import storage_exception as se
 from vdsm import define
 
 
@@ -87,6 +88,29 @@
         md = volume.VolumeMetadata(**params)
         self.assertEqual(volume.DESCRIPTION_SIZE, len(md.description))
 
+    def test_from_lines_missing_param(self):
+        self.assertRaises(TypeError, volume.VolumeMetadata.from_lines, [])
+
+    def test_from_lines_invalid_param(self):
+        self.assertRaises(se.VolumeMetadataReadError,
+                          volume.VolumeMetadata.from_lines, ["FOO=bar"])
+
+    def test_from_lines_invalid_format(self):
+        lines = ["DOMAIN=domain", "IMAGE=image", "PUUID=parent", "SIZE=FOO",
+                 "FORMAT=format", "TYPE=type", "VOLTYPE=voltype",
+                 "DISKTYPE=disktype", "EOF"]
+        self.assertRaises(ValueError,
+                          volume.VolumeMetadata.from_lines, lines)
+
+    def test_from_lines(self):
+        lines = ["DOMAIN=domain", "IMAGE=image", "PUUID=parent", "SIZE=0",
+                 "FORMAT=format", "TYPE=type", "VOLTYPE=voltype",
+                 "DISKTYPE=disktype", "EOF"]
+        md = volume.VolumeMetadata.from_lines(lines)
+        self.assertEqual("", md.description)
+        self.assertEqual(0, md.mtime)
+        self.assertEqual("", md.pool_id)
+
     def _compare_params_to_info(self, params, info):
         for param_name, info_field in self.params_to_fields.items():
             self.assertEqual(params[param_name], info[info_field])
diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py
index 1dd6ab5..3b87d4d 100644
--- a/vdsm/storage/blockVolume.py
+++ b/vdsm/storage/blockVolume.py
@@ -106,24 +106,15 @@
         vgname, offs = metaId
 
         try:
-            meta = misc.readblock(lvm.lvPath(vgname, sd.METADATA),
-                                  offs * volume.METADATA_SIZE,
-                                  volume.METADATA_SIZE)
-            # TODO: factor out logic below for sharing with file volumes
-            out = {}
-            for l in meta:
-                if l.startswith("EOF"):
-                    return out
-                if l.find("=") < 0:
-                    continue
-                key, value = l.split("=", 1)
-                out[key.strip()] = value.strip()
-
+            lines = misc.readblock(lvm.lvPath(vgname, sd.METADATA),
+                                   offs * volume.METADATA_SIZE,
+                                   volume.METADATA_SIZE)
+            md = volume.VolumeMetadata.from_text(lines)
         except Exception as e:
             self.log.error(e, exc_info=True)
             raise se.VolumeMetadataReadError("%s: %s" % (metaId, e))
 
-        return out
+        return md.info()
 
     def validateImagePath(self):
         """
diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py
index fd58b26..d49535a 100644
--- a/vdsm/storage/fileVolume.py
+++ b/vdsm/storage/fileVolume.py
@@ -136,22 +136,13 @@
         metaPath = self._getMetaVolumePath(volPath)
 
         try:
-            f = self.oop.directReadLines(metaPath)
-            # TODO: factor out logic below for sharing with block volumes
-            out = {}
-            for l in f:
-                if l.startswith("EOF"):
-                    return out
-                if l.find("=") < 0:
-                    continue
-                key, value = l.split("=", 1)
-                out[key.strip()] = value.strip()
-
+            lines = self.oop.directReadLines(metaPath)
+            md = volume.VolumeMetadata.from_text(lines)
         except Exception as e:
             self.log.error(e, exc_info=True)
             raise se.VolumeMetadataReadError("%s: %s" % (metaId, e))
 
-        return out
+        return md.info()
 
     def getParent(self):
         """
diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py
index cf2a8f1..6d4d3af 100644
--- a/vdsm/storage/volume.py
+++ b/vdsm/storage/volume.py
@@ -166,6 +166,22 @@
 
 
 class VolumeMetadata(object):
+    # Metadata_key: (class_attribute, filter_fn)
+    MD_FIELDS = {
+        DOMAIN: ('sd_id', str),
+        IMAGE:  ('img_id', str),
+        PUUID:  ('parent_vol_id', str),
+        SIZE:   ('size', int),
+        FORMAT: ('vol_format', str),
+        TYPE:   ('prealloc', str),
+        VOLTYPE: ('vol_type', str),
+        DISKTYPE: ('disk_type', str),
+        DESCRIPTION: ('description', str),
+        LEGALITY: ('legality', str),
+        CTIME: ('ctime', int),
+        MTIME: ('mtime', int),
+    }
+
     log = logging.getLogger('Storage.VolumeMetadata')
 
     def __init__(self, sd_id, img_id, parent_vol_id, size, vol_format,
@@ -183,6 +199,24 @@
         self.legality = str(legality)
         self.ctime = int(time.time())
         self.mtime = 0
+        self.pool_id = ""
+
+    @classmethod
+    def from_lines(cls, lines):
+        kwargs = {}
+        for l in lines:
+            if l.startswith("EOF"):
+                break
+            if l.find("=") < 0:
+                continue
+            key, value = l.split("=", 1)
+            try:
+                param = cls.MD_FIELDS[key.strip()][0]
+            except KeyError:
+                raise se.VolumeMetadataReadError("Invalid key: %s" %
+                                                 key.strip())
+            kwargs[param] = value.strip()
+        return cls(**kwargs)
 
     @property
     def description(self):
@@ -210,21 +244,11 @@
         """
         Return metadata in dictionary format
         """
-        return {
-            FORMAT: str(self.vol_format),
-            TYPE: str(self.prealloc),
-            VOLTYPE: str(self.vol_type),
-            DISKTYPE: str(self.disk_type),
-            SIZE: int(self.size),
-            CTIME: int(self.ctime),
-            sd.DMDK_POOLS: "",  # obsolete
-            DOMAIN: str(self.sd_id),
-            IMAGE: str(self.img_id),
-            DESCRIPTION: str(self.description),
-            PUUID: str(self.parent_vol_id),
-            MTIME: int(self.mtime),
-            LEGALITY: str(self.legality),
-        }
+        ret = {}
+        for field, (attr, filter_fn) in self.MD_FIELDS.items():
+            ret[field] = filter_fn(getattr(self, attr))
+        ret[sd.DMDK_POOLS] = ""  # Deprecated
+        return ret
 
 
 class VolumeManifest(object):


-- 
To view, visit https://gerrit.ovirt.org/52672
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id7f63dce8e42e99d3240f4e916a9bd6ee5ee4b61
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>


More information about the vdsm-patches mailing list