From b3d020a4781faccc5f211b6eaf068dd94ceac3a0 Mon Sep 17 00:00:00 2001
From: Arik Hadas <ahadas@redhat.com>
Date: Mon, 25 Mar 2024 10:46:05 +0200
Subject: [PATCH] vsphere: transfer updated disk state on remote migrations

The problem these changes address is as follow: when initiating a cold
migration from vSphere to a remote provider of a VM with a snapshot(s),
the transferred disk didn't include the most updated state of the disk
because the very first volume of the disk, that doesn't include later
changes, was transferred.

Unlike warm migrations, in which we intend to transfer the first (base)
volume of the disk initially and then copy the changes using CBT, in
cold migrations we need to transfer the state of the very last volume to
include the most updated state of the disk.

In Forklift 2.3 and below, this issue affected all cold migrations. As
from 2.4, it only affects cold migrations to a remote cluster, in which
we still use CDI to transfer the disks from vSphere.

Signed-off-by: Arik Hadas <ahadas@redhat.com>
---
 .../plan/adapter/vsphere/builder.go           | 28 +++++++++++++++----
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/pkg/controller/plan/adapter/vsphere/builder.go b/pkg/controller/plan/adapter/vsphere/builder.go
index 1bcf08d96..272243c82 100644
--- a/pkg/controller/plan/adapter/vsphere/builder.go
+++ b/pkg/controller/plan/adapter/vsphere/builder.go
@@ -365,7 +365,7 @@ func (r *Builder) DataVolumes(vmRef ref.Ref, secret *core.Secret, _ *core.Config
 					// Let CDI do the copying
 					dvSource = cdi.DataVolumeSource{
 						VDDK: &cdi.DataVolumeSourceVDDK{
-							BackingFile:  trimBackingFileName(disk.File),
+							BackingFile:  r.baseVolume(disk.File),
 							UUID:         vm.UUID,
 							URL:          url,
 							SecretRef:    secret.Name,
@@ -399,7 +399,7 @@ func (r *Builder) DataVolumes(vmRef ref.Ref, secret *core.Secret, _ *core.Config
 				if dv.ObjectMeta.Annotations == nil {
 					dv.ObjectMeta.Annotations = make(map[string]string)
 				}
-				dv.ObjectMeta.Annotations[planbase.AnnDiskSource] = trimBackingFileName(disk.File)
+				dv.ObjectMeta.Annotations[planbase.AnnDiskSource] = r.baseVolume(disk.File)
 				dvs = append(dvs, *dv)
 			}
 		}
@@ -615,7 +615,7 @@ func (r *Builder) mapDisks(vm *model.VM, persistentVolumeClaims []*core.Persiste
 		}
 	}
 	for i, disk := range disks {
-		pvc := pvcMap[trimBackingFileName(disk.File)]
+		pvc := pvcMap[r.baseVolume(disk.File)]
 		volumeName := fmt.Sprintf("vol-%v", i)
 		volume := cnv.Volume{
 			Name: volumeName,
@@ -662,7 +662,7 @@ func (r *Builder) Tasks(vmRef ref.Ref) (list []*plan.Task, err error) {
 		list = append(
 			list,
 			&plan.Task{
-				Name: trimBackingFileName(disk.File),
+				Name: r.baseVolume(disk.File),
 				Progress: libitr.Progress{
 					Total: mB,
 				},
@@ -717,12 +717,12 @@ func (r *Builder) TemplateLabels(vmRef ref.Ref) (labels map[string]string, err e
 
 // Return a stable identifier for a VDDK DataVolume.
 func (r *Builder) ResolveDataVolumeIdentifier(dv *cdi.DataVolume) string {
-	return trimBackingFileName(dv.ObjectMeta.Annotations[planbase.AnnDiskSource])
+	return r.baseVolume(dv.ObjectMeta.Annotations[planbase.AnnDiskSource])
 }
 
 // Return a stable identifier for a PersistentDataVolume.
 func (r *Builder) ResolvePersistentVolumeClaimIdentifier(pvc *core.PersistentVolumeClaim) string {
-	return trimBackingFileName(pvc.Annotations[AnnImportBackingFile])
+	return r.baseVolume(pvc.Annotations[AnnImportBackingFile])
 }
 
 // Load
@@ -826,6 +826,22 @@ func trimBackingFileName(fileName string) string {
 	return backingFilePattern.ReplaceAllString(fileName, ".vmdk")
 }
 
+func (r *Builder) baseVolume(fileName string) string {
+	if r.Plan.Spec.Warm {
+		// for warm migrations, we return the very first volume of the disk
+		// as the base volume and CBT will be used to transfer later changes
+		return trimBackingFileName(fileName)
+	} else {
+		// for cold migrations, we return the latest volume as the base,
+		// e.g., my-vm/disk-name-000015.vmdk, since we should transfer
+		// only its state
+		// note that this setting is insignificant when we use virt-v2v on
+		// el9 since virt-v2v doesn't receive the volume to transfer - we
+		// only need this to be consistent for correlating disks with PVCs
+		return fileName
+	}
+}
+
 // Build LUN PVs.
 func (r *Builder) LunPersistentVolumes(vmRef ref.Ref) (pvs []core.PersistentVolume, err error) {
 	// do nothing