On 6/5/20 8:43 PM, Thomas Lamprecht wrote:
On 5/6/20 11:57 AM, Aaron Lauterer wrote:
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.laute...@proxmox.com>
---
v4->v5:
* use new foreach_volume call
* change $ret_volumes to $return_volumes
* use dedicated variables in PVE::VZDump::QemuServer where hash is used
more than once
* renamed $backup_included and other variables to (IMHO) better names
v3->v4: changed function call for `foreach_drive` to QemuServer::Drive
This can be changed to `foreach_volume` once the patches by Fabian Ebner
are through.
is that now in?
yes, see v4->v5. Or do you mean something else?
Looks OK, one not to important style comment below.
PVE/QemuConfig.pm | 31 ++++++++++++++++++++++++++++
PVE/VZDump/QemuServer.pm | 44 +++++++++++++++++++++-------------------
2 files changed, 54 insertions(+), 21 deletions(-)
diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 240fc06..dac913f 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -165,6 +165,37 @@ sub get_replicatable_volumes {
return $volhash;
}
+sub get_backup_volumes {
+ my ($class, $conf) = @_;
+
+ my $return_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 @$return_volumes, $volume;
Not sure if I did already comment this on an earlier revision, but personally
I'd prefer
something like:
push @$return_volumes, {
key => $key,
included => $included,
...
};
(or at least save the whole info to my $volume directly on declaration and then
push)
There was some comment in an earlier revision at this point but I think
about something different.
I do see your point, will be changed in the next revision.
+ };
+
+ PVE::QemuConfig->foreach_volume($conf, $test_volume);
+
+ return $return_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 f122539..480979c 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -69,37 +69,39 @@ sub prepare {
my $vollist = [];
my $drivehash = {};
- PVE::QemuConfig->foreach_volume($conf, sub {
- my ($ds, $drive) = @_;
-
- return if PVE::QemuServer::drive_is_cdrom($drive);
-
- 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}) {
+ my $backup_volumes = PVE::QemuConfig->get_backup_volumes($conf);
+
+ foreach my $volume (@{$backup_volumes}) {
+ my $name = $volume->{key};
+ my $data = $volume->{data};
+ my $volid = $data->{file};
+ my $size = $data->{size};
+
+ 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} && $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 $size) {
+ my $readable_size = PVE::JSONSchema::format_size($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);
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
