[pve-devel] [PATCH qemu-server 3/6] template: mark efidisk as read-only

[Fabian Grünbichler]

On June 7, 2021 11:29 am, Stefan Reiter wrote:
> High-level: If you make the $read_only_str contain the entire 
> ",readonly=on" stanza, you can just leave it blank by default and avoid 
> updating all the test cases.

yeah, but once you add a few of those the resulting code gets rather 
messy/ugly. except for a bit of test churn it does not hurt to spell it 
out (and if we want to leave it out, I'd rather factor out the combined 
string and conditionally add to it than have multiple variables that 
might be empty concatenated).

> 
> On 6/4/21 11:47 AM, Fabian Grünbichler 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.
>> 
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
>> ---
>>   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(-)
>> 
>> 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;
>>   
>>   	my ($path, $format);
>> +	my $read_only_str = 'off'; >   	if (my $efidisk = $conf->{efidisk0}) {
>>   	    my $d = parse_drive('efidisk0', $efidisk);
>>   	    my ($storeid, $volname) = PVE::Storage::parse_volume_id($d->{file}, 1);
>> @@ -3294,6 +3295,8 @@ sub config_to_command {
>>   		die "efidisk format must be specified\n"
>>   		    if !defined($format);
>>   	    }
>> +
>> +	    $read_only_str = 'on' if drive_is_read_only($conf, $d);
>>   	} else {
>>   	    warn "no efidisk configured! Using temporary efivars disk.\n";
>>   	    $path = "/tmp/$vmid-ovmf.fd";
>> @@ -3308,7 +3311,7 @@ sub config_to_command {
>>   	}
>>   
>>   	push @$cmd, '-drive', "if=pflash,unit=0,format=raw,readonly=on,file=$ovmf_code";
>> -	push @$cmd, '-drive', "if=pflash,unit=1,format=$format,id=drive-efidisk0$size_str,file=$path";
>> +	push @$cmd, '-drive', "if=pflash,unit=1,format=$format,id=drive-efidisk0$size_str,file=$path,readonly=$read_only_str";
>>       }
>>   
>>       # load q35 config
>> diff --git a/test/cfg2cmd/efi-raw-old.conf.cmd b/test/cfg2cmd/efi-raw-old.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=1,uuid=7b10d7af-b932-4c66-b2c3-3996152ec465' \
>>     -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
>> -  -drive 'if=pflash,unit=1,format=raw,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-disk-100-0.raw' \
>> +  -drive 'if=pflash,unit=1,format=raw,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-disk-100-0.raw,readonly=off' \
>>     -smp '1,sockets=1,cores=1,maxcpus=1' \
>>     -nodefaults \
>>     -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
>> diff --git a/test/cfg2cmd/efi-raw.conf.cmd b/test/cfg2cmd/efi-raw.conf.cmd
>> 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=1,uuid=7b10d7af-b932-4c66-b2c3-3996152ec465' \
>>     -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
>> -  -drive 'if=pflash,unit=1,format=raw,id=drive-efidisk0,size=131072,file=/var/lib/vz/images/100/vm-disk-100-0.raw' \
>> +  -drive 'if=pflash,unit=1,format=raw,id=drive-efidisk0,size=131072,file=/var/lib/vz/images/100/vm-disk-100-0.raw,readonly=off' \
>>     -smp '1,sockets=1,cores=1,maxcpus=1' \
>>     -nodefaults \
>>     -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
>> diff --git a/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd b/test/cfg2cmd/i440fx-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=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
>>     -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
>> -  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
>> +  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2,readonly=off' \
>>     -smp '2,sockets=2,cores=1,maxcpus=2' \
>>     -nodefaults \
>>     -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
>> diff --git a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd b/test/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=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
>>     -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
>> -  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
>> +  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2,readonly=off' \
>>     -smp '2,sockets=2,cores=1,maxcpus=2' \
>>     -nodefaults \
>>     -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/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=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
>>     -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
>> -  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
>> +  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2,readonly=off' \
>>     -smp '2,sockets=2,cores=1,maxcpus=2' \
>>     -nodefaults \
>>     -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/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=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
>>     -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
>> -  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
>> +  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2,readonly=off' \
>>     -smp '2,sockets=2,cores=1,maxcpus=2' \
>>     -nodefaults \
>>     -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
>> 
>