[pve-devel] [PATCH qemu-server v6 3/3] mark volumes pending detach as unused if storage was removed

[Michael Köppl]

On 5/21/25 13:16, Fiona Ebner wrote:
> Am 20.05.25 um 11:08 schrieb Michael Köppl:
>> If the storage backing the volume was removed, the volume will be marked
>> as unused when pending actions are processed (e.g. volume was detached).
>> Previously, the volume would simply disappear from the config.
> 
> Why? If the storage doesn't exist, the volume doesn't exist (from PVE's
> perspective). What is the benefit of adding it as unused?
> 
> We could improve the current behavior by printing an informational
> message for volumes that are not added as unused and a warning message
> if the ownership couldn't be determined. I.e. warn if path() in
> vm_is_volid_owner() fails, rather than quietly ignoring a failure there.

Thanks for having a look at this and for your suggestion! The idea
behind this patch was to avoid the volume simply disappearing (from the
user's point of view). That way, the volume would still be there. In
theory, if the storage "re-appeared", it could even be mounted again as
it's not gone from the configuration. But it doesn't really make that
much sense, I agree.

I'll add the warnings you suggested for a v7.

> 
>> Co-authored-by: Stefan Hrdlicka
>> Signed-off-by: Michael Köppl <m.koeppl at proxmox.com>
>> ---
>>  PVE/QemuServer.pm | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index 68bbf4ce..d47fa51c 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -1838,7 +1838,11 @@ sub vmconfig_register_unused_drive {
>>  	delete $conf->{'special-sections'}->{cloudinit};
>>      } elsif (!drive_is_cdrom($drive)) {
>>  	my $volid = $drive->{file};
>> -	if (vm_is_volid_owner($storecfg, $vmid, $volid)) {
>> +	my ($storeid, undef) = PVE::Storage::parse_volume_id($volid);
>> +	if (
>> +	    !PVE::Storage::storage_config($storecfg, $storeid, 1)
>> +	    || vm_is_volid_owner($storecfg, $vmid, $volid)
>> +	) {
>>  	    PVE::QemuConfig->add_unused_volume($conf, $volid, $vmid);
>>  	}
>>      }
>