Re: [pve-devel] [PATCH v2 storage 1/2] rbd: improve handling of missing images

[Aaron Lauterer <a.lauterer@proxmox.com>]

ping?

On 6/14/23 13:10, Aaron Lauterer wrote:
> It can happen, that an RBD image isn't cleaned up 100%. Calling 'rbd ls
> -l' will then show errors that it is not possible to open the image in
> question:
> ```
> rbd: error opening vm-103-disk-1: (2) No such file or directory
> rbd: listing images failed: (2) No such file or directory
> ```
> 
> Originally we only showed the last error line which is too generic and
> doesn't give a good hint what is actually wrong.
> 
> We can improve that by catching these specific errors and add the
> problematic disk images to the returned list with a size of '-1'.
> 
> When the 'rbd rm' command is used on such an image, it will clean up
> whatever is still left.
> But for that to work, we also need to handle these errors in the
> 'rbd_ls_snap' sub as it is called from 'free_image'.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> no changes since v1
> 
>   src/PVE/Storage/RBDPlugin.pm | 52 +++++++++++++++++++++++++++++++-----
>   1 file changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
> index f45ad3f..c4e4467 100644
> --- a/src/PVE/Storage/RBDPlugin.pm
> +++ b/src/PVE/Storage/RBDPlugin.pm
> @@ -169,6 +169,8 @@ my $krbd_feature_update = sub {
>       }
>   };
>   
> +my $missing_image_err_regex = '((?:vm|base)-\d+-.*): \(2\) No such file or directory$';
> +
>   sub run_rbd_command {
>       my ($cmd, %args) = @_;
>   
> @@ -207,13 +209,28 @@ sub rbd_ls {
>       my $raw = '';
>       my $parser = sub { $raw .= shift };
>   
> +    my $show_err = 1;
> +    my $missing_images = {};
> +    my $err_parser = sub {
> +	my $line = shift;
> +	if ($line =~ m/$missing_image_err_regex/) {
> +	    $show_err = 0;
> +	    $missing_images->{$1} = 1;
> +	} elsif ($line ne "rbd: listing images failed: (2) No such file or directory") {
> +	    # this generic error is shown after the image specific "No such file..." one,
> +	    # ignore it but not other errors
> +	    $show_err = 1;
> +	    die $line;
> +	}
> +    };
> +
>       my $cmd = $rbd_cmd->($scfg, $storeid, 'ls', '-l', '--format', 'json');
>       eval {
> -	run_rbd_command($cmd, errmsg => "rbd error", errfunc => sub {}, outfunc => $parser);
> +	run_rbd_command($cmd, errmsg => "rbd error", errfunc => $err_parser, outfunc => $parser);
>       };
>       my $err = $@;
>   
> -    die $err if $err && $err !~ m/doesn't contain rbd images/ ;
> +    die $err if $err && $show_err && $err !~ m/doesn't contain rbd images/ ;
>   
>       my $result;
>       if ($raw eq '') {
> @@ -224,6 +241,13 @@ sub rbd_ls {
>   	die "got unexpected data from rbd ls: '$raw'\n";
>       }
>   
> +    for my $image (keys %$missing_images) {
> +	push @$result, {
> +	    image => $image,
> +	    size => -1,
> +	};
> +    }
> +
>       my $list = {};
>   
>       foreach my $el (@$result) {
> @@ -251,7 +275,20 @@ sub rbd_ls_snap {
>       my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'ls', $name, '--format', 'json');
>   
>       my $raw = '';
> -    run_rbd_command($cmd, errmsg => "rbd error", errfunc => sub {}, outfunc => sub { $raw .= shift; });
> +    my $show_err = 0;
> +    my $err_parser = sub {
> +	my $line = shift;
> +	if ($line !~ m/$missing_image_err_regex/) {
> +	    $show_err = 1;
> +	    die $line;
> +	}
> +    };
> +    eval {
> +	run_rbd_command($cmd, errmsg => "rbd error", errfunc => $err_parser, outfunc => sub { $raw .= shift; });
> +    };
> +    my $err = $@;
> +    die $err if $err && $show_err;
> +    return {} if $err && !$show_err; # could not open image, probably missing
>   
>       my $list;
>       if ($raw =~ m/^(\[.*\])$/s) { # untaint
> @@ -633,10 +670,13 @@ sub free_image {
>   
>       $class->deactivate_volume($storeid, $scfg, $volname);
>   
> -    my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'purge',  $name);
> -    run_rbd_command($cmd, errmsg => "rbd snap purge '$name' error");
>   
> -    $cmd = $rbd_cmd->($scfg, $storeid, 'rm', $name);
> +    if (keys %{$snaps}) {
> +	my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'purge',  $name);
> +	run_rbd_command($cmd, errmsg => "rbd snap purge '$name' error");
> +    }
> +
> +    my $cmd = $rbd_cmd->($scfg, $storeid, 'rm', $name);
>       run_rbd_command($cmd, errmsg => "rbd rm '$name' error");
>   
>       return undef;