Re: [pve-devel] [PATCH proxmox-firewall 09/37] config: firewall: add types for rules

2024-04-09 Thread Lukas Wagner
On 2024-04-03 12:46, Max Carrara wrote: >> + >> +#[derive(Clone, Debug)] >> +#[cfg_attr(test, derive(Eq, PartialEq))] >> +pub enum IcmpType { >> +Numeric(u8), >> +Named(&'static str), >> +} >> + >> +// MUST BE SORTED! > > Should maaaybe note that it must be sorted for binary search,

[pve-devel] [PATCH qemu-server 1/1] fix #5365: drive: add drive_is_cloudinit check to get_scsi_devicetype

2024-04-09 Thread Hannes Duerr
When we obtain the devicetype, we check whether it is a CD drive. Cloudinit drives are always allocated CD drives, but if the drive has not yet been allocated, the test fails because the cd attribute has not yet been set. We therefore now explicitly check whether it is a cloudinit drive that has

Re: [pve-devel] [PATCH proxmox-firewall 06/37] config: host: add helpers for host network configuration

2024-04-09 Thread Lukas Wagner
On 2024-04-02 19:15, Stefan Hanreich wrote: > Currently the helpers for obtaining the host network configuration > panic on error, which could be avoided by the use of > OnceLock::get_or_init, but this method is currently only available in > nightly versions. > > Generally, if there is a

Re: [pve-devel] [PATCH manager] Revert "ui: dc: remove notify key from datacenter option view"

2024-04-09 Thread Lukas Wagner
On 2024-04-09 15:07, Thomas Lamprecht wrote: > I'd propose two changes: > > - add a hint to redirect users to the new mechanisms so that a future > deprecation would be more expected (if we already plan that now) > > - only show it if defined? While that's a bit magic, it'd avoid that >

[pve-devel] [PATCH manager 19/19] notifications: use named templates instead of in-code templates

2024-04-09 Thread Lukas Wagner
This commit adapts notification sending for - package update - replication - backups to use named templates (installed in /usr/share/proxmox-ve/templates) instead of passing template strings defined in code to the notification stack. Signed-off-by: Lukas Wagner Tested-by: Folke

[pve-devel] [PATCH pve-ha-manager 16/19] env: notify: use named templates instead of passing template strings

2024-04-09 Thread Lukas Wagner
Signed-off-by: Lukas Wagner Tested-by: Folke Gleumes --- debian/pve-ha-manager.install | 3 +++ src/Makefile | 1 + src/PVE/HA/Env/PVE2.pm| 4 ++-- src/PVE/HA/NodeStatus.pm | 20 +--

[pve-devel] [PATCH proxmox 11/19] notify: renderer: add relative-percentage helper from PBS

2024-04-09 Thread Lukas Wagner
Signed-off-by: Lukas Wagner --- proxmox-notify/src/renderer/mod.rs | 29 + 1 file changed, 29 insertions(+) diff --git a/proxmox-notify/src/renderer/mod.rs b/proxmox-notify/src/renderer/mod.rs index a51ece6..ddb241d 100644 --- a/proxmox-notify/src/renderer/mod.rs

[pve-devel] [PATCH cluster 15/19] notify: use named template instead of passing template strings

2024-04-09 Thread Lukas Wagner
The notification system will now load template files from a defined location. The template to use is now passed to proxmox_notify, instead of separate template strings for subject/body. Signed-off-by: Lukas Wagner Tested-by: Folke Gleumes --- src/PVE/Notify.pm | 29

[pve-devel] [PATCH proxmox 05/19] notify: make the `mail-forwarder` feature depend on proxmox-sys

2024-04-09 Thread Lukas Wagner
It uses proxmox_sys::nodename - the dep is needed, otherwise the code does not compile in some feature flag permutations. Signed-off-by: Lukas Wagner Tested-by: Folke Gleumes --- proxmox-notify/Cargo.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git

[pve-devel] [PATCH proxmox 01/19] notify: switch to file-based templating system

2024-04-09 Thread Lukas Wagner
Instead of passing the template strings for subject and body when constructing a notification, we pass only the name of a template. When rendering the template, the name of the template is used to find corresponding template files. For PVE, they are located at

[pve-devel] [PATCH proxmox 08/19] notify: derive `api` for Deleteable*Property

2024-04-09 Thread Lukas Wagner
The API endpoints in Proxmox Backup Server require ApiType to be implemented for any deserialized parameter. Signed-off-by: Lukas Wagner --- proxmox-notify/src/endpoints/gotify.rs | 3 +++ proxmox-notify/src/endpoints/sendmail.rs | 7 +++ proxmox-notify/src/endpoints/smtp.rs | 9

[pve-devel] [PATCH proxmox 03/19] notify: convert Option> -> Vec in config structs

2024-04-09 Thread Lukas Wagner
Suggested-by: Wolfgang Bumiller Signed-off-by: Lukas Wagner Tested-by: Folke Gleumes --- proxmox-notify/src/api/matcher.rs | 27 +++ proxmox-notify/src/api/mod.rs | 22 +--- proxmox-notify/src/api/sendmail.rs | 22 ++--

[pve-devel] [PATCH proxmox 04/19] notify: don't make tests require pve-context

2024-04-09 Thread Lukas Wagner
Tests now have their own context, so requiring pve-context is not necessary any more. Signed-off-by: Lukas Wagner Tested-by: Folke Gleumes --- proxmox-notify/src/api/gotify.rs | 2 +- proxmox-notify/src/api/matcher.rs | 2 +- proxmox-notify/src/api/sendmail.rs | 2 +-

[pve-devel] [PATCH proxmox-perl-rs 12/19] notify: use file based notification templates

2024-04-09 Thread Lukas Wagner
Instead of passing literal template strings to the notification system, we now only pass an identifier. This identifier will be used load the template files from a product-specific directory. Signed-off-by: Lukas Wagner Tested-by: Folke Gleumes --- common/src/notify.rs | 8 +++- 1 file

[pve-devel] [PATCH proxmox 09/19] notify: derive Deserialize/Serialize for Notification struct

2024-04-09 Thread Lukas Wagner
Signed-off-by: Lukas Wagner --- proxmox-notify/src/lib.rs | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs index 91c0b61..8d4dc63 100644 --- a/proxmox-notify/src/lib.rs +++ b/proxmox-notify/src/lib.rs @@ -159,11

[pve-devel] [PATCH proxmox 10/19] notify: pbs context: include nodename in default sendmail author

2024-04-09 Thread Lukas Wagner
The old notification stack in proxmox-backup includes the nodename, so we include it here as well. Signed-off-by: Lukas Wagner --- proxmox-notify/src/context/pbs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxmox-notify/src/context/pbs.rs

[pve-devel] [PATCH proxmox 06/19] notify: give each notification a unique ID

2024-04-09 Thread Lukas Wagner
We need this for queuing notifications on PBS from the unprivileged proxy process. Signed-off-by: Lukas Wagner --- proxmox-notify/Cargo.toml | 1 + proxmox-notify/src/lib.rs | 11 +++ 2 files changed, 12 insertions(+) diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml

[pve-devel] [PATCH proxmox 02/19] notify: make api methods take config struct ownership

2024-04-09 Thread Lukas Wagner
This saves us from some of the awkward cloning steps when updating. Suggested-by: Wolfgang Bumiller Signed-off-by: Lukas Wagner Tested-by: Folke Gleumes --- proxmox-notify/src/api/gotify.rs | 46 +- proxmox-notify/src/api/matcher.rs | 38 +++

[pve-devel] [PATCH manager 18/19] tests: remove vzdump_notification test

2024-04-09 Thread Lukas Wagner
With the upcoming changes in how we send notifications, this one really becomes pretty annoying to keep working. The location where templates are looked up are defined in the proxmox_notify crate, so there is no easy way to mock this for testing. The test itself seemed not super valuable, mainly

[pve-devel] [PATCH proxmox-perl-rs 13/19] notify: don't pass config structs by reference

