[pve-devel] [RFC pve-storage 22/36] plugin: btrfs: make helper methods private

[Max Carrara <m.carrara@proxmox.com>]

The methods `btrfs_cmd` and `btrfs_get_subvol_id` are made private in
order to prevent them from accidentally becoming part of the plugin's
public interface in the future.

The call sites are adapted accordingly, due to the methods not being
accessible by reference via `$class` anymore. Therefore, calls like

  $class->btrfs_cmd(['some', 'command']);

are changed to

  btrfs_cmd($class, ['some', 'command']);

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
 src/PVE/Storage/BTRFSPlugin.pm | 48 +++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm
index daff8a4..24694a6 100644
--- a/src/PVE/Storage/BTRFSPlugin.pm
+++ b/src/PVE/Storage/BTRFSPlugin.pm
@@ -234,7 +234,7 @@ sub filesystem_path {
     return wantarray ? ($path, $vmid, $vtype) : $path;
 }
 
-sub btrfs_cmd {
+my sub btrfs_cmd {
     my ($class, $cmd, $outfunc) = @_;
 
     my $msg = '';
@@ -252,9 +252,9 @@ sub btrfs_cmd {
     return $msg;
 }
 
-sub btrfs_get_subvol_id {
+my sub btrfs_get_subvol_id {
     my ($class, $path) = @_;
-    my $info = $class->btrfs_cmd(['subvolume', 'show', '--', $path]);
+    my $info = btrfs_cmd($class, ['subvolume', 'show', '--', $path]);
     if ($info !~ /^\s*(?:Object|Subvolume) ID:\s*(\d+)$/m) {
 	die "failed to get btrfs subvolume ID from: $info\n";
     }
@@ -299,7 +299,7 @@ sub create_base {
 
     rename($subvol, $newsubvol)
 	|| die "rename '$subvol' to '$newsubvol' failed - $!\n";
-    eval { $class->btrfs_cmd(['property', 'set', $newsubvol, 'ro', 'true']) };
+    eval { btrfs_cmd($class, ['property', 'set', $newsubvol, 'ro', 'true']) };
     warn $@ if $@;
 
     return $newvolname;
@@ -336,7 +336,7 @@ sub clone_image {
 	$newsubvol = raw_file_to_subvol($newsubvol);
     }
 
-    $class->btrfs_cmd(['subvolume', 'snapshot', '--', $subvol, $newsubvol]);
+    btrfs_cmd($class, ['subvolume', 'snapshot', '--', $subvol, $newsubvol]);
 
     return $newvolname;
 }
@@ -380,7 +380,7 @@ sub alloc_image {
 	die "btrfs quotas are currently not supported, use an unsized subvolume or a raw file\n";
     }
 
-    $class->btrfs_cmd(['subvolume', 'create', '--', $subvol]);
+    btrfs_cmd($class, ['subvolume', 'create', '--', $subvol]);
 
     eval {
 	if ($fmt eq 'subvol') {
@@ -391,9 +391,9 @@ sub alloc_image {
 	    # eval {
 	    #     # This call should happen at storage creation instead and therefore governed by a
 	    #     # configuration option!
-	    #     # $class->btrfs_cmd(['quota', 'enable', $subvol]);
-	    #     my $id = $class->btrfs_get_subvol_id($subvol);
-	    #     $class->btrfs_cmd(['qgroup', 'limit', "${size}k", "0/$id", $subvol]);
+	    #     # btrfs_cmd($class, ['quota', 'enable', $subvol]);
+	    #     my $id = btrfs_get_subvol_id($class, $subvol);
+	    #     btrfs_cmd($class, ['qgroup', 'limit', "${size}k", "0/$id", $subvol]);
 	    # };
 	} elsif ($fmt eq 'raw') {
 	    sysopen my $fh, $path, O_WRONLY | O_CREAT | O_EXCL
@@ -408,7 +408,7 @@ sub alloc_image {
     };
 
     if (my $err = $@) {
-	eval { $class->btrfs_cmd(['subvolume', 'delete', '--', $subvol]); };
+	eval { btrfs_cmd($class, ['subvolume', 'delete', '--', $subvol]); };
 	warn $@ if $@;
 	die $err;
     }
@@ -466,7 +466,7 @@ sub free_image {
 	push @snapshot_vols, "$dir/$volume";
     });
 
-    $class->btrfs_cmd(['subvolume', 'delete', '--', @snapshot_vols, $subvol]);
+    btrfs_cmd($class, ['subvolume', 'delete', '--', @snapshot_vols, $subvol]);
     # try to cleanup directory to not clutter storage with empty $vmid dirs if
     # all images from a guest got deleted
     rmdir($dir);
@@ -477,10 +477,10 @@ sub free_image {
 # Currently not used because quotas clash with send/recv.
 # my sub btrfs_subvol_quota {
 #     my ($class, $path) = @_;
-#     my $id = '0/' . $class->btrfs_get_subvol_id($path);
+#     my $id = '0/' . btrfs_get_subvol_id($class, $path);
 #     my $search = qr/^\Q$id\E\s+(\d)+\s+\d+\s+(\d+)\s*$/;
 #     my ($used, $size);
-#     $class->btrfs_cmd(['qgroup', 'show', '--raw', '-rf', '--', $path], sub {
+#     btrfs_cmd($class, ['qgroup', 'show', '--raw', '-rf', '--', $path], sub {
 # 	return if defined($size);
 # 	if ($_[0] =~ $search) {
 # 	    ($used, $size) = ($1, $2);
@@ -519,8 +519,8 @@ sub volume_resize {
     my $format = ($class->parse_volname($volname))[6];
     if ($format eq 'subvol') {
 	my $path = $class->filesystem_path($scfg, $volname);
-	my $id = '0/' . $class->btrfs_get_subvol_id($path);
-	$class->btrfs_cmd(['qgroup', 'limit', '--', "${size}k", "0/$id", $path]);
+	my $id = '0/' . btrfs_get_subvol_id($class, $path);
+	btrfs_cmd($class, ['qgroup', 'limit', '--', "${size}k", "0/$id", $path]);
 	return undef;
     }
 
@@ -546,7 +546,7 @@ sub volume_snapshot {
     my $snapshot_dir = $class->get_subdir($scfg, 'images') . "/$vmid";
     mkpath $snapshot_dir;
 
-    $class->btrfs_cmd(['subvolume', 'snapshot', '-r', '--', $path, $snap_path]);
+    btrfs_cmd($class, ['subvolume', 'snapshot', '-r', '--', $path, $snap_path]);
     return undef;
 }
 
@@ -580,11 +580,11 @@ sub volume_snapshot_rollback {
     # But for atomicity in case the rename after create-failure *also* fails, we create the new
     # subvol first, then use RENAME_EXCHANGE, 
     my $tmp_path = "$path.tmp.$$";
-    $class->btrfs_cmd(['subvolume', 'snapshot', '--', $snap_path, $tmp_path]);
+    btrfs_cmd($class, ['subvolume', 'snapshot', '--', $snap_path, $tmp_path]);
     # The paths are absolute, so pass -1 as file descriptors.
     my $ok = PVE::Tools::renameat2(-1, $tmp_path, -1, $path, &PVE::Tools::RENAME_EXCHANGE);
 
-    eval { $class->btrfs_cmd(['subvolume', 'delete', '--', $tmp_path]) };
+    eval { btrfs_cmd($class, ['subvolume', 'delete', '--', $tmp_path]) };
     warn "failed to remove '$tmp_path' subvolume: $@" if $@;
 
     if (!$ok) {
@@ -609,7 +609,7 @@ sub volume_snapshot_delete {
 	$path = raw_file_to_subvol($path);
     }
 
-    $class->btrfs_cmd(['subvolume', 'delete', '--', $path]);
+    btrfs_cmd($class, ['subvolume', 'delete', '--', $path]);
 
     return undef;
 }
@@ -888,7 +888,7 @@ sub volume_import {
 	# Rotate the disk into place, first the current state:
 	# Note that read-only subvolumes cannot be moved into different directories, but for the
 	# "current" state we also want a writable copy, so start with that:
-	$class->btrfs_cmd(['property', 'set', "$tmppath/$diskname\@$snapshot", 'ro', 'false']);
+	btrfs_cmd($class, ['property', 'set', "$tmppath/$diskname\@$snapshot", 'ro', 'false']);
 	PVE::Tools::renameat2(
 	    -1,
 	    "$tmppath/$diskname\@$snapshot",
@@ -899,7 +899,7 @@ sub volume_import {
 	    . " into place at '$destination' - $!\n";
 
 	# Now recreate the actual snapshot:
-	$class->btrfs_cmd([
+	btrfs_cmd($class, [
 	    'subvolume',
 	    'snapshot',
 	    '-r',
@@ -910,7 +910,7 @@ sub volume_import {
 
 	# Now go through the remaining snapshots (if any)
 	foreach my $snap (@snapshots) {
-	    $class->btrfs_cmd(['property', 'set', "$tmppath/$diskname\@$snap", 'ro', 'false']);
+	    btrfs_cmd($class, ['property', 'set', "$tmppath/$diskname\@$snap", 'ro', 'false']);
 	    PVE::Tools::renameat2(
 		-1,
 		"$tmppath/$diskname\@$snap",
@@ -919,7 +919,7 @@ sub volume_import {
 		&PVE::Tools::RENAME_NOREPLACE,
 	    ) or die "failed to move received snapshot '$tmppath/$diskname\@$snap'"
 		. " into place at '$destination\@$snap' - $!\n";
-	    eval { $class->btrfs_cmd(['property', 'set', "$destination\@$snap", 'ro', 'true']) };
+	    eval { btrfs_cmd($class, ['property', 'set', "$destination\@$snap", 'ro', 'true']) };
 	    warn "failed to make $destination\@$snap read-only - $!\n" if $@;
 	}
     };
@@ -932,7 +932,7 @@ sub volume_import {
 	    $dh->rewind;
 	    while (defined(my $entry = $dh->read)) {
 		next if $entry eq '.' || $entry eq '..';
-		eval { $class->btrfs_cmd(['subvolume', 'delete', '--', "$tmppath/$entry"]) };
+		eval { btrfs_cmd($class, ['subvolume', 'delete', '--', "$tmppath/$entry"]) };
 		warn $@ if $@;
 	    }
 	    $dh->close; undef $dh;
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel