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 EF4311FF2C8 for ; Wed, 17 Jul 2024 11:40:49 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 16DB8380E8; Wed, 17 Jul 2024 11:41:19 +0200 (CEST) From: Max Carrara To: pve-devel@lists.proxmox.com Date: Wed, 17 Jul 2024 11:40:06 +0200 Message-Id: <20240717094034.124857-9-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.030 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 Subject: [pve-devel] [RFC pve-storage 08/36] plugin: dir: move helper subs of directory plugin to common modules 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" Because the directory plugin's subroutines are frequently used by other plugins, it's best to factor these helpers into the common module. This is a first step in making other plugins not depend on the directory plugin anymore. Simultaneously this commit also introduces the `Common::Path` module, which ought to provide shared subroutines for operations on paths. The changes of this commit are backwards-compatible; the old subs act as mere wrappers and will emit a warning when used. Signed-off-by: Max Carrara --- src/PVE/Storage/Common.pm | 31 ++++++++++++++++++++ src/PVE/Storage/Common/Makefile | 1 + src/PVE/Storage/Common/Path.pm | 51 +++++++++++++++++++++++++++++++++ src/PVE/Storage/DirPlugin.pm | 38 ++++++++++++------------ 4 files changed, 101 insertions(+), 20 deletions(-) create mode 100644 src/PVE/Storage/Common/Path.pm diff --git a/src/PVE/Storage/Common.pm b/src/PVE/Storage/Common.pm index f828c8c..a2ae979 100644 --- a/src/PVE/Storage/Common.pm +++ b/src/PVE/Storage/Common.pm @@ -3,10 +3,13 @@ package PVE::Storage::Common; use strict; use warnings; +use PVE::JSONSchema; + use parent qw(Exporter); our @EXPORT_OK = qw( get_deprecation_warning + storage_parse_is_mountpoint ); =pod @@ -29,10 +32,19 @@ be grouped in a submodule can also be found here. =over +=item C + +Utilities concerned with working with paths. + =back =head1 FUNCTIONS +B Functions prefixed with C are related to the C +API and usually expect an C<$scfg> ("storage config") hash. This hash is +expected to contain the configuration for I storage, which is (usually) +acquired via C. + =cut =pod @@ -56,4 +68,23 @@ sub get_deprecation_warning : prototype($) { . "the future. Please use '$new_sub_name' instead."; } +=head3 storage_parse_is_mountpoint + + $path = storage_parse_is_mountpoint($scfg) + +Helper that tries to return a path if the given I hash C<$scfg> +contains an C property. Returns C if it can't. + +=cut + +sub storage_parse_is_mountpoint : prototype($) { + my ($scfg) = @_; + my $is_mp = $scfg->{is_mountpoint}; + return undef if !defined $is_mp; + if (defined(my $bool = PVE::JSONSchema::parse_boolean($is_mp))) { + return $bool ? $scfg->{path} : undef; + } + return $is_mp; # contains a path +} + 1; diff --git a/src/PVE/Storage/Common/Makefile b/src/PVE/Storage/Common/Makefile index 0c4bba5..9455b81 100644 --- a/src/PVE/Storage/Common/Makefile +++ b/src/PVE/Storage/Common/Makefile @@ -1,4 +1,5 @@ SOURCES = \ + Path.pm \ .PHONY: install diff --git a/src/PVE/Storage/Common/Path.pm b/src/PVE/Storage/Common/Path.pm new file mode 100644 index 0000000..7535dda --- /dev/null +++ b/src/PVE/Storage/Common/Path.pm @@ -0,0 +1,51 @@ +package PVE::Storage::Common::Path; + +use strict; +use warnings; + +use Cwd; + +use PVE::ProcFSTools; + +use parent qw(Exporter); + +our @EXPORT_OK = qw( + path_is_mounted +); + +=pod + +=head1 NAME + +PVE::Storage::Common::Path - Shared functions and utilities for manipulating paths + +=head1 FUNCTIONS + +=cut + +=pod + +=head3 path_is_mounted + + $is_mounted = path_is_mounted($mountpoint) + $is_mounted = path_is_mounted($mountpoint, $mountdata) + +Checks if the given path in C<$mountpoint> is actually mounted. Optionally takes +a C<$mountdata> hash returned by C in +order to avoid repeatedly reading and parsing C. + +=cut + +# NOTE: should ProcFSTools::is_mounted accept an optional cache like this? +sub path_is_mounted { + my ($mountpoint, $mountdata) = @_; + + $mountpoint = Cwd::realpath($mountpoint); # symlinks + return 0 if !defined($mountpoint); # path does not exist + + $mountdata = PVE::ProcFSTools::parse_proc_mounts() if !$mountdata; + return 1 if grep { $_->[1] eq $mountpoint } @$mountdata; + return undef; +} + +1; diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm index 2efa8d5..ac0d365 100644 --- a/src/PVE/Storage/DirPlugin.pm +++ b/src/PVE/Storage/DirPlugin.pm @@ -3,12 +3,16 @@ package PVE::Storage::DirPlugin; use strict; use warnings; -use Cwd; use Encode qw(decode encode); use File::Path; use IO::File; use POSIX; +use PVE::Storage::Common qw( + get_deprecation_warning + storage_parse_is_mountpoint +); +use PVE::Storage::Common::Path; use PVE::Storage::Plugin; use PVE::JSONSchema qw(get_standard_option); @@ -86,26 +90,20 @@ sub options { # Storage implementation # -# NOTE: should ProcFSTools::is_mounted accept an optional cache like this? sub path_is_mounted { - my ($mountpoint, $mountdata) = @_; + warn get_deprecation_warning( + "PVE::Storage::Common::Path::path_is_mounted" + ); - $mountpoint = Cwd::realpath($mountpoint); # symlinks - return 0 if !defined($mountpoint); # path does not exist - - $mountdata = PVE::ProcFSTools::parse_proc_mounts() if !$mountdata; - return 1 if grep { $_->[1] eq $mountpoint } @$mountdata; - return undef; + return PVE::Storage::Common::Path::path_is_mounted(@_); } sub parse_is_mountpoint { - my ($scfg) = @_; - my $is_mp = $scfg->{is_mountpoint}; - return undef if !defined $is_mp; - if (defined(my $bool = PVE::JSONSchema::parse_boolean($is_mp))) { - return $bool ? $scfg->{path} : undef; - } - return $is_mp; # contains a path + warn get_deprecation_warning( + "PVE::Storage::Common::storage_parse_is_mountpoint" + ); + + return storage_parse_is_mountpoint(@_); } # FIXME move into 'get_volume_attribute' when removing 'get_volume_notes' @@ -211,11 +209,11 @@ sub update_volume_attribute { sub status { my ($class, $storeid, $scfg, $cache) = @_; - if (defined(my $mp = parse_is_mountpoint($scfg))) { + if (defined(my $mp = PVE::Storage::Common::storage_parse_is_mountpoint($scfg))) { $cache->{mountdata} = PVE::ProcFSTools::parse_proc_mounts() if !$cache->{mountdata}; - return undef if !path_is_mounted($mp, $cache->{mountdata}); + return undef if !PVE::Storage::Common::Path::path_is_mounted($mp, $cache->{mountdata}); } return $class->SUPER::status($storeid, $scfg, $cache); @@ -227,8 +225,8 @@ sub activate_storage { my $path = $scfg->{path}; - my $mp = parse_is_mountpoint($scfg); - if (defined($mp) && !path_is_mounted($mp, $cache->{mountdata})) { + my $mp = storage_parse_is_mountpoint($scfg); + if (defined($mp) && !PVE::Storage::Common::Path::path_is_mounted($mp, $cache->{mountdata})) { die "unable to activate storage '$storeid' - " . "directory is expected to be a mount point but is not mounted: '$mp'\n"; } -- 2.39.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel