Nir Soffer has uploaded a new change for review.

Change subject: volume: Unify metadata formatting and validation
......................................................................

volume: Unify metadata formatting and validation

We had duplicate code for formatting volume metadata, and the file
volume version did not protect from metadata overflow. On file storage
the metadata size is not limited, but when copying file based volume to
block storage, the description field will be truncated.

Metadata is formatted now using Volume.formatMetata(), which also
validate the formatted metadata size.

Since metadata size is relevant for both block and file volumes,
blockVolume.VOLUME_METASIZE was moved up to volume.METADATA_SIZE.

Change-Id: I0ec696639fb57c462e9fab9d22aba5b662fceb66
Signed-off-by: Nir Soffer <[email protected]>
---
M vdsm/storage/blockVolume.py
M vdsm/storage/fileVolume.py
M vdsm/storage/volume.py
3 files changed, 25 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/02/45502/1

diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py
index 4518ae8..e30b3c6 100644
--- a/vdsm/storage/blockVolume.py
+++ b/vdsm/storage/blockVolume.py
@@ -52,9 +52,6 @@
                TAG_PREFIX_MDNUMBLKS]
 
 BLOCK_SIZE = volume.BLOCK_SIZE
-
-# volume meta data block size
-VOLUME_METASIZE = BLOCK_SIZE
 VOLUME_MDNUMBLKS = 1
 
 SECTORS_TO_MB = 2048
@@ -537,18 +534,12 @@
     def __putMetadata(cls, metaId, meta):
         vgname, offs = metaId
 
-        lines = ["%s=%s\n" % (key.strip(), str(value).strip())
-                 for key, value in meta.iteritems()]
-        lines.append("EOF\n")
-        data = "".join(lines)
-        if len(data) > VOLUME_METASIZE:
-            raise se.MetadataOverflowError(data)
-
-        data += "\0" * (VOLUME_METASIZE - len(data))
+        data = cls.formatMetadata(meta)
+        data += "\0" * (volume.METADATA_SIZE - len(data))
 
         metavol = lvm.lvPath(vgname, sd.METADATA)
         with fileUtils.DirectFile(metavol, "r+d") as f:
-            f.seek(offs * VOLUME_METASIZE)
+            f.seek(offs * volume.METADATA_SIZE)
             f.write(data)
 
     @classmethod
@@ -586,7 +577,8 @@
 
         try:
             meta = misc.readblock(lvm.lvPath(vgname, sd.METADATA),
-                                  offs * VOLUME_METASIZE, VOLUME_METASIZE)
+                                  offs * volume.METADATA_SIZE,
+                                  volume.METADATA_SIZE)
             out = {}
             for l in meta:
                 if l.startswith("EOF"):
diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py
index 20573bd..b15ffb3 100644
--- a/vdsm/storage/fileVolume.py
+++ b/vdsm/storage/fileVolume.py
@@ -314,10 +314,10 @@
         volPath, = metaId
         metaPath = cls.__metaVolumePath(volPath)
 
+        data = cls.formatMetadata(meta)
+
         with open(metaPath + ".new", "w") as f:
-            for key, value in meta.iteritems():
-                f.write("%s=%s\n" % (key.strip(), str(value).strip()))
-            f.write("EOF\n")
+            f.write(data)
 
         sdUUID = getDomUuidFromVolumePath(volPath)
         oop.getProcessPool(sdUUID).os.rename(metaPath + ".new", metaPath)
diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py
index 9300abc..4e4987f 100644
--- a/vdsm/storage/volume.py
+++ b/vdsm/storage/volume.py
@@ -98,6 +98,8 @@
 # it will depend on the storage domain.
 BLOCK_SIZE = 512
 
+METADATA_SIZE = BLOCK_SIZE
+
 # In block storage, metadata size is limited to BLOCK_SIZE (512), to
 # ensure that metadata is written atomically. This is big enough for the
 # actual metadata, but may not be big enough for the description field.
@@ -885,6 +887,21 @@
         return meta
 
     @classmethod
+    def formatMetadata(cls, meta):
+        """
+        Format metadata string in storage format.
+
+        Raises MetadataOverflowError if formatted metadata is too long.
+        """
+        lines = ["%s=%s\n" % (key.strip(), str(value).strip())
+                 for key, value in meta.iteritems()]
+        lines.append("EOF\n")
+        data = "".join(lines)
+        if len(data) > METADATA_SIZE:
+            raise se.MetadataOverflowError(data)
+        return data
+
+    @classmethod
     def validateDescription(cls, desc):
         desc = str(desc)
         # We cannot fail when the description is too long, since we must


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0ec696639fb57c462e9fab9d22aba5b662fceb66
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to