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
From 35398d973bb5e87d12a40fe46449a7da849c7f7d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
