From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id A823462E0A for ; Wed, 23 Feb 2022 10:08:57 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9766E2C154 for ; Wed, 23 Feb 2022 10:08:27 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 155D02C148 for ; Wed, 23 Feb 2022 10:08:27 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id D0E9F46311 for ; Wed, 23 Feb 2022 10:08:20 +0100 (CET) Message-ID: <71ba5a80-69e4-ed02-c1ef-8d5ab7e3216b@proxmox.com> Date: Wed, 23 Feb 2022 10:08:04 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.0 Content-Language: en-US To: pve-devel@lists.proxmox.com, m.heiserer@proxmox.com References: <20220217141251.739753-1-m.heiserer@proxmox.com> From: Fabian Ebner In-Reply-To: <20220217141251.739753-1-m.heiserer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.131 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [PATCH qemu-server] fix 3674: QEMU restore: verify storage allows images before writing X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Feb 2022 09:08:57 -0000 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 > --- > 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.