Change in vdsm[master]: cow wip

alitke at redhat.com alitke at redhat.com
Tue Apr 26 13:39:41 UTC 2016


Adam Litke has uploaded a new change for review.

Change subject: cow wip
......................................................................

cow wip

Add support for qcow2 volumes.  Snapshots and Templates will be
supported in future patches.  This is a work in progress.

Change-Id: If31e013187830322d4c9ebf0d76b4d6d4d81445d
Signed-off-by: Adam Litke <alitke at redhat.com>
---
M tests/storage_volume_artifacts_test.py
M vdsm/storage/sdm/volume_artifacts.py
2 files changed, 159 insertions(+), 47 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/56629/1

diff --git a/tests/storage_volume_artifacts_test.py b/tests/storage_volume_artifacts_test.py
index aa27210..1c454d2 100644
--- a/tests/storage_volume_artifacts_test.py
+++ b/tests/storage_volume_artifacts_test.py
@@ -25,6 +25,9 @@
 from testValidation import brokentest
 from storagetestlib import fake_block_env, fake_file_env
 
+from vdsm import qemuimg
+from vdsm import utils
+from vdsm.config import config
 from vdsm.storage import exception as se
 from vdsm.storage.constants import TEMP_VOL_LVTAG
 
@@ -35,14 +38,19 @@
 class ExpectedFailure(Exception):
     pass
 
-
-BASE_RAW_PARAMS = (1073741824, volume.RAW_FORMAT,
-                   image.SYSTEM_DISK_TYPE, 'raw_volume')
-BASE_COW_PARAMS = (1073741824, volume.COW_FORMAT,
-                   image.SYSTEM_DISK_TYPE, 'cow_volume')
 MB = 1024 ** 2
+VOL_SIZE = 1073741824
+BLOCK_INITIAL_CHUNK_SIZE = MB * config.getint("irs",
+                                              "volume_utilization_chunk_mb")
+BASE_PARAMS = {
+    volume.RAW_FORMAT: (VOL_SIZE, volume.RAW_FORMAT,
+                        image.SYSTEM_DISK_TYPE, 'raw_volume'),
+    volume.COW_FORMAT: (VOL_SIZE, volume.COW_FORMAT,
+                        image.SYSTEM_DISK_TYPE, 'cow_volume')
+}
 
 
+ at expandPermutations
 class VolumeArtifactsTestsMixin(object):
 
     def setUp(self):
@@ -62,7 +70,7 @@
         with self.fake_env() as env:
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            artifacts.create(*BASE_RAW_PARAMS)
+            artifacts.create(*BASE_PARAMS[volume.RAW_FORMAT])
             self.assertTrue(artifacts.is_garbage())
             self.assertFalse(artifacts.is_image())
             self.validate_artifacts(artifacts, env)
@@ -71,15 +79,15 @@
         with self.fake_env() as env:
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            artifacts.create(*BASE_RAW_PARAMS)
+            artifacts.create(*BASE_PARAMS[volume.RAW_FORMAT])
             self.assertRaises(se.DomainHasGarbage, artifacts.create,
-                              *BASE_RAW_PARAMS)
+                              *BASE_PARAMS[volume.RAW_FORMAT])
 
     def test_state_image(self):
         with self.fake_env() as env:
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            artifacts.create(*BASE_RAW_PARAMS)
+            artifacts.create(*BASE_PARAMS[volume.RAW_FORMAT])
             artifacts.commit()
             self.assertFalse(artifacts.is_garbage())
             self.assertTrue(artifacts.is_image())
@@ -88,36 +96,36 @@
         with self.fake_env() as env:
             first = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            first.create(*BASE_RAW_PARAMS)
+            first.create(*BASE_PARAMS[volume.RAW_FORMAT])
             first.commit()
             second = env.sd_manifest.get_volume_artifacts(
                 self.img_id, str(uuid.uuid4()))
-            self.assertRaises(NotImplementedError,
-                              second.create, *BASE_COW_PARAMS)
+            self.assertRaises(se.InvalidParameterException,
+                              second.create, *BASE_PARAMS[volume.COW_FORMAT])
 
     def test_create_additional_raw_vol(self):
         with self.fake_env() as env:
             first = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            first.create(*BASE_RAW_PARAMS)
