[pve-devel] [PATCH v3 qemu-server] vzdump: move include logic for volumes to method
Fabian Ebner
f.ebner at proxmox.com
Tue Mar 24 10:16:40 CET 2020
On 17.03.20 15:34, Fabian Grünbichler wrote:
> On March 16, 2020 4:44 pm, 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.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);
>
> nit: this is in PVE::QemuServer::Drive now, and we don't want to add new
> dependencies from QemuConfig to QemuServer.
>
Once it's available, the new abstract foreach_volume (will be part of
QemuConfig.pm via AbstractConfig.pm) can also be used here. I'll send
the next version of the series[0] later this week and hopefully it's in
good enough shape to be applied.
[0]: https://pve.proxmox.com/pipermail/pve-devel/2020-March/042324.html
>> +
>> + 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
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel at pve.proxmox.com
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
More information about the pve-devel
mailing list