The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6897
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
- The check for ".importing" file wasn't project aware, could cause the container on storage device to be incorrectly removed.
- When using `--force` flag, the old DB volume records removal code was not project aware, preventing the container from being imported.
From 1d9e00c0bb7633d81752cec6e7d97335eaac3b6b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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)
}
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
