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

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

Am 17.02.22 um 15:12 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 | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index a99f1a5..2a1ec48 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -6299,6 +6299,10 @@ my $restore_allocate_devices = sub {
>  	my $supported = grep { $_ eq $d->{format} } @$validFormats;
>  	$d->{format} = $defFormat if !$supported;
>  
> +	# check if images can be stored on the requested storage

Nit: The comment isn't needed IMHO, because the code is pretty clear on
its own.

> +	die "Content type 'images' is not available on storage '$storeid'\n"
> +	    if !$scfg->{content}->{images};
> +
>  	my $name;
>  	if ($d->{is_cloudinit}) {
>  	    $name = "vm-$vmid-cloudinit";

Nothing wrong with the patch (except for the bug number as you already
pointed out ;)), it's just that the permission check for accessing the
storage is currently done in parse_backup_hints(), so it might be a bit
cleaner to add the new check there too. Like that, both checks are in
one place and we can abort early, before starting to allocate any disks.

It seems that there's another small bug in parse_backup_hints(), because
there's no permission check for a cloudinit disk. Would be nice if you
could fix that too.