From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id 525BD1FF168
	for <inbox@lore.proxmox.com>; Tue,  4 Mar 2025 11:46:06 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 9BE141B7D4;
	Tue,  4 Mar 2025 11:44:57 +0100 (CET)
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Tue,  4 Mar 2025 11:44:09 +0100
Message-Id: <20250304104413.38638-9-f.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250304104413.38638-1-f.ebner@proxmox.com>
References: <20250304104413.38638-1-f.ebner@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.043 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 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: [pve-devel] [PATCH qemu-server 08/12] backup restore: improve
 missing drive handling
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>

Currently, the configuration line for a drive that is marked for
backup (i.e. 'backup' flag not explicitly set to 0), but missing from
the restore map will be added verbatim to the restored configuration.

As reported in the community forum [0], this can cause issues with a
VM with a configured EFI disk, but not using OVMF BIOS. In such a
case, the EFI disk is not backed up, see the get_backup_volumes()
helper in QemuConfig.pm.

Writing out references to non-existent volumes to the restored config
will lead to issues starting the VM. Write such references as comments
instead, like is already done for volumes explicitly excluded from
backup. But add a warning since this is not expected, and info for the
EFI disk case.

Note that drives using absolute paths are either also backed-up and
will be allocated as new volumes during restore or explicitly marked
as excluded.

Continue preserving configuration lines for CD-ROMs. No CD-ROMs were
included in backups before commit "backup: include owned CD-ROM
volumes" and non-owned CD-ROM and cloud-init drives continue to be
excluded. Skipping owned CD-ROMs that are missing from the backup here
too (and maybe even those owned by another VM?) could be done in the
future, but might be considered a breaking change. Currently, the old
VM ID is also not readily available for such a check.

[0]: https://forum.proxmox.com/threads/160483/

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm                     | 12 +++++++++++-
 test/restore-config-expected/140.conf |  4 ++--
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 454ee64a..f937b1d7 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6714,6 +6714,7 @@ sub restore_update_config_line {
 	$res .= "$id: $netstr\n";
     } elsif ($line =~ m/^((ide|scsi|virtio|sata|efidisk|tpmstate)\d+):\s*(\S+)\s*$/) {
 	my $virtdev = $1;
+	my $interface = $2;
 	my $value = $3;
 	my $di = parse_drive($virtdev, $value);
 	if (defined($di->{backup}) && !$di->{backup}) {
@@ -6723,8 +6724,17 @@ sub restore_update_config_line {
 	    $di->{file} = $map->{$virtdev};
 	    $value = print_drive($di);
 	    $res .= "$virtdev: $value\n";
-	} else {
+	} elsif (PVE::QemuServer::Drive::drive_is_cdrom($di)) {
+	    # TODO PVE 9 - skip owned (either by this or also other VM?) CD-ROMs that are not in
+	    # backup (except cloud-init)
 	    $res .= $line;
+	} else {
+	    if ($interface eq 'efidisk') {
+		print "$virtdev: not part of the backup - SeaBIOS configured?\n";
+	    } else {
+		log_warn("$virtdev: not part of the backup");
+	    }
+	    $res .= "#$line";
 	}
     } elsif (($line =~ m/^vmgenid: (.*)/)) {
 	my $vmgenid = $1;
diff --git a/test/restore-config-expected/140.conf b/test/restore-config-expected/140.conf
index af1a4d59..33ba37a7 100644
--- a/test/restore-config-expected/140.conf
+++ b/test/restore-config-expected/140.conf
@@ -2,7 +2,7 @@
 bios: seabios
 boot: order=scsi0;ide2;net0
 cores: 1
-efidisk0: mydir:140/vm-140-disk-0.qcow2,size=128K
+#efidisk0: mydir:140/vm-140-disk-0.qcow2,size=128K
 ide2: local:iso/debian-10.6.0-amd64-netinst.iso,media=cdrom
 memory: 2048
 name: eficloneclone
@@ -10,7 +10,7 @@ net0: virtio=7A:6C:A5:8B:11:93,bridge=vmbr0,firewall=1
 numa: 0
 ostype: l26
 scsi0: target:140/vm-140-disk-0.raw,size=4G
-scsi1: rbdkvm:vm-140-disk-2,size=1G
+#scsi1: rbdkvm:vm-140-disk-2,size=1G
 scsihw: virtio-scsi-pci
 smbios1: uuid=21a7e7bc-3cd2-4232-a009-a41f4ee992ae
 sockets: 1
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel