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

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

[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};
 	    $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};
 
 		$virtdev_hash->{$virtdev} = {
 		    format => $format,
-- 
2.30.2

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

[Fabian Ebner]

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,