Fixes disk size issue during image boot on Hyper-V

Fixes bug: 1135155

The local root disk size provided in the instance flavor was not properly
taken into account in the Hyper-V driver, this patches provides a
fix for this feature.

In order to resize VHD images with differencing disks (CoW), the cached
base disk is copied to a new file, which is resized to the size
specified in the flavor and cached. This is necessary due to the fact that
differencing VHD disks cannot be resized.

The procedure described above is applied during image spawn, resize and
live migration as needed.

Trying to spawn an instance with a local root disk size smaller than the
image VHD max. internal size will result in an error.

Change-Id: I04f18f0e25c92ed1e1f9f6f18750329a3f9f1711

7 files changed, 170 insertions, 55 deletions

diff --git a/nova/tests/hyperv/db_fakes.py b/nova/tests/hyperv/db_fakes.py
index 7169edf8d..5152bd035 100644
--- a/nova/tests/hyperv/db_fakes.py
+++ b/nova/tests/hyperv/db_fakes.py
@@ -41,7 +41,7 @@ def get_fake_instance_data(name, project_id, user_id):
             {'name': 'm1.tiny',
              'memory_mb': 512,
              'vcpus': 1,
-             'root_gb': 0,
+             'root_gb': 1024,
              'flavorid': 1,
              'rxtx_factor': 1}
             }
@@ -69,8 +69,6 @@ def get_fake_volume_info_data(target_portal, volume_id):
             'target_portal': target_portal,
             'target_lun': 1,
             'auth_method': 'CHAP',
-            'auth_method': 'fake',
-            'auth_method': 'fake',
         }
     }
 
@@ -121,6 +119,9 @@ def stub_out_db_instance_api(stubs):
         def __getitem__(self, key):
             return self.get(key)
 
+        def __setitem__(self, key, value):
+            self.values[key] = value
+
         def __str__(self):
             return str(self.values)
 
diff --git a/nova/tests/test_hypervapi.py b/nova/tests/test_hypervapi.py
index aaceff8ec..5dc77b911 100644
--- a/nova/tests/test_hypervapi.py
+++ b/nova/tests/test_hypervapi.py
@@ -154,6 +154,8 @@ class HyperVAPITestCase(test.TestCase):
         self._mox.StubOutWithMock(vhdutils.VHDUtils, 'merge_vhd')
         self._mox.StubOutWithMock(vhdutils.VHDUtils, 'get_vhd_parent_path')
         self._mox.StubOutWithMock(vhdutils.VHDUtils, 'get_vhd_info')
+        self._mox.StubOutWithMock(vhdutils.VHDUtils, 'resize_vhd')
+        self._mox.StubOutWithMock(vhdutils.VHDUtils, 'validate_vhd')
 
         self._mox.StubOutWithMock(hostutils.HostUtils, 'get_cpus_info')
         self._mox.StubOutWithMock(hostutils.HostUtils,
@@ -567,6 +569,8 @@ class HyperVAPITestCase(test.TestCase):
         self.flags(use_cow_images=cow)
 
         instance_data = self._get_instance_data()
+        instance = db.instance_create(self._context, instance_data)
+        instance['system_metadata'] = {}
 
         network_info = fake_network.fake_get_instance_nw_info(self.stubs,
                                                               spectacular=True)
@@ -579,10 +583,14 @@ class HyperVAPITestCase(test.TestCase):
                                                                   None)
             m.AndReturn(False)
 
-            vhdutils.VHDUtils.get_vhd_info(mox.Func(self._check_img_path))
+            m = vhdutils.VHDUtils.get_vhd_info(mox.Func(self._check_img_path))
+            m.AndReturn({'MaxInternalSize': 1024})
+
+            fake.PathUtils.copyfile(mox.IsA(str), mox.IsA(str))
+            vhdutils.VHDUtils.resize_vhd(mox.IsA(str), mox.IsA(object))
 
         self._mox.ReplayAll()
-        self._conn.pre_live_migration(self._context, instance_data,
+        self._conn.pre_live_migration(self._context, instance,
                                       None, network_info)
         self._mox.VerifyAll()
 
@@ -697,6 +705,7 @@ class HyperVAPITestCase(test.TestCase):
 
         self._instance_data = self._get_instance_data()
         instance = db.instance_create(self._context, self._instance_data)
+        instance['system_metadata'] = {}
 
         image = db_fakes.get_fake_image_data(self._project_id, self._user_id)
 
@@ -763,12 +772,16 @@ class HyperVAPITestCase(test.TestCase):
         m.AndReturn(boot_from_volume)
 
         if not boot_from_volume:
-            vhdutils.VHDUtils.get_vhd_info(mox.Func(self._check_img_path))
+            m = vhdutils.VHDUtils.get_vhd_info(mox.Func(self._check_img_path))
+            m.AndReturn({'MaxInternalSize': 1024})
 
             if cow:
-                vhdutils.VHDUtils.create_differencing_vhd(
-                    mox.IsA(str), mox.Func(self._check_img_path))
+                fake.PathUtils.copyfile(mox.IsA(str), mox.IsA(str))
+                vhdutils.VHDUtils.resize_vhd(mox.IsA(str), mox.IsA(object))
+                vhdutils.VHDUtils.create_differencing_vhd(mox.IsA(str),
+                                                          mox.IsA(str))
             else:
+                vhdutils.VHDUtils.resize_vhd(mox.IsA(str), mox.IsA(object))
                 fake.PathUtils.copyfile(mox.IsA(str), mox.IsA(str))
 
         self._setup_create_instance_mocks(setup_vif_mocks_func,
@@ -1009,6 +1022,7 @@ class HyperVAPITestCase(test.TestCase):
     def test_finish_migration(self):
         self._instance_data = self._get_instance_data()
         instance = db.instance_create(self._context, self._instance_data)
+        instance['system_metadata'] = {}
         network_info = fake_network.fake_get_instance_nw_info(
             self.stubs, spectacular=True)
 
@@ -1032,6 +1046,12 @@ class HyperVAPITestCase(test.TestCase):
 
         vhdutils.VHDUtils.reconnect_parent_vhd(mox.IsA(str), mox.IsA(str))
 
+        m = vhdutils.VHDUtils.get_vhd_info(mox.IsA(str))
+        m.AndReturn({'MaxInternalSize': 1024})
+
+        m = fake.PathUtils.exists(mox.IsA(str))
+        m.AndReturn(True)
+
         self._set_vm_name(instance['name'])
         self._setup_create_instance_mocks(None, False)
 
diff --git a/nova/virt/hyperv/imagecache.py b/nova/virt/hyperv/imagecache.py
index 93ea32b25..5d68bab61 100644
--- a/nova/virt/hyperv/imagecache.py
+++ b/nova/virt/hyperv/imagecache.py
@@ -19,15 +19,21 @@ Image caching and management.
 """
 import os
 
+from nova.compute import instance_types
+from nova.openstack.common import excutils
 from nova.openstack.common import lockutils
 from nova.openstack.common import log as logging
 from nova.virt.hyperv import pathutils
 from nova.virt.hyperv import vhdutils
 from nova.virt.hyperv import vmutils
 from nova.virt import images
+from oslo.config import cfg
 
 LOG = logging.getLogger(__name__)
 
+CONF = cfg.CONF
+CONF.import_opt('use_cow_images', 'nova.virt.driver')
+
 
 class ImageCache(object):
     def __init__(self):
@@ -36,12 +42,59 @@ class ImageCache(object):
 
     def _validate_vhd_image(self, vhd_path):
         try:
-            self._vhdutils.get_vhd_info(vhd_path)
+            self._vhdutils.validate_vhd(vhd_path)
         except Exception as ex:
             LOG.exception(ex)
             raise vmutils.HyperVException(_('The image is not a valid VHD: %s')
                                           % vhd_path)
 
+    def _get_root_vhd_size_gb(self, instance):
+        try:
+            # In case of resizes we need the old root disk size
+            old_instance_type = instance_types.extract_instance_type(
+                instance, prefix='old_')
+            return old_instance_type['root_gb']
+        except KeyError:
+            return instance['root_gb']
+
+    def _resize_and_cache_vhd(self, instance, vhd_path):
+        vhd_info = self._vhdutils.get_vhd_info(vhd_path)
+        vhd_size = vhd_info['MaxInternalSize']
+
+        root_vhd_size_gb = self._get_root_vhd_size_gb(instance)
+        root_vhd_size = root_vhd_size_gb * 1024 ** 3
+
+        if root_vhd_size < vhd_size:
+            raise vmutils.HyperVException(_("Cannot resize the image to a "
+                                            "size smaller than the VHD max. "
+                                            "internal size: %(vhd_size)s. "
+                                            "Requested disk size: "
+                                            "%(root_vhd_size)s") % locals())
+        if root_vhd_size > vhd_size:
+            path_parts = os.path.splitext(vhd_path)
+            resized_vhd_path = '%s_%s%s' % (path_parts[0],
+                                            root_vhd_size_gb,
+                                            path_parts[1])
+
+            @lockutils.synchronized(resized_vhd_path, 'nova-')
+            def copy_and_resize_vhd():
+                if not self._pathutils.exists(resized_vhd_path):
+                    try:
+                        LOG.debug(_("Copying VHD %(vhd_path)s to "
+                                    "%(resized_vhd_path)s") % locals())
+                        self._pathutils.copyfile(vhd_path, resized_vhd_path)
+                        LOG.debug(_("Resizing VHD %(resized_vhd_path)s to new "
+                                    "size %(root_vhd_size)s") % locals())
+                        self._vhdutils.resize_vhd(resized_vhd_path,
+                                                  root_vhd_size)
+                    except Exception:
+                        with excutils.save_and_reraise_exception():
+                            if self._pathutils.exists(resized_vhd_path):
+                                self._pathutils.remove(resized_vhd_path)
+
+            copy_and_resize_vhd()
+            return resized_vhd_path
+
     def get_cached_image(self, context, instance):
         image_id = instance['image_ref']
 
@@ -51,10 +104,22 @@ class ImageCache(object):
         @lockutils.synchronized(vhd_path, 'nova-')
         def fetch_image_if_not_existing():
             if not self._pathutils.exists(vhd_path):
-                images.fetch(context, image_id, vhd_path,
-                             instance['user_id'],
-                             instance['project_id'])
-                self._validate_vhd_image(vhd_path)
+                try:
+                    images.fetch(context, image_id, vhd_path,
+                                 instance['user_id'],
+                                 instance['project_id'])
+                except Exception:
+                    with excutils.save_and_reraise_exception():
+                        if self._pathutils.exists(vhd_path):
+                            self._pathutils.remove(vhd_path)
 
         fetch_image_if_not_existing()
+
+        if CONF.use_cow_images:
+            # Resize the base VHD image as it's not possible to resize a
+            # differencing VHD.
+            resized_vhd_path = self._resize_and_cache_vhd(instance, vhd_path)
+            if resized_vhd_path:
+                return resized_vhd_path
+
         return vhd_path
diff --git a/nova/virt/hyperv/migrationops.py b/nova/virt/hyperv/migrationops.py
index 8d5b5e90c..07bf453e5 100644
--- a/nova/virt/hyperv/migrationops.py
+++ b/nova/virt/hyperv/migrationops.py
@@ -137,16 +137,15 @@ class MigrationOps(object):
         self._revert_migration_files(instance_name)
 
         if self._volumeops.ebs_root_in_block_devices(block_device_info):
-            boot_vhd_path = None
+            root_vhd_path = None
         else:
-            boot_vhd_path = self._pathutils.get_vhd_path(instance_name)
+            root_vhd_path = self._pathutils.get_vhd_path(instance_name)
         self._vmops.create_instance(instance, network_info, block_device_info,
-                                    boot_vhd_path)
+                                    root_vhd_path)
 
         self._vmops.power_on(instance)
 
     def _merge_base_vhd(self, diff_vhd_path, base_vhd_path):
-
         base_vhd_copy_path = os.path.join(os.path.dirname(diff_vhd_path),
                                           os.path.basename(base_vhd_path))
         try:
@@ -183,10 +182,6 @@ class MigrationOps(object):
 
     def _check_base_disk(self, context, instance, diff_vhd_path,
                          src_base_disk_path):
-        base_disk_file_name = os.path.basename(src_base_disk_path)
-        if os.path.splitext(base_disk_file_name)[0] != instance["image_ref"]:
-            raise vmutils.HyperVException(_("Unexpected base VHD path"))
-
         base_vhd_path = self._imagecache.get_cached_image(context, instance)
 
         # If the location of the base host differs between source
@@ -206,28 +201,28 @@ class MigrationOps(object):
         instance_name = instance['name']
 
         if self._volumeops.ebs_root_in_block_devices(block_device_info):
-            boot_vhd_path = None
+            root_vhd_path = None
         else:
-            boot_vhd_path = self._pathutils.get_vhd_path(instance_name)
-            if not self._pathutils.exists(boot_vhd_path):
+            root_vhd_path = self._pathutils.get_vhd_path(instance_name)
+            if not self._pathutils.exists(root_vhd_path):
                 raise vmutils.HyperVException(_("Cannot find boot VHD "
-                                                "file: %s") % boot_vhd_path)
+                                                "file: %s") % root_vhd_path)
 
-            vhd_info = self._vhdutils.get_vhd_info(boot_vhd_path)
+            vhd_info = self._vhdutils.get_vhd_info(root_vhd_path)
             src_base_disk_path = vhd_info.get("ParentPath")
             if src_base_disk_path:
-                self._check_base_disk(context, instance, boot_vhd_path,
+                self._check_base_disk(context, instance, root_vhd_path,
                                       src_base_disk_path)
 
             if resize_instance:
                 curr_size = vhd_info['MaxInternalSize']
-                new_size = instance['root_gb'] * 1024 * 1024 * 1024
+                new_size = instance['root_gb'] * 1024 ** 3
                 if new_size < curr_size:
                     raise vmutils.HyperVException(_("Cannot resize a VHD to a "
                                                     "smaller size"))
                 elif new_size > curr_size:
-                    self._resize_vhd(boot_vhd_path, new_size)
+                    self._resize_vhd(root_vhd_path, new_size)
 
         self._vmops.create_instance(instance, network_info, block_device_info,
-                                    boot_vhd_path)
+                                    root_vhd_path)
         self._vmops.power_on(instance)
diff --git a/nova/virt/hyperv/pathutils.py b/nova/virt/hyperv/pathutils.py
index 1297cd1ed..7ee4d06ea 100644
--- a/nova/virt/hyperv/pathutils.py
+++ b/nova/virt/hyperv/pathutils.py
@@ -18,9 +18,9 @@
 import os
 import shutil
 
-from oslo.config import cfg
-
+from eventlet.green import subprocess
 from nova.openstack.common import log as logging
+from oslo.config import cfg
 
 LOG = logging.getLogger(__name__)
 
@@ -58,10 +58,17 @@ class PathUtils(object):
         os.rename(src, dest)
 
     def copyfile(self, src, dest):
-        shutil.copyfile(src, dest)
+        self.copy(src, dest)
 
     def copy(self, src, dest):
-        shutil.copy(src, dest)
+        # With large files this is 2x-3x faster than shutil.copy(src, dest),
+        # especially when copying to a UNC target.
+        # shutil.copyfileobj(...) with a proper buffer is better than
+        # shutil.copy(...) but still 20% slower than a shell copy.
+        # It can be replaced with Win32 API calls to avoid the process
+        # spawning overhead.
+        if subprocess.call(['cmd.exe', '/C', 'copy', '/Y', src, dest]):
+            raise IOError(_('The file copy from %(src)s to %(dest)s failed'))
 
     def rmtree(self, path):
         shutil.rmtree(path)
diff --git a/nova/virt/hyperv/vhdutils.py b/nova/virt/hyperv/vhdutils.py
index 1e529807d..c21799051 100644
--- a/nova/virt/hyperv/vhdutils.py
+++ b/nova/virt/hyperv/vhdutils.py
@@ -31,6 +31,13 @@ class VHDUtils(object):
         if sys.platform == 'win32':
             self._conn = wmi.WMI(moniker='//./root/virtualization')
 
+    def validate_vhd(self, vhd_path):
+        image_man_svc = self._conn.Msvm_ImageManagementService()[0]
+
+        (job_path, ret_val) = image_man_svc.ValidateVirtualHardDisk(
+            Path=vhd_path)
+        self._vmutils.check_ret_val(ret_val, job_path)
+
     def create_differencing_vhd(self, path, parent_path):
         image_man_svc = self._conn.Msvm_ImageManagementService()[0]
 
diff --git a/nova/virt/hyperv/vmops.py b/nova/virt/hyperv/vmops.py
index f488e993f..48edae7fd 100644
--- a/nova/virt/hyperv/vmops.py
+++ b/nova/virt/hyperv/vmops.py
@@ -111,19 +111,39 @@ class VMOps(object):
                 'num_cpu': info['NumberOfProcessors'],
                 'cpu_time': info['UpTime']}
 
-    def _create_boot_vhd(self, context, instance):
+    def _create_root_vhd(self, context, instance):
         base_vhd_path = self._imagecache.get_cached_image(context, instance)
-        boot_vhd_path = self._pathutils.get_vhd_path(instance['name'])
-
-        if CONF.use_cow_images:
-            LOG.debug(_("Creating differencing VHD. Parent: "
-                        "%(base_vhd_path)s, Target: %(boot_vhd_path)s")
-                      % locals())
-            self._vhdutils.create_differencing_vhd(boot_vhd_path,
-                                                   base_vhd_path)
-        else:
-            self._pathutils.copyfile(base_vhd_path, boot_vhd_path)
-        return boot_vhd_path
+        root_vhd_path = self._pathutils.get_vhd_path(instance['name'])
+
+        try:
+            if CONF.use_cow_images:
+                LOG.debug(_("Creating differencing VHD. Parent: "
+                            "%(base_vhd_path)s, Target: %(root_vhd_path)s")
+                          % locals())
+                self._vhdutils.create_differencing_vhd(root_vhd_path,
+                                                       base_vhd_path)
+            else:
+                LOG.debug(_("Copying VHD image %(base_vhd_path)s to target: "
+                            "%(root_vhd_path)s") % locals())
+                self._pathutils.copyfile(base_vhd_path, root_vhd_path)
+
+                base_vhd_info = self._vhdutils.get_vhd_info(base_vhd_path)
+                base_vhd_size = base_vhd_info['MaxInternalSize']
+                root_vhd_size = instance['root_gb'] * 1024 ** 3
+
+                if root_vhd_size < base_vhd_size:
+                    raise vmutils.HyperVException(_("Cannot resize a VHD to a "
+                                                    "smaller size"))
+                elif root_vhd_size > base_vhd_size:
+                    LOG.debug(_("Resizing VHD %(root_vhd_path)s to new "
+                                "size %(root_vhd_size)s") % locals())
+                    self._vhdutils.resize_vhd(root_vhd_path, root_vhd_size)
+        except Exception:
+            with excutils.save_and_reraise_exception():
+                if self._pathutils.exists(root_vhd_path):
+                    self._pathutils.remove(root_vhd_path)
+
+        return root_vhd_path
 
     def spawn(self, context, instance, image_meta, injected_files,
               admin_password, network_info, block_device_info=None):
@@ -135,13 +155,13 @@ class VMOps(object):
             raise exception.InstanceExists(name=instance_name)
 
         if self._volumeops.ebs_root_in_block_devices(block_device_info):
-            boot_vhd_path = None
+            root_vhd_path = None
         else:
-            boot_vhd_path = self._create_boot_vhd(context, instance)
+            root_vhd_path = self._create_root_vhd(context, instance)
 
         try:
             self.create_instance(instance, network_info, block_device_info,
-                                 boot_vhd_path)
+                                 root_vhd_path)
 
             if configdrive.required_by(instance):
                 self._create_config_drive(instance, injected_files,
@@ -154,7 +174,7 @@ class VMOps(object):
             raise vmutils.HyperVException(_('Spawn instance failed'))
 
     def create_instance(self, instance, network_info,
-                        block_device_info, boot_vhd_path):
+                        block_device_info, root_vhd_path):
         instance_name = instance['name']
 
         self._vmutils.create_vm(instance_name,
@@ -162,9 +182,9 @@ class VMOps(object):
                                 instance['vcpus'],
                                 CONF.limit_cpu_features)
 
-        if boot_vhd_path:
+        if root_vhd_path:
             self._vmutils.attach_ide_drive(instance_name,
-                                           boot_vhd_path,
+                                           root_vhd_path,
                                            0,
                                            0,
                                            constants.IDE_DISK)
@@ -173,7 +193,7 @@ class VMOps(object):
 
         self._volumeops.attach_volumes(block_device_info,
                                        instance_name,
-                                       boot_vhd_path is None)
+                                       root_vhd_path is None)
 
         for vif in network_info:
             LOG.debug(_('Creating nic for instance: %s'), instance_name)
@@ -238,8 +258,8 @@ class VMOps(object):
 
     def _delete_disk_files(self, instance_name):
         self._pathutils.get_instance_dir(instance_name,
-                                          create_dir=False,
-                                          remove_dir=True)
+                                         create_dir=False,
+                                         remove_dir=True)
 
     def destroy(self, instance, network_info=None, block_device_info=None,
                 destroy_disks=True):