Re: [pve-devel] [PATCH storage] lvm: avoid warning due to human-readable text in vgs output

[Friedrich Weber <f.weber@proxmox.com>]

On 12/01/2024 11:28, Fabian Grünbichler wrote:
>> The vgs message is printed to stdout, so we could do something like
>>
>> warn $line if !defined($size);
>>
>> ?
> 
> yep, that would be an option (warn+return ;))

Right, thanks. Thinking about this some more, printing a user-visible
warning sounds more sensible than suppressing the warning complete
(either by passing `-qq` or ignoring the line, as in the current patch).
I'll send a v2.
>> Another complication I forgot about: For that user, /etc/lvm/archive had
>> 800000 files amounting to >1GiB, which also slowed down `vgs -o vg_name`
>> considerably (to >10s), presumably because `vgs -o vg_name` read all
>> those files. But unexpectedly, as soon as `-o` included `pv_name` the
>> command was fast again, presumably because it does not do the reads. So
>> I was considering to modify `sub lvm_vgs` to always include `-o pv_name`
>> in the command (not only if $includepvs is true), but was unsure if the
>> edge case warranted this (somewhat weird) workaround.
> 
> that sounds weird ;)
Yeah, I think I won't implement this for now. If users wonder about the
long status update times of pvestatd, they will look into the journal
and, in v2, see the warnings about the large archive.

>> By the way, the message also causes `vgs` to print invalid JSON:
>>
>> # rm -f /etc/lvm/backup/spam ; vgs -o vg_name -q --reportformat json
>> 2>/dev/null
>>   {
>>       "report": [
>>   Consider pruning spam VG archive with more then 8 MiB in 8305 files
>> (check archiving is needed in lvm.conf).
>>           {
>>               "vg": [
>>                   {"vg_name":"pve"},
>>                   {"vg_name":"spam"},
>>                   {"vg_name":"testvg"}
>>               ]
>>           }
>>       ]
>>   }
>>
>> Dominik suggested that this very much looks like an LVM bug, so I'll
>> check whether it still happens on latest upstream LVM and, if yes, file
>> a bug report there.
> 
> yes, that definitely sounds like a bug. potentially they'd also be open
> to switching the log level/target so that it ends up on STDERR at
> least..

done: https://github.com/lvmteam/lvm2/issues/137