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 21F441FF146 for ; Tue, 28 Apr 2026 17:04:16 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0033F17C71; Tue, 28 Apr 2026 17:04:15 +0200 (CEST) Date: Tue, 28 Apr 2026 17:04:08 +0200 From: Wolfgang Bumiller To: "Max R. Carrara" Subject: Re: [PATCH pve-storage v1 08/54] plugin: break up needless if-elsif chain into separate if-blocks Message-ID: References: <20260422111322.257380-1-m.carrara@proxmox.com> <20260422111322.257380-9-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260422111322.257380-9-m.carrara@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1777388552540 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.088 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 Message-ID-Hash: HSENHHN7EIPXJLBEUISX45AT3ERYY22S X-Message-ID-Hash: HSENHHN7EIPXJLBEUISX45AT3ERYY22S X-MailFrom: w.bumiller@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 CC: pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Maybe not call it "needless" - this is a pure style change. While this is personal preference, I do agree that it would be nicer this way and have thought about doing this as well. I mostly didn't to avoid unnecessary merge conflicts, but I think this is the least likely conflict in this series anyway. Also, later patches in the series do the same in other places, so we'll at least be consistent... On Wed, Apr 22, 2026 at 01:12:34PM +0200, Max R. Carrara wrote: > Signed-off-by: Max R. Carrara > --- > src/PVE/Storage/Plugin.pm | 28 +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm > index 55268b29..2d76f452 100644 > --- a/src/PVE/Storage/Plugin.pm > +++ b/src/PVE/Storage/Plugin.pm > @@ -808,29 +808,43 @@ sub parse_volname { > my ($vmid, $name) = ($3, $4); > my (undef, $format, $isBase) = parse_name_dir($name); > return ('images', $name, $vmid, $basename, $basedvmid, $isBase, $format); > - } elsif ($volname =~ m!^(\d+)/(\S+)$!) { > + } > + > + if ($volname =~ m!^(\d+)/(\S+)$!) { > my ($vmid, $name) = ($1, $2); > my (undef, $format, $isBase) = parse_name_dir($name); > return ('images', $name, $vmid, undef, undef, $isBase, $format); > - } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!) { > + } > + > + if ($volname =~ m!^iso/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!) { > return ('iso', $1, undef, undef, undef, undef, 'raw'); > - } elsif ($volname =~ m!^vztmpl/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!) { > + } > + > + if ($volname =~ m!^vztmpl/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!) { > return ('vztmpl', $1, undef, undef, undef, undef, 'raw'); > - } elsif ($volname =~ m!^backup/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!) { > + } > + > + if ($volname =~ m!^backup/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!) { > my $fn = $1; > if ($fn =~ m/^vzdump-(openvz|lxc|qemu)-(\d+)-.+/) { > return ('backup', $fn, $2, undef, undef, undef, 'raw'); > } > return ('backup', $fn, undef, undef, undef, undef, 'raw'); > - } elsif ($volname =~ m!^snippets/([^/]+)$!) { > + } > + > + if ($volname =~ m!^snippets/([^/]+)$!) { > return ('snippets', $1, undef, undef, undef, undef, 'raw'); > - } elsif ($volname =~ > + } > + > + if ($volname =~ > m!^import/(${PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+\.ova\/${PVE::Storage::OVA_CONTENT_RE_1})$! > ) { > my $packed_image = $1; > my $format = $2; > return ('import', $packed_image, undef, undef, undef, undef, "ova+$format"); > - } elsif ($volname =~ > + } > + > + if ($volname =~ > m!^import/(${PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$! > ) { > return ('import', $1, undef, undef, undef, undef, $2); > -- > 2.47.3 > > > > > --