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 AC0941FF13E
	for <inbox@lore.proxmox.com>; Wed, 01 Jul 2026 12:02:08 +0200 (CEST)
Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1])
	by gate001.proxmox.com (Proxmox) with ESMTP id 9028D2143C;
	Wed, 01 Jul 2026 12:02:07 +0200 (CEST)
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Wed, 01 Jul 2026 12:01:32 +0200
Message-Id: <DJN5C38XF1BM.2UCDVGQAHD26I@proxmox.com>
Subject: Re: [PATCH storage v9 01/10] fix #3711: vdisk_free: print warning
 if storage does not exist
From: =?utf-8?q?Michael_K=C3=B6ppl?= <m.koeppl@proxmox.com>
To: "Fiona Ebner" <f.ebner@proxmox.com>, =?utf-8?q?Michael_K=C3=B6ppl?=
 <m.koeppl@proxmox.com>, <pve-devel@lists.proxmox.com>
X-Mailer: aerc 0.21.0
References: <20260629140439.184878-1-m.koeppl@proxmox.com>
 <20260629140439.184878-2-m.koeppl@proxmox.com>
 <885d0e46-94e6-4328-a913-1353ae0a88c6@proxmox.com>
In-Reply-To: <885d0e46-94e6-4328-a913-1353ae0a88c6@proxmox.com>
X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2
X-Bm-Transport-Timestamp: 1782900078909
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: 4WHNF5CXCMI2ZV2E2VBVXRC6F7SEPIMO
X-Message-ID-Hash: 4WHNF5CXCMI2ZV2E2VBVXRC6F7SEPIMO
X-MailFrom: m.koeppl@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>

On Tue Jun 30, 2026 at 4:21 PM CEST, Fiona Ebner wrote:

[snip]

> =20
>>      my ($storeid, $volname) =3D parse_volume_id($volid);
>> -    my $scfg =3D storage_config($cfg, $storeid);
>> +    my $scfg =3D storage_config($cfg, $storeid, 1);
>> +    if (!$scfg) {
>> +        log_warn("storage '$storeid' does not exist, volume '$volid' co=
uld not be freed");
>> +        return;
>> +    }
>> +
>>      my $plugin =3D PVE::Storage::Plugin->lookup($scfg->{type});
>> =20
>>      activate_storage($cfg, $storeid);
>
> Not sure about this one. Note that vdisk_free() is also used for the
> DELETE API, so the volume ID can also be user input and not something
> generated by our stack. While the API call still returns an error code
> for an invalid volume ID, that's more by chance, because of an earlier
> call to path(). Failing does seem more natural and callers can decide
> what they want to do. This change is not required for the rest of the
> series to work or is it?

Thanks for having a look at this series!

I wanted to double-check this before replying, but yes, you're right,
this was an oversight on my side. I'll drop the patch for the v10 of
this series. The call sites used for removal of guests/disks should all
be covered, so removal still works for all relevant cases even if the
underlying storage is gone.