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 EDB7D7074C for ; Mon, 7 Jun 2021 12:23:42 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E3C50FDBE for ; Mon, 7 Jun 2021 12:23:12 +0200 (CEST) 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 BEFA0FDB2 for ; Mon, 7 Jun 2021 12:23:08 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 95F5642B78 for ; Mon, 7 Jun 2021 12:23:08 +0200 (CEST) Date: Mon, 07 Jun 2021 12:23:00 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion , Stefan Reiter References: <20210604094748.3383339-1-f.gruenbichler@proxmox.com> <20210604094748.3383339-4-f.gruenbichler@proxmox.com> In-Reply-To: MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1623061033.usnvayvh3c.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.951 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH qemu-server 3/6] template: mark efidisk as read-only 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: Mon, 07 Jun 2021 10:23:43 -0000 On June 7, 2021 11:29 am, Stefan Reiter wrote: > High-level: If you make the $read_only_str contain the entire=20 > ",readonly=3Don" stanza, you can just leave it blank by default and avoid= =20 > updating all the test cases. yeah, but once you add a few of those the resulting code gets rather=20 messy/ugly. except for a bit of test churn it does not hurt to spell it=20 out (and if we want to leave it out, I'd rather factor out the combined=20 string and conditionally add to it than have multiple variables that=20 might be empty concatenated). >=20 > On 6/4/21 11:47 AM, Fabian Gr=C3=BCnbichler wrote: >> otherwise backups of templates using UEFI fail with storages like LVM >> thin, where the volumes are not writable. disk controllers like IDE and >> SATA that don't support being read-only are still broken for UEFI. >>=20 >> Signed-off-by: Fabian Gr=C3=BCnbichler >> --- >> PVE/QemuServer.pm | 5 ++++- >> test/cfg2cmd/efi-raw-old.conf.cmd | 2 +- >> test/cfg2cmd/efi-raw.conf.cmd | 2 +- >> test/cfg2cmd/i440fx-win10-hostpci.conf.cmd | 2 +- >> test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd | 2 +- >> test/cfg2cmd/q35-linux-hostpci.conf.cmd | 2 +- >> test/cfg2cmd/q35-win10-hostpci.conf.cmd | 2 +- >> 7 files changed, 10 insertions(+), 7 deletions(-) >>=20 >> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm >> index 0d49415..3d996af 100644 >> --- a/PVE/QemuServer.pm >> +++ b/PVE/QemuServer.pm >> @@ -3279,6 +3279,7 @@ sub config_to_command { >> die "uefi base image '$ovmf_code' not found\n" if ! -f $ovmf_code; >> =20 >> my ($path, $format); >> + my $read_only_str =3D 'off'; > if (my $efidisk =3D $conf->{efidisk0= }) { >> my $d =3D parse_drive('efidisk0', $efidisk); >> my ($storeid, $volname) =3D PVE::Storage::parse_volume_id($d->{fi= le}, 1); >> @@ -3294,6 +3295,8 @@ sub config_to_command { >> die "efidisk format must be specified\n" >> if !defined($format); >> } >> + >> + $read_only_str =3D 'on' if drive_is_read_only($conf, $d); >> } else { >> warn "no efidisk configured! Using temporary efivars disk.\n"; >> $path =3D "/tmp/$vmid-ovmf.fd"; >> @@ -3308,7 +3311,7 @@ sub config_to_command { >> } >> =20 >> push @$cmd, '-drive', "if=3Dpflash,unit=3D0,format=3Draw,readonly=3Do= n,file=3D$ovmf_code"; >> - push @$cmd, '-drive', "if=3Dpflash,unit=3D1,format=3D$format,id=3Ddriv= e-efidisk0$size_str,file=3D$path"; >> + push @$cmd, '-drive', "if=3Dpflash,unit=3D1,format=3D$format,id=3Ddriv= e-efidisk0$size_str,file=3D$path,readonly=3D$read_only_str"; >> } >> =20 >> # load q35 config >> diff --git a/test/cfg2cmd/efi-raw-old.conf.cmd b/test/cfg2cmd/efi-raw-ol= d.conf.cmd >> index 0f8b7c7..622ade3 100644 >> --- a/test/cfg2cmd/efi-raw-old.conf.cmd >> +++ b/test/cfg2cmd/efi-raw-old.conf.cmd >> @@ -10,7 +10,7 @@ >> -daemonize \ >> -smbios 'type=3D1,uuid=3D7b10d7af-b932-4c66-b2c3-3996152ec465' \ >> -drive 'if=3Dpflash,unit=3D0,format=3Draw,readonly=3Don,file=3D/usr/= share/pve-edk2-firmware//OVMF_CODE.fd' \ >> - -drive 'if=3Dpflash,unit=3D1,format=3Draw,id=3Ddrive-efidisk0,file=3D= /var/lib/vz/images/100/vm-disk-100-0.raw' \ >> + -drive 'if=3Dpflash,unit=3D1,format=3Draw,id=3Ddrive-efidisk0,file=3D= /var/lib/vz/images/100/vm-disk-100-0.raw,readonly=3Doff' \ >> -smp '1,sockets=3D1,cores=3D1,maxcpus=3D1' \ >> -nodefaults \ >> -boot 'menu=3Don,strict=3Don,reboot-timeout=3D1000,splash=3D/usr/sha= re/qemu-server/bootsplash.jpg' \ >> diff --git a/test/cfg2cmd/efi-raw.conf.cmd b/test/cfg2cmd/efi-raw.conf.c= md >> index 8af615c..a7a7373 100644 >> --- a/test/cfg2cmd/efi-raw.conf.cmd >> +++ b/test/cfg2cmd/efi-raw.conf.cmd >> @@ -10,7 +10,7 @@ >> -daemonize \ >> -smbios 'type=3D1,uuid=3D7b10d7af-b932-4c66-b2c3-3996152ec465' \ >> -drive 'if=3Dpflash,unit=3D0,format=3Draw,readonly=3Don,file=3D/usr/= share/pve-edk2-firmware//OVMF_CODE.fd' \ >> - -drive 'if=3Dpflash,unit=3D1,format=3Draw,id=3Ddrive-efidisk0,size=3D= 131072,file=3D/var/lib/vz/images/100/vm-disk-100-0.raw' \ >> + -drive 'if=3Dpflash,unit=3D1,format=3Draw,id=3Ddrive-efidisk0,size=3D= 131072,file=3D/var/lib/vz/images/100/vm-disk-100-0.raw,readonly=3Doff' \ >> -smp '1,sockets=3D1,cores=3D1,maxcpus=3D1' \ >> -nodefaults \ >> -boot 'menu=3Don,strict=3Don,reboot-timeout=3D1000,splash=3D/usr/sha= re/qemu-server/bootsplash.jpg' \ >> diff --git a/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd b/test/cfg2cmd/i= 440fx-win10-hostpci.conf.cmd >> index c79576f..1e11710 100644 >> --- a/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd >> +++ b/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd >> @@ -10,7 +10,7 @@ >> -daemonize \ >> -smbios 'type=3D1,uuid=3D3dd750ce-d910-44d0-9493-525c0be4e687' \ >> -drive 'if=3Dpflash,unit=3D0,format=3Draw,readonly=3Don,file=3D/usr/= share/pve-edk2-firmware//OVMF_CODE.fd' \ >> - -drive 'if=3Dpflash,unit=3D1,format=3Dqcow2,id=3Ddrive-efidisk0,file= =3D/var/lib/vz/images/100/vm-100-disk-1.qcow2' \ >> + -drive 'if=3Dpflash,unit=3D1,format=3Dqcow2,id=3Ddrive-efidisk0,file= =3D/var/lib/vz/images/100/vm-100-disk-1.qcow2,readonly=3Doff' \ >> -smp '2,sockets=3D2,cores=3D1,maxcpus=3D2' \ >> -nodefaults \ >> -boot 'menu=3Don,strict=3Don,reboot-timeout=3D1000,splash=3D/usr/sha= re/qemu-server/bootsplash.jpg' \ >> diff --git a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd b/tes= t/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd >> index 1f9beda..d140501 100644 >> --- a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd >> +++ b/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd >> @@ -10,7 +10,7 @@ >> -daemonize \ >> -smbios 'type=3D1,uuid=3D3dd750ce-d910-44d0-9493-525c0be4e687' \ >> -drive 'if=3Dpflash,unit=3D0,format=3Draw,readonly=3Don,file=3D/usr/= share/pve-edk2-firmware//OVMF_CODE.fd' \ >> - -drive 'if=3Dpflash,unit=3D1,format=3Dqcow2,id=3Ddrive-efidisk0,file= =3D/var/lib/vz/images/100/vm-100-disk-1.qcow2' \ >> + -drive 'if=3Dpflash,unit=3D1,format=3Dqcow2,id=3Ddrive-efidisk0,file= =3D/var/lib/vz/images/100/vm-100-disk-1.qcow2,readonly=3Doff' \ >> -smp '2,sockets=3D2,cores=3D1,maxcpus=3D2' \ >> -nodefaults \ >> -boot 'menu=3Don,strict=3Don,reboot-timeout=3D1000,splash=3D/usr/sha= re/qemu-server/bootsplash.jpg' \ >> diff --git a/test/cfg2cmd/q35-linux-hostpci.conf.cmd b/test/cfg2cmd/q35-= linux-hostpci.conf.cmd >> index dd1bece..7d69134 100644 >> --- a/test/cfg2cmd/q35-linux-hostpci.conf.cmd >> +++ b/test/cfg2cmd/q35-linux-hostpci.conf.cmd >> @@ -10,7 +10,7 @@ >> -daemonize \ >> -smbios 'type=3D1,uuid=3D3dd750ce-d910-44d0-9493-525c0be4e687' \ >> -drive 'if=3Dpflash,unit=3D0,format=3Draw,readonly=3Don,file=3D/usr/= share/pve-edk2-firmware//OVMF_CODE.fd' \ >> - -drive 'if=3Dpflash,unit=3D1,format=3Dqcow2,id=3Ddrive-efidisk0,file= =3D/var/lib/vz/images/100/vm-100-disk-1.qcow2' \ >> + -drive 'if=3Dpflash,unit=3D1,format=3Dqcow2,id=3Ddrive-efidisk0,file= =3D/var/lib/vz/images/100/vm-100-disk-1.qcow2,readonly=3Doff' \ >> -smp '2,sockets=3D2,cores=3D1,maxcpus=3D2' \ >> -nodefaults \ >> -boot 'menu=3Don,strict=3Don,reboot-timeout=3D1000,splash=3D/usr/sha= re/qemu-server/bootsplash.jpg' \ >> diff --git a/test/cfg2cmd/q35-win10-hostpci.conf.cmd b/test/cfg2cmd/q35-= win10-hostpci.conf.cmd >> index 37ef8f7..87a847d 100644 >> --- a/test/cfg2cmd/q35-win10-hostpci.conf.cmd >> +++ b/test/cfg2cmd/q35-win10-hostpci.conf.cmd >> @@ -10,7 +10,7 @@ >> -daemonize \ >> -smbios 'type=3D1,uuid=3D3dd750ce-d910-44d0-9493-525c0be4e687' \ >> -drive 'if=3Dpflash,unit=3D0,format=3Draw,readonly=3Don,file=3D/usr/= share/pve-edk2-firmware//OVMF_CODE.fd' \ >> - -drive 'if=3Dpflash,unit=3D1,format=3Dqcow2,id=3Ddrive-efidisk0,file= =3D/var/lib/vz/images/100/vm-100-disk-1.qcow2' \ >> + -drive 'if=3Dpflash,unit=3D1,format=3Dqcow2,id=3Ddrive-efidisk0,file= =3D/var/lib/vz/images/100/vm-100-disk-1.qcow2,readonly=3Doff' \ >> -smp '2,sockets=3D2,cores=3D1,maxcpus=3D2' \ >> -nodefaults \ >> -boot 'menu=3Don,strict=3Don,reboot-timeout=3D1000,splash=3D/usr/sha= re/qemu-server/bootsplash.jpg' \ >>=20 >=20