Re: [PATCH qemu-server v2 2/3] add new classify_drive_file utility

["Thomas Ellmenreich" <t.ellmenreich@proxmox.com>]

On Fri Aug 7, 2026 at 11:30 AM CEST, Max R. Carrara wrote:

[snip]

>> +# Tries to classify the drive file as either 'none', 'cdrom', 'absolute'
>> +# or 'volume'. In all other cases it will return undef.
>> +sub classify_drive_file {
>> +    my ($volid) = @_;
>> +
>> +    if ($volid eq 'none') {
>> +        return 'none';
>> +    } elsif ($volid eq 'cdrom') {
>> +        return 'cdrom';
>> +    } elsif ($volid =~ m|^/|) {
>> +        return 'absolute';
>> +    } elsif (PVE::Storage::parse_volume_id($volid, 1)) {
>> +        return 'volume';
>> +    }
>> +
>> +    return 'unknown';
>> +}
>
> Small note regarding style: While the above is completely fine (and you
> shouldn't change it!) I do wanna note that for more complex subroutines,
> I would personally prefer avoiding if-elsif chains, mainly because..
>
> 1. they tend to become a little too "packed" when it comes to reading the
>    code
>
> 2. they *may* inadvertently make future changes harder if a little more
>    complex logic is involved
>
> To give you an example, have a look at some of our older code in
> `pve-storage` [0] that I'm trying to refactor at the moment.
>
> Eventually I replace each branch one-by-one in my series [1], until I
> finally condense the refactored logic in (yet) another patch [2].
>
> Again, the code above is completely fine! Please don't feel like you
> have to refresh your series. I just wanted to mention it for future
> cases where the chains might be longer and the conditions are nastier :P
>
> [0]: https://git.proxmox.com/?p=pve-storage.git;a=blob;f=src/PVE/Storage/Plugin.pm;h=4f69f9b5db69674335eb3024d61d4a3430bca1ec;hb=refs/heads/master#l799
> [1]: https://lore.proxmox.com/pve-devel/20260422111322.257380-23-m.carrara@proxmox.com/#Z31src:PVE:Storage:Plugin.pm
> [2]: https://lore.proxmox.com/pve-devel/20260422111322.257380-30-m.carrara@proxmox.com/#Z31src:PVE:Storage:Plugin.pm

I totally agree, I also found the style a bit too dense for my liking, but
was not sure what the better option would be. In the future, I will try to
avoid it and go for a less "chainy" method.

Also regarding the other mail about the imperative subject, it's also a
thing I have to properly get used to. (the fullstop was a typo)

Thanks for pointing these things out. ;)

[snip]