2024-04-09 Thread Lukas Wagner
proxmox_notify's api functions have been changed so that they take ownership of config structs. Signed-off-by: Lukas Wagner Tested-by: Folke Gleumes --- common/src/notify.rs | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/common/src/notify.rs

[pve-devel] [PATCH proxmox-perl-rs 14/19] notify: adapt to Option> to Vec changes in proxmox_notify

2024-04-09 Thread Lukas Wagner
Signed-off-by: Lukas Wagner Tested-by: Folke Gleumes --- common/src/notify.rs | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/common/src/notify.rs b/common/src/notify.rs index 00a6056..e1b006b 100644 --- a/common/src/notify.rs +++ b/common/src/notify.rs @@

[pve-devel] [PATCH proxmox 07/19] notify: api: add get_targets

2024-04-09 Thread Lukas Wagner
This method allows us to get a list of all notification targets. Signed-off-by: Lukas Wagner --- proxmox-notify/src/api/mod.rs | 75 +++ 1 file changed, 75 insertions(+) diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs index

[pve-devel] [PATCH manager 17/19] gitignore: ignore any test artifacts

2024-04-09 Thread Lukas Wagner
Signed-off-by: Lukas Wagner --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index e8d1eb27..48975d55 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,5 @@ dest/ /www/mobile/pvemanager-mobile.js /www/touch/touch-[0-9]*/ /pve-manager-[0-9]*/

[pve-devel] [PATCH many 00/19] notifications: move template strings to template files; PBS preparations

2024-04-09 Thread Lukas Wagner
The notification system uses handlebar templates to render the subject and the body of notifications. Previously, the template strings were defined inline at the call site. This patch series extracts the templates into template files and installs them at /usr/share/proxmox-ve/templates/default

Re: [pve-devel] [PATCH manager] Revert "ui: dc: remove notify key from datacenter option view"

2024-04-09 Thread Thomas Lamprecht
Am 09/02/2024 um 11:16 schrieb Lukas Wagner: > This reverts commit c81bca2d28744616098448b81fa58e133d3ac5ed. > > In the first iteration of the notification system, notification > routing and enabling/disabling notifications was configured via > the (extended) `notify` parameter in

[pve-devel] [RFC pve-kernel] split lock detection: warn more prominently about misery mode

2024-04-09 Thread Friedrich Weber
On processors with the `split_lock_detect` feature, the kernel will detect user-space split locks and, by default, apply a "misery mode" to offending tasks [1]. When a split lock is detected, the misery mode artificially slows down the task by forcing a 10ms and serialization with other offending

[pve-devel] applied: [PATCH pve-storage] esxi: add mapping for windows server 2016/2019

2024-04-09 Thread Wolfgang Bumiller
applied, thanks ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH pve-storage] esxi: add mapping for windows server 2016/2019

2024-04-09 Thread Stefan Sterz
previously these were mapped to the linux 2.6 default Signed-off-by: Stefan Sterz --- src/PVE/Storage/ESXiPlugin.pm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm index 4212c36..e5082ea 100644 ---

Re: [pve-devel] [PATCH v5 pve-storage, pve-manager 00/11] Fix #4759: Configure Permissions for ceph-crash.service

2024-04-09 Thread Max Carrara
On Tue Apr 9, 2024 at 11:48 AM CEST, Maximiliano Sandoval wrote: > > Max Carrara writes: > > > Fix #4759: Configure Permissions for ceph-crash.service - Version 5 > > === > > I tested this patch series on a testing cluster updated to

Re: [pve-devel] [PATCH v5 pve-storage, pve-manager 00/11] Fix #4759: Configure Permissions for ceph-crash.service

2024-04-09 Thread Maximiliano Sandoval
Maximiliano Sandoval writes: > Max Carrara writes: > >> Fix #4759: Configure Permissions for ceph-crash.service - Version 5 >> === > > I tested this patch series on a testing cluster updated to > no-subscription with ceph-base

Re: [pve-devel] [PATCH v5 pve-storage, pve-manager 00/11] Fix #4759: Configure Permissions for ceph-crash.service

2024-04-09 Thread Maximiliano Sandoval
Max Carrara writes: > Fix #4759: Configure Permissions for ceph-crash.service - Version 5 > === I tested this patch series on a testing cluster updated to no-subscription with ceph-base 18.2.2-pve1. For the purposes of testing I

[pve-devel] applied-series: [PATCH v3 container 0/2] fix #5160: fix move_mount regression for mount point hotplug

2024-04-09 Thread Wolfgang Bumiller
applied both patches, thanks ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH v2 container 1/2] fix #5160: fix move_mount regression for mount point hotplug

2024-04-09 Thread Filip Schauer
Agreed. This appear to be a leftover from testing. A patch v3 with this removed is available: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062693.html On 09/04/2024 10:23, Wolfgang Bumiller wrote: diff --gita/src/pve-container-debug@.service b/src/pve-container-debug@.service

[pve-devel] [PATCH v3 container 1/2] fix #5160: fix move_mount regression for mount point hotplug

2024-04-09 Thread Filip Schauer
Set up an Apparmor profile to allow moving mounts for mount point hotplug. This fixes a regression caused by kernel commit 157a3537d6 ("apparmor: Fix regression in mount mediation") The commit introduced move_mount mediation, which now requires move_mount to be allowed in the Apparmor profile.

[pve-devel] [PATCH v3 container 2/2] fix undef warning when apparmor changeprofile fails

2024-04-09 Thread Filip Schauer
Fix a "Use of uninitialized value in numeric ne (!=)" warning when syswrite returns undef when trying to change the apparmor profile. Signed-off-by: Filip Schauer --- src/PVE/LXC.pm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm index

[pve-devel] [PATCH v3 container 0/2] fix #5160: fix move_mount regression for mount point hotplug

2024-04-09 Thread Filip Schauer
Changes since v2: * Remove apparmor profile load on ExecStartPre in the container service files. Changes since v1: * Fix loading of apparmor profile not working in postinst, since the profile is not found by dh_apparmor. This is fixed by moving pve-container-mounthotplug out of the pve

Re: [pve-devel] [PATCH manager 1/2] ui: dc: backup: improve UX for the different 'notification-mode's

2024-04-09 Thread Maximiliano Sandoval
Lukas Wagner writes: > - Switch order of 'mailto' and 'mailnotification' field > - When mode is 'auto', disable 'mailtnotification' field > - When mode is 'auto' and 'mailto' is empty, show > hint that the notification system will be used > > Signed-off-by: Lukas Wagner > --- ping 

Re: [pve-devel] [RFC container/firewall/manager/proxmox-firewall/qemu-server 00/37] proxmox firewall nftables implementation

2024-04-09 Thread Stefan Hanreich
On 4/3/24 12:46, Max Carrara wrote: > Four overall things I want to mention: > > 1. IMO a lot of the `pub` items should eventually be documented, > preferably once the actual series is out. I don't think we need to > be as thorough as e.g. the Rust STL's documentation, but I don't >

Re: [pve-devel] [PATCH v4 installer 31/30 follow-up] auto-installer: answer: deny unknown fields

2024-04-09 Thread Christoph Heiss
LGTM, does exactly what is says on the tin. Tested it using both the `proxmox-autoinst-helper validate-answer` tool and trying to boot the auto-installer itself with a bogus answer file. So please consider this also: Reviewed-by: Christoph Heiss Tested-by: Christoph Heiss On Fri, Apr 05,

Re: [pve-devel] [PATCH proxmox-firewall 21/37] nftables: statement: add types

2024-04-09 Thread Stefan Hanreich
On 4/3/24 12:47, Max Carrara wrote: > Hmm, you don't use either here - you sure you didn't mean to introduce > `anyhow` later? Very possible that is a mishap of mine while trying to prettify git history. ___ pve-devel mailing list

Re: [pve-devel] [PATCH proxmox-firewall 33/37] firewall: add files for debian packaging

2024-04-09 Thread Stefan Hanreich
Thank you very much for the comprehensive comments and remarks, I've already incorporated all of them and we should now have proper clean builds for the firewall! ___ pve-devel mailing list pve-devel@lists.proxmox.com

Re: [pve-devel] [PATCH proxmox-firewall 13/37] config: firewall: add host specific config + option types

2024-04-09 Thread Stefan Hanreich
On 4/3/24 12:47, Max Carrara wrote: > Should maybe document such defaults in the docstring of the `pub` > function above? > >> + >> +pub fn synflood_burst() -> i64 { >> +self.config >> +.options >> +.protection_synflood_burst >> +.unwrap_or(1000) >>

[pve-devel] applied: [PATCH backup-qemu] make capi_types module public

2024-04-09 Thread Wolfgang Bumiller
applied, thanks ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH proxmox-firewall 11/37] config: firewall: add generic parser for firewall configs

2024-04-09 Thread Stefan Hanreich
On 4/3/24 12:47, Max Carrara wrote: > Since this is `pub`, I think a more complete docstring here would be > better instead of a comment. Though I haven't generated the docs for all > of this (yet) I have to admit, so I'm not sure if this actually shows > up. It's generally something I've been a

Re: [pve-devel] [PATCH proxmox-firewall 09/37] config: firewall: add types for rules

2024-04-09 Thread Stefan Hanreich
On 4/3/24 12:46, Max Carrara wrote: > Hmm, since this is only used below, IMO it's fine that this returns a > tuple like that on `Ok` - but should functions like that be used in > multiple places, it might be beneficial to use a type alias or even a > tuple struct for readability's sake. Yes, I

Re: [pve-devel] [PATCH widget-toolkit v4] window: edit: avoid sharing custom config objects between subclasses

2024-04-09 Thread Stefan Sterz
On Tue Apr 9, 2024 at 10:16 AM CEST, Friedrich Weber wrote: > Currently, `Proxmox.window.Edit` initializes `extraRequestParams` and > `submitOptions` to two objects that, if not overwritten, are shared > between all instances of subclasses. This bears the danger of > modifying the shared object in

Re: [pve-devel] [PATCH proxmox-firewall 06/37] config: host: add helpers for host network configuration

2024-04-09 Thread Stefan Hanreich
On 4/3/24 12:46, Max Carrara wrote: > IMO the closures here and below could later be put into something like > proxmox-sys (or similar) as freestanding functions without static data > and then called here - but this is fine as it is; just an idea! Yes, we are lacking networking-related helpers

Re: [pve-devel] [PATCH proxmox-firewall 02/37] config: firewall: add types for ip addresses

2024-04-09 Thread Stefan Hanreich
On 4/3/24 12:46, Max Carrara wrote: > You could just `match` on a slice of `entries` here and then have ... Yes, that is much nicer! Will incorporate. ___ pve-devel mailing list pve-devel@lists.proxmox.com

Re: [pve-devel] [PATCH v2 container 1/2] fix #5160: fix move_mount regression for mount point hotplug

2024-04-09 Thread Wolfgang Bumiller
looks mostly good, just the ExecStartPre= lines in the service files should be dropped On Mon, Mar 25, 2024 at 06:28:28PM +0100, Filip Schauer wrote: > Set up an Apparmor profile to allow moving mounts for mount point > hotplug. > > This fixes a regression caused by > kernel commit 157a3537d6

[pve-devel] [PATCH widget-toolkit v4] window: edit: avoid sharing custom config objects between subclasses

2024-04-09 Thread Friedrich Weber
Currently, `Proxmox.window.Edit` initializes `extraRequestParams` and `submitOptions` to two objects that, if not overwritten, are shared between all instances of subclasses. This bears the danger of modifying the shared object in a subclass instead of overwriting it, which affects all edit

Re: [pve-devel] [PATCH widget-toolkit v3] window: edit: avoid sharing custom config objects between subclasses

2024-04-09 Thread Friedrich Weber
On 08/04/2024 14:36, Thomas Lamprecht wrote: > Am 08/04/2024 um 12:36 schrieb Stefan Sterz: >> [...] >> so, this seems like a fix bug a) creates bug b) type of situation... >> this patch means that editing a pool allows changing the name suddenly, >> but since we don't support that in the backend,