Re: [pve-devel] [PATCH manager 3/5] api ceph osd: add volume details endpoint

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 05/07/2022 16:19, Aaron Lauterer wrote:
> Okay, so IIUC, drop the Datastore.Audit and only keep the Sys.Audit?

yes.

> 
> Being unsure 😉 Looking closer at run_command, it will die if there is some issue running the specified command, not found or returning an error. So this would only be needed if I would like to catch the error to handle it in some way, right?

>>> +    my $cmd = ['/usr/sbin/ceph-volume', 'lvm', 'list', '--format', 'json'];
>>> +    eval { run_command($cmd, errmsg => 'ceph volume error', outfunc => $parser) };
>>> +    die $@ if $@;
>>
>> what's the point in eval'ing if you do a plain die then anyway on $@?
> 
> Being unsure 😉 Looking closer at run_command, it will die if there is some issue running the specified command, not found or returning an error. So this would only be needed if I would like to catch the error to handle it in some way, right?

Most of the time an eval with a single (!) function call, for example like
`eval { foo() }; die $@ if $@;`, is identical with '`foo();`, I say most of the
time because a function may avoid die directly but only set $@ manually, and in
that case the eval can help to ensure it was an error from inside the function,
as `eval {}` will reset $@ to undef.

IIRC we only do such "set $@ manually without die" in the PVE::Tools
lock_file_full (and thus the depending lock_file) method though, so just
mentioning for completeness sake.

Otherwise you're right, doing eval { .. } + die if $@ is basically done when
we require to transform the error or have additional condition for when to
actually die, e.g., a $noerr parameter.