[lxc-devel] [lxd/master] Storage: ZFS migration improvement

[tomponline on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 782 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200505/ec5816d8/attachment.bin>
-------------- next part --------------
From f57ba566ce6073ab40ddf129296bcb7dc1d74ffd Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 5 May 2020 15:55:25 +0100
Subject: [PATCH 1/2] lxd/storage/drivers/generic/vfs: Dont require access to
 block device when excluding root image file from rsync in
 genericVFSMigrateVolume

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

diff --git a/lxd/storage/drivers/generic_vfs.go b/lxd/storage/drivers/generic_vfs.go
index eaee1fb6df..b08e691df7 100644
--- a/lxd/storage/drivers/generic_vfs.go
+++ b/lxd/storage/drivers/generic_vfs.go
@@ -144,20 +144,21 @@ func genericVFSMigrateVolume(d Driver, s *state.State, vol Volume, conn io.ReadW
 
 	var rsyncArgs []string
 
-	// For VM volumes, if the root volume disk path is a file image then exclude it from being transferred via
-	// rsync, it will be transferred later using a different method.
+	// For VM volumes, if the root volume disk path is a file image in the volume's mount path then exclude it
+	// from being transferred via rsync during the filesystem volume transfer, as it will be transferred later
+	// using a different method.
 	if vol.IsVMBlock() {
 		if volSrcArgs.MigrationType.FSType != migration.MigrationFSType_BLOCK_AND_RSYNC {
 			return ErrNotSupported
 		}
 
-		path, err := d.GetVolumeDiskPath(vol)
+		diskPath, err := d.GetVolumeDiskPath(vol)
 		if err != nil {
 			return errors.Wrapf(err, "Error getting VM block volume disk path")
 		}
 
-		if !shared.IsBlockdevPath(path) {
-			rsyncArgs = []string{"--exclude", filepath.Base(path)}
+		if strings.HasPrefix(diskPath, vol.MountPath()) {
+			rsyncArgs = []string{"--exclude", filepath.Base(diskPath)}
 		}
 	} else if volSrcArgs.MigrationType.FSType != migration.MigrationFSType_RSYNC {
 		return ErrNotSupported

From ad18bd16e4882bd1308ba6de63dc6bede0fe6c44 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 5 May 2020 16:16:50 +0100
Subject: [PATCH 2/2] lxd/storage/drivers/driver/zfs/volumes: Updates
 MigrateVolume to avoid need to premount snapshot volume

This is now done inside genericVFSMigrateVolume and the block device doesn't need to be accessible from the start for rsync exclusion see 47b8fa8cb

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

diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go
index b9f88d13d8..ea0a35ce3c 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -1182,10 +1182,8 @@ func (d *zfs) RenameVolume(vol Volume, newVolName string, op *operations.Operati
 func (d *zfs) MigrateVolume(vol Volume, conn io.ReadWriteCloser, volSrcArgs *migration.VolumeSourceArgs, op *operations.Operation) error {
 	// Handle simple rsync and block_and_rsync through generic.
 	if volSrcArgs.MigrationType.FSType == migration.MigrationFSType_RSYNC || volSrcArgs.MigrationType.FSType == migration.MigrationFSType_BLOCK_AND_RSYNC {
-		// We need to mount the parent volume before calling genericVFSMigrateVolume for two reasons.
-		// 1. In order to get the block device disk path to read from the device must be activated.
-		// 2. If copying snapshots the parent volume must be activated before the snapshot volume's block
-		// device can be made visible.
+		// Before doing a generic volume migration, we need to ensure volume (or snap volume parent) is
+		// activated to avoid issues activating the snapshot volume device.
 		parent, _, _ := shared.InstanceGetParentAndSnapshotName(vol.Name())
 		parentVol := NewVolume(d, d.Name(), vol.volType, vol.contentType, parent, vol.config, vol.poolConfig)
 		ourMount, err := d.MountVolume(parentVol, op)
@@ -1196,18 +1194,6 @@ func (d *zfs) MigrateVolume(vol Volume, conn io.ReadWriteCloser, volSrcArgs *mig
 			defer d.UnmountVolume(parentVol, op)
 		}
 
-		// In addition to above, if the volume we are sending is a snapshot, we also need to mount that
-		// so that genericVFSMigrateVolume can discover its block device (same reason as 1. above).
-		if vol.IsSnapshot() {
-			ourMount, err = d.MountVolumeSnapshot(vol, op)
-			if err != nil {
-				return err
-			}
-			if ourMount {
-				defer d.UnmountVolumeSnapshot(vol, op)
-			}
-		}
-
 		return genericVFSMigrateVolume(d, d.state, vol, conn, volSrcArgs, op)
 	} else if volSrcArgs.MigrationType.FSType != migration.MigrationFSType_ZFS {
 		return ErrNotSupported