[pve-devel] [PATCH-SERIES v4 docs/qemu-server] more robust handling of fleecing images

[pve-devel] [PATCH-SERIES v4 docs/qemu-server] more robust handling of fleecing images

[Fiona Ebner]

Record the created fleecing images in the VM configuration to be able
to remove left-overs after hard failures.

The new config key uses a hyphen. Add some documentation about why
there are different styles.

v3 was part of the series adding fleecing:
https://lore.proxmox.com/pve-devel/20240411092943.57377-19-f.ebner@proxmox.com/


docs:

Fiona Ebner (2):
  configuration files: add general section
  cli appendix: reference section about casing style

 cli-general.adoc         |  6 ++++++
 configuration-files.adoc | 28 ++++++++++++++++++++++++++++
 pve-admin-guide.adoc     |  4 +++-
 3 files changed, 37 insertions(+), 1 deletion(-)
 create mode 100644 cli-general.adoc
 create mode 100644 configuration-files.adoc


qemu-server:

Fiona Ebner (7):
  backup: prepare: factor out getting running status
  backup: prepare: cancel previous job if still running
  parse config: allow config keys with minus sign
  schema: add fleecing-images config property
  fix #5440: vzdump: better cleanup fleecing images after hard errors
  migration: attempt to clean up potential left-over fleecing images
  destroy vm: clean up potential left-over fleecing images

 PVE/API2/Qemu.pm         |  9 ++++++
 PVE/QemuConfig.pm        | 68 ++++++++++++++++++++++++++++++++++++++++
 PVE/QemuMigrate.pm       |  7 +++++
 PVE/QemuServer.pm        | 12 ++++++-
 PVE/VZDump/QemuServer.pm | 52 +++++++++++++++++++++++-------
 5 files changed, 136 insertions(+), 12 deletions(-)

-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH v4 docs 1/9] configuration files: add general section

[Fiona Ebner]

Explain the mixed casing styles for (sub-)properties in configuration
files and plans regarding the future.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v4.

 configuration-files.adoc | 28 ++++++++++++++++++++++++++++
 pve-admin-guide.adoc     |  2 +-
 2 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 configuration-files.adoc

diff --git a/configuration-files.adoc b/configuration-files.adoc
new file mode 100644
index 0000000..aeb6cf4
--- /dev/null
+++ b/configuration-files.adoc
@@ -0,0 +1,28 @@
+[[configuration_files]]
+General
+=======
+
+Most configuration files in {pve} reside on the
+xref:chapter_pmxcfs[shared cluster file system] mounted at `/etc/pve`. There are
+exceptions, like the node-specific configuration file for backups in
+`/etc/vzdump.conf`.
+
+Usually, the properties in a configuration file are derived from the JSON Schema
+that is also used for the associated API endpoints.
+
+[[configuration_files_casing]]
+Casing of Property Names
+------------------------
+
+Historically, longer properties (and sub-properties) often used `snake_case`, or
+were written as one word. This can likely be attributed to the {pve} stack being
+developed mostly in the programming language Perl, where access to properties
+using `kebab-case` requires additional quotes, as well as less style enforcement
+during early development, so different developers used different conventions.
+
+For new properties, `kebab-case` is the preferred way and it is planned to
+introduce aliases for existing `snake_case` properties, and in the long term,
+switch over to `kebab-case` for the API, CLI and in-use configuration files
+while maintaining backwards-compatibility when restoring a configuration.
+
+include::datacenter.cfg.adoc[]
diff --git a/pve-admin-guide.adoc b/pve-admin-guide.adoc
index 2a37df4..9a0ec85 100644
--- a/pve-admin-guide.adoc
+++ b/pve-admin-guide.adoc
@@ -308,7 +308,7 @@ Configuration Files
 -------------------
 
 :leveloffset: 2
-include::datacenter.cfg.adoc[]
+include::configuration-files.adoc[]
 
 :leveloffset: 0
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH v4 docs 2/9] cli appendix: reference section about casing style

[Fiona Ebner]

The section about casing style also applies to CLI option names.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v4.

 cli-general.adoc     | 6 ++++++
 pve-admin-guide.adoc | 2 ++
 2 files changed, 8 insertions(+)
 create mode 100644 cli-general.adoc

diff --git a/cli-general.adoc b/cli-general.adoc
new file mode 100644
index 0000000..b030205
--- /dev/null
+++ b/cli-general.adoc
@@ -0,0 +1,6 @@
+[[cli_general]]
+General
+-------
+
+Regarding the historically non-uniform casing style for options, see
+xref:configuration_files_casing[the related section for configuration files].
diff --git a/pve-admin-guide.adoc b/pve-admin-guide.adoc
index 9a0ec85..03e1dad 100644
--- a/pve-admin-guide.adoc
+++ b/pve-admin-guide.adoc
@@ -98,6 +98,8 @@ Command-line Interface
 
 :leveloffset: 1
 
+include::cli-general.adoc[]
+
 include::output-format.adoc[]
 
 :leveloffset: 0
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH v4 qemu-server 3/9] backup: prepare: factor out getting running status

[Fiona Ebner]

In preparation to use it to conditionally issue a QMP 'backup-cancel'
should a previous backup still be running.

While at it, avoid using the compat-only check_running() helper.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v4.

 PVE/VZDump/QemuServer.pm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 012c9210..bd4795ca 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -57,6 +57,8 @@ sub vmlist {
 sub prepare {
     my ($self, $task, $vmid, $mode) = @_;
 
+    my $running = PVE::QemuServer::Helpers::vm_running_locally($vmid);
+
     $task->{disks} = [];
 
     my $conf = $self->{vmlist}->{$vmid} = PVE::QemuConfig->load_config($vmid);
@@ -64,11 +66,9 @@ sub prepare {
     $self->loginfo("VM Name: $conf->{name}")
 	if defined($conf->{name});
 
-    $self->{vm_was_running} = 1;
+    $self->{vm_was_running} = $running ? 1 : 0;
     $self->{vm_was_paused} = 0;
-    if (!PVE::QemuServer::check_running($vmid)) {
-	$self->{vm_was_running} = 0;
-    } elsif (PVE::QemuServer::vm_is_paused($vmid, 0)) {
+    if ($running && PVE::QemuServer::vm_is_paused($vmid, 0)) {
 	# Do not treat a suspended VM as paused, as it would cause us to skip
 	# fs-freeze even if the VM wakes up before we reach qga_fs_freeze.
 	$self->{vm_was_paused} = 1;
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH v4 qemu-server 4/9] backup: prepare: cancel previous job if still running

[Fiona Ebner]

This can happen after a hard failure, e.g. if the vzdump task was
killed. The next backup (after unlocking the VM) would then fail with

> ERROR: VM 125 qmp command 'backup' failed - previous backup not finished

During the failure path of that attempt, 'backup-cancel' is executed
and the subsequent attempt would then work again. Do it up-front with
a warning instead of relying on this behavior.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v4.

 PVE/VZDump/QemuServer.pm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index bd4795ca..8ebe6772 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -59,6 +59,13 @@ sub prepare {
 
     my $running = PVE::QemuServer::Helpers::vm_running_locally($vmid);
 
+    if ($running && (my $status = mon_cmd($vmid, 'query-backup'))) {
+	if ($status->{status} && $status->{status} eq 'active') {
+	    $self->log('warn', "left-over backup job still running inside QEMU - canceling now");
+	    mon_cmd($vmid, 'backup-cancel');
+	}
+    }
+
     $task->{disks} = [];
 
     my $conf = $self->{vmlist}->{$vmid} = PVE::QemuConfig->load_config($vmid);
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH v4 qemu-server 5/9] parse config: allow config keys with minus sign

[Fiona Ebner]

In preparation for the upcoming 'fleecing-images' key. To avoid mixing
of options with - and options with _, which is not very user-friendly,
it would be nice to add aliases for existing options with _. And
long-term, backup restore handlers could switch to the modern keys
with -.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

No changes in v4.

 PVE/QemuServer.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 0df3bda0..c5f80588 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2382,7 +2382,7 @@ sub parse_vm_config {
 	    } else {
 		$handle_error->("vm $vmid - property 'delete' is only allowed in [PENDING]\n");
 	    }
-	} elsif ($line =~ m/^([a-z][a-z_]*\d*):\s*(.+?)\s*$/) {
+	} elsif ($line =~ m/^([a-z][a-z_\-]*\d*):\s*(.+?)\s*$/) {
 	    my $key = $1;
 	    my $value = $2;
 	    if ($section eq 'cloudinit') {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH v4 qemu-server 6/9] schema: add fleecing-images config property

[Fiona Ebner]

to be used internally to record volume IDs of fleecing images
allocated during backup.

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

No changes in v4.

 PVE/API2/Qemu.pm         | 9 +++++++++
 PVE/QemuServer.pm        | 7 +++++++
 PVE/VZDump/QemuServer.pm | 1 +
 3 files changed, 17 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 848001b6..3b05977d 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -967,6 +967,9 @@ __PACKAGE__->register_method({
 	$param->{cpuunits} = PVE::CGroup::clamp_cpu_shares($param->{cpuunits})
 	    if defined($param->{cpuunits}); # clamp value depending on cgroup version
 
+	raise_param_exc({ 'fleecing-images' => "Cannot set option - for internal use only." })
+	    if $param->{'fleecing-images'};
+
 	PVE::Cluster::check_cfs_quorum();
 
 	my $filename = PVE::QemuConfig->config_file($vmid);
@@ -1680,6 +1683,9 @@ my $update_vm_api  = sub {
 	push @paramarr, "-$key", $value;
     }
 
+    raise_param_exc({ 'fleecing-images' => "Cannot set option - for internal use only." })
+	if $param->{'fleecing-images'};
+
     my $skiplock = extract_param($param, 'skiplock');
     raise_param_exc({ skiplock => "Only root may use this option." })
 	if $skiplock && $authuser ne 'root@pam';
@@ -3794,6 +3800,9 @@ __PACKAGE__->register_method({
 		next if $opt eq 'snapshots' ||  $opt eq 'parent' || $opt eq 'snaptime' ||
 		    $opt eq 'vmstate' || $opt eq 'snapstate';
 
+		# left-overs, not cloned
+		next if $opt eq 'fleecing-images';
+
 		# no need to copy unused images, because VMID(owner) changes anyways
 		next if $opt =~ m/^unused\d+$/;
 
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index c5f80588..649843aa 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -732,6 +732,13 @@ EODESCR
 	description => "List of host cores used to execute guest processes, for example: 0,5,8-11",
 	optional => 1,
     },
+    'fleecing-images' => {
+	type => 'string',
+	format => 'pve-volume-id-list',
+	description => "For internal use only. List of fleecing images allocated during backup."
+	    ." If no backup is running, these are left-overs that failed to be removed.",
+	optional => 1,
+    },
 };
 
 my $cicustom_fmt = {
diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 8ebe6772..17f63568 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -252,6 +252,7 @@ sub assemble {
 	    next;
 	}
 	next if $line =~ m/^lock:/ || $line =~ m/^parent:/;
+	next if $line =~ m/^fleecing-images:/;
 
 	print $outfd $line;
     }
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH v4 qemu-server 7/9] fix #5440: vzdump: better cleanup fleecing images after hard errors

[Fiona Ebner]

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>
---

Changes in v4:
* detach left-over fleecing images from a running VM before attempting
  cleanup
* more logging

 PVE/QemuConfig.pm        | 68 ++++++++++++++++++++++++++++++++++++++++
 PVE/VZDump/QemuServer.pm | 36 ++++++++++++++++-----
 2 files changed, 97 insertions(+), 7 deletions(-)

diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 8e8a7828..3084f831 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);
@@ -573,4 +574,71 @@ sub has_cloudinit {
     return $found;
 }
 
+# Caller is expected to deal with volumes from an already existing 'fleecing-images' entry in the
+# configuration 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, $log_func) = @_;
+
+    if (!$log_func) {
+	$log_func = sub {
+	    my ($level, $line) = @_;
+	    chomp($line);
+	    if ($level eq 'info') {
+		print "$line\n";
+	    } else {
+		log_warn($line);
+	    }
+	};
+    }
+
+    my $volids = [];
+    my $failed = [];
+
+    # detach any left-overs from a running VM
+    if (PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
+	my $block_info = mon_cmd($vmid, "query-block");
+	for my $info ($block_info->@*) {
+	    my $device_id = $info->{device};
+	    next if $device_id !~ m/-fleecing$/;
+
+	    $log_func->('info', "detaching (old) fleecing image for '$device_id'");
+	    $device_id =~ s/^drive-//; # re-added by qemu_drivedel()
+	    eval { PVE::QemuServer::qemu_drivedel($vmid, $device_id) };
+	    $log_func->('warn', "error detaching (old) fleecing image '$device_id' - $@") if $@;
+	}
+    }
+
+    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->@*) {
+	$log_func->('info', "removing (old) fleecing image '$volid'");
+	eval { PVE::Storage::vdisk_free($storecfg, $volid); };
+	if (my $err = $@) {
+	    $log_func->('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 17f63568..240e1e95 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -530,15 +530,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 {
@@ -546,8 +556,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
@@ -564,6 +573,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}')";
@@ -571,9 +582,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 {
@@ -633,6 +646,13 @@ 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}, sub { $self->log($_[0], $_[1]); });
+    };
+    $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});
@@ -794,7 +814,8 @@ 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}, sub { $self->log($_[0], $_[1]); });
     }
 
     die $err if $err;
@@ -994,7 +1015,8 @@ 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}, sub { $self->log($_[0], $_[1]); });
     }
 
     if ($err) {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH v4 qemu-server 8/9] migration: attempt to clean up potential left-over fleecing images

[Fiona Ebner]

Clean up left-over fleecing images before the guest is migrated to a
different node and they'd really become orphaned.

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

Changes in v4:
* order before loading the configuration for migration

 PVE/QemuMigrate.pm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 6591f3f7..32eeade2 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -177,6 +177,13 @@ sub prepare {
 
     my $storecfg = $self->{storecfg} = PVE::Storage::config();
 
+    # updates the configuration, so ordered before saving the configuration in $self
+    eval {
+	PVE::QemuConfig::cleanup_fleecing_images(
+	    $vmid, $storecfg, sub { $self->log($_[0], $_[1]); });
+    };
+    $self->log('warn', "attempt to clean up left-over fleecing images failed - $@") if $@;
+
     # test if VM exists
     my $conf = $self->{vmconf} = PVE::QemuConfig->load_config($vmid);
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH v4 qemu-server 9/9] destroy vm: clean up potential left-over fleecing images

[Fiona Ebner]

Avoids that any left-over fleecing images become orphaned.

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

No changes in v4.

 PVE/QemuServer.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 649843aa..24f0480e 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2234,6 +2234,9 @@ sub check_type {
 sub destroy_vm {
     my ($storecfg, $vmid, $skiplock, $replacement_conf, $purge_unreferenced) = @_;
 
+    eval { PVE::QemuConfig::cleanup_fleecing_images($vmid, $storecfg) };
+    log_warn("attempt to clean up left-over fleecing images failed - $@") if $@;
+
     my $conf = PVE::QemuConfig->load_config($vmid);
 
     if (!$skiplock && !PVE::QemuConfig->has_lock($conf, 'suspended')) {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] partially-applied: [PATCH-SERIES v4 docs/qemu-server] more robust handling of fleecing images

[Thomas Lamprecht]

Am 11.11.24 um 14:54 schrieb Fiona Ebner:
> Record the created fleecing images in the VM configuration to be able
> to remove left-overs after hard failures.
> 
> The new config key uses a hyphen. Add some documentation about why
> there are different styles.
> 
> v3 was part of the series adding fleecing:
> https://lore.proxmox.com/pve-devel/20240411092943.57377-19-f.ebner@proxmox.com/
> 
> 
> docs:
> 
> Fiona Ebner (2):
>   configuration files: add general section
>   cli appendix: reference section about casing style
> 
>  cli-general.adoc         |  6 ++++++
>  configuration-files.adoc | 28 ++++++++++++++++++++++++++++
>  pve-admin-guide.adoc     |  4 +++-
>  3 files changed, 37 insertions(+), 1 deletion(-)
>  create mode 100644 cli-general.adoc
>  create mode 100644 configuration-files.adoc
> 

applied above two and...

> 
> qemu-server:
> 
> Fiona Ebner (7):
>   backup: prepare: factor out getting running status
>   backup: prepare: cancel previous job if still running
>   parse config: allow config keys with minus sign
>   schema: add fleecing-images config property

applied above four, thanks! Below needs to be rebased due to the commit
moving around cleanup of fleecign that was applied by Fabian a few days
ago.

>   fix #5440: vzdump: better cleanup fleecing images after hard errors
>   migration: attempt to clean up potential left-over fleecing images
>   destroy vm: clean up potential left-over fleecing images
> 
>  PVE/API2/Qemu.pm         |  9 ++++++
>  PVE/QemuConfig.pm        | 68 ++++++++++++++++++++++++++++++++++++++++
>  PVE/QemuMigrate.pm       |  7 +++++
>  PVE/QemuServer.pm        | 12 ++++++-
>  PVE/VZDump/QemuServer.pm | 52 +++++++++++++++++++++++-------
>  5 files changed, 136 insertions(+), 12 deletions(-)
> 


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] partially-applied: [PATCH-SERIES v4 docs/qemu-server] more robust handling of fleecing images

[Fiona Ebner]

Am 17.11.24 um 19:43 schrieb Thomas Lamprecht:
> Am 11.11.24 um 14:54 schrieb Fiona Ebner:
>> qemu-server:
>>
>> Fiona Ebner (7):
>>   backup: prepare: factor out getting running status
>>   backup: prepare: cancel previous job if still running
>>   parse config: allow config keys with minus sign
>>   schema: add fleecing-images config property
> 
> applied above four, thanks! Below needs to be rebased due to the commit
> moving around cleanup of fleecign that was applied by Fabian a few days
> ago.
> 

Regarding the patch "schema: add fleecing-images config property",
Fabian off-list suggested using a config section "special:fleecing"
instead of a property, so that it is truly internal-only. If we go for
that, the commit should be reverted. Which approach do you prefer?


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] partially-applied: [PATCH-SERIES v4 docs/qemu-server] more robust handling of fleecing images

[Thomas Lamprecht]

Am 18.11.24 um 09:52 schrieb Fiona Ebner:
> Am 17.11.24 um 19:43 schrieb Thomas Lamprecht:
>> Am 11.11.24 um 14:54 schrieb Fiona Ebner:
>>> qemu-server:
>>>
>>> Fiona Ebner (7):
>>>   backup: prepare: factor out getting running status
>>>   backup: prepare: cancel previous job if still running
>>>   parse config: allow config keys with minus sign
>>>   schema: add fleecing-images config property
>>
>> applied above four, thanks! Below needs to be rebased due to the commit
>> moving around cleanup of fleecign that was applied by Fabian a few days
>> ago.
>>
> 
> Regarding the patch "schema: add fleecing-images config property",
> Fabian off-list suggested using a config section "special:fleecing"
> instead of a property, so that it is truly internal-only. If we go for
> that, the commit should be reverted. Which approach do you prefer?

hmm, that would be fine to me too, so go with whatever you find nicer
as long-term design.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] partially-applied: [PATCH-SERIES v4 docs/qemu-server] more robust handling of fleecing images

[Thomas Lamprecht]

Am 18.11.24 um 09:52 schrieb Fiona Ebner:
> Regarding the patch "schema: add fleecing-images config property",
> Fabian off-list suggested using a config section "special:fleecing"
> instead of a property, so that it is truly internal-only. If we go for
> that, the commit should be reverted. Which approach do you prefer?

FYI: I reverted that commit for now, this can always be implemented,
no need to rush it now if there are better ideas to represent it are
already floating around.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] partially-applied: [PATCH-SERIES v4 docs/qemu-server] more robust handling of fleecing images

[Fiona Ebner]

Am 18.11.24 um 21:31 schrieb Thomas Lamprecht:
> Am 18.11.24 um 09:52 schrieb Fiona Ebner:
>> Regarding the patch "schema: add fleecing-images config property",
>> Fabian off-list suggested using a config section "special:fleecing"
>> instead of a property, so that it is truly internal-only. If we go for
>> that, the commit should be reverted. Which approach do you prefer?
> 
> FYI: I reverted that commit for now, this can always be implemented,
> no need to rush it now if there are better ideas to represent it are
> already floating around.

Thank you! Yes, sorry, I didn't get around to flesh out the "special
section" approach yesterday.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel