]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Windows SMBFS: Fix image resize errors during volume creation
authorLucian Petrut <lpetrut@cloudbasesolutions.com>
Fri, 27 Mar 2015 13:00:28 +0000 (15:00 +0200)
committerLucian Petrut <lpetrut@cloudbasesolutions.com>
Mon, 11 May 2015 13:08:35 +0000 (16:08 +0300)
When creating volumes from images or snapshots, the corresponding
virtual disk image is resized up to the volume size.

At the moment, the resize is performed even if the image already
has the requested size. This can raise an excepion in some
environments.

This patch checks the image size and performs the resize only if
it's required.

Change-Id: Ib65f0e8d69f93ae21975eb46ba24e085c06e4e34
Closes-Bug: #1437306

cinder/tests/unit/test_smbfs.py
cinder/tests/unit/windows/test_smbfs.py
cinder/volume/drivers/windows/smbfs.py
cinder/volume/drivers/windows/windows_utils.py

index d97ce97cd52fcf073007437d6af1ed820c646164..05cf1f2a71999ce5ecbe64af0c70523816793e8b 100644 (file)
@@ -60,8 +60,8 @@ class SmbFsTestCase(test.TestCase):
 
     def setUp(self):
         super(SmbFsTestCase, self).setUp()
-        smbfs.SmbfsDriver.__init__ = lambda x: None
-        self._smbfs_driver = smbfs.SmbfsDriver()
+
+        self._smbfs_driver = smbfs.SmbfsDriver(configuration=mock.Mock())
         self._smbfs_driver._remotefsclient = mock.Mock()
         self._smbfs_driver._local_volume_dir = mock.Mock(
             return_value=self._FAKE_MNT_POINT)
index 5fa4061b987702a1ab6184216b656770210aaca1..9e28634eb9ed397f261916b40354dafa3db833fb 100644 (file)
@@ -12,7 +12,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import importlib
 import os
 import sys
 
@@ -21,6 +20,7 @@ import mock
 from cinder import exception
 from cinder.image import image_utils
 from cinder import test
+from cinder.volume.drivers.windows import smbfs
 
 
 class WindowsSmbFsTestCase(test.TestCase):
@@ -53,25 +53,19 @@ class WindowsSmbFsTestCase(test.TestCase):
         super(WindowsSmbFsTestCase, self).setUp()
         self._mock_wmi = mock.MagicMock()
 
-        self._platform_patcher = mock.patch('sys.platform', 'win32')
+        mock.patch('sys.platform', 'win32').start()
+        mock.patch.dict(sys.modules, ctypes=mock.DEFAULT).start()
+        mock.patch('__builtin__.wmi', create=True).start()
 
-        mock.patch.dict(sys.modules, wmi=self._mock_wmi,
-                        ctypes=self._mock_wmi).start()
-
-        self._platform_patcher.start()
-        # self._wmi_patcher.start()
         self.addCleanup(mock.patch.stopall)
 
-        smbfs = importlib.import_module(
-            'cinder.volume.drivers.windows.smbfs')
-        smbfs.WindowsSmbfsDriver.__init__ = lambda x: None
-        self._smbfs_driver = smbfs.WindowsSmbfsDriver()
+        self._smbfs_driver = smbfs.WindowsSmbfsDriver(
+            configuration=mock.Mock())
         self._smbfs_driver._remotefsclient = mock.Mock()
         self._smbfs_driver._delete = mock.Mock()
         self._smbfs_driver.local_path = mock.Mock(
             return_value=self._FAKE_VOLUME_PATH)
         self._smbfs_driver.vhdutils = mock.Mock()
-        self._smbfs_driver._check_os_platform = mock.Mock()
 
     def _test_create_volume(self, volume_exists=False, volume_format='vhdx'):
         self._smbfs_driver.create_dynamic_vhd = mock.MagicMock()
@@ -241,6 +235,7 @@ class WindowsSmbFsTestCase(test.TestCase):
             return_value=self._FAKE_VOLUME_PATH)
         drv.configuration = mock.MagicMock()
         drv.configuration.volume_dd_blocksize = mock.sentinel.block_size
+        drv._extend_vhd_if_needed = mock.Mock()
 
         with mock.patch.object(image_utils,
                                'fetch_to_volume_format') as fake_fetch:
@@ -254,6 +249,8 @@ class WindowsSmbFsTestCase(test.TestCase):
                 mock.sentinel.image_id,
                 self._FAKE_VOLUME_PATH, mock.sentinel.volume_format,
                 mock.sentinel.block_size)
+            drv._extend_vhd_if_needed.assert_called_once_with(
+                self._FAKE_VOLUME_PATH, self._FAKE_VOLUME['size'])
 
     def test_copy_volume_from_snapshot(self):
         drv = self._smbfs_driver
@@ -272,6 +269,7 @@ class WindowsSmbFsTestCase(test.TestCase):
             return_value=fake_img_info)
         drv.local_path = mock.Mock(
             return_value=mock.sentinel.new_volume_path)
+        drv._extend_vhd_if_needed = mock.Mock()
 
         drv._copy_volume_from_snapshot(
             self._FAKE_SNAPSHOT, self._FAKE_VOLUME,
@@ -281,8 +279,8 @@ class WindowsSmbFsTestCase(test.TestCase):
         drv.vhdutils.convert_vhd.assert_called_once_with(
             self._FAKE_VOLUME_PATH,
             mock.sentinel.new_volume_path)
-        drv.vhdutils.resize_vhd.assert_called_once_with(
-            mock.sentinel.new_volume_path, self._FAKE_VOLUME['size'] << 30)
+        drv._extend_vhd_if_needed.assert_called_once_with(
+            mock.sentinel.new_volume_path, self._FAKE_VOLUME['size'])
 
     def test_rebase_img(self):
         self._smbfs_driver._rebase_img(
@@ -290,3 +288,41 @@ class WindowsSmbFsTestCase(test.TestCase):
             self._FAKE_VOLUME_NAME + '.vhdx', 'vhdx')
         self._smbfs_driver.vhdutils.reconnect_parent.assert_called_once_with(
             self._FAKE_SNAPSHOT_PATH, self._FAKE_VOLUME_PATH)
+
+    def _test_extend_vhd_if_needed(self, virtual_size_gb, requested_size_gb):
+        drv = self._smbfs_driver
+        virtual_size_bytes = virtual_size_gb << 30
+        requested_size_bytes = requested_size_gb << 30
+
+        virtual_size_dict = {'VirtualSize': virtual_size_bytes}
+        drv.vhdutils.get_vhd_size = mock.Mock(return_value=virtual_size_dict)
+
+        if virtual_size_gb > requested_size_gb:
+            self.assertRaises(exception.VolumeBackendAPIException,
+                              drv._extend_vhd_if_needed,
+                              mock.sentinel.vhd_path,
+                              requested_size_gb)
+        else:
+            drv._extend_vhd_if_needed(mock.sentinel.vhd_path,
+                                      requested_size_gb)
+
+            if virtual_size_gb < requested_size_gb:
+                drv.vhdutils.resize_vhd.assert_called_once_with(
+                    mock.sentinel.vhd_path, requested_size_bytes)
+            else:
+                self.assertFalse(drv.vhdutils.resize_vhd.called)
+
+        drv.vhdutils.get_vhd_size.assert_called_once_with(
+            mock.sentinel.vhd_path)
+
+    def test_extend_vhd_if_needed_bigger_size(self):
+        self._test_extend_vhd_if_needed(virtual_size_gb=1,
+                                        requested_size_gb=2)
+
+    def test_extend_vhd_if_needed_equal_size(self):
+        self._test_extend_vhd_if_needed(virtual_size_gb=1,
+                                        requested_size_gb=1)
+
+    def test_extend_vhd_if_needed_smaller_size(self):
+        self._test_extend_vhd_if_needed(virtual_size_gb=2,
+                                        requested_size_gb=1)
index e6f98e93c83b53efa5570e62b688680ba6a29300..480035f2bf3044afd3dbb4635a89b60ce7251091 100644 (file)
@@ -30,6 +30,7 @@ from cinder import utils
 from cinder.volume.drivers import smbfs
 from cinder.volume.drivers.windows import remotefs
 from cinder.volume.drivers.windows import vhdutils
+from cinder.volume.drivers.windows import windows_utils
 
 VERSION = '1.1.0'
 
@@ -57,6 +58,7 @@ class WindowsSmbfsDriver(smbfs.SmbfsDriver):
             'cifs', root_helper=None, smbfs_mount_point_base=self.base,
             smbfs_mount_options=opts)
         self.vhdutils = vhdutils.VHDUtils()
+        self._windows_utils = windows_utils.WindowsUtils()
 
     def do_setup(self, context):
         self._check_os_platform()
@@ -216,8 +218,7 @@ class WindowsSmbfsDriver(smbfs.SmbfsDriver):
             volume_path, volume_format,
             self.configuration.volume_dd_blocksize)
 
-        self.vhdutils.resize_vhd(volume_path,
-                                 volume['size'] * units.Gi)
+        self._extend_vhd_if_needed(self.local_path(volume), volume['size'])
 
     def _copy_volume_from_snapshot(self, snapshot, volume, volume_size):
         """Copy data from snapshot to destination volume."""
@@ -244,4 +245,14 @@ class WindowsSmbfsDriver(smbfs.SmbfsDriver):
         self._delete(volume_path)
         self.vhdutils.convert_vhd(snapshot_path,
                                   volume_path)
-        self.vhdutils.resize_vhd(volume_path, volume_size * units.Gi)
+        self._extend_vhd_if_needed(volume_path, volume_size)
+
+    def _extend_vhd_if_needed(self, vhd_path, new_size_gb):
+        old_size_bytes = self.vhdutils.get_vhd_size(vhd_path)['VirtualSize']
+        new_size_bytes = new_size_gb * units.Gi
+
+        # This also ensures we're not attempting to shrink the image.
+        is_resize_needed = self._windows_utils.is_resize_needed(
+            vhd_path, new_size_bytes, old_size_bytes)
+        if is_resize_needed:
+            self.vhdutils.resize_vhd(vhd_path, new_size_bytes)
index 3339055701e69c757eb01fc19671b21fcc9916dd..2b22bc3ba5613e6c3bed8584bc95efd057bcae98 100644 (file)
@@ -18,6 +18,7 @@ Utility class for Windows Storage Server 2012 volume related operations.
 
 import ctypes
 import os
+import sys
 
 from oslo_config import cfg
 from oslo_log import log as logging
@@ -27,7 +28,7 @@ from cinder.i18n import _, _LI
 from cinder.volume.drivers.windows import constants
 
 # Check needed for unit testing on Unix
-if os.name == 'nt':
+if sys.platform == 'win32':
     import wmi
 
     from ctypes import wintypes