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

[pve-devel] [PATCH qemu-server v4] 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.

Adds the Datastore.AllocateSpace check to cloudinit drives.

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
Thanks to Fabian!
Changes from v3:
 + Restore a line I accidentally deleted

Changes from v2:
 + Also check Datastore.AllocateSpace permissions for cloudinit device
 + Move content type check closer to allocate permission check

Changes from v1:
 + Check content type for cloud init drives
 
 PVE/QemuServer.pm | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a99f1a5..a07a4eb 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6249,6 +6249,10 @@ my $parse_backup_hints = sub {
 	    $devinfo->{$devname}->{format} = $format;
 	    $devinfo->{$devname}->{storeid} = $storeid;
 
+	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
+	    die "Content type 'images' is not available on storage '$storeid'\n"
+		if !$scfg->{content}->{images};
+
 	    # check permission on storage
 	    my $pool = $options->{pool}; # todo: do we need that?
 	    if ($user ne 'root@pam') {
@@ -6265,6 +6269,12 @@ my $parse_backup_hints = sub {
 		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};
+		if ($user ne 'root@pam') {
+		    $rpcenv->check($user, "/storage/$storeid", ['Datastore.AllocateSpace']);
+		}
+
 		$virtdev_hash->{$virtdev} = {
 		    format => $format,
 		    storeid => $storeid,
-- 
2.30.2

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

[Thomas Lamprecht]

On 24.02.22 11:45, Matthias Heiserer wrote:
> 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.
> 
> Adds the Datastore.AllocateSpace check to cloudinit drives.

that belongs in a separate patch, I split that out.

> 
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>

Please pick up T-b/R-b tags from earlier reviews if the patch did not change
substantially in between - thx!

> ---
> Thanks to Fabian!
> Changes from v3:
>  + Restore a line I accidentally deleted
> 
> Changes from v2:
>  + Also check Datastore.AllocateSpace permissions for cloudinit device
>  + Move content type check closer to allocate permission check
> 
> Changes from v1:
>  + Check content type for cloud init drives
>  
>  PVE/QemuServer.pm | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
>

applied, thanks! Followed up with using a closure for the shared checks, not
less lines of code but slightly less noise in the looping over backup config
part.