compute: RequestSpec sanitization for cross-HV resize

nova/compute/resource_tracker.py

@@ -348,6 +348,14 @@ def _move_claim(
         claimed_resources = self._claim_resources(allocations)
         old_resources = instance.resources
 
+        existing_old_image_properties = None
+        if (instance.obj_attr_is_set('migration_context') and
+                instance.migration_context is not None and
+                instance.migration_context.obj_attr_is_set(
+                    'old_image_properties')):
+            existing_old_image_properties = (
+                instance.migration_context.old_image_properties)
+
         # TODO(jaypipes): Move claimed_numa_topology out of the Claim's
         # constructor flow so the Claim constructor only tests whether
         # resources can be claimed, not consume the resources directly.
@@ -366,7 +374,8 @@ def _move_claim(
             old_pci_requests=instance.pci_requests,
             new_pci_requests=new_pci_requests,
             old_resources=old_resources,
-            new_resources=claimed_resources)
+            new_resources=claimed_resources,
+            old_image_properties=existing_old_image_properties)
 
         instance.migration_context = mig_context
         instance.save()

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,65 @@ 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') and
+            props.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,13 @@ def _execute(self):
         self.request_spec.ensure_network_information(self.instance)
         compute_utils.heal_reqspec_is_bfv(
             self.context, self.request_spec, self.instance)
+        self._old_image_properties = {}
+        if self._is_vmware_to_kvm_resize():
+            # Sanitize VMware-pinning image properties so the scheduler
+            # can select KVM/CH hosts.
+            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)