[lxc-devel] [lxd/master] Storage updates generic copy and migrate functions to use revert package

tomponline on Github lxc-bot at linuxcontainers.org
Mon Jan 6 11:27:29 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 465 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200106/7ddb0ef4/attachment.bin>
-------------- next part --------------
From 910f728249f0e7979698c320085eb1802083ed34 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 6 Jan 2020 11:20:18 +0000
Subject: [PATCH 1/3] lxd/storage/drivers/generic: Uses revert package on
 genericCreateVolumeFromMigration

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/generic.go | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/lxd/storage/drivers/generic.go b/lxd/storage/drivers/generic.go
index b0a1855444..0f22309611 100644
--- a/lxd/storage/drivers/generic.go
+++ b/lxd/storage/drivers/generic.go
@@ -93,37 +93,25 @@ func genericCopyVolume(d Driver, initVolume func(vol Volume) (func(), error), vo
 		return err
 	}
 
-	revertSnaps = nil // Don't revert.
+	revert.Success()
 	return nil
 }
 
 // genericCreateVolumeFromMigration receives a volume and its snapshots over a non-optimized method.
 // initVolume is run against the main volume (not the snapshots) and is often used for quota initialization.
 func genericCreateVolumeFromMigration(d Driver, initVolume func(vol Volume) (func(), error), vol Volume, conn io.ReadWriteCloser, volTargetArgs migration.VolumeTargetArgs, preFiller *VolumeFiller, op *operations.Operation) error {
-	// Create the main volume path.
+	revert := revert.New()
+	defer revert.Fail()
+
+	// Create the main volume if not refreshing.
 	if !volTargetArgs.Refresh {
 		err := d.CreateVolume(vol, preFiller, op)
 		if err != nil {
 			return err
 		}
-	}
 
-	// Create slice of snapshots created if revert needed later.
-	revertSnaps := []string{}
-	defer func() {
-		if revertSnaps == nil {
-			return
-		}
-
-		// Remove any paths created if we are reverting.
-		for _, snapName := range revertSnaps {
-			fullSnapName := GetSnapshotVolumeName(vol.Name(), snapName)
-			snapVol := NewVolume(d, d.Name(), vol.volType, vol.contentType, fullSnapName, vol.config, vol.poolConfig)
-			d.DeleteVolumeSnapshot(snapVol, op)
-		}
-
-		d.DeleteVolume(vol, op)
-	}()
+		revert.Add(func() { d.DeleteVolume(vol, op) })
+	}
 
 	// Ensure the volume is mounted.
 	err := vol.MountTask(func(mountPath string, op *operations.Operation) error {
@@ -153,7 +141,9 @@ func genericCreateVolumeFromMigration(d Driver, initVolume func(vol Volume) (fun
 			}
 
 			// Setup the revert.
-			revertSnaps = append(revertSnaps, snapName)
+			revert.Add(func() {
+				d.DeleteVolumeSnapshot(snapVol, op)
+			})
 		}
 
 		// Run volume-specific init logic.
@@ -195,7 +185,7 @@ func genericCreateVolumeFromMigration(d Driver, initVolume func(vol Volume) (fun
 		return err
 	}
 
-	revertSnaps = nil
+	revert.Success()
 	return nil
 }
 

From c2f7ce3a67319c9aeaad1993c84bd6fc09a94983 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 6 Jan 2020 11:21:05 +0000
Subject: [PATCH 2/3] lxd/storage/drivers/generic: Adds refresh arg to
 genericCopyVolume

Allows the function to use the storage driver's CreateVolume() function bringing it inline with genericCreateVolumeFromMigration() and allowing it to work with non-dir drivers, such as LVM, that still use rsync for syncing.

Also switches to revert package for simpler reverting.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/generic.go | 42 +++++++++++++---------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/lxd/storage/drivers/generic.go b/lxd/storage/drivers/generic.go
index 0f22309611..71b46fb3d5 100644
--- a/lxd/storage/drivers/generic.go
+++ b/lxd/storage/drivers/generic.go
@@ -3,7 +3,6 @@ package drivers
 import (
 	"fmt"
 	"io"
-	"os"
 
 	"github.com/lxc/lxd/lxd/migration"
 	"github.com/lxc/lxd/lxd/operations"
@@ -16,48 +15,37 @@ import (
 
 // genericCopyVolume copies a volume and its snapshots using a non-optimized method.
 // initVolume is run against the main volume (not the snapshots) and is often used for quota initialization.
-func genericCopyVolume(d Driver, initVolume func(vol Volume) (func(), error), vol Volume, srcVol Volume, srcSnapshots []Volume, op *operations.Operation) error {
+func genericCopyVolume(d Driver, initVolume func(vol Volume) (func(), error), vol Volume, srcVol Volume, srcSnapshots []Volume, refresh bool, op *operations.Operation) error {
 	if vol.contentType != ContentTypeFS || srcVol.contentType != ContentTypeFS {
 		return fmt.Errorf("Content type not supported")
 	}
 
 	bwlimit := d.Config()["rsync.bwlimit"]
 
-	// Create the main volume path.
-	volPath := vol.MountPath()
-	err := vol.EnsureMountPath()
-	if err != nil {
-		return err
-	}
-
-	// Create slice of snapshots created if revert needed later.
-	revertSnaps := []string{}
-	defer func() {
-		if revertSnaps == nil {
-			return
-		}
+	revert := revert.New()
+	defer revert.Fail()
 
-		// Remove any paths created if we are reverting.
-		for _, snapName := range revertSnaps {
-			fullSnapName := GetSnapshotVolumeName(vol.name, snapName)
-			snapVol := NewVolume(d, d.Name(), vol.volType, vol.contentType, fullSnapName, vol.config, vol.poolConfig)
-			d.DeleteVolumeSnapshot(snapVol, op)
+	// Create the main volume if not refreshing.
+	if !refresh {
+		err := d.CreateVolume(vol, nil, op)
+		if err != nil {
+			return err
 		}
 
-		os.RemoveAll(volPath)
-	}()
+		revert.Add(func() { d.DeleteVolume(vol, op) })
+	}
 
 	// Ensure the volume is mounted.
-	err = vol.MountTask(func(mountPath string, op *operations.Operation) error {
+	err := vol.MountTask(func(mountPath string, op *operations.Operation) error {
 		// If copying snapshots is indicated, check the source isn't itself a snapshot.
 		if len(srcSnapshots) > 0 && !srcVol.IsSnapshot() {
 			for _, srcSnapshot := range srcSnapshots {
 				_, snapName, _ := shared.InstanceGetParentAndSnapshotName(srcSnapshot.name)
 
 				// Mount the source snapshot.
-				err = srcSnapshot.MountTask(func(srcMountPath string, op *operations.Operation) error {
+				err := srcSnapshot.MountTask(func(srcMountPath string, op *operations.Operation) error {
 					// Copy the snapshot.
-					_, err = rsync.LocalCopy(srcMountPath, mountPath, bwlimit, true)
+					_, err := rsync.LocalCopy(srcMountPath, mountPath, bwlimit, true)
 					return err
 				}, op)
 
@@ -71,7 +59,9 @@ func genericCopyVolume(d Driver, initVolume func(vol Volume) (func(), error), vo
 				}
 
 				// Setup the revert.
-				revertSnaps = append(revertSnaps, snapName)
+				revert.Add(func() {
+					d.DeleteVolumeSnapshot(snapVol, op)
+				})
 			}
 		}
 

From ef96c8217ddce60fa9a75b886662577242f58eb9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 6 Jan 2020 11:22:48 +0000
Subject: [PATCH 3/3] lxd/storage/drivers: genericCopyVolume updated usage for
 refresh arg

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_btrfs_volumes.go | 2 +-
 lxd/storage/drivers/driver_dir_volumes.go   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lxd/storage/drivers/driver_btrfs_volumes.go b/lxd/storage/drivers/driver_btrfs_volumes.go
index 09c221b542..c8826b447f 100644
--- a/lxd/storage/drivers/driver_btrfs_volumes.go
+++ b/lxd/storage/drivers/driver_btrfs_volumes.go
@@ -298,7 +298,7 @@ func (d *btrfs) CreateVolumeFromMigration(vol Volume, conn io.ReadWriteCloser, v
 
 // RefreshVolume provides same-pool volume and specific snapshots syncing functionality.
 func (d *btrfs) RefreshVolume(vol Volume, srcVol Volume, srcSnapshots []Volume, op *operations.Operation) error {
-	return genericCopyVolume(d, nil, vol, srcVol, srcSnapshots, op)
+	return genericCopyVolume(d, nil, vol, srcVol, srcSnapshots, true, op)
 }
 
 // DeleteVolume deletes a volume of the storage device. If any snapshots of the volume remain then
diff --git a/lxd/storage/drivers/driver_dir_volumes.go b/lxd/storage/drivers/driver_dir_volumes.go
index ffa3acf227..0c0e850539 100644
--- a/lxd/storage/drivers/driver_dir_volumes.go
+++ b/lxd/storage/drivers/driver_dir_volumes.go
@@ -112,7 +112,7 @@ func (d *dir) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool
 	}
 
 	// Run the generic copy.
-	return genericCopyVolume(d, d.setupInitialQuota, vol, srcVol, srcSnapshots, op)
+	return genericCopyVolume(d, d.setupInitialQuota, vol, srcVol, srcSnapshots, false, op)
 }
 
 // CreateVolumeFromMigration creates a volume being sent via a migration.
@@ -130,7 +130,7 @@ func (d *dir) CreateVolumeFromMigration(vol Volume, conn io.ReadWriteCloser, vol
 
 // RefreshVolume provides same-pool volume and specific snapshots syncing functionality.
 func (d *dir) RefreshVolume(vol Volume, srcVol Volume, srcSnapshots []Volume, op *operations.Operation) error {
-	return genericCopyVolume(d, d.setupInitialQuota, vol, srcVol, srcSnapshots, op)
+	return genericCopyVolume(d, d.setupInitialQuota, vol, srcVol, srcSnapshots, true, op)
 }
 
 // DeleteVolume deletes a volume of the storage device. If any snapshots of the volume remain then


More information about the lxc-devel mailing list