[pve-devel] [PATCH v3 qemu-server] vzdump: move include logic for volumes to method

[Aaron Lauterer]

Move the logic which volumes are included in the backup job to its own
method and adapt the VZDump code accordingly. This makes it possible to
develop other features around backup jobs.

Signed-off-by: Aaron Lauterer <a.lauterer at proxmox.com>
---

v2 -> v3: rebased

v1 -> v2:
* implemented the suggestions from Fabian [0]
* incorporated changes that happened in the meantime, most notably the
  check for efidisk without omvf bios set

I decided to keep the check for IOThreaded VMs where it is because it
will only trigger if the backup is run with an older qemu version. By
the time this patch series gets applied and shipped I think it unlikely
that this will actually be triggered anymore.

There also seems to be a problem with the way the IFs are nested in that
section. I sent in separate small patch to fix it [1]. I wasn't sure if
I should have implemented that patch here as well. Depending on which
patch gets applied first, the other will need some rebasing.

[0] https://pve.proxmox.com/pipermail/pve-devel/2020-January/041360.html
[1] https://pve.proxmox.com/pipermail/pve-devel/2020-February/041968.html

 PVE/QemuConfig.pm        | 31 +++++++++++++++++++++++++++++++
 PVE/VZDump/QemuServer.pm | 38 +++++++++++++++++++-------------------
 2 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 1ba728a..f5b737c 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -130,6 +130,37 @@ sub get_replicatable_volumes {
     return $volhash;
 }
 
+sub get_backup_volumes {
+    my ($class, $conf) = @_;
+
+    my $ret_volumes = [];
+
+    my $test_volume = sub {
+	my ($key, $drive) = @_;
+
+	return if PVE::QemuServer::drive_is_cdrom($drive);
+
+	my $volume = {};
+	my $included = $drive->{backup} // 1;;
+	my $reason = defined($drive->{backup}) ? 'disabled' : 'enabled';
+
+	if ($key =~ m/^efidisk/ && (!defined($conf->{bios}) || $conf->{bios} ne 'ovmf')) {
+	    $included = 0;
+	    $reason = "efidisk with non omvf bios";
+	}
+	$volume->{key} = $key;
+	$volume->{included} = $included;
+	$volume->{reason} = $reason;
+	$volume->{data} = $drive;
+
+	push @$ret_volumes, $volume;
+    };
+
+    PVE::QemuServer::foreach_drive($conf, $test_volume);
+
+    return $ret_volumes;
+}
+
 sub __snapshot_save_vmstate {
     my ($class, $vmid, $conf, $snapname, $storecfg, $statestorage, $suspend) = @_;
 
diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 7695ad6..38471de 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -69,37 +69,37 @@ sub prepare {
 
     my $vollist = [];
     my $drivehash = {};
-    PVE::QemuServer::foreach_drive($conf, sub {
-	my ($ds, $drive) = @_;
+    my $backup_included = PVE::QemuConfig->get_backup_volumes($conf);
 
-	return if PVE::QemuServer::drive_is_cdrom($drive);
+    foreach my $volume(@{$backup_included}) {
+	my $volid = $volume->{data}->{file};
+	my $name = $volume->{key};
 
-	my $volid = $drive->{file};
-
-	if (defined($drive->{backup}) && !$drive->{backup}) {
-	    $self->loginfo("exclude disk '$ds' '$volid' (backup=no)");
-	    return;
-	} elsif ($self->{vm_was_running} && $drive->{iothread}) {
+	if (!$volume->{included}) {
+	    if ($volume->{reason} eq 'efidisk with non omvf bios') {
+		$self->loginfo("excluding '$name' (efidisks can only be backed up when BIOS is set to 'ovmf')");
+		next;
+	    }
+	    $self->loginfo("exclude disk '$name' '$volid' (backup=no)");
+	    next;
+	} elsif ($self->{vm_was_running} && $volume->{data}->{iothread}) {
 	    if (!PVE::QemuServer::Machine::runs_at_least_qemu_version($vmid, 4, 0, 1)) {
-		die "disk '$ds' '$volid' (iothread=on) can't use backup feature with running QEMU " .
+		die "disk '$name' '$volid' (iothread=on) can't use backup feature with running QEMU " .
 		    "version < 4.0.1! Either set backup=no for this drive or upgrade QEMU and restart VM\n";
 	    }
-	} elsif ($ds =~ m/^efidisk/ && (!defined($conf->{bios}) || $conf->{bios} ne 'ovmf')) {
-	    $self->loginfo("excluding '$ds' (efidisks can only be backed up when BIOS is set to 'ovmf')");
-	    return;
 	} else {
-	    my $log = "include disk '$ds' '$volid'";
-	   if (defined $drive->{size}) {
-		my $readable_size = PVE::JSONSchema::format_size($drive->{size});
+	    my $log = "include disk '$name' '$volid'";
+	    if (defined $volume->{data}->{size}) {
+		my $readable_size = PVE::JSONSchema::format_size($volume->{data}->{size});
 		$log .= " $readable_size";
-	   }
+	    }
 	    $self->loginfo($log);
 	}
 
 	my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
 	push @$vollist, $volid if $storeid;
-	$drivehash->{$ds} = $drive;
-    });
+	$drivehash->{$name} = $volume->{data};
+    }
 
     PVE::Storage::activate_volumes($self->{storecfg}, $vollist);
 
-- 
2.20.1