[lxc-devel] [lxd/master] Storage: Improve failed snapshot create revert cleanup
tomponline on Github
lxc-bot at linuxcontainers.org
Fri Apr 3 10:55:51 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 564 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200403/341736f5/attachment.bin>
-------------- next part --------------
From 6d5f4e3cd324dd7500d1cce58f319496edbacdfc Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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)
More information about the lxc-devel
mailing list