From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 136E31FF165
for <inbox@lore.proxmox.com>; Thu, 10 Apr 2025 10:29:15 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id C624A31895;
Thu, 10 Apr 2025 10:29:08 +0200 (CEST)
Message-ID: <6e6228f1-378b-4fd4-a806-561caad2464c@proxmox.com>
Date: Thu, 10 Apr 2025 10:29:05 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20250410071638.11497-1-f.ebner@proxmox.com>
<61890f96-5720-4122-be8c-683adebc9f2c@proxmox.com>
Content-Language: en-US
From: Fiona Ebner <f.ebner@proxmox.com>
In-Reply-To: <61890f96-5720-4122-be8c-683adebc9f2c@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.036 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
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH manager] ui: util: simplify
volume_is_qemu_backup again
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
Am 10.04.25 um 10:12 schrieb Thomas Lamprecht:
> Am 10.04.25 um 09:16 schrieb Fiona Ebner:
>> Commit f8087e0f ("ui: fix regression with checking if volume is QEMU
>> backup") opted for making the function support multiple types of
>> callers making the function more complex than it needs to be. Simply
>
> it's a simple if/else though, so complex is really relative.
Yes, I used a relative statement: "more than it needs to be".
>> adapt the rest of the call sites that the commit introducing the
>> regression missed, i.e. commit 3f8246030 ("ui: backup: also check for
>> backup subtype to classify archive").
>>
>> By always checking the subtype, this also makes the function work
>> correctly should there ever be another storage type supporting file
>> restore with different format names than PBS or volid patterns than
>> directory storages.
>
> FWIW I weighted both options when fixing this and used the central
> one by choice for more flexibility, passing opaque objects is a bit
> of a later move at best IMO move to a single string per clearly named
> parameter, or does yours have any advantage?
>
> Some actual type that can be checked at a compile time would be a
> benefit, but that's another language story.
Agreed and I get your point. It just felt more natural to go with the
object approach since all call sites have that very object and would
need to do the very same deconstruction if going with function(volume,
format, subtype). Independent of the chosen approach, keeping it
consistent with the volume_is_lxc_backup() helper is also an advantage.
In any case, nothing urgent in this follow-up and thank you for the
quick fix!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel