On 2/5/20 10:29 AM, Fabian Grünbichler wrote:
On January 29, 2020 2:29 pm, Fabian Ebner wrote:
This function is intened to be used after doing a migration where some
of the volume IDs changed.
Signed-off-by: Fabian Ebner <f.eb...@proxmox.com>
---
PVE/AbstractConfig.pm | 61 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm
index a94a379..fb833cb 100644
--- a/PVE/AbstractConfig.pm
+++ b/PVE/AbstractConfig.pm
@@ -366,6 +366,67 @@ sub get_replicatable_volumes {
die "implement me - abstract method\n";
}
+sub foreach_volume {
+ my ($class, $conf, $func, @param) = @_;
+
+ die "abstract method - implement me\n";
+}
+
+sub print_volume {
+ my ($class, $volume) = @_;
+
+ die "abstract method - implement me\n";
+}
if we do this, we probably also want a parse_volume here? see comments
on qemu-server #12
>> +
+# $volume_map is a hash of 'old_volid' => 'new_volid' pairs.
+# This method replaces 'old_volid' by 'new_volid' throughout
+# the config including snapshots, both for volumes appearing in
+# foreach_volume as well as vmstate and unusedN values.
+sub update_volume_ids {
+ my ($class, $conf, $volume_map) = @_;
+
+ my $newconf = {};
why not modify the config in place? you replace the old one with the
returned new one anyway in the single caller, and write it out directly
afterwards ;) it would make the code shorter, and more inline with how
we usually modify $conf
Ok, I'll do that.
+
+ my $do_replace = sub {
+ my ($key, $volume, $newconf, $volume_map) = @_;
no need to pass $newconf and $volume_map as parameter, the one from
update_volume_ids is accessible here anyway.
+
+ my $old_volid = $volume->{file} // $volume->{volume};
it might make sense to expose this (under which key the volid is stored)
via AbstractConfig/QemuConfig/LXC::Config, Aaron's backup status patch series
also
needs this information in pve-manager.
You suggested creating new helper modules for Qemu drives in the reply
to #12. Would it make sense to go all the way and create a base
AbstractVolume.pm and then QemuDrive.pm and LXCMountpoint.pm? Initially
it might only contain parse_volume, print_volume, foreach_volume and
this key (or maybe get_volid and set_volid?) and in the long term more
code could be moved there.
+ if (my $new_volid = $volume_map->{$old_volid}) {
+ $volume->{file} = $new_volid if defined($volume->{file});
+ $volume->{volume} = $new_volid if defined($volume->{volume});
which would make this a single line
+ $newconf->{$key} = $class->print_volume($volume);
+ }
+ };
+
+ my $replace_volids = sub {
+ my ($conf) = @_;
+
+ my $newconf = {};
+ foreach my $key (keys %{$conf}) {
+ next if $key =~ m/^snapshots$/;
+ # these keys are not handled by foreach_volume
would it make sense to include them optionally?
we have lots of use cases where we really want to iterate over ALL the
currently referenced volumes.. I know both of them only have a volid,
but we could just set that into $parsed->{file} or $parsed->{volume} and
leave the rest empty. if you call foreach_volume with
$opts->{include_vmstate} or $opts->{include_unused} you of course need
to be able to handle them properly, and we'd need to look whether
pve-container can easily support such an interface as well.
Yes, I'll look into it.
+ if ($key =~ m/^(vmstate)|(unused\d+)$/) {
+ my $old_volid = $conf->{$key};
+ $newconf->{$key} = $volume_map->{$old_volid};
in-place, this could become
$conf->{$key} = $volume_map->{$conf->{$key}};
+ }
+ $newconf->{$key} = $conf->{$key} if !defined($newconf->{$key});
not needed for in-place
+ }
+
+ $class->foreach_volume($conf, $do_replace, $newconf, $volume_map);
+ return $newconf;
+ };
+
+ $newconf = $replace_volids->($conf);
+ foreach my $snap (keys %{$conf->{snapshots}}) {
+ my $newsnap = $replace_volids->($conf->{snapshots}->{$snap});
+ foreach my $k (keys %{$newsnap}) {
+ $newconf->{snapshots}->{$snap}->{$k} = $newsnap->{$k};
+ }
this second foreach should not be needed:
foreach my $snap (keys %${$conf->{snapshots}}) {
$newconf->{snapshots}->{$snap} =
$replace_volids->($conf->{snapshots}->{$snap});
}
(or $conf-> if we drop $newconf)
+ }
+
+ return $newconf;
+}
+
# Internal snapshots
# NOTE: Snapshot create/delete involves several non-atomic
--
2.20.1
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
