[lxc-devel] [lxd/master] Storage Instance Rename

tomponline on Github lxc-bot at linuxcontainers.org
Tue Nov 5 18:37:34 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 354 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20191105/9a29bf38/attachment-0001.bin>
-------------- next part --------------
From 2a5f7bec1784cd68b70d32f9a3328d96b06b06bf Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 5 Nov 2019 18:34:08 +0000
Subject: [PATCH 1/4] lxd/container/lxc: Links Rename to new storage package

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container_lxc.go | 168 ++++++++++++++++++++++++++-----------------
 1 file changed, 102 insertions(+), 66 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index f2ceb68781..cd175ee210 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -3736,13 +3736,7 @@ func (c *containerLXC) Rename(newName string) error {
 
 	logger.Info("Renaming container", ctxMap)
 
-	// Initialize storage interface for the container.
-	err := c.initStorage()
-	if err != nil {
-		return err
-	}
-
-	// Sanity checks
+	// Sanity checks.
 	if !c.IsSnapshot() && !shared.ValidHostname(newName) {
 		return fmt.Errorf("Invalid container name")
 	}
@@ -3751,62 +3745,89 @@ func (c *containerLXC) Rename(newName string) error {
 		return fmt.Errorf("Renaming of running container not allowed")
 	}
 
-	// Clean things up
+	// Clean things up.
 	c.cleanup()
 
-	// Rename the MAAS entry
-	if !c.IsSnapshot() {
-		err = c.maasRename(newName)
+	// Check if we can load new storage layer for pool driver type.
+	pool, err := storagePools.GetPoolByInstance(c.state, c)
+	if err != storageDrivers.ErrUnknownDriver && err != storageDrivers.ErrNotImplemented {
 		if err != nil {
-			return err
+			return errors.Wrap(err, "Load instance storage pool")
 		}
-	}
 
-	// Rename the logging path
-	os.RemoveAll(shared.LogPath(newName))
-	if shared.PathExists(c.LogPath()) {
-		err := os.Rename(c.LogPath(), shared.LogPath(newName))
-		if err != nil {
-			logger.Error("Failed renaming container", ctxMap)
-			return err
+		if c.IsSnapshot() {
+			err = pool.RenameInstanceSnapshot(c, newName, nil)
+			if err != nil {
+				return errors.Wrap(err, "Rename instance snapshot")
+			}
+		} else {
+			err = pool.RenameInstance(c, newName, nil)
+			if err != nil {
+				return errors.Wrap(err, "Rename instance")
+			}
 		}
-	}
-
-	// Rename the storage entry
-	if c.IsSnapshot() {
-		err := c.storage.ContainerSnapshotRename(c, newName)
+	} else if c.Type() == instancetype.Container {
+		// Initialize storage interface for the container.
+		err = c.initStorage()
 		if err != nil {
-			logger.Error("Failed renaming container", ctxMap)
 			return err
 		}
-	} else {
-		err := c.storage.ContainerRename(c, newName)
-		if err != nil {
-			logger.Error("Failed renaming container", ctxMap)
-			return err
+
+		// Rename the storage entry.
+		if c.IsSnapshot() {
+			err := c.storage.ContainerSnapshotRename(c, newName)
+			if err != nil {
+				logger.Error("Failed renaming container", ctxMap)
+				return err
+			}
+		} else {
+			err := c.storage.ContainerRename(c, newName)
+			if err != nil {
+				logger.Error("Failed renaming container", ctxMap)
+				return err
+			}
 		}
-	}
 
-	// Rename the backups
-	backups, err := c.Backups()
-	if err != nil {
-		return err
-	}
+		poolID, _, _ := c.storage.GetContainerPoolInfo()
 
-	for _, backup := range backups {
-		backupName := strings.Split(backup.Name(), "/")[1]
-		newName := fmt.Sprintf("%s/%s", newName, backupName)
+		if !c.IsSnapshot() {
+			// Rename all the snapshot volumes.
+			results, err := c.state.Cluster.ContainerGetSnapshots(c.project, oldName)
+			if err != nil {
+				logger.Error("Failed to get container snapshots", ctxMap)
+				return err
+			}
 
-		err = backup.Rename(newName)
+			for _, sname := range results {
+				// Rename the snapshot volume.
+				baseSnapName := filepath.Base(sname)
+				newSnapshotName := newName + shared.SnapshotDelimiter + baseSnapName
+
+				// Rename storage volume for the snapshot.
+				err = c.state.Cluster.StoragePoolVolumeRename(c.project, sname, newSnapshotName, storagePoolVolumeTypeContainer, poolID)
+				if err != nil {
+					logger.Error("Failed renaming storage volume", ctxMap)
+					return err
+				}
+			}
+		}
+
+		// Rename storage volume for the container.
+		err = c.state.Cluster.StoragePoolVolumeRename(c.project, oldName, newName, storagePoolVolumeTypeContainer, poolID)
 		if err != nil {
+			logger.Error("Failed renaming storage volume", ctxMap)
 			return err
 		}
-	}
 
-	poolID, _, _ := c.storage.GetContainerPoolInfo()
+		// Update the storage volume name in the storage interface.
+		sNew := c.storage.GetStoragePoolVolumeWritable()
+		c.storage.SetStoragePoolVolumeWritable(&sNew)
+	} else {
+		return fmt.Errorf("Instance type not supported")
+	}
 
 	if !c.IsSnapshot() {
-		// Rename all the snapshots
+		// Rename all the instance snapshot database entries.
 		results, err := c.state.Cluster.ContainerGetSnapshots(c.project, oldName)
 		if err != nil {
 			logger.Error("Failed to get container snapshots", ctxMap)
@@ -3814,10 +3835,9 @@ func (c *containerLXC) Rename(newName string) error {
 		}
 
 		for _, sname := range results {
-			// Rename the snapshot
+			// Rename the snapshot.
 			oldSnapName := strings.SplitN(sname, shared.SnapshotDelimiter, 2)[1]
 			baseSnapName := filepath.Base(sname)
-			newSnapshotName := newName + shared.SnapshotDelimiter + baseSnapName
 			err := c.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
 				return tx.InstanceSnapshotRename(c.project, oldName, oldSnapName, baseSnapName)
 			})
@@ -3825,46 +3845,62 @@ func (c *containerLXC) Rename(newName string) error {
 				logger.Error("Failed renaming snapshot", ctxMap)
 				return err
 			}
-
-			// Rename storage volume for the snapshot.
-			err = c.state.Cluster.StoragePoolVolumeRename(c.project, sname, newSnapshotName, storagePoolVolumeTypeContainer, poolID)
-			if err != nil {
-				logger.Error("Failed renaming storage volume", ctxMap)
-				return err
-			}
 		}
 	}
 
-	// Rename the database entry
+	// Rename the instance database entry.
 	err = c.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
 		if c.IsSnapshot() {
 			oldParts := strings.SplitN(oldName, shared.SnapshotDelimiter, 2)
 			newParts := strings.SplitN(newName, shared.SnapshotDelimiter, 2)
 			return tx.InstanceSnapshotRename(c.project, oldParts[0], oldParts[1], newParts[1])
-		} else {
-			return tx.InstanceRename(c.project, oldName, newName)
 		}
+
+		return tx.InstanceRename(c.project, oldName, newName)
 	})
 	if err != nil {
 		logger.Error("Failed renaming container", ctxMap)
 		return err
 	}
 
