From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40])
	by lore.proxmox.com (Postfix) with ESMTPS id 4F44A1FF13E
	for <inbox@lore.proxmox.com>; Wed, 01 Jul 2026 14:22:28 +0200 (CEST)
Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1])
	by gate001.proxmox.com (Proxmox) with ESMTP id 8B34721386;
	Wed, 01 Jul 2026 14:22:27 +0200 (CEST)
Message-ID: <7f0d85c3-3ed2-4266-8410-c067367e359f@proxmox.com>
Date: Wed, 1 Jul 2026 14:22:23 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH container v9 05/10] allow pending mount point changes if
 storage is gone
To: =?UTF-8?Q?Michael_K=C3=B6ppl?= <m.koeppl@proxmox.com>,
 pve-devel@lists.proxmox.com
References: <20260629140439.184878-1-m.koeppl@proxmox.com>
 <20260629140439.184878-6-m.koeppl@proxmox.com>
 <576f756c-9c81-4058-9b41-ed81a1a66a07@proxmox.com>
 <DJN86S1O37OM.27Q9EN0ISR3KS@proxmox.com>
Content-Language: en-US
From: Fiona Ebner <f.ebner@proxmox.com>
In-Reply-To: <DJN86S1O37OM.27Q9EN0ISR3KS@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2
X-Bm-Transport-Timestamp: 1782908538890
X-SPAM-LEVEL: Spam detection results:  0
	DMARC_MISSING             0.1 Missing DMARC policy
	KAM_DMARC_STATUS         0.01 Test Rule for DKIM or SPF Failure with Strict
 Alignment (newer systems)
	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: NFQFGAK2N4PN4AUF24M5NIC5PXPKVAOU
X-Message-ID-Hash: NFQFGAK2N4PN4AUF24M5NIC5PXPKVAOU
X-MailFrom: f.ebner@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 <pve-devel.lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Owner: <mailto:pve-devel-owner@lists.proxmox.com>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Subscribe: <mailto:pve-devel-join@lists.proxmox.com>
List-Unsubscribe: <mailto:pve-devel-leave@lists.proxmox.com>

Am 01.07.26 um 2:15 PM schrieb Michael Köppl:
> On Tue Jun 30, 2026 at 3:41 PM CEST, Fiona Ebner wrote:
>> I'm thinking about something like
>>
>> with_checked_volid($opt, $conf->{opt}, sub {
>>     my ($volid) = @_;
>>     PVE::LXC::delete_mountpoint_volume($storecfg, $vmid, $volid)
>>         if !$class->is_volume_in_use($conf, $volid, 1, 1);
>> });
>>
>> where the helper does the parsing of the volume and volid, logs the
>> warning and returns if the storage does not exist and calls the
>> passed-in sub if it does exist.
> 
> Sounds good, I'll add a helper. Thanks for the suggestion. Though I'd go
> with something like:
> 
> ```
> sub with_checked_volid {
>     my ($storecfg, $volid, $callback) = @_;
> 
>     my ($storeid) = PVE::Storage::parse_volume_id($volid);
>     if (PVE::Storage::storage_config($storecfg, $storeid, 1)) {
>         callback->();
>     } else {
>         PVE::RESTEnvironment::log_warn(
>             "storage '$storeid' no longer exists, volume '$volid' will be removed");
>     }
> }
> ```
> 
> I'd keep the parsing of the volume outside the helper. The call sites
> don't use the result of parse_volume uniformly. It seems easier to me
> using the same helper across all call sites by passing in an already
> resolved volid as the common denominator. What do you think?

Sound good :)