much thanks in general for following up on my request, this irked
me since almost ever.
a few nits inline.
On 04.02.21 13:52, Stefan Reiter wrote:
> Signed-off-by: Stefan Reiter <s.rei...@proxmox.com>
> ---
> PVE/QemuConfig.pm | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
> index 3f4605f..37db347 100644
> --- a/PVE/QemuConfig.pm
> +++ b/PVE/QemuConfig.pm
> @@ -14,6 +14,7 @@ use PVE::QemuServer;
> use PVE::QemuServer::Machine;
> use PVE::Storage;
> use PVE::Tools;
> +use PVE::CLIFormatter;
>
> use base qw(PVE::AbstractConfig);
>
> @@ -280,14 +281,26 @@ sub __snapshot_create_vol_snapshots_hook {
> PVE::Storage::activate_volumes($storecfg, [$snap->{vmstate}]);
>
> mon_cmd($vmid, "savevm-start", statefile => $path);
> + print "saving VM state and RAM\n";
> + my $start = time();
The SaveVMInfo struct returned by 'query-savevm' would have a 'total-time'
field,
why not use that?
> + my $state = sub {
over general helper name
> + my ($bytes) = @_;
> + my $b = PVE::CLIFormatter::render_bytes($bytes);
> + my $t = PVE::CLIFormatter::render_duration(time() - $start);
> + return "$b in $t";
could return a ($b, $t) tuple here, just an idea, this is totally fine to
with the current usage.
> + };
> for(;;) {
> my $stat = mon_cmd($vmid, "query-savevm");
> if (!$stat->{status}) {
> die "savevm not active\n";
> } elsif ($stat->{status} eq 'active') {
> sleep(1);
> + my $s = $state->($stat->{bytes});
> + print "progress: $s\n";
I'd drop the "progress", IMO just noise.
> next;
> } elsif ($stat->{status} eq 'completed') {
> + my $s = $state->($stat->{bytes});
> + print "saved $s\n";
I'd explicitly out put that we're done:
"completed, saved $s\n";
> last;
> } else {
> die "query-savevm returned status '$stat->{status}'\n";
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