-	// Rename storage volume for the container.
-	err = c.state.Cluster.StoragePoolVolumeRename(c.project, oldName, newName, storagePoolVolumeTypeContainer, poolID)
+	// Rename the logging path.
+	os.RemoveAll(shared.LogPath(newName))
+	if shared.PathExists(c.LogPath()) {
+		err := os.Rename(c.LogPath(), shared.LogPath(newName))
+		if err != nil {
+			logger.Error("Failed renaming container", ctxMap)
+			return err
+		}
+	}
+
+	// Rename the MAAS entry.
+	if !c.IsSnapshot() {
+		err = c.maasRename(newName)
+		if err != nil {
+			return err
+		}
+	}
+
+	// Rename the backups.
+	backups, err := c.Backups()
 	if err != nil {
-		logger.Error("Failed renaming storage volume", ctxMap)
 		return err
 	}
 
-	// Set the new name in the struct
-	c.name = newName
+	for _, backup := range backups {
+		backupName := strings.Split(backup.Name(), "/")[1]
+		newName := fmt.Sprintf("%s/%s", newName, backupName)
+
+		err = backup.Rename(newName)
+		if err != nil {
+			return err
+		}
+	}
 
-	// Update the storage volume name in the storage interface.
-	sNew := c.storage.GetStoragePoolVolumeWritable()
-	c.storage.SetStoragePoolVolumeWritable(&sNew)
+	// Set the new name in the struct.
+	c.name = newName
 