+            first.create(*BASE_PARAMS[volume.RAW_FORMAT])
             first.commit()
             second = env.sd_manifest.get_volume_artifacts(
                 self.img_id, str(uuid.uuid4()))
             self.assertRaises(se.InvalidParameterException, second.create,
-                              *BASE_RAW_PARAMS)
+                              *BASE_PARAMS[volume.RAW_FORMAT])
 
-    @brokentest("Broken until COW volume support is added")
+    @brokentest("Broken until parent volume support is added")
     def test_create_same_volume_in_image(self):
         with self.fake_env() as env:
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            artifacts.create(*BASE_RAW_PARAMS)
+            artifacts.create(*BASE_PARAMS[volume.RAW_FORMAT])
             artifacts.commit()
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
             parent = storage.sdm.api.create_volume.ParentVolumeInfo(
                 dict(img_id=self.img_id, vol_id=self.vol_id))
-            params = BASE_COW_PARAMS + (parent,)
+            params = BASE_PARAMS[volume.COW_FORMAT] + (parent,)
 
             self.assertRaises(se.VolumeAlreadyExists,
                               artifacts.create, *params)
@@ -126,7 +134,7 @@
         with self.fake_env() as env:
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            size, vol_format, disk_type, desc = BASE_RAW_PARAMS
+            size, vol_format, disk_type, desc = BASE_PARAMS[volume.RAW_FORMAT]
             artifacts.create(size, vol_format, disk_type, desc)
             artifacts.commit()
             vol = env.sd_manifest.produceVolume(self.img_id, self.vol_id)
@@ -139,6 +147,19 @@
             self.assertEqual(vol_format, vol.getFormat())
             self.assertEqual(str(disk_type), vol.getDiskType())
 
+    @permutations([[volume.RAW_FORMAT, 'raw'], [volume.COW_FORMAT, 'qcow2']])
+    def test_qemuimg_info(self, vol_format, qemu_format):
+        with self.fake_env() as env:
+            artifacts = env.sd_manifest.get_volume_artifacts(
+                self.img_id, self.vol_id)
+            size, vol_format, disk_type, desc = BASE_PARAMS[vol_format]
+            artifacts.create(size, vol_format, disk_type, desc)
+            artifacts.commit()
+            info = qemuimg.info(artifacts.volume_path)
+            self.assertEqual(qemu_format, info['format'])
+            self.assertEqual(size, info['virtualsize'])
+            self.assertNotIn('backingfile', info)
+
     # Artifacts visibility
 
     def test_getallvolumes(self):
@@ -147,7 +168,7 @@
             self.assertEqual({}, env.sd_manifest.getAllVolumes())
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            artifacts.create(*BASE_RAW_PARAMS)
+            artifacts.create(*BASE_PARAMS[volume.RAW_FORMAT])
             self.assertEqual({}, env.sd_manifest.getAllVolumes())
             artifacts.commit()
             self.assertIn(self.vol_id, env.sd_manifest.getAllVolumes())
@@ -159,12 +180,12 @@
     def fake_env(self):
         return fake_file_env()
 
-    def test_raw_volume_preallocation(self):
+    @permutations([[volume.RAW_FORMAT], [volume.COW_FORMAT]])
+    def test_volume_preallocation(self, vol_format):
         with self.fake_env() as env:
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            size, vol_format, disk_type, desc = BASE_RAW_PARAMS
-            artifacts.create(size, vol_format, disk_type, desc)
+            artifacts.create(*BASE_PARAMS[vol_format])
             artifacts.commit()
             vol = env.sd_manifest.produceVolume(self.img_id, self.vol_id)
             self.assertEqual(volume.SPARSE_VOL, vol.getType())
@@ -177,7 +198,7 @@
                 self.img_id, self.vol_id)
             artifacts._create_metadata_artifact = self.failure
             self.assertRaises(ExpectedFailure, artifacts.create,
-                              *BASE_RAW_PARAMS)
+                              *BASE_PARAMS[volume.RAW_FORMAT])
             self.validate_new_image_path(artifacts)
 
     def test_new_image_create_lease_failure(self):
@@ -188,14 +209,14 @@
                 self.img_id, self.vol_id)
             artifacts._create_lease_file = self.failure
             self.assertRaises(ExpectedFailure, artifacts.create,
-                              *BASE_RAW_PARAMS)
+                              *BASE_PARAMS[volume.RAW_FORMAT])
             self.validate_new_image_path(artifacts, has_md=True)
 
             # We cannot re-create in this state because garbage left behind
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
             self.assertRaises(se.DomainHasGarbage, artifacts.create,
-                              *BASE_RAW_PARAMS)
+                              *BASE_PARAMS[volume.RAW_FORMAT])
 
     def test_new_image_create_container_failure(self):
         # If we fail before the container is created we will have a garbage
@@ -205,7 +226,7 @@
                 self.img_id, self.vol_id)
             artifacts._create_volume_file = self.failure
             self.assertRaises(ExpectedFailure, artifacts.create,
-                              *BASE_RAW_PARAMS)
+                              *BASE_PARAMS[volume.RAW_FORMAT])
             self.validate_new_image_path(artifacts,
                                          has_md=True, has_lease=True)
 
@@ -213,7 +234,7 @@
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
             self.assertRaises(se.DomainHasGarbage, artifacts.create,
-                              *BASE_RAW_PARAMS)
+                              *BASE_PARAMS[volume.RAW_FORMAT])
 
     def test_garbage_image_dir(self):
         # Creating the an artifact using an existing garbage image directory is
@@ -223,11 +244,11 @@
                 self.img_id, self.vol_id)
             artifacts._create_metadata_artifact = self.failure
             self.assertRaises(ExpectedFailure, artifacts.create,
-                              *BASE_RAW_PARAMS)
+                              *BASE_PARAMS[volume.RAW_FORMAT])
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
             self.assertRaises(se.DomainHasGarbage, artifacts.create,
-                              *BASE_RAW_PARAMS)
+                              *BASE_PARAMS[volume.RAW_FORMAT])
 
     # Invalid use of artifacts
 
@@ -241,7 +262,7 @@
         with self.fake_env() as env:
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            artifacts.create(*BASE_RAW_PARAMS)
+            artifacts.create(*BASE_PARAMS[volume.RAW_FORMAT])
             artifacts.commit()
             self.assertRaises(OSError, artifacts.commit)
 
