Merge pull request #12910 from roosterfish/mv_snapshot_comparison

Storage: Move snapshot comparison logic into the driver

lxd/storage/drivers/driver_btrfs_volumes.go

@@ -1519,7 +1519,7 @@ func (d *btrfs) BackupVolume(vol VolumeCopy, tarWriter *instancewriter.InstanceT
 
 	if len(snapshots) > 0 {
 		// Check requested snapshot match those in storage.
-		err := vol.SnapshotsMatch(snapshots, op)
+		err := d.CheckVolumeSnapshots(vol.Volume, vol.Snapshots, op)
 		if err != nil {
 			return err
 		}

lxd/storage/drivers/driver_common.go

@@ -16,6 +16,7 @@ import (
 	"github.com/canonical/lxd/lxd/state"
 	"github.com/canonical/lxd/lxd/storage/filesystem"
 	"github.com/canonical/lxd/shared"
+	"github.com/canonical/lxd/shared/api"
 	"github.com/canonical/lxd/shared/logger"
 	"github.com/canonical/lxd/shared/revert"
 )
@@ -438,6 +439,38 @@ func (d *common) VolumeSnapshots(vol Volume, op *operations.Operation) ([]string
 	return nil, ErrNotSupported
 }
 
+// CheckVolumeSnapshots checks that the volume's snapshots, according to the storage driver, match those provided.
+func (d *common) CheckVolumeSnapshots(vol Volume, snapVols []Volume, op *operations.Operation) error {
+	// Use the volume's driver reference to pick the actual method as implemented by the driver.
+	storageSnapshotNames, err := vol.driver.VolumeSnapshots(vol, op)
+	if err != nil {
+		return err
+	}
+
+	// Create a list of all wanted snapshots.
+	wantedSnapshotNames := make([]string, 0, len(snapVols))
+	for _, snap := range snapVols {
+		_, snapName, _ := api.GetParentAndSnapshotName(snap.name)
+		wantedSnapshotNames = append(wantedSnapshotNames, snapName)
+	}
+
+	// Check if the provided list of volume snapshots matches the ones from storage.
+	for _, wantedSnapshotName := range wantedSnapshotNames {
+		if !shared.ValueInSlice(wantedSnapshotName, storageSnapshotNames) {
+			return fmt.Errorf("Snapshot %q expected but not in storage", wantedSnapshotName)
+		}
+	}
+
+	// Check if the snapshots in storage match the ones from the provided list.
+	for _, storageSnapshotName := range storageSnapshotNames {
+		if !shared.ValueInSlice(storageSnapshotName, wantedSnapshotNames) {
+			return fmt.Errorf("Snapshot %q in storage but not expected", storageSnapshotName)
+		}
+	}
+
+	return nil
+}
+
 // RestoreVolume resets a volume to its snapshotted state.
 func (d *common) RestoreVolume(vol Volume, snapVol Volume, op *operations.Operation) error {
 	return ErrNotSupported

lxd/storage/drivers/driver_zfs_volumes.go

@@ -2714,7 +2714,7 @@ func (d *zfs) BackupVolume(vol VolumeCopy, tarWriter *instancewriter.InstanceTar
 
 	if len(snapshots) > 0 {
 		// Check requested snapshot match those in storage.
-		err := vol.SnapshotsMatch(snapshots, op)
+		err := d.CheckVolumeSnapshots(vol.Volume, vol.Snapshots, op)
 		if err != nil {
 			return err
 		}

lxd/storage/drivers/generic_vfs.go

@@ -486,7 +486,7 @@ func genericVFSGetVolumeDiskPath(vol Volume) (string, error) {
 func genericVFSBackupVolume(d Driver, vol VolumeCopy, tarWriter *instancewriter.InstanceTarWriter, snapshots []string, op *operations.Operation) error {
 	if len(snapshots) > 0 {
 		// Check requested snapshot match those in storage.
-		err := vol.SnapshotsMatch(snapshots, op)
+		err := d.CheckVolumeSnapshots(vol.Volume, vol.Snapshots, op)
 		if err != nil {
 			return err
 		}

lxd/storage/drivers/interface.go

@@ -99,6 +99,7 @@ type Driver interface {
 	DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation) error
 	RenameVolumeSnapshot(snapVol Volume, newSnapshotName string, op *operations.Operation) error
 	VolumeSnapshots(vol Volume, op *operations.Operation) ([]string, error)
+	CheckVolumeSnapshots(vol Volume, snapVols []Volume, op *operations.Operation) error
 	RestoreVolume(vol Volume, snapVol Volume, op *operations.Operation) error
 
 	// Migration.

lxd/storage/drivers/volume.go

@@ -353,33 +353,6 @@ func (v Volume) Snapshots(op *operations.Operation) ([]Volume, error) {
 	return snapVols, nil
 }
 
-// SnapshotsMatch checks that the snapshots, according to the storage driver, match those provided (although not
-// necessarily in the same order).
-func (v Volume) SnapshotsMatch(snapNames []string, op *operations.Operation) error {
-	if v.IsSnapshot() {
-		return fmt.Errorf("Volume is a snapshot")
-	}
-
-	snapshots, err := v.driver.VolumeSnapshots(v, op)
-	if err != nil {
-		return err
-	}
-
-	for _, snapName := range snapNames {
-		if !shared.ValueInSlice(snapName, snapshots) {
-			return fmt.Errorf("Snapshot %q expected but not in storage", snapName)
-		}
-	}
-
-	for _, snapshot := range snapshots {
-		if !shared.ValueInSlice(snapshot, snapNames) {
-			return fmt.Errorf("Snapshot %q in storage but not expected", snapshot)
-		}
-	}
-
-	return nil
-}
-
 // IsBlockBacked indicates whether storage device is block backed.
 func (v Volume) IsBlockBacked() bool {
 	return v.driver.isBlockBacked(v) || v.mountFilesystemProbe