Skip to content

Commit 024cdcd

Browse files
committed
MTV-1656 | Disable snapshot consolidation
Issue: When deleting the snapshot the users with slow storage could fail to get data corruption due to the consolidation process. The consolidation writes the snapshot overlay to the disk and if the user has slow storage it can take a long time. However, the Forklift Controller does not check if the consolidation is finished and creates a new snapshot. This new snapshot could be created in the middle of the writing process and it would not see the changes in the underlying image from the consolidation. Wrong order 1. Delete snapshot 2. Create snapshot 3. Consolidate Correct order 1. Delete snapshot 2. Consolidate 3. Create snapshot Fix: Disable consolidation in the middle of migration and consolidate in RemoveFinalSnapshot phase. This will make the warm migration faster as we don't need to wait for the consolidation but at the same time we will consolidate at the end of migration so we won't cause any issues on the users env. Ref: https://issues.redhat.com/browse/MTV-1656 Signed-off-by: Martin Necas <mnecas@redhat.com>
1 parent 29be033 commit 024cdcd

File tree

7 files changed

+15
-11
lines changed

7 files changed

+15
-11
lines changed

pkg/controller/plan/adapter/base/doc.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ type Client interface {
106106
// Create a snapshot of the source VM.
107107
CreateSnapshot(vmRef ref.Ref, hostsFunc util.HostsFunc) (string, error)
108108
// Remove a snapshot.
109-
RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc) error
109+
RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc, consolidate bool) error
110110
// Check if a snapshot is ready to transfer.
111111
CheckSnapshotReady(vmRef ref.Ref, snapshot string) (ready bool, err error)
112112
// Set DataVolume checkpoints.

pkg/controller/plan/adapter/ocp/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func (r *Client) CreateSnapshot(vmRef ref.Ref, hostsFunc util.HostsFunc) (string
4040
}
4141

4242
// Remove a VM snapshot. No-op for this provider.
43-
func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc) (err error) {
43+
func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc, consolidate bool) (err error) {
4444
return
4545
}
4646

pkg/controller/plan/adapter/openstack/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func (r *Client) SetCheckpoints(vmRef ref.Ref, precopies []planapi.Precopy, data
125125
}
126126

127127
// Remove a VM snapshot. No-op for this provider.
128-
func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc) (err error) {
128+
func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc, consolidate bool) (err error) {
129129
return
130130
}
131131

pkg/controller/plan/adapter/ova/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (r *Client) CreateSnapshot(vmRef ref.Ref, hostsFunc util.HostsFunc) (snapsh
5151
}
5252

5353
// Remove a VM snapshot. No-op for this provider.
54-
func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc) (err error) {
54+
func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc, consolidate bool) (err error) {
5555
return
5656
}
5757

pkg/controller/plan/adapter/ovirt/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func (r *Client) CheckSnapshotReady(vmRef ref.Ref, snapshot string) (ready bool,
105105
}
106106

107107
// Remove a VM snapshot. No-op for this provider.
108-
func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc) (err error) {
108+
func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc, consolidate bool) (err error) {
109109
return
110110
}
111111

pkg/controller/plan/adapter/vsphere/client.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,11 @@ func (r *Client) CheckSnapshotReady(vmRef ref.Ref, snapshot string) (ready bool,
6767
}
6868

6969
// Remove a VM snapshot.
70-
func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hosts util.HostsFunc) (err error) {
70+
func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hosts util.HostsFunc, consolidate bool) (err error) {
7171
r.Log.V(1).Info("RemoveSnapshot",
7272
"vmRef", vmRef,
7373
"snapshot", snapshot)
74-
err = r.removeSnapshot(vmRef, snapshot, false, hosts)
74+
err = r.removeSnapshot(vmRef, snapshot, false, hosts, consolidate)
7575
return
7676
}
7777

@@ -371,7 +371,7 @@ func nullableHosts() (hosts map[string]*v1beta1.Host, err error) {
371371
}
372372

373373
// Remove a VM snapshot and optionally its children.
374-
func (r *Client) removeSnapshot(vmRef ref.Ref, snapshot string, children bool, hosts util.HostsFunc) (err error) {
374+
func (r *Client) removeSnapshot(vmRef ref.Ref, snapshot string, children bool, hosts util.HostsFunc, consolidate bool) (err error) {
375375
r.Log.Info("Removing snapshot",
376376
"vmRef", vmRef,
377377
"snapshot", snapshot,
@@ -381,7 +381,7 @@ func (r *Client) removeSnapshot(vmRef ref.Ref, snapshot string, children bool, h
381381
if err != nil {
382382
return
383383
}
384-
_, err = vm.RemoveSnapshot(context.TODO(), snapshot, children, nil)
384+
_, err = vm.RemoveSnapshot(context.TODO(), snapshot, children, &consolidate)
385385
if err != nil {
386386
err = liberr.Wrap(err)
387387
return

pkg/controller/plan/migration.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ func (r *Migration) removeLastWarmSnapshot(vm *plan.VMStatus) {
517517
return
518518
}
519519
snapshot := vm.Warm.Precopies[n-1].Snapshot
520-
if err := r.provider.RemoveSnapshot(vm.Ref, snapshot, r.kubevirt.loadHosts); err != nil {
520+
if err := r.provider.RemoveSnapshot(vm.Ref, snapshot, r.kubevirt.loadHosts, true); err != nil {
521521
r.Log.Error(
522522
err,
523523
"Failed to clean up warm migration snapshots.",
@@ -995,7 +995,11 @@ func (r *Migration) execute(vm *plan.VMStatus) (err error) {
995995
break
996996
}
997997
n := len(vm.Warm.Precopies)
998-
err = r.provider.RemoveSnapshot(vm.Ref, vm.Warm.Precopies[n-1].Snapshot, r.kubevirt.loadHosts)
998+
consolidate := false
999+
if vm.Phase == RemoveFinalSnapshot {
1000+
consolidate = true
1001+
}
1002+
err = r.provider.RemoveSnapshot(vm.Ref, vm.Warm.Precopies[n-1].Snapshot, r.kubevirt.loadHosts, consolidate)
9991003
if err != nil {
10001004
step.AddError(err.Error())
10011005
err = nil

0 commit comments

Comments
 (0)