From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id CE1EC1FF2C8 for ; Wed, 17 Jul 2024 11:43:04 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D9251388D1; Wed, 17 Jul 2024 11:43:00 +0200 (CEST) From: Max Carrara To: pve-devel@lists.proxmox.com Date: Wed, 17 Jul 2024 11:40:20 +0200 Message-Id: <20240717094034.124857-23-m.carrara@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240717094034.124857-1-m.carrara@proxmox.com> References: <20240717094034.124857-1-m.carrara@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.220 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URI_NOVOWEL 0.5 URI hostname has long non-vowel sequence Subject: [pve-devel] [RFC pve-storage 22/36] plugin: btrfs: make helper methods private X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 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 --- 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