-	// Invalidate the go-lxc cache
+	// Invalidate the go-lxc cache.
 	if c.c != nil {
 		c.c.Release()
 		c.c = nil
@@ -3872,7 +3908,7 @@ func (c *containerLXC) Rename(newName string) error {
 
 	c.cConfig = false
 
-	// Update lease files
+	// Update lease files.
 	networkUpdateStatic(c.state, "")
 
 	logger.Info("Renamed container", ctxMap)

From a320cf54c259810895fa9c6acb5e3999b1c4603b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 5 Nov 2019 18:35:09 +0000
Subject: [PATCH 2/4] lxd/storage/backend/lxd: Reworks symlink functions

And makes some function comments use consistent terminology.

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

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 5600ec4c3a..c6cc9357ba 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -161,9 +161,12 @@ func (b *lxdBackend) Unmount() (bool, error) {
 	return b.driver.Unmount()
 }
 
-// createInstanceSymlink creates a symlink in the instance directory to the instance's mount path.
-func (b *lxdBackend) createInstanceSymlink(inst Instance, mountPath string) error {
-	symlinkPath := inst.Path()
+// ensureInstanceSymlink creates a symlink in the instance directory to the instance's mount path
+// if doesn't exist already.
+func (b *lxdBackend) ensureInstanceSymlink(instanceType instancetype.Type, projectName, instanceName, mountPath string) error {
+	volStorageName := project.Prefix(projectName, instanceName)
+	symlinkPath := ContainerPath(volStorageName, false)
+
 	if !shared.PathExists(symlinkPath) {
 		err := os.Symlink(mountPath, symlinkPath)
 		if err != nil {
@@ -175,8 +178,10 @@ func (b *lxdBackend) createInstanceSymlink(inst Instance, mountPath string) erro
 }
 
 // removeInstanceSymlink removes a symlink in the instance directory to the instance's mount path.
-func (b *lxdBackend) removeInstanceSymlink(inst Instance) error {
-	symlinkPath := inst.Path()
+func (b *lxdBackend) removeInstanceSymlink(instanceType instancetype.Type, projectName, instanceName string) error {
+	volStorageName := project.Prefix(projectName, instanceName)
+	symlinkPath := ContainerPath(volStorageName, false)
+
 	if shared.PathExists(symlinkPath) {
 		err := os.Remove(symlinkPath)
 		if err != nil {
@@ -189,15 +194,16 @@ func (b *lxdBackend) removeInstanceSymlink(inst Instance) error {
 
 // ensureInstanceSnapshotSymlink creates a symlink in the snapshot directory to the instance's
 // snapshot path if doesn't exist already.
-func (b *lxdBackend) ensureInstanceSnapshotSymlink(inst Instance) error {
+func (b *lxdBackend) ensureInstanceSnapshotSymlink(instanceType instancetype.Type, projectName, instanceName string) error {
 	// Check we can convert the instance to the volume type needed.
-	volType, err := InstanceTypeToVolumeType(inst.Type())
+	volType, err := InstanceTypeToVolumeType(instanceType)
 	if err != nil {
 		return err
 	}
 
-	snapshotSymlink := shared.VarPath("snapshots", project.Prefix(inst.Project(), inst.Name()))
-	volStorageName := project.Prefix(inst.Project(), inst.Name())
+	parentName, _, _ := shared.ContainerGetParentAndSnapshotName(instanceName)
+	snapshotSymlink := shared.VarPath("snapshots", project.Prefix(projectName, parentName))
+	volStorageName := project.Prefix(projectName, parentName)
 
 	snapshotTargetPath, err := drivers.GetVolumeSnapshotDir(b.name, volType, volStorageName)
 	if err != nil {
@@ -217,15 +223,16 @@ func (b *lxdBackend) ensureInstanceSnapshotSymlink(inst Instance) error {
 // removeInstanceSnapshotSymlinkIfUnused removes the symlink in the snapshot directory to the
 // instance's snapshot path if the snapshot path is missing. It is expected that the driver will
 // remove the instance's snapshot path after the last snapshot is removed or the volume is deleted.
-func (b *lxdBackend) removeInstanceSnapshotSymlinkIfUnused(inst Instance) error {
+func (b *lxdBackend) removeInstanceSnapshotSymlinkIfUnused(instanceType instancetype.Type, projectName, instanceName string) error {
 	// Check we can convert the instance to the volume type needed.
-	volType, err := InstanceTypeToVolumeType(inst.Type())
+	volType, err := InstanceTypeToVolumeType(instanceType)
 	if err != nil {
 		return err
 	}
 
-	snapshotSymlink := shared.VarPath("snapshots", project.Prefix(inst.Project(), inst.Name()))
-	volStorageName := project.Prefix(inst.Project(), inst.Name())
+	parentName, _, _ := shared.ContainerGetParentAndSnapshotName(instanceName)
+	snapshotSymlink := shared.VarPath("snapshots", project.Prefix(projectName, parentName))
+	volStorageName := project.Prefix(projectName, parentName)
 
 	snapshotTargetPath, err := drivers.GetVolumeSnapshotDir(b.name, volType, volStorageName)
 	if err != nil {
@@ -270,7 +277,7 @@ func (b *lxdBackend) CreateInstance(inst Instance, op *operations.Operation) err
 		return err
 	}
 
-	err = b.createInstanceSymlink(inst, vol.MountPath())
+	err = b.ensureInstanceSymlink(inst.Type(), inst.Project(), inst.Name(), vol.MountPath())
 	if err != nil {
 		return err
 	}
@@ -356,7 +363,7 @@ func (b *lxdBackend) CreateInstanceFromImage(inst Instance, fingerprint string,
 		}
 	}
 
-	err = b.createInstanceSymlink(inst, vol.MountPath())
+	err = b.ensureInstanceSymlink(inst.Type(), inst.Project(), inst.Name(), vol.MountPath())
 	if err != nil {
 		return err
 	}
@@ -422,12 +429,12 @@ func (b *lxdBackend) DeleteInstance(inst Instance, op *operations.Operation) err
 	}
 
 	// Remove symlinks.
-	err = b.removeInstanceSymlink(inst)
+	err = b.removeInstanceSymlink(inst.Type(), inst.Project(), inst.Name())
 	if err != nil {
 		return err
 	}
 
-	err = b.removeInstanceSnapshotSymlinkIfUnused(inst)
+	err = b.removeInstanceSnapshotSymlinkIfUnused(inst.Type(), inst.Project(), inst.Name())
 	if err != nil {
 		return err
 	}
@@ -453,7 +460,7 @@ func (b *lxdBackend) BackupInstance(inst Instance, targetPath string, optimized
 	return ErrNotImplemented
 }
 
-// GetInstanceUsage returns the disk usage of the instance's root device.
+// GetInstanceUsage returns the disk usage of the instance's root volume.
 func (b *lxdBackend) GetInstanceUsage(inst Instance) (int64, error) {
 	logger := logging.AddContext(b.logger, log.Ctx{"project": inst.Project(), "instance": inst.Name()})
 	logger.Debug("GetInstanceUsage started")
@@ -466,7 +473,7 @@ func (b *lxdBackend) GetInstanceUsage(inst Instance) (int64, error) {
 	return -1, ErrNotImplemented
 }
 
-// SetInstanceQuota sets the quota on the instance's root device.
+// SetInstanceQuota sets the quota on the instance's root volume.
 // Returns ErrRunningQuotaResizeNotSupported if the instance is running and the storage driver
 // doesn't support resizing whilst the instance is running.
 func (b *lxdBackend) SetInstanceQuota(inst Instance, size string, op *operations.Operation) error {
@@ -490,7 +497,7 @@ func (b *lxdBackend) SetInstanceQuota(inst Instance, size string, op *operations
 	return b.driver.SetVolumeQuota(volType, volStorageName, size, op)
 }
 
-// MountInstance mounts the instance's device.
+// MountInstance mounts the instance's root volume.
 func (b *lxdBackend) MountInstance(inst Instance, op *operations.Operation) (bool, error) {
 	logger := logging.AddContext(b.logger, log.Ctx{"project": inst.Project(), "instance": inst.Name()})
 	logger.Debug("MountInstance started")
@@ -508,7 +515,7 @@ func (b *lxdBackend) MountInstance(inst Instance, op *operations.Operation) (boo
 	return b.driver.MountVolume(volType, volStorageName, op)
 }
 
-// UnmountInstance unmounts the instance's device.
+// UnmountInstance unmounts the instance's root volume.
 func (b *lxdBackend) UnmountInstance(inst Instance, op *operations.Operation) (bool, error) {
 	logger := logging.AddContext(b.logger, log.Ctx{"project": inst.Project(), "instance": inst.Name()})
 	logger.Debug("UnmountInstance started")
@@ -572,7 +579,7 @@ func (b *lxdBackend) DeleteInstanceSnapshot(inst Instance, op *operations.Operat
 	}
 
 	// Delete symlink if needed.
-	err = b.removeInstanceSnapshotSymlinkIfUnused(inst)
+	err = b.removeInstanceSnapshotSymlinkIfUnused(inst.Type(), inst.Project(), inst.Name())
 	if err != nil {
 		return err
 	}

From ef53b0136800acac3b5cd01a423700c64b2db959 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 5 Nov 2019 18:35:44 +0000
Subject: [PATCH 3/4] lxd/storage/backend/lxd: RenameInstance

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

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index c6cc9357ba..2fa6448e11 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -381,8 +381,109 @@ func (b *lxdBackend) CreateInstanceFromMigration(inst Instance, conn io.ReadWrit
 	return ErrNotImplemented
 }
 
+// RenameInstance renames the instance's root volume and any snapshot volumes.
 func (b *lxdBackend) RenameInstance(inst Instance, newName string, op *operations.Operation) error {
-	return ErrNotImplemented
+	logger := logging.AddContext(b.logger, log.Ctx{"project": inst.Project(), "instance": inst.Name(), "newName": newName})
+	logger.Debug("RenameInstance started")
+	defer logger.Debug("RenameInstance finished")
+
+	if inst.IsSnapshot() {
+		return fmt.Errorf("Instance cannot be a snapshot")
+	}
+
+	if shared.IsSnapshot(newName) {
+		return fmt.Errorf("New volume name cannot be a snapshot")
+	}
+
+	// Check we can convert the instance to the volume types needed.
+	volType, err := InstanceTypeToVolumeType(inst.Type())
+	if err != nil {
+		return err
+	}
+
+	volDBType, err := VolumeTypeToDBType(volType)
+	if err != nil {
+		return err
+	}
+
+	type volRevert struct {
+		oldName string
+		newName string
+	}
+
+	// Create slice to record DB volumes renamed if revert needed later.
+	revertDBVolumes := []volRevert{}
+	defer func() {
+		// Remove any DB volume rows created if we are reverting.
+		for _, vol := range revertDBVolumes {
+			b.state.Cluster.StoragePoolVolumeRename(inst.Project(), vol.newName, vol.oldName, volDBType, b.ID())
+		}
+	}()
+
+	// Get any snapshots the instance has in the format <instance name>/<snapshot name>.
+	snapshots, err := b.state.Cluster.ContainerGetSnapshots(inst.Project(), inst.Name())
+	if err != nil {
+		return err
+	}
+
+	// Rename each snapshot DB record to have the new parent volume prefix.
+	for _, srcSnapshot := range snapshots {
+		_, snapName, _ := shared.ContainerGetParentAndSnapshotName(srcSnapshot)
+		newSnapVolName := drivers.GetSnapshotVolumeName(newName, snapName)
+		err = b.state.Cluster.StoragePoolVolumeRename(inst.Project(), srcSnapshot, newSnapVolName, volDBType, b.ID())
+		if err != nil {
+			return err
+		}
+
+		revertDBVolumes = append(revertDBVolumes, volRevert{
+			newName: newSnapVolName,
+			oldName: srcSnapshot,
+		})
+	}
+
+	// Rename the parent volume DB record.
+	err = b.state.Cluster.StoragePoolVolumeRename(inst.Project(), inst.Name(), newName, volDBType, b.ID())
+	if err != nil {
+		return err
+	}
+
+	revertDBVolumes = append(revertDBVolumes, volRevert{
+		newName: newName,
+		oldName: inst.Name(),
+	})
+
+	// Rename the volume and its snapshots on the storage device.
+	err = b.driver.RenameVolume(volType, inst.Name(), newName, op)
+	if err != nil {
+		return err
+	}
+
+	// Remove old instance symlink and create new one.
+	err = b.removeInstanceSymlink(inst.Type(), inst.Project(), inst.Name())
+	if err != nil {
+		return err
+	}
+
+	err = b.ensureInstanceSymlink(inst.Type(), inst.Project(), newName, drivers.GetVolumeMountPath(b.name, volType, newName))
+	if err != nil {
+		return err
+	}
+
+	// Remove old instance snapshot symlink and create a new one if needed.
+	err = b.removeInstanceSnapshotSymlinkIfUnused(inst.Type(), inst.Project(), inst.Name())
+	if err != nil {
+		return err
+	}
+
+	if len(snapshots) > 0 {
+		err = b.ensureInstanceSnapshotSymlink(inst.Type(), inst.Project(), newName)
+		if err != nil {
+			return err
+		}
+	}
+
+	revertDBVolumes = nil
+	return nil
 }
 
 // DeleteInstance removes the instance's root volume (all snapshots need to be removed first).

From 7514973b25ad2af56c45d3a5a08a94587dec34d3 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 5 Nov 2019 18:35:58 +0000
Subject: [PATCH 4/4] lxd/storage/interfaces: Removes unused Path function in
 Instance interface

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

diff --git a/lxd/storage/interfaces.go b/lxd/storage/interfaces.go
index 398adc21eb..cc7d249a55 100644
--- a/lxd/storage/interfaces.go
+++ b/lxd/storage/interfaces.go
@@ -16,7 +16,6 @@ type Instance interface {
 	Name() string
 	Project() string
 	Type() instancetype.Type
-	Path() string
 
 	IsRunning() bool
 	IsSnapshot() bool


More information about the lxc-devel mailing list