* [pve-devel] [PATCH qemu-server 1/6] test: unbreak restore_config_test
2021-06-04 9:47 [pve-devel] [PATCH qemu-server 0/6] fix #2862: more template backup fixes Fabian Grünbichler
@ 2021-06-04 9:47 ` Fabian Grünbichler
2021-06-04 9:47 ` [pve-devel] [PATCH qemu-server 2/6] drive: factor out read-only helper Fabian Grünbichler
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Fabian Grünbichler @ 2021-06-04 9:47 UTC (permalink / raw)
To: pve-devel
for unprivileged users (and possibly some root setups). reading from
pmxcfs now results in a hard error for unprivileged users, so there
might be some more of these lurking somewhere..
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
test/run_qemu_restore_config_tests.pl | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/test/run_qemu_restore_config_tests.pl b/test/run_qemu_restore_config_tests.pl
index e5d9f2a..dc10647 100755
--- a/test/run_qemu_restore_config_tests.pl
+++ b/test/run_qemu_restore_config_tests.pl
@@ -6,6 +6,7 @@ use warnings;
use lib qw(..);
use Test::More;
+use Test::MockModule;
use File::Basename;
@@ -18,6 +19,19 @@ my $EXPECTED_DIR = './restore-config-expected';
# NOTE update when you add/remove tests
plan tests => 4;
+my $cfs_mock = Test::MockModule->new("PVE::Cluster");
+$cfs_mock->mock(
+ cfs_read_file => sub {
+ my ($file) = @_;
+
+ if ($file eq 'datacenter.cfg') {
+ return {};
+ } else {
+ die "'cfs_read_file' called - missing mock?\n";
+ }
+ },
+);
+
dir_glob_foreach('./restore-config-input', '[0-9]+.conf', sub {
my ($file) = @_;
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [pve-devel] [PATCH qemu-server 2/6] drive: factor out read-only helper
2021-06-04 9:47 [pve-devel] [PATCH qemu-server 0/6] fix #2862: more template backup fixes Fabian Grünbichler
2021-06-04 9:47 ` [pve-devel] [PATCH qemu-server 1/6] test: unbreak restore_config_test Fabian Grünbichler
@ 2021-06-04 9:47 ` Fabian Grünbichler
2021-06-07 9:29 ` Stefan Reiter
2021-06-04 9:47 ` [pve-devel] [PATCH qemu-server 3/6] template: mark efidisk as read-only Fabian Grünbichler
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Fabian Grünbichler @ 2021-06-04 9:47 UTC (permalink / raw)
To: pve-devel
we also need it for efidisks.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
PVE/QemuServer.pm | 8 ++------
PVE/QemuServer/Drive.pm | 10 ++++++++++
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 25ac052..0d49415 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -48,7 +48,7 @@ use PVE::QemuServer::Helpers qw(min_version config_aware_timeout);
use PVE::QemuServer::Cloudinit;
use PVE::QemuServer::CGroup;
use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options);
-use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom parse_drive print_drive);
+use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
use PVE::QemuServer::Machine;
use PVE::QemuServer::Memory;
use PVE::QemuServer::Monitor qw(mon_cmd);
@@ -3662,11 +3662,7 @@ sub config_to_command {
my $drive_cmd = print_drive_commandline_full($storecfg, $vmid, $drive, $pbs_name);
# extra protection for templates, but SATA and IDE don't support it..
- my $read_only = PVE::QemuConfig->is_template($conf)
- && $drive->{interface} ne 'sata'
- && $drive->{interface} ne 'ide';
-
- $drive_cmd .= ',readonly=on' if $read_only;
+ $drive_cmd .= ',readonly=on' if drive_is_read_only($conf, $drive);
push @$devices, '-drive',$drive_cmd;
push @$devices, '-device', print_drivedevice_full(
diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index 146a4ab..0408e32 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -12,6 +12,7 @@ our @EXPORT_OK = qw(
is_valid_drivename
drive_is_cloudinit
drive_is_cdrom
+drive_is_read_only
parse_drive
print_drive
);
@@ -422,6 +423,15 @@ sub drive_is_cdrom {
return $drive && $drive->{media} && ($drive->{media} eq 'cdrom');
}
+sub drive_is_read_only {
+ my ($conf, $drive) = @_;
+
+ return 0 if !PVE::QemuConfig->is_template($conf);
+
+ # don't support being marked read-only
+ return $drive->{interface} ne 'sata' && $drive->{interface} ne 'ide';
+}
+
# ideX = [volume=]volume-id[,media=d][,cyls=c,heads=h,secs=s[,trans=t]]
# [,snapshot=on|off][,cache=on|off][,format=f][,backup=yes|no]
# [,rerror=ignore|report|stop][,werror=enospc|ignore|report|stop]
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pve-devel] [PATCH qemu-server 2/6] drive: factor out read-only helper
2021-06-04 9:47 ` [pve-devel] [PATCH qemu-server 2/6] drive: factor out read-only helper Fabian Grünbichler
@ 2021-06-07 9:29 ` Stefan Reiter
2021-06-07 10:23 ` Fabian Grünbichler
0 siblings, 1 reply; 15+ messages in thread
From: Stefan Reiter @ 2021-06-07 9:29 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
On 6/4/21 11:47 AM, Fabian Grünbichler wrote:
> we also need it for efidisks.
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> PVE/QemuServer.pm | 8 ++------
> PVE/QemuServer/Drive.pm | 10 ++++++++++
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 25ac052..0d49415 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -48,7 +48,7 @@ use PVE::QemuServer::Helpers qw(min_version config_aware_timeout);
> use PVE::QemuServer::Cloudinit;
> use PVE::QemuServer::CGroup;
> use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options);
> -use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom parse_drive print_drive);
> +use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
> use PVE::QemuServer::Machine;
> use PVE::QemuServer::Memory;
> use PVE::QemuServer::Monitor qw(mon_cmd);
> @@ -3662,11 +3662,7 @@ sub config_to_command {
> my $drive_cmd = print_drive_commandline_full($storecfg, $vmid, $drive, $pbs_name);
>
> # extra protection for templates, but SATA and IDE don't support it..
> - my $read_only = PVE::QemuConfig->is_template($conf)
> - && $drive->{interface} ne 'sata'
> - && $drive->{interface} ne 'ide';
> -
> - $drive_cmd .= ',readonly=on' if $read_only;
> + $drive_cmd .= ',readonly=on' if drive_is_read_only($conf, $drive);
>
> push @$devices, '-drive',$drive_cmd;
> push @$devices, '-device', print_drivedevice_full(
> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
> index 146a4ab..0408e32 100644
> --- a/PVE/QemuServer/Drive.pm
> +++ b/PVE/QemuServer/Drive.pm
> @@ -12,6 +12,7 @@ our @EXPORT_OK = qw(
> is_valid_drivename
> drive_is_cloudinit
> drive_is_cdrom
> +drive_is_read_only
> parse_drive
> print_drive
> );
> @@ -422,6 +423,15 @@ sub drive_is_cdrom {
> return $drive && $drive->{media} && ($drive->{media} eq 'cdrom');
> }
>
> +sub drive_is_read_only {
I really don't like this name, this checks if the drive *should* be
read-only, and only related to template backups, not in general.
Maybe 'drive_template_read_only'?
The function does two pretty unrelated things in general IMO, so maybe
it would be clearer to do the is_template check at call site and make
this 'drive_supports_read_only', even if it causes a little bit more
duplication.
> + my ($conf, $drive) = @_;
> +
> + return 0 if !PVE::QemuConfig->is_template($conf);
> +
> + # don't support being marked read-only
> + return $drive->{interface} ne 'sata' && $drive->{interface} ne 'ide';
> +}
> +
> # ideX = [volume=]volume-id[,media=d][,cyls=c,heads=h,secs=s[,trans=t]]
> # [,snapshot=on|off][,cache=on|off][,format=f][,backup=yes|no]
> # [,rerror=ignore|report|stop][,werror=enospc|ignore|report|stop]
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pve-devel] [PATCH qemu-server 2/6] drive: factor out read-only helper
2021-06-07 9:29 ` Stefan Reiter
@ 2021-06-07 10:23 ` Fabian Grünbichler
2021-06-07 10:35 ` Stefan Reiter
0 siblings, 1 reply; 15+ messages in thread
From: Fabian Grünbichler @ 2021-06-07 10:23 UTC (permalink / raw)
To: Proxmox VE development discussion, Stefan Reiter
On June 7, 2021 11:29 am, Stefan Reiter wrote:
> On 6/4/21 11:47 AM, Fabian Grünbichler wrote:
>> we also need it for efidisks.
>>
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>> ---
>> PVE/QemuServer.pm | 8 ++------
>> PVE/QemuServer/Drive.pm | 10 ++++++++++
>> 2 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index 25ac052..0d49415 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -48,7 +48,7 @@ use PVE::QemuServer::Helpers qw(min_version config_aware_timeout);
>> use PVE::QemuServer::Cloudinit;
>> use PVE::QemuServer::CGroup;
>> use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options);
>> -use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom parse_drive print_drive);
>> +use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
>> use PVE::QemuServer::Machine;
>> use PVE::QemuServer::Memory;
>> use PVE::QemuServer::Monitor qw(mon_cmd);
>> @@ -3662,11 +3662,7 @@ sub config_to_command {
>> my $drive_cmd = print_drive_commandline_full($storecfg, $vmid, $drive, $pbs_name);
>>
>> # extra protection for templates, but SATA and IDE don't support it..
>> - my $read_only = PVE::QemuConfig->is_template($conf)
>> - && $drive->{interface} ne 'sata'
>> - && $drive->{interface} ne 'ide';
>> -
>> - $drive_cmd .= ',readonly=on' if $read_only;
>> + $drive_cmd .= ',readonly=on' if drive_is_read_only($conf, $drive);
>>
>> push @$devices, '-drive',$drive_cmd;
>> push @$devices, '-device', print_drivedevice_full(
>> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
>> index 146a4ab..0408e32 100644
>> --- a/PVE/QemuServer/Drive.pm
>> +++ b/PVE/QemuServer/Drive.pm
>> @@ -12,6 +12,7 @@ our @EXPORT_OK = qw(
>> is_valid_drivename
>> drive_is_cloudinit
>> drive_is_cdrom
>> +drive_is_read_only
>> parse_drive
>> print_drive
>> );
>> @@ -422,6 +423,15 @@ sub drive_is_cdrom {
>> return $drive && $drive->{media} && ($drive->{media} eq 'cdrom');
>> }
>>
>> +sub drive_is_read_only {
>
> I really don't like this name, this checks if the drive *should* be
> read-only, and only related to template backups, not in general.
yeah, `drive_should_be_read_only` would be more apt, but also sounds
wrong. I did have the non-template case in mind as well (e.g., adding a
'ro' flag to the drive in our VM config as a future addon, like we have
for container mountpoints).
>
> Maybe 'drive_template_read_only'?
>
> The function does two pretty unrelated things in general IMO, so maybe
> it would be clearer to do the is_template check at call site and make
> this 'drive_supports_read_only', even if it causes a little bit more
> duplication.
would work as well. or we drop all of it and no longer mark any drives
as read-only, if we use the patch that adds '-snapshot' for
'start-template-for-backup'? at the risk of re-doing it if we ever add a
'ro' property for individual regular disks/drives..
>
>> + my ($conf, $drive) = @_;
>> +
>> + return 0 if !PVE::QemuConfig->is_template($conf);
>> +
>> + # don't support being marked read-only
>> + return $drive->{interface} ne 'sata' && $drive->{interface} ne 'ide';
>> +}
>> +
>> # ideX = [volume=]volume-id[,media=d][,cyls=c,heads=h,secs=s[,trans=t]]
>> # [,snapshot=on|off][,cache=on|off][,format=f][,backup=yes|no]
>> # [,rerror=ignore|report|stop][,werror=enospc|ignore|report|stop]
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pve-devel] [PATCH qemu-server 2/6] drive: factor out read-only helper
2021-06-07 10:23 ` Fabian Grünbichler
@ 2021-06-07 10:35 ` Stefan Reiter
0 siblings, 0 replies; 15+ messages in thread
From: Stefan Reiter @ 2021-06-07 10:35 UTC (permalink / raw)
To: Fabian Grünbichler, Proxmox VE development discussion
On 6/7/21 12:23 PM, Fabian Grünbichler wrote:
> On June 7, 2021 11:29 am, Stefan Reiter wrote:
>> On 6/4/21 11:47 AM, Fabian Grünbichler wrote:
>>> we also need it for efidisks.
>>>
>>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>>> ---
>>> PVE/QemuServer.pm | 8 ++------
>>> PVE/QemuServer/Drive.pm | 10 ++++++++++
>>> 2 files changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>>> index 25ac052..0d49415 100644
>>> --- a/PVE/QemuServer.pm
>>> +++ b/PVE/QemuServer.pm
>>> @@ -48,7 +48,7 @@ use PVE::QemuServer::Helpers qw(min_version config_aware_timeout);
>>> use PVE::QemuServer::Cloudinit;
>>> use PVE::QemuServer::CGroup;
>>> use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options);
>>> -use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom parse_drive print_drive);
>>> +use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
>>> use PVE::QemuServer::Machine;
>>> use PVE::QemuServer::Memory;
>>> use PVE::QemuServer::Monitor qw(mon_cmd);
>>> @@ -3662,11 +3662,7 @@ sub config_to_command {
>>> my $drive_cmd = print_drive_commandline_full($storecfg, $vmid, $drive, $pbs_name);
>>>
>>> # extra protection for templates, but SATA and IDE don't support it..
>>> - my $read_only = PVE::QemuConfig->is_template($conf)
>>> - && $drive->{interface} ne 'sata'
>>> - && $drive->{interface} ne 'ide';
>>> -
>>> - $drive_cmd .= ',readonly=on' if $read_only;
>>> + $drive_cmd .= ',readonly=on' if drive_is_read_only($conf, $drive);
>>>
>>> push @$devices, '-drive',$drive_cmd;
>>> push @$devices, '-device', print_drivedevice_full(
>>> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
>>> index 146a4ab..0408e32 100644
>>> --- a/PVE/QemuServer/Drive.pm
>>> +++ b/PVE/QemuServer/Drive.pm
>>> @@ -12,6 +12,7 @@ our @EXPORT_OK = qw(
>>> is_valid_drivename
>>> drive_is_cloudinit
>>> drive_is_cdrom
>>> +drive_is_read_only
>>> parse_drive
>>> print_drive
>>> );
>>> @@ -422,6 +423,15 @@ sub drive_is_cdrom {
>>> return $drive && $drive->{media} && ($drive->{media} eq 'cdrom');
>>> }
>>>
>>> +sub drive_is_read_only {
>>
>> I really don't like this name, this checks if the drive *should* be
>> read-only, and only related to template backups, not in general.
>
> yeah, `drive_should_be_read_only` would be more apt, but also sounds
> wrong. I did have the non-template case in mind as well (e.g., adding a
> 'ro' flag to the drive in our VM config as a future addon, like we have
> for container mountpoints).
>
Yes, I actually assumed we could already do that before I looked more
closely when reviewing your patches :)
>>
>> Maybe 'drive_template_read_only'?
>>
>> The function does two pretty unrelated things in general IMO, so maybe
>> it would be clearer to do the is_template check at call site and make
>> this 'drive_supports_read_only', even if it causes a little bit more
>> duplication.
>
> would work as well. or we drop all of it and no longer mark any drives
> as read-only, if we use the patch that adds '-snapshot' for
> 'start-template-for-backup'? at the risk of re-doing it if we ever add a
> 'ro' property for individual regular disks/drives..
>
Hm, I do like the idea of marking them read-only if possible, even if we
pass '-snapshot' - all of this is just preventative anyway, as the guest
is stopped and should never write anything, so at this point might as
well make it in-depth if it's cheap like here. And potentially reuseable
for a ro flag.
>>
>>> + my ($conf, $drive) = @_;
>>> +
>>> + return 0 if !PVE::QemuConfig->is_template($conf);
>>> +
>>> + # don't support being marked read-only
>>> + return $drive->{interface} ne 'sata' && $drive->{interface} ne 'ide';
>>> +}
>>> +
>>> # ideX = [volume=]volume-id[,media=d][,cyls=c,heads=h,secs=s[,trans=t]]
>>> # [,snapshot=on|off][,cache=on|off][,format=f][,backup=yes|no]
>>> # [,rerror=ignore|report|stop][,werror=enospc|ignore|report|stop]
>>>
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pve-devel] [PATCH qemu-server 3/6] template: mark efidisk as read-only
2021-06-04 9:47 [pve-devel] [PATCH qemu-server 0/6] fix #2862: more template backup fixes Fabian Grünbichler
2021-06-04 9:47 ` [pve-devel] [PATCH qemu-server 1/6] test: unbreak restore_config_test Fabian Grünbichler
2021-06-04 9:47 ` [pve-devel] [PATCH qemu-server 2/6] drive: factor out read-only helper Fabian Grünbichler
@ 2021-06-04 9:47 ` Fabian Grünbichler
2021-06-07 9:29 ` Stefan Reiter
2021-06-04 9:47 ` [pve-devel] [PATCH qemu-server 4/6] test: add template drive read-only tests Fabian Grünbichler
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Fabian Grünbichler @ 2021-06-04 9:47 UTC (permalink / raw)
To: pve-devel
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@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' \
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pve-devel] [PATCH qemu-server 3/6] template: mark efidisk as read-only
2021-06-04 9:47 ` [pve-devel] [PATCH qemu-server 3/6] template: mark efidisk as read-only Fabian Grünbichler
@ 2021-06-07 9:29 ` Stefan Reiter
2021-06-07 10:23 ` Fabian Grünbichler
0 siblings, 1 reply; 15+ messages in thread
From: Stefan Reiter @ 2021-06-07 9:29 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
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.
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@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' \
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pve-devel] [PATCH qemu-server 3/6] template: mark efidisk as read-only
2021-06-07 9:29 ` Stefan Reiter
@ 2021-06-07 10:23 ` Fabian Grünbichler
0 siblings, 0 replies; 15+ messages in thread
From: Fabian Grünbichler @ 2021-06-07 10:23 UTC (permalink / raw)
To: Proxmox VE development discussion, Stefan Reiter
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@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' \
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pve-devel] [PATCH qemu-server 4/6] test: add template drive read-only tests
2021-06-04 9:47 [pve-devel] [PATCH qemu-server 0/6] fix #2862: more template backup fixes Fabian Grünbichler
` (2 preceding siblings ...)
2021-06-04 9:47 ` [pve-devel] [PATCH qemu-server 3/6] template: mark efidisk as read-only Fabian Grünbichler
@ 2021-06-04 9:47 ` Fabian Grünbichler
2021-06-04 9:47 ` [pve-devel] [PATCH qemu-server 5/6] template: add -snapshot to KVM command Fabian Grünbichler
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Fabian Grünbichler @ 2021-06-04 9:47 UTC (permalink / raw)
To: pve-devel
ensuring the current behaviour:
templates will pass readonly=on to Qemu, except for SATA and IDE drives
which don't support that flag.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
test/cfg2cmd/efi-raw-template.conf | 5 ++++
test/cfg2cmd/efi-raw-template.conf.cmd | 27 +++++++++++++++++++
test/cfg2cmd/simple1-template.conf | 17 ++++++++++++
test/cfg2cmd/simple1-template.conf.cmd | 36 ++++++++++++++++++++++++++
4 files changed, 85 insertions(+)
create mode 100644 test/cfg2cmd/efi-raw-template.conf
create mode 100644 test/cfg2cmd/efi-raw-template.conf.cmd
create mode 100644 test/cfg2cmd/simple1-template.conf
create mode 100644 test/cfg2cmd/simple1-template.conf.cmd
diff --git a/test/cfg2cmd/efi-raw-template.conf b/test/cfg2cmd/efi-raw-template.conf
new file mode 100644
index 0000000..a181522
--- /dev/null
+++ b/test/cfg2cmd/efi-raw-template.conf
@@ -0,0 +1,5 @@
+# TEST: Test raw efidisk size parameter
+smbios1: uuid=7b10d7af-b932-4c66-b2c3-3996152ec465
+bios: ovmf
+efidisk0: local:100/base-disk-100-0.raw
+template: 1
diff --git a/test/cfg2cmd/efi-raw-template.conf.cmd b/test/cfg2cmd/efi-raw-template.conf.cmd
new file mode 100644
index 0000000..636f03e
--- /dev/null
+++ b/test/cfg2cmd/efi-raw-template.conf.cmd
@@ -0,0 +1,27 @@
+/usr/bin/kvm \
+ -id 8006 \
+ -name vm8006 \
+ -no-shutdown \
+ -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+ -mon 'chardev=qmp,mode=control' \
+ -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+ -mon 'chardev=qmp-event,mode=control' \
+ -pidfile /var/run/qemu-server/8006.pid \
+ -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/base-disk-100-0.raw,readonly=on' \
+ -smp '1,sockets=1,cores=1,maxcpus=1' \
+ -nodefaults \
+ -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+ -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \
+ -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+ -m 512 \
+ -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
+ -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
+ -device 'piix3-usb-uhci,id=uhci,bus=pci.0,addr=0x1.0x2' \
+ -device 'usb-tablet,id=tablet,bus=uhci.0,port=1' \
+ -device 'VGA,id=vga,bus=pci.0,addr=0x2' \
+ -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \
+ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+ -machine 'type=pc+pve0'
diff --git a/test/cfg2cmd/simple1-template.conf b/test/cfg2cmd/simple1-template.conf
new file mode 100644
index 0000000..c26de40
--- /dev/null
+++ b/test/cfg2cmd/simple1-template.conf
@@ -0,0 +1,17 @@
+# TEST: Simple test for a basic template configuration
+# QEMU_VERSION: 3.0
+bootdisk: scsi0
+cores: 3
+ide2: none,media=cdrom
+memory: 768
+name: simple
+net0: virtio=A2:C0:43:77:08:A0,bridge=vmbr0
+numa: 0
+ostype: l26
+sata0: local:8006/base-8006-disk-0.qcow2,discard=on,size=104858K
+scsi0: local:8006/base-8006-disk-1.qcow2,discard=on,size=104858K
+scsihw: virtio-scsi-pci
+smbios1: uuid=7b10d7af-b932-4c66-b2c3-3996152ec465
+sockets: 1
+template: 1
+vmgenid: c773c261-d800-4348-9f5d-167fadd53cf8
diff --git a/test/cfg2cmd/simple1-template.conf.cmd b/test/cfg2cmd/simple1-template.conf.cmd
new file mode 100644
index 0000000..20a6dcf
--- /dev/null
+++ b/test/cfg2cmd/simple1-template.conf.cmd
@@ -0,0 +1,36 @@
+/usr/bin/kvm \
+ -id 8006 \
+ -name simple \
+ -no-shutdown \
+ -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+ -mon 'chardev=qmp,mode=control' \
+ -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+ -mon 'chardev=qmp-event,mode=control' \
+ -pidfile /var/run/qemu-server/8006.pid \
+ -daemonize \
+ -smbios 'type=1,uuid=7b10d7af-b932-4c66-b2c3-3996152ec465' \
+ -smp '3,sockets=1,cores=3,maxcpus=3' \
+ -nodefaults \
+ -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+ -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \
+ -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+ -m 768 \
+ -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
+ -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
+ -device 'vmgenid,guid=c773c261-d800-4348-9f5d-167fadd53cf8' \
+ -device 'piix3-usb-uhci,id=uhci,bus=pci.0,addr=0x1.0x2' \
+ -device 'usb-tablet,id=tablet,bus=uhci.0,port=1' \
+ -device 'VGA,id=vga,bus=pci.0,addr=0x2' \
+ -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \
+ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+ -drive 'if=none,id=drive-ide2,media=cdrom,aio=threads' \
+ -device 'ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2,bootindex=200' \
+ -device 'virtio-scsi-pci,id=scsihw0,bus=pci.0,addr=0x5' \
+ -drive 'file=/var/lib/vz/images/8006/base-8006-disk-1.qcow2,if=none,id=drive-scsi0,discard=on,format=qcow2,cache=none,aio=native,detect-zeroes=unmap,readonly=on' \
+ -device 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=100' \
+ -device 'ahci,id=ahci0,multifunction=on,bus=pci.0,addr=0x7'
+ -drive 'file=/var/lib/vz/images/8006/base-8006-disk-0.qcow2,if=none,id=drive-sata0,discard=on,format=qcow2,cache=none,aio=native,detect-zeroes=unmap'
+ -device 'ide-hd,bus=ahci0.0,drive=drive-sata0,id=sata0'
+ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \
+ -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \
+ -machine 'type=pc'
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [pve-devel] [PATCH qemu-server 5/6] template: add -snapshot to KVM command
2021-06-04 9:47 [pve-devel] [PATCH qemu-server 0/6] fix #2862: more template backup fixes Fabian Grünbichler
` (3 preceding siblings ...)
2021-06-04 9:47 ` [pve-devel] [PATCH qemu-server 4/6] test: add template drive read-only tests Fabian Grünbichler
@ 2021-06-04 9:47 ` Fabian Grünbichler
2021-06-04 9:47 ` [pve-devel] [RFC qemu-server 6/6] template: start VM for VMA backup Fabian Grünbichler
2021-06-23 10:50 ` [pve-devel] partially-applied: [PATCH qemu-server 0/6] fix #2862: more template backup fixes Thomas Lamprecht
6 siblings, 0 replies; 15+ messages in thread
From: Fabian Grünbichler @ 2021-06-04 9:47 UTC (permalink / raw)
To: pve-devel
this allows effectively setting ALL volumes as read-only, even if the
disk controller does not support it. without it, IDE and SATA disks
with (base) volumes which are marked read-only/immutable on the storage
level prevent the template VM from starting for backup purposes.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
PVE/QemuServer.pm | 5 +++++
test/cfg2cmd/efi-raw-template.conf.cmd | 3 ++-
test/cfg2cmd/simple1-template.conf.cmd | 3 ++-
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 3d996af..ce392f8 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3765,6 +3765,11 @@ sub config_to_command {
print "activating and using '$vmstate' as vmstate\n";
}
+ if (PVE::QemuConfig->is_template($conf)) {
+ # needed to workaround base volumes being read-only
+ push @$cmd, '-snapshot';
+ }
+
# add custom args
if ($conf->{args}) {
my $aa = PVE::Tools::split_args($conf->{args});
diff --git a/test/cfg2cmd/efi-raw-template.conf.cmd b/test/cfg2cmd/efi-raw-template.conf.cmd
index 636f03e..d99862b 100644
--- a/test/cfg2cmd/efi-raw-template.conf.cmd
+++ b/test/cfg2cmd/efi-raw-template.conf.cmd
@@ -24,4 +24,5 @@
-device 'VGA,id=vga,bus=pci.0,addr=0x2' \
-device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \
-iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
- -machine 'type=pc+pve0'
+ -machine 'type=pc+pve0' \
+ -snapshot
diff --git a/test/cfg2cmd/simple1-template.conf.cmd b/test/cfg2cmd/simple1-template.conf.cmd
index 20a6dcf..7fb6fd1 100644
--- a/test/cfg2cmd/simple1-template.conf.cmd
+++ b/test/cfg2cmd/simple1-template.conf.cmd
@@ -33,4 +33,5 @@
-device 'ide-hd,bus=ahci0.0,drive=drive-sata0,id=sata0'
-netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \
-device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \
- -machine 'type=pc'
+ -machine 'type=pc' \
+ -snapshot
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [pve-devel] [RFC qemu-server 6/6] template: start VM for VMA backup
2021-06-04 9:47 [pve-devel] [PATCH qemu-server 0/6] fix #2862: more template backup fixes Fabian Grünbichler
` (4 preceding siblings ...)
2021-06-04 9:47 ` [pve-devel] [PATCH qemu-server 5/6] template: add -snapshot to KVM command Fabian Grünbichler
@ 2021-06-04 9:47 ` Fabian Grünbichler
2021-06-07 9:29 ` Stefan Reiter
2021-06-23 10:50 ` [pve-devel] partially-applied: [PATCH qemu-server 0/6] fix #2862: more template backup fixes Thomas Lamprecht
6 siblings, 1 reply; 15+ messages in thread
From: Fabian Grünbichler @ 2021-06-04 9:47 UTC (permalink / raw)
To: pve-devel
since using 'vma create ..' no longer works with immutable/read-only
base volumes.
first hunk drops a leftover variable from when we did the same change to
the PBS code path.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
this mimics what we already did for PBS, we still might want to think
about limiting memory for the started VM for template backups which
can't be resumed into operation anyway..
PVE/VZDump/QemuServer.pm | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 44b705f..9ac77c1 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -446,7 +446,6 @@ sub archive_pbs {
# proxmox-backup-client can only handle raw files and block devs
# only use it (directly) for disk-less VMs
if (!$diskcount) {
- my @pathlist;
$self->loginfo("backup contains no disks");
local $ENV{PBS_PASSWORD} = $password;
@@ -621,19 +620,8 @@ sub archive_vma {
}
my $diskcount = scalar(@{$task->{disks}});
- if (PVE::QemuConfig->is_template($self->{vmlist}->{$vmid}) || !$diskcount) {
- my @pathlist;
- foreach my $di (@{$task->{disks}}) {
- if ($di->{type} eq 'block' || $di->{type} eq 'file') {
- push @pathlist, "$di->{qmdevice}=$di->{path}";
- } else {
- die "implement me";
- }
- }
-
- if (!$diskcount) {
- $self->loginfo("backup contains no disks");
- }
+ if (!$diskcount) {
+ $self->loginfo("backup doesn't contain any disks");
my $outcmd;
if ($comp) {
@@ -646,9 +634,9 @@ sub archive_vma {
my $cmd = ['/usr/bin/vma', 'create', '-v', '-c', $conffile];
push @$cmd, '-c', $firewall if -e $firewall;
- push @$cmd, $outcmd, @pathlist;
+ push @$cmd, $outcmd;
- $self->loginfo("starting template backup");
+ $self->loginfo("starting backup");
$self->loginfo(join(' ', @$cmd));
if ($opts->{stdout}) {
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pve-devel] [RFC qemu-server 6/6] template: start VM for VMA backup
2021-06-04 9:47 ` [pve-devel] [RFC qemu-server 6/6] template: start VM for VMA backup Fabian Grünbichler
@ 2021-06-07 9:29 ` Stefan Reiter
2021-06-07 10:22 ` Fabian Grünbichler
0 siblings, 1 reply; 15+ messages in thread
From: Stefan Reiter @ 2021-06-07 9:29 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
On 6/4/21 11:47 AM, Fabian Grünbichler wrote:
> since using 'vma create ..' no longer works with immutable/read-only
> base volumes.
Why that? It shouldn't matter to vma if the base is read-only? Though I
do see that in vma.c we don't tell QEMU that it should open the block
backend read-only...
Anyway, I do personally think that just starting a VM is also a good way
of doing this, though as you say, for templates it would make a lot of
sense to restrict the VM to use less resources.
>
> first hunk drops a leftover variable from when we did the same change to
> the PBS code path.
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> this mimics what we already did for PBS, we still might want to think
> about limiting memory for the started VM for template backups which
> can't be resumed into operation anyway..
>
> PVE/VZDump/QemuServer.pm | 20 ++++----------------
> 1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
> index 44b705f..9ac77c1 100644
> --- a/PVE/VZDump/QemuServer.pm
> +++ b/PVE/VZDump/QemuServer.pm
> @@ -446,7 +446,6 @@ sub archive_pbs {
> # proxmox-backup-client can only handle raw files and block devs
> # only use it (directly) for disk-less VMs
> if (!$diskcount) {
> - my @pathlist;
> $self->loginfo("backup contains no disks");
>
> local $ENV{PBS_PASSWORD} = $password;
> @@ -621,19 +620,8 @@ sub archive_vma {
> }
>
> my $diskcount = scalar(@{$task->{disks}});
> - if (PVE::QemuConfig->is_template($self->{vmlist}->{$vmid}) || !$diskcount) {
> - my @pathlist;
> - foreach my $di (@{$task->{disks}}) {
> - if ($di->{type} eq 'block' || $di->{type} eq 'file') {
> - push @pathlist, "$di->{qmdevice}=$di->{path}";
> - } else {
> - die "implement me";
> - }
> - }
> -
> - if (!$diskcount) {
> - $self->loginfo("backup contains no disks");
> - }
> + if (!$diskcount) {
> + $self->loginfo("backup doesn't contain any disks");
nit: I'd leave the "backup contains no disks" message the same
>
> my $outcmd;
> if ($comp) {
> @@ -646,9 +634,9 @@ sub archive_vma {
>
> my $cmd = ['/usr/bin/vma', 'create', '-v', '-c', $conffile];
> push @$cmd, '-c', $firewall if -e $firewall;
> - push @$cmd, $outcmd, @pathlist;
> + push @$cmd, $outcmd;
>
> - $self->loginfo("starting template backup");
> + $self->loginfo("starting backup");
> $self->loginfo(join(' ', @$cmd));
>
> if ($opts->{stdout}) {
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pve-devel] [RFC qemu-server 6/6] template: start VM for VMA backup
2021-06-07 9:29 ` Stefan Reiter
@ 2021-06-07 10:22 ` Fabian Grünbichler
0 siblings, 0 replies; 15+ messages in thread
From: Fabian Grünbichler @ 2021-06-07 10:22 UTC (permalink / raw)
To: Proxmox VE development discussion, Stefan Reiter
On June 7, 2021 11:29 am, Stefan Reiter wrote:
> On 6/4/21 11:47 AM, Fabian Grünbichler wrote:
>> since using 'vma create ..' no longer works with immutable/read-only
>> base volumes.
>
> Why that? It shouldn't matter to vma if the base is read-only? Though I
> do see that in vma.c we don't tell QEMU that it should open the block
> backend read-only...
I have not checked in-depth on the Qemu side what changed, but
alternatively we could fix the vma binary to mark as read-only like the
rest of the series and past patches did for the regular startup for
backup purposes.
> Anyway, I do personally think that just starting a VM is also a good way
> of doing this, though as you say, for templates it would make a lot of
> sense to restrict the VM to use less resources.
yeah, I am on the fence (hence the RFC) - on the one hand, it would be
more consistent with PBSp-backups to just start the VM, OTOH we'd save
on memory usage/.. if we fix vma.
>
>>
>> first hunk drops a leftover variable from when we did the same change to
>> the PBS code path.
>>
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>> ---
>> this mimics what we already did for PBS, we still might want to think
>> about limiting memory for the started VM for template backups which
>> can't be resumed into operation anyway..
>>
>> PVE/VZDump/QemuServer.pm | 20 ++++----------------
>> 1 file changed, 4 insertions(+), 16 deletions(-)
>>
>> diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
>> index 44b705f..9ac77c1 100644
>> --- a/PVE/VZDump/QemuServer.pm
>> +++ b/PVE/VZDump/QemuServer.pm
>> @@ -446,7 +446,6 @@ sub archive_pbs {
>> # proxmox-backup-client can only handle raw files and block devs
>> # only use it (directly) for disk-less VMs
>> if (!$diskcount) {
>> - my @pathlist;
>> $self->loginfo("backup contains no disks");
>>
>> local $ENV{PBS_PASSWORD} = $password;
>> @@ -621,19 +620,8 @@ sub archive_vma {
>> }
>>
>> my $diskcount = scalar(@{$task->{disks}});
>> - if (PVE::QemuConfig->is_template($self->{vmlist}->{$vmid}) || !$diskcount) {
>> - my @pathlist;
>> - foreach my $di (@{$task->{disks}}) {
>> - if ($di->{type} eq 'block' || $di->{type} eq 'file') {
>> - push @pathlist, "$di->{qmdevice}=$di->{path}";
>> - } else {
>> - die "implement me";
>> - }
>> - }
>> -
>> - if (!$diskcount) {
>> - $self->loginfo("backup contains no disks");
>> - }
>> + if (!$diskcount) {
>> + $self->loginfo("backup doesn't contain any disks");
>
> nit: I'd leave the "backup contains no disks" message the same
>
>>
>> my $outcmd;
>> if ($comp) {
>> @@ -646,9 +634,9 @@ sub archive_vma {
>>
>> my $cmd = ['/usr/bin/vma', 'create', '-v', '-c', $conffile];
>> push @$cmd, '-c', $firewall if -e $firewall;
>> - push @$cmd, $outcmd, @pathlist;
>> + push @$cmd, $outcmd;
>>
>> - $self->loginfo("starting template backup");
>> + $self->loginfo("starting backup");
>> $self->loginfo(join(' ', @$cmd));
>>
>> if ($opts->{stdout}) {
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pve-devel] partially-applied: [PATCH qemu-server 0/6] fix #2862: more template backup fixes
2021-06-04 9:47 [pve-devel] [PATCH qemu-server 0/6] fix #2862: more template backup fixes Fabian Grünbichler
` (5 preceding siblings ...)
2021-06-04 9:47 ` [pve-devel] [RFC qemu-server 6/6] template: start VM for VMA backup Fabian Grünbichler
@ 2021-06-23 10:50 ` Thomas Lamprecht
6 siblings, 0 replies; 15+ messages in thread
From: Thomas Lamprecht @ 2021-06-23 10:50 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
On 04.06.21 11:47, Fabian Grünbichler wrote:
> first one is unrelated, rest should fix the remaining cases where
> template backups to PBS fail.
>
> Fabian Grünbichler (6):
> test: unbreak restore_config_test
> drive: factor out read-only helper
> template: mark efidisk as read-only
> test: add template drive read-only tests
> template: add -snapshot to KVM command
applied above, i.e., all but the RFC below. I adapted the EFI-disk one to what
Stefan proposed, i.e., just drop the readonly if off. There also was the need to
solve a merge conflict, so I just squashed it all into that one.
> template: start VM for VMA backup
^ permalink raw reply [flat|nested] 15+ messages in thread