@@ -250,8 +271,8 @@
         with self.fake_env() as env:
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            self.assertRaises(se.InvalidParameterException,
-                              artifacts.create, *BASE_RAW_PARAMS,
+            self.assertRaises(se.InvalidParameterException, artifacts.create,
+                              *BASE_PARAMS[volume.RAW_FORMAT],
                               initial_size=initial_size)
 
     def validate_new_image_path(self, artifacts, has_md=False,
@@ -299,7 +320,7 @@
             self.assertEqual(set(), env.sd_manifest.getAllImages())
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            artifacts.create(*BASE_RAW_PARAMS)
+            artifacts.create(*BASE_PARAMS[volume.RAW_FORMAT])
             self.assertEqual({garbage_img_id}, env.sd_manifest.getAllImages())
             artifacts.commit()
             self.assertEqual({self.img_id}, env.sd_manifest.getAllImages())
@@ -311,25 +332,51 @@
     def fake_env(self):
         return fake_block_env()
 
-    def test_raw_volume_preallocation(self):
+    @permutations([
+        [volume.RAW_FORMAT, volume.PREALLOCATED_VOL],
+        [volume.COW_FORMAT, volume.SPARSE_VOL]
+    ])
+    def test_volume_preallocation(self, vol_format, alloc_policy):
         with self.fake_env() as env:
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            size, vol_format, disk_type, desc = BASE_RAW_PARAMS
+            size, vol_format, disk_type, desc = BASE_PARAMS[vol_format]
             artifacts.create(size, vol_format, disk_type, desc)
             artifacts.commit()
             vol = env.sd_manifest.produceVolume(self.img_id, self.vol_id)
-            self.assertEqual(volume.PREALLOCATED_VOL, vol.getType())
+            self.assertEqual(alloc_policy, vol.getType())
 
     @permutations([[0], [volume.BLOCK_SIZE]])
     def test_raw_volume_initial_size(self, initial_size):
-        # TODO: Write a test for COW volumes when support is added
         with self.fake_env() as env:
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            self.assertRaises(se.InvalidParameterException,
-                              artifacts.create, *BASE_RAW_PARAMS,
+            self.assertRaises(se.InvalidParameterException, artifacts.create,
+                              *BASE_PARAMS[volume.RAW_FORMAT],
                               initial_size=initial_size)
+
+    @permutations([
+        [None, BLOCK_INITIAL_CHUNK_SIZE],
+        [MB, utils.round(MB * blockVolume.QCOW_OVERHEAD_FACTOR, MB)]
+    ])
+    def test_cow_volume_initial_size(self, requested_size, actual_size):
+        test_size = 2 * BLOCK_INITIAL_CHUNK_SIZE
+        with self.fake_env() as env:
+            artifacts = env.sd_manifest.get_volume_artifacts(
+                self.img_id, self.vol_id)
+            size, vol_format, disk_type, desc = BASE_PARAMS[volume.COW_FORMAT]
+            artifacts.create(test_size, vol_format, disk_type, desc,
+                             initial_size=requested_size)
+            artifacts.commit()
+
+            # Note: Here we check the size via FakeLVM instead of by using
+            # sd_manifest.getVSize.  The qemu-img program sees our fake LV as a
+            # file and 'helpfully' truncates it to the minimal size required.
+            # Therefore, we cannot use file size for this test.
+            lv = env.lvm.getLV(env.sd_manifest.sdUUID, self.vol_id)
+            self.assertEqual(actual_size, int(lv.size))
+            vol = env.sd_manifest.produceVolume(self.img_id, self.vol_id)
+            self.assertEqual(test_size, vol.getSize() * volume.BLOCK_SIZE)
 
     def test_size_rounded_up(self):
         # If the underlying device is larger the size will be updated
@@ -357,9 +404,38 @@
         with self.fake_env() as env:
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            artifacts.create(*BASE_RAW_PARAMS)
+            artifacts.create(*BASE_PARAMS[volume.RAW_FORMAT])
             artifacts.commit()
             artifacts.commit()  # removing nonexistent tags is allowed
+
+    def test_unaligned_size_raises(self):
+        with fake_block_env() as env:
+            artifacts = env.sd_manifest.get_volume_artifacts(
+                self.img_id, self.vol_id)
+            size, vol_format, disk_type, desc = BASE_PARAMS[volume.COW_FORMAT]
+            size = MB + 1
+            self.assertRaises(se.InvalidParameterException,
+                              artifacts.create, size, vol_format, disk_type,
+                              desc)
+
+    def test_unaligned_initial_size_raises(self):
+        with fake_block_env() as env:
+            artifacts = env.sd_manifest.get_volume_artifacts(
+                self.img_id, self.vol_id)
+            size, vol_format, disk_type, desc = BASE_PARAMS[volume.COW_FORMAT]
+            initial_size = size - 1
+            self.assertRaises(se.InvalidParameterException,
+                              artifacts.create, size, vol_format, disk_type,
+                              desc, initial_size=initial_size)
+
+    def test_oversized_initial_size_raises(self):
+        with self.fake_env() as env:
+            artifacts = env.sd_manifest.get_volume_artifacts(
+                self.img_id, self.vol_id)
+            size, vol_format, disk_type, desc = BASE_PARAMS[volume.COW_FORMAT]
+            self.assertRaises(se.InvalidParameterException,
+                              artifacts.create, size, vol_format, disk_type,
+                              desc, initial_size=size + 1)
 
     def validate_artifacts(self, artifacts, env):
         try:
@@ -403,7 +479,7 @@
             self.assertEqual(set(), env.sd_manifest.getAllImages())
             artifacts = env.sd_manifest.get_volume_artifacts(
                 self.img_id, self.vol_id)
-            artifacts.create(*BASE_RAW_PARAMS)
+            artifacts.create(*BASE_PARAMS[volume.RAW_FORMAT])
             self.assertEqual(set(), env.sd_manifest.getAllImages())
             artifacts.commit()
             self.assertEqual({self.img_id}, env.sd_manifest.getAllImages())
diff --git a/vdsm/storage/sdm/volume_artifacts.py b/vdsm/storage/sdm/volume_artifacts.py
index 2ae0380..5a67fd4 100644
--- a/vdsm/storage/sdm/volume_artifacts.py
+++ b/vdsm/storage/sdm/volume_artifacts.py
@@ -61,6 +61,8 @@
     TEMP_VOL_LVTAG
 )
 
+from vdsm import qemuimg
+from vdsm import utils
 from storage import blockVolume, lvm, volume
 
 
@@ -76,6 +78,13 @@
         self.vol_class = self.sd_manifest.getVolumeClass()
         self.img_id = img_id
         self.vol_id = vol_id
+
+    @property
+    def volume_path(self):
+        """
+        Return a path which can be used to access the volume data area.
+        """
+        raise NotImplementedError()
 
     def create(self, size, vol_format, disk_type, desc, parent=None,
                initial_size=None):
@@ -109,10 +118,9 @@
         """
         raise NotImplementedError()
 
-    def _validate_create_params(self, vol_format, parent, prealloc):
+    def _validate_create_params(self, size, vol_format, disk_type, desc,
+                                parent, initial_size, prealloc):
         # XXX: Remove these when support is added:
-        if vol_format != volume.RAW_FORMAT:
-            raise NotImplementedError("Only raw volumes are supported")
         if parent:
             raise NotImplementedError("parent_vol_id not supported")
 
@@ -124,6 +132,10 @@
         parent_vol_id = parent.vol_id if parent else volume.BLANK_UUID
         self.sd_manifest.validateCreateVolumeParams(
             vol_format, parent_vol_id, preallocate=prealloc)
+
+    def _initialize_volume(self, vol_format, size):
+        if vol_format == volume.COW_FORMAT:
+            qemuimg.create(self.volume_path, size, volume.fmt2str(vol_format))
 
 
 class FileVolumeArtifacts(VolumeArtifacts):
@@ -214,7 +226,8 @@
         Create metadata file artifact, lease file, and volume file on storage.
         """
         prealloc = self._get_volume_preallocation(vol_format)
-        self._validate_create_params(vol_format, parent, prealloc)
+        self._validate_create_params(size, vol_format, disk_type, desc, parent,
+                                     initial_size, prealloc)
 
         if initial_size is not None:
             self.log.debug("initial_size is not supported for file volumes")
@@ -227,6 +240,7 @@
                                        desc, parent)
         self._create_lease_file()
         self._create_volume_file(vol_format, size)
+        self._initialize_volume(vol_format, size)
 
     def commit(self):
         try:
@@ -356,18 +370,22 @@
     def create(self, size, vol_format, disk_type, desc, parent=None,
                initial_size=None):
         prealloc = self.get_volume_preallocation(vol_format)
-        self._validate_create_params(vol_format, parent, prealloc)
+        self._validate_create_params(size, vol_format, disk_type, desc, parent,
+                                     initial_size, prealloc)
 
         if initial_size is not None and vol_format != volume.COW_FORMAT:
             self.log.debug("initial_size is supported only for COW volumes")
             raise se.InvalidParameterException("initial_size", initial_size)
 
+        initial_size_blk = (None if initial_size is None else
+                            initial_size / volume.BLOCK_SIZE)
         lv_size = self.vol_class.calculate_volume_alloc_size(
-            prealloc, size / volume.BLOCK_SIZE, None)
+            prealloc, size / volume.BLOCK_SIZE, initial_size_blk)
         parent_vol_id = parent.vol_id if parent else volume.BLANK_UUID
         self._create_lv_artifact(parent_vol_id, lv_size)
         meta_id = self._create_metadata(size, vol_format, prealloc, disk_type,
                                         desc, parent_vol_id)
+        self._initialize_volume(vol_format, size)
         self._create_lease(meta_id)
 
     def commit(self):
@@ -380,6 +398,24 @@
         else:
             return volume.SPARSE_VOL
 
+    def _validate_create_params(self, size, vol_format, disk_type, desc,
+                                parent, initial_size, prealloc):
+        super(BlockVolumeArtifacts, self)._validate_create_params(
+            size, vol_format, disk_type, desc, parent, initial_size, prealloc)
+
+        if size % volume.BLOCK_SIZE != 0:
+            self.log.debug("size %s not a multiple of the block size", size)
+            raise se.InvalidParameterException("size", size)
+
+        if initial_size and initial_size % volume.BLOCK_SIZE != 0:
+            self.log.debug("initial_size %s not a multiple of the block size",
+                           initial_size)
+            raise se.InvalidParameterException("initial_size", initial_size)
+
+        if initial_size is not None and vol_format != volume.COW_FORMAT:
+            self.log.debug("initial_size is supported only for COW volumes")
+            raise se.InvalidParameterException("initial_size", initial_size)
+
     def _create_lv_artifact(self, parent_vol_id, lv_size):
         try:
             lv = lvm.getLV(self.sd_manifest.sdUUID, self.vol_id)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If31e013187830322d4c9ebf0d76b4d6d4d81445d
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