The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/7132
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) ===
Addresses failed snapshot creation leaving database records as discussed in https://discuss.linuxcontainers.org/t/not-able-create-snapshot-through-zfs-snapshot-storage-name-containers-containers-name-snapshot-snapshot-name-due-to-invalid-argument-in-its-name/7243
From 6d5f4e3cd324dd7500d1cce58f319496edbacdfc Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 3 Apr 2020 11:44:54 +0100
Subject: [PATCH 1/3] lxd/storage/backend/lxd: Checks for existance of volume
before deleting
Allows revert process to remove partially created volumes from database.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/backend_lxd.go | 46 +++++++++++++++++++++++---------------
1 file changed, 28 insertions(+), 18 deletions(-)
diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 9f83135f81..cbb8728da7 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -1245,9 +1245,12 @@ func (b *lxdBackend) DeleteInstance(inst
instance.Instance, op *operations.Opera
// Delete the volume from the storage device. Must come after snapshots
are removed.
// Must come before DB StoragePoolVolumeDelete so that the volume ID is
still available.
logger.Debug("Deleting instance volume", log.Ctx{"volName":
volStorageName})
- err = b.driver.DeleteVolume(vol, op)
- if err != nil {
- return errors.Wrapf(err, "Error deleting storage volume")
+
+ if b.driver.HasVolume(vol) {
+ err = b.driver.DeleteVolume(vol, op)
+ if err != nil {
+ return errors.Wrapf(err, "Error deleting storage
volume")
+ }
}
// Remove symlinks.
@@ -1746,13 +1749,14 @@ func (b *lxdBackend) DeleteInstanceSnapshot(inst
instance.Instance, op *operatio
snapVolName := drivers.GetSnapshotVolumeName(parentStorageName,
snapName)
- // There's no need to pass config as it's not needed when deleting a
volume
- // snapshot.
+ // There's no need to pass config as it's not needed when deleting a
volume snapshot.
vol := b.newVolume(volType, contentType, snapVolName, nil)
- err = b.driver.DeleteVolumeSnapshot(vol, op)
- if err != nil {
- return err
+ if b.driver.HasVolume(vol) {
+ err = b.driver.DeleteVolumeSnapshot(vol, op)
+ if err != nil {
+ return err
+ }
}
// Delete symlink if needed.
@@ -2039,9 +2043,11 @@ func (b *lxdBackend) DeleteImage(fingerprint string, op
*operations.Operation) e
vol := b.newVolume(drivers.VolumeTypeImage, contentType, fingerprint,
storageVol.Config)
- err = b.driver.DeleteVolume(vol, op)
- if err != nil {
- return err
+ if b.driver.HasVolume(vol) {
+ err = b.driver.DeleteVolume(vol, op)
+ if err != nil {
+ return err
+ }
}
err = b.state.Cluster.StoragePoolVolumeDelete(project.Default,
fingerprint, db.StoragePoolVolumeTypeImage, b.ID())
@@ -2639,9 +2645,11 @@ func (b *lxdBackend) DeleteCustomVolume(projectName
string, volName string, op *
vol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS,
volStorageName, nil)
// Delete the volume from the storage device. Must come after snapshots
are removed.
- err = b.driver.DeleteVolume(vol, op)
- if err != nil {
- return err
+ if b.driver.HasVolume(vol) {
+ err = b.driver.DeleteVolume(vol, op)
+ if err != nil {
+ return err
+ }
}
// Finally, remove the volume record from the database.
@@ -2824,13 +2832,15 @@ func (b *lxdBackend)
DeleteCustomVolumeSnapshot(projectName, volName string, op
// Delete the snapshot from the storage device.
// Must come before DB StoragePoolVolumeDelete so that the volume ID is
still available.
- err := b.driver.DeleteVolumeSnapshot(vol, op)
- if err != nil {
- return err
+ if b.driver.HasVolume(vol) {
+ err := b.driver.DeleteVolumeSnapshot(vol, op)
+ if err != nil {
+ return err
+ }
}
// Remove the snapshot volume record from the database.
- err = b.state.Cluster.StoragePoolVolumeDelete(projectName, volName,
db.StoragePoolVolumeTypeCustom, b.ID())
+ err := b.state.Cluster.StoragePoolVolumeDelete(projectName, volName,
db.StoragePoolVolumeTypeCustom, b.ID())
if err != nil {
return err
}
From 04545c020887686424a76369fa74246230a1c071 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 3 Apr 2020 11:54:27 +0100
Subject: [PATCH 2/3] lxd/instance: Switches to revert package for
instanceCreateAsSnapshot
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/instance.go | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/lxd/instance.go b/lxd/instance.go
index e66561789c..456655d947 100644
--- a/lxd/instance.go
+++ b/lxd/instance.go
@@ -22,6 +22,7 @@ import (
"github.com/lxc/lxd/lxd/instance/instancetype"
"github.com/lxc/lxd/lxd/operations"
"github.com/lxc/lxd/lxd/project"
+ "github.com/lxc/lxd/lxd/revert"
"github.com/lxc/lxd/lxd/state"
storagePools "github.com/lxc/lxd/lxd/storage"
"github.com/lxc/lxd/lxd/task"
@@ -381,20 +382,15 @@ func instanceCreateAsSnapshot(s *state.State, args
db.InstanceArgs, sourceInstan
}
}
+ revert := revert.New()
+ defer revert.Fail()
+
// Create the snapshot.
inst, err := instanceCreateInternal(s, args)
if err != nil {
return nil, err
}
-
- revert := true
- defer func() {
- if !revert {
- return
- }
-
- inst.Delete()
- }()
+ revert.Add(func() { inst.Delete() })
pool, err := storagePools.GetPoolByInstance(s, inst)
if err != nil {
@@ -432,7 +428,7 @@ func instanceCreateAsSnapshot(s *state.State, args
db.InstanceArgs, sourceInstan
"snapshot_name": args.Name,
})
- revert = false
+ revert.Success()
return inst, nil
}
From 0e74d03ab647fba72750c60fa9f3b244e6a9c875 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 3 Apr 2020 11:54:37 +0100
Subject: [PATCH 3/3] lxd/storage/backend/lxd: Comment tweak
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/backend_lxd.go | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index cbb8728da7..4621290a7f 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -1627,8 +1627,7 @@ func (b *lxdBackend) CreateInstanceSnapshot(inst
instance.Instance, src instance
volStorageName := project.Instance(inst.Project(), inst.Name())
// Get the volume.
- // There's no need to pass config as it's not needed when creating
volume
- // snapshots.
+ // There's no need to pass config as it's not needed when creating
volume snapshots.
vol := b.newVolume(volType, contentType, volStorageName, nil)
err = b.driver.CreateVolumeSnapshot(vol, op)
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
