From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id ADDB01FF13F for ; Thu, 23 Apr 2026 11:46:30 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 82CB0104BE; Thu, 23 Apr 2026 11:46:30 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH pve-storage v2 3/3] plugin: improve error handling when creating a content subdir fails Date: Thu, 23 Apr 2026 11:46:16 +0200 Message-ID: <20260423094619.205988-4-m.carrara@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260423094619.205988-1-m.carrara@proxmox.com> References: <20260423094619.205988-1-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776937496908 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.031 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 PROLO_LEO2 0.1 Meta Catches all Leo drug variations so far SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: GUFAKTRT4ME5FU3NTWFHKR3SXMZXFUE3 X-Message-ID-Hash: GUFAKTRT4ME5FU3NTWFHKR3SXMZXFUE3 X-MailFrom: m.carrara@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Replace the call to `mkpath()` with a helper inline subroutine that uses `make_path()` instead. Follow the docs of `File::Path` [1] in order to make our error handling here more finely-grained. Note that the helper sub is mainly there to make the body of the loop where `mkpath()` was originally called less convoluted / complex. Add some additional context to the error message, namely that creating the content directory for a given vtype failed. Also extract and attach `make_path()`'s error message. Handle "Permission denied" and "Read-only file system" errors separately and, in addition to the context, tell the user to ensure that the storage has read and write access when such errors occur. Initially spotted this in the community forum [2]. [1]: https://perldoc.perl.org/File::Path#ERROR-HANDLING [2]: https://forum.proxmox.com/threads/bug-creating-nfs-volume-storage.177354/ Signed-off-by: Max R. Carrara --- Notes: NOTE: I ran into the second branch (the one for "Read-only file system") when testing things using my local NFS share—I figured I might as well handle that one too, since it's similar in nature. src/PVE/Storage/Plugin.pm | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index 04d87842..294a59e8 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -1903,6 +1903,33 @@ sub activate_storage { return; } + my $try_create_subdir = sub { + my ($vtype) = @_; + + my $subdir = $class->get_subdir($scfg, $vtype); + + File::Path::make_path($subdir, { error => \my $errors }); + if (defined($errors) && scalar($errors->@*)) { + my $msg = "failed to create content directory '$subdir' for content '$vtype'"; + + for my $err_hash ($errors->@*) { + my ($file, $err_msg) = $err_hash->%*; + + if ( + $err_msg =~ m/permission \s denied/ix + || $err_msg =~ m/read -? only \s file \s? system/ix + ) { + my $msg_prompt = + "ensure that you have read and write access to your storage!"; + + die "$msg - $err_msg - $msg_prompt\n"; + } + + die "$msg - $err_msg\n"; + } + } + }; + # TODO: mkdir is basically deprecated since 8.0, but we don't warn here until 8.4 or 9.0, as we # only got the replacement in 8.0, so no real replacement window, and its really noisy. @@ -1918,8 +1945,7 @@ sub activate_storage { defined($scfg->{content}->{$vtype}) || ($vtype eq 'backup' && defined($scfg->{content}->{'rootdir'})) ) { - my $subdir = $class->get_subdir($scfg, $vtype); - mkpath $subdir; + $try_create_subdir->($vtype); } } } -- 2.47.3