[pve-devel] [PATCH-SERIES qemu(-server)/guest-common/manager/docs] make QEMU's max-workers setting configurable as a vzdump setting

[pve-devel] [PATCH-SERIES qemu(-server)/guest-common/manager/docs] make QEMU's max-workers setting configurable as a vzdump setting

[Fiona Ebner]

A few users reported IO-related issues during backup after upgrading
to PVE 7.x and using a modified QEMU build with max-workers reduced to
8 instead of 16 helped them [0].

Other BackupPerf settings are not exposed, see the qemu patch for why.

Introduces a more general 'performance' property string as a vzdump
setting to be expanded in the future. And in the long run, existing
performance settings like bwlimit could be moved into there.

To detect if the currently running QEMU supports the setting, the
query-proxmox-support QMP command is used. If not supported, a
warning is printed and the setting is ignored.

All but the last documentation patches are independent. The recently
created dedicated section for jobs is expanded and the introductory
section for backups is updated to mention PBS. The last patch mentions
the new performance/max-workers setting.

[0]: https://forum.proxmox.com/threads/113790/

Dependency bump manager -> guest-common is needed for the new
$PROPERTY_STRING hash.


qemu:

Fiona Ebner (1):
  PVE Backup: allow passing max-workers performance setting

 block/monitor/block-hmp-cmds.c |  4 +++-
 pve-backup.c                   | 18 +++++++++++++-----
 qapi/block-core.json           |  9 +++++++--
 3 files changed, 23 insertions(+), 8 deletions(-)


guest-common:

Fiona Ebner (1):
  vzdump: add 'performance' property string as a setting

 src/PVE/VZDump/Common.pm | 42 ++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)


qemu-server:

Fiona Ebner (1):
  vzdump: set max-workers QMP option when specified and supported

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


manager:

Fiona Ebner (1):
  vzdump: handle new 'performance' property string

 PVE/API2/VZDump.pm        |  9 +++++----
 PVE/Jobs/VZDump.pm        | 23 ++++++++++-------------
 PVE/VZDump.pm             | 12 ++++++++++++
 configs/vzdump.conf       |  1 +
 www/manager6/dc/Backup.js |  8 +++++---
 5 files changed, 33 insertions(+), 20 deletions(-)


docs:

Fiona Ebner (4):
  backup: rework storage section, mentioning and recommending PBS
  backup: expand section for jobs
  backup: merge sections describing jobs
  backup: mention max-workers performance setting

 vzdump.adoc | 61 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 21 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH qemu 1/1] PVE Backup: allow passing max-workers performance setting

[Fiona Ebner]

For query-proxmox-support, add an indication that it's possible to use
the setting.

For now, the other two BackupPerf settings are not exposed:

* use-copy-range: would need to be implemented by the backup-dump
block driver first, and in fact, the default for backup was changed,
because it wasn't as fast for backup in QEMU, see commit
6a30f663d4c0b3c45a544d541e0c4e214b2473a1.

* max-chunk: enforced to be at least the backup cluster size, which is
4 MiB for PBS and otherwise maximum of source and target cluster size.
And block-copy has a maximum buffer size of 1 MiB, so setting a larger
max-chunk doesn't even have an effect. To make the setting sensibly
usable the check would need to be removed and optionally the
block-copy max buffer size would need to be bumped. I tried doing just
that, and tested different source/target combinations with different
max-chunk settings, but there were no noticable improvements over the
default "unlimited" (resulting in 1 MiB for block-copy).

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 block/monitor/block-hmp-cmds.c |  4 +++-
 pve-backup.c                   | 18 +++++++++++++-----
 qapi/block-core.json           |  9 +++++++--
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 0502f42be6..cc231ec3f2 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -1049,7 +1049,9 @@ void coroutine_fn hmp_backup(Monitor *mon, const QDict *qdict)
         false, false, // PBS encrypt
         true, dir ? BACKUP_FORMAT_DIR : BACKUP_FORMAT_VMA,
         false, NULL, false, NULL, !!devlist,
-        devlist, qdict_haskey(qdict, "speed"), speed, &error);
+        devlist, qdict_haskey(qdict, "speed"), speed,
+        false, 0, // BackupPerf max-workers
+        &error);
 
     hmp_handle_error(mon, error);
 }
diff --git a/pve-backup.c b/pve-backup.c
index 2e22030eec..e9aa7e0f49 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -55,6 +55,7 @@ static struct PVEBackupState {
         bool starting;
     } stat;
     int64_t speed;
+    BackupPerf perf;
     VmaWriter *vmaw;
     ProxmoxBackupHandle *pbs;
     GList *di_list;
@@ -492,8 +493,6 @@ static void create_backup_jobs_bh(void *opaque) {
     }
     backup_state.txn = job_txn_new_seq();
 
-    BackupPerf perf = { .max_workers = 16 };
-
     /* create and start all jobs (paused state) */
     GList *l =  backup_state.di_list;
     while (l) {
@@ -513,8 +512,9 @@ static void create_backup_jobs_bh(void *opaque) {
 
         BlockJob *job = backup_job_create(
             NULL, di->bs, di->target, backup_state.speed, sync_mode, di->bitmap,
-            bitmap_mode, false, NULL, &perf, BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
-            JOB_DEFAULT, pvebackup_complete_cb, di, backup_state.txn, &local_err);
+            bitmap_mode, false, NULL, &backup_state.perf, BLOCKDEV_ON_ERROR_REPORT,
+            BLOCKDEV_ON_ERROR_REPORT, JOB_DEFAULT, pvebackup_complete_cb, di, backup_state.txn,
+            &local_err);
 
         di->job = job;
         if (job) {
@@ -584,7 +584,9 @@ UuidInfo coroutine_fn *qmp_backup(
     bool has_config_file, const char *config_file,
     bool has_firewall_file, const char *firewall_file,
     bool has_devlist, const char *devlist,
-    bool has_speed, int64_t speed, Error **errp)
+    bool has_speed, int64_t speed,
+    bool has_max_workers, int64_t max_workers,
+    Error **errp)
 {
     assert(qemu_in_coroutine());
 
@@ -914,6 +916,11 @@ UuidInfo coroutine_fn *qmp_backup(
 
     backup_state.speed = (has_speed && speed > 0) ? speed : 0;
 
+    backup_state.perf = (BackupPerf){ .max_workers = 16 };
+    if (has_max_workers) {
+        backup_state.perf.max_workers = max_workers;
+    }
+
     backup_state.vmaw = vmaw;
     backup_state.pbs = pbs;
 
@@ -1089,5 +1096,6 @@ ProxmoxSupportStatus *qmp_query_proxmox_support(Error **errp)
     ret->pbs_dirty_bitmap_migration = true;
     ret->query_bitmap_info = true;
     ret->pbs_masterkey = true;
+    ret->backup_max_workers = true;
     return ret;
 }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 89875f309c..43281aca79 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -833,6 +833,8 @@
 #
 # @encrypt: use encryption ((optional for format 'pbs', defaults to true if there is a keyfile)
 #
+# @max-workers: see @BackupPerf for details. Default 16.
+#
 # Returns: the uuid of the backup job
 #
 ##
@@ -851,7 +853,9 @@
                                     '*format': 'BackupFormat',
                                     '*config-file': 'str',
                                     '*firewall-file': 'str',
-                                    '*devlist': 'str', '*speed': 'int' },
+                                    '*devlist': 'str',
+                                    '*speed': 'int',
+                                    '*max-workers': 'int' },
   'returns': 'UuidInfo', 'coroutine': true }
 
 ##
@@ -906,7 +910,8 @@
             'pbs-dirty-bitmap-savevm': 'bool',
             'pbs-dirty-bitmap-migration': 'bool',
             'pbs-masterkey': 'bool',
-            'pbs-library-version': 'str' } }
+            'pbs-library-version': 'str',
+            'backup-max-workers': 'bool' } }
 
 ##
 # @query-proxmox-support:
-- 
2.30.2

[pve-devel] [PATCH guest-common 1/1] vzdump: add 'performance' property string as a setting

[Fiona Ebner]

Initially, to be used for tuning backup performance with QEMU.

A few users reported IO-related issues during backup after upgrading
to PVE 7.x and using a modified QEMU build with max-workers reduced to
8 instead of 16 helped them [0].

Also generalizes the way vzdump property string are handled for easier
extension in the future.

[0]: https://forum.proxmox.com/threads/113790/

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/VZDump/Common.pm | 42 ++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index cb46de1..8b8b75a 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -29,16 +29,21 @@ my $dowhash_to_dow = sub {
     return join ',', @da;
 };
 
-my $fixup_prune_backups_option = sub {
+our $PROPERTY_STRINGS = {
+    'performance' => 'backup-performance',
+    'prune-backups' => 'prune-backups',
+};
+
+my sub parse_property_strings {
     my ($opts) = @_;
 
-    return if !defined($opts->{'prune-backups'});
+    for my $opt (keys $PROPERTY_STRINGS->%*) {
+	next if !defined($opts->{$opt});
 
-    $opts->{'prune-backups'} = PVE::JSONSchema::parse_property_string(
-	'prune-backups',
-	$opts->{'prune-backups'}
-    );
-};
+	my $format = $PROPERTY_STRINGS->{$opt};
+	$opts->{$opt} = PVE::JSONSchema::parse_property_string($format, $opts->{$opt});
+    }
+}
 
 # parse crontab style day of week
 sub parse_dow {
@@ -71,6 +76,17 @@ sub parse_dow {
     return $res;
 };
 
+PVE::JSONSchema::register_format('backup-performance', {
+    'max-workers' => {
+	description => "Applies to VMs. Allow up to this many IO workers at the same time.",
+	type => 'integer',
+	minimum => 1,
+	maximum => 256,
+	default => 16,
+	optional => 1,
+    },
+});
+
 my $confdesc = {
     vmid => {
 	type => 'string', format => 'pve-vmid-list',
@@ -196,6 +212,12 @@ my $confdesc = {
 	maximum => 8,
 	default => 7,
     },
+    performance => {
+	type => 'string',
+	description => "Other performance-related settings.",
+	format => 'backup-performance',
+	optional => 1,
+    },
     lockwait => {
 	type => 'integer',
 	description => "Maximal time to wait for the global lock (minutes).",
@@ -311,7 +333,7 @@ sub parse_vzdump_cron_config {
 		$opts->{starttime} = sprintf "%02d:%02d", $hour, $minute;
 		$opts->{dow} = &$dowhash_to_dow($dowhash);
 
-		$fixup_prune_backups_option->($opts); # parse the property string
+		parse_property_strings($opts);
 
 		push @$jobs, $opts;
 	    };
@@ -402,8 +424,8 @@ sub command_line {
 	    }
 	} else {
 	    $v = join(",", PVE::Tools::split_list($v)) if $p eq 'mailto';
-	    $v = PVE::JSONSchema::print_property_string($v, 'prune-backups')
-		if $p eq 'prune-backups';
+	    $v = PVE::JSONSchema::print_property_string($v, $PROPERTY_STRINGS->{$p})
+		if $PROPERTY_STRINGS->{$p};
 
 	    $cmd .= " --$p " . PVE::Tools::shellquote($v) if defined($v) && $v ne '';
 	}
-- 
2.30.2

[pve-devel] [PATCH qemu-server 1/1] vzdump: set max-workers QMP option when specified and supported

[Fiona Ebner]

If not supported, a warning is printed and the setting is ignored.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/VZDump/QemuServer.pm | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index bf0d1c56..7fb51c7d 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -15,6 +15,7 @@ use PVE::INotify;
 use PVE::IPCC;
 use PVE::JSONSchema;
 use PVE::PBSClient;
+use PVE::RESTEnvironment qw(log_warn);
 use PVE::QMPClient;
 use PVE::Storage::Plugin;
 use PVE::Storage::PBSPlugin;
@@ -453,6 +454,26 @@ my $detach_tpmstate_drive = sub {
     eval { PVE::QemuServer::qemu_drivedel($vmid, "tpmstate0-backup"); };
 };
 
+my sub add_backup_performance_options {
+    my ($qmp_param, $perf, $qemu_support) = @_;
+
+    return if !$perf || scalar(keys $perf->%*) == 0;
+
+    if (!$qemu_support) {
+	my $settings_string = join(', ', sort keys $perf->%*);
+	log_warn("ignoring setting(s): $settings_string - issue checking if supported");
+	return;
+    }
+
+    if (defined($perf->{'max-workers'})) {
+	if ($qemu_support->{'backup-max-workers'}) {
+	    $qmp_param->{'max-workers'} = int($perf->{'max-workers'});
+	} else {
+	    log_warn("ignoring 'max-workers' setting - not supported by running QEMU");
+	}
+    }
+}
+
 sub archive_pbs {
     my ($self, $task, $vmid) = @_;
 
@@ -548,7 +569,10 @@ sub archive_pbs {
 	if (defined(my $ns = $scfg->{namespace})) {
 	    $params->{'backup-ns'} = $ns;
 	}
+
 	$params->{speed} = $opts->{bwlimit}*1024 if $opts->{bwlimit};
+	add_backup_performance_options($params, $opts->{performance}, $qemu_support);
+
 	$params->{fingerprint} = $fingerprint if defined($fingerprint);
 	$params->{'firewall-file'} = $firewall if -e $firewall;
 	if (-e $keyfile) {
@@ -716,6 +740,11 @@ sub archive_vma {
 	    die "interrupted by signal\n";
 	};
 
+	# Currently, failing to determine Proxmox support is not critical here, because it's only
+	# used for performance settings like 'max-workers'.
+	my $qemu_support = eval { mon_cmd($vmid, "query-proxmox-support") };
+	log_warn($@) if $@;
+
 	$attach_tpmstate_drive->($self, $task, $vmid);
 
 	my $outfh;
@@ -746,6 +775,7 @@ sub archive_vma {
 		devlist => $devlist
 	    };
 	    $params->{'firewall-file'} = $firewall if -e $firewall;
+	    add_backup_performance_options($params, $opts->{performance}, $qemu_support);
 
 	    $qmpclient->queue_cmd($vmid, $backup_cb, 'backup', %$params);
 	};
-- 
2.30.2

[pve-devel] [PATCH manager 1/1] vzdump: handle new 'performance' property string

[Fiona Ebner]

Also generalizes the way vzdump property strings are handled for jobs.
Something similar could be done in VZDump.pm, but there the maxfiles
and prune-backups settings are currently coupled, so a dedicated
parse_performance() is used instead. Can be changed once maxfiles is
dropped.

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

Dependency bump for libpve-guest-common-perl needed.

 PVE/API2/VZDump.pm        |  9 +++++----
 PVE/Jobs/VZDump.pm        | 23 ++++++++++-------------
 PVE/VZDump.pm             | 12 ++++++++++++
 configs/vzdump.conf       |  1 +
 www/manager6/dc/Backup.js |  8 +++++---
 5 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 13b6cd46..82b28db5 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -25,9 +25,10 @@ __PACKAGE__->register_method ({
     method => 'POST',
     description => "Create backup.",
     permissions => {
-	description => "The user needs 'VM.Backup' permissions on any VM, and 'Datastore.AllocateSpace'"
-	    ." on the backup storage. The 'maxfiles', 'prune-backups', 'tmpdir', 'dumpdir', 'script',"
-	    ." 'bwlimit' and 'ionice' parameters are restricted to the 'root\@pam' user.",
+	description => "The user needs 'VM.Backup' permissions on any VM, and "
+	    ."'Datastore.AllocateSpace' on the backup storage. The 'maxfiles', 'prune-backups', "
+	    ."'tmpdir', 'dumpdir', 'script', 'bwlimit', 'performance' and 'ionice' parameters are "
+	    ."restricted to the 'root\@pam' user.",
 	user => 'all',
     },
     protected => 1,
@@ -60,7 +61,7 @@ __PACKAGE__->register_method ({
 		if $param->{stdout};
 	}
 
-	foreach my $key (qw(maxfiles prune-backups tmpdir dumpdir script bwlimit ionice)) {
+	for my $key (qw(maxfiles prune-backups tmpdir dumpdir script bwlimit performance ionice)) {
 	    raise_param_exc({ $key => "Only root may set this option."})
 		if defined($param->{$key}) && ($user ne 'root@pam');
 	}
diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
index 7feb06a2..2963b348 100644
--- a/PVE/Jobs/VZDump.pm
+++ b/PVE/Jobs/VZDump.pm
@@ -42,11 +42,8 @@ sub options {
 sub decode_value {
     my ($class, $type, $key, $value) = @_;
 
-    if ($key eq 'prune-backups' && !ref($value)) {
-	$value = PVE::JSONSchema::parse_property_string(
-	    'prune-backups',
-	    $value,
-	);
+    if ((my $format = $PVE::VZDump::Common::PROPERTY_STRINGS->{$key}) && !ref($value)) {
+	$value = PVE::JSONSchema::parse_property_string($format, $value);
     }
 
     return $value;
@@ -55,11 +52,8 @@ sub decode_value {
 sub encode_value {
     my ($class, $type, $key, $value) = @_;
 
-    if ($key eq 'prune-backups' && ref($value) eq 'HASH') {
-	$value = PVE::JSONSchema::print_property_string(
-	    $value,
-	    'prune-backups',
-	);
+    if ((my $format = $PVE::VZDump::Common::PROPERTY_STRINGS->{$key}) && ref($value) eq 'HASH') {
+	$value = PVE::JSONSchema::print_property_string($value, $format);
     }
 
     return $value;
@@ -73,9 +67,12 @@ sub run {
 	delete $conf->{$opt} if !defined($props->{$opt});
     }
 
-    my $retention = $conf->{'prune-backups'};
-    if ($retention && ref($retention) eq 'HASH') { # fixup, its required as string parameter
-	$conf->{'prune-backups'} = PVE::JSONSchema::print_property_string($retention, 'prune-backups');
+    # Required as string parameters
+    for my $key (keys $PVE::VZDump::Common::PROPERTY_STRINGS->%*) {
+	if ($conf->{$key} && ref($conf->{$key}) eq 'HASH') {
+	    my $format = $PVE::VZDump::Common::PROPERTY_STRINGS->{$key};
+	    $conf->{$key} = PVE::JSONSchema::print_property_string($conf->{$key}, $format);
+	}
     }
 
     $conf->{quiet} = 1; # do not write to stdout/stderr
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 87bb0bd2..d5b3c500 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -122,6 +122,15 @@ my $generate_notes = sub {
     return $notes_template;
 };
 
+my sub parse_performance {
+    my ($param) = @_;
+
+    if (defined(my $perf = $param->{performance})) {
+	return if ref($perf) eq 'HASH'; # already parsed
+	$param->{performance} = PVE::JSONSchema::parse_property_string('backup-performance', $perf);
+    }
+}
+
 my $parse_prune_backups_maxfiles = sub {
     my ($param, $kind) = @_;
 
@@ -261,6 +270,7 @@ sub read_vzdump_defaults {
 	} keys %$confdesc_for_defaults
     };
     $parse_prune_backups_maxfiles->($defaults, "defaults in VZDump schema");
+    parse_performance($defaults);
 
     my $raw;
     eval { $raw = PVE::Tools::file_get_contents($fn); };
@@ -276,6 +286,7 @@ sub read_vzdump_defaults {
 	$res->{mailto} = [ @mailto ];
     }
     $parse_prune_backups_maxfiles->($res, "options in '$fn'");
+    parse_performance($res);
 
     foreach my $key (keys %$defaults) {
 	$res->{$key} = $defaults->{$key} if !defined($res->{$key});
@@ -1354,6 +1365,7 @@ sub verify_vzdump_parameters {
 	if defined($param->{'prune-backups'}) && defined($param->{maxfiles});
 
     $parse_prune_backups_maxfiles->($param, 'CLI parameters');
+    parse_performance($param);
 
     if (my $template = $param->{'notes-template'}) {
 	eval { $verify_notes_template->($template); };
diff --git a/configs/vzdump.conf b/configs/vzdump.conf
index fe4b18ab..2ea09ae0 100644
--- a/configs/vzdump.conf
+++ b/configs/vzdump.conf
@@ -5,6 +5,7 @@
 #storage: STORAGE_ID
 #mode: snapshot|suspend|stop
 #bwlimit: KBPS
+#performance: max-workers=N
 #ionice: PRI
 #lockwait: MINUTES
 #stopwait: MINUTES
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index e9d74fb6..c81a76ce 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -594,9 +594,11 @@ Ext.define('PVE.dc.BackupView', {
 	    delete job['repeat-missed'];
 	    job.all = job.all === true ? 1 : 0;
 
-	    if (job['prune-backups']) {
-		job['prune-backups'] = PVE.Parser.printPropertyString(job['prune-backups']);
-	    }
+	    ['performance', 'prune-backups'].forEach(key => {
+		if (job[key]) {
+		    job[key] = PVE.Parser.printPropertyString(job[key]);
+		}
+	    });
 
 	    let allNodes = PVE.data.ResourceStore.getNodes();
 	    let nodes = allNodes.filter(node => node.status === 'online').map(node => node.node);
-- 
2.30.2

[pve-devel] [PATCH docs 1/4] backup: rework storage section, mentioning and recommending PBS

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 vzdump.adoc | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/vzdump.adoc b/vzdump.adoc
index 115c740..0c5b32e 100644
--- a/vzdump.adoc
+++ b/vzdump.adoc
@@ -37,12 +37,14 @@ the `vzdump` command line tool.
 
 .Backup Storage
 
-Before a backup can run, a backup storage must be defined. Refer to
-the Storage documentation on how to add a storage. A backup storage
-must be a file level storage, as backups are stored as regular files.
-In most situations, using a NFS server is a good way to store backups.
-You can save those backups later to a tape drive, for off-site
-archiving.
+Before a backup can run, a backup storage must be defined. Refer to the
+xref:chapter_storage[storage documentation] on how to add a storage. It can
+either be a Proxmox Backup Server storage, where backups are stored as
+de-duplicated chunks and metadata, or a file-level storage, where backups are
+stored as regular files. Using Proxmox Backup Server on a dedicated host is
+recommended, because of its advanced features. Using an NFS server is a good
+alternative. In both cases, you might want to save those backups later to a tape
+drive, for off-site archiving.
 
 .Scheduled Backup
 
-- 
2.30.2

[pve-devel] [PATCH docs 2/4] backup: expand section for jobs

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 vzdump.adoc | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/vzdump.adoc b/vzdump.adoc
index 0c5b32e..01e1c20 100644
--- a/vzdump.adoc
+++ b/vzdump.adoc
@@ -200,9 +200,27 @@ Backup Jobs
 -----------
 
 Besides triggering a backup manually, you can also setup periodic jobs that
-backup all, or a selection of virtual guest to a storage.
-
-// TODO: extend, link to retention below, ... di & document perf max-worker settings
+backup all, or a selection of virtual guest to a storage. You can manage the
+jobs in the UI under 'Datacenter' -> 'Backup' or via the `/cluster/backup` API
+endpoint.
+
+A job is either configured for all cluster nodes or a specific node, and is
+executed according to a given schedule. The format for the schedule is very
+similar to `systemd` calendar events, see the
+xref:chapter_calendar_events[calendar events] section for details. The
+'Schedule' field in the UI can be freely edited, and it contains several
+examples that can be used as a starting point in its drop-down list.
+
+You can configure job-specific xref:vzdump_retention[retention options]
+overriding those from the storage or node configuration, as well as a
+xref:vzdump_notes[template for notes] for additional information to be saved
+together with the backup.
+
+There are a few settings for tuning backup performance not exposed in the UI.
+The most notable is `bwlimit` for limiting IO bandwidth. The amount of threads
+used for the compressor can be controlled with the `pigz` (replacing `gzip`),
+respectively, `zstd` setting. Furthermore, there is `ionice`. See the
+xref:vzdump_configuration[configuration options] for details.
 
 [[vzdump_retention]]
 Backup Retention
-- 
2.30.2

[pve-devel] [PATCH docs 3/4] backup: merge sections describing jobs

[Fiona Ebner]

keeping a short intro referring to the full section at the beginning.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 vzdump.adoc | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/vzdump.adoc b/vzdump.adoc
index 01e1c20..841cf3d 100644
--- a/vzdump.adoc
+++ b/vzdump.adoc
@@ -48,19 +48,9 @@ drive, for off-site archiving.
 
 .Scheduled Backup
 
-Backup jobs can be scheduled so that they are executed automatically
-on specific days and times, for selectable nodes and guest systems.
-Configuration of scheduled backups is done at the Datacenter level in
-the GUI, which will generate a job entry in /etc/pve/jobs.cfg, which
-will in turn be parsed and executed by the `pvescheduler` daemon.
-These jobs use the xref:chapter_calendar_events[calendar events] for
-defining the schedule.
-
-Since scheduled backups miss their execution when the host was offline or the
-pvescheduler was disabled during the scheduled time, it is possible to configure
-the behaviour for catching up. By enabling the `Repeat missed` option
-(`repeat-missed` in the config), you can tell the scheduler that it should run
-missed jobs as soon as possible.
+Backup jobs can be scheduled so that they are executed automatically on specific
+days and times, for selectable nodes and guest systems. See the
+xref:vzdump_jobs[Backup Jobs] section for more.
 
 Backup Modes
 ------------
@@ -196,13 +186,15 @@ Backup Encryption
 For Proxmox Backup Server storages, you can optionally set up client-side
 encryption of backups, see xref:storage_pbs_encryption[the corresponding section.]
 
+[[vzdump_jobs]]
 Backup Jobs
 -----------
 
 Besides triggering a backup manually, you can also setup periodic jobs that
 backup all, or a selection of virtual guest to a storage. You can manage the
 jobs in the UI under 'Datacenter' -> 'Backup' or via the `/cluster/backup` API
-endpoint.
+endpoint. Both will generate job entries in `/etc/pve/jobs.cfg`, which are
+parsed and executed by the `pvescheduler` daemon.
 
 A job is either configured for all cluster nodes or a specific node, and is
 executed according to a given schedule. The format for the schedule is very
@@ -216,6 +208,12 @@ overriding those from the storage or node configuration, as well as a
 xref:vzdump_notes[template for notes] for additional information to be saved
 together with the backup.
 
+Since scheduled backups miss their execution when the host was offline or the
+pvescheduler was disabled during the scheduled time, it is possible to configure
+the behaviour for catching up. By enabling the `Repeat missed` option
+(`repeat-missed` in the config), you can tell the scheduler that it should run
+missed jobs as soon as possible.
+
 There are a few settings for tuning backup performance not exposed in the UI.
 The most notable is `bwlimit` for limiting IO bandwidth. The amount of threads
 used for the compressor can be controlled with the `pigz` (replacing `gzip`),
-- 
2.30.2

[pve-devel] [PATCH docs 4/4] backup: mention max-workers performance setting

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 vzdump.adoc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/vzdump.adoc b/vzdump.adoc
index 841cf3d..8658e38 100644
--- a/vzdump.adoc
+++ b/vzdump.adoc
@@ -217,7 +217,8 @@ missed jobs as soon as possible.
 There are a few settings for tuning backup performance not exposed in the UI.
 The most notable is `bwlimit` for limiting IO bandwidth. The amount of threads
 used for the compressor can be controlled with the `pigz` (replacing `gzip`),
-respectively, `zstd` setting. Furthermore, there is `ionice`. See the
+respectively, `zstd` setting. Furthermore, there are `ionice` and, as part of
+the `performance` setting, `max-workers` (affects VM backups only). See the
 xref:vzdump_configuration[configuration options] for details.
 
 [[vzdump_retention]]
-- 
2.30.2

[pve-devel] applied: [PATCH qemu 1/1] PVE Backup: allow passing max-workers performance setting

[Wolfgang Bumiller]

applied this one, thanks

On Mon, Oct 03, 2022 at 03:52:04PM +0200, Fiona Ebner wrote:
> For query-proxmox-support, add an indication that it's possible to use
> the setting.
> 
> For now, the other two BackupPerf settings are not exposed:
> 
> * use-copy-range: would need to be implemented by the backup-dump
> block driver first, and in fact, the default for backup was changed,
> because it wasn't as fast for backup in QEMU, see commit
> 6a30f663d4c0b3c45a544d541e0c4e214b2473a1.
> 
> * max-chunk: enforced to be at least the backup cluster size, which is
> 4 MiB for PBS and otherwise maximum of source and target cluster size.
> And block-copy has a maximum buffer size of 1 MiB, so setting a larger
> max-chunk doesn't even have an effect. To make the setting sensibly
> usable the check would need to be removed and optionally the
> block-copy max buffer size would need to be bumped. I tried doing just
> that, and tested different source/target combinations with different
> max-chunk settings, but there were no noticable improvements over the
> default "unlimited" (resulting in 1 MiB for block-copy).
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  block/monitor/block-hmp-cmds.c |  4 +++-
>  pve-backup.c                   | 18 +++++++++++++-----
>  qapi/block-core.json           |  9 +++++++--
>  3 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
> index 0502f42be6..cc231ec3f2 100644
> --- a/block/monitor/block-hmp-cmds.c
> +++ b/block/monitor/block-hmp-cmds.c
> @@ -1049,7 +1049,9 @@ void coroutine_fn hmp_backup(Monitor *mon, const QDict *qdict)
>          false, false, // PBS encrypt
>          true, dir ? BACKUP_FORMAT_DIR : BACKUP_FORMAT_VMA,
>          false, NULL, false, NULL, !!devlist,
> -        devlist, qdict_haskey(qdict, "speed"), speed, &error);
> +        devlist, qdict_haskey(qdict, "speed"), speed,
> +        false, 0, // BackupPerf max-workers
> +        &error);
>  
>      hmp_handle_error(mon, error);
>  }
> diff --git a/pve-backup.c b/pve-backup.c
> index 2e22030eec..e9aa7e0f49 100644
> --- a/pve-backup.c
> +++ b/pve-backup.c
> @@ -55,6 +55,7 @@ static struct PVEBackupState {
>          bool starting;
>      } stat;
>      int64_t speed;
> +    BackupPerf perf;
>      VmaWriter *vmaw;
>      ProxmoxBackupHandle *pbs;
>      GList *di_list;
> @@ -492,8 +493,6 @@ static void create_backup_jobs_bh(void *opaque) {
>      }
>      backup_state.txn = job_txn_new_seq();
>  
> -    BackupPerf perf = { .max_workers = 16 };
> -
>      /* create and start all jobs (paused state) */
>      GList *l =  backup_state.di_list;
>      while (l) {
> @@ -513,8 +512,9 @@ static void create_backup_jobs_bh(void *opaque) {
>  
>          BlockJob *job = backup_job_create(
>              NULL, di->bs, di->target, backup_state.speed, sync_mode, di->bitmap,
> -            bitmap_mode, false, NULL, &perf, BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
> -            JOB_DEFAULT, pvebackup_complete_cb, di, backup_state.txn, &local_err);
> +            bitmap_mode, false, NULL, &backup_state.perf, BLOCKDEV_ON_ERROR_REPORT,
> +            BLOCKDEV_ON_ERROR_REPORT, JOB_DEFAULT, pvebackup_complete_cb, di, backup_state.txn,
> +            &local_err);
>  
>          di->job = job;
>          if (job) {
> @@ -584,7 +584,9 @@ UuidInfo coroutine_fn *qmp_backup(
>      bool has_config_file, const char *config_file,
>      bool has_firewall_file, const char *firewall_file,
>      bool has_devlist, const char *devlist,
> -    bool has_speed, int64_t speed, Error **errp)
> +    bool has_speed, int64_t speed,
> +    bool has_max_workers, int64_t max_workers,
> +    Error **errp)
>  {
>      assert(qemu_in_coroutine());
>  
> @@ -914,6 +916,11 @@ UuidInfo coroutine_fn *qmp_backup(
>  
>      backup_state.speed = (has_speed && speed > 0) ? speed : 0;
>  
> +    backup_state.perf = (BackupPerf){ .max_workers = 16 };
> +    if (has_max_workers) {
> +        backup_state.perf.max_workers = max_workers;
> +    }
> +
>      backup_state.vmaw = vmaw;
>      backup_state.pbs = pbs;
>  
> @@ -1089,5 +1096,6 @@ ProxmoxSupportStatus *qmp_query_proxmox_support(Error **errp)
>      ret->pbs_dirty_bitmap_migration = true;
>      ret->query_bitmap_info = true;
>      ret->pbs_masterkey = true;
> +    ret->backup_max_workers = true;
>      return ret;
>  }
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 89875f309c..43281aca79 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -833,6 +833,8 @@
>  #
>  # @encrypt: use encryption ((optional for format 'pbs', defaults to true if there is a keyfile)
>  #
> +# @max-workers: see @BackupPerf for details. Default 16.
> +#
>  # Returns: the uuid of the backup job
>  #
>  ##
> @@ -851,7 +853,9 @@
>                                      '*format': 'BackupFormat',
>                                      '*config-file': 'str',
>                                      '*firewall-file': 'str',
> -                                    '*devlist': 'str', '*speed': 'int' },
> +                                    '*devlist': 'str',
> +                                    '*speed': 'int',
> +                                    '*max-workers': 'int' },
>    'returns': 'UuidInfo', 'coroutine': true }
>  
>  ##
> @@ -906,7 +910,8 @@
>              'pbs-dirty-bitmap-savevm': 'bool',
>              'pbs-dirty-bitmap-migration': 'bool',
>              'pbs-masterkey': 'bool',
> -            'pbs-library-version': 'str' } }
> +            'pbs-library-version': 'str',
> +            'backup-max-workers': 'bool' } }
>  
>  ##
>  # @query-proxmox-support:
> -- 
> 2.30.2

[pve-devel] applied-series: [PATCH-SERIES qemu(-server)/guest-common/manager/docs] make QEMU's max-workers setting configurable as a vzdump setting

[Wolfgang Bumiller]

applied series

On Mon, Oct 03, 2022 at 03:52:03PM +0200, Fiona Ebner wrote:
> A few users reported IO-related issues during backup after upgrading
> to PVE 7.x and using a modified QEMU build with max-workers reduced to
> 8 instead of 16 helped them [0].
> 
> Other BackupPerf settings are not exposed, see the qemu patch for why.
> 
> Introduces a more general 'performance' property string as a vzdump
> setting to be expanded in the future. And in the long run, existing
> performance settings like bwlimit could be moved into there.
> 
> To detect if the currently running QEMU supports the setting, the
> query-proxmox-support QMP command is used. If not supported, a
> warning is printed and the setting is ignored.
> 
> All but the last documentation patches are independent. The recently
> created dedicated section for jobs is expanded and the introductory
> section for backups is updated to mention PBS. The last patch mentions
> the new performance/max-workers setting.
> 
> [0]: https://forum.proxmox.com/threads/113790/
> 
> Dependency bump manager -> guest-common is needed for the new
> $PROPERTY_STRING hash.
> 
> 
> qemu:
> 
> Fiona Ebner (1):
>   PVE Backup: allow passing max-workers performance setting
> 
>  block/monitor/block-hmp-cmds.c |  4 +++-
>  pve-backup.c                   | 18 +++++++++++++-----
>  qapi/block-core.json           |  9 +++++++--
>  3 files changed, 23 insertions(+), 8 deletions(-)
> 
> 
> guest-common:
> 
> Fiona Ebner (1):
>   vzdump: add 'performance' property string as a setting
> 
>  src/PVE/VZDump/Common.pm | 42 ++++++++++++++++++++++++++++++----------
>  1 file changed, 32 insertions(+), 10 deletions(-)
> 
> 
> qemu-server:
> 
> Fiona Ebner (1):
>   vzdump: set max-workers QMP option when specified and supported
> 
>  PVE/VZDump/QemuServer.pm | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> 
> manager:
> 
> Fiona Ebner (1):
>   vzdump: handle new 'performance' property string
> 
>  PVE/API2/VZDump.pm        |  9 +++++----
>  PVE/Jobs/VZDump.pm        | 23 ++++++++++-------------
>  PVE/VZDump.pm             | 12 ++++++++++++
>  configs/vzdump.conf       |  1 +
>  www/manager6/dc/Backup.js |  8 +++++---
>  5 files changed, 33 insertions(+), 20 deletions(-)
> 
> 
> docs:
> 
> Fiona Ebner (4):
>   backup: rework storage section, mentioning and recommending PBS
>   backup: expand section for jobs
>   backup: merge sections describing jobs
>   backup: mention max-workers performance setting
> 
>  vzdump.adoc | 61 +++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 40 insertions(+), 21 deletions(-)
> 
> -- 
> 2.30.2