Windows iSCSI: fix volume clone

In some environments, volume cloning fails when using the Windows
iSCSI volume driver. This involves creating a new VHD image and
copying the data from the source volume on top.

This can raise an error, caused by the fact that the destination
path exists.

This patch slightly changes the workflow used when cloning an image.
Instead of first creating a new WT disk and then copying the cloned
image data on top, the source image is copied, resized if needed
and then imported as a WT disk.

Change-Id: I07d45fa9324a34ef45ed2aa8051338ee6a0f8a5b
Closes-Bug: #1435865

diff --git a/cinder/tests/windows/test_windows.py b/cinder/tests/windows/test_windows.py
index cd54729f7d8dcdf16e9ef3af6ecbbff6b015d4dd..759cb747b93561c6e4a8bcebecb89e5b58898144 100644 (file)
--- a/cinder/tests/windows/test_windows.py
+++ b/cinder/tests/windows/test_windows.py
@@ -349,20 +349,29 @@ class TestWindowsDriver(test.TestCase):
 
         volume = db_fakes.get_fake_volume_info()
         volume_cloned = db_fakes.get_fake_volume_info_cloned()
+        new_vhd_path = self.fake_local_path(volume)
+        src_vhd_path = self.fake_local_path(volume_cloned)
 
         self.mox.StubOutWithMock(windows_utils.WindowsUtils,
-                                 'create_volume')
+                                 'copy_vhd_disk')
+        self.mox.StubOutWithMock(windows_utils.WindowsUtils,
+                                 'import_wt_disk')
+        self.mox.StubOutWithMock(vhdutils.VHDUtils,
+                                 'resize_vhd')
 
+        self.stubs.Set(drv.utils,
+                       'is_resize_needed',
+                       lambda vhd_path, new_size, old_size: True)
         self.stubs.Set(drv, 'local_path', self.fake_local_path)
 
-        windows_utils.WindowsUtils.create_volume(mox.IgnoreArg(),
-                                                 mox.IgnoreArg(),
-                                                 mox.IgnoreArg())
-
-        self.mox.StubOutWithMock(windows_utils.WindowsUtils,
-                                 'copy_vhd_disk')
-        windows_utils.WindowsUtils.copy_vhd_disk(self.fake_local_path(
-            volume_cloned), self.fake_local_path(volume))
+        windows_utils.WindowsUtils.copy_vhd_disk(src_vhd_path,
+                                                 new_vhd_path)
+        drv.utils.is_resize_needed(new_vhd_path,
+                                   volume['size'],
+                                   volume_cloned['size'])
+        vhdutils.VHDUtils.resize_vhd(new_vhd_path, volume['size'] << 30)
+        windows_utils.WindowsUtils.import_wt_disk(new_vhd_path,
+                                                  volume['name'])
 
         self.mox.ReplayAll()
 

diff --git a/cinder/tests/windows/test_windows_utils.py b/cinder/tests/windows/test_windows_utils.py
index 9e2377b2042ec9a59c58c8c6b8689819963ae044..ae2008a6ec25d82bd2adc5fa06b15eca020c35c5 100644 (file)
--- a/cinder/tests/windows/test_windows_utils.py
+++ b/cinder/tests/windows/test_windows_utils.py
@@ -26,6 +26,7 @@ class WindowsUtilsTestCase(test.TestCase):
 
         windows_utils.WindowsUtils.__init__ = lambda x: None
         self.wutils = windows_utils.WindowsUtils()
+        self.wutils._conn_wmi = mock.Mock()
         self.wutils._conn_cimv2 = mock.MagicMock()
 
     def _test_copy_vhd_disk(self, source_exists=True, copy_failed=False):
@@ -59,3 +60,33 @@ class WindowsUtilsTestCase(test.TestCase):
 
     def test_copy_vhd_disk_copy_failed(self):
         self._test_copy_vhd_disk(copy_failed=True)
+
+    @mock.patch.object(windows_utils, 'wmi', create=True)
+    def test_import_wt_disk_exception(self, mock_wmi):
+        mock_wmi.x_wmi = Exception
+        mock_import_disk = self.wutils._conn_wmi.WT_Disk.ImportWTDisk
+        mock_import_disk.side_effect = mock_wmi.x_wmi
+
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.wutils.import_wt_disk,
+                          mock.sentinel.vhd_path,
+                          mock.sentinel.vol_name)
+        mock_import_disk.assert_called_once_with(
+            DevicePath=mock.sentinel.vhd_path,
+            Description=mock.sentinel.vol_name)
+
+    def test_check_if_resize_is_needed_bigger_requested_size(self):
+        ret_val = self.wutils.is_resize_needed(
+            mock.sentinel.vhd_path, 1, 0)
+        self.assertTrue(ret_val)
+
+    def test_check_if_resize_is_needed_equal_requested_size(self):
+        ret_val = self.wutils.is_resize_needed(
+            mock.sentinel.vhd_path, 1, 1)
+        self.assertFalse(ret_val)
+
+    def test_check_if_resize_is_needed_smaller_requested_size(self):
+        self.assertRaises(
+            exception.VolumeBackendAPIException,
+            self.wutils.is_resize_needed,
+            mock.sentinel.vhd_path, 1, 2)

diff --git a/cinder/volume/drivers/windows/windows.py b/cinder/volume/drivers/windows/windows.py
index b3d7f3a68f263418a092cd775725a15c1bef0e33..fee2a206ed2ea4d79eed0c8f0afdc30749818d84 100644 (file)
--- a/cinder/volume/drivers/windows/windows.py
+++ b/cinder/volume/drivers/windows/windows.py
@@ -208,11 +208,20 @@ class WindowsDriver(driver.ISCSIDriver):
 
     def create_cloned_volume(self, volume, src_vref):
         """Creates a clone of the specified volume."""
-        # Create a new volume
-        # Copy VHD file of the volume to clone to the created volume
-        self.create_volume(volume)
-        self.utils.copy_vhd_disk(self.local_path(src_vref),
-                                 self.local_path(volume))
+        vol_name = volume['name']
+        vol_size = volume['size']
+        src_vol_size = src_vref['size']
+
+        new_vhd_path = self.local_path(volume)
+        src_vhd_path = self.local_path(src_vref)
+
+        self.utils.copy_vhd_disk(src_vhd_path,
+                                 new_vhd_path)
+
+        if self.utils.is_resize_needed(new_vhd_path, vol_size, src_vol_size):
+            self.vhdutils.resize_vhd(new_vhd_path, vol_size << 30)
+
+        self.utils.import_wt_disk(new_vhd_path, vol_name)
 
     def get_volume_stats(self, refresh=False):
         """Get volume stats.

diff --git a/cinder/volume/drivers/windows/windows_utils.py b/cinder/volume/drivers/windows/windows_utils.py
index 24c801d23f4226e554f542716c0b569e970c15a1..d4275a0eb030a29b0ccd77e76066af6a93305694 100644 (file)
--- a/cinder/volume/drivers/windows/windows_utils.py
+++ b/cinder/volume/drivers/windows/windows_utils.py
@@ -158,6 +158,19 @@ class WindowsUtils(object):
             LOG.error(err_msg)
             raise exception.VolumeBackendAPIException(data=err_msg)
 
+    def import_wt_disk(self, vhd_path, vol_name):
+        """Import a vhd/x image to be used by Windows iSCSI targets"""
+        try:
+            self._conn_wmi.WT_Disk.ImportWTDisk(DevicePath=vhd_path,
+                                                Description=vol_name)
+        except wmi.x_wmi as exc:
+            err_msg = (_("Failed to import disk: %(vhd_path)s. "
+                         "WMI exception: %(exc)s") %
+                       {'vhd_path': vhd_path,
+                        'exc': exc})
+            LOG.error(err_msg)
+            raise exception.VolumeBackendAPIException(data=err_msg)
+
     def change_disk_status(self, vol_name, enabled):
         try:
             cl = self._conn_wmi.WT_Disk(Description=vol_name)[0]
@@ -336,6 +349,20 @@ class WindowsUtils(object):
             LOG.error(err_msg)
             raise exception.VolumeBackendAPIException(data=err_msg)
 
+    def is_resize_needed(self, vhd_path, new_size, old_size):
+        if new_size > old_size:
+            return True
+        elif old_size > new_size:
+            err_msg = (_("Cannot resize image %(vhd_path)s "
+                         "to a smaller size. "
+                         "Image size: %(old_size)s, "
+                         "Requested size: %(new_size)s") %
+                       {'vhd_path': vhd_path,
+                        'old_size': old_size,
+                        'new_size': new_size})
+            raise exception.VolumeBackendAPIException(data=err_msg)
+        return False
+
     def extend(self, vol_name, additional_size):
         """Extend an existing volume."""
         try: