[lxc-devel] [lxd/master] Storage: Updates custom volume rename patch to handle daemon storage

tomponline on Github lxc-bot at linuxcontainers.org
Thu Mar 12 11:43:19 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 431 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200312/e5e72608/attachment.bin>
-------------- next part --------------
From b24c607a7c7beb753f1252ef74f6a0b426b60efa Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 12 Mar 2020 09:10:59 +0000
Subject: [PATCH 1/7] lxd/patches: Adds concept of stage to patch system

Defined two patch stages:

	Pre daemon storage
	Post daemon storage

Updates all existing patches, except `storage_rename_custom_volume_add_project` to run at the post daemon storage stage.

Sets `storage_rename_custom_volume_add_project` to run at pre daemon storage stage, so that it is able to rename the custom volumes used by daemon storage so that the mount doesn't fail at daemon storage initialisation time.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/patches.go | 104 +++++++++++++++++++++++++++++--------------------
 1 file changed, 61 insertions(+), 43 deletions(-)

diff --git a/lxd/patches.go b/lxd/patches.go
index 4af43ebcd4..43cf2753de 100644
--- a/lxd/patches.go
+++ b/lxd/patches.go
@@ -31,6 +31,15 @@ import (
 	"github.com/lxc/lxd/shared/logger"
 )
 
+type patchStage int
+
+// Define the stages that patches can run at.
+const (
+	patchNoStageSet patchStage = iota
+	patchPreDaemonStorage
+	patchPostDaemonStorage
+)
+
 /* Patches are one-time actions that are sometimes needed to update
    existing container configuration or move things around on the
    filesystem.
@@ -47,52 +56,52 @@ import (
 
    Only append to the patches list, never remove entries and never re-order them.
 */
-
 var patches = []patch{
-	{name: "shrink_logs_db_file", run: patchShrinkLogsDBFile},
-	{name: "invalid_profile_names", run: patchInvalidProfileNames},
-	{name: "leftover_profile_config", run: patchLeftoverProfileConfig},
-	{name: "network_permissions", run: patchNetworkPermissions},
-	{name: "storage_api", run: patchStorageApi},
-	{name: "storage_api_v1", run: patchStorageApiV1},
-	{name: "storage_api_dir_cleanup", run: patchStorageApiDirCleanup},
-	{name: "storage_api_lvm_keys", run: patchStorageApiLvmKeys},
-	{name: "storage_api_keys", run: patchStorageApiKeys},
-	{name: "storage_api_update_storage_configs", run: patchStorageApiUpdateStorageConfigs},
-	{name: "storage_api_lxd_on_btrfs", run: patchStorageApiLxdOnBtrfs},
-	{name: "storage_api_lvm_detect_lv_size", run: patchStorageApiDetectLVSize},
-	{name: "storage_api_insert_zfs_driver", run: patchStorageApiInsertZfsDriver},
-	{name: "storage_zfs_noauto", run: patchStorageZFSnoauto},
-	{name: "storage_zfs_volume_size", run: patchStorageZFSVolumeSize},
-	{name: "network_dnsmasq_hosts", run: patchNetworkDnsmasqHosts},
-	{name: "storage_api_dir_bind_mount", run: patchStorageApiDirBindMount},
-	{name: "fix_uploaded_at", run: patchFixUploadedAt},
-	{name: "storage_api_ceph_size_remove", run: patchStorageApiCephSizeRemove},
-	{name: "devices_new_naming_scheme", run: patchDevicesNewNamingScheme},
-	{name: "storage_api_permissions", run: patchStorageApiPermissions},
-	{name: "container_config_regen", run: patchContainerConfigRegen},
-	{name: "lvm_node_specific_config_keys", run: patchLvmNodeSpecificConfigKeys},
-	{name: "candid_rename_config_key", run: patchCandidConfigKey},
-	{name: "move_backups", run: patchMoveBackups},
-	{name: "storage_api_rename_container_snapshots_dir", run: patchStorageApiRenameContainerSnapshotsDir},
-	{name: "storage_api_rename_container_snapshots_links", run: patchStorageApiUpdateContainerSnapshots},
-	{name: "fix_lvm_pool_volume_names", run: patchRenameCustomVolumeLVs},
-	{name: "storage_api_rename_container_snapshots_dir_again", run: patchStorageApiRenameContainerSnapshotsDir},
-	{name: "storage_api_rename_container_snapshots_links_again", run: patchStorageApiUpdateContainerSnapshots},
-	{name: "storage_api_rename_container_snapshots_dir_again_again", run: patchStorageApiRenameContainerSnapshotsDir},
-	{name: "clustering_add_roles", run: patchClusteringAddRoles},
-	{name: "clustering_add_roles_again", run: patchClusteringAddRoles},
-	{name: "storage_create_vm", run: patchGenericStorage},
-	{name: "storage_zfs_mount", run: patchGenericStorage},
-	{name: "network_pid_files", run: patchNetworkPIDFiles},
-	{name: "storage_create_vm_again", run: patchGenericStorage},
-	{name: "storage_zfs_volmode", run: patchGenericStorage},
-	{name: "storage_rename_custom_volume_add_project", run: patchGenericStorage},
+	{name: "shrink_logs_db_file", stage: patchPostDaemonStorage, run: patchShrinkLogsDBFile},
+	{name: "invalid_profile_names", stage: patchPostDaemonStorage, run: patchInvalidProfileNames},
+	{name: "leftover_profile_config", stage: patchPostDaemonStorage, run: patchLeftoverProfileConfig},
+	{name: "network_permissions", stage: patchPostDaemonStorage, run: patchNetworkPermissions},
+	{name: "storage_api", stage: patchPostDaemonStorage, run: patchStorageApi},
+	{name: "storage_api_v1", stage: patchPostDaemonStorage, run: patchStorageApiV1},
+	{name: "storage_api_dir_cleanup", stage: patchPostDaemonStorage, run: patchStorageApiDirCleanup},
+	{name: "storage_api_lvm_keys", stage: patchPostDaemonStorage, run: patchStorageApiLvmKeys},
+	{name: "storage_api_keys", stage: patchPostDaemonStorage, run: patchStorageApiKeys},
+	{name: "storage_api_update_storage_configs", stage: patchPostDaemonStorage, run: patchStorageApiUpdateStorageConfigs},
+	{name: "storage_api_lxd_on_btrfs", stage: patchPostDaemonStorage, run: patchStorageApiLxdOnBtrfs},
+	{name: "storage_api_lvm_detect_lv_size", stage: patchPostDaemonStorage, run: patchStorageApiDetectLVSize},
+	{name: "storage_api_insert_zfs_driver", stage: patchPostDaemonStorage, run: patchStorageApiInsertZfsDriver},
+	{name: "storage_zfs_noauto", stage: patchPostDaemonStorage, run: patchStorageZFSnoauto},
+	{name: "storage_zfs_volume_size", stage: patchPostDaemonStorage, run: patchStorageZFSVolumeSize},
+	{name: "network_dnsmasq_hosts", stage: patchPostDaemonStorage, run: patchNetworkDnsmasqHosts},
+	{name: "storage_api_dir_bind_mount", stage: patchPostDaemonStorage, run: patchStorageApiDirBindMount},
+	{name: "fix_uploaded_at", stage: patchPostDaemonStorage, run: patchFixUploadedAt},
+	{name: "storage_api_ceph_size_remove", stage: patchPostDaemonStorage, run: patchStorageApiCephSizeRemove},
+	{name: "devices_new_naming_scheme", stage: patchPostDaemonStorage, run: patchDevicesNewNamingScheme},
+	{name: "storage_api_permissions", stage: patchPostDaemonStorage, run: patchStorageApiPermissions},
+	{name: "container_config_regen", stage: patchPostDaemonStorage, run: patchContainerConfigRegen},
+	{name: "lvm_node_specific_config_keys", stage: patchPostDaemonStorage, run: patchLvmNodeSpecificConfigKeys},
+	{name: "candid_rename_config_key", stage: patchPostDaemonStorage, run: patchCandidConfigKey},
+	{name: "move_backups", stage: patchPostDaemonStorage, run: patchMoveBackups},
+	{name: "storage_api_rename_container_snapshots_dir", stage: patchPostDaemonStorage, run: patchStorageApiRenameContainerSnapshotsDir},
+	{name: "storage_api_rename_container_snapshots_links", stage: patchPostDaemonStorage, run: patchStorageApiUpdateContainerSnapshots},
+	{name: "fix_lvm_pool_volume_names", stage: patchPostDaemonStorage, run: patchRenameCustomVolumeLVs},
+	{name: "storage_api_rename_container_snapshots_dir_again", stage: patchPostDaemonStorage, run: patchStorageApiRenameContainerSnapshotsDir},
+	{name: "storage_api_rename_container_snapshots_links_again", stage: patchPostDaemonStorage, run: patchStorageApiUpdateContainerSnapshots},
+	{name: "storage_api_rename_container_snapshots_dir_again_again", stage: patchPostDaemonStorage, run: patchStorageApiRenameContainerSnapshotsDir},
+	{name: "clustering_add_roles", stage: patchPostDaemonStorage, run: patchClusteringAddRoles},
+	{name: "clustering_add_roles_again", stage: patchPostDaemonStorage, run: patchClusteringAddRoles},
+	{name: "storage_create_vm", stage: patchPostDaemonStorage, run: patchGenericStorage},
+	{name: "storage_zfs_mount", stage: patchPostDaemonStorage, run: patchGenericStorage},
+	{name: "network_pid_files", stage: patchPostDaemonStorage, run: patchNetworkPIDFiles},
+	{name: "storage_create_vm_again", stage: patchPostDaemonStorage, run: patchGenericStorage},
+	{name: "storage_zfs_volmode", stage: patchPostDaemonStorage, run: patchGenericStorage},
+	{name: "storage_rename_custom_volume_add_project", stage: patchPreDaemonStorage, run: patchGenericStorage},
 }
 
 type patch struct {
-	name string
-	run  func(name string, d *Daemon) error
+	name  string
+	stage patchStage
+	run   func(name string, d *Daemon) error
 }
 
 func (p *patch) apply(d *Daemon) error {
@@ -115,18 +124,27 @@ func (p *patch) apply(d *Daemon) error {
 func patchesGetNames() []string {
 	names := make([]string, len(patches))
 	for i, patch := range patches {
+		if patch.stage == patchNoStageSet {
+			continue // Ignore any patch without explicitly set stage (it is defined incorrectly).
+		}
+
 		names[i] = patch.name
 	}
 	return names
 }
 
-func patchesApplyAll(d *Daemon) error {
+// patchesApplyPostDaemonStorage applies the patches that need to run after the daemon storage is initialised.
+func patchesApply(d *Daemon, stage patchStage) error {
 	appliedPatches, err := d.db.Patches()
 	if err != nil {
 		return err
 	}
 
 	for _, patch := range patches {
+		if patch.stage == patchNoStageSet {
+			return fmt.Errorf("Patch %q has no stage set: %d", patch.name, patch.stage)
+		}
+
 		if shared.StringInSlice(patch.name, appliedPatches) {
 			continue
 		}

From a6d8c25bd9f78468b9df95a22c39d4ae110ea184 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 12 Mar 2020 09:13:21 +0000
Subject: [PATCH 2/7] lxd/daemon: Applies pre daemon storage patches

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/daemon.go | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/lxd/daemon.go b/lxd/daemon.go
index 571e0ead5b..9de17fb9e6 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -822,21 +822,28 @@ func (d *Daemon) init() error {
 		version.UserAgentFeatures([]string{"cluster"})
 	}
 
-	// Read the storage pools.
+	// Mount the storage pools.
 	logger.Infof("Initializing storage pools")
 	err = setupStorageDriver(d.State(), false)
 	if err != nil {
 		return err
 	}
 
-	// Mount any daemon storage.
+	// Apply all patches that need to be run before daemon storage is initialised.
+	err = patchesApply(d, patchPreDaemonStorage)
+	if err != nil {
+		return err
+	}
+
+	// Mount any daemon storage volumes.
+	logger.Infof("Initializing daemon storage mounts")
 	err = daemonStorageMount(d.State())
 	if err != nil {
 		return err
 	}
 
-	// Apply all patches.
-	err = patchesApplyAll(d)
+	// Apply all patches that need to be run after daemon storage is initialised.
+	err = patchesApply(d, patchPostDaemonStorage)
 	if err != nil {
 		return err
 	}

From ed713f2e00303759278aab40fd1d6009949f42e9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 12 Mar 2020 10:09:32 +0000
Subject: [PATCH 3/7] lxd/storage/backend/lxd/patches: Skip already renamed
 volumes

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

diff --git a/lxd/storage/backend_lxd_patches.go b/lxd/storage/backend_lxd_patches.go
index 36a3e918fa..9414783f96 100644
--- a/lxd/storage/backend_lxd_patches.go
+++ b/lxd/storage/backend_lxd_patches.go
@@ -49,6 +49,13 @@ func lxdPatchStorageRenameCustomVolumeAddProject(b *lxdBackend) error {
 
 			// Add default project prefix to current volume name.
 			newVolStorageName := project.StorageVolume(project.Default, curVol.Name)
+			newVol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS, newVolStorageName, nil)
+
+			// Check if volume has already been renamed.
+			if b.driver.HasVolume(newVol) {
+				logger.Infof("Skipping already renamed custom volume %q in pool %q", newVol.Name(), b.Name())
+				return nil
+			}
 
 			// Check if volume is currently mounted.
 			oldMntPath := drivers.GetVolumeMountPath(b.Name(), drivers.VolumeTypeCustom, curVol.Name)
@@ -71,8 +78,6 @@ func lxdPatchStorageRenameCustomVolumeAddProject(b *lxdBackend) error {
 				return err
 			}
 
-			newVol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS, newVolStorageName, nil)
-
 			// Ensure we don't use the wrong volume for revert by using a temporary function.
 			revert.Add(func() {
 				logger.Infof("Reverting rename of custom volume %q in pool %q to %q", newVol.Name(), b.Name(), curVol.Name)

From a6a431f85a96e47c322e0c90022896f3d4a4f969 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 12 Mar 2020 10:47:12 +0000
Subject: [PATCH 4/7] lxd/daemon/storage: Removes daemonStorageUsed function

Being moves to storage package as VolumeUsedByDaemon().

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/daemon_storage.go | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/lxd/daemon_storage.go b/lxd/daemon_storage.go
index 341053fab3..3f4d5118d8 100644
--- a/lxd/daemon_storage.go
+++ b/lxd/daemon_storage.go
@@ -78,32 +78,6 @@ func daemonStorageMount(s *state.State) error {
 	return nil
 }
 
-func daemonStorageUsed(s *state.State, poolName string, volumeName string) (bool, error) {
-	var storageBackups string
-	var storageImages string
-	err := s.Node.Transaction(func(tx *db.NodeTx) error {
-		nodeConfig, err := node.ConfigLoad(tx)
-		if err != nil {
-			return err
-		}
-
-		storageBackups = nodeConfig.StorageBackupsVolume()
-		storageImages = nodeConfig.StorageImagesVolume()
-
-		return nil
-	})
-	if err != nil {
-		return false, err
-	}
-
-	fullName := fmt.Sprintf("%s/%s", poolName, volumeName)
-	if storageBackups == fullName || storageImages == fullName {
-		return true, nil
-	}
-
-	return false, nil
-}
-
 func daemonStorageValidate(s *state.State, target string) error {
 	// Check syntax.
 	if target == "" {

From e630eba255e5ed3b64e199039069d0be1e768bc3 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 12 Mar 2020 10:47:44 +0000
Subject: [PATCH 5/7] lxd/storage/utils: Adds VolumeUsedByDaemon function

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/utils.go | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index f5a4ebe734..db41fbeb7e 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -9,6 +9,7 @@ import (
 	"github.com/lxc/lxd/lxd/db"
 	"github.com/lxc/lxd/lxd/instance"
 	"github.com/lxc/lxd/lxd/instance/instancetype"
+	"github.com/lxc/lxd/lxd/node"
 	"github.com/lxc/lxd/lxd/state"
 	"github.com/lxc/lxd/lxd/storage/drivers"
 	"github.com/lxc/lxd/shared"
@@ -684,3 +685,30 @@ func VolumeUsedByRunningInstancesWithProfilesGet(s *state.State, projectName str
 
 	return instUsingVolume, nil
 }
+
+// VolumeUsedByDaemon indicates whether the volume is used by daemon storage.
+func VolumeUsedByDaemon(s *state.State, poolName string, volumeName string) (bool, error) {
+	var storageBackups string
+	var storageImages string
+	err := s.Node.Transaction(func(tx *db.NodeTx) error {
+		nodeConfig, err := node.ConfigLoad(tx)
+		if err != nil {
+			return err
+		}
+
+		storageBackups = nodeConfig.StorageBackupsVolume()
+		storageImages = nodeConfig.StorageImagesVolume()
+
+		return nil
+	})
+	if err != nil {
+		return false, err
+	}
+
+	fullName := fmt.Sprintf("%s/%s", poolName, volumeName)
+	if storageBackups == fullName || storageImages == fullName {
+		return true, nil
+	}
+
+	return false, nil
+}

From 274c00e187a6a70837824b384afb38e40dc81143 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 12 Mar 2020 10:48:03 +0000
Subject: [PATCH 6/7] lxd: storagePools.VolumeUsedByDaemon usage

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage_volumes.go          | 2 +-
 lxd/storage_volumes_snapshot.go | 2 +-
 lxd/storage_volumes_utils.go    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index 222e7e0518..ef724195d4 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -604,7 +604,7 @@ func storagePoolVolumeTypePost(d *Daemon, r *http.Request, volumeTypeName string
 	}
 
 	// Check if the daemon itself is using it.
-	used, err := daemonStorageUsed(d.State(), poolName, volumeName)
+	used, err := storagePools.VolumeUsedByDaemon(d.State(), poolName, volumeName)
 	if err != nil {
 		return response.SmartError(err)
 	}
diff --git a/lxd/storage_volumes_snapshot.go b/lxd/storage_volumes_snapshot.go
index 1c4139ff9d..75bfd9367c 100644
--- a/lxd/storage_volumes_snapshot.go
+++ b/lxd/storage_volumes_snapshot.go
@@ -81,7 +81,7 @@ func storagePoolVolumeSnapshotsTypePost(d *Daemon, r *http.Request) response.Res
 	}
 
 	// Check that this isn't a restricted volume
-	used, err := daemonStorageUsed(d.State(), poolName, volumeName)
+	used, err := storagePools.VolumeUsedByDaemon(d.State(), poolName, volumeName)
 	if err != nil {
 		return response.InternalError(err)
 	}
diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index 4fc6724854..3de665d985 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -221,7 +221,7 @@ func storagePoolVolumeUsedByGet(s *state.State, project, poolName string, volume
 	}
 
 	// Check if the daemon itself is using it
-	used, err := daemonStorageUsed(s, poolName, volumeName)
+	used, err := storagePools.VolumeUsedByDaemon(s, poolName, volumeName)
 	if err != nil {
 		return []string{}, err
 	}

From 2c8835bb32eeb612102df1846c555ab6bf9fce62 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 12 Mar 2020 11:32:35 +0000
Subject: [PATCH 7/7] lxd/storage/backend/lxd/patches: Adds daemon storage
 symlink update to lxdPatchStorageRenameCustomVolumeAddProject

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/backend_lxd_patches.go | 41 +++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/lxd/storage/backend_lxd_patches.go b/lxd/storage/backend_lxd_patches.go
index 9414783f96..470cd313de 100644
--- a/lxd/storage/backend_lxd_patches.go
+++ b/lxd/storage/backend_lxd_patches.go
@@ -1,8 +1,9 @@
 package storage
 
 import (
-	"github.com/pkg/errors"
+	"os"
 
+	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 
 	"github.com/lxc/lxd/lxd/db"
@@ -84,6 +85,44 @@ func lxdPatchStorageRenameCustomVolumeAddProject(b *lxdBackend) error {
 				b.driver.RenameVolume(newVol, curVol.Name, nil)
 			})
 
+			// Check if volume is being used by daemon storage and needs its symlink updating.
+			used, err := VolumeUsedByDaemon(b.state, b.Name(), curVol.Name)
+			if err != nil {
+				return err
+			}
+
+			if used {
+				logger.Infof("Updating daemon storage symlinks for volume %q in pool %q", curVol.Name, b.Name())
+				for _, storageType := range []string{"images", "backups"} {
+					err = func(storageType string) error {
+						symlinkPath := shared.VarPath(storageType)
+						destPath, err := os.Readlink(symlinkPath)
+
+						// Check if storage type path is a symlink and points to volume.
+						if err == nil && destPath == oldVol.MountPath() {
+							newDestPath := newVol.MountPath()
+							logger.Infof("Updating daemon storage symlink at %q to %q", storageType, symlinkPath, newDestPath)
+							os.Remove(symlinkPath)
+							err = os.Symlink(newDestPath, symlinkPath)
+							if err != nil {
+								return errors.Wrapf(err, "Failed to create the new symlink at %q to %q", symlinkPath, newDestPath)
+							}
+
+							revert.Add(func() {
+								logger.Infof("Reverting daemon storage symlink at %q to %q", storageType, symlinkPath, destPath)
+								os.Remove(symlinkPath)
+								os.Symlink(destPath, symlinkPath)
+							})
+						}
+
+						return nil
+					}(storageType)
+					if err != nil {
+						return err
+					}
+				}
+			}
+
 			if ourUnmount {
 				logger.Infof("Mount custom volume %q in pool %q", newVolStorageName, b.Name())
 				_, err = b.driver.MountVolume(newVol, nil)


More information about the lxc-devel mailing list