From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 6352A1FF2AD
for <inbox@lore.proxmox.com>; Thu, 4 Jul 2024 12:16:24 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 72EB23021A;
Thu, 4 Jul 2024 12:16:38 +0200 (CEST)
MIME-Version: 1.0
In-Reply-To: <20240529114519.2419494-1-d.csapak@proxmox.com>
References: <20240529114519.2419494-1-d.csapak@proxmox.com>
From: Fabian =?utf-8?q?Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Date: Thu, 04 Jul 2024 12:15:58 +0200
Message-ID: <172008815851.63060.5652157369713340865@yuna.proxmox.com>
User-Agent: alot/0.10
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.050 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH guest-common] abstract config: snapshot
create/rollback/remove: print more log statements
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
Quoting Dominik Csapak (2024-05-29 13:45:19)
> this increases verbosity of the actions, especially when including the
> snapshot name, since that will appear in the task logs wereas before
> there was no mention of any action, just the storage specific output for
> creating/rollback/removal.
>
> Logs are printed at all main actions that can fail or take potentially
> long, so that it's clear what started/finished.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> replaces https://lists.proxmox.com/pipermail/pve-devel/2024-May/064010.html
>
> src/PVE/AbstractConfig.pm | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm
> index 5d5f9b4..b685b7f 100644
> --- a/src/PVE/AbstractConfig.pm
> +++ b/src/PVE/AbstractConfig.pm
> @@ -806,6 +806,8 @@ sub __snapshot_activate_storages {
> sub snapshot_create {
> my ($class, $vmid, $snapname, $save_vmstate, $comment) = @_;
>
> + print "Creating snapshot '$snapname'\n";
> +
> my $snap = $class->__snapshot_prepare($vmid, $snapname, $save_vmstate, $comment);
>
> $save_vmstate = 0 if !$snap->{vmstate};
> @@ -820,24 +822,30 @@ sub snapshot_create {
> $class->__snapshot_activate_storages($conf, 0);
>
> if ($freezefs) {
> + print "Trying to freeze guest filesystems\n";
> $class->__snapshot_freeze($vmid, 0);
> + print "Succesfully frozen guest filesystems\n";
> }
>
> $class->__snapshot_create_vol_snapshots_hook($vmid, $snap, $running, "before");
>
> + print "Creating volume snapshots\n";
> $class->foreach_volume($snap, sub {
> my ($vs, $volume) = @_;
>
> $class->__snapshot_create_vol_snapshot($vmid, $vs, $volume, $snapname);
> $drivehash->{$vs} = 1;
> });
> + print "Succesfully created volume snapshots\n";
> };
> my $err = $@;
>
> if ($running) {
> $class->__snapshot_create_vol_snapshots_hook($vmid, $snap, $running, "after");
> if ($freezefs) {
> + print "Trying to thaw guest filesystems\n";
> $class->__snapshot_freeze($vmid, 1);
> + print "Succesfully thawed guest filesystems\n";
> }
> $class->__snapshot_create_vol_snapshots_hook($vmid, $snap, $running, "after-unfreeze");
> }
> @@ -850,6 +858,8 @@ sub snapshot_create {
> }
>
> $class->__snapshot_commit($vmid, $snapname);
> +
> + print "Succesfully created snapshot '$snapname'\n";
> }
>
> # Check if the snapshot might still be needed by a replication job.
> @@ -895,6 +905,8 @@ my $snapshot_delete_assert_not_needed_by_replication = sub {
> sub snapshot_delete {
> my ($class, $vmid, $snapname, $force, $drivehash) = @_;
>
> + print "Removing snapshot '$snapname'\n";
> +
> my $unused = [];
>
> my $conf = $class->load_config($vmid);
> @@ -963,12 +975,15 @@ sub snapshot_delete {
>
> # now remove vmstate file
> if ($snap->{vmstate}) {
> + print "Removing vmstate file\n";
> $class->__snapshot_delete_vmstate_file($snap, $force);
>
> # save changes (remove vmstate from snapshot)
> $class->lock_config($vmid, $remove_drive, 'vmstate') if !$force;
> + print "Succesfully removed vmstate file\n";
> };
>
> + print "Removing volume snapshots\n";
> # now remove all volume snapshots
> $class->foreach_volume($snap, sub {
> my ($vs, $volume) = @_;
> @@ -985,6 +1000,7 @@ sub snapshot_delete {
> # save changes (remove drive from snapshot)
> $class->lock_config($vmid, $remove_drive, $vs) if !$force;
> });
> + print "Succesfully removed volume snapshots\n";
>
> # now cleanup config
> $class->lock_config($vmid, sub {
> @@ -1010,6 +1026,8 @@ sub snapshot_delete {
>
> $class->write_config($vmid, $conf);
> });
> +
> + print "Succesfully removed snapshot '$snapname'\n";
> }
>
> # Remove replication snapshots to make a rollback possible.
> @@ -1082,6 +1100,8 @@ my $rollback_remove_replication_snapshots = sub {
> sub snapshot_rollback {
> my ($class, $vmid, $snapname) = @_;
>
> + print "Rolling back to snapshot '$snapname'\n";
> +
> my $prepare = 1;
>
> my $data = {};
> @@ -1121,6 +1141,7 @@ sub snapshot_rollback {
>
> if ($prepare) {
> $class->check_lock($conf);
> + print "Stopping guest\n";
nit: this can be a bit misleading, since it is also printed if the guest isn't
running.. for containers, the implementation of this is guarded by
check_running, for VMs it basically is as well (since this only calls vm_stop,
which returns immediately if the VM is not running).
should we just add that check to the if condition here? and then drop it from
LXC::Config, since it's redundant then?
> $class->__snapshot_rollback_vm_stop($vmid);
> }
>
> @@ -1149,20 +1170,25 @@ sub snapshot_rollback {
> $class->write_config($vmid, $conf);
>
> if (!$prepare && $snap->{vmstate}) {
> + print "Starting guest\n";
> $class->__snapshot_rollback_vm_start($vmid, $snap->{vmstate}, $data);
> }
> };
>
> $class->lock_config($vmid, $updatefn);
>
> + print "Rolling back volume snapshots\n";
> $class->foreach_volume($snap, sub {
> my ($vs, $volume) = @_;
>
> $class->__snapshot_rollback_vol_rollback($volume, $snapname);
> });
> + print "Succesfully rolled back volume snapshots\n";
>
> $prepare = 0;
> $class->lock_config($vmid, $updatefn);
> +
> + print "Succesfully rolled back to snapshot '$snapname'\n";
> }
>
> # Calculate a derived property from a configuration. Derived properties are:
> --
> 2.39.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel