[pve-devel] [PATCH pve-storage v3] fix #6561: zfspool: track refquota for subvolumes via user properties

[Shannon Sterz]

zfs itself does not track the refquota per snapshot so we need handle
this ourselves. otherwise rolling back a volume that has been resize
since the snapshot, will retain the new size. this is problematic, as
it means the value in the guest config does not longer match the size
of the disk on the storage.

this implementation tries to do so by leveraging a user property per
snapshot.

Reported-by: Lukas Wagner <l.wagner at proxmox.com>
Suggested-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
Signed-off-by: Shannon Sterz <s.sterz at proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner at proxmox.com>
---

Notes:
    keeping fiona's R-b from v2 as v3 basically just incorporates her
    suggestions (or suggestions she signed off on), but i'd drop the T-b
    as these patches now behave somewhat differently (warning on unknown
    values).

    hope that reasoning is sound!

    changes since v2, thanks @ Fiona Ebner:
    - use variable for the snapshot name instead of constructing it
      multiple times
    - use `zfs_get_properties` to extract properties instead of
      `zfs_request`
    - warn on unknown refquota user property values

    changes since v1:
    - remove useless if statement
    - reword commit message and comments to make them clearer
    - add missing Suggested-by and Reported-by trailers

 src/PVE/Storage/ZFSPoolPlugin.pm | 38 +++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/src/PVE/Storage/ZFSPoolPlugin.pm b/src/PVE/Storage/ZFSPoolPlugin.pm
index cdf5868..d8d8d0f 100644
--- a/src/PVE/Storage/ZFSPoolPlugin.pm
+++ b/src/PVE/Storage/ZFSPoolPlugin.pm
@@ -482,9 +482,25 @@ sub volume_size_info {
 sub volume_snapshot {
     my ($class, $scfg, $storeid, $volname, $snap) = @_;

-    my $vname = ($class->parse_volname($volname))[1];
+    my (undef, $vname, undef, undef, undef, undef, $format) = $class->parse_volname($volname);
+    my $snapshot_name = "$scfg->{pool}/$vname\@$snap";

-    $class->zfs_request($scfg, undef, 'snapshot', "$scfg->{pool}/$vname\@$snap");
+    $class->zfs_request($scfg, undef, 'snapshot', $snapshot_name);
+
+    # if this is a subvol, track refquota information via user properties. zfs
+    # does not track this property for snapshosts and consequently does not roll
+    # it back. so track this information manually.
+    if ($format eq 'subvol') {
+        my $refquota = $class->zfs_get_properties($scfg, 'refquota', "$scfg->{pool}/$vname");
+
+        $class->zfs_request(
+            $scfg,
+            undef,
+            'set',
+            "pve-storage:refquota=${refquota}",
+            $snapshot_name,
+        );
+    }
 }

 sub volume_snapshot_delete {
@@ -500,8 +516,24 @@ sub volume_snapshot_rollback {
     my ($class, $scfg, $storeid, $volname, $snap) = @_;

     my (undef, $vname, undef, undef, undef, undef, $format) = $class->parse_volname($volname);
+    my $snapshot_name = "$scfg->{pool}/$vname\@$snap";

-    my $msg = $class->zfs_request($scfg, undef, 'rollback', "$scfg->{pool}/$vname\@$snap");
+    my $msg = $class->zfs_request($scfg, undef, 'rollback', $snapshot_name);
+
+    # if this is a subvol, check if we tracked the refquota manually via user
+    # properties and if so, set it appropriatelly again.
+    if ($format eq 'subvol') {
+        my $refquota = $class->zfs_get_properties($scfg, 'pve-storage:refquota', $snapshot_name);
+
+        if ($refquota =~ m/^\d+$/) {
+            $class->zfs_request(
+                $scfg, undef, 'set', "refquota=${refquota}", "$scfg->{pool}/$vname",
+            );
+        } elsif ($refquota ne "-") {
+            # refquota user property was set, but not a number -> warn
+            warn "property for refquota tracking contained unknown value '$refquota'\n";
+        }
+    }

     # we have to unmount rollbacked subvols, to invalidate wrong kernel
     # caches, they get mounted in activate volume again
--
2.47.2