[lxc-devel] [lxd/master] Storage: Fixes container import bugs when using projects

tomponline on Github lxc-bot at linuxcontainers.org
Wed Feb 19 11:40:45 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 565 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200219/b7a87979/attachment.bin>
-------------- next part --------------
From 1d9e00c0bb7633d81752cec6e7d97335eaac3b6b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 19 Feb 2020 11:05:14 +0000
Subject: [PATCH 1/6] lxd/api/internal: Removes duplicate storage package
 import

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

diff --git a/lxd/api_internal.go b/lxd/api_internal.go
index 8bbcbb20c0..e514c67d00 100644
--- a/lxd/api_internal.go
+++ b/lxd/api_internal.go
@@ -25,7 +25,6 @@ import (
 	"github.com/lxc/lxd/lxd/instance/instancetype"
 	"github.com/lxc/lxd/lxd/project"
 	"github.com/lxc/lxd/lxd/response"
-	driver "github.com/lxc/lxd/lxd/storage"
 	storagePools "github.com/lxc/lxd/lxd/storage"
 	storageDrivers "github.com/lxc/lxd/lxd/storage/drivers"
 	"github.com/lxc/lxd/shared"
@@ -430,7 +429,7 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 	containerMntPoints := []string{}
 	containerPoolName := ""
 	for _, poolName := range storagePoolNames {
-		containerMntPoint := driver.GetContainerMountPoint(projectName, poolName, req.Name)
+		containerMntPoint := storagePools.GetContainerMountPoint(projectName, poolName, req.Name)
 		if shared.PathExists(containerMntPoint) {
 			containerMntPoints = append(containerMntPoints, containerMntPoint)
 			containerPoolName = poolName
@@ -545,7 +544,7 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 	if len(backup.Snapshots) > 0 {
 		switch backup.Pool.Driver {
 		case "btrfs":
-			snapshotsDirPath := driver.GetSnapshotMountPoint(projectName, poolName, req.Name)
+			snapshotsDirPath := storagePools.GetSnapshotMountPoint(projectName, poolName, req.Name)
 			snapshotsDir, err := os.Open(snapshotsDirPath)
 			if err != nil {
 				return response.InternalError(err)
@@ -557,7 +556,7 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 			}
 			snapshotsDir.Close()
 		case "dir":
-			snapshotsDirPath := driver.GetSnapshotMountPoint(projectName, poolName, req.Name)
+			snapshotsDirPath := storagePools.GetSnapshotMountPoint(projectName, poolName, req.Name)
 			snapshotsDir, err := os.Open(snapshotsDirPath)
 			if err != nil {
 				return response.InternalError(err)
@@ -676,7 +675,7 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 				onDiskPoolName = poolName
 			}
 			snapName := fmt.Sprintf("%s/%s", req.Name, od)
-			snapPath := driver.InstancePath(instancetype.Container, projectName, snapName, true)
+			snapPath := storagePools.InstancePath(instancetype.Container, projectName, snapName, true)
 			err = lvmContainerDeleteInternal(projectName, poolName, req.Name,
 				true, onDiskPoolName, snapPath)
 		case "ceph":
@@ -712,15 +711,15 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 	for _, snap := range backup.Snapshots {
 		switch backup.Pool.Driver {
 		case "btrfs":
-			snpMntPt := driver.GetSnapshotMountPoint(projectName, backup.Pool.Name, snap.Name)
-			if !shared.PathExists(snpMntPt) || !isBtrfsSubVolume(snpMntPt) {
+			snpMntPt := storagePools.GetSnapshotMountPoint(projectName, backup.Pool.Name, snap.Name)
+			if !shared.PathExists(snpMntPt) || !btrfsIsSubVolume(snpMntPt) {
 				if req.Force {
 					continue
 				}
 				return response.BadRequest(needForce)
 			}
 		case "dir":
-			snpMntPt := driver.GetSnapshotMountPoint(projectName, backup.Pool.Name, snap.Name)
+			snpMntPt := storagePools.GetSnapshotMountPoint(projectName, backup.Pool.Name, snap.Name)
 			if !shared.PathExists(snpMntPt) {
 				if req.Force {
 					continue
@@ -903,12 +902,12 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 		return response.SmartError(err)
 	}
 
-	containerPath := driver.InstancePath(instancetype.Container, projectName, req.Name, false)
+	containerPath := storagePools.InstancePath(instancetype.Container, projectName, req.Name, false)
 	isPrivileged := false
 	if backup.Container.Config["security.privileged"] == "" {
 		isPrivileged = true
 	}
-	err = driver.CreateContainerMountpoint(containerMntPoint, containerPath,
+	err = storagePools.CreateContainerMountpoint(containerMntPoint, containerPath,
 		isPrivileged)
 	if err != nil {
 		return response.InternalError(err)
@@ -1005,13 +1004,13 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 		}
 
 		// Recreate missing mountpoints and symlinks.
-		snapshotMountPoint := driver.GetSnapshotMountPoint(projectName, backup.Pool.Name,
+		snapshotMountPoint := storagePools.GetSnapshotMountPoint(projectName, backup.Pool.Name,
 			snap.Name)
 		sourceName, _, _ := shared.InstanceGetParentAndSnapshotName(snap.Name)
 		sourceName = project.Prefix(projectName, sourceName)
 		snapshotMntPointSymlinkTarget := shared.VarPath("storage-pools", backup.Pool.Name, "containers-snapshots", sourceName)
 		snapshotMntPointSymlink := shared.VarPath("snapshots", sourceName)
-		err = driver.CreateSnapshotMountpoint(snapshotMountPoint, snapshotMntPointSymlinkTarget, snapshotMntPointSymlink)
+		err = storagePools.CreateSnapshotMountpoint(snapshotMountPoint, snapshotMntPointSymlinkTarget, snapshotMntPointSymlink)
 		if err != nil {
 			return response.InternalError(err)
 		}

From 67bd345875735aae405e1ecd4f68cee9d876cbb3 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 19 Feb 2020 11:05:50 +0000
Subject: [PATCH 2/6] lxd/storage: Adds InstanceImportingFilePath function

This centralises the logic of generating the ".importing" file for detection of `lxd import` to avoid issues with the file creating logic being different to the file checking logic.

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

diff --git a/lxd/storage/storage.go b/lxd/storage/storage.go
index a30471982b..8a37e3ae43 100644
--- a/lxd/storage/storage.go
+++ b/lxd/storage/storage.go
@@ -26,6 +26,21 @@ func InstancePath(instanceType instancetype.Type, projectName, instanceName stri
 	return shared.VarPath("containers", fullName)
 }
 
+// InstanceImportingFilePath returns the file path used to indicate an instance import is in progress.
+// This marker file is created when using `lxd import` to import an instance that exists on the storage device
+// but does not exist in the LXD database. The presence of this file causes the instance not to be removed from
+// the storage device if the import should fail for some reason.
+func InstanceImportingFilePath(instanceType instancetype.Type, poolName, projectName, instanceName string) string {
+	fullName := project.Prefix(projectName, instanceName)
+
+	typeDir := "containers"
+	if instanceType == instancetype.VM {
+		typeDir = "virtual-machines"
+	}
+
+	return shared.VarPath("storage-pools", poolName, typeDir, fullName, ".importing")
+}
+
 // GetStoragePoolMountPoint returns the mountpoint of the given pool.
 // {LXD_DIR}/storage-pools/<pool>
 // Deprecated, use GetPoolMountPath in storage/drivers package.

From 4ad0c146a4bbe5bb98e6c253835992260f56b2e4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 19 Feb 2020 11:05:35 +0000
Subject: [PATCH 3/6] lxd/api/internal: storagePools.InstanceImportingFilePath
 usage

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

diff --git a/lxd/api_internal.go b/lxd/api_internal.go
index e514c67d00..4d11c85d54 100644
--- a/lxd/api_internal.go
+++ b/lxd/api_internal.go
@@ -850,7 +850,8 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 	rootDev["pool"] = containerPoolName
 
 	// Mark the filesystem as going through an import
-	fd, err := os.Create(filepath.Join(containerMntPoint, ".importing"))
+	importingFilePath := storagePools.InstanceImportingFilePath(instancetype.Container, containerPoolName, projectName, req.Name)
+	fd, err := os.Create(importingFilePath)
 	if err != nil {
 		return response.InternalError(err)
 	}

From fca9ccce6318c8b9bb46570e608dccc171f58eca Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 19 Feb 2020 11:06:48 +0000
Subject: [PATCH 4/6] lxd/container/lxc: storagePools.InstanceImportingFilePath
 usage

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

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 361ff52d6e..2fb6e0f705 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -3454,16 +3454,10 @@ func (c *containerLXC) Delete() error {
 		// the creation of the instance and we are now being asked to delete the instance,
 		// we should not remove the storage volumes themselves as this would cause data loss.
 		isImport := false
-		poolName := pool.Name()
-		if c.IsSnapshot() {
-			cName, _, _ := shared.InstanceGetParentAndSnapshotName(c.name)
-			if shared.PathExists(shared.VarPath("storage-pools", poolName, "containers", cName, ".importing")) {
-				isImport = true
-			}
-		} else {
-			if shared.PathExists(shared.VarPath("storage-pools", poolName, "containers", c.name, ".importing")) {
-				isImport = true
-			}
+		cName, _, _ := shared.InstanceGetParentAndSnapshotName(c.Name())
+		importingFilePath := storagePools.InstanceImportingFilePath(c.Type(), pool.Name(), c.Project(), cName)
+		if shared.PathExists(importingFilePath) {
+			isImport = true
 		}
 
 		if c.IsSnapshot() {
@@ -3508,15 +3502,10 @@ func (c *containerLXC) Delete() error {
 		// the creation of the instance and we are now being asked to delete the instance,
 		// we should not remove the storage volumes themselves as this would cause data loss.
 		isImport := false
-		if c.IsSnapshot() {
-			cName, _, _ := shared.InstanceGetParentAndSnapshotName(c.name)
-			if shared.PathExists(shared.VarPath("storage-pools", poolName, "containers", cName, ".importing")) {
-				isImport = true
-			}
-		} else {
-			if shared.PathExists(shared.VarPath("storage-pools", poolName, "containers", c.name, ".importing")) {
-				isImport = true
-			}
+		cName, _, _ := shared.InstanceGetParentAndSnapshotName(c.Name())
+		importingFilePath := storagePools.InstanceImportingFilePath(c.Type(), poolName, c.Project(), cName)
+		if shared.PathExists(importingFilePath) {
+			isImport = true
 		}
 
 		if c.IsSnapshot() {

From 08549f8c7b18e144d81d229c209f37c8dd883ed5 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 19 Feb 2020 11:32:41 +0000
Subject: [PATCH 5/6] lxd/api: projectParam comments

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

diff --git a/lxd/api.go b/lxd/api.go
index cdca5ae0c7..999e0fad4a 100644
--- a/lxd/api.go
+++ b/lxd/api.go
@@ -120,7 +120,7 @@ func isClusterNotification(r *http.Request) bool {
 	return r.Header.Get("User-Agent") == "lxd-cluster-notifier"
 }
 
-// Extract the project query parameter from the given request.
+// projectParam returns the project query parameter from the given request or "default" if parameter is not set.
 func projectParam(request *http.Request) string {
 	project := queryParam(request, "project")
 	if project == "" {

From 29632577a9bfb7709ee66f87adcaf9e0bb02ba65 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 19 Feb 2020 11:33:14 +0000
Subject: [PATCH 6/6] lxd/api/internal: Uses
 StoragePoolNodeVolumeGetTypeByProject for project support

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/api_internal.go | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/lxd/api_internal.go b/lxd/api_internal.go
index 4d11c85d54..8e6cab8f35 100644
--- a/lxd/api_internal.go
+++ b/lxd/api_internal.go
@@ -788,13 +788,13 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 	}
 
 	// Check if a storage volume entry for the container already exists.
-	_, volume, ctVolErr := d.cluster.StoragePoolNodeVolumeGetType(
-		req.Name, storagePoolVolumeTypeContainer, poolID)
+	_, volume, ctVolErr := d.cluster.StoragePoolNodeVolumeGetTypeByProject(projectName, req.Name, storagePoolVolumeTypeContainer, poolID)
 	if ctVolErr != nil {
 		if ctVolErr != db.ErrNoSuchObject {
 			return response.SmartError(ctVolErr)
 		}
 	}
+
 	// If a storage volume entry exists only proceed if force was specified.
 	if ctVolErr == nil && !req.Force {
 		return response.BadRequest(fmt.Errorf(`Storage volume for instance %q already exists in the database. Set "force" to overwrite`, req.Name))
@@ -807,6 +807,7 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 			return response.SmartError(containerErr)
 		}
 	}
+
 	// If a db entry exists only proceed if force was specified.
 	if containerErr == nil && !req.Force {
 		return response.BadRequest(fmt.Errorf(`Entry for instance %q already exists in the database. Set "force" to overwrite`, req.Name))
@@ -825,18 +826,15 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 			return response.BadRequest(fmt.Errorf(`The type %q of the storage volume is not identical to the instance's type %q`, volume.Type, backup.Volume.Type))
 		}
 
-		// Remove the storage volume db entry for the container since
-		// force was specified.
-		err := d.cluster.StoragePoolVolumeDelete("default", req.Name,
-			storagePoolVolumeTypeContainer, poolID)
+		// Remove the storage volume db entry for the container since force was specified.
+		err := d.cluster.StoragePoolVolumeDelete(projectName, req.Name, storagePoolVolumeTypeContainer, poolID)
 		if err != nil {
 			return response.SmartError(err)
 		}
 	}
 
 	if containerErr == nil {
-		// Remove the storage volume db entry for the container since
-		// force was specified.
+		// Remove the storage volume db entry for the container since force was specified.
 		err := d.cluster.InstanceRemove(projectName, req.Name)
 		if err != nil {
 			return response.SmartError(err)
@@ -931,8 +929,7 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 		}
 
 		// Check if a storage volume entry for the snapshot already exists.
-		_, _, csVolErr := d.cluster.StoragePoolNodeVolumeGetTypeByProject(
-			projectName, snap.Name, storagePoolVolumeTypeContainer, poolID)
+		_, _, csVolErr := d.cluster.StoragePoolNodeVolumeGetTypeByProject(projectName, snap.Name, storagePoolVolumeTypeContainer, poolID)
 		if csVolErr != nil {
 			if csVolErr != db.ErrNoSuchObject {
 				return response.SmartError(csVolErr)
@@ -952,8 +949,7 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
 		}
 
 		if csVolErr == nil {
-			err := d.cluster.StoragePoolVolumeDelete(projectName, snap.Name,
-				storagePoolVolumeTypeContainer, poolID)
+			err := d.cluster.StoragePoolVolumeDelete(projectName, snap.Name, storagePoolVolumeTypeContainer, poolID)
 			if err != nil {
 				return response.SmartError(err)
 			}


More information about the lxc-devel mailing list