[pve-devel] [PATCH storage 1/5] Disks API: allow also unused disks for db/wal

[Dominik Csapak]

On 6/4/19 9:54 AM, Thomas Lamprecht wrote:
> On 6/4/19 9:21 AM, Dominik Csapak wrote:
>> since we will create a pv/vg/lv on it with nautilus
>>
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>>   PVE/API2/Disks.pm | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/PVE/API2/Disks.pm b/PVE/API2/Disks.pm
>> index ce4acee..7433066 100644
>> --- a/PVE/API2/Disks.pm
>> +++ b/PVE/API2/Disks.pm
>> @@ -137,7 +137,7 @@ __PACKAGE__->register_method ({
>>   	    my $entry = $disks->{$disk};
>>   	    if ($type eq 'journal_disks') {
>>   		next if $entry->{osdid} >= 0;
>> -		next if !$entry->{gpt} && $entry->{used} ne 'LVM';
>> +		next if !$entry->{gpt} && $entry->{used} ne 'LVM' && $entry->{used};
> 
> is the sub-expression ($entry->{used} ne 'LVM' && $entry->{used})
> not somewhat redundant? Or should at least be switched, no point in
> checking if it's a LWN if it's not even defined?

i guess you are right about the switching but no they are not redundant
(i admit the code is not the cleanes one) because

it skips a disk if it is not gpt and it is used and used is not lvm

i will try to find a better condition that is easier to read

> 
> as get_disk from Diskmanage does:
> 
> $disklist->{$dev}->{used} = $used if $used;
> 
> it is either truthy or not defined at all, it cannot be defined but
> falsy, so switching this would avoid a potential "undefined in ..
> comparison" warning, or not?
> 
>>   	    } elsif ($type eq 'unused') {
>>   		next if $entry->{used};
>>   	    } elsif ($type ne '') {
>>
>