Re: [PATCH storage v9 3/5] fix #7339: lvm: add discard action for removed volumes

["Lukas Sichert" <l.sichert@proxmox.com>]

On 2026-07-15 12:26, Fiona Ebner <f.ebner@proxmox.com> wrote:
> Am 13.07.26 um 6:00 PM schrieb Lukas Sichert:

>> +sub assert_discard_supported {
>> +    my ($vgname, $on_remove) = @_;
>> +
>> +    return if !defined($on_remove);
>> +
>> +    my $on_remove_opts = PVE::JSONSchema::parse_property_string('on-volume-remove', $on_remove);
>> +    if ($on_remove_opts->{discard}) {
>> +
>> +        my $vgs = lvm_vgs(1);
>> +        my $vg = $vgs->{$vgname};
>> +        die "no such volume group '$vgname'\n" if !$vg;
>> +
>> +        my $pvs = $vg->{pvs};
>> +        die "volume group '$vgname' has no physical volumes\n"
>> +            if !defined($pvs) || scalar($pvs->@*) == 0;
>> +
>> +        # check if all the block devices configured to the volume group support discard
>> +        for my $pv ($vg->{pvs}->@*) {
>> +
>> +            my $dev_path = abs_path($pv->{name}) // $pv->{name};
>> +
>> +            if ($dev_path =~ m!^(/dev/[A-Za-z0-9_+./=-]+)$!) {
>
> The information comes from LVM and is only used as the argument for
> lsblk, so should we untaint with .* ? Or is there a specific reason for
> this exact regex?

Yes, the regex is unnecessary. But IMO .+ is better than .*, because we should not
match against '/dev/'.

>
>> +                $dev_path = $1; # untaint