[prev in list] [next in list] [prev in thread] [next in thread]
List: lxc-devel
Subject: [lxc-devel] [lxd/master] Storage: Fix snapshot remove subsequent
From: tomponline on Github <lxc-bot () linuxcontainers ! org>
Date: 2020-12-18 12:19:55
Message-ID: 5fdc9e6b.1c69fb81.be59c.a559SMTPIN_ADDED_MISSING () mx ! google ! com
[Download RAW message or body]
[Attachment #2 (text/x-mailbox)]
The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/8274
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) ===
When setting `volume.zfs.remove_snapshots=true` on a ZFS storage pool, this fixes \
several issues:
- The wrong snapshot was being checked for deletion suitability (resulting in not \
deleting any snapshots).
- Once that was fixed, there was also an issue with only the storage volume and \
storage volume DB record of the snapshot being deleted, not the instance snapshot \
record as well. Leaving orphaned snapshots in `lxc info <instance>` output and \
preventing deletion of instance (because snapshot volume DB record \
had been removed).
- Because of the scope of the `err` being returned, it was likely that as a new `err` \
was created inside the subsequent snapshot deletion block, that the original error \
would be returned even on successful restore. Added `return nil` after successful \
restore.
- Modified `DeleteInstanceSnapshot` to not fail if the storage volume DB record has \
already been removed (as that is the desired result anyway).
Fixes https://discuss.linuxcontainers.org/t/snapshot-c1-20201218-03-cannot-be-restored-due-to-subsequent-snapshot-s-set-zfs-remove-snapshots-to-override/9742
[Attachment #3 (text/plain)]
From 35398d973bb5e87d12a40fe46449a7da849c7f7d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott@canonical.com>
Date: Fri, 18 Dec 2020 12:13:10 +0000
Subject: [PATCH 1/3] lxd/storage/drivers/driver/zfs/volumes: Error quoting in
RestoreVolume
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
---
lxd/storage/drivers/driver_zfs_volumes.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/storage/drivers/driver_zfs_volumes.go \
b/lxd/storage/drivers/driver_zfs_volumes.go index 29e38998d8..cec417f814 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -1788,14 +1788,14 @@ func (d *zfs) RestoreVolume(vol Volume, snapshotName string, \
op *operations.Oper
if strings.HasPrefix(entry, "@") {
// Located an internal snapshot.
- return fmt.Errorf("Snapshot '%s' cannot be restored due to subsequent internal \
snapshot(s) (from a copy)", snapshotName) + return fmt.Errorf("Snapshot %q cannot \
be restored due to subsequent internal snapshot(s) (from a copy)", snapshotName) }
}
// Check if snapshot removal is allowed.
if len(snapshots) > 0 {
if !shared.IsTrue(vol.ExpandedConfig("zfs.remove_snapshots")) {
- return fmt.Errorf("Snapshot '%s' cannot be restored due to subsequent \
snapshot(s). Set zfs.remove_snapshots to override", snapshotName) + return \
fmt.Errorf("Snapshot %q cannot be restored due to subsequent snapshot(s). Set \
zfs.remove_snapshots to override", snapshotName) }
// Setup custom error to tell the backend what to delete.
From 4efdfbc4fc20fdca860f99e9ac55d0948c3bd8ca Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott@canonical.com>
Date: Fri, 18 Dec 2020 12:13:35 +0000
Subject: [PATCH 2/3] lxd/storage/backend/lxd: Don't fail in
DeleteInstanceSnapshot if volume DB record already deleted
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
---
lxd/storage/backend_lxd.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 2ca935dc0c..8d3358b2ca 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -1996,9 +1996,9 @@ func (b *lxdBackend) DeleteInstanceSnapshot(inst \
instance.Instance, op *operatio return err
}
- // Remove the snapshot volume record from the database.
+ // Remove the snapshot volume record from the database if exists.
err = b.state.Cluster.RemoveStoragePoolVolume(inst.Project(), \
drivers.GetSnapshotVolumeName(parentName, snapName), volDBType, \
b.ID())
- if err != nil {
+ if err != nil && err != db.ErrNoSuchObject {
return err
}
From 841fcd1491216e470944a6c087a2e6fb61988e30 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott@canonical.com>
Date: Fri, 18 Dec 2020 12:14:16 +0000
Subject: [PATCH 3/3] lxd/storage/backend/lxd: Fix deleting subsequent
snapshots for ZFS in RestoreInstanceSnapshot
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
---
lxd/storage/backend_lxd.go | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 8d3358b2ca..09ff3c95c5 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -2064,23 +2064,25 @@ func (b *lxdBackend) RestoreInstanceSnapshot(inst \
instance.Instance, src instanc
// Go through all the snapshots.
for _, snap := range snaps {
- _, snapName, _ := shared.InstanceGetParentAndSnapshotName(src.Name())
+ _, snapName, _ := shared.InstanceGetParentAndSnapshotName(snap.Name())
if !shared.StringInSlice(snapName, snapErr.Snapshots) {
continue
}
- // Delete if listed.
- err := b.DeleteInstanceSnapshot(snap, op)
+ // Delete snapshot instance if listed in the error as one that needs removing.
+ err := snap.Delete(true)
if err != nil {
return err
}
}
- // Now try again.
+ // Now try restoring again.
err = b.driver.RestoreVolume(vol, snapshotName, op)
if err != nil {
return err
}
+
+ return nil
}
return err
[Attachment #4 (text/plain)]
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic