all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [RFC qemu-server v2 18/21] vzdump: better cleanup fleecing images after hard errors
Date: Fri, 15 Mar 2024 11:24:59 +0100	[thread overview]
Message-ID: <20240315102502.84163-19-f.ebner@proxmox.com> (raw)
In-Reply-To: <20240315102502.84163-1-f.ebner@proxmox.com>

By recording the allocated fleecing images in the VM config, they
are not immediately orphaned, should a hard error occur during
backup that prevents cleanup.

They are attempted to be cleaned up during the next backup run.

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v2.

 PVE/QemuConfig.pm        | 40 ++++++++++++++++++++++++++++++++++++++++
 PVE/VZDump/QemuServer.pm | 31 ++++++++++++++++++++++++-------
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index ca30eda0..fcfce1fb 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -13,6 +13,7 @@ use PVE::QemuServer::Monitor qw(mon_cmd);
 use PVE::QemuServer;
 use PVE::QemuServer::Machine;
 use PVE::QemuServer::Memory qw(get_current_memory);
+use PVE::RESTEnvironment qw(log_warn);
 use PVE::Storage;
 use PVE::Tools;
 use PVE::Format qw(render_bytes render_duration);
@@ -572,4 +573,43 @@ sub has_cloudinit {
     return $found;
 }
 
+# Caller is expected to deal with volumes from an already exisitng entry first.
+sub record_fleecing_images {
+    my ($vmid, $volids) = @_;
+
+    return if scalar($volids->@*) == 0;
+
+    PVE::QemuConfig->lock_config($vmid, sub {
+	my $conf = PVE::QemuConfig->load_config($vmid);
+	$conf->{'fleecing-images'} = join(',', $volids->@*);
+	PVE::QemuConfig->write_config($vmid, $conf);
+    });
+}
+
+sub cleanup_fleecing_images {
+    my ($vmid, $storecfg) = @_;
+
+    my $volids = [];
+    my $failed = [];
+
+    PVE::QemuConfig->lock_config($vmid, sub {
+	my $conf = PVE::QemuConfig->load_config($vmid);
+	if ($conf->{'fleecing-images'}) {
+	    $volids = [PVE::Tools::split_list($conf->{'fleecing-images'})];
+	    delete $conf->{'fleecing-images'};
+	    PVE::QemuConfig->write_config($vmid, $conf);
+	}
+    });
+
+    for my $volid ($volids->@*) {
+	eval { PVE::Storage::vdisk_free($storecfg, $volid); };
+	if (my $err = $@) {
+	    log_warn("error removing fleecing image '$volid' - $err");
+	    push $failed->@*, $volid;
+	}
+    }
+
+    record_fleecing_images($vmid, $failed);
+}
+
 1;
diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 8a44a1fd..ad0212bc 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -527,15 +527,25 @@ sub get_and_check_pbs_encryption_config {
     die "internal error - unhandled case for getting & checking PBS encryption ($keyfile, $master_keyfile)!";
 }
 
