Re: [pve-devel] [PATCH qemu-server v2] fix 3886: QEMU restore: verify storage allows images before writing

[Fabian Ebner <f.ebner@proxmox.com>]

Am 23.02.22 um 12:15 schrieb Matthias Heiserer:
> When restoring a backup and the storage the disks would be created on
> doesn't allow 'images', the process errors without cleanup.
> This is the same behaviour we currently have when the storage is
> disabled.
> 
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
>  PVE/QemuServer.pm | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index a99f1a5..aaada7a 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -6243,6 +6243,9 @@ my $parse_backup_hints = sub {
>  	    } elsif (!$storeid) {
>  		$storeid = 'local';
>  	    }
> +	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
> +	    die "Content type 'images' is not available on storage '$storeid'\n"
> +		if !$scfg->{content}->{images};

Style nit: could be placed closer to the permission check for
Datastore.AllocateSpace that follows below.

>  	    $format = 'raw' if !$format;
>  	    $devinfo->{$devname}->{devname} = $devname;
>  	    $devinfo->{$devname}->{virtdev} = $virtdev;
> @@ -6264,6 +6267,8 @@ my $parse_backup_hints = sub {
>  		$storeid = $options->{storage} if defined ($options->{storage});
>  		my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
>  		my $format = qemu_img_format($scfg, $volname); # has 'raw' fallback
> +		die "Content type 'images' is not available on storage '$storeid'\n"
> +		    if !$scfg->{content}->{images};

Here, the permission check for Datastore.AllocateSpace is missing, which
is the other existing bug. Maybe it's even worth having a small closure
doing both checks to re-use in both cases, but not really sure.

>  
>  		$virtdev_hash->{$virtdev} = {
>  		    format => $format,