[pve-devel] [PATCH qemu-server] live-restore: preload efidisk before starting VM

[pve-devel] [PATCH qemu-server] live-restore: preload efidisk before starting VM

[Stefan Reiter]

The efidisk never got restored correctly before, since we don't use the
generic print_drive_commandline_full for it, and as such it didn't get a
backing image attached. This not only causes the efidisk data to be lost
on restore, but also an error at the end, since we try to remove a
non-existing PBS blockdev.

Since it is attached differently to a regular drive, adding PBS backing
would be more difficult, but not to worry: an efidisk is small enough
that it doesn't hurt performance to just restore it via the regular
mechanism before starting the VM, and simply excluding it from the live
restore entirely.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---

best viewed with -w

I guess in QEMU < 6.0 this didn't cause an error, since I definitely tested with
EFI VMs before, however, it would have never actually restored the data and
instead always created a new one.

 PVE/QemuServer.pm | 46 ++++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index f760467..ce5a1b1 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6365,35 +6365,36 @@ sub restore_proxmox_backup_archive {
 	# allocate volumes
 	my $map = $restore_allocate_devices->($storecfg, $virtdev_hash, $vmid);
 
-	if (!$options->{live}) {
-	    foreach my $virtdev (sort keys %$virtdev_hash) {
-		my $d = $virtdev_hash->{$virtdev};
-		next if $d->{is_cloudinit}; # no need to restore cloudinit
+	foreach my $virtdev (sort keys %$virtdev_hash) {
+	    my $d = $virtdev_hash->{$virtdev};
+	    next if $d->{is_cloudinit}; # no need to restore cloudinit
 
-		my $volid = $d->{volid};
+	    # for live-restore we only want to preload the efidisk
+	    next if $options->{live} && $virtdev ne 'efidisk0';
 
-		my $path = PVE::Storage::path($storecfg, $volid);
+	    my $volid = $d->{volid};
 
-		my $pbs_restore_cmd = [
-		    '/usr/bin/pbs-restore',
-		    '--repository', $repo,
-		    $pbs_backup_name,
-		    "$d->{devname}.img.fidx",
-		    $path,
-		    '--verbose',
-		    ];
+	    my $path = PVE::Storage::path($storecfg, $volid);
 
-		push @$pbs_restore_cmd, '--format', $d->{format} if $d->{format};
-		push @$pbs_restore_cmd, '--keyfile', $keyfile if -e $keyfile;
+	    my $pbs_restore_cmd = [
+		'/usr/bin/pbs-restore',
+		'--repository', $repo,
+		$pbs_backup_name,
+		"$d->{devname}.img.fidx",
+		$path,
+		'--verbose',
+		];
 
-		if (PVE::Storage::volume_has_feature($storecfg, 'sparseinit', $volid)) {
-		    push @$pbs_restore_cmd, '--skip-zero';
-		}
+	    push @$pbs_restore_cmd, '--format', $d->{format} if $d->{format};
+	    push @$pbs_restore_cmd, '--keyfile', $keyfile if -e $keyfile;
 
-		my $dbg_cmdstring = PVE::Tools::cmd2string($pbs_restore_cmd);
-		print "restore proxmox backup image: $dbg_cmdstring\n";
-		run_command($pbs_restore_cmd);
+	    if (PVE::Storage::volume_has_feature($storecfg, 'sparseinit', $volid)) {
+		push @$pbs_restore_cmd, '--skip-zero';
 	    }
+
+	    my $dbg_cmdstring = PVE::Tools::cmd2string($pbs_restore_cmd);
+	    print "restore proxmox backup image: $dbg_cmdstring\n";
+	    run_command($pbs_restore_cmd);
 	}
 
 	$fh->seek(0, 0) || die "seek failed - $!\n";
@@ -6448,6 +6449,7 @@ sub restore_proxmox_backup_archive {
 	my $conf = PVE::QemuConfig->load_config($vmid);
 	die "cannot do live-restore for template\n" if PVE::QemuConfig->is_template($conf);
 
+	delete $devinfo->{'drive-efidisk0'};
 	pbs_live_restore($vmid, $conf, $storecfg, $devinfo, $repo, $keyfile, $pbs_backup_name);
 
 	PVE::QemuConfig->remove_lock($vmid, "create");
-- 
2.30.2

[pve-devel] applied: [PATCH qemu-server] live-restore: preload efidisk before starting VM

[Thomas Lamprecht]

On 01.07.21 11:37, Stefan Reiter wrote:
> The efidisk never got restored correctly before, since we don't use the
> generic print_drive_commandline_full for it, and as such it didn't get a
> backing image attached. This not only causes the efidisk data to be lost
> on restore, but also an error at the end, since we try to remove a
> non-existing PBS blockdev.
> 
> Since it is attached differently to a regular drive, adding PBS backing
> would be more difficult, but not to worry: an efidisk is small enough
> that it doesn't hurt performance to just restore it via the regular
> mechanism before starting the VM, and simply excluding it from the live
> restore entirely.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> 
> best viewed with -w
> 
> I guess in QEMU < 6.0 this didn't cause an error, since I definitely tested with
> EFI VMs before, however, it would have never actually restored the data and
> instead always created a new one.

could still be good to backport, I'd figure?

> 
>  PVE/QemuServer.pm | 46 ++++++++++++++++++++++++----------------------
>  1 file changed, 24 insertions(+), 22 deletions(-)
> 
>

applied, thanks!

Re: [pve-devel] applied: [PATCH qemu-server] live-restore: preload efidisk before starting VM

[Stefan Reiter]

On 7/1/21 12:57 PM, Thomas Lamprecht wrote:
> On 01.07.21 11:37, Stefan Reiter wrote:
>> The efidisk never got restored correctly before, since we don't use the
>> generic print_drive_commandline_full for it, and as such it didn't get a
>> backing image attached. This not only causes the efidisk data to be lost
>> on restore, but also an error at the end, since we try to remove a
>> non-existing PBS blockdev.
>>
>> Since it is attached differently to a regular drive, adding PBS backing
>> would be more difficult, but not to worry: an efidisk is small enough
>> that it doesn't hurt performance to just restore it via the regular
>> mechanism before starting the VM, and simply excluding it from the live
>> restore entirely.
>>
>> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
>> ---
>>
>> best viewed with -w
>>
>> I guess in QEMU < 6.0 this didn't cause an error, since I definitely tested with
>> EFI VMs before, however, it would have never actually restored the data and
>> instead always created a new one.
> 
> could still be good to backport, I'd figure?
> 

certainly, this should apply cleanly to 6 too I reckon?

>>
>>   PVE/QemuServer.pm | 46 ++++++++++++++++++++++++----------------------
>>   1 file changed, 24 insertions(+), 22 deletions(-)
>>
>>
> 
> applied, thanks!
>