[pve-devel] [PATCH storage] set filesize to undef on error during qemu-img info

[Thomas Lamprecht]

On 12.09.19 12:26, Tim Marx wrote:
> Signed-off-by: Tim Marx <t.marx at proxmox.com>
> ---
>  PVE/Storage/Plugin.pm | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 39622f3..08cb260 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -738,9 +738,18 @@ sub file_size_info {
>  	}
>      };
>  
> +    my $error_function = sub {
> +	my $line = shift;
> +	$size = undef;
> +	$used = undef;
> +
> +	warn $line;
> +    };
> +
>      my $cmd = ['/usr/bin/qemu-img', 'info', $filename];
>      eval {
> -	run_command($cmd, timeout => $timeout, outfunc => $parse_qemu_img_info );
> +	run_command($cmd, timeout => $timeout, outfunc => $parse_qemu_img_info,
> +	errfunc => $error_function);
>      };
>      warn $@ if $@;
>  
> 

hmm, this could be racy.. E.g., qemu-img process outputs first on stderr about
something and then continues to print on stdout, $size and/or $used could still
be set to some value.

In that case it isn't probably a big issue, but the code and your intentions
look like you want to always want to set it to undef in such a case?

So we could do a few things, e.g.:
* have an error string and in the errfunc just add to that one, at the end below the
  warn $@ one could then also print that one and set size/used to undef?
* set both to undef at first, only allow to set it to a real integer value if it gets
  parsed correctly.


Also, we really could use the "--output=json" param and just decode the json from
stdout directly to a hash?