[pve-devel] [PATCH qemu-server 2/3] Include vmstate and unused volumes in foreach_volid
Fabian Ebner
f.ebner at proxmox.com
Thu Apr 16 15:06:22 CEST 2020
On 16.04.20 14:54, Fabian Ebner wrote:
> and refactor the test_volid closure. Like this get_replicatable_volumes doesn't
> need a separate loop for unused volumes anymore. For get_vm_volumes, which is used
> for activation/deactivation of volumes at migration and deactivation in vm_stop_cleanup,
> includes those volumes now. For migration it's an improvement, because those volumes
> might need to be migrated and for vm_stop_cleanup it shouldn't hurt. The last user
> of foreach_volid is check_vm_disks_local used by migrate_vm_precondition,
> where information about the additional volumes doesn't hurt either.
>
> Note that replicate is (still) set by default, so the behavior for
> get_replicatable_volumes for unused volumes should not change.
>
> Hibernation vmstate files are now also included and recognized as 'is_vmstate'.
> The 'size' attribute will not be overwritten by subsequent iterations for the
> same volid anymore (a volid may appear both in the config and in snapshots),
> so the size from the current config is now preferred.
>
Just saw that the size was not passed along previously for the
iterations for the snapshots, but my change certainly makes the switch
to '//=' in
$volhash->{$volid}->{size} //= $size if $size;
necessary to avoid overriding the size.
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
>
> Feel free to ignore this one until after the release if it's too
> big of a change/too much to review.
>
> PVE/QemuConfig.pm | 7 -------
> PVE/QemuMigrate.pm | 1 +
> PVE/QemuServer.pm | 33 +++++++++++++++++++++++----------
> 3 files changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
> index 7874081..240fc06 100644
> --- a/PVE/QemuConfig.pm
> +++ b/PVE/QemuConfig.pm
> @@ -162,13 +162,6 @@ sub get_replicatable_volumes {
>
> PVE::QemuServer::foreach_volid($conf, $test_volid);
>
> - # add 'unusedX' volumes to volhash
> - foreach my $key (keys %$conf) {
> - if ($key =~ m/^unused/) {
> - $test_volid->($conf->{$key}, { replicate => 1 });
> - }
> - }
> -
> return $volhash;
> }
>
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 2e2e430..bac4e67 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -388,6 +388,7 @@ sub sync_disks {
> return if $scfg->{shared};
>
> $local_volumes->{$volid}->{ref} = $attr->{referenced_in_config} ? 'config' : 'snapshot';
> + $local_volumes->{$volid}->{ref} = 'storage' if $attr->{is_unused};
>
> $local_volumes->{$volid}->{is_vmstate} = $attr->{is_vmstate} ? 1 : 0;
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 5a42532..e9784b0 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -4292,39 +4292,52 @@ sub foreach_volid {
> my $volhash = {};
>
> my $test_volid = sub {
> - my ($volid, $is_cdrom, $replicate, $shared, $snapname, $size) = @_;
> + my ($key, $drive, $snapname) = @_;
>
> + my $volid = $drive->{file};
> return if !$volid;
>
> $volhash->{$volid}->{cdrom} //= 1;
> - $volhash->{$volid}->{cdrom} = 0 if !$is_cdrom;
> + $volhash->{$volid}->{cdrom} = 0 if !drive_is_cdrom($drive);
>
> + my $replicate = $drive->{replicate} // 1;
> $volhash->{$volid}->{replicate} //= 0;
> $volhash->{$volid}->{replicate} = 1 if $replicate;
>
> $volhash->{$volid}->{shared} //= 0;
> - $volhash->{$volid}->{shared} = 1 if $shared;
> + $volhash->{$volid}->{shared} = 1 if $drive->{shared};
>
> $volhash->{$volid}->{referenced_in_config} //= 0;
> $volhash->{$volid}->{referenced_in_config} = 1 if !defined($snapname);
>
> $volhash->{$volid}->{referenced_in_snapshot}->{$snapname} = 1
> if defined($snapname);
> - $volhash->{$volid}->{size} = $size if $size;
> +
> + my $size = $drive->{size};
> + $volhash->{$volid}->{size} //= $size if $size;
> +
> + $volhash->{$volid}->{is_vmstate} //= 0;
> + $volhash->{$volid}->{is_vmstate} = 1 if $key eq 'vmstate';
> +
> + $volhash->{$volid}->{is_unused} //= 0;
> + $volhash->{$volid}->{is_unused} = 1 if $key =~ /^unused\d+$/;
> };
>
> - PVE::QemuConfig->foreach_volume($conf, sub {
> + my $include_opts = {
> + extra_keys => ['vmstate'],
> + include_unused => 1,
> + };
> +
> + PVE::QemuConfig->foreach_volume_full($conf, $include_opts, sub {
> my ($ds, $drive) = @_;
> - $test_volid->($drive->{file}, drive_is_cdrom($drive), $drive->{replicate} // 1, $drive->{shared}, undef, $drive->{size});
> + $test_volid->($ds, $drive);
> });
>
> foreach my $snapname (keys %{$conf->{snapshots}}) {
> my $snap = $conf->{snapshots}->{$snapname};
> - $test_volid->($snap->{vmstate}, 0, 1, 0, $snapname);
> - $volhash->{$snap->{vmstate}}->{is_vmstate} = 1 if $snap->{vmstate};
> - PVE::QemuConfig->foreach_volume($snap, sub {
> + PVE::QemuConfig->foreach_volume_full($snap, $include_opts, sub {
> my ($ds, $drive) = @_;
> - $test_volid->($drive->{file}, drive_is_cdrom($drive), $drive->{replicate} // 1, $drive->{shared}, $snapname);
> + $test_volid->($ds, $drive, $snapname);
> });
> }
>
>
More information about the pve-devel
mailing list