resync: "Adds RemoteFX support to the Hyper-V driver"

(cherry-picked from commit a39710244a8f26b0d9d80bffa41c04c84b133f17)

Change-Id: I873053f7e16941ad7b272693ece8b69e49fc7c18

hyperv/nova/constants.py

@@ -69,15 +69,6 @@ IMAGE_PROP_VM_GEN_2 = "hyperv-gen2"
 VM_GEN_1 = 1
 VM_GEN_2 = 2
 
-REMOTEFX_MAX_RES_1024x768 = "1024x768"
-REMOTEFX_MAX_RES_1280x1024 = "1280x1024"
-REMOTEFX_MAX_RES_1600x1200 = "1600x1200"
-REMOTEFX_MAX_RES_1920x1200 = "1920x1200"
-REMOTEFX_MAX_RES_2560x1600 = "2560x1600"
-REMOTEFX_MAX_RES_3840x2160 = "3840x2160"
-
-
-FLAVOR_REMOTE_FX_EXTRA_SPEC_KEY = "hyperv:remotefx"
 
 IMAGE_PROP_INTERACTIVE_SERIAL_PORT = "interactive_serial_port"
 IMAGE_PROP_LOGGING_SERIAL_PORT = "logging_serial_port"
@@ -94,6 +85,10 @@ SERIAL_PORT_TYPES = {
 # logging and interactive sessions.
 DEFAULT_SERIAL_CONSOLE_PORT = 1
 
+FLAVOR_ESPEC_REMOTEFX_RES = 'os:resolution'
+FLAVOR_ESPEC_REMOTEFX_MONITORS = 'os:monitors'
+FLAVOR_ESPEC_REMOTEFX_VRAM = 'os:vram'
+
 SERIAL_CONSOLE_BUFFER_SIZE = 4 * units.Ki
 MAX_CONSOLE_LOG_FILE_SIZE = units.Mi // 2
 

hyperv/nova/hostops.py

@@ -138,20 +138,20 @@ class HostOps(object):
         return objects.NUMATopology(cells=cells)
 
     def _get_remotefx_gpu_info(self):
-        remotefx_total_video_ram = 0
+        total_video_ram = 0
-        remotefx_available_video_ram = 0
+        available_video_ram = 0
 
         if CONF.hyperv.enable_remotefx:
             gpus = self._hostutils.get_remotefx_gpu_info()
             for gpu in gpus:
-                remotefx_total_video_ram += int(gpu['total_video_ram'])
+                total_video_ram += int(gpu['total_video_ram'])
-                remotefx_available_video_ram += int(gpu['available_video_ram'])
+                available_video_ram += int(gpu['available_video_ram'])
         else:
             gpus = []
 
-        return {'remotefx_total_video_ram': remotefx_total_video_ram,
+        return {'total_video_ram': total_video_ram,
-                'remotefx_available_video_ram': remotefx_available_video_ram,
+                'used_video_ram': total_video_ram - available_video_ram,
-                'remotefx_gpu_info': jsonutils.dumps(gpus)}
+                'gpu_info': jsonutils.dumps(gpus)}
 
     def get_available_resource(self):
         """Retrieve resource info.
@@ -178,8 +178,6 @@ class HostOps(object):
                  cpu_topology['cores'] *
                  cpu_topology['threads'])
 
-        gpu_info = self._get_remotefx_gpu_info()
-
         dic = {'vcpus': vcpus,
                'memory_mb': total_mem_mb,
                'memory_mb_used': used_mem_mb,
@@ -194,6 +192,8 @@ class HostOps(object):
                    [(arch.I686, hv_type.HYPERV, vm_mode.HVM),
                     (arch.X86_64, hv_type.HYPERV, vm_mode.HVM)],
                }
+
+        gpu_info = self._get_remotefx_gpu_info()
         dic.update(gpu_info)
 
         numa_topology = self._get_host_numa_topology()

hyperv/nova/vmops.py

@@ -393,11 +393,7 @@ class VMOps(object):
                                 CONF.hyperv.limit_cpu_features,
                                 dynamic_memory_ratio)
 
-        flavor_extra_specs = instance.flavor.extra_specs
+        self._configure_remotefx(instance, vm_gen)
-        remote_fx_config = flavor_extra_specs.get(
-                constants.FLAVOR_REMOTE_FX_EXTRA_SPEC_KEY)
-        if remote_fx_config:
-            self._configure_remotefx(instance, vm_gen, remote_fx_config)
 
         self._vmutils.create_scsi_controller(instance_name)
 
@@ -430,6 +426,45 @@ class VMOps(object):
         self._configure_secure_vm(context, instance, image_meta,
                                   secure_boot_enabled)
 
+    def _configure_remotefx(self, instance, vm_gen):
+        extra_specs = instance.flavor.extra_specs
+        remotefx_max_resolution = extra_specs.get(
+            constants.FLAVOR_ESPEC_REMOTEFX_RES)
+        if not remotefx_max_resolution:
+            # RemoteFX not required.
+            return
+
+        if not CONF.hyperv.enable_remotefx:
+            raise exception.InstanceUnacceptable(
+                _("enable_remotefx configuration option needs to be set to "
+                  "True in order to use RemoteFX."))
+
+        if not self._hostutils.check_server_feature(
+                self._hostutils.FEATURE_RDS_VIRTUALIZATION):
+            raise exception.InstanceUnacceptable(
+                _("The RDS-Virtualization feature must be installed in order "
+                  "to use RemoteFX."))
+
+        if not self._vmutils.vm_gen_supports_remotefx(vm_gen):
+            raise exception.InstanceUnacceptable(
+                _("RemoteFX is not supported on generation %s virtual "
+                  "machines on this version of Windows.") % vm_gen)
+
+        instance_name = instance.name
+        LOG.debug('Configuring RemoteFX for instance: %s', instance_name)
+
+        remotefx_monitor_count = int(extra_specs.get(
+            constants.FLAVOR_ESPEC_REMOTEFX_MONITORS) or 1)
+        remotefx_vram = extra_specs.get(
+            constants.FLAVOR_ESPEC_REMOTEFX_VRAM)
+        vram_bytes = int(remotefx_vram) * units.Mi if remotefx_vram else None
+
+        self._vmutils.enable_remotefx_video_adapter(
+            instance_name,
+            remotefx_monitor_count,
+            remotefx_max_resolution,
+            vram_bytes)
+
     def _attach_root_device(self, instance_name, root_dev_info):
         if root_dev_info['type'] == constants.VOLUME:
             self._volumeops.attach_volume(root_dev_info['connection_info'],
@@ -570,40 +605,6 @@ class VMOps(object):
 
         return memory_per_numa_node, cpus_per_numa_node
 
-    def _configure_remotefx(self, instance, vm_gen, config):
-        if not CONF.hyperv.enable_remotefx:
-            reason = _("enable_remotefx configuration option needs to be set "
-                       "to True in order to use RemoteFX")
-            raise exception.InstanceUnacceptable(instance_id=instance.uuid,
-                                                 reason=reason)
-
-        if not self._hostutils.check_server_feature(
-                        self._hostutils.FEATURE_RDS_VIRTUALIZATION):
-            reason = _("The RDS-Virtualization feature must be installed in "
-                       "order to use RemoteFX")
-            raise exception.InstanceUnacceptable(instance_id=instance.uuid,
-                                                 reason=reason)
-
-        if not self._vmutils.vm_gen_supports_remotefx(vm_gen):
-            reason = _("RemoteFX is not supported on generation %s virtual "
-                       "machines on this version of Windows.") % vm_gen
-            raise exception.InstanceUnacceptable(instance_id=instance.uuid,
-                                                 reason=reason)
-
-        instance_name = instance.name
-        LOG.debug('Configuring RemoteFX for instance: %s', instance_name)
-        remotefx_args = config.split(',')
-        remotefx_max_resolution = remotefx_args[0]
-        remotefx_monitor_count = int(remotefx_args[1])
-        remotefx_vram = remotefx_args[2] if len(remotefx_args) == 3 else None
-        vram_bytes = int(remotefx_vram) * units.Mi if remotefx_vram else None
-
-        self._vmutils.enable_remotefx_video_adapter(
-            instance_name,
-            remotefx_monitor_count,
-            remotefx_max_resolution,
-            vram_bytes)
-
     def attach_config_drive(self, instance, configdrive_path, vm_gen):
         configdrive_ext = configdrive_path[(configdrive_path.rfind('.') + 1):]
         # Do the attach here and if there is a certain file format that isn't

hyperv/tests/unit/test_hostops.py

@@ -135,6 +135,28 @@ class HostOpsTestCase(test_base.HyperVBaseTestCase):
         mock_NUMATopology.assert_called_once_with(
             cells=[mock_NUMACell.return_value])
 
+    def test_get_remotefx_gpu_info(self):
+        self.flags(enable_remotefx=True, group='hyperv')
+        fake_gpus = [{'total_video_ram': '2048',
+                      'available_video_ram': '1024'},
+                     {'total_video_ram': '1024',
+                      'available_video_ram': '1024'}]
+        self._hostops._hostutils.get_remotefx_gpu_info.return_value = fake_gpus
+
+        ret_val = self._hostops._get_remotefx_gpu_info()
+
+        self.assertEqual(3072, ret_val['total_video_ram'])
+        self.assertEqual(1024, ret_val['used_video_ram'])
+
+    def test_get_remotefx_gpu_info_disabled(self):
+        self.flags(enable_remotefx=False, group='hyperv')
+
+        ret_val = self._hostops._get_remotefx_gpu_info()
+
+        self.assertEqual(0, ret_val['total_video_ram'])
+        self.assertEqual(0, ret_val['used_video_ram'])
+        self._hostops._hostutils.get_remotefx_gpu_info.assert_not_called()
+
     @mock.patch.object(hostops.HostOps, '_get_host_numa_topology')
     @mock.patch.object(hostops.HostOps, '_get_remotefx_gpu_info')
     @mock.patch.object(hostops.HostOps, '_get_cpu_info')
@@ -185,7 +207,7 @@ class HostOpsTestCase(test_base.HyperVBaseTestCase):
                     'numa_topology': mock.sentinel.numa_topology_json,
                     'remotefx_available_video_ram': 2048,
                     'remotefx_gpu_info': mock.sentinel.FAKE_GPU_INFO,
-                    'remotefx_total_video_ram': 4096
+                    'remotefx_total_video_ram': 4096,
                     }
         self.assertEqual(expected, response)
 
@@ -227,18 +249,6 @@ class HostOpsTestCase(test_base.HyperVBaseTestCase):
 
         self.assertEqual(expected, response)
 
-    def test_get_remotefx_gpu_info(self):
-        self.flags(enable_remotefx=True, group='hyperv')
-        fake_gpus = [{'total_video_ram': '2048',
-                      'available_video_ram': '1024'},
-                      {'total_video_ram': '1024',
-                      'available_video_ram': '1024'}]
-        self._hostops._hostutils.get_remotefx_gpu_info.return_value = fake_gpus
-
-        ret_val = self._hostops._get_remotefx_gpu_info()
-        self.assertEqual(3072, ret_val['remotefx_total_video_ram'])
-        self.assertEqual(2048, ret_val['remotefx_available_video_ram'])
-
     @mock.patch.object(hostops.HostOps, '_wait_for_instance_pending_task')
     @mock.patch.object(hostops.HostOps, '_set_service_state')
     @mock.patch.object(hostops.HostOps, '_migrate_vm')

hyperv/tests/unit/test_vmops.py

@@ -557,7 +557,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
                               mock_configure_secure_vm,
                               enable_instance_metrics,
                               vm_gen=constants.VM_GEN_1, vnuma_enabled=False,
-                              requires_sec_boot=True, remotefx=False):
+                              requires_sec_boot=True):
         mock_vif_driver = mock_get_vif_driver()
         self.flags(dynamic_memory_ratio=2.0, group='hyperv')
         self.flags(enable_instance_metrics_collection=enable_instance_metrics,
@@ -582,77 +582,61 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
             dynamic_memory_ratio = CONF.hyperv.dynamic_memory_ratio
 
         flavor = flavor_obj.Flavor(**test_flavor.fake_flavor)
-        if remotefx is True:
-            flavor.extra_specs['hyperv:remotefx'] = "1920x1200,2"
         mock_instance.flavor = flavor
 
-        if remotefx is True and vm_gen == constants.VM_GEN_2:
+        self._vmops.create_instance(
-            self.assertRaises(os_win_exc.HyperVException,
+                context=self.context,
-                              self._vmops.create_instance,
+                instance=mock_instance,
-                              context=self.context,
+                network_info=[fake_network_info],
-                              instance=mock_instance,
+                block_device_info=block_device_info,
-                              network_info=[fake_network_info],
+                root_device=root_device_info,
-                              block_device_info=block_device_info,
+                vm_gen=vm_gen,
-                              root_device=root_device_info,
+                image_meta=mock.sentinel.image_meta)
-                              vm_gen=vm_gen,
-                              image_meta=mock.sentinel.image_meta)
-        else:
-            self._vmops.create_instance(
-                    context=self.context,
-                    instance=mock_instance,
-                    network_info=[fake_network_info],
-                    block_device_info=block_device_info,
-                    root_device=root_device_info,
-                    vm_gen=vm_gen,
-                    image_meta=mock.sentinel.image_meta)
-            if remotefx is True:
-                mock_configure_remotefx.assert_called_once_with(
-                    mock_instance,
-                    vm_gen,
-                    flavor.extra_specs['hyperv:remotefx'])
 
-            self._vmops._vmutils.create_vm.assert_called_once_with(
+        mock_configure_remotefx.assert_called_once_with(mock_instance, vm_gen)
-                mock_instance.name, vnuma_enabled, vm_gen,
-                instance_path, [mock_instance.uuid])
-            self._vmops._vmutils.update_vm.assert_called_once_with(
-                mock_instance.name, mock_instance.memory_mb, mem_per_numa,
-                mock_instance.vcpus, cpus_per_numa,
-                CONF.hyperv.limit_cpu_features, dynamic_memory_ratio)
 
-            mock_create_scsi_ctrl = self._vmops._vmutils.create_scsi_controller
+        self._vmops._vmutils.create_vm.assert_called_once_with(
-            mock_create_scsi_ctrl.assert_called_once_with(mock_instance.name)
+            mock_instance.name, vnuma_enabled, vm_gen,
+            instance_path, [mock_instance.uuid])
+        self._vmops._vmutils.update_vm.assert_called_once_with(
+            mock_instance.name, mock_instance.memory_mb, mem_per_numa,
+            mock_instance.vcpus, cpus_per_numa,
+            CONF.hyperv.limit_cpu_features, dynamic_memory_ratio)
 
-            mock_attach_root_device.assert_called_once_with(mock_instance.name,
+        mock_create_scsi_ctrl = self._vmops._vmutils.create_scsi_controller
-                root_device_info)
+        mock_create_scsi_ctrl.assert_called_once_with(mock_instance.name)
-            mock_attach_ephemerals.assert_called_once_with(mock_instance.name,
-                block_device_info['ephemerals'])
-            mock_attach_volumes.assert_called_once_with(
-                block_device_info['block_device_mapping'], mock_instance.name)
 
-            mock_get_port_settings.assert_called_with(mock.sentinel.image_meta)
+        mock_attach_root_device.assert_called_once_with(mock_instance.name,
-            mock_create_pipes.assert_called_once_with(
+            root_device_info)
-                mock_instance, mock_get_port_settings.return_value)
+        mock_attach_ephemerals.assert_called_once_with(mock_instance.name,
+            block_device_info['ephemerals'])
+        mock_attach_volumes.assert_called_once_with(
+            block_device_info['block_device_mapping'], mock_instance.name)
 
-            self._vmops._vmutils.create_nic.assert_called_once_with(
+        mock_get_port_settings.assert_called_with(mock.sentinel.image_meta)
-                mock_instance.name, mock.sentinel.ID, mock.sentinel.ADDRESS)
+        mock_create_pipes.assert_called_once_with(
-            mock_vif_driver.plug.assert_called_once_with(mock_instance,
+            mock_instance, mock_get_port_settings.return_value)
-                                                         fake_network_info)
+
-            mock_enable = (
+        self._vmops._vmutils.create_nic.assert_called_once_with(
-                self._vmops._metricsutils.enable_vm_metrics_collection)
+            mock_instance.name, mock.sentinel.ID, mock.sentinel.ADDRESS)
-            if enable_instance_metrics:
+        mock_vif_driver.plug.assert_called_once_with(mock_instance,
-                mock_enable.assert_called_once_with(mock_instance.name)
+                                                     fake_network_info)
-            mock_set_qos_specs.assert_called_once_with(mock_instance)
+        mock_enable = (
-            if requires_sec_boot:
+            self._vmops._metricsutils.enable_vm_metrics_collection)
-                mock_requires_secure_boot.assert_called_once_with(
+        if enable_instance_metrics:
-                    mock_instance, mock.sentinel.image_meta, vm_gen)
+            mock_enable.assert_called_once_with(mock_instance.name)
-                mock_requires_certificate.assert_called_once_with(
+        mock_set_qos_specs.assert_called_once_with(mock_instance)
-                    mock_instance.uuid,
+        if requires_sec_boot:
-                    mock.sentinel.image_meta)
+            mock_requires_secure_boot.assert_called_once_with(
-                enable_secure_boot = self._vmops._vmutils.enable_secure_boot
+                mock_instance, mock.sentinel.image_meta, vm_gen)
-                enable_secure_boot.assert_called_once_with(
+            mock_requires_certificate.assert_called_once_with(
-                    mock_instance.name, mock_requires_certificate.return_value)
+                mock_instance.uuid,
-                mock_configure_secure_vm.assert_called_once_with(self.context,
+                mock.sentinel.image_meta)
-                    mock_instance, mock.sentinel.image_meta, requires_sec_boot)
+            enable_secure_boot = self._vmops._vmutils.enable_secure_boot
+            enable_secure_boot.assert_called_once_with(
+                mock_instance.name, mock_requires_certificate.return_value)
+            mock_configure_secure_vm.assert_called_once_with(self.context,
+                mock_instance, mock.sentinel.image_meta, requires_sec_boot)
 
     def test_create_instance(self):
         self._test_create_instance(enable_instance_metrics=True)
@@ -670,14 +654,6 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
         self._test_create_instance(enable_instance_metrics=False,
                                    vm_gen=constants.VM_GEN_2)
 
-    def test_create_instance_with_remote_fx(self):
-        self._test_create_instance(enable_instance_metrics=False,
-                                   remotefx=True)
-
-    def test_create_instance_with_remote_fx_gen2(self):
-        self._test_create_instance(enable_instance_metrics=False,
-                                   remotefx=True)
-
     @mock.patch.object(vmops.volumeops.VolumeOps, 'attach_volume')
     def test_attach_root_device_volume(self, mock_attach_volume):
         mock_instance = fake_instance.fake_instance_obj(self.context)
@@ -1292,6 +1268,64 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
                             mock.call(mock.sentinel.FAKE_DVD_PATH2,
                                       mock.sentinel.FAKE_DEST_PATH))
 
+    def _setup_remotefx_mocks(self):
+        mock_instance = fake_instance.fake_instance_obj(self.context)
+        mock_instance.flavor.extra_specs = {
+            'os:resolution': os_win_const.REMOTEFX_MAX_RES_1920x1200,
+            'os:monitors': '2',
+            'os:vram': '256'}
+
+        return mock_instance
+
+    def test_configure_remotefx_not_required(self):
+        self.flags(enable_remotefx=False, group='hyperv')
+        mock_instance = fake_instance.fake_instance_obj(self.context)
+
+        self._vmops._configure_remotefx(mock_instance, mock.sentinel.VM_GEN)
+
+    def test_configure_remotefx_exception_enable_config(self):
+        self.flags(enable_remotefx=False, group='hyperv')
+        mock_instance = self._setup_remotefx_mocks()
+
+        self.assertRaises(exception.InstanceUnacceptable,
+                          self._vmops._configure_remotefx,
+                          mock_instance, mock.sentinel.VM_GEN)
+
+    def test_configure_remotefx_exception_server_feature(self):
+        self.flags(enable_remotefx=True, group='hyperv')
+        mock_instance = self._setup_remotefx_mocks()
+        self._vmops._hostutils.check_server_feature.return_value = False
+
+        self.assertRaises(exception.InstanceUnacceptable,
+                          self._vmops._configure_remotefx,
+                          mock_instance, mock.sentinel.VM_GEN)
+
+    def test_configure_remotefx_exception_vm_gen(self):
+        self.flags(enable_remotefx=True, group='hyperv')
+        mock_instance = self._setup_remotefx_mocks()
+        self._vmops._hostutils.check_server_feature.return_value = True
+        self._vmops._vmutils.vm_gen_supports_remotefx.return_value = False
+
+        self.assertRaises(exception.InstanceUnacceptable,
+                          self._vmops._configure_remotefx,
+                          mock_instance, mock.sentinel.VM_GEN)
+
+    def test_configure_remotefx(self):
+        self.flags(enable_remotefx=True, group='hyperv')
+        mock_instance = self._setup_remotefx_mocks()
+        self._vmops._hostutils.check_server_feature.return_value = True
+        self._vmops._vmutils.vm_gen_supports_remotefx.return_value = True
+        extra_specs = mock_instance.flavor.extra_specs
+
+        self._vmops._configure_remotefx(mock_instance,
+                                        constants.VM_GEN_1)
+        mock_enable_remotefx = (
+            self._vmops._vmutils.enable_remotefx_video_adapter)
+        mock_enable_remotefx.assert_called_once_with(
+            mock_instance.name, int(extra_specs['os:monitors']),
+            extra_specs['os:resolution'],
+            int(extra_specs['os:vram']) * units.Mi)
+
     @mock.patch('nova.virt.configdrive.required_by')
     @mock.patch.object(vmops.VMOps, '_create_root_vhd')
     @mock.patch.object(vmops.VMOps, 'get_image_vm_generation')
@@ -1506,46 +1540,6 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
     def test_get_instance_vnuma_config_no_topology(self):
         self._check_get_instance_vnuma_config()
 
-    def _test_configure_remotefx(self, fail=True, enable_remotefx=True):
-        self.flags(enable_remotefx=enable_remotefx, group='hyperv')
-        mock_instance = fake_instance.fake_instance_obj(self.context)
-
-        fake_resolution = "1920x1200"
-        fake_monitor_count = 3
-        fake_vram_mb = 64
-        fake_config = "%s,%s,%s" % (
-            fake_resolution, fake_monitor_count, fake_vram_mb)
-
-        enable_remotefx = self._vmops._vmutils.enable_remotefx_video_adapter
-
-        if fail:
-            self.assertRaises(exception.InstanceUnacceptable,
-                              self._vmops._configure_remotefx,
-                              mock_instance, mock.sentinel.vm_gen, fake_config)
-        else:
-            self._vmops._configure_remotefx(
-                mock_instance, mock.sentinel.vm_gen, fake_config)
-            enable_remotefx.assert_called_once_with(mock_instance.name,
-                                                    fake_monitor_count,
-                                                    fake_resolution,
-                                                    fake_vram_mb * units.Mi)
-
-    def test_configure_remotefx_disabled(self):
-        self._test_configure_remotefx(enable_remotefx=False)
-
-    def test_configure_remotefx_no_server_feature(self):
-        self._vmops._hostutils.check_server_feature.return_value = False
-        self._test_configure_remotefx()
-        self._vmops._hostutils.check_server_feature.assert_called_once_with(
-            self._vmops._hostutils.FEATURE_RDS_VIRTUALIZATION)
-
-    def test_configure_remotefx_unsupported_vm_gen(self):
-        self._vmops._vmutils.vm_gen_supports_remotefx.return_value = False
-        self._test_configure_remotefx()
-
-    def test_configure_remotefx(self):
-        self._test_configure_remotefx(fail=False)
-
     @mock.patch.object(vmops.VMOps, '_get_vif_driver')
     def test_unplug_vifs(self, mock_get_vif_driver):
         mock_instance = fake_instance.fake_instance_obj(self.context)