[pve-devel] applied: [PATCH v2 pve-qemu-kvm] patches: pbs block driver: correct a data type
Am 10.06.24 um 14:05 schrieb Jing Luo: > gcc warns (-Werror=type-limits) that it will always be false for the > if statement. This is because here s->aid is defined as char, while > proxmox_restore_open_image() returns an int. > > This is probably because chars are treated as unsigned on arm arch but > signed on x86 arch: > > https://developer.arm.com/documentation/den0013/d/Porting/Miscellaneous-C-porting-issues/unsigned-char-and-signed-char > > Signed-off-by: Jing Luo > -- applied, thanks! Here, a '-' was missing, so the below stuff would still be interpreted as part of the commit message. I fixed that when applying, improved the message slightly and made a follow-up to have a dedicated error message for when the returned value is too large. Best Regards, Fiona ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC qemu 2/7] block/copy-before-write: allow passing additional options for bdrv_cbw_append()
In preparation to allow configuring 'on-cbw-error' and 'cbw-timeout'
for a backup job. The previously already passed-in 'min-cluster-size'
option is also passed-in via the QDict now.
Signed-off-by: Fiona Ebner
---
block/backup.c| 9 -
block/copy-before-write.c | 8
block/copy-before-write.h | 2 +-
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index fe69723ada..e0acfe6758 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -21,6 +21,7 @@
#include "block/block_backup.h"
#include "block/block-copy.h"
#include "block/dirty-bitmap.h"
+#include "block/qdict.h"
#include "qapi/error.h"
#include "qemu/cutils.h"
#include "sysemu/block-backend.h"
@@ -346,6 +347,7 @@ BlockJob *backup_job_create(const char *job_id,
BlockDriverState *bs,
int64_t cluster_size;
BlockDriverState *cbw = NULL;
BlockCopyState *bcs = NULL;
+QDict *cbw_opts = NULL;
assert(bs);
assert(target);
@@ -433,8 +435,13 @@ BlockJob *backup_job_create(const char *job_id,
BlockDriverState *bs,
goto error;
}
+if (perf->has_min_cluster_size) {
+cbw_opts = qdict_new();
+qdict_put_int(cbw_opts, "min-cluster-size", perf->min_cluster_size);
+}
+
cbw = bdrv_cbw_append(bs, target, filter_node_name, discard_source,
- perf->min_cluster_size, &bcs, errp);
+ cbw_opts, &bcs, errp);
if (!cbw) {
goto error;
}
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 50cc4c7aae..5e0c1252f6 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -546,26 +546,26 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState
*source,
BlockDriverState *target,
const char *filter_node_name,
bool discard_source,
- int64_t min_cluster_size,
+ QDict *opts,
BlockCopyState **bcs,
Error **errp)
{
BDRVCopyBeforeWriteState *state;
BlockDriverState *top;
-QDict *opts;
int flags = BDRV_O_RDWR | (discard_source ? BDRV_O_CBW_DISCARD_SOURCE : 0);
assert(source->total_sectors == target->total_sectors);
GLOBAL_STATE_CODE();
-opts = qdict_new();
+if (!opts) {
+opts = qdict_new();
+}
qdict_put_str(opts, "driver", "copy-before-write");
if (filter_node_name) {
qdict_put_str(opts, "node-name", filter_node_name);
}
qdict_put_str(opts, "file", bdrv_get_node_name(source));
qdict_put_str(opts, "target", bdrv_get_node_name(target));
-qdict_put_int(opts, "min-cluster-size", min_cluster_size);
top = bdrv_insert_node(source, opts, flags, errp);
if (!top) {
diff --git a/block/copy-before-write.h b/block/copy-before-write.h
index a27d2d7d9f..4c22bd282e 100644
--- a/block/copy-before-write.h
+++ b/block/copy-before-write.h
@@ -40,7 +40,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
BlockDriverState *target,
const char *filter_node_name,
bool discard_source,
- int64_t min_cluster_size,
+ QDict *opts,
BlockCopyState **bcs,
Error **errp);
void bdrv_cbw_drop(BlockDriverState *bs);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC qemu 5/7] fix #3231+#3631: PVE backup: add timeout for copy-before-write operations and fail backup instead of guest writes
If the backup target can't be reached or is very slow, then the
default behavior for QEMU backup is to break the guest write. This is
undesirable and it is more expected and less intrusive to make the
backup error out instead.
A timeout of 45 seconds for copy-before-write operations is set, like
for fleecing. Guest drivers like virtio-win have issues when a write
takes more than 60 seconds and still completes afterwards, so a value
below that was chosen.
Unfortunately, with this alone, the backup would still try to run to
completion and fail only at the very end. This can be improved by
adding a callback function that will abort the backup once a
copy-before-write operation fails.
Signed-off-by: Fiona Ebner
---
pve-backup.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/pve-backup.c b/pve-backup.c
index 108e185a20..9843d8d122 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -560,6 +560,7 @@ static void create_backup_jobs_bh(void *opaque) {
bdrv_drained_begin(di->bs);
BackupPerf perf = (BackupPerf){ .max_workers =
backup_state.perf.max_workers };
+QDict *backup_cbw_opts = qdict_new();
BlockDriverState *source_bs = di->bs;
bool discard_source = false;
@@ -631,11 +632,18 @@ static void create_backup_jobs_bh(void *opaque) {
perf.min_cluster_size = MAX(perf.min_cluster_size,
bdi.cluster_size);
}
perf.has_min_cluster_size = true;
+} else {
+/*
+ * When fleecing is not used, need to set the options on the
copy-before-write node
+ * installed by the backup job itself.
+ */
+qdict_put_str(backup_cbw_opts, "on-cbw-error", "break-snapshot");
+qdict_put_int(backup_cbw_opts, "cbw-timeout", 45);
}
BlockJob *job = backup_job_create(
-job_id, source_bs, di->target, backup_state.speed, sync_mode,
di->bitmap,
-bitmap_mode, false, discard_source, NULL, &perf, NULL,
BLOCKDEV_ON_ERROR_REPORT,
+job_id, source_bs, di->target, backup_state.speed, sync_mode,
di->bitmap, bitmap_mode,
+false, discard_source, NULL, &perf, backup_cbw_opts,
BLOCKDEV_ON_ERROR_REPORT,
BLOCKDEV_ON_ERROR_REPORT, JOB_DEFAULT, pvebackup_complete_cb, di,
backup_state.txn,
&local_err);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC qemu 7/7] block/backup: set callback for cbw errors
The callback is invoked when cbw is configured to not break the guest
write and will abort a backup job immediately. Currently the backup
has to wait for the rest of the block copy operation to finish before
checking the cbw error state.
Signed-off-by: Fiona Ebner
---
Note for testers: if e.g. the PBS is compeletly unreachable, the
backup job still will need to wait until the in-flight request is
aborted after 15 minutes. But the guest writes should be fast again.
block/backup.c | 41 +
1 file changed, 41 insertions(+)
diff --git a/block/backup.c b/block/backup.c
index ba153110d3..43d34ce4c2 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -32,6 +32,45 @@
static const BlockJobDriver backup_job_driver;
+typedef struct {
+Job *job;
+int ret;
+} BackupOnCbwError;
+
+static void backup_on_cbw_error_cb_bh(void *opaque)
+{
+BackupOnCbwError *data = opaque;
+if (data->job) {
+WITH_JOB_LOCK_GUARD() {
+if (!job_is_completed_locked(data->job)) {
+error_report("backup was cancelled because of
copy-before-write error: %s",
+ strerror(-data->ret));
+job_cancel_locked(data->job, true);
+}
+}
+} else {
+error_report("backup_on_cbw_error_cb_bh: no job! Error: %s",
strerror(-data->ret));
+}
+
+g_free(data);
+}
+
+static void backup_on_cbw_error_cb(void *opaque, int ret)
+{
+BackupOnCbwError *data = g_new0(BackupOnCbwError, 1);
+data->job = opaque;
+data->ret = ret;
+
+/*
+ * backup_cancel() cannot run in coroutine context.
+ */
+if (qemu_in_coroutine()) {
+aio_bh_schedule_oneshot(qemu_get_aio_context(),
backup_on_cbw_error_cb_bh, data);
+} else {
+backup_on_cbw_error_cb_bh(data);
+}
+}
+
static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret)
{
BdrvDirtyBitmap *bm;
@@ -477,6 +516,8 @@ BlockJob *backup_job_create(const char *job_id,
BlockDriverState *bs,
goto error;
}
+bdrv_cbw_set_error_cb(cbw, backup_on_cbw_error_cb, job);
+
job->cbw = cbw;
job->source_bs = bs;
job->target_bs = target;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC qemu 4/7] block/backup: make cbw error also fail backup that does not use fleecing
If the backup target can't be reached or is very slow, then the
default behavior for QEMU backup is to break the guest write. While an
option to change this was introduced with the 'on-cbw-error' option
for the copy-before-write filter, it only took an effect in
combination with a snapshot-access node like is used for backup
fleecing.
In preparation to set the 'on-cbw-error' option for PVE backups that
do not use fleecing.
Unfortunately, the name for the option is "break-snapshot", but in the
non-fleecing case, there is no snapshot-access node involved, so it's
a bit of a misnomer.
Signed-off-by: Fiona Ebner
---
block/backup.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/block/backup.c b/block/backup.c
index 82fedf1680..ba153110d3 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -202,6 +202,9 @@ static int coroutine_fn backup_loop(BackupBlockJob *job)
out:
block_copy_call_free(s);
job->bg_bcs_call = NULL;
+if (!ret) {
+ret = bdrv_cbw_snapshot_error(job->cbw);
+}
return ret;
}
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC qemu 3/7] block/backup: allow passing additional options for copy-before-write upon job creation
In particular, useful for setting the 'on-cbw-error' and 'cbw-timeout'
options (see BlockdevOptionsCbw in QAPI).
Signed-off-by: Fiona Ebner
---
block/backup.c | 10 +++---
block/replication.c| 2 +-
blockdev.c | 2 +-
include/block/block_int-global-state.h | 2 ++
pve-backup.c | 2 +-
5 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index e0acfe6758..82fedf1680 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -336,6 +336,7 @@ BlockJob *backup_job_create(const char *job_id,
BlockDriverState *bs,
bool compress, bool discard_source,
const char *filter_node_name,
BackupPerf *perf,
+ QDict *cbw_opts,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
int creation_flags,
@@ -347,7 +348,6 @@ BlockJob *backup_job_create(const char *job_id,
BlockDriverState *bs,
int64_t cluster_size;
BlockDriverState *cbw = NULL;
BlockCopyState *bcs = NULL;
-QDict *cbw_opts = NULL;
assert(bs);
assert(target);
@@ -436,8 +436,12 @@ BlockJob *backup_job_create(const char *job_id,
BlockDriverState *bs,
}
if (perf->has_min_cluster_size) {
-cbw_opts = qdict_new();
-qdict_put_int(cbw_opts, "min-cluster-size", perf->min_cluster_size);
+if (!cbw_opts) {
+cbw_opts = qdict_new();
+}
+if (!qdict_haskey(cbw_opts, "min-cluster-size")) {
+qdict_put_int(cbw_opts, "min-cluster-size",
perf->min_cluster_size);
+}
}
cbw = bdrv_cbw_append(bs, target, filter_node_name, discard_source,
diff --git a/block/replication.c b/block/replication.c
index 0415a5e8b7..c5a27f593e 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -583,7 +583,7 @@ static void replication_start(ReplicationState *rs,
ReplicationMode mode,
s->backup_job = backup_job_create(
NULL, s->secondary_disk->bs,
s->hidden_disk->bs,
0, MIRROR_SYNC_MODE_NONE, NULL, 0, false,
false,
-NULL, &perf,
+NULL, &perf, NULL,
BLOCKDEV_ON_ERROR_REPORT,
BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL,
backup_job_completed, bs, NULL, &local_err);
diff --git a/blockdev.c b/blockdev.c
index cbe224387b..55ca967430 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2732,7 +2732,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,
backup->sync, bmap, backup->bitmap_mode,
backup->compress, backup->discard_source,
backup->filter_node_name,
-&perf,
+&perf, NULL,
backup->on_source_error,
backup->on_target_error,
job_flags, NULL, NULL, txn, errp);
diff --git a/include/block/block_int-global-state.h
b/include/block/block_int-global-state.h
index f0c642b194..7332680087 100644
--- a/include/block/block_int-global-state.h
+++ b/include/block/block_int-global-state.h
@@ -179,6 +179,7 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
* @bitmap_mode: The bitmap synchronization policy to use.
* @perf: Performance options. All actual fields assumed to be present,
*all ".has_*" fields are ignored.
+ * @cbw_opts: Additional options to configure cbw filter with.
* @on_source_error: The action to take upon error reading from the source.
* @on_target_error: The action to take upon error writing to the target.
* @creation_flags: Flags that control the behavior of the Job lifetime.
@@ -198,6 +199,7 @@ BlockJob *backup_job_create(const char *job_id,
BlockDriverState *bs,
bool compress, bool discard_source,
const char *filter_node_name,
BackupPerf *perf,
+QDict *cbw_opts,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
int creation_flags,
diff --git a/pve-backup.c b/pve-backup.c
index 4e80a9f283..108e185a20 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -635,7 +635,7 @@ static void create_backup_jobs_bh(void *opaque) {
BlockJob *job = backup_job_create(
job_id, source_bs, di->target, backup_state.speed, sync_mode,
di->bitmap,
-bitmap_mode, false, discard_source, NULL, &perf,
BLOCKDEV_ON_ERROR_REPORT,
+bitmap_mode, false, discard_source, NULL, &perf, NULL,
BLOCKDEV_ON_ERROR_REPORT,
BLOCKDEV_ON_ERROR_REPORT, JOB_DEFAULT, pvebackup_com
[pve-devel] [RFC qemu 6/7] block/copy-before-write: allow specifying error callback
The callback is invoked when cbw is configured to not break the guest
write. Will be useful to be able to abort a backup job immediately.
Currently, it has to wait for the rest of the block copy operation to
finish before checking the cbw error state. Since the job will be
required for the callback and the cbw node is created before the
backup job, the error callback can only be set after the cbw node is
created. That is why a dedicated function is used for that.
Signed-off-by: Fiona Ebner
---
block/copy-before-write.c | 33 -
block/copy-before-write.h | 7 +++
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 5e0c1252f6..00ce1e957e 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -79,6 +79,9 @@ typedef struct BDRVCopyBeforeWriteState {
* atomics.
*/
int snapshot_error;
+
+CbwErrorCallbackFunc error_cb;
+void *error_cb_opaque;
} BDRVCopyBeforeWriteState;
static int coroutine_fn GRAPH_RDLOCK
@@ -140,7 +143,15 @@ static coroutine_fn int
cbw_do_copy_before_write(BlockDriverState *bs,
WITH_QEMU_LOCK_GUARD(&s->lock) {
if (ret < 0) {
assert(s->on_cbw_error == ON_CBW_ERROR_BREAK_SNAPSHOT);
-qatomic_cmpxchg(&s->snapshot_error, 0, ret);
+int old = qatomic_cmpxchg(&s->snapshot_error, 0, ret);
+CbwErrorCallbackFunc error_cb = qatomic_load_acquire(&s->error_cb);
+/*
+ * Only invoke error callback the first time. There can be massive
amounts of errros
+ * when the backup target cannot be reached.
+ */
+if (!old && error_cb) {
+error_cb(s->error_cb_opaque, ret);
+}
} else {
bdrv_set_dirty_bitmap(s->done_bitmap, off, end - off);
}
@@ -591,6 +602,26 @@ int bdrv_cbw_snapshot_error(BlockDriverState *bs)
return qatomic_read(&s->snapshot_error);
}
+void bdrv_cbw_set_error_cb(BlockDriverState *bs, CbwErrorCallbackFunc error_cb,
+ void *error_cb_opaque)
+{
+BDRVCopyBeforeWriteState *s = bs->opaque;
+
+/*
+ * Supporting multiple calls requires using locking or indirection of
callback+arguments into
+ * another struct whose pointer can then be set with atomics.
+ */
+assert(!s->error_cb);
+
+s->error_cb_opaque = error_cb_opaque;
+/*
+ * The only user of this function is during initalization of the backup
job while drained. And
+ * while there should be enough stuff happening in between to ensure the
callback and argument
+ * is seen by cbw_do_copy_before_write(), play it safe.
+ */
+qatomic_store_release(&s->error_cb, error_cb);
+}
+
static void cbw_init(void)
{
bdrv_register(&bdrv_cbw_filter);
diff --git a/block/copy-before-write.h b/block/copy-before-write.h
index 4c22bd282e..cddbd5d9a2 100644
--- a/block/copy-before-write.h
+++ b/block/copy-before-write.h
@@ -29,6 +29,8 @@
#include "block/block_int.h"
#include "block/block-copy.h"
+typedef void (*CbwErrorCallbackFunc)(void *opaque, int error);
+
/*
* Global state (GS) API. These functions run under the BQL.
*
@@ -45,5 +47,10 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
Error **errp);
void bdrv_cbw_drop(BlockDriverState *bs);
int bdrv_cbw_snapshot_error(BlockDriverState *bs);
+/*
+ * Can only be called once for a given cbw node.
+ */
+void bdrv_cbw_set_error_cb(BlockDriverState *bs, CbwErrorCallbackFunc error_cb,
+ void *error_cb_opaque);
#endif /* COPY_BEFORE_WRITE_H */
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC qemu] fix #3231+#3631: PVE backup: fail backup rather than guest write when backup target cannot be reached or is too slow
A long-standing issue with VM backups in Proxmox VE is that a slow or unreachable target would lead to a copy-before-write (cbw) operation to break the guest write rather than abort the backup. This is unexpected to users and the will end up without a successful backup and without a working guest in such cases. This series prevents the latter by changing the behavior to fail the backup instead of the guest write. This is done by re-using the already existing 'on-cbw-error' and 'cbw-timeout' options that are already used for fleecing and having regular backup also check for the cbw's snapshot_error (unfortunately this becomes a bit of a misnomer). If a given copy-before-write operation cannot complete within 45 seconds, it's extremely likely that aborting the backup is the better choice than keeping the guest IO blocked. Just checking for the error already makes it work (i.e. without the last two patches), but backup can only check the error at the end. To abort backup immediately, an error callback for the copy-before-write node is introduced. A potential alternative would be give the block-copy operation a pointer to the snapshot_error and have it check it during its operation, but my initial attempt failed. Likely I missed adapting certain logic that checks for whether the block-copy operation failed and it's questionable if this approach would be cleaner. An error callback is nice and explicit. Note for testers: if e.g. the PBS is compeletly unreachable, the backup job still will need to wait until the in-flight request is aborted after 15 minutes. But the guest writes should be fast again. Should it really be required to make the option more flexible, i.e. allow users to specify a custom timeout or go back to the old behavior then the 'backup' QMP call can be extended with those parameters. Unfortunately, this is a non-trivial amount of code to make it work, but there is quite a bit of boilerplate and some comments, so hopefully the logic is straight-forward enough. The first patch can be applied regardless of whether we want to go with this or not. Fiona Ebner (7): PVE backup: fleecing: properly set minimum cluster size block/copy-before-write: allow passing additional options for bdrv_cbw_append() block/backup: allow passing additional options for copy-before-write upon job creation block/backup: make cbw error also fail backup that does not use fleecing fix #3231+#3631: PVE backup: add timeout for copy-before-write operations and fail backup instead of guest writes block/copy-before-write: allow specifying error callback block/backup: set callback for cbw errors block/backup.c | 57 +- block/copy-before-write.c | 41 +++--- block/copy-before-write.h | 9 +++- block/replication.c| 2 +- blockdev.c | 2 +- include/block/block_int-global-state.h | 2 + pve-backup.c | 13 +- 7 files changed, 115 insertions(+), 11 deletions(-) -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu 1/7] PVE backup: fleecing: properly set minimum cluster size
While the current code doesn't care, prepare for future changes
relying on the flag being set as well.
Signed-off-by: Fiona Ebner
---
To be squashed into "PVE backup: add fleecing option"
pve-backup.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/pve-backup.c b/pve-backup.c
index 07709aa350..4e80a9f283 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -630,6 +630,7 @@ static void create_backup_jobs_bh(void *opaque) {
if (bdrv_get_info(di->fleecing.bs, &bdi) >= 0) {
perf.min_cluster_size = MAX(perf.min_cluster_size,
bdi.cluster_size);
}
+perf.has_min_cluster_size = true;
}
BlockJob *job = backup_job_create(
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-firewall 1/1] rules: allow vital ICMP(v6) types
There are certain ICMP messages that should always pass through a
firewall irregardless of any other rules. This is particularly
important for ICMPv6. While we already handled NDP, there are certain
control messages that should always be able to pass through any
firewall, according to RFC 4890.
For ICMP we additionally allow 'Source Quench' as well.
Signed-off-by: Stefan Hanreich
---
While Source Quench is deprecated, there might be niche use cases
using it and allowing it shouldn't really hurt so I've thrown it into
the mix as well.
.../resources/proxmox-firewall.nft| 22 +--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/proxmox-firewall/resources/proxmox-firewall.nft
b/proxmox-firewall/resources/proxmox-firewall.nft
index 537ba88..ea2cd7d 100644
--- a/proxmox-firewall/resources/proxmox-firewall.nft
+++ b/proxmox-firewall/resources/proxmox-firewall.nft
@@ -16,6 +16,7 @@ add chain inet proxmox-firewall allow-ndp-out
add chain inet proxmox-firewall block-ndp-out
add chain inet proxmox-firewall block-conntrack-invalid
add chain inet proxmox-firewall block-smurfs
+add chain inet proxmox-firewall allow-icmp
add chain inet proxmox-firewall log-drop-smurfs
add chain inet proxmox-firewall default-in
add chain inet proxmox-firewall default-out
@@ -32,6 +33,7 @@ add chain bridge proxmox-firewall-guests allow-ndp-out
add chain bridge proxmox-firewall-guests block-ndp-out
add chain bridge proxmox-firewall-guests allow-ra-out
add chain bridge proxmox-firewall-guests block-ra-out
+add chain bridge proxmox-firewall-guests allow-icmp
add chain bridge proxmox-firewall-guests do-reject
add chain bridge proxmox-firewall-guests vm-out {type filter hook prerouting
priority 0; policy accept;}
add chain bridge proxmox-firewall-guests vm-in {type filter hook postrouting
priority 0; policy accept;}
@@ -47,6 +49,7 @@ flush chain inet proxmox-firewall allow-ndp-out
flush chain inet proxmox-firewall block-ndp-out
flush chain inet proxmox-firewall block-conntrack-invalid
flush chain inet proxmox-firewall block-smurfs
+flush chain inet proxmox-firewall allow-icmp
flush chain inet proxmox-firewall log-drop-smurfs
flush chain inet proxmox-firewall default-in
flush chain inet proxmox-firewall default-out
@@ -63,6 +66,7 @@ flush chain bridge proxmox-firewall-guests allow-ndp-out
flush chain bridge proxmox-firewall-guests block-ndp-out
flush chain bridge proxmox-firewall-guests allow-ra-out
flush chain bridge proxmox-firewall-guests block-ra-out
+flush chain bridge proxmox-firewall-guests allow-icmp
flush chain bridge proxmox-firewall-guests do-reject
flush chain bridge proxmox-firewall-guests vm-out
flush chain bridge proxmox-firewall-guests vm-in
@@ -175,9 +179,16 @@ table inet proxmox-firewall {
drop
}
+chain allow-icmp {
+icmp type { destination-unreachable, source-quench, time-exceeded }
accept
+# based on RFC 4890 - NDP is handled separately
+icmpv6 type { destination-unreachable, packet-too-big, time-exceeded,
parameter-problem } accept
+}
+
chain default-in {
iifname "lo" accept
+jump allow-icmp
ct state related,established accept
meta l4proto igmp accept
@@ -185,8 +196,6 @@ table inet proxmox-firewall {
tcp dport { 8006, 5900-5999, 3128, 22 } jump accept-management
udp dport 5405-5412 accept
-meta l4proto icmp icmp type { destination-unreachable, time-exceeded }
accept
-
# Drop Microsoft SMB noise
udp dport { 135, 137-139, 445 } goto do-reject
udp sport 137 udp dport 1024-65535 goto do-reject
@@ -203,6 +212,7 @@ table inet proxmox-firewall {
chain default-out {
oifname "lo" accept
+jump allow-icmp
ct state vmap { invalid : drop, established : accept, related : accept
}
}
@@ -284,6 +294,12 @@ table bridge proxmox-firewall-guests {
icmpv6 type { nd-router-advert, nd-redirect } drop
}
+chain allow-icmp {
+icmp type { destination-unreachable, source-quench, time-exceeded }
accept
+# based on RFC 4890 - NDP is handled separately
+icmpv6 type { destination-unreachable, packet-too-big, time-exceeded,
parameter-problem } accept
+}
+
chain do-reject {
meta pkttype broadcast drop
ip saddr 224.0.0.0/4 drop
@@ -297,12 +313,14 @@ table bridge proxmox-firewall-guests {
chain vm-out {
type filter hook prerouting priority 0; policy accept;
+jump allow-icmp
ether type != arp ct state vmap { established : accept, related :
accept, invalid : drop }
iifname vmap @vm-map-out
}
chain vm-in {
type filter hook postrouting priority 0; policy accept;
+jump allow-icmp
ether type != arp ct state vmap { established : accept, related :
accept, invalid : drop }
ether type arp accept
oifname vmap @vm-map-in
--
2.39.2
_
[pve-devel] applied-series: [PATCH v4 pve-guest-common pve-container pve-manager 0/4] change-detection-mode for PBS
On June 10, 2024 11:57 am, Christian Ebner wrote:
> This patch series defines the vzdump configuration description as well
> as backup job configuration option in the WebUI to set the
> experimental `change-detection-mode` parameter to the
> proxmox-backup-client invocation for container backups having a Proxmox
> Backup Server storage target.
>
> This allows to either set the parameter to the node wide vzdump.conf
> or on a per job level. The following shows an exemplary job
> configuration taken from /etc/pve/jobs.cfg:
>
> ```
> vzdump: backup-ab2edf62-d43c
> schedule yearly
> compress zstd
> enabled 1
> fleecing 0
> mode snapshot
> notes-template {{guestname}}
> pbs-change-detection-mode metadata
> storage pbs-local
> vmid 100
> ```
>
> This patches depend on a proxmox-backup-client compiled with the
> following 2 patches applied:
> https://lists.proxmox.com/pipermail/pbs-devel/2024-June/009779.html
>
> Changes since version 3:
> - Use PBS default instead of redefining on PVE side
>
> pve-guest-common:
>
> Christian Ebner (1):
> vzdump: add PBS change detection mode configuration
>
> src/PVE/VZDump/Common.pm | 7 +++
> 1 file changed, 7 insertions(+)
>
> pve-container:
>
> Christian Ebner (1):
> vzdump: conditionally set PBS change detection mode option
>
> src/PVE/VZDump/LXC.pm | 2 ++
> 1 file changed, 2 insertions(+)
>
> pve-manager:
>
> Christian Ebner (2):
> www: advanced backup: add pbs change detection mode selector
> vzdump: add pbs-change-detection-mode to config template
>
> configs/vzdump.conf | 1 +
> www/manager6/panel/BackupAdvancedOptions.js | 23 +
> 2 files changed, 24 insertions(+)
>
> --
> 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
[pve-devel] [PATCH v4 pve-guest-common 1/4] vzdump: add PBS change detection mode configuration
Define the possible modes to be configured in a PBS file change
detection mode.
Signed-off-by: Christian Ebner
---
changes since version 3:
- dropped default value, use the PBS default
src/PVE/VZDump/Common.pm | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index 1539444..1996c5b 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -354,6 +354,13 @@ my $confdesc = {
requires => 'storage',
optional => 1,
},
+'pbs-change-detection-mode' => {
+ type => 'string',
+ description => "EXPERIMENTAL: PBS mode used to detect file changes and
switch encoding"
+ . " format for container backups.",
+ optional => 1,
+ enum => [ 'legacy', 'data', 'metadata' ],
+},
};
sub get_confdesc {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 pve-manager 4/4] vzdump: add pbs-change-detection-mode to config template
Include the additional parameter to set the `change-detection-mode`
for backup jobs with Proxmox Backup Server target as node wide
configuration, including possible variants to be set.
Signed-off-by: Christian Ebner
---
changes since version 3:
- no changes
configs/vzdump.conf | 1 +
1 file changed, 1 insertion(+)
diff --git a/configs/vzdump.conf b/configs/vzdump.conf
index 33893e69..54913073 100644
--- a/configs/vzdump.conf
+++ b/configs/vzdump.conf
@@ -16,3 +16,4 @@
#exclude-path: PATHLIST
#pigz: N
#notes-template: {{guestname}}
+#pbs-change-detection-mode: legacy|data|metadata
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 pve-manager 3/4] www: advanced backup: add pbs change detection mode selector
The proxmox backup client allows to switch the method used to encode
data based on a change-detection-mode parameter. Expose this setting
as experimental feature in the advanced panel for a backup job.
Signed-off-by: Christian Ebner
---
changes since version 3:
- drop default value on set
www/manager6/panel/BackupAdvancedOptions.js | 23 +
1 file changed, 23 insertions(+)
diff --git a/www/manager6/panel/BackupAdvancedOptions.js
b/www/manager6/panel/BackupAdvancedOptions.js
index 1026c6f4..7dd19f96 100644
--- a/www/manager6/panel/BackupAdvancedOptions.js
+++ b/www/manager6/panel/BackupAdvancedOptions.js
@@ -97,6 +97,9 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
}
delete values.fleecing;
}
+ if (values["pbs-change-detection-mode"] === '__default__') {
+ delete values["pbs-change-detection-mode"];
+ }
return values;
},
@@ -236,6 +239,26 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
value: gettext("Run jobs as soon as possible if they couldn't
start on schedule, for example, due to the node being offline."),
},
},
+ {
+ xtype: 'pveTwoColumnContainer',
+ startColumn: {
+ xtype: 'proxmoxKVComboBox',
+ fieldLabel: gettext('PBS change detection mode'),
+ name: 'pbs-change-detection-mode',
+ deleteEmpty: true,
+ value: '__default__',
+ comboItems: [
+ ['__default__', "Default"],
+ ['data', "Data"],
+ ['metadata', "Metadata"],
+ ],
+ },
+ endFlex: 2,
+ endColumn: {
+ xtype: 'displayfield',
+ value: gettext("EXPERIMENTAL: Mode to detect file changes and
archive encoding format for container backups."),
+ },
+ },
{
xtype: 'component',
padding: '5 1',
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 pve-container 2/4] vzdump: conditionally set PBS change detection mode option
Allows to switch the backup clients change detection mode based on
the option set in the backup jobs configuration for backup jobs with
Proxmox Backup Server target.
Signed-off-by: Christian Ebner
---
changes since version 4:
- no changes
src/PVE/VZDump/LXC.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/PVE/VZDump/LXC.pm b/src/PVE/VZDump/LXC.pm
index 8c28a5e..06e02eb 100644
--- a/src/PVE/VZDump/LXC.pm
+++ b/src/PVE/VZDump/LXC.pm
@@ -394,6 +394,8 @@ sub archive {
push @$param, '--backup-type', 'ct';
push @$param, '--backup-id', $vmid;
push @$param, '--backup-time', $task->{backup_time};
+ push @$param, '--change-detection-mode',
$opts->{"pbs-change-detection-mode"}
+ if $opts->{"pbs-change-detection-mode"};
if (my $entries_max = $opts->{"performance"}->{"pbs-entries-max"}) {
push $param->@*, '--entries-max', $entries_max;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 pve-guest-common pve-container pve-manager 0/4] change-detection-mode for PBS
This patch series defines the vzdump configuration description as well
as backup job configuration option in the WebUI to set the
experimental `change-detection-mode` parameter to the
proxmox-backup-client invocation for container backups having a Proxmox
Backup Server storage target.
This allows to either set the parameter to the node wide vzdump.conf
or on a per job level. The following shows an exemplary job
configuration taken from /etc/pve/jobs.cfg:
```
vzdump: backup-ab2edf62-d43c
schedule yearly
compress zstd
enabled 1
fleecing 0
mode snapshot
notes-template {{guestname}}
pbs-change-detection-mode metadata
storage pbs-local
vmid 100
```
This patches depend on a proxmox-backup-client compiled with the
following 2 patches applied:
https://lists.proxmox.com/pipermail/pbs-devel/2024-June/009779.html
Changes since version 3:
- Use PBS default instead of redefining on PVE side
pve-guest-common:
Christian Ebner (1):
vzdump: add PBS change detection mode configuration
src/PVE/VZDump/Common.pm | 7 +++
1 file changed, 7 insertions(+)
pve-container:
Christian Ebner (1):
vzdump: conditionally set PBS change detection mode option
src/PVE/VZDump/LXC.pm | 2 ++
1 file changed, 2 insertions(+)
pve-manager:
Christian Ebner (2):
www: advanced backup: add pbs change detection mode selector
vzdump: add pbs-change-detection-mode to config template
configs/vzdump.conf | 1 +
www/manager6/panel/BackupAdvancedOptions.js | 23 +
2 files changed, 24 insertions(+)
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH pve-qemu-kvm] patch: pbs block driver: correct a data type
Am 10.06.24 um 03:48 schrieb Jing Luo:
> On 2024-06-07 20:49, Fiona Ebner wrote:
>> Hi,
>>
>> if you haven't already done so, please send a signed copy of the Harmony
>> CLA to off...@proxmox.com, see [0].
>
> Hi, thanks for letting me know, this was done some time last week.
>
Okay :)
>> Am 07.06.24 um 11:43 schrieb Jing Luo:
>>> gcc warns (-Werror=type-limits) that it will always be false for the
>>> if statement. This is because here s->aid is defined as char, while
>>> proxmox_restore_open_image() returns an int. Change the type to int.
>>> Strangely gcc warns it on arm64 build but not amd64 build...
>>>
>>
>> Thank you for the report!
>>
>>> Signed-off-by: Jing Luo
>>> ---
>>> ...2-PVE-Add-PBS-block-driver-to-map-backup-archives-into.patch | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git
>>> a/debian/patches/pve/0032-PVE-Add-PBS-block-driver-to-map-backup-archives-into.patch
>>>
>>> b/debian/patches/pve/0032-PVE-Add-PBS-block-driver-to-map-backup-archives-into.patch
>>> index b9578ba..9e68167 100644
>>> ---
>>> a/debian/patches/pve/0032-PVE-Add-PBS-block-driver-to-map-backup-archives-into.patch
>>> +++
>>> b/debian/patches/pve/0032-PVE-Add-PBS-block-driver-to-map-backup-archives-into.patch
>>> @@ -68,7 +68,7 @@ index 00..dd72356bd3
>>> +
>>> +typedef struct {
>>> + ProxmoxRestoreHandle *conn;
>>> -+ char aid;
>>> ++ int aid;
>>> + int64_t length;
>>> +
>>> + char *repository;
>>
>> I'd rather make it an explicit uint8_t here (because that is the type
>> for other functions taking the aid as a parameter, e.g.
>> proxmox_restore_get_image_length()). And to fix the original issue, I'd
>> use the ret variable to store the result from
>> proxmox_restore_open_image() and only assign to s->aid after checking
>> that the returned value is not an error and that it is small enough to
>> fit into uint8_t.
>>
>> [0]:
>> https://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_Copyright
>
> Thanks. I'm more of a sysadmin than a programmer but I'll try my best. I
> might send a v2 patch in a few days. (Unless I need an urgent surgery to
> take out the fish bone deeply stuck in my throat since last
> Friday...I'll find out if I need the surgery after meeting with my
> doctor today:)
No need to hurry, and if you want to, I can also send a patch for this
with a Reported-by tag for you. Hope all goes well regarding the medical
issue!
Best Wishes,
Fiona
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH storage] volume import: assume target API version is at least 9
The storage API version has been bumped to at least 9 since
libpve-storage = 7.0-4. If the source node is on Proxmox VE 8, where
this change will come in, then the target node can be assumed to be
running either Proxmox VE 8 or, during upgrade, the latest version of
Proxmox VE 7.4, so it's safe to assume a storage API version of at
least 9 in all cases.
As reported by Maximiliano, the fact that the 'apiinfo' call was
guarded with a quiet eval could lead to strange errors for replication
on a customer system where an SSH connection could not always be
established, because the target's API version would fall back to 1.
Because of that, the '-base' argument would be missing for the import
call on the target which would in turn lead to an error about the
target ZFS volume already existing (rather than doing an incremental
sync).
Reported-by: Maximiliano Sandoval
Signed-off-by: Fiona Ebner
---
src/PVE/Storage.pm | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index f19a115..bee2361 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -709,7 +709,7 @@ sub storage_migrate_snapshot {
}
my $volume_import_prepare = sub {
-my ($volid, $format, $path, $apiver, $opts) = @_;
+my ($volid, $format, $path, $opts) = @_;
my $base_snapshot = $opts->{base_snapshot};
my $snapshot = $opts->{snapshot};
@@ -724,11 +724,11 @@ my $volume_import_prepare = sub {
if ($migration_snapshot) {
push @$recv, '-delete-snapshot', $snapshot;
}
-push @$recv, '-allow-rename', $allow_rename if $apiver >= 5;
+push @$recv, '-allow-rename', $allow_rename;
if (defined($base_snapshot)) {
# Check if the snapshot exists on the remote side:
- push @$recv, '-base', $base_snapshot if $apiver >= 9;
+ push @$recv, '-base', $base_snapshot;
}
return $recv;
@@ -814,12 +814,7 @@ sub storage_migrate {
$import_fn = "tcp://$net";
}
-my $target_apiver = 1; # if there is no apiinfo call, assume 1
-my $get_api_version = [@$ssh, 'pvesm', 'apiinfo'];
-my $match_api_version = sub { $target_apiver = $1 if $_[0] =~ m!^APIVER
(\d+)$!; };
-eval { run_command($get_api_version, logfunc => $match_api_version); };
-
-my $recv = [ @$ssh, '--', $volume_import_prepare->($target_volid, $format,
$import_fn, $target_apiver, $opts)->@* ];
+my $recv = [ @$ssh, '--', $volume_import_prepare->($target_volid, $format,
$import_fn, $opts)->@* ];
my $new_volid;
my $pattern = volume_imported_message(undef, 1);
@@ -911,8 +906,7 @@ sub storage_migrate {
run_command($cmds, logfunc => $match_volid_and_log);
}
- die "unable to get ID of the migrated volume\n"
- if !defined($new_volid) && $target_apiver >= 5;
+ die "unable to get ID of the migrated volume\n" if !defined($new_volid);
};
my $err = $@;
if ($opts->{migration_snapshot}) {
@@ -1961,7 +1955,7 @@ sub volume_import_start {
my $info = IO::File->new();
my $unix = $opts->{unix} // "/run/pve/storage-migrate-$vmid.$$.unix";
-my $import = $volume_import_prepare->($volid, $format, "unix://$unix",
APIVER, $opts);
+my $import = $volume_import_prepare->($volid, $format, "unix://$unix",
$opts);
unlink $unix;
my $cpid = open3($input, $info, $info, @$import)
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH pve-qemu-kvm] patch: pbs block driver: correct a data type
On Fri Jun 7, 2024 at 11:43 AM CEST, Jing Luo wrote:
> gcc warns (-Werror=type-limits) that it will always be false for the
> if statement. This is because here s->aid is defined as char, while
> proxmox_restore_open_image() returns an int. Change the type to int.
> Strangely gcc warns it on arm64 build but not amd64 build...
probably because char is unsigned on arm architectures (usually), but
signed on x86 [1]:
> Although the ARM architecture has the LDRSB instruction, that loads a
> signed byte into a 32-bit register with sign extension, the earliest
> versions of the architecture did not. It made sense at the time for
> the compiler to treat simple chars as unsigned, whereas on the x86
> simple chars are, by default, treated as signed.
[1]:
https://developer.arm.com/documentation/den0013/d/Porting/Miscellaneous-C-porting-issues/unsigned-char-and-signed-char
>
> Signed-off-by: Jing Luo
> ---
> ...2-PVE-Add-PBS-block-driver-to-map-backup-archives-into.patch | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/debian/patches/pve/0032-PVE-Add-PBS-block-driver-to-map-backup-archives-into.patch
>
> b/debian/patches/pve/0032-PVE-Add-PBS-block-driver-to-map-backup-archives-into.patch
> index b9578ba..9e68167 100644
> ---
> a/debian/patches/pve/0032-PVE-Add-PBS-block-driver-to-map-backup-archives-into.patch
> +++
> b/debian/patches/pve/0032-PVE-Add-PBS-block-driver-to-map-backup-archives-into.patch
> @@ -68,7 +68,7 @@ index 00..dd72356bd3
> +
> +typedef struct {
> +ProxmoxRestoreHandle *conn;
> -+char aid;
> ++int aid;
> +int64_t length;
> +
> +char *repository;
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v7 19/19] notifications: backport some rephrased parts from PBS docs
Most of the changes were done when adapting the PVE docs to
the new PBS notification system, so now we 'backport' those
improvements.
Signed-off-by: Lukas Wagner
---
notifications.adoc | 99 +++---
1 file changed, 67 insertions(+), 32 deletions(-)
diff --git a/notifications.adoc b/notifications.adoc
index 9c5228c..bdfebd0 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -9,37 +9,38 @@ Overview
[thumbnail="screenshot/gui-datacenter-notification-overview.png"]
-{pve} will send notifications if case of noteworthy events in the system.
-
-There are a number of different xref:notification_events[notification events],
-each with their own set of metadata fields that can be used in
-notification matchers.
-
-A xref:notification_matchers[notification matcher] determines
-_which_ notifications shall be sent _where_.
-A matcher has _match rules_, that can be used to
-match on certain notification properties (e.g. timestamp, severity,
-metadata fields).
-If a matcher matches a notification, the notification will be routed
-to a configurable set of notification targets.
-
-A xref:notification_targets[notification target] is an abstraction for a
-destination where a notification should be sent to - for instance,
-a Gotify server instance, or a set of email addresses.
-There are multiple types of notification targets, including
-`sendmail`, which uses the system's sendmail command to send emails,
-or `gotify`, which sends a notification to a Gotify instance.
+* {pve} emits xref:notification_events[Notification Events] in case of
+ storage replication failures, node fencing, finished/failed backups
+ and other events.
+ These events are handled by the notification system. A notification
+ event has metadata, for example a timestamp, a severity level,
+ a type, and other optional metadata fields.
+* xref:notification_matchers[Notification Matchers] route a notification
+ event to one or more notification targets. A matcher can have match
+ rules to selectively route based on the metadata of a notification event.
+* xref:notification_targets[Notification Targets] are a destination to
+ which a notification event is routed to by a matcher.
+ There are multiple types of target, mail-based (Sendmail and SMTP)
+ and Gotify.
+
+Backup jobs have a configurable xref:notification_mode[Notification Mode].
+It allows you to choose between the notification system and a legacy mode
+for sending notification emails. The legacy mode is equivalent to the
+way notifications were handled before {pve} 8.1.
The notification system can be configured in the GUI under
-Datacenter -> Notifications. The configuration is stored in
+Datacenter → Notifications. The configuration is stored in
`/etc/pve/notifications.cfg` and `/etc/pve/priv/notifications.cfg` -
the latter contains sensitive configuration options such as
-passwords or authentication tokens for notification targets.
+passwords or authentication tokens for notification targets and can
+only be read by `root`.
[[notification_targets]]
Notification Targets
+{pve} offers multiple types of notification targets.
+
[[notification_targets_sendmail]]
Sendmail
@@ -50,14 +51,19 @@ that handles the sending of email messages.
It is a command-line utility that allows users and applications to send emails
directly from the command line or from within scripts.
-The sendmail notification target uses the `sendmail` binary to send emails.
-
+The sendmail notification target uses the `sendmail` binary to send emails to a
+list of configured users or email addresses. If a user is selected as a
recipient,
+the email address configured in user's settings will be used.
+For the `root@pam` user, this is the email address entered during installation.
+A user's email address can be configured in
+`Datacenter → Permissions → Users`.
+If a user has no associated email address, no email will be sent.
NOTE: In standard {pve} installations, the `sendmail` binary is provided by
-Postfix. For this type of target to work correctly, it might be necessary to
-change Postfix's configuration so that it can correctly deliver emails.
-For cluster setups it is necessary to have a working Postfix configuration on
-every single cluster node.
+Postfix. It may be necessary to configure Postfix so that it can deliver
+mails correctly - for example by setting an external mail relay (smart host).
+In case of failed delivery, check the system logs for messages logged by
+the Postfix daemon.
The configuration for Sendmail target plugins has the following options:
@@ -90,6 +96,12 @@ SMTP
[thumbnail="screenshot/gui-datacenter-notification-smtp.png"]
SMTP notification targets can send emails directly to an SMTP mail relay.
+This target does not use the system's MTA to deliver emails.
+Similar to sendmail targets, if a user is selected as a recipient, the user's
configured
+email address will be used.
[pve-devel] [PATCH widget-toolkit v7 11/19] notification: matcher: match-field: show known fields/values
These changes introduce combogrid pickers for the 'field' and 'value'
form elements for 'match-field' match rules. The 'field' picker shows
a list of all known metadata fields, while the 'value' picker shows a
list of all known values, filtered depending on the current value of
'field'.
The list of known fields/values is retrieved from new API endpoints.
Some values are marked 'internal' by the backend. This means that the
'value' field was not user-created (counter example: backup job
IDs) and can therefore be used as a base for translations.
Signed-off-by: Lukas Wagner
Tested-by: Maximiliano Sandoval
---
src/data/model/NotificationConfig.js | 12 ++
src/window/NotificationMatcherEdit.js | 297 +-
2 files changed, 253 insertions(+), 56 deletions(-)
diff --git a/src/data/model/NotificationConfig.js
b/src/data/model/NotificationConfig.js
index e8ebf28..03cf317 100644
--- a/src/data/model/NotificationConfig.js
+++ b/src/data/model/NotificationConfig.js
@@ -15,3 +15,15 @@ Ext.define('proxmox-notification-matchers', {
},
idProperty: 'name',
});
+
+Ext.define('proxmox-notification-fields', {
+extend: 'Ext.data.Model',
+fields: ['name', 'description'],
+idProperty: 'name',
+});
+
+Ext.define('proxmox-notification-field-values', {
+extend: 'Ext.data.Model',
+fields: ['value', 'comment', 'field'],
+idProperty: 'value',
+});
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index e717ad7..be33efe 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -79,7 +79,7 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', {
labelWidth: 120,
},
-width: 700,
+width: 800,
initComponent: function() {
let me = this;
@@ -416,10 +416,22 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
let me = this;
let record = me.get('selectedRecord');
let currentData = record.get('data');
+
+ let newValue = [];
+
+ // Build equivalent regular expression if switching
+ // to 'regex' mode
+ if (value === 'regex') {
+ let regexVal = "^(";
+ regexVal += currentData.value.join('|') + ")$";
+ newValue.push(regexVal);
+ }
+
record.set({
data: {
...currentData,
type: value,
+ value: newValue,
},
});
},
@@ -441,6 +453,8 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
data: {
...currentData,
field: value,
+ // Reset value if field changes
+ value: [],
},
});
},
@@ -549,6 +563,9 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
column2: [
{
xtype: 'pmxNotificationMatchRuleSettings',
+ cbind: {
+ baseUrl: '{baseUrl}',
+ },
},
],
@@ -601,7 +618,7 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
let value = data.value;
text = Ext.String.format(gettext("Match field: {0}={1}"),
field, value);
iconCls = 'fa fa-square-o';
- if (!field || !value) {
+ if (!field || !value || (Ext.isArray(value) && !value.length)) {
iconCls += ' internal-error';
}
} break;
@@ -821,6 +838,11 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
if (type === undefined) {
type = "exact";
}
+
+ if (type === 'exact') {
+ matchedValue = matchedValue.split(',');
+ }
+
return {
type: 'match-field',
data: {
@@ -982,7 +1004,9 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
extend: 'Ext.panel.Panel',
xtype: 'pmxNotificationMatchRuleSettings',
+mixins: ['Proxmox.Mixin.CBind'],
border: false,
+layout: 'anchor',
items: [
{
@@ -1000,6 +1024,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings',
{
['notall', gettext('At least one rule does not match')],
['notany', gettext('No rule matches')],
],
+ // Hide initially to avoid glitches when opening the window
+ hidden: true,
bind: {
hidden: '{!showMatchingMode}',
disabled: '{!showMatchingMode}',
@@ -1011,7 +1037,8 @@ Ext.define('Proxmox.panel.NotificationMatchR
[pve-devel] [PATCH widget-toolkit v7 12/19] notification: matcher: move match-field formulas to local viewModel
This should make the code more cohesive and easier to follow.
No functional changes.
Signed-off-by: Lukas Wagner
Tested-by: Maximiliano Sandoval
---
src/window/NotificationMatcherEdit.js | 189 +-
1 file changed, 95 insertions(+), 94 deletions(-)
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index be33efe..559b405 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -380,15 +380,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
}
return !record.isRoot();
},
- typeIsMatchField: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- get: function(record) {
- return record?.get('type') === 'match-field';
- },
- },
typeIsMatchSeverity: {
bind: {
bindTo: '{selectedRecord}',
@@ -407,89 +398,13 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
return record?.get('type') === 'match-calendar';
},
},
- matchFieldType: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
- let currentData = record.get('data');
-
- let newValue = [];
-
- // Build equivalent regular expression if switching
- // to 'regex' mode
- if (value === 'regex') {
- let regexVal = "^(";
- regexVal += currentData.value.join('|') + ")$";
- newValue.push(regexVal);
- }
-
- record.set({
- data: {
- ...currentData,
- type: value,
- value: newValue,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.type;
- },
- },
- matchFieldField: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
- let currentData = record.get('data');
-
- record.set({
- data: {
- ...currentData,
- field: value,
- // Reset value if field changes
- value: [],
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.field;
- },
- },
- matchFieldValue: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
- let currentData = record.get('data');
- record.set({
- data: {
- ...currentData,
- value: value,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.value;
- },
- },
matchSeverityValue: {
bind: {
bindTo: '{selectedRecord}',
deep: true,
},
set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
+ let record = this.get('selectedRecord');
let currentData = record.get('data');
record.set({
data: {
@@ -1137,7 +1052,98 @@ Ext.define('Proxmox.panel.MatchFieldSettings', {
},
},
},
+viewModel: {
+ // parent is set in `initComponents`
+ formulas: {
+ typeIsMatchField: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-field';
+ },
+ },
+ isRegex: function(get) {
+ return get('matchFieldType') === 'regex';
+ },
+ matchFieldType: {
+ bind: {
+ bindTo: '{selecte
[pve-devel] [PATCH manager v7 09/19] ui: utils: add overrides for translatable notification fields/values
Signed-off-by: Lukas Wagner
---
www/manager6/Utils.js | 11 +++
1 file changed, 11 insertions(+)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 5b0d51eb..ea448bfb 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -2060,6 +2060,17 @@ Ext.define('PVE.Utils', {
zfscreate: [gettext('ZFS Storage'), gettext('Create')],
zfsremove: ['ZFS Pool', gettext('Remove')],
});
+
+ Proxmox.Utils.overrideNotificationFieldName({
+ 'job-id': gettext('Job ID'),
+ });
+
+ Proxmox.Utils.overrideNotificationFieldValue({
+ 'package-updates': gettext('Package updates are available'),
+ 'vzdump': gettext('Backup notifications'),
+ 'replication': gettext('Replication job notifications'),
+ 'fencing': gettext('Node fencing notifications'),
+ });
},
});
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 06/19] vzdump: apt: notification: do not include domain in 'hostname' field
- The man page warns about the usage of `hostname -f`, since a host
may have multiple domains (or none at all)
- The fallback PVE::INotify::nodename() already only returned the
hostname without the domain part
- Fencing notifications didn't include the domain part anyway
This may result in soft-breakage for any users who have already relied
on the domain being present. If there is need for it, it could include
a fqdn metadata field.
The hostname property used for rendering the notification template
is unaffected for now.
Signed-off-by: Lukas Wagner
---
PVE/API2/APT.pm | 3 ++-
PVE/VZDump.pm | 8
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index ec7c21b2..47c50961 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -348,7 +348,8 @@ __PACKAGE__->register_method({
# matchers.
my $metadata_fields = {
type => 'package-updates',
- hostname => $hostname,
+ # Hostname (without domain part)
+ hostname => PVE::INotify::nodename(),
};
PVE::Notify::info(
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 2167b289..db3a02a9 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -517,10 +517,9 @@ sub send_notification {
"See Task History for details!\n";
};
-my $hostname = get_hostname();
-
my $notification_props = {
- "hostname" => $hostname,
+ # Hostname, might include domain part
+ "hostname" => get_hostname(),
"error-message" => $err,
"guest-table" => build_guest_table($tasklist),
"logs" => $text_log_part,
@@ -531,7 +530,8 @@ sub send_notification {
my $fields = {
type => "vzdump",
- hostname => $hostname,
+ # Hostname (without domain part)
+ hostname => PVE::INotify::nodename(),
};
# Add backup-job metadata field in case this is a backup job.
$fields->{'job-id'} = $job_id if $job_id;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 02/19] api: jobs: vzdump: pass job 'job-id' parameter
This allows us to access us the backup job id in the send_notification
function, where we can set it as metadata for the notification.
Signed-off-by: Lukas Wagner
---
PVE/API2/VZDump.pm | 8
PVE/Jobs/VZDump.pm | 4 +++-
PVE/VZDump.pm | 6 +++---
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 7f92e7ec..84dbc100 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -53,6 +53,14 @@ __PACKAGE__->register_method ({
parameters => {
additionalProperties => 0,
properties => PVE::VZDump::Common::json_config_properties({
+ 'job-id' => {
+ description => "The ID of the backup job. If set, the
'backup-job' metadata field"
+ . " of the backup notification will be set to this value.",
+ type => 'string',
+ format => 'pve-configid',
+ maxLength => 256,
+ optional => 1,
+ },
stdout => {
type => 'boolean',
description => "Write tar to stdout, not to a file.",
diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
index b8e57945..2dad3f55 100644
--- a/PVE/Jobs/VZDump.pm
+++ b/PVE/Jobs/VZDump.pm
@@ -12,7 +12,7 @@ use PVE::API2::VZDump;
use base qw(PVE::VZDump::JobBase);
sub run {
-my ($class, $conf) = @_;
+my ($class, $conf, $job_id) = @_;
my $props = $class->properties();
# remove all non vzdump related options
@@ -20,6 +20,8 @@ sub run {
delete $conf->{$opt} if !defined($props->{$opt});
}
+$conf->{'job-id'} = $job_id;
+
# Required as string parameters # FIXME why?! we could just check ref()
for my $key (keys $PVE::VZDump::Common::PROPERTY_STRINGS->%*) {
if ($conf->{$key} && ref($conf->{$key}) eq 'HASH') {
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 5b7080f3..2167b289 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -483,6 +483,7 @@ sub send_notification {
my ($self, $tasklist, $total_time, $err, $detail_pre, $detail_post) = @_;
my $opts = $self->{opts};
+my $job_id = $opts->{'job-id'};
my $mailto = $opts->{mailto};
my $cmdline = $self->{cmdline};
my $policy = $opts->{mailnotification} // 'always';
@@ -529,12 +530,11 @@ sub send_notification {
};
my $fields = {
- # TODO: There is no straight-forward way yet to get the
- # backup job id here... (I think pvescheduler would need
- # to pass that to the vzdump call?)
type => "vzdump",
hostname => $hostname,
};
+# Add backup-job metadata field in case this is a backup job.
+$fields->{'job-id'} = $job_id if $job_id;
my $severity = $failed ? "error" : "info";
my $email_configured = $mailto && scalar(@$mailto);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-guest-common v7 01/19] vzdump: common: allow 'job-id' as a parameter without being in schema
'job-id' is passed when a backup as started as a job and will be
passed to the notification system as matchable metadata. It it
can be considered 'internal'.
Signed-off-by: Lukas Wagner
---
src/PVE/VZDump/Common.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index 1539444..e835b05 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -496,7 +496,7 @@ sub command_line {
foreach my $p (keys %$param) {
next if $p eq 'id' || $p eq 'vmid' || $p eq 'starttime' ||
- $p eq 'dow' || $p eq 'stdout' || $p eq 'enabled';
+ $p eq 'dow' || $p eq 'stdout' || $p eq 'enabled' || $p eq
'job-id';
my $v = $param->{$p};
my $pd = $confdesc->{$p} || die "no such vzdump option '$p'\n";
if ($p eq 'exclude-path') {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH widget-toolkit v7 14/19] notification: matcher: move match-severity fields to panel
Also introduce a local viewModel that is linked to a parent viewModel,
allowing us to move the formulas to the panel.
This should make the code more cohesive and easier to follow.
No functional changes.
Signed-off-by: Lukas Wagner
Tested-by: Maximiliano Sandoval
---
src/window/NotificationMatcherEdit.js | 129 --
1 file changed, 80 insertions(+), 49 deletions(-)
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index 50145e3..9ab443f 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -380,34 +380,7 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
}
return !record.isRoot();
},
- typeIsMatchSeverity: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- get: function(record) {
- return record?.get('type') === 'match-severity';
- },
- },
- matchSeverityValue: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let record = this.get('selectedRecord');
- let currentData = record.get('data');
- record.set({
- data: {
- ...currentData,
- value: value,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.value;
- },
- },
+
rootMode: {
bind: {
bindTo: '{selectedRecord}',
@@ -944,27 +917,7 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
},
},
{
- xtype: 'proxmoxKVComboBox',
- fieldLabel: gettext('Severities to match'),
- isFormField: false,
- allowBlank: true,
- multiSelect: true,
- field: 'value',
- // Hide initially to avoid glitches when opening the window
- hidden: true,
- bind: {
- value: '{matchSeverityValue}',
- hidden: '{!typeIsMatchSeverity}',
- disabled: '{!typeIsMatchSeverity}',
- },
-
- comboItems: [
- ['info', gettext('Info')],
- ['notice', gettext('Notice')],
- ['warning', gettext('Warning')],
- ['error', gettext('Error')],
- ['unknown', gettext('Unknown')],
- ],
+ xtype: 'pmxNotificationMatchSeveritySettings',
},
{
xtype: 'pmxNotificationMatchCalendarSettings',
@@ -1047,6 +1000,84 @@ Ext.define('Proxmox.panel.MatchCalendarSettings', {
},
});
+Ext.define('Proxmox.panel.MatchSeveritySettings', {
+extend: 'Ext.panel.Panel',
+xtype: 'pmxNotificationMatchSeveritySettings',
+border: false,
+layout: 'anchor',
+// Hide initially to avoid glitches when opening the window
+hidden: true,
+bind: {
+ hidden: '{!typeIsMatchSeverity}',
+},
+viewModel: {
+ // parent is set in `initComponents`
+ formulas: {
+ typeIsMatchSeverity: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-severity';
+ },
+ },
+ matchSeverityValue: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ set: function(value) {
+ let record = this.get('selectedRecord');
+ let currentData = record.get('data');
+ record.set({
+ data: {
+ ...currentData,
+ value: value,
+ },
+ });
+ },
+ get: function(record) {
+ return record?.get('data')?.value;
+ },
+ },
+ },
+},
+items: [
+ {
+ xtype: 'proxmoxKVComboBox',
+ fieldLabel: gettext('Severities to match'),
+ isFormField: false,
+ allowBlank: true,
+ multiSelect: true,
+ field: 'value',
+ // Hide initially to avoid glitches when opening the window
+ hidden: true,
+ bind: {
+ value: '{matchSeverityValue}',
+ hidden: '{!typeIsMatchSeverity}',
+ disabled: '{!typeIsMatchSeverity}',
+ },
+
+ comboItems: [
+ ['info', gettext('Info')],
+ ['notice', gettext('Notice')],
+ ['war
[pve-devel] [PATCH widget-toolkit v7 13/19] notification: matcher: move match-calendar fields to panel
Also introduce a local viewModel that is linked to a parent viewModel,
allowing us to move the formulas to the panel.
This should make the code more cohesive and easier to follow.
No functional changes.
Signed-off-by: Lukas Wagner
Tested-by: Maximiliano Sandoval
---
src/window/NotificationMatcherEdit.js | 92 +--
1 file changed, 60 insertions(+), 32 deletions(-)
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index 559b405..50145e3 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -389,15 +389,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
return record?.get('type') === 'match-severity';
},
},
- typeIsMatchCalendar: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- get: function(record) {
- return record?.get('type') === 'match-calendar';
- },
- },
matchSeverityValue: {
bind: {
bindTo: '{selectedRecord}',
@@ -417,26 +408,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
return record?.get('data')?.value;
},
},
- matchCalendarValue: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
- let currentData = record.get('data');
- record.set({
- data: {
- ...currentData,
- value: value,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.value;
- },
- },
rootMode: {
bind: {
bindTo: '{selectedRecord}',
@@ -995,6 +966,58 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
['unknown', gettext('Unknown')],
],
},
+ {
+ xtype: 'pmxNotificationMatchCalendarSettings',
+ },
+],
+});
+
+Ext.define('Proxmox.panel.MatchCalendarSettings', {
+extend: 'Ext.panel.Panel',
+xtype: 'pmxNotificationMatchCalendarSettings',
+border: false,
+layout: 'anchor',
+// Hide initially to avoid glitches when opening the window
+hidden: true,
+bind: {
+ hidden: '{!typeIsMatchCalendar}',
+},
+viewModel: {
+ // parent is set in `initComponents`
+ formulas: {
+ typeIsMatchCalendar: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-calendar';
+ },
+ },
+
+ matchCalendarValue: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ set: function(value) {
+ let me = this;
+ let record = me.get('selectedRecord');
+ let currentData = record.get('data');
+ record.set({
+ data: {
+ ...currentData,
+ value: value,
+ },
+ });
+ },
+ get: function(record) {
+ return record?.get('data')?.value;
+ },
+ },
+ },
+},
+items: [
{
xtype: 'proxmoxKVComboBox',
fieldLabel: gettext('Timespan to match'),
@@ -1003,11 +1026,8 @@
Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
editable: true,
displayField: 'key',
field: 'value',
- // Hide initially to avoid glitches when opening the window
- hidden: true,
bind: {
value: '{matchCalendarValue}',
- hidden: '{!typeIsMatchCalendar}',
disabled: '{!typeIsMatchCalender}',
},
@@ -1017,6 +1037,14 @@
Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
],
},
],
+
+initComponent: function() {
+ let me = this;
+ Ext.apply(me.viewModel, {
+ parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
+ });
+ me.callParent();
+},
});
Ext.define('Proxmox.panel.MatchFieldSettings', {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 08/19] api: notification: add API for getting known metadata fields/values
This new API route returns known notification metadata fields and
a list of known possible values. This will be used by the UI to
provide suggestions when adding/modifying match rules.
Signed-off-by: Lukas Wagner
---
PVE/API2/Cluster/Notifications.pm | 139 ++
1 file changed, 139 insertions(+)
diff --git a/PVE/API2/Cluster/Notifications.pm
b/PVE/API2/Cluster/Notifications.pm
index 68fdda2a..2b202c28 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -79,12 +79,151 @@ __PACKAGE__->register_method ({
{ name => 'endpoints' },
{ name => 'matchers' },
{ name => 'targets' },
+ { name => 'matcher-fields' },
+ { name => 'matcher-field-values' },
];
return $result;
}
});
+__PACKAGE__->register_method ({
+name => 'get_matcher_fields',
+path => 'matcher-fields',
+method => 'GET',
+description => 'Returns known notification metadata fields',
+permissions => {
+ check => ['or',
+ ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ ['perm', '/mapping/notifications', ['Mapping.Audit']],
+ ],
+},
+protected => 0,
+parameters => {
+ additionalProperties => 0,
+ properties => {},
+},
+returns => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ name => {
+ description => 'Name of the field.',
+ type => 'string',
+ },
+ },
+ },
+ links => [ { rel => 'child', href => '{name}' } ],
+},
+code => sub {
+ # TODO: Adapt this API handler once we have a 'notification registry'
+
+ my $result = [
+ { name => 'type' },
+ { name => 'hostname' },
+ { name => 'job-id' },
+ ];
+
+ return $result;
+}
+});
+
+__PACKAGE__->register_method ({
+name => 'get_matcher_field_values',
+path => 'matcher-field-values',
+method => 'GET',
+description => 'Returns known notification metadata fields and their known
values',
+permissions => {
+ check => ['or',
+ ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ ['perm', '/mapping/notifications', ['Mapping.Audit']],
+ ],
+},
+protected => 1,
+parameters => {
+ additionalProperties => 0,
+},
+returns => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ 'value' => {
+ description => 'Notification metadata value known by the
system.',
+ type => 'string'
+ },
+ 'comment' => {
+ description => 'Additional comment for this value.',
+ type => 'string',
+ optional => 1,
+ },
+ 'field' => {
+ description => 'Field this value belongs to.',
+ type => 'string',
+ },
+ },
+ },
+},
+code => sub {
+ # TODO: Adapt this API handler once we have a 'notification registry'
+ my $rpcenv = PVE::RPCEnvironment::get();
+ my $user = $rpcenv->get_user();
+
+ my $values = [
+ {
+ value => 'package-updates',
+ field => 'type',
+ },
+ {
+ value => 'fencing',
+ field => 'type',
+ },
+ {
+ value => 'replication',
+ field => 'type',
+ },
+ {
+ value => 'vzdump',
+ field => 'type',
+ },
+ {
+ value => 'system-mail',
+ field => 'type',
+ },
+ ];
+
+ # Here we need a manual permission check.
+ if ($rpcenv->check($user, "/", ["Sys.Audit"], 1)) {
+ for my $backup_job (@{PVE::API2::Backup->index({})}) {
+ push @$values, {
+ value => $backup_job->{id},
+ comment => $backup_job->{comment},
+ field => 'job-id'
+ };
+ }
+ }
+ # The API call returns only returns jobs for which the user
+ # has adequate permissions.
+ for my $sync_job (@{PVE::API2::ReplicationConfig->index({})}) {
+ push @$values, {
+ value => $sync_job->{id},
+ comment => $sync_job->{comment},
+ field => 'job-id'
+ };
+ }
+
+ for my $node (@{PVE::Cluster::get_nodelist()}) {
+ push @$values, {
+ value => $node,
+ field => 'hostname',
+ }
+ }
+
+ return $values;
+}
+});
+
__PACKAGE__->register_method ({
name => 'endpoints_index',
path => 'endpoints',
--
2.39.2
___
pve-devel ma
[pve-devel] [PATCH docs v7 16/19] notifications: describe new notification metadata fields
Signed-off-by: Lukas Wagner --- notifications.adoc | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/notifications.adoc b/notifications.adoc index 57053c8..dec878a 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -289,19 +289,21 @@ Notification Events [width="100%",options="header"] |=== -| Event| `type`| Severity | Metadata fields (in addition to `type`) -| System updates available |`package-updates` | `info` | `hostname` -| Cluster node fenced |`fencing` | `error` | `hostname` -| Storage replication failed |`replication` | `error` | - -| Backup finished |`vzdump` | `info` (`error` on failure) | `hostname` -| Mail for root|`system-mail` | `unknown`| - +| Event| `type`| Severity | Metadata fields (in addition to `type`) +| System updates available |`package-updates` | `info` | `hostname` +| Cluster node fenced |`fencing` | `error` | `hostname` +| Storage replication job failed |`replication` | `error` | `hostname`, `job-id` +| Backup succeeded |`vzdump` | `info` | `hostname`, `job-id` (only for backup jobs) +| Backup failed|`vzdump` | `error` | `hostname`, `job-id` (only for backup jobs) +| Mail for root|`system-mail` | `unknown`| `hostname` |=== [width="100%",options="header"] |=== -| Field name | Description -| `type` | Type of the notifcation -| `hostname` | Hostname, without domain (e.g. `pve1`) +| Field name| Description +| `type`| Type of the notifcation +| `hostname`| Hostname, without domain (e.g. `pve1`) +| `job-id` | Job ID |=== System Mail Forwarding -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v7 18/19] notifications: fix typo in 'notification'
Signed-off-by: Lukas Wagner --- notifications.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notifications.adoc b/notifications.adoc index 07f0b3e..9c5228c 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -295,7 +295,7 @@ Notification Events [width="100%",options="header"] |=== | Field name| Description -| `type`| Type of the notifcation +| `type`| Type of the notification | `hostname`| Hostname, without domain (e.g. `pve1`) | `job-id` | Job ID |=== -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v7 15/19] notification: clarify that 'hostname' does not include the domain
This was a bit inconsistent between the different notification types: - APT/VZDump included the domain part - fence notifications did not A decision has been made to unify this by removing the domain part from APT/VZDump notifications. Signed-off-by: Lukas Wagner --- notifications.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notifications.adoc b/notifications.adoc index 46aff6a..57053c8 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -301,7 +301,7 @@ Notification Events |=== | Field name | Description | `type` | Type of the notifcation -| `hostname` | Hostname, including domain (e.g. `pve1.example.com`) +| `hostname` | Hostname, without domain (e.g. `pve1`) |=== System Mail Forwarding -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v7 17/19] notifications: match-field 'exact'-mode can now match multiple values
Signed-off-by: Lukas Wagner --- notifications.adoc | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/notifications.adoc b/notifications.adoc index dec878a..07f0b3e 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -221,11 +221,16 @@ configurable schedule. Field Matching Rules Notifications have a selection of metadata fields that can be matched. +When using `exact` as a matching mode, a `,` can be used as a separator. +The matching rule then matches if the metadata field has *any* of the specified +values. * `match-field exact:type=vzdump` Only match notifications about backups. +* `match-field exact:type=replication,fencing` Match `replication` and `fencing` notifications. * `match-field regex:hostname=^.+\.example\.com$` Match the hostname of the node. + If a matched metadata field does not exist, the notification will not be matched. For instance, a `match-field regex:hostname=.*` directive will only match @@ -267,18 +272,7 @@ matcher: backup-failures comment Send notifications about backup failures to one group of admins matcher: cluster-failures -match-field exact:type=replication -match-field exact:type=fencing -mode any -target cluster-admins -comment Send cluster-related notifications to other group of admins - - -The last matcher could also be rewritten using a field matcher with a regular -expression: - -matcher: cluster-failures -match-field regex:type=^(replication|fencing)$ +match-field exact:type=replication,fencing target cluster-admins comment Send cluster-related notifications to other group of admins -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 10/19] d/control: bump proxmox-widget-toolkit dependency to 4.1.4
We need "utils: add mechanism to add and override translatable notification event descriptions in the product specific UIs" otherwise there is an error in the browser console. Signed-off-by: Lukas Wagner --- debian/control | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/control b/debian/control index 4988a7d2..e400ddc5 100644 --- a/debian/control +++ b/debian/control @@ -20,7 +20,7 @@ Build-Depends: debhelper-compat (= 13), libtemplate-perl, libtest-mockmodule-perl, lintian, - proxmox-widget-toolkit (>= 4.0.7), + proxmox-widget-toolkit (>= 4.1.4), pve-cluster, pve-container, pve-doc-generator (>= 8.0.5), -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH many v7 00/19] notifications: notification metadata matching improvements
This patch series attempts to improve the user experience when creating notification matchers. Some of the noteworthy changes: - Fixup inconsistent 'hostname' field. Some notification events sent the hostname including a domain, while other did not. This series unifies the behavior, now the field only includes the hostname without a domain. Also updated the docs to reflect this change. - Allow setting a custom backup job ID, similar how we handle it for sync/prune jobs in PBS (to allow recognizable names used in matchers) - Adding columns for backup job ID/replication job ID in the UI - New metadata fields: - job-id: Job ID for backup-jobs or replication-jobs - Add an API that enumerates known notification metadata fields/values - Suggest known fields/values in match rule window - Some code clean up for match rule edit window - Extended the 'exact' match-field mode - it now allows setting multiple allowed values, separated via ',': e.g. `match-field exact:type=replication,fencing Originally, I created a separate 'list' match type for this, but since the semantics for a list with one value and 'exact' mode are identical, I decided to just extend 'exact'. This is a safe change since there are are no values where a ',' makes any sense (config IDs, hostnames) NOTE: Might need a versionened break, since the widget-toolkit-patches depend on new APIs provided by pve-manager. If the API is not present, creating matchers with 'match-field' does not work (cannot load lists of known values/fields) Inter-Dependencies: - the widget-toolkit dep in pve-manager needs to be bumped to at least 4.1.4 (we need "utils: add mechanism to add and override translatable notification event descriptions in the product specific UIs", otherwise the UI breaks with the pve-manager patches applied) --> already included a patch for this - widget-toolkit relies on a new API endpoint provided by pve-manager: --> we require a versioned break in widget-toolkit on pve-manager Changelog: - v7: incorporated some more feedback from @Fiona, thx! - Fixed error when switching from 'exact' to 'regex' if the text field was empty - rebased to latest master - 'backport' doc improvements from PBS - bumped widget-toolkit dep - v6: incorporate feedback from @Fiona, thx! - rename 'id' -> 'job-id' in VZDump API handler - consolidate 'replication-job'/'backup-job' to 'job-id' - Move 'job-id' setting to advanced tab in backup job edit. - Don't use 'internal' flag to mark translatable fields, since the only field where that's necessary is 'type' for now - so just add a hardcoded check - v5: - Rebased onto latest master, resolving some small conflict - v4: - widget-toolkit: break out changes for the utils module so that they can be applied ahead of time to ease dep bumping - don't show Job IDs in the backup/replication job columns - v3: - Drop already applied patches for `proxmox` - Rebase onto latest master - minor conflict resolution was needed - v2: - include 'type' metadata field for forwarded mails --> otherwise it's not possible to match them - include Maximilliano's T-b trailer in UI patches pve-guest-common: Lukas Wagner (1): vzdump: common: allow 'job-id' as a parameter without being in schema src/PVE/VZDump/Common.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) pve-manager: Lukas Wagner (9): api: jobs: vzdump: pass job 'job-id' parameter ui: dc: backup: send 'job-id' property when starting a backup job manually ui: dc: backup: allow to set custom job id in advanced settings api: replication: add 'job-id' to notification metadata vzdump: apt: notification: do not include domain in 'hostname' field api: replication: include 'hostname' field for notifications api: notification: add API for getting known metadata fields/values ui: utils: add overrides for translatable notification fields/values d/control: bump proxmox-widget-toolkit dependency to 4.1.4 PVE/API2/APT.pm | 3 +- PVE/API2/Cluster/Notifications.pm | 139 PVE/API2/Replication.pm | 4 +- PVE/API2/VZDump.pm | 8 ++ PVE/Jobs/VZDump.pm | 4 +- PVE/VZDump.pm | 14 +- debian/control | 2 +- www/manager6/Utils.js | 12 ++ www/manager6/dc/Backup.js | 7 +- www/manager6/panel/BackupAdvancedOptions.js | 23 10 files changed, 200 insertions(+), 16 deletions(-) proxmox-widget-toolkit: Lukas Wagner (4): notification: matcher: match-field: show known fields/values notification: matcher: move match-field formulas to local viewModel notification: matcher: move match-calendar fields to panel notification: matcher: move match-se
[pve-devel] [PATCH manager v7 04/19] ui: dc: backup: allow to set custom job id in advanced settings
This might be useful if somebody wants to match on the new
'backup-job' field in a notification match rule.
Signed-off-by: Lukas Wagner
---
www/manager6/Utils.js | 1 +
www/manager6/dc/Backup.js | 4
www/manager6/panel/BackupAdvancedOptions.js | 23 +
3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index f5608944..5b0d51eb 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1963,6 +1963,7 @@ Ext.define('PVE.Utils', {
singleton: true,
constructor: function() {
var me = this;
+
Ext.apply(me, me.utilities);
Proxmox.Utils.override_task_descriptions({
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 4b45b5c6..5975cb1c 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -45,10 +45,6 @@ Ext.define('PVE.dc.BackupEdit', {
Proxmox.Utils.assemble_field_data(values, { 'delete':
'notification-target' });
}
- if (!values.id && isCreate) {
- values.id = 'backup-' +
Ext.data.identifier.Uuid.Global.generate().slice(0, 13);
- }
-
let selMode = values.selMode;
delete values.selMode;
diff --git a/www/manager6/panel/BackupAdvancedOptions.js
b/www/manager6/panel/BackupAdvancedOptions.js
index 1026c6f4..8860e42c 100644
--- a/www/manager6/panel/BackupAdvancedOptions.js
+++ b/www/manager6/panel/BackupAdvancedOptions.js
@@ -37,6 +37,10 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
return {};
}
+ if (!formValues.id && me.isCreate) {
+ formValues.id = 'backup-' +
Ext.data.identifier.Uuid.Global.generate().slice(0, 13);
+ }
+
let options = {};
if (!me.isCreate) {
@@ -105,6 +109,25 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
},
items: [
+ {
+ xtype: 'pveTwoColumnContainer',
+ startColumn: {
+ xtype: 'pmxDisplayEditField',
+ vtype: 'ConfigId',
+ fieldLabel: gettext('Job ID'),
+ emptyText: gettext('Autogenerate'),
+ name: 'id',
+ allowBlank: true,
+ cbind: {
+ editable: '{isCreate}',
+ },
+ },
+ endFlex: 2,
+ endColumn: {
+ xtype: 'displayfield',
+ value: gettext('Can be used in notification matchers to match
this job.'),
+ },
+ },
{
xtype: 'pveTwoColumnContainer',
startColumn: {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 07/19] api: replication: include 'hostname' field for notifications
The field contains the hostname of the host (without any domain part)
which sends the notification. This field can be used in match-field
match rules.
Signed-off-by: Lukas Wagner
---
PVE/API2/Replication.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index 6208a1a2..e4a7180f 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -125,6 +125,8 @@ my sub _handle_job_err {
my $metadata_fields = {
type => "replication",
"job-id" => $job->{id},
+ # Hostname (without domain part)
+ hostname => PVE::INotify::nodename(),
};
eval {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 05/19] api: replication: add 'job-id' to notification metadata
This allows users to create notification match rules for specific
replication jobs, if they so desire.
Signed-off-by: Lukas Wagner
---
PVE/API2/Replication.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index d84ac1ab..6208a1a2 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -123,8 +123,8 @@ my sub _handle_job_err {
};
my $metadata_fields = {
- # TODO: Add job-id?
type => "replication",
+ "job-id" => $job->{id},
};
eval {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 03/19] ui: dc: backup: send 'job-id' property when starting a backup job manually
Signed-off-by: Lukas Wagner
---
www/manager6/dc/Backup.js | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 4ba80b31..4b45b5c6 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -604,11 +604,12 @@ Ext.define('PVE.dc.BackupView', {
job = Ext.clone(job);
let jobNode = job.node;
+ job['job-id'] = job.id;
+ delete job.id;
// Remove properties related to scheduling
delete job.enabled;
delete job.starttime;
delete job.dow;
- delete job.id;
delete job.schedule;
delete job.type;
delete job.node;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
