compute: RequestSpec sanitization for cross-HV resize

nova/compute/utils.py

@@ -34,6 +34,7 @@
 from nova.compute import vm_states
 import nova.conf
 from nova import exception
+from nova.network import model as network_model
 from nova import notifications
 from nova.notifications.objects import aggregate as aggregate_notification
 from nova.notifications.objects import base as notification_base
@@ -301,6 +302,64 @@ def heal_reqspec_is_bfv(ctxt, request_spec, instance):
     request_spec.save()
 
 
+def sanitize_image_props_for_kvm(request_spec):
+    """Sanitize VMware-specific image properties for KVM scheduling.
+
+    Mutates request_spec.image.properties in place to remove or replace
+    VMware-pinning values so the scheduler can select KVM/CH hosts.
+
+    Returns a dict of original values that were overwritten or removed,
+    suitable for storage in MigrationContext.old_image_properties as a
+    rollback journal.
+
+    Precondition: only valid for VMware-to-KVM cross-hypervisor resize.
+
+    :param request_spec: nova.objects.RequestSpec to sanitize in place
+    :returns: dict of {field_name: original_value} for rollback, or
+              empty dict if nothing was changed
+    """
+    if (not request_spec.obj_attr_is_set('image') or
+            request_spec.image is None):
+        return {}
+
+    image = request_spec.image
+    if not image.obj_attr_is_set('properties'):
+        return {}
+
+    props = image.properties
+    old = {}
+
+    if props.obj_attr_is_set('img_hv_type'):
+        old['img_hv_type'] = props.img_hv_type
+        delattr(props, 'img_hv_type')
+
+    if props.obj_attr_is_set('hw_disk_bus'):
+        old['hw_disk_bus'] = props.hw_disk_bus
+        props.hw_disk_bus = fields.DiskBus.VIRTIO
+
+    if props.obj_attr_is_set('hw_cdrom_bus'):
+        old['hw_cdrom_bus'] = props.hw_cdrom_bus
+        props.hw_cdrom_bus = fields.DiskBus.VIRTIO
+
+    if props.obj_attr_is_set('hw_scsi_model'):
+        old['hw_scsi_model'] = props.hw_scsi_model
+        delattr(props, 'hw_scsi_model')
+
+    if props.obj_attr_is_set('hw_vif_model'):
+        old['hw_vif_model'] = props.hw_vif_model
+        props.hw_vif_model = network_model.VIF_MODEL_VIRTIO
+
+    if props.obj_attr_is_set('hw_video_model'):
+        old['hw_video_model'] = props.hw_video_model
+        props.hw_video_model = fields.VideoModel.VIRTIO
+
+    if props.obj_attr_is_set('img_hv_requested_version'):
+        old['img_hv_requested_version'] = props.img_hv_requested_version
+        delattr(props, 'img_hv_requested_version')
+
+    return old
+
+
 def convert_mb_to_ceil_gb(mb_value):
     gb_int = 0
     if mb_value:

nova/conductor/manager.py

@@ -449,6 +449,24 @@ def _cold_migrate(self, context, instance, flavor, filter_properties,
         # NOTE(sbauza): Make sure we persist the new flavor in case we had
         # a successful scheduler call if and only if nothing bad happened
         if request_spec.obj_what_changed():
+            # Persist the rollback journal for image properties BEFORE
+            # saving the modified request_spec, so that revert can always
+            # find the originals even if a crash occurs between these two
+            # writes.
+            old_img_props = getattr(task, '_old_image_properties', None)
+            if old_img_props and isinstance(old_img_props, dict):
+                if ('migration_context' in instance and
+                        instance.migration_context is not None):
+                    mig_context = instance.migration_context
+                else:
+                    mig_context = objects.MigrationContext(
+                        context=context,
+                        instance_uuid=instance.uuid,
+                        migration_id=task._migration.id)
+                mig_context.old_image_properties = old_img_props
+                instance.migration_context = mig_context
+                instance.save()
+
             # NOTE(jkulik): Make sure we do not store the "volume_sizes"
             # scheduler_hint as that can change at any time and saving it in
             # the DB thus is not useful.

nova/conductor/tasks/migrate.py

@@ -133,6 +133,15 @@ def __init__(self, context, instance, flavor,
         self._held_allocations = None
         self._source_cn = None
 
+    def _is_vmware_to_kvm_resize(self):
+        """Return True if this resize is specifically VMware to KVM.
+
+        Stub — always returns False until VMware-to-KVM detection is
+        implemented (see: Add Cross-Hypervisor Resize Detection and
+        Guardrails ticket).
+        """
+        return False
+
     def _preallocate_migration(self):
         # If this is a rescheduled migration, don't create a new record.
         migration_type = ("resize" if self.instance.flavor.id != self.flavor.id
@@ -283,6 +292,16 @@ def _execute(self):
         self.request_spec.ensure_network_information(self.instance)
         compute_utils.heal_reqspec_is_bfv(
             self.context, self.request_spec, self.instance)
+        # For cross-hypervisor resize (VMware to KVM), sanitize the image
+        # properties directly on the canonical request_spec so the scheduler
+        # can select KVM/CH hosts.  The returned dict of original values is
+        # stashed for the conductor to persist into MigrationContext before
+        # request_spec.save().
+        self._old_image_properties = {}
+        if self._is_vmware_to_kvm_resize():
+            self._old_image_properties = (
+                compute_utils.sanitize_image_props_for_kvm(
+                    self.request_spec))
         # On an initial call to migrate, 'self.host_list' will be None, so we
         # have to call the scheduler to get a list of acceptable hosts to
         # migrate to. That list will consist of a selected host, along with

nova/objects/migration_context.py

@@ -40,7 +40,8 @@ class MigrationContext(base.NovaPersistentObject, base.NovaObject):
     # Version 1.0: Initial version
     # Version 1.1: Add old/new pci_devices and pci_requests
     # Version 1.2: Add old/new resources
-    VERSION = '1.2'
+    # Version 1.3: Add old_image_properties
+    VERSION = '1.3'
 
     fields = {
         'instance_uuid': fields.UUIDField(),
@@ -61,11 +62,14 @@ class MigrationContext(base.NovaPersistentObject, base.NovaObject):
                                             nullable=True),
         'old_resources': fields.ObjectField('ResourceList',
                                             nullable=True),
+        'old_image_properties': fields.DictOfStringsField(nullable=True),
     }
 
     @classmethod
     def obj_make_compatible(cls, primitive, target_version):
         target_version = versionutils.convert_version_to_tuple(target_version)
+        if target_version < (1, 3):
+            primitive.pop('old_image_properties', None)
         if target_version < (1, 2):
             primitive.pop('old_resources', None)
             primitive.pop('new_resources', None)

nova/tests/unit/compute/test_utils.py

@@ -2074,3 +2074,134 @@ def test_delete_with_arq_uuid_and_dp(self, mock_del_inst, mock_del_uuid):
         compute_utils.delete_arqs_if_needed(self.context, instance, arq_uuids)
         mock_del_inst.assert_called_once_with(instance.uuid)
         mock_del_uuid.assert_called_once_with(arq_uuids)
+
+
+class TestSanitizeImagePropsForKvm(test.NoDBTestCase):
+    """Tests for sanitize_image_props_for_kvm (early-commit approach)."""
+
+    def _make_request_spec(self, **image_props):
+        props = objects.ImageMetaProps(**image_props)
+        image = objects.ImageMeta(properties=props)
+        return objects.RequestSpec(image=image)
+
+    def test_img_hv_type_removed_and_journaled(self):
+        reqspec = self._make_request_spec(img_hv_type='vmware')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertFalse(
+            reqspec.image.properties.obj_attr_is_set('img_hv_type'))
+        self.assertEqual('vmware', old['img_hv_type'])
+
+    def test_img_hv_type_any_value_removed(self):
+        reqspec = self._make_request_spec(img_hv_type='kvm')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertFalse(
+            reqspec.image.properties.obj_attr_is_set('img_hv_type'))
+        self.assertEqual('kvm', old['img_hv_type'])
+
+    def test_hw_disk_bus_replaced_with_virtio(self):
+        reqspec = self._make_request_spec(hw_disk_bus='scsi')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertEqual('virtio', reqspec.image.properties.hw_disk_bus)
+        self.assertEqual('scsi', old['hw_disk_bus'])
+
+    def test_hw_disk_bus_already_virtio_still_journaled(self):
+        reqspec = self._make_request_spec(hw_disk_bus='virtio')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertEqual('virtio', reqspec.image.properties.hw_disk_bus)
+        self.assertEqual('virtio', old['hw_disk_bus'])
+
+    def test_hw_scsi_model_removed_and_journaled(self):
+        reqspec = self._make_request_spec(hw_scsi_model='virtio-scsi')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertFalse(
+            reqspec.image.properties.obj_attr_is_set('hw_scsi_model'))
+        self.assertEqual('virtio-scsi', old['hw_scsi_model'])
+
+    def test_hw_scsi_model_absent_no_error(self):
+        reqspec = self._make_request_spec(hw_disk_bus='scsi')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertNotIn('hw_scsi_model', old)
+
+    def test_hw_vif_model_replaced_with_virtio(self):
+        reqspec = self._make_request_spec(hw_vif_model='vmxnet3')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertEqual('virtio', reqspec.image.properties.hw_vif_model)
+        self.assertEqual('vmxnet3', old['hw_vif_model'])
+
+    def test_hw_video_model_replaced_with_virtio(self):
+        reqspec = self._make_request_spec(hw_video_model='vmvga')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertEqual('virtio',
+                         reqspec.image.properties.hw_video_model)
+        self.assertEqual('vmvga', old['hw_video_model'])
+
+    def test_mutates_in_place(self):
+        reqspec = self._make_request_spec(
+            img_hv_type='vmware', hw_disk_bus='scsi')
+        props_before = reqspec.image.properties
+        compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertIs(props_before, reqspec.image.properties)
+
+    def test_returns_complete_journal(self):
+        reqspec = self._make_request_spec(
+            img_hv_type='vmware', hw_disk_bus='scsi',
+            hw_cdrom_bus='ide', hw_scsi_model='virtio-scsi',
+            hw_vif_model='vmxnet3', hw_video_model='vmvga',
+            img_hv_requested_version='>=6.0')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertEqual({
+            'img_hv_type': 'vmware',
+            'hw_disk_bus': 'scsi',
+            'hw_cdrom_bus': 'ide',
+            'hw_scsi_model': 'virtio-scsi',
+            'hw_vif_model': 'vmxnet3',
+            'hw_video_model': 'vmvga',
+            'img_hv_requested_version': '>=6.0',
+        }, old)
+
+    def test_no_image_returns_empty_dict(self):
+        reqspec = objects.RequestSpec()
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertEqual({}, old)
+
+    def test_image_none_returns_empty_dict(self):
+        reqspec = objects.RequestSpec(image=None)
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertEqual({}, old)
+
+    def test_hw_cdrom_bus_replaced_with_virtio(self):
+        reqspec = self._make_request_spec(hw_cdrom_bus='ide')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertEqual('virtio', reqspec.image.properties.hw_cdrom_bus)
+        self.assertEqual('ide', old['hw_cdrom_bus'])
+
+    def test_hw_cdrom_bus_sata_replaced_with_virtio(self):
+        reqspec = self._make_request_spec(hw_cdrom_bus='sata')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertEqual('virtio', reqspec.image.properties.hw_cdrom_bus)
+        self.assertEqual('sata', old['hw_cdrom_bus'])
+
+    def test_hw_cdrom_bus_already_virtio_still_journaled(self):
+        reqspec = self._make_request_spec(hw_cdrom_bus='virtio')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertEqual('virtio', reqspec.image.properties.hw_cdrom_bus)
+        self.assertEqual('virtio', old['hw_cdrom_bus'])
+
+    def test_img_hv_requested_version_removed_and_journaled(self):
+        reqspec = self._make_request_spec(
+            img_hv_requested_version='>=6.0,<7.0')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertFalse(
+            reqspec.image.properties.obj_attr_is_set(
+                'img_hv_requested_version'))
+        self.assertEqual('>=6.0,<7.0', old['img_hv_requested_version'])
+
+    def test_img_hv_requested_version_absent_no_error(self):
+        reqspec = self._make_request_spec(img_hv_type='vmware')
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertNotIn('img_hv_requested_version', old)
+
+    def test_no_props_set_returns_empty_dict(self):
+        reqspec = self._make_request_spec()
+        old = compute_utils.sanitize_image_props_for_kvm(reqspec)
+        self.assertEqual({}, old)

nova/tests/unit/conductor/tasks/test_migrate.py

@@ -62,6 +62,11 @@ def setUp(self):
         self.heal_reqspec_is_bfv_mock = _p.start()
         self.addCleanup(_p.stop)
 
+        _p = mock.patch('nova.compute.utils.sanitize_image_props_for_kvm')
+        self.sanitize_mock = _p.start()
+        self.sanitize_mock.return_value = {}
+        self.addCleanup(_p.stop)
+
         _p = mock.patch('nova.objects.RequestSpec.ensure_network_information')
         self.ensure_network_information_mock = _p.start()
         self.addCleanup(_p.stop)
@@ -393,6 +398,81 @@ def test_is_selected_host_in_source_cell_false(self):
         selection = objects.Selection(cell_uuid=uuids.cell2, service_host='x')
         self.assertFalse(task._is_selected_host_in_source_cell(selection))
 
+    @mock.patch.object(objects.MigrationList, 'get_by_filters')
+    @mock.patch('nova.scheduler.client.report.SchedulerReportClient')
+    @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
+    @mock.patch('nova.objects.Migration.save')
+    @mock.patch('nova.objects.Migration.create')
+    @mock.patch('nova.objects.Service.get_minimum_version_multi')
+    @mock.patch('nova.availability_zones.get_host_availability_zone')
+    @mock.patch.object(scheduler_utils, 'setup_instance_group')
+    @mock.patch.object(query.SchedulerQueryClient, 'select_destinations')
+    @mock.patch.object(compute_rpcapi.ComputeAPI, 'prep_resize')
+    def test_execute_calls_sanitize_and_stashes_journal(
+            self, prep_resize_mock, sel_dest_mock, sig_mock, az_mock,
+            gmv_mock, cm_mock, sm_mock, cn_mock, rc_mock, gbf_mock):
+        """Verify sanitize is called for cross-HV and result stashed."""
+        sel_dest_mock.return_value = self.host_lists
+        az_mock.return_value = 'myaz'
+        gbf_mock.return_value = objects.MigrationList()
+        mock_get_resources = \
+            self.mock_network_api.get_requested_resource_for_instance
+        mock_get_resources.return_value = ([], objects.RequestLevelParams())
+        gmv_mock.return_value = 23
+
+        fake_journal = {'img_hv_type': 'vmware', 'hw_disk_bus': 'scsi'}
+        self.sanitize_mock.return_value = fake_journal
+
+        task = self._generate_task()
+
+        def set_migration_uuid(*a, **k):
+            task._migration.uuid = uuids.migration
+            return mock.MagicMock()
+
+        cn_mock.side_effect = set_migration_uuid
+
+        with mock.patch.object(task, '_is_vmware_to_kvm_resize',
+                               return_value=True):
+            task.execute()
+
+        self.sanitize_mock.assert_called_once_with(self.request_spec)
+        self.assertEqual(fake_journal, task._old_image_properties)
+
+    @mock.patch.object(objects.MigrationList, 'get_by_filters')
+    @mock.patch('nova.scheduler.client.report.SchedulerReportClient')
+    @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
+    @mock.patch('nova.objects.Migration.save')
+    @mock.patch('nova.objects.Migration.create')
+    @mock.patch('nova.objects.Service.get_minimum_version_multi')
+    @mock.patch('nova.availability_zones.get_host_availability_zone')
+    @mock.patch.object(scheduler_utils, 'setup_instance_group')
+    @mock.patch.object(query.SchedulerQueryClient, 'select_destinations')
+    @mock.patch.object(compute_rpcapi.ComputeAPI, 'prep_resize')
+    def test_execute_skips_sanitize_for_same_hv(
+            self, prep_resize_mock, sel_dest_mock, sig_mock, az_mock,
+            gmv_mock, cm_mock, sm_mock, cn_mock, rc_mock, gbf_mock):
+        """Verify sanitize is NOT called for same-HV resize."""
+        sel_dest_mock.return_value = self.host_lists
+        az_mock.return_value = 'myaz'
+        gbf_mock.return_value = objects.MigrationList()
+        mock_get_resources = \
+            self.mock_network_api.get_requested_resource_for_instance
+        mock_get_resources.return_value = ([], objects.RequestLevelParams())
+        gmv_mock.return_value = 23
+
+        task = self._generate_task()
+
+        def set_migration_uuid(*a, **k):
+            task._migration.uuid = uuids.migration
+            return mock.MagicMock()
+
+        cn_mock.side_effect = set_migration_uuid
+        # _is_vmware_to_kvm_resize stub returns False by default
+        task.execute()
+
+        self.sanitize_mock.assert_not_called()
+        self.assertEqual({}, task._old_image_properties)
+
 
 class MigrationTaskAllocationUtils(test.NoDBTestCase):
     @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')