+# Helper is intended to be called from allocate_fleecing_images() only. Otherwise, fleecing volids
+# have already been recorded in the configuration and PVE::QemuConfig::cleanup_fleecing_images()
+# should be used instead.
 my sub cleanup_fleecing_images {
-    my ($self, $disks) = @_;
+    my ($self, $vmid, $disks) = @_;
+
+    my $failed = [];
 
     for my $di ($disks->@*) {
 	if (my $volid = $di->{'fleece-volid'}) {
 	    eval { PVE::Storage::vdisk_free($self->{storecfg}, $volid); };
-	    $self->log('warn', "error removing fleecing image '$volid' - $@") if $@;
+	    if (my $err = $@) {
+		$self->log('warn', "error removing fleecing image '$volid' - $err");
+		push $failed->@*, $volid;
+	    }
 	}
     }
+
+    PVE::QemuConfig::record_fleecing_images($vmid, $failed);
 }
 
 my sub allocate_fleecing_images {
@@ -543,8 +553,7 @@ my sub allocate_fleecing_images {
 
     die "internal error - no fleecing storage specified\n" if !$fleecing_storeid;
 
-    # TODO what about potential left-over images from a failed attempt? Just
-    # auto-remove? While unlikely, could conflict with manually created image from user...
+    my $fleece_volids = [];
 
     eval {
 	my $n = 0; # counter for fleecing image names
@@ -561,6 +570,8 @@ my sub allocate_fleecing_images {
 		$di->{'fleece-volid'} = PVE::Storage::vdisk_alloc(
 		    $self->{storecfg}, $fleecing_storeid, $vmid, $format, $name, $size);
 
+		push $fleece_volids->@*, $di->{'fleece-volid'};
+
 		$n++;
 	    } else {
 		die "implement me (type '$di->{type}')";
@@ -568,9 +579,11 @@ my sub allocate_fleecing_images {
 	}
     };
     if (my $err = $@) {
-	cleanup_fleecing_images($self, $disks);
+	cleanup_fleecing_images($self, $vmid, $disks);
 	die $err;
     }
+
+    PVE::QemuConfig::record_fleecing_images($vmid, $fleece_volids);
 }
 
 my sub detach_fleecing_images {
@@ -630,6 +643,10 @@ my sub check_and_prepare_fleecing {
 	$use_fleecing = 0;
     }
 
+    # clean up potential left-overs from a previous attempt
+    eval { PVE::QemuConfig::cleanup_fleecing_images($vmid, $self->{storecfg}); };
+    $self->log('warn', "attempt to clean up left-over fleecing images failed - $@") if $@;
+
     if ($use_fleecing) {
 	my ($default_format, $valid_formats) = PVE::Storage::storage_default_format(
 	    $self->{storecfg}, $fleecing_opts->{storage});
@@ -791,7 +808,7 @@ sub archive_pbs {
 
     if ($use_fleecing) {
 	detach_fleecing_images($task->{disks}, $vmid);
-	cleanup_fleecing_images($self, $task->{disks});
+	PVE::QemuConfig::cleanup_fleecing_images($vmid, $self->{storecfg});
     }
 
     die $err if $err;
@@ -991,7 +1008,7 @@ sub archive_vma {
 
     if ($use_fleecing) {
 	detach_fleecing_images($task->{disks}, $vmid);
-	cleanup_fleecing_images($self, $task->{disks});
+	PVE::QemuConfig::cleanup_fleecing_images($vmid, $self->{storecfg});
     }
 
     if ($err) {
-- 
2.39.2





  parent reply	other threads:[~2024-03-15 10:34 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-15 10:24 [pve-devel] [PATCH-SERIES v2] fix #4136: implement backup fleecing Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [PATCH qemu v2 01/21] block/copy-before-write: fix permission Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [PATCH qemu v2 02/21] block/copy-before-write: support unligned snapshot-discard Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [PATCH qemu v2 03/21] block/copy-before-write: create block_copy bitmap in filter node Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [PATCH qemu v2 04/21] qapi: blockdev-backup: add discard-source parameter Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [PATCH qemu v2 05/21] copy-before-write: allow specifying minimum cluster size Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [PATCH qemu v2 06/21] backup: add minimum cluster size to performance options Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [PATCH qemu v2 07/21] PVE backup: add fleecing option Fiona Ebner
2024-04-08 12:45   ` Wolfgang Bumiller
2024-04-10  9:30     ` Fiona Ebner
2024-04-10 11:38       ` Wolfgang Bumiller
2024-03-15 10:24 ` [pve-devel] [PATCH common v2 08/21] json schema: add format description for pve-storage-id standard option Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [PATCH guest-common v2 09/21] vzdump: schema: add fleecing property string Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [PATCH guest-common v2 10/21] vzdump: schema: make storage for fleecing semi-optional Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [RFC guest-common v2 11/21] abstract config: do not copy fleecing images entry for snapshot Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [PATCH manager v2 12/21] vzdump: handle new 'fleecing' property string Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [PATCH manager v2 13/21] api: backup/vzdump: add permission check for fleecing storage Fiona Ebner
2024-04-08  8:47   ` Wolfgang Bumiller
2024-04-10  9:57     ` Fiona Ebner
2024-04-10 11:37       ` Wolfgang Bumiller
2024-03-15 10:24 ` [pve-devel] [PATCH qemu-server v2 14/21] backup: disk info: also keep track of size Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [PATCH qemu-server v2 15/21] backup: implement fleecing option Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [RFC qemu-server v2 16/21] parse config: allow config keys with minus sign Fiona Ebner
2024-03-15 10:24 ` [pve-devel] [RFC qemu-server v2 17/21] schema: add fleecing-images config property Fiona Ebner
2024-03-15 10:24 ` Fiona Ebner [this message]
2024-03-15 10:25 ` [pve-devel] [RFC qemu-server v2 19/21] migration: attempt to clean up potential left-over fleecing images Fiona Ebner
2024-03-15 10:25 ` [pve-devel] [RFC qemu-server v2 20/21] destroy vm: " Fiona Ebner
2024-03-15 10:25 ` [pve-devel] [PATCH docs v2 21/21] vzdump: add section about backup fleecing Fiona Ebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240315102502.84163-19-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal