[pve-devel] [PATCH storage] fix insecure migration failing if waiting on lock

2024-04-15 Thread Mira Limbeck
both STDOUT and STDERR are written into `$info` which is then parsed for
IP and port of the target socket listening.
when the ports file can't be locked immediately `trying to acquire
lock...` is printed on STDERR and in turn written into `$info`.
trying to parse the IP then fails, resulting in a migration or
replication failing.

the bare open3 call is replaced by the run_command wrapper from
pve-common to use a safe wrapper around open3 with the same
functionality.
STDERR is now read separately from STDOUT and the last line of STDERR
is kept in case of errors.

Signed-off-by: Mira Limbeck 
---
I've replaced open3 with run_command on recommendation from others. tt
complicates the logic a little bit compared to the `simple` open3
solution, but may be less error prone since run_command abstracts open3
nicely behind a safe wrapper.

one thing I could not verify was the now removed line:
`if (!close($info)) { # does waitpid()`

I could not find any information mentioning this behavior of close() on
a simple file.


 src/PVE/Storage.pm | 79 +-
 1 file changed, 43 insertions(+), 36 deletions(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 40314a8..b6045d5 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -851,44 +851,51 @@ sub storage_migrate {
 
 eval {
if ($insecure) {
-   my $input = IO::File->new();
-   my $info = IO::File->new();
-   open3($input, $info, $info, @$recv)
-   or die "receive command failed: $!\n";
-   close($input);
-
-   my $try_ip = <$info> // '';
-   my ($ip) = $try_ip =~ /^($PVE::Tools::IPRE)$/ # untaint
-   or die "no tunnel IP received, got '$try_ip'\n";
-
-   my $try_port = <$info> // '';
-   my ($port) = $try_port =~ /^(\d+)$/ # untaint
-   or die "no tunnel port received, got '$try_port'\n";
-
-   my $socket = IO::Socket::IP->new(PeerHost => $ip, PeerPort => 
$port, Type => SOCK_STREAM)
-   or die "failed to connect to tunnel at $ip:$port\n";
-   # we won't be reading from the socket
-   shutdown($socket, 0);
-
-   eval { run_command($cmds, output => '>&'.fileno($socket), errfunc 
=> $match_volid_and_log); };
-   my $send_error = $@;
-
-   # don't close the connection entirely otherwise the receiving end
-   # might not get all buffered data (and fails with 'connection reset 
by peer')
-   shutdown($socket, 1);
-
-   # wait for the remote process to finish
-   while (my $line = <$info>) {
-   $match_volid_and_log->("[$target_sshinfo->{name}] $line");
+   my $last_err = '';
+   my $ip;
+   my $port;
+   my $socket;
+   my $send_error;
+   my $handle_insecure_migration = sub {
+   my $line = shift;
+
+   if (!$ip) {
+   if ($line =~ /^($PVE::Tools::IPRE)$/) {
+   $ip = $1;
+   } else {
+   die "no tunnel IP received, got $line\n";
+   }
+   } elsif(!$port) {
+   if ($line =~ /^(\d+)$/) {
+   $port = $1;
+   } else {
+   die "no tunnel port received, got $line\n";
+   }
+   }
+   if ($ip && $port && !$socket) {
+   # create socket, run command
+   $socket = IO::Socket::IP->new(PeerHost => $ip, PeerPort => 
$port, Type => SOCK_STREAM)
+   or die "failed to connect to tunnel at $ip:$port\n";
+   # we won't be reading from the socket
+   shutdown($socket, 0);
+
+   eval { run_command($cmds, output => '>&'.fileno($socket), 
errfunc => $match_volid_and_log); };
+   $send_error = $@;
+
+   # don't close the connection entirely otherwise the 
receiving end
+   # might not get all buffered data (and fails with 
'connection reset by peer')
+   shutdown($socket, 1);
+   } elsif ($ip && $port) {
+   $match_volid_and_log->("[$target_sshinfo->{name}] $line");
+   }
+   };
+   eval { run_command($recv, outfunc => $handle_insecure_migration, 
errfunc => sub { $last_err = shift; }); };
+   if (my $err = $@) {
+   chomp($err);
+   die "failed to run insecure migration: $err - $last_err\n";
}
-
# now close the socket
-   close($socket);
-   if (!close($info)) { # does waitpid()
-   die "import failed: $!\n" if $!;
-   die "import failed: exit code ".($?>>8)."\n";
-   }
-
+   close($socket) if $socket;
die $send_error if $send_error;
} else {
push @$cmds, $recv;
-- 
2.39.2


___

[pve-devel] applied: [PATCH pve-manager] ui: lxc: add firewall log view filtering

2024-04-15 Thread Fiona Ebner
Am 05.12.23 um 15:36 schrieb Christian Ebner:
> Allow to filter firewall logs analogous to node and VM firewall logs.
> 
> Signed-off-by: Christian Ebner 
> ---
>  www/manager6/lxc/Config.js | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
> index 4516ee8f..8ef66f39 100644
> --- a/www/manager6/lxc/Config.js
> +++ b/www/manager6/lxc/Config.js
> @@ -355,6 +355,8 @@ Ext.define('PVE.lxc.Config', {
>   itemId: 'firewall-fwlog',
>   xtype: 'proxmoxLogView',
>   url: '/api2/extjs' + base_url + '/firewall/log',
> + log_select_timespan: true,
> + submitFormat: 'U',
>   },
>   );
>   }

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 v2 pve-manager] ui: storage: add is_mountpoint checkmark to directory storage edit

2024-04-15 Thread Fabian Grünbichler
On February 23, 2024 1:03 pm, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer 
> ---
> 
> came up in enterprise support, and I don't think there is a reason to
> not have it in the UI, while having it in the API
> 
> v2:
>  - use Aaron's improved help text
> 
>  www/manager6/storage/DirEdit.js | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/www/manager6/storage/DirEdit.js b/www/manager6/storage/DirEdit.js
> index 3e2025fc..4d1921dc 100644
> --- a/www/manager6/storage/DirEdit.js
> +++ b/www/manager6/storage/DirEdit.js
> @@ -37,6 +37,18 @@ Ext.define('PVE.storage.DirInputPanel', {
>   },
>   ];
>  
> + me.advancedColumn2 = [
> + {
> + xtype: 'proxmoxcheckbox',

this isn't a boolean value though, but a string that has a special
interpretation when its value is a boolean.. this would need at least
read-only support for the "contains a path" case, else it breaks the
config entry (resetting to it not being treated as mountpoint!) when
doing unrelated edits via the UI..

> + name: 'is_mountpoint',
> + uncheckedValue: 0,
> + fieldLabel: gettext('Mountpoint'),
> + autoEl: {
> + tag: 'div',
> + 'data-qtip': gettext('Enable if something is mounted at 
> this path. Storage is considered offline as long as nothing is mounted.'),
> + },
> + },
> + ];
>   me.callParent();
>  },
>  });
> -- 
> 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



Re: [pve-devel] [PATCH common v2 1/2] add get_device_stat helper subroutine

2024-04-15 Thread Fiona Ebner
Am 15.04.24 um 15:17 schrieb Filip Schauer:
> The get_device_stat subroutine gets a device path, validates it, and
> returns the file mode and the device identifier.
> 
> Signed-off-by: Filip Schauer 
> ---
>  src/PVE/Tools.pm | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
> index 766c809..d1b53e5 100644
> --- a/src/PVE/Tools.pm
> +++ b/src/PVE/Tools.pm
> @@ -7,7 +7,8 @@ use Date::Format qw(time2str);
>  use Digest::MD5;
>  use Digest::SHA;
>  use Encode;
> -use Fcntl qw(:DEFAULT :flock);
> +use Errno qw(ENOENT);

We already have "use POSIX" with various error codes, so should be added
there.

> +use Fcntl qw(:DEFAULT :flock :mode);
>  use File::Basename;
>  use File::Path qw(make_path);
>  use Filesys::Df (); # don't overwrite our df()
> @@ -1853,6 +1854,21 @@ sub dev_t_minor($) {
>  return (($dev_t >> 12) & 0xfff00) | ($dev_t & 0xff);
>  }
>  
> +sub get_device_stat {

It's not the whole stat, so the name is a bit misleading. Tools.pm is
already very big and mixes different things, so not super happy about
adding new things to it. If it would return the whole stat, it could
still be fine IMHO, because it's more likely to be reusable later.
Alternatively, we can keep this code in pve-container.

> +my ($path) = @_;
> +
> +die "Path is not defined\n" if !defined($path);
> +
> +my ($mode, $rdev) = (stat($path))[2, 6];
> +die "Device $path does not exist\n" if $! == ENOENT;
> +die "Error accessing device $path\n"
> + if (!defined($mode) || !defined($rdev));
> +die "$path is not a device\n"
> + if (!S_ISBLK($mode) && !S_ISCHR($mode));

Style nit: useless parentheses, post-if can fit on the same line.

> +
> +return ($mode, $rdev);
> +}
> +
>  # Given an array of array refs [ \[a b c], \[a b b], \[e b a] ]
>  # Returns the intersection of elements as a single array [a b]
>  sub array_intersect {


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



Re: [pve-devel] [PATCH installer v2 0/3] expose zfs arc size setting for all products

2024-04-15 Thread Christoph Heiss
v3: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062976.html

On Wed, Feb 07, 2024 at 03:28:11PM +0100, Christoph Heiss wrote:
> As suggested by Thomas, leaves the ZFS default if the user never touches
> the setting in the installer (i.e. not writing a modprobe file).
> See also the discussion in v1 [0].
>
> [0] https://lists.proxmox.com/pipermail/pve-devel/2024-February/061659.html
>
> Testing
> ---
> Tested the installation of PVE and PBS, with each once letting the arc
> size setting untouched and once setting it to some specific value.
> Afterwards, checked that for PVE the module parameter was always written
> to /etc/modprobe.d/, for PBS that it was only written in case it was
> set.
>
> Previous revisions
> --
> v1: https://lists.proxmox.com/pipermail/pve-devel/2023-November/060898.html
>
> Changes v1 -> v2:
>   * rebased on latest master
>   * add placeholder functionality for arc max size in TUI
>   * "emulate" placeholder functionality in GTK on best-effort basis
>
> Christoph Heiss (3):
>   tui: NumericEditView: add optional placeholder value
>   tui: expose arc size setting for zfs bootdisks for all products
>   proxinstall: expose arc size setting for zfs bootdisks for all
> products
>
>  Proxmox/Install/RunEnv.pm   |  3 +-
>  proxinstall | 37 +++
>  proxmox-installer-common/src/options.rs |  3 +-
>  proxmox-tui-installer/src/views/bootdisk.rs | 48 --
>  proxmox-tui-installer/src/views/mod.rs  | 69 +++--
>  5 files changed, 124 insertions(+), 36 deletions(-)
>
> --
> 2.42.0
>
>
>
> ___
> 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 installer v3 1/3] tui: NumericEditView: add optional placeholder value

2024-04-15 Thread Christoph Heiss
Enables to add an optional placeholder value to `NumericEditView`, which
will be displayed in a different (darker) color and not returned by
`.get_content*()`.

Can be used for having default values in the TUI, but with different
handling in the back.

Signed-off-by: Christoph Heiss 
---
Changes v2 -> v3:
  * when empty & focused, do not show the placeholder value at all

Changes v1 -> v2:
  * new patch

 proxmox-tui-installer/src/views/mod.rs | 66 --
 1 file changed, 62 insertions(+), 4 deletions(-)

diff --git a/proxmox-tui-installer/src/views/mod.rs 
b/proxmox-tui-installer/src/views/mod.rs
index 3244e76..5e5f4fb 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -2,9 +2,10 @@ use std::{net::IpAddr, rc::Rc, str::FromStr};

 use cursive::{
 event::{Event, EventResult},
+theme::BaseColor,
 view::{Resizable, ViewWrapper},
 views::{EditView, LinearLayout, NamedView, ResizedView, SelectView, 
TextView},
-Rect, Vec2, View,
+Printer, Rect, Vec2, View,
 };

 use proxmox_installer_common::utils::CidrAddress;
@@ -24,6 +25,7 @@ pub use timezone::*;
 pub struct NumericEditView {
 view: LinearLayout,
 max_value: Option,
+placeholder: Option,
 max_content_width: Option,
 allow_empty: bool,
 }
@@ -36,6 +38,7 @@ impl 
NumericEditView {
 Self {
 view,
 max_value: None,
+placeholder: None,
 max_content_width: None,
 allow_empty: false,
 }
@@ -54,6 +57,7 @@ impl 
NumericEditView {
 Self {
 view,
 max_value: None,
+placeholder: None,
 max_content_width: None,
 allow_empty: false,
 }
@@ -84,15 +88,42 @@ impl 
NumericEditView {
 self
 }

+/// Sets a placeholder value for this view. Implies `allow_empty(true)`.
+/// Implies `allow_empty(true)`.
+///
+/// # Arguments
+/// `placeholder` - The placeholder value to set for this view.
+#[allow(unused)]
+pub fn placeholder(mut self, placeholder: T) -> Self {
+self.placeholder = Some(placeholder);
+self.allow_empty(true)
+}
+
+/// Returns the current value of the view. If a placeholder is defined and
+/// no value is currently set, the placeholder value is returned.
+///
+/// **This should only be called when `allow_empty = false` or a 
placeholder
+/// is set.**
 pub fn get_content(&self) -> Result::Err> {
-assert!(!self.allow_empty);
-self.inner().get_content().parse()
+let content = self.inner().get_content();
+
+if content.is_empty() {
+if let Some(placeholder) = self.placeholder {
+return Ok(placeholder);
+}
+}
+
+assert!(!(self.allow_empty && self.placeholder.is_none()));
+content.parse()
 }

+/// Returns the current value of the view, or [`None`] if the view is
+/// currently empty.
 pub fn get_content_maybe(&self) -> Option::Err>> {
 let content = self.inner().get_content();
+
 if !content.is_empty() {
-Some(self.inner().get_content().parse())
+Some(content.parse())
 } else {
 None
 }
@@ -157,6 +188,25 @@ impl 
NumericEditView {
 std::mem::swap(self.inner_mut(), &mut inner);
 self
 }
+
+/// Generic `wrap_draw()` implementation for [`ViewWrapper`].
+///
+/// # Arguments
+/// * `printer` - The [`Printer`] to draw to the base view.
+fn wrap_draw_inner(&self, printer: &Printer) {
+self.view.draw(printer);
+
+if self.inner().get_content().is_empty() && !printer.focused {
+if let Some(placeholder) = self.placeholder {
+let placeholder = placeholder.to_string();
+
+printer.with_color(
+(BaseColor::Blue.light(), BaseColor::Blue.dark()).into(),
+|printer| printer.print((0, 0), &placeholder),
+);
+}
+}
+}
 }

 pub type FloatEditView = NumericEditView;
@@ -165,6 +215,10 @@ pub type IntegerEditView = NumericEditView;
 impl ViewWrapper for FloatEditView {
 cursive::wrap_impl!(self.view: LinearLayout);

+fn wrap_draw(&self, printer: &Printer) {
+self.wrap_draw_inner(printer);
+}
+
 fn wrap_on_event(&mut self, event: Event) -> EventResult {
 let original = self.inner_mut().get_content();

@@ -204,6 +258,10 @@ impl FloatEditView {
 impl ViewWrapper for IntegerEditView {
 cursive::wrap_impl!(self.view: LinearLayout);

+fn wrap_draw(&self, printer: &Printer) {
+self.wrap_draw_inner(printer);
+}
+
 fn wrap_on_event(&mut self, event: Event) -> EventResult {
 let original = self.inner_mut().get_content();

--
2.44.0



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.pro

[pve-devel] [PATCH installer v3 2/3] tui: expose arc size setting for zfs bootdisks for all products

2024-04-15 Thread Christoph Heiss
For non-PVE products, simply use the ZFS defaults (aka. 50%) and leave
unset, if the user never touches that setting.

Signed-off-by: Christoph Heiss 
---
Changes v2 -> v3:
  * no changes

Changes v1 -> v2:
  * use new placeholder functionality for non-PVE products

 proxmox-installer-common/src/options.rs |  3 +-
 proxmox-tui-installer/src/views/bootdisk.rs | 45 -
 proxmox-tui-installer/src/views/mod.rs  |  1 -
 3 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/proxmox-installer-common/src/options.rs 
b/proxmox-installer-common/src/options.rs
index 1aa8f65..a210142 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -205,7 +205,8 @@ impl ZfsBootdiskOptions {
 /// The default ZFS maximum ARC size in MiB for this system.
 fn default_zfs_arc_max(product: ProxmoxProduct, total_memory: usize) -> usize {
 if product != ProxmoxProduct::PVE {
-// Use ZFS default for non-PVE
+// For products other the PVE, just let ZFS decide on its own. Setting 
`0`
+// causes the installer to skip writing the `zfs_arc_max` module 
parameter.
 0
 } else {
 ((total_memory as f64) / 10.)
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs 
b/proxmox-tui-installer/src/views/bootdisk.rs
index 7e13e91..3d9be50 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -564,7 +564,18 @@ impl ZfsBootdiskOptionsView {
 options: &ZfsBootdiskOptions,
 product_conf: &ProductConfig,
 ) -> Self {
-let is_pve = product_conf.product == ProxmoxProduct::PVE;
+let arc_max_view = {
+let view = 
IntegerEditView::new_with_suffix("MiB").max_value(runinfo.total_memory);
+
+// For PVE "force" the default value, for other products place the 
recommended value
+// only in the placeholder. This causes for the latter to not 
write the module option
+// if the value is never modified by the user.
+if product_conf.product == ProxmoxProduct::PVE {
+view.content(options.arc_max)
+} else {
+view.placeholder(runinfo.total_memory / 2)
+}
+};

 let inner = FormView::new()
 .child("ashift", IntegerEditView::new().content(options.ashift))
@@ -592,14 +603,11 @@ impl ZfsBootdiskOptionsView {
 .unwrap_or_default(),
 ),
 )
-.child("copies", 
IntegerEditView::new().content(options.copies).max_value(3))
-.child_conditional(
-is_pve,
-"ARC max size",
-IntegerEditView::new_with_suffix("MiB")
-.max_value(runinfo.total_memory)
-.content(options.arc_max),
+.child(
+"copies",
+IntegerEditView::new().content(options.copies).max_value(3),
 )
+.child("ARC max size", arc_max_view)
 .child("hdsize", 
DiskSizeEditView::new().content(options.disk_size));

 let view = MultiDiskOptionsView::new(&runinfo.disks, 
&options.selected_disks, inner)
@@ -621,21 +629,22 @@ impl ZfsBootdiskOptionsView {
 fn get_values(&mut self) -> Option<(Vec, ZfsBootdiskOptions)> {
 let (disks, selected_disks) = self.view.get_disks_and_selection()?;
 let view = self.view.inner_mut()?;
-let has_arc_max = view.len() >= 6;
-let disk_size_index = if has_arc_max { 5 } else { 4 };

 let ashift = view.get_value::(0)?;
 let compress = view.get_value::, _>(1)?;
 let checksum = view.get_value::, _>(2)?;
 let copies = view.get_value::(3)?;
-let disk_size = view.get_value::(disk_size_index)?;
-
-let arc_max = if has_arc_max {
-view.get_value::(4)?
-.max(ZFS_ARC_MIN_SIZE_MIB)
-} else {
-0 // use built-in ZFS default value
-};
+let disk_size = view.get_value::(5)?;
+
+// If a value is set, return that and clamp it to at least 
[`ZFS_ARC_MIN_SIZE_MIB`].
+//
+// Otherwise, if no value was set or an error occured return `0`. The 
former simply means
+// that the placeholder value is still there.
+let arc_max = view
+.get_child::(4)?
+.get_content_maybe()
+.map_or(Ok(0), |v| v.map(|v| v.max(ZFS_ARC_MIN_SIZE_MIB)))
+.unwrap_or(0);

 Some((
 disks,
diff --git a/proxmox-tui-installer/src/views/mod.rs 
b/proxmox-tui-installer/src/views/mod.rs
index 5e5f4fb..eab258c 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -93,7 +93,6 @@ impl 
NumericEditView {
 ///
 /// # Arguments
 /// `placeholder` - The placeholder value to set for this view.
-#[allow(unused)]
 pub fn placeholder(mut self, plac

[pve-devel] [PATCH installer v3 3/3] proxinstall: expose arc size setting for zfs bootdisks for all products

2024-04-15 Thread Christoph Heiss
For non-PVE products, simply use the ZFS defaults (aka. 50%) and leave
unset, if the user never touches that setting.

Signed-off-by: Christoph Heiss 
---
Changes v2 -> v3:
  * rework based on Maximilano's suggestion using Gtk3::Adjustment

Changes v1 -> v2:
  * add some more explanatory comments

 Proxmox/Install/RunEnv.pm |  3 ++-
 proxinstall   | 26 +-
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index 25b6bb3..4e24da6 100644
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -319,7 +319,8 @@ our $ZFS_ARC_SYSMEM_PERCENTAGE = 0.1; # use 10% of 
available system memory by de
 # Calculates the default upper limit for the ZFS ARC size.
 # Returns the default ZFS maximum ARC size in MiB.
 sub default_zfs_arc_max {
-# Use ZFS default on non-PVE
+# For products other the PVE, just let ZFS decide on its own. Setting `0`
+# causes the installer to skip writing the `zfs_arc_max` module parameter.
 return 0 if Proxmox::Install::ISOEnv::get('product') ne 'pve';

 my $default_mib = get('total_memory') * $ZFS_ARC_SYSMEM_PERCENTAGE;
diff --git a/proxinstall b/proxinstall
index a6a4cfb..bc0e1e4 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1138,20 +1138,20 @@ my $create_raid_advanced_grid = sub {
 $spinbutton_copies->set_value($copies);
 push @$labeled_widgets, ['copies', $spinbutton_copies];

-if ($iso_env->{product} eq 'pve') {
-   my $total_memory = Proxmox::Install::RunEnv::get('total_memory');
+my $total_memory = Proxmox::Install::RunEnv::get('total_memory');
+my $arc_max = Proxmox::Install::Config::get_zfs_opt('arc_max') || 
($total_memory * 0.5);
+
+my $arc_max_adjustment = Gtk3::Adjustment->new(
+   $arc_max, $Proxmox::Install::RunEnv::ZFS_ARC_MIN_SIZE_MIB,
+   $total_memory, 1, 10, 0);
+my $spinbutton_arc_max = Gtk3::SpinButton->new($arc_max_adjustment, 1, 0);
+$spinbutton_arc_max->set_tooltip_text('Maximum ARC size in megabytes');
+$spinbutton_arc_max->signal_connect('value-changed' => sub {
+   my $w = shift;
+   Proxmox::Install::Config::set_zfs_opt('arc_max', 
$w->get_value_as_int());
+});

-   my $spinbutton_arc_max = Gtk3::SpinButton->new_with_range(
-   $Proxmox::Install::RunEnv::ZFS_ARC_MIN_SIZE_MIB, $total_memory, 1);
-   $spinbutton_arc_max->set_tooltip_text('Maximum ARC size in megabytes');
-   $spinbutton_arc_max->signal_connect('value-changed' => sub {
-   my $w = shift;
-   Proxmox::Install::Config::set_zfs_opt('arc_max', 
$w->get_value_as_int());
-   });
-   my $arc_max = Proxmox::Install::Config::get_zfs_opt('arc_max');
-   $spinbutton_arc_max->set_value($arc_max);
-   push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB'];
-}
+push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB'];

 push @$labeled_widgets, ['hdsize', $hdsize_btn, 'GB'];
 return $create_label_widget_grid->($labeled_widgets);;
--
2.44.0



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



[pve-devel] [PATCH installer v3 0/3] expose zfs arc size setting for all products

2024-04-15 Thread Christoph Heiss
As suggested by Thomas, leaves the ZFS default if the user never touches
the setting in the installer (i.e. not writing a modprobe file).
See also the discussion in v1 [0].

[0] https://lists.proxmox.com/pipermail/pve-devel/2024-February/061659.html

Testing
---
Tested the installation of PVE and PBS (PMG is handled the same as PBS),
with each once letting the arc size setting untouched and once setting
it to some specific value. Also checked for each whether the correct
default value was displayed. Afterwards, checked that for PVE the module
parameter was always written to /etc/modprobe.d/, for PBS that it was
only written in case it was explicitly set.

Previous revisions
--
v2: https://lists.proxmox.com/pipermail/pve-devel/2024-February/061667.html
v1: https://lists.proxmox.com/pipermail/pve-devel/2023-November/060898.html

Changes v2 -> v3:
  * tui: when empty & focused, do not show the placeholder value at all
  * gui: rework using Gtk3::Adjustment based on Maximilanos suggestion

Changes v1 -> v2:
  * rebased on latest master
  * add placeholder functionality for arc max size in TUI
  * "emulate" placeholder functionality in GTK on best-effort basis

Christoph Heiss (3):
  tui: NumericEditView: add optional placeholder value
  tui: expose arc size setting for zfs bootdisks for all products
  proxinstall: expose arc size setting for zfs bootdisks for all
products

 Proxmox/Install/RunEnv.pm   |  3 +-
 proxinstall | 26 -
 proxmox-installer-common/src/options.rs |  3 +-
 proxmox-tui-installer/src/views/bootdisk.rs | 45 --
 proxmox-tui-installer/src/views/mod.rs  | 65 +++--
 5 files changed, 105 insertions(+), 37 deletions(-)

--
2.42.0



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



Re: [pve-devel] [PATCH container] Fix invalid device passthrough being added to config

2024-04-15 Thread Filip Schauer

On 11/04/2024 14:18, Fiona Ebner wrote:

-
my $absolute_path = $device->{path};
my ($mode, $rdev) = (stat($absolute_path))[2, 6];
  
-	die "Device $absolute_path does not exist\n" if $! == ENOENT;

-
die "Error accessing device $absolute_path\n"
if (!defined($mode) || !defined($rdev));
  
-	die "$absolute_path is not a device\n"

-   if (!S_ISBLK($mode) && !S_ISCHR($mode));
-

Is there any downside to keeping these checks here as well? What a path
points to might change in between being set in the config and some later
time when the container is started, so these checks still make sense
here IMHO. Could then become a helper function since it's used in two
places, which would also reduce the amount of lines in the
update_{pct,lxc}_config functions.


Good point, I sent a patch v2:
https://lists.proxmox.com/pipermail/pve-devel/2024-April/062973.html



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



[pve-devel] [PATCH common/container v2 0/2] fix invalid device passthrough being added to config

2024-04-15 Thread Filip Schauer
Fix a bug that allows a device passthrough entry to be added to the
config despite the device path not pointing to a device. Previously,
adding an invalid device passthrough entry would throw an error, but the
entry would still be added to the config. This is fixed by moving the
respective checks from update_lxc_config to update_pct_config, which is
run before the entry is written to the config file.

Changes since v1:
* Use "if" instead of "unless"
* Move device path validation and stat to seperate helper function

pve-common:

Filip Schauer (1):
  add get_device_stat helper subroutine

 src/PVE/Tools.pm | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)


pve-container:

Filip Schauer (1):
  fix invalid device passthrough being added to config

 src/PVE/LXC.pm| 18 --
 src/PVE/LXC/Config.pm | 11 ++-
 2 files changed, 14 insertions(+), 15 deletions(-)


Summary over all repositories:
  3 files changed, 31 insertions(+), 16 deletions(-)

-- 
Generated by git-murpp 0.6.0


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



[pve-devel] [PATCH container v2 2/2] fix invalid device passthrough being added to config

2024-04-15 Thread Filip Schauer
Fix a bug that allows a device passthrough entry to be added to the
config despite the device path not pointing to a device. Previously,
adding an invalid device passthrough entry would throw an error, but the
entry would still be added to the config. This is fixed by moving the
respective checks from update_lxc_config to update_pct_config, which is
run before the entry is written to the config file.

Signed-off-by: Filip Schauer 
---
 src/PVE/LXC.pm| 18 --
 src/PVE/LXC/Config.pm | 11 ++-
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 9681d74..933b7f7 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -4,7 +4,7 @@ use strict;
 use warnings;
 
 use Cwd qw();
-use Errno qw(ELOOP ENOENT ENOTDIR EROFS ECONNREFUSED EEXIST);
+use Errno qw(ELOOP ENOTDIR EROFS ECONNREFUSED EEXIST);
 use Fcntl qw(O_RDONLY O_WRONLY O_NOFOLLOW O_DIRECTORY :mode);
 use File::Path;
 use File::Spec;
@@ -643,20 +643,10 @@ sub update_lxc_config {
 PVE::LXC::Config->foreach_passthrough_device($conf, sub {
my ($key, $device) = @_;
 
-   die "Path is not defined for passthrough device $key"
-   unless (defined($device->{path}));
-
-   my $absolute_path = $device->{path};
-   my ($mode, $rdev) = (stat($absolute_path))[2, 6];
-
-   die "Device $absolute_path does not exist\n" if $! == ENOENT;
-
-   die "Error accessing device $absolute_path\n"
-   if (!defined($mode) || !defined($rdev));
-
-   die "$absolute_path is not a device\n"
-   if (!S_ISBLK($mode) && !S_ISCHR($mode));
+   die "Path is not defined for passthrough device $key\n"
+   if !defined($device->{path});
 
+   my ($mode, $rdev) = PVE::Tools::get_device_stat($device->{path});
my $major = PVE::Tools::dev_t_major($rdev);
my $minor = PVE::Tools::dev_t_minor($rdev);
my $device_type_char = S_ISBLK($mode) ? 'b' : 'c';
diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 5ac1446..fba20a1 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -3,7 +3,8 @@ package PVE::LXC::Config;
 use strict;
 use warnings;
 
-use Fcntl qw(O_RDONLY);
+use Errno qw(ENOENT);
+use Fcntl qw(O_RDONLY :mode);
 
 use PVE::AbstractConfig;
 use PVE::Cluster qw(cfs_register_file);
@@ -1193,6 +1194,14 @@ sub update_pct_config {
die "$opt: MTU size '$mtu' is bigger than bridge MTU 
'$bridge_mtu'\n"
if ($mtu > $bridge_mtu);
}
+   } elsif ($opt =~ m/^dev(\d+)$/) {
+   my $device = $class->parse_device($value);
+
+   die "Path is not defined for passthrough device $opt"
+   if !defined($device->{path});
+
+# Validate device
+PVE::Tools::get_device_stat($device->{path});
}
$conf->{pending}->{$opt} = $value;
$class->remove_from_pending_delete($conf, $opt);
-- 
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 common v2 1/2] add get_device_stat helper subroutine

2024-04-15 Thread Filip Schauer
The get_device_stat subroutine gets a device path, validates it, and
returns the file mode and the device identifier.

Signed-off-by: Filip Schauer 
---
 src/PVE/Tools.pm | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 766c809..d1b53e5 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -7,7 +7,8 @@ use Date::Format qw(time2str);
 use Digest::MD5;
 use Digest::SHA;
 use Encode;
-use Fcntl qw(:DEFAULT :flock);
+use Errno qw(ENOENT);
+use Fcntl qw(:DEFAULT :flock :mode);
 use File::Basename;
 use File::Path qw(make_path);
 use Filesys::Df (); # don't overwrite our df()
@@ -1853,6 +1854,21 @@ sub dev_t_minor($) {
 return (($dev_t >> 12) & 0xfff00) | ($dev_t & 0xff);
 }
 
+sub get_device_stat {
+my ($path) = @_;
+
+die "Path is not defined\n" if !defined($path);
+
+my ($mode, $rdev) = (stat($path))[2, 6];
+die "Device $path does not exist\n" if $! == ENOENT;
+die "Error accessing device $path\n"
+   if (!defined($mode) || !defined($rdev));
+die "$path is not a device\n"
+   if (!S_ISBLK($mode) && !S_ISCHR($mode));
+
+return ($mode, $rdev);
+}
+
 # Given an array of array refs [ \[a b c], \[a b b], \[e b a] ]
 # Returns the intersection of elements as a single array [a b]
 sub array_intersect {
-- 
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 v2 pve-manager] ui: storage: add is_mountpoint checkmark to directory storage edit

2024-04-15 Thread Fiona Ebner
Am 23.02.24 um 13:03 schrieb Hannes Laimer:
> Signed-off-by: Hannes Laimer 

Reviewed-by: Fiona Ebner 

> ---
> 
> came up in enterprise support, and I don't think there is a reason to
> not have it in the UI, while having it in the API
> 

This rationale could/should become the commit message, i.e. the fact
that having it in the UI is convenient for many users.

The plugin for BTRFS also supports this option. I think it makes sense
adding it there too. Could even be done in storage/Base.js like for
preallocation, but as long as there is no third plugin with the option,
duplication is also fine IMHO.


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



[pve-devel] [PATCH kernel 1/1] cherry-pick improved erratum 1386 workaround

2024-04-15 Thread Folke Gleumes
The original fix disabled the xsaves feature for zen1/2. The issue has
since been fixed in the cpus microcode and this patch keeps the feature enabled
if the microcode version is recent enough to contain the fix.
The patch had to be altered slightly to apply cleanly on 6.5, but no
changes content-wise.

Signed-off-by: Folke Gleumes 
---

Tested this on an AMD Epyc 7302P v2.
This patch is intended for the bookworm-6.5 branch.

 ...-Improve-the-erratum-1386-workaround.patch | 83 +++
 1 file changed, 83 insertions(+)
 create mode 100644 
patches/kernel/0017-x86-CPU-AMD-Improve-the-erratum-1386-workaround.patch

diff --git 
a/patches/kernel/0017-x86-CPU-AMD-Improve-the-erratum-1386-workaround.patch 
b/patches/kernel/0017-x86-CPU-AMD-Improve-the-erratum-1386-workaround.patch
new file mode 100644
index 000..86b1222
--- /dev/null
+++ b/patches/kernel/0017-x86-CPU-AMD-Improve-the-erratum-1386-workaround.patch
@@ -0,0 +1,83 @@
+From fe4261ef5f99878f60290709d10d44bba326f95f Mon Sep 17 00:00:00 2001
+From: "Borislav Petkov (AMD)" 
+Date: Sun, 24 Mar 2024 20:51:35 +0100
+Subject: [PATCH] x86/CPU/AMD: Improve the erratum 1386 workaround
+
+Disable XSAVES only on machines which haven't loaded the microcode
+revision containing the erratum fix.
+
+This will come in handy when running archaic OSes as guests. OSes whose
+brilliant programmers thought that CPUID is overrated and one should not
+query it but use features directly, ala shoot first, ask questions
+later... but only if you're alive after the shooting.
+
+Signed-off-by: Borislav Petkov (AMD) 
+[ FG: port to 6.5 ]
+Signed-off-by: Folke Gleumes 
+Tested-by: "Maciej S. Szmigiero" 
+Cc: Boris Ostrovsky 
+Link: 
https://lore.kernel.org/r/20240324200525.GBZgCHhYFsBj12PrKv@fat_crate.local
+---
+ arch/x86/include/asm/cpu_device_id.h |  8 
+ arch/x86/kernel/cpu/amd.c| 11 +++
+ 2 files changed, 19 insertions(+)
+
+diff --git a/arch/x86/include/asm/cpu_device_id.h 
b/arch/x86/include/asm/cpu_device_id.h
+index eb8fcede9e3b..bf4e065cf1e2 100644
+--- a/arch/x86/include/asm/cpu_device_id.h
 b/arch/x86/include/asm/cpu_device_id.h
+@@ -190,6 +190,14 @@ struct x86_cpu_desc {
+   .x86_microcode_rev  = (revision),   \
+ }
+ 
++#define AMD_CPU_DESC(fam, model, stepping, revision) {\
++  .x86_family = (fam),\
++  .x86_vendor = X86_VENDOR_AMD,   \
++  .x86_model  = (model),  \
++  .x86_stepping   = (stepping),   \
++  .x86_microcode_rev  = (revision),   \
++}
++
+ extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match);
+ extern bool x86_cpu_has_min_microcode_rev(const struct x86_cpu_desc *table);
+ 
+diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
+index 9390074ddb25..8201271f6505 100644
+--- a/arch/x86/kernel/cpu/amd.c
 b/arch/x86/kernel/cpu/amd.c
+@@ -13,6 +13,7 @@
+ #include 
+ #include 
+ #include 
++#include 
+ #include 
+ #include 
+ #include 
+@@ -945,6 +946,11 @@ static void init_amd_bd(struct cpuinfo_x86 *c)
+   clear_rdrand_cpuid_bit(c);
+ }
+ 
++static const struct x86_cpu_desc erratum_1386_microcode[] = {
++  AMD_CPU_DESC(0x17,  0x1, 0x2, 0x0800126e),
++  AMD_CPU_DESC(0x17, 0x31, 0x0, 0x08301052),
++};
++
+ void init_spectral_chicken(struct cpuinfo_x86 *c)
+ {
+ #ifdef CONFIG_CPU_UNRET_ENTRY
+@@ -972,7 +978,12 @@ void init_spectral_chicken(struct cpuinfo_x86 *c)
+*
+* Affected parts all have no supervisor XSAVE states, meaning that
+* the XSAVEC instruction (which works fine) is equivalent.
++   * Clear the feature flag only on microcode revisions which
++   * don't have the fix.
+*/
++  if (x86_cpu_has_min_microcode_rev(erratum_1386_microcode))
++  return;
++
+   clear_cpu_cap(c, X86_FEATURE_XSAVES);
+ }
+ 
+-- 
+2.39.2
+
-- 
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 kernel 1/1] cherry-pick improved erratum 1386 workaround

2024-04-15 Thread Folke Gleumes
The original fix disabled the xsaves feature for zen1/2. The issue has
since been fixed in the cpus microcode and this patch keeps the feature enabled
if the microcode version is recent enough to contain the fix.

Signed-off-by: Folke Gleumes 
---

Tested this on an AMD Epyc 7302P v2.

 ...-improve-the-erratum-1386-workaround.patch | 82 +++
 1 file changed, 82 insertions(+)
 create mode 100644 
patches/kernel/0013-improve-the-erratum-1386-workaround.patch

diff --git a/patches/kernel/0013-improve-the-erratum-1386-workaround.patch 
b/patches/kernel/0013-improve-the-erratum-1386-workaround.patch
new file mode 100644
index 000..969c10c
--- /dev/null
+++ b/patches/kernel/0013-improve-the-erratum-1386-workaround.patch
@@ -0,0 +1,82 @@
+From 29ba89f1895285f06c333546882e0c5ae9a6df23 Mon Sep 17 00:00:00 2001
+From: "Borislav Petkov (AMD)" 
+Date: Sun, 24 Mar 2024 20:51:35 +0100
+Subject: x86/CPU/AMD: Improve the erratum 1386 workaround
+
+Disable XSAVES only on machines which haven't loaded the microcode
+revision containing the erratum fix.
+
+This will come in handy when running archaic OSes as guests. OSes whose
+brilliant programmers thought that CPUID is overrated and one should not
+query it but use features directly, ala shoot first, ask questions
+later... but only if you're alive after the shooting.
+
+Signed-off-by: Borislav Petkov (AMD) 
+Tested-by: "Maciej S. Szmigiero" 
+Cc: Boris Ostrovsky 
+Link: 
https://lore.kernel.org/r/20240324200525.GBZgCHhYFsBj12PrKv@fat_crate.local
+---
+ arch/x86/include/asm/cpu_device_id.h |  8 
+ arch/x86/kernel/cpu/amd.c| 12 
+ 2 files changed, 20 insertions(+)
+
+diff --git a/arch/x86/include/asm/cpu_device_id.h 
b/arch/x86/include/asm/cpu_device_id.h
+index eb8fcede9e3bf4..bf4e065cf1e2fc 100644
+--- a/arch/x86/include/asm/cpu_device_id.h
 b/arch/x86/include/asm/cpu_device_id.h
+@@ -190,6 +190,14 @@ struct x86_cpu_desc {
+   .x86_microcode_rev  = (revision),   \
+ }
+ 
++#define AMD_CPU_DESC(fam, model, stepping, revision) {\
++  .x86_family = (fam),\
++  .x86_vendor = X86_VENDOR_AMD,   \
++  .x86_model  = (model),  \
++  .x86_stepping   = (stepping),   \
++  .x86_microcode_rev  = (revision),   \
++}
++
+ extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match);
+ extern bool x86_cpu_has_min_microcode_rev(const struct x86_cpu_desc *table);
+ 
+diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
+index 6d8677e80ddbb1..873f0fdc2ef8a4 100644
+--- a/arch/x86/kernel/cpu/amd.c
 b/arch/x86/kernel/cpu/amd.c
+@@ -13,6 +13,7 @@
+ #include 
+ #include 
+ #include 
++#include 
+ #include 
+ #include 
+ #include 
+@@ -802,6 +803,11 @@ static void init_amd_bd(struct cpuinfo_x86 *c)
+   clear_rdrand_cpuid_bit(c);
+ }
+ 
++static const struct x86_cpu_desc erratum_1386_microcode[] = {
++  AMD_CPU_DESC(0x17,  0x1, 0x2, 0x0800126e),
++  AMD_CPU_DESC(0x17, 0x31, 0x0, 0x08301052),
++};
++
+ static void fix_erratum_1386(struct cpuinfo_x86 *c)
+ {
+   /*
+@@ -811,7 +817,13 @@ static void fix_erratum_1386(struct cpuinfo_x86 *c)
+*
+* Affected parts all have no supervisor XSAVE states, meaning that
+* the XSAVEC instruction (which works fine) is equivalent.
++   *
++   * Clear the feature flag only on microcode revisions which
++   * don't have the fix.
+*/
++  if (x86_cpu_has_min_microcode_rev(erratum_1386_microcode))
++  return;
++
+   clear_cpu_cap(c, X86_FEATURE_XSAVES);
+ }
+ 
+-- 
+cgit 1.2.3-korg
+
-- 
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] storage: pbs: document port option

2024-04-15 Thread Fiona Ebner
Signed-off-by: Fiona Ebner 
---
 pve-storage-pbs.adoc | 4 
 1 file changed, 4 insertions(+)

diff --git a/pve-storage-pbs.adoc b/pve-storage-pbs.adoc
index c33ee40..84d598f 100644
--- a/pve-storage-pbs.adoc
+++ b/pve-storage-pbs.adoc
@@ -24,6 +24,10 @@ server::
 
 Server IP or DNS name. Required.
 
+port::
+
+Use this port instead of the default one, i.e. `8007`. Optional.
+
 username::
 
 The username for the Proxmox Backup Server storage. Required.
-- 
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 storage] plugin: move definition for 'port' option to base plugin

2024-04-15 Thread Fiona Ebner
Commit 7020491 ("esxi: add 'port' config parameter") started using
the 'port' option in a second plugin, but the definition stayed in the
PBS plugin. Avoid the hidden dependency and move the definition to the
base plugin instead.

It is necessary to mark it as optional or it would be required always.

Signed-off-by: Fiona Ebner 
---
 src/PVE/Storage/PBSPlugin.pm | 6 --
 src/PVE/Storage/Plugin.pm| 8 
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/PVE/Storage/PBSPlugin.pm b/src/PVE/Storage/PBSPlugin.pm
index 08ceb88..0808bcc 100644
--- a/src/PVE/Storage/PBSPlugin.pm
+++ b/src/PVE/Storage/PBSPlugin.pm
@@ -49,12 +49,6 @@ sub properties {
description => "Base64-encoded, PEM-formatted public RSA key. Used 
to encrypt a copy of the encryption-key which will be added to each encrypted 
backup.",
type => 'string',
},
-   port => {
-   description => "For non default port.",
-   type => 'integer',
-   minimum => 1,
-   maximum => 65535,
-   },
 };
 }
 
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 22a9729..5f49830 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -205,6 +205,14 @@ my $defaultData = {
format => 'pve-storage-options',
optional => 1,
},
+   port => {
+   description => "For PBS/ESXi, use this port to connect to the 
storage instead of the"
+   ." default one.",
+   type => 'integer',
+   minimum => 1,
+   maximum => 65535,
+   optional => 1,
+   },
 },
 };
 
-- 
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 manager 2/4] pvestatd: collect and broadcast pool usage

2024-04-15 Thread Fabian Grünbichler
On April 11, 2024 11:32 am, Wolfgang Bumiller wrote:
> On Wed, Apr 10, 2024 at 03:13:08PM +0200, Fabian Grünbichler wrote:
>> so that other nodes can query it and both block changes that would violate 
>> the
>> limits, and mark pools which are violating it currently accordingly.
>> 
>> Signed-off-by: Fabian Grünbichler 
>> ---
>>  PVE/Service/pvestatd.pm | 59 ++---
>>  1 file changed, 55 insertions(+), 4 deletions(-)
>> 
>> diff --git a/PVE/Service/pvestatd.pm b/PVE/Service/pvestatd.pm
>> index 2515120c6..d7e4755e9 100755
>> --- a/PVE/Service/pvestatd.pm
>> +++ b/PVE/Service/pvestatd.pm
>> @@ -231,7 +231,7 @@ sub auto_balloning {
>>  }
>>  
>>  sub update_qemu_status {
>> -my ($status_cfg) = @_;
>> +my ($status_cfg, $pool_membership, $pool_usage) = @_;
>>  
>>  my $ctime = time();
>>  my $vmstatus = PVE::QemuServer::vmstatus(undef, 1);
>> @@ -242,6 +242,21 @@ sub update_qemu_status {
>>  my $transactions = PVE::ExtMetric::transactions_start($status_cfg);
>>  foreach my $vmid (keys %$vmstatus) {
>>  my $d = $vmstatus->{$vmid};
>> +
>> +if (my $pool = $pool_membership->{$vmid}) {
>> +$pool_usage->{$pool}->{$vmid} = {
>> +cpu => {
>> +config => ($d->{confcpus} // 0),
>> +run => ($d->{runcpus} // 0),
>> +},
>> +mem => {
>> +config => ($d->{confmem} // 0),
>> +run => ($d->{runmem} // 0),
>> +},
> 
> I feel like it should be possible to build this hash given the `keys` in
> the limit hash... The `cpu-run/config` vs `{cpu}->{run}` vs `runcpu`
> naming feels a bit awkward to me.

not really, unless you want me to introduce a helper that is longer than
the hard-coded variant here?

I already have one for limit key -> usage hash (keys) for places where
we are mostly 'key agnostic' (i.e., PVE::GuestHelpers), but the vmstatus
hash has different keys again (see reply there) and those are only
relevant (in relation to the other limit/usage stuff) for the two subs
here, so if we extend the vmstatus return schema (e.g., with fields for
disk limits), then we'd need to extend the helper here anyway, just like
we'd need to extend the hard-coded variant, so I don't really see a
benefit?

adding a new limit "category" will require changes to:
- pve-access-control (for the user.cfg schema)
- qemu-server/pve-container (for actually providing the usage data via
  vmstatus, and checking the limits in all the places where current
  usage would change)
- pve-manager (for displaying/editing/.. and pvestatd conversion between
  vmstatus and usage, i.e., this part here)


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


Re: [pve-devel] [PATCH v3 manager 2/2] ui: lxc: add edit window for device passthrough

2024-04-15 Thread Fiona Ebner
Am 31.01.24 um 16:03 schrieb Filip Schauer:
> diff --git a/www/manager6/lxc/DeviceEdit.js b/www/manager6/lxc/DeviceEdit.js
> new file mode 100644
> index ..445f8607
> --- /dev/null
> +++ b/www/manager6/lxc/DeviceEdit.js
> @@ -0,0 +1,190 @@
> +Ext.define('PVE.lxc.DeviceInputPanel', {
> +extend: 'Proxmox.panel.InputPanel',
> +
> +autoComplete: false,
> +
> +controller: {
> + xclass: 'Ext.app.ViewController',
> + init: function(view) {
> + let me = this;
> + let vm = this.getViewModel();
> + vm.set('confid', view.confid);

Nit: Is the confid in the viewModel (and therefore the whole view model)
only used for the isCreate formula? That could also be directly passed
in and bound via cbind instead.

> + },
> +},
> +
> +viewModel: {
> + data: {
> + confid: '',
> + },
> +
> + formulas: {
> + isCreate: function(get) {
> + return !get('confid');
> + },
> + },
> +},
> +
> +setVMConfig: function(vmconfig) {
> + let me = this;
> + me.vmconfig = vmconfig;
> +
> + if (!me.confid) {

Nit: Should this be guarded by isCreate instead? If for whatever reason
setVMConfig() would be called a second time with a different config (not
currently happenening AFAICT), I suppose it would make sense to pick the
first free slot based on the new config again?

> + PVE.Utils.forEachLxcDev((i) => {
> + let name = "dev" + i.toString();
> + if (!Ext.isDefined(vmconfig[name])) {
> + me.confid = name;
> + me.down('field[name=devid]').setValue(i);
> + return false;
> + }
> + return undefined;
> + });
> + }
> +},
> +

(...)

> +
> +advancedColumn2: [
> + {
> + xtype: 'textfield',
> + name: 'mode',
> + editable: true,
> + fieldLabel: gettext('Access Mode'),
> + emptyText: '0660',
> + labelAlign: 'right',
> + validator: function(value) {
> + if (/^0[0-7]{3}$|^$/i.test(value)) {

Should we require the leading zero here? Many users will be familiar
with chown, where it is not required.

> + return true;
> + }
> +
> + return "Access mode has to be an octal number";
> + },
> + },
> +],
> +});
> +

With that said:

Reviewed-by: Fiona Ebner 


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



Re: [pve-devel] [PATCH v3 manager 1/2] utils: clarify naming of LXC mount point utils

2024-04-15 Thread Fiona Ebner
Am 31.01.24 um 16:03 schrieb Filip Schauer:
> Clarify the naming of mount point utils to clearly indicate their
> relation to LXC containers.
> 
> Signed-off-by: Filip Schauer 

Reviewed-by: Fiona Ebner 


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



Re: [pve-devel] [PATCH firewall 1/1] fix #5335: sort cluster.fw entries in ALIASES section

2024-04-15 Thread Fabian Grünbichler
> Daniel Krambrock via pve-devel  hat am 
> 11.04.2024 10:09 CEST geschrieben:

Hi!

for both this and the access-control patch, please add
- *some* commit message (e.g., something like "stable sorting in XX config file 
allows tracking changes by checking into git or when using automation like 
ansible" would be enough. try to describe the *why*, rather than the *what* - 
the latter is usually obvious when looking at the diff, especially for such 
small patches ;))
- add your Signed-off-by trailer ("git commit --amend -s" should do the trick)

note that you don't need to send a cover letter for single patches, those are 
just needed when you send bigger patch series where it makes sense to summarize 
all the patches to get a quick overview of the overall aim of the series.

thanks!

> ---
>  src/PVE/Firewall.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 77cbaf4..81a8798 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -3360,7 +3360,7 @@ my $format_aliases = sub {
>  my $raw = '';
>  
>  $raw .= "[ALIASES]\n\n";
> -foreach my $k (keys %$aliases) {
> +foreach my $k (sort keys %$aliases) {
>   my $e = $aliases->{$k};
>   $raw .= "$e->{name} $e->{cidr}";
>   $raw .= " # " . encode('utf8', $e->{comment})
> -- 
> 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-flutter-frontend 3/5] node overview: use correct color for rrd icons

2024-04-15 Thread Dominik Csapak
same as the guests use for that.

Signed-off-by: Dominik Csapak 
---
 lib/widgets/pve_node_overview.dart | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/widgets/pve_node_overview.dart 
b/lib/widgets/pve_node_overview.dart
index 020eeba..219d498 100644
--- a/lib/widgets/pve_node_overview.dart
+++ b/lib/widgets/pve_node_overview.dart
@@ -46,6 +46,8 @@ class PveNodeOverview extends StatelessWidget {
   builder: (context, state) {
 final status = state.status;
 final rrd = state.rrdData.where((e) => e.time != null);
+final fgColor =
+Theme.of(context).colorScheme.onPrimary.withOpacity(0.75);
 return SafeArea(
   child: Scaffold(
 appBar: AppBar(
@@ -97,7 +99,7 @@ class PveNodeOverview extends StatelessWidget {
   (e) => Point(
   e.time!.millisecondsSinceEpoch,
   (e.cpu ?? 0) * 100.0)),
-  icon: const Icon(Icons.memory),
+  icon: Icon(Icons.memory, color: fgColor),
   bottomRight: pageIndicator,
   dataRenderer: (data) =>
   '${data.toStringAsFixed(2)} %',
@@ -112,7 +114,8 @@ class PveNodeOverview extends StatelessWidget {
   data: rrd.map((e) => Point(
   e.time!.millisecondsSinceEpoch,
   e.memused ?? 0)),
-  icon: const 
Icon(FontAwesomeIcons.memory),
+  icon: Icon(FontAwesomeIcons.memory,
+  color: fgColor),
   bottomRight: pageIndicator,
   dataRenderer: (data) =>
   Renderers.formatSize(data),
@@ -128,7 +131,7 @@ class PveNodeOverview extends StatelessWidget {
   data: rrd.map((e) => Point(
   e.time!.millisecondsSinceEpoch,
   e.iowait ?? 0)),
-  icon: const Icon(Icons.timer),
+  icon: Icon(Icons.timer, color: fgColor),
   bottomRight: pageIndicator,
   dataRenderer: (data) =>
   data.toStringAsFixed(3),
@@ -144,7 +147,8 @@ class PveNodeOverview extends StatelessWidget {
   data: rrd.map((e) => Point(
   e.time!.millisecondsSinceEpoch,
   e.loadavg ?? 0)),
-  icon: const Icon(Icons.show_chart),
+  icon: Icon(Icons.show_chart,
+  color: fgColor),
   bottomRight: pageIndicator,
   dataRenderer: (data) =>
   data.toStringAsFixed(2),
-- 
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-login-manager 1/2] login: show custom alert dialog for password saving errors

2024-04-15 Thread Dominik Csapak
in those cases, we sometimes get ugly stack traces/exceptions, so
instead of just showing that and aborting, show a custom dialog with
the basic info that we could not save the password (+details box with
the original error) and continue instead.

Signed-off-by: Dominik Csapak 
---
 lib/proxmox_login_form.dart | 32 
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/lib/proxmox_login_form.dart b/lib/proxmox_login_form.dart
index e31d0c0..9c25126 100644
--- a/lib/proxmox_login_form.dart
+++ b/lib/proxmox_login_form.dart
@@ -527,10 +527,34 @@ class _ProxmoxLoginPageState extends 
State {
   }
 
   if (id != null) {
-if (savePW) {
-  await savePassword(id, enteredPassword);
-} else if (deletePW) {
-  await deletePassword(id);
+try {
+  if (savePW) {
+await savePassword(id, enteredPassword);
+  } else if (deletePW) {
+await deletePassword(id);
+  }
+} catch (e) {
+  await showDialog(
+  context: context,
+  builder: (context) => AlertDialog(
+title: const Text('Password saving error'),
+scrollable: true,
+content: Column(
+  mainAxisSize: MainAxisSize.min,
+  children: [
+Text('Could not save or delete password.'),
+ExpansionTile(
+  title: const Text('Details'),
+  children: [Text(e.toString())],
+)
+  ],
+),
+actions: [
+  TextButton(
+  onPressed: () => Navigator.of(context).pop(),
+  child: const Text('Continue')),
+],
+  ));
 }
   }
   await loginStorage.saveToDisk();
-- 
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-flutter-frontend 2/5] console: wrap console with appbar

2024-04-15 Thread Dominik Csapak
so one can return to the previous view without having to use the back
button of the device.

Signed-off-by: Dominik Csapak 
---
 lib/widgets/pve_console_menu_widget.dart | 107 ---
 1 file changed, 57 insertions(+), 50 deletions(-)

diff --git a/lib/widgets/pve_console_menu_widget.dart 
b/lib/widgets/pve_console_menu_widget.dart
index 243baf1..9c91d30 100644
--- a/lib/widgets/pve_console_menu_widget.dart
+++ b/lib/widgets/pve_console_menu_widget.dart
@@ -234,63 +234,70 @@ class PVEWebConsoleState extends State {
   value: ticket,
 ),
 builder: (context, snapshot) {
-  return SafeArea(
-child: InAppWebView(
-  onReceivedServerTrustAuthRequest: (controller, challenge) async {
-final cert = challenge.protectionSpace.sslCertificate;
-final certBytes = cert?.x509Certificate?.encoded;
-final sslError = challenge.protectionSpace.sslError?.message;
+  return Scaffold(
+  appBar: AppBar(
+title: const Text("Console"),
+  ),
+  body: SafeArea(
+child: InAppWebView(
+  onReceivedServerTrustAuthRequest:
+  (controller, challenge) async {
+final cert = challenge.protectionSpace.sslCertificate;
+final certBytes = cert?.x509Certificate?.encoded;
+final sslError =
+challenge.protectionSpace.sslError?.message;
 
-String? issuedTo = cert?.issuedTo?.CName.toString();
-String? hash = certBytes != null
-? sha256.convert(certBytes).toString()
-: null;
+String? issuedTo = cert?.issuedTo?.CName.toString();
+String? hash = certBytes != null
+? sha256.convert(certBytes).toString()
+: null;
 
-final settings =
-await ProxmoxGeneralSettingsModel.fromLocalStorage();
+final settings =
+await ProxmoxGeneralSettingsModel.fromLocalStorage();
 
-bool trust = false;
-if (hash != null && settings.trustedFingerprints != null) {
-  trust = settings.trustedFingerprints!.contains(hash);
-}
+bool trust = false;
+if (hash != null && settings.trustedFingerprints != null) {
+  trust = settings.trustedFingerprints!.contains(hash);
+}
 
-if (!trust) {
-  // format hash to '01:23:...' format
-  String? formattedHash = hash?.toUpperCase().replaceAllMapped(
-  RegExp(r"[a-zA-Z0-9]{2}"),
-  (match) => "${match.group(0)}:");
-  formattedHash = formattedHash?.substring(
-  0, formattedHash.length - 1); // remove last ':'
+if (!trust) {
+  // format hash to '01:23:...' format
+  String? formattedHash = hash
+  ?.toUpperCase()
+  .replaceAllMapped(RegExp(r"[a-zA-Z0-9]{2}"),
+  (match) => "${match.group(0)}:");
+  formattedHash = formattedHash?.substring(
+  0, formattedHash.length - 1); // remove last ':'
 
-  if (context.mounted) {
-trust = await showTLSWarning(
-context,
-sslError ?? 'An unknown TLS error has occurred',
-issuedTo ?? 'unknown',
-formattedHash ?? 'unknown');
-  }
-}
+  if (context.mounted) {
+trust = await showTLSWarning(
+context,
+sslError ?? 'An unknown TLS error has occurred',
+issuedTo ?? 'unknown',
+formattedHash ?? 'unknown');
+  }
+}
 
-// save Fingerprint
-if (trust && hash != null) {
-  await settings
-  .rebuild((b) => b..trustedFingerprints.add(hash))
-  .toLocalStorage();
-  print(settings.toJson());
-}
+// save Fingerprint
+if (trust && hash != null) {
+  await settings
+  .rebuild((b) => b..trustedFingerprints.add(hash))
+  .toLocalStorage();
+  print(settings.toJson());
+}
 
-final action = trust
-? ServerTrustAuthResponseAction.PROCEED
-: ServerTrustAuthResponseAction

[pve-devel] [PATCH pve-flutter-frontend 4/5] nove overview: add power settings menu

2024-04-15 Thread Dominik Csapak
similar to how it works for qemu

Signed-off-by: Dominik Csapak 
---
this depends on the dart-api-client patch, otherwise the url is wrong

 lib/bloc/pve_node_overview_bloc.dart  | 11 
 lib/widgets/pve_node_overview.dart| 24 
 .../pve_node_power_settings_widget.dart   | 59 +++
 3 files changed, 94 insertions(+)
 create mode 100644 lib/widgets/pve_node_power_settings_widget.dart

diff --git a/lib/bloc/pve_node_overview_bloc.dart 
b/lib/bloc/pve_node_overview_bloc.dart
index caeb1c4..2f07ef9 100644
--- a/lib/bloc/pve_node_overview_bloc.dart
+++ b/lib/bloc/pve_node_overview_bloc.dart
@@ -52,9 +52,20 @@ class PveNodeOverviewBloc
   final disks = await apiClient.getNodeDisksList(nodeID);
   yield latestState.rebuild((b) => b..disks.replace(disks));
 }
+if (event is PerformNodeAction) {
+  await apiClient.doResourceAction(nodeID, '', 'node', event.action,
+  parameters: {});
+  yield latestState;
+}
   }
 }
 
 abstract class PveNodeOverviewEvent {}
 
 class UpdateNodeStatus extends PveNodeOverviewEvent {}
+
+class PerformNodeAction extends PveNodeOverviewEvent {
+  final PveClusterResourceAction action;
+
+  PerformNodeAction(this.action);
+}
diff --git a/lib/widgets/pve_node_overview.dart 
b/lib/widgets/pve_node_overview.dart
index 219d498..a5d79a3 100644
--- a/lib/widgets/pve_node_overview.dart
+++ b/lib/widgets/pve_node_overview.dart
@@ -8,6 +8,7 @@ import 
'package:pve_flutter_frontend/states/pve_node_overview_state.dart';
 import 'package:pve_flutter_frontend/states/pve_task_log_state.dart';
 import 'package:pve_flutter_frontend/utils/renderers.dart';
 import 'package:pve_flutter_frontend/utils/utils.dart';
+import 
'package:pve_flutter_frontend/widgets/pve_node_power_settings_widget.dart';
 import 'package:pve_flutter_frontend/widgets/proxmox_capacity_indicator.dart';
 import 
'package:pve_flutter_frontend/widgets/proxmox_stream_builder_widget.dart';
 import 'package:pve_flutter_frontend/widgets/pve_action_card_widget.dart';
@@ -189,6 +190,16 @@ class PveNodeOverview extends StatelessWidget {
   child: Row(
 mainAxisAlignment: MainAxisAlignment.spaceEvenly,
 children: [
+  ActionCard(
+icon: const Icon(
+  Icons.power_settings_new,
+  size: 55,
+  color: Colors.white24,
+),
+title: 'Power Settings',
+onTap: () =>
+showPowerMenuBottomSheet(context, nBloc),
+  ),
   ActionCard(
 icon: const Icon(
   Icons.queue_play_next,
@@ -439,4 +450,17 @@ class PveNodeOverview extends StatelessWidget {
   },
 );
   }
+
+  Future showPowerMenuBottomSheet(
+  BuildContext context, PveNodeOverviewBloc nodeBloc) async {
+return showModalBottomSheet(
+  shape: const RoundedRectangleBorder(
+  borderRadius: BorderRadius.vertical(top: Radius.circular(10))),
+  context: context,
+  builder: (context) => Provider.value(
+value: nodeBloc,
+child: const PveNodePowerSettings(),
+  ),
+);
+  }
 }
diff --git a/lib/widgets/pve_node_power_settings_widget.dart 
b/lib/widgets/pve_node_power_settings_widget.dart
new file mode 100644
index 000..f8d35d1
--- /dev/null
+++ b/lib/widgets/pve_node_power_settings_widget.dart
@@ -0,0 +1,59 @@
+import 'package:flutter/material.dart';
+import 'package:provider/provider.dart';
+import 'package:proxmox_dart_api_client/proxmox_dart_api_client.dart';
+import 'package:pve_flutter_frontend/bloc/pve_node_overview_bloc.dart';
+import 'package:pve_flutter_frontend/states/pve_node_overview_state.dart';
+import 
'package:pve_flutter_frontend/widgets/proxmox_stream_builder_widget.dart';
+
+class PveNodePowerSettings extends StatelessWidget {
+  const PveNodePowerSettings({
+super.key,
+  });
+  @override
+  Widget build(BuildContext context) {
+final bloc = Provider.of(context);
+return ProxmoxStreamBuilder(
+bloc: bloc,
+builder: (context, state) {
+  return SafeArea(
+child: SingleChildScrollView(
+  child: Container(
+constraints: BoxConstraints(
+minHeight: MediaQuery.of(context).size.height / 3),
+child: Column(
+  mainAxisSize: MainAxisSize.min,
+  children: [
+ListTile(
+  leading: const Icon(Icons.autorenew),
+  title: const Text(
+"Reboot",
+style: TextStyle(fontWeight: FontWeight.bold),
+  ),
+  subtitle: const Text("Reboot Node"),
+ 

[pve-devel] [PATCH pve-flutter-frontend 1/5] console: use flutter inappwebview as webview

2024-04-15 Thread Dominik Csapak
instead of flutter webview. This new dependency does not have the
limitations of the "official" flutter webview. We now can ignore
https errors and even get some info on the certificate.

We use that to now show a 'do you want to trust this cert' dialog with
the fingerprint of the server, which the user can then either trust or
abort.

When pressing yes, we save that fingerprint in the shared preferences so
that we don't have to ask the next time.

Signed-off-by: Dominik Csapak 
---
this depends on the login-manager patch to add the 'trustedFingerprints' list

 lib/widgets/pve_console_menu_widget.dart | 128 ---
 pubspec.lock |  42 ++--
 pubspec.yaml |   3 +-
 3 files changed, 126 insertions(+), 47 deletions(-)

diff --git a/lib/widgets/pve_console_menu_widget.dart 
b/lib/widgets/pve_console_menu_widget.dart
index 767a51c..243baf1 100644
--- a/lib/widgets/pve_console_menu_widget.dart
+++ b/lib/widgets/pve_console_menu_widget.dart
@@ -6,7 +6,9 @@ import 'package:flutter/material.dart';
 import 'package:flutter/services.dart';
 import 'package:path_provider/path_provider.dart';
 import 'package:proxmox_dart_api_client/proxmox_dart_api_client.dart';
-import 'package:webview_flutter/webview_flutter.dart';
+import 'package:proxmox_login_manager/proxmox_general_settings_model.dart';
+import 'package:flutter_inappwebview/flutter_inappwebview.dart';
+import 'package:crypto/crypto.dart';
 
 class PveConsoleMenu extends StatelessWidget {
   static const platform =
@@ -104,8 +106,7 @@ class PveConsoleMenu extends StatelessWidget {
   "noVNC Console", // xterm.js doesn't work that well on mobile
   style: TextStyle(fontWeight: FontWeight.bold),
 ),
-subtitle: const Text(
-"Open console view (requires trusted SSL certificate)"),
+subtitle: const Text("Open console view"),
 onTap: () async {
   if (Platform.isAndroid) {
 if (['qemu', 'lxc'].contains(type)) {
@@ -209,6 +210,8 @@ class PVEWebConsole extends StatefulWidget {
 }
 
 class PVEWebConsoleState extends State {
+  InAppWebViewController? webViewController;
+
   @override
   Widget build(BuildContext context) {
 final ticket = widget.apiClient.credentials.ticket!;
@@ -222,24 +225,123 @@ class PVEWebConsoleState extends State {
 } else {
   consoleUrl += "&console=shell";
 }
-
-final controller = WebViewController()
-  ..setJavaScriptMode(JavaScriptMode.unrestricted)
-  ..setBackgroundColor(Theme.of(context).colorScheme.background);
+WidgetsFlutterBinding.ensureInitialized();
 
 return FutureBuilder(
-future: WebViewCookieManager().setCookie(WebViewCookie(
+future: CookieManager.instance().setCookie(
+  url: Uri.parse(consoleUrl),
   name: 'PVEAuthCookie',
   value: ticket,
-  domain: baseUrl.origin,
-)),
+),
 builder: (context, snapshot) {
-  controller.loadRequest(Uri.parse(consoleUrl));
   return SafeArea(
-child: WebViewWidget(
-  controller: controller,
+child: InAppWebView(
+  onReceivedServerTrustAuthRequest: (controller, challenge) async {
+final cert = challenge.protectionSpace.sslCertificate;
+final certBytes = cert?.x509Certificate?.encoded;
+final sslError = challenge.protectionSpace.sslError?.message;
+
+String? issuedTo = cert?.issuedTo?.CName.toString();
+String? hash = certBytes != null
+? sha256.convert(certBytes).toString()
+: null;
+
+final settings =
+await ProxmoxGeneralSettingsModel.fromLocalStorage();
+
+bool trust = false;
+if (hash != null && settings.trustedFingerprints != null) {
+  trust = settings.trustedFingerprints!.contains(hash);
+}
+
+if (!trust) {
+  // format hash to '01:23:...' format
+  String? formattedHash = hash?.toUpperCase().replaceAllMapped(
+  RegExp(r"[a-zA-Z0-9]{2}"),
+  (match) => "${match.group(0)}:");
+  formattedHash = formattedHash?.substring(
+  0, formattedHash.length - 1); // remove last ':'
+
+  if (context.mounted) {
+trust = await showTLSWarning(
+context,
+sslError ?? 'An unknown TLS error has occurred',
+issuedTo ?? 'unknown',
+formattedHash ?? 'unknown');
+  }
+}
+
+// save Fingerprint
+if (trust && hash != null) {
+  await settings
+  .rebuild(

[pve-devel] [PATCH pve-flutter-frontend 5/5] improve back button behavior

2024-04-15 Thread Dominik Csapak
by using 'SystemNavigator.pop()' on the top level page when pressing the
back button. On Android this minimizes/closes the app an presents
whatever was shown before.

This fixes an issue where we would show a blank screen when pressing the
back button on the dashboard.

Signed-off-by: Dominik Csapak 
---
not sure if this is the fix that we want, but i did not find any other
way. It seems that using PopScope does not really handle popping the
last route correctly. Also, this will not work for iOS (but since we
don't provide the app there, probably irrelevant)

 lib/pages/main_layout_slim.dart | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/pages/main_layout_slim.dart b/lib/pages/main_layout_slim.dart
index ec80dba..1761b26 100644
--- a/lib/pages/main_layout_slim.dart
+++ b/lib/pages/main_layout_slim.dart
@@ -1,5 +1,6 @@
 import 'package:built_collection/built_collection.dart';
 import 'package:flutter/material.dart';
+import 'package:flutter/services.dart';
 import 'package:font_awesome_flutter/font_awesome_flutter.dart';
 import 'package:intl/intl.dart';
 import 'package:provider/provider.dart';
@@ -74,7 +75,8 @@ class _MainLayoutSlimState extends State {
 if (pageSelector.value != 0) pageSelector.add(0);
 return;
   }
-  if (mounted) Navigator.of(context).pop();
+  // minimize/return from the app
+  SystemNavigator.pop();
 },
 child: StreamBuilder(
   stream: pageSelector.stream,
-- 
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 flutter-repositories] some improvements/fixes for the app

2024-04-15 Thread Dominik Csapak
some improvements:
* better error handling for saving the password
* adding node power actions (shutdown/restart)
* change webview so we can show it without a trusted certificate
* improve back button behavior

flutter-frontend patch 1 depends on login-manager patch 2
flutter-frontend patch 4 depends on dart-api-client to work correctly

this is intended on top of my last series:
https://lists.proxmox.com/pipermail/pve-devel/2024-April/062891.html

proxmox-dart-api-client:

Dominik Csapak (1):
  client: correctly set parameter for node actions

 lib/src/client.dart | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

proxmox-login-manager:

Dominik Csapak (2):
  login: show custom alert dialog for password saving errors
  general settings: add trustedFingerprints list

 lib/proxmox_general_settings_model.dart |  6 -
 lib/proxmox_login_form.dart | 32 +
 2 files changed, 33 insertions(+), 5 deletions(-)

pve-flutter-frontend:

Dominik Csapak (5):
  console: use flutter inappwebview as webview
  console: wrap console with appbar
  node overview: use correct color for rrd icons
  nove overview: add power settings menu
  improve back button behavior

 lib/bloc/pve_node_overview_bloc.dart  |  11 ++
 lib/pages/main_layout_slim.dart   |   4 +-
 lib/widgets/pve_console_menu_widget.dart  | 139 --
 lib/widgets/pve_node_overview.dart|  36 -
 .../pve_node_power_settings_widget.dart   |  59 
 pubspec.lock  |  42 ++
 pubspec.yaml  |   3 +-
 7 files changed, 240 insertions(+), 54 deletions(-)
 create mode 100644 lib/widgets/pve_node_power_settings_widget.dart

-- 
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-dart-api-client 1/1] client: correctly set parameter for node actions

2024-04-15 Thread Dominik Csapak
using '?' in the url will be escaped and not used for the get
parameters. Instead add the command to the parameters map.

Signed-off-by: Dominik Csapak 
---
 lib/src/client.dart | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/src/client.dart b/lib/src/client.dart
index 447335e..f597c28 100644
--- a/lib/src/client.dart
+++ b/lib/src/client.dart
@@ -366,7 +366,9 @@ class ProxmoxApiClient extends http.BaseClient {
 
'/api2/json/nodes/$targetNode/$type/$guestId/status/${action.name}';
 break;
   case 'node':
-url = '/api2/json/nodes/$targetNode/status/?command=${action.name}';
+parameters ??= {};
+parameters["command"] = action.name;
+url = '/api2/json/nodes/$targetNode/status';
 break;
   default:
 throw ArgumentError.value(
-- 
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-login-manager 2/2] general settings: add trustedFingerprints list

2024-04-15 Thread Dominik Csapak
where we can save the fingerprints the user wants to trust

Signed-off-by: Dominik Csapak 
---
 lib/proxmox_general_settings_model.dart | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/proxmox_general_settings_model.dart 
b/lib/proxmox_general_settings_model.dart
index 767bdaf..4c8305b 100644
--- a/lib/proxmox_general_settings_model.dart
+++ b/lib/proxmox_general_settings_model.dart
@@ -1,5 +1,6 @@
 import 'dart:convert';
 
+import 'package:built_collection/built_collection.dart';
 import 'package:built_value/built_value.dart';
 import 'package:built_value/serializer.dart';
 import 'package:proxmox_login_manager/serializers.dart';
@@ -11,6 +12,7 @@ abstract class ProxmoxGeneralSettingsModel
 implements
 Built 
{
   bool? get sslValidation;
+  BuiltList? get trustedFingerprints;
 
   ProxmoxGeneralSettingsModel._();
   factory ProxmoxGeneralSettingsModel(
@@ -18,7 +20,9 @@ abstract class ProxmoxGeneralSettingsModel
   _$ProxmoxGeneralSettingsModel;
 
   factory ProxmoxGeneralSettingsModel.defaultValues() =>
-  ProxmoxGeneralSettingsModel((b) => b..sslValidation = true);
+  ProxmoxGeneralSettingsModel((b) => b
+..sslValidation = true
+..trustedFingerprints = ListBuilder());
 
   Object toJson() {
 return serializers.serializeWith(
-- 
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 v2 2/2] ui: one-off backup: show hint if notification-system is used

2024-04-15 Thread Lukas Wagner
When mode is 'auto' and 'mailto' is empty, show hint that the
notification system will be used.

Signed-off-by: Lukas Wagner 
Tested-by: Maximiliano Sandoval 
---
 www/manager6/window/Backup.js | 37 +++
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/www/manager6/window/Backup.js b/www/manager6/window/Backup.js
index 4418a9c7..ce679971 100644
--- a/www/manager6/window/Backup.js
+++ b/www/manager6/window/Backup.js
@@ -30,10 +30,27 @@ Ext.define('PVE.window.Backup', {
name: 'mode',
});
 
+   let viewModel = new Ext.app.ViewModel({
+   formulas: {
+   showMailtoField: (get) =>
+   ['auto', 
'legacy-sendmail'].includes(get('notificationMode')),
+   hintTextVisible: (get) => get('notificationMode') === 'auto' && 
!get('mailto'),
+   },
+   data: {
+   mailto: '',
+   notificationMode: 'auto',
+   },
+   });
+
let mailtoField = Ext.create('Ext.form.field.Text', {
fieldLabel: gettext('Send email to'),
name: 'mailto',
emptyText: Proxmox.Utils.noneText,
+   viewModel,
+   bind: {
+   value: '{mailto}',
+   hidden: '{!showMailtoField}',
+   },
});
 
let notificationModeSelector = Ext.create({
@@ -46,10 +63,21 @@ Ext.define('PVE.window.Backup', {
fieldLabel: gettext('Notification mode'),
name: 'notification-mode',
value: 'auto',
-   listeners: {
-   change: function(field, value) {
-   mailtoField.setDisabled(value === 'notification-system');
-   },
+   viewModel,
+   bind: {
+   value: '{notificationMode}',
+   },
+   });
+
+   let notificationSystemHint = Ext.create({
+   xtype: 'displayfield',
+   padding: '0 0 0 5',
+   userCls: 'pmx-hint',
+   hidden: true,
+   value: gettext('No email configured, the notification system will 
be used'),
+   viewModel,
+   bind: {
+   hidden: '{!hintTextVisible}',
},
});
 
@@ -198,6 +226,7 @@ Ext.define('PVE.window.Backup', {
compressionSelector,
notificationModeSelector,
mailtoField,
+   notificationSystemHint,
removeCheckbox,
],
columnB: [
-- 
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 v2 1/2] ui: dc: backup: improve UX for the different 'notification-mode's

2024-04-15 Thread Lukas Wagner
  - 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 
Tested-by: Maximiliano Sandoval 
---

Notes:
Changes since v1:
  - Set 'mailNotification' in viewModel to 'always'
to  avoid the combobox being empty initially

 www/manager6/dc/Backup.js | 36 ++--
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 70903bdc..70b5604e 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -207,13 +207,25 @@ Ext.define('PVE.dc.BackupEdit', {
data: {
selMode: 'include',
notificationMode: '__default__',
+   mailto: '',
+   mailNotification: 'always',
},
 
formulas: {
poolMode: (get) => get('selMode') === 'pool',
-   disableVMSelection: (get) => get('selMode') !== 'include' && 
get('selMode') !== 'exclude',
+   disableVMSelection: (get) => get('selMode') !== 'include' &&
+   get('selMode') !== 'exclude',
showMailtoFields: (get) =>
['auto', 'legacy-sendmail', 
'__default__'].includes(get('notificationMode')),
+   enableMailnotificationField: (get) => {
+   let mode = get('notificationMode');
+   let mailto = get('mailto');
+
+   return (['auto', '__default__'].includes(mode) && mailto) ||
+   mode === 'legacy-sendmail';
+   },
+   hintTextVisible: (get) =>
+   ['auto', '__default__'].includes(get('notificationMode')) && 
!get('mailto'),
},
 },
 
@@ -325,6 +337,15 @@ Ext.define('PVE.dc.BackupEdit', {
value: '{notificationMode}',
},
},
+   {
+   xtype: 'textfield',
+   fieldLabel: gettext('Send email to'),
+   name: 'mailto',
+   bind: {
+   hidden: '{!showMailtoFields}',
+   value: '{mailto}',
+   },
+   },
{
xtype: 'pveEmailNotificationSelector',
fieldLabel: gettext('Send email'),
@@ -334,15 +355,18 @@ Ext.define('PVE.dc.BackupEdit', {
deleteEmpty: '{!isCreate}',
},
bind: {
-   disabled: '{!showMailtoFields}',
+   hidden: '{!showMailtoFields}',
+   disabled: 
'{!enableMailnotificationField}',
+   value: '{mailNotification}',
},
},
{
-   xtype: 'textfield',
-   fieldLabel: gettext('Send email to'),
-   name: 'mailto',
+   xtype: 'displayfield',
+   userCls: 'pmx-hint',
+   hidden: true,
+   value: gettext('No email configured, the 
notification system will be used'),
bind: {
-   disabled: '{!showMailtoFields}',
+   hidden: '{!hintTextVisible}',
},
},
{
-- 
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 guest-common 1/1] helpers: add pool limit/usage helpers

2024-04-15 Thread Fabian Grünbichler
On April 11, 2024 11:17 am, Wolfgang Bumiller wrote:
> On Wed, Apr 10, 2024 at 03:13:06PM +0200, Fabian Grünbichler wrote:
>> one for combining the per-node broadcasted values, one for checking a pool's
>> limit, and one specific helper for checking guest-related actions such as
>> starting a VM.
>> 
>> Signed-off-by: Fabian Grünbichler 
>> ---
>>  src/PVE/GuestHelpers.pm | 190 
>>  1 file changed, 190 insertions(+)
>> 
>> diff --git a/src/PVE/GuestHelpers.pm b/src/PVE/GuestHelpers.pm
>> index 961a7b8..36b44bb 100644
>> --- a/src/PVE/GuestHelpers.pm
>> +++ b/src/PVE/GuestHelpers.pm
>> @@ -416,4 +416,194 @@ sub check_vnet_access {
>>  if !($tag || $trunks);
>>  }
>>  
>> +# combines the broadcasted pool usage information to get per-pool stats
>> +#
>> +# $pools parsed pool info from user.cfg
>> +# $usage broadcasted KV hash
>> +# $pool filter for specific pool
>> +# $skip skip a certain guest to ignore its current usage
>> +#
>> +# returns usage hash:
>> +# pool -> cpu/mem/.. -> run/config -> $usage
>> +sub get_pool_usage {
>> +my ($pools, $usage, $pool, $skip) = @_;
>> +
>> +my $res = {};
>> +my $included_guests = {};
>> +for my $node (keys $usage->%*) {
>> +my $node_usage = JSON::decode_json($usage->{$node} // '');
>> +
>> +# long IDs first, so we can add children to their parents right away
>> +for my $poolid (sort {$b cmp $a} keys $pools->%*) {
>> +next if defined($pool) && !($pool eq $poolid || $poolid =~ 
>> m!^$pool/! || $pool =~ m!^$poolid/!);
>> +
>> +my $d = $res->{$poolid} //= {
>> +cpu => {
>> +run => 0,
>> +config => 0,
>> +},
>> +mem => {
>> +run => 0,
>> +config => 0,
>> +},
>> +};
>> +
>> +my $pool_usage = $node_usage->{data}->{$poolid} // {};
>> +for my $vmid (keys $pool_usage->%*) {
>> +# only include once in case of migration between broadcast
>> +next if $included_guests->{$vmid};
>> +next if $skip && $skip->{$vmid};
>> +$included_guests->{$vmid} = 1;
>> +
>> +my $vm_data = $pool_usage->{$vmid};
>> +for my $key (keys $vm_data->%*) {
>> +next if $key eq 'running';
>> +$d->{$key}->{run} += $vm_data->{$key}->{run} if 
>> $vm_data->{running};
>> +$d->{$key}->{config} += $vm_data->{$key}->{config};
> 
> So here we autovivify more keys inside $d, so simply receiving them in
> the $pool_usage will add them

well, yeah.. if they are set in pool_usage, then they represent a usage
attribute and should be summed up? ;)

> ...
> 
>> +}
>> +}
>> +
>> +if (my $parent = $pools->{$poolid}->{parent}) {
>> +$res->{$parent} //= {
>> +cpu => {
>> +run=> 0,
>> +config => 0,
>> +},
>> +mem => {
>> +run => 0,
>> +config => 0,
>> +},
>> +};
> 
> ...
> so would it make sense to just iterate over `keys %$d` here instead
> of having `cpu` and `mem` hardcoded?

yes

> And/or maybe access-control should expose a list derived from
> $pool_limits_desc directly (or helper sub) to deal with the
> `-run`/`-config` suffixes

that might make sense as well, yes.

> ...
> 
>> +$res->{$parent}->{cpu}->{run} += $d->{cpu}->{run};
>> +$res->{$parent}->{mem}->{run} += $d->{mem}->{run};
>> +$res->{$parent}->{cpu}->{config} += $d->{cpu}->{config};
>> +$res->{$parent}->{mem}->{config} += $d->{mem}->{config};
>> +}
>> +}
>> +}
>> +
>> +return $res;
>> +}
>> +
>> +# checks whether a pool is (or would be) over its resource limits
>> +#
>> +# $changes is for checking limits for config/state changes like VM starts, 
>> if
>> +# set, only the limits with changes are checked (see check_guest_pool_limit)
>> +#
>> +# return value indicates whether any limit was overstepped or not (if 
>> $noerr is set)
>> +sub check_pool_limits {
>> +my ($usage, $limits, $noerr, $changes) = @_;
>> +
>> +my $over = {};
>> +my $only_changed = defined($changes);
>> +
>> +my $check_limit = sub {
>> +my ($key, $running, $limit, $change) = @_;
>> +
>> +return if $only_changed && $change == 0;
>> +
>> +my $kind = $running ? 'run' : 'config';
>> +
>> +my $value = $usage->{$key}->{$kind};
>> +$value = int($value);
>> +$value += $change;
>> +$value = $value / (1024*1024) if $key eq 'mem';
>> +if ($limit < $value) {
>> +$over->{$key}->{$kind}->{change} = $change if $change;
>> +$over->{$key}->{$kind}->{over} = 1;
>> +}
>> +};
>> +
>> +my $get_change = sub {
>> +my ($key, $running) = @_;
>> +
>> +return 0 if !defined($changes);
>> +
>> +my $check_running = defined($changes->{running}) &&

Re: [pve-devel] [PATCH container 7/7] update: handle pool limits

2024-04-15 Thread Fabian Grünbichler
On April 11, 2024 12:03 pm, Wolfgang Bumiller wrote:
> On Thu, Apr 11, 2024 at 09:23:53AM +0200, Fabian Grünbichler wrote:
>> On April 10, 2024 3:13 pm, Fabian Grünbichler wrote:
>> > Signed-off-by: Fabian Grünbichler 
>> > ---
>> >  src/PVE/API2/LXC/Config.pm | 21 +
>> >  1 file changed, 21 insertions(+)
>> > 
>> > diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
>> > index e6c0980..3fb3885 100644
>> > --- a/src/PVE/API2/LXC/Config.pm
>> > +++ b/src/PVE/API2/LXC/Config.pm
>> > @@ -208,6 +208,27 @@ __PACKAGE__->register_method({
>> >  
>> >my $running = PVE::LXC::check_running($vmid);
>> >  
>> > +  my $usage = PVE::LXC::Config->get_pool_usage($conf);
>> > +  if (defined($param->{memory}) || defined($param->{swap})) {
>> > +  my $old = $usage->{mem};
>> > +  my $new = $param->{memory} || $usage->{memory};
>> > +  $new *= ($param->{swap} || $usage->{swap});
>> 
>> as Dominik pointed out off-list, this should be an addition, not a
>> multiplication..
> 
> Do we even want to mix mem & swap? Feels cgroupv1-y... (as in bad)

well, we want a single value (because both VMs and CTs count against the
pool limit, so counting swap separately doesn't make much sense..). I
guess we could either ignore swap altogether (assuming v2), or
conditionalize based on current cgroup mode?


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


Re: [pve-devel] [PATCH container 2/7] status: add pool usage fields

2024-04-15 Thread Fabian Grünbichler
On April 11, 2024 11:28 am, Wolfgang Bumiller wrote:
> On Wed, Apr 10, 2024 at 03:13:00PM +0200, Fabian Grünbichler wrote:
>> these are similar to existing ones, but with slightly different semantics.
>> 
>> Signed-off-by: Fabian Grünbichler 
>> ---
>>  src/PVE/LXC.pm | 29 +
>>  1 file changed, 29 insertions(+)
>> 
>> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
>> index 88a9d6f..78c0e18 100644
>> --- a/src/PVE/LXC.pm
>> +++ b/src/PVE/LXC.pm
>> @@ -138,6 +138,18 @@ our $vmstatus_return_properties = {
>>  optional => 1,
>>  renderer => 'bytes',
>>  },
>> +confmem => {
> 
> Would it make sense, for easier reuse of code, to stick to the same
> naming as in user.cfg - `foo-config`, `foo-run` here?

then it would be inconsistent with the rest of the schema here (which is
directly exported into metrics, so we can't change it easily without
breaking a ton of stuff..).

>> +description => "Configured amount of memory (inc. swap), might be 
>> higher than 'maxmem'.",
>> +type => 'integer',
>> +optional => 1,
>> +renderer => 'bytes',
>> +},
>> +runmem => {
>> +description => "Currently configured amount of memory (inc. swap).",
>> +type => 'integer',
>> +optional => 1,
>> +renderer => 'bytes',
>> +},
>>  maxdisk => {
>>  description => "Root disk size in bytes.",
>>  type => 'integer',
>> @@ -160,6 +172,16 @@ our $vmstatus_return_properties = {
>>  type => 'number',
>>  optional => 1,
>>  },
>> +confcpus => {
>> +description => "Configured amount of CPUs, might be higher than 
>> 'cpus'.",
>> +type => 'integer',
>> +optional => 1,
>> +},
>> +runcpus => {
>> +description => "Currently used amount of CPUs.",
>> +type => 'integer',
>> +optional => 1,
>> +},
>>  lock => {
>>  description => "The current config lock, if any.",
>>  type => 'string',
>> @@ -200,6 +222,7 @@ sub vmstatus {
>>  my $conf = PVE::Cluster::cfs_read_file($cfspath) || {};
>>  
>>  $unprivileged->{$vmid} = $conf->{unprivileged};
>> +my $usage = PVE::LXC::Config->get_pool_usage($conf);
>>  
>>  $d->{name} = $conf->{'hostname'} || "CT$vmid";
>>  $d->{name} =~ s/[\s]//g;
>> @@ -207,6 +230,9 @@ sub vmstatus {
>>  $d->{cpus} = $conf->{cores} || $conf->{cpulimit};
>>  $d->{cpus} = $cpucount if !$d->{cpus};
>>  
>> +$d->{confcpus} = $usage->{cpu};
>> +$d->{runcpus} = $conf->{cores} || $cpucount;
>> +
>>  $d->{tags} = $conf->{tags} if defined($conf->{tags});
>>  
>>  if ($d->{pid}) {
>> @@ -229,6 +255,9 @@ sub vmstatus {
>>  $d->{maxmem} = ($conf->{memory}||512)*1024*1024;
>>  $d->{maxswap} = ($conf->{swap}//0)*1024*1024;
>>  
>> +$d->{confmem} = $usage->{mem};
>> +$d->{runmem} = $conf->{maxmem} + $conf->{swap};
>> +
>>  $d->{uptime} = 0;
>>  $d->{cpu} = 0;
>>  
>> -- 
>> 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 manager 1/2] ui: dc: backup: improve UX for the different 'notification-mode's

2024-04-15 Thread Lukas Wagner
On  2024-04-11 09:44, Thomas Lamprecht wrote:
>> Tested both commits, they do what they promise.
>>
>>> +   let notificationSystemHint = Ext.create({
>>> +   xtype: 'displayfield',
>>> +   padding: '0 0 0 5',
>>> +   userCls: 'pmx-hint',
>>> +   hidden: true,
>>> +   value: gettext('No email configured, the notification system will 
>>> be used'),
>>> +   viewModel,
>>> +   bind: {
>>> +   hidden: '{!hintTextVisible}',
>>
>> I think the `pmx-hint` being displayed as yellow suggests it is a
>> warning/error rather than a hint. I wonder if there is a better
>> approach, as this is certainly not a warning.
> 
> Yeah, the hint is even often used as warning, having something more like a
> "notice" or well, actual "hint" level, that isn't as flashy, might be a
> good idea in the long run..
> 
> For now, we could also set a 'Hint' label and then add the pmx-hint class only
> to the labelClsExtra config?
> 
> But I'm fine with this as is if you, Lukas, think that it's warranted (and
> maybe even not showed that often in practice anyway).

Actually this is visible when creating new backup jobs, because 'Notification 
mode' is 
set to 'Default (auto)' and there is no email entered. So yeah, a more gentle 
hint would be better. That being said, I'd like to get this merged ASAP to
resolve pontential for confusion for our users[1].

I would suggest to merge the variant with 'pmx-hint' (I'll send a v2 though, 
because
I fixed a minor issue just now when reevaluating this) and then revisit
this soonish to introduce a more hinty, less warning CSS class. There are other 
places where
that one could be useful (e.g. second page of the same dialog - Retention, or 
Rentention
settings for storage plugins), so we could fix them all in one go in a followup.


[1] 
https://forum.proxmox.com/threads/pve-sending-email-notifications-on-successful-backup-jobs.136768/post-635769
-- 
- Lukas


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



[pve-devel] applied: [PATCH pve-kernel] fix #5373: cherry-pick USB ethernet naming fix

2024-04-15 Thread Thomas Lamprecht
Am 12/04/2024 um 15:25 schrieb Fabian Grünbichler:
> Signed-off-by: Fabian Grünbichler 
> ---
> test-built 6.8, but I assume 6.5 works as well since the patch applies cleanly
> there (build hasn't finished yet ;))
> 
> I also assume this will be picked up fairly fast by stable point releases, but
> not sure how fast those will be folded atm on the Ubuntu side with the freeze
> going on ;)
> 
>  ...178a-avoid-the-interface-always-conf.patch | 50 +++
>  1 file changed, 50 insertions(+)
>  create mode 100644 
> patches/kernel/0013-net-usb-ax88179_178a-avoid-the-interface-always-conf.patch
> 
>

applied, thanks!


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


[pve-devel] [PATCH manager v10 2/2] ui: machine: add link to documentation of the system settings

2024-04-15 Thread Markus Frank
---
 www/manager6/qemu/MachineEdit.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/qemu/MachineEdit.js b/www/manager6/qemu/MachineEdit.js
index 48c72c1d..ee2b2dac 100644
--- a/www/manager6/qemu/MachineEdit.js
+++ b/www/manager6/qemu/MachineEdit.js
@@ -1,6 +1,7 @@
 Ext.define('PVE.qemu.MachineInputPanel', {
 extend: 'Proxmox.panel.InputPanel',
 xtype: 'pveMachineInputPanel',
+onlineHelp: 'qm_system_settings',
 
 viewModel: {
data: {
-- 
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 v10 1/2] ui: machine: add viommu ComboBox

2024-04-15 Thread Markus Frank
Added a proxmoxKVComboBox for selecting a vIOMMU implementation for a VM.
If i440fx is selected, another ComboBox will be enabled/visible that does not
have the Intel option, as Intel-vIOMMU is not compatible with i440fx.

Uses the new machine property-string from the qemu-server's "config: define
machine schema as property-string" commit and the viommu option added in the
qemu-server's "fix #3784: config: Parameter for guest vIOMMU + test-cases"
commit.

Signed-off-by: Markus Frank 
---
 www/manager6/qemu/MachineEdit.js | 62 +++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/www/manager6/qemu/MachineEdit.js b/www/manager6/qemu/MachineEdit.js
index f928c80c..48c72c1d 100644
--- a/www/manager6/qemu/MachineEdit.js
+++ b/www/manager6/qemu/MachineEdit.js
@@ -2,6 +2,15 @@ Ext.define('PVE.qemu.MachineInputPanel', {
 extend: 'Proxmox.panel.InputPanel',
 xtype: 'pveMachineInputPanel',
 
+viewModel: {
+   data: {
+   type: '__default__',
+   },
+   formulas: {
+   q35: get => get('type') === 'q35',
+   },
+},
+
 controller: {
xclass: 'Ext.app.ViewController',
control: {
@@ -35,17 +44,29 @@ Ext.define('PVE.qemu.MachineInputPanel', {
 },
 
 onGetValues: function(values) {
+   if (values.delete === 'machine' && values.viommu) {
+   delete values.delete;
+   values.machine = 'pc';
+   }
if (values.version && values.version !== 'latest') {
values.machine = values.version;
delete values.delete;
}
delete values.version;
-   return values;
+   if (values.delete === 'machine' && !values.viommu) {
+   return values;
+   }
+   let ret = {};
+   ret.machine = PVE.Parser.printPropertyString(values, 'machine');
+   return ret;
 },
 
 setValues: function(values) {
let me = this;
 
+   let machineConf = PVE.Parser.parsePropertyString(values.machine, 
'type');
+   values.machine = machineConf.type;
+
me.isWindows = values.isWindows;
if (values.machine === 'pc') {
values.machine = '__default__';
@@ -58,6 +79,9 @@ Ext.define('PVE.qemu.MachineInputPanel', {
values.version = 'pc-q35-5.1';
}
}
+
+   values.viommu = machineConf.viommu || '__default__';
+
if (values.machine !== '__default__' && values.machine !== 'q35') {
values.version = values.machine;
values.machine = values.version.match(/q35/) ? 'q35' : 
'__default__';
@@ -78,6 +102,9 @@ Ext.define('PVE.qemu.MachineInputPanel', {
['__default__', PVE.Utils.render_qemu_machine('')],
['q35', 'q35'],
],
+   bind: {
+   value: '{type}',
+   },
 },
 
 advancedItems: [
@@ -113,6 +140,39 @@ Ext.define('PVE.qemu.MachineInputPanel', {
fieldLabel: gettext('Note'),
value: gettext('Machine version change may affect hardware layout 
and settings in the guest OS.'),
},
+   {
+   xtype: 'proxmoxKVComboBox',
+   name: 'viommu',
+   fieldLabel: gettext('vIOMMU'),
+   reference: 'viommu-q35',
+   deleteEmpty: false,
+   value: '__default__',
+   comboItems: [
+   ['__default__', Proxmox.Utils.defaultText + ' (None)'],
+   ['intel', 'Intel'],
+   ['virtio', 'VirtIO'],
+   ],
+   bind: {
+   hidden: '{!q35}',
+   disabled: '{!q35}',
+   },
+   },
+   {
+   xtype: 'proxmoxKVComboBox',
+   name: 'viommu',
+   fieldLabel: gettext('vIOMMU'),
+   reference: 'viommu-i440fx',
+   deleteEmpty: false,
+   value: '__default__',
+   comboItems: [
+   ['__default__', Proxmox.Utils.defaultText + ' (None)'],
+   ['virtio', 'VirtIO'],
+   ],
+   bind: {
+   hidden: '{q35}',
+   disabled: '{q35}',
+   },
+   },
 ],
 });
 
-- 
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-kernel] fix #5373: cherry-pick USB ethernet naming fix

2024-04-15 Thread Fabian Grünbichler
On April 12, 2024 3:25 pm, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler 
> ---
> test-built 6.8, but I assume 6.5 works as well since the patch applies cleanly
> there (build hasn't finished yet ;))

FWIW, the 6.5 build worked as well


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


[pve-devel] [PATCH widget-toolkit v5 11/16] notification: matcher: move match-field formulas to local viewModel

2024-04-15 Thread Lukas Wagner
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 | 186 +-
 1 file changed, 92 insertions(+), 94 deletions(-)

diff --git a/src/window/NotificationMatcherEdit.js 
b/src/window/NotificationMatcherEdit.js
index 34ed573..ded9db5 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,95 @@ 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 widget-toolkit v5 12/16] notification: matcher: move match-calendar fields to panel

2024-04-15 Thread Lukas Wagner
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 ded9db5..75eee42 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 v5 08/16] api: notification: add API for getting known metadata fields/values

2024-04-15 Thread Lukas Wagner
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 | 152 ++
 1 file changed, 152 insertions(+)

diff --git a/PVE/API2/Cluster/Notifications.pm 
b/PVE/API2/Cluster/Notifications.pm
index 68fdda2a..16548bec 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -79,12 +79,164 @@ __PACKAGE__->register_method ({
{ name => 'endpoints' },
{ name => 'matchers' },
{ name => 'targets' },
+   { name => 'fields' },
+   { name => 'values' },
];
 
return $result;
 }
 });
 
+__PACKAGE__->register_method ({
+name => 'get_fields',
+path => 'fields',
+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,
+   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 => 'backup-job' },
+   { name => 'replication-job' },
+   ];
+
+   return $result;
+}
+});
+
+__PACKAGE__->register_method ({
+name => 'get_field_values',
+path => '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',
+   optional => 1,
+   },
+   'internal' => {
+   description => 'Set if "value" was generated by the system 
and can'
+  . ' safely be used as base for translations.',
+   type => 'boolean',
+   optional => 1,
+   },
+   },
+   },
+},
+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',
+   internal => 1,
+   field => 'type',
+   },
+   {
+   value => 'fencing',
+   internal => 1,
+   field => 'type',
+   },
+   {
+   value => 'replication',
+   internal => 1,
+   field => 'type',
+   },
+   {
+   value => 'vzdump',
+   internal => 1,
+   field => 'type',
+   },
+   {
+   value => 'system-mail',
+   internal => 1,
+   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 => 'backup-job'
+   };
+   }
+   }
+   # 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},
+

[pve-devel] [PATCH docs v5 15/16] notifications: describe new notification metadata fields

2024-04-15 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 notifications.adoc | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/notifications.adoc b/notifications.adoc
index 57053c8..0eeed6a 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -289,19 +289,22 @@ 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`, `replication-job`
+| Backup succeeded |`vzdump`   | `info`   | 
`hostname`, `backup-job` (only for backup jobs)
+| Backup failed|`vzdump`   | `error`  | 
`hostname`, `backup-job` (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`)
+| `backup-job`  | Backup job ID
+| `replication-job` | Replication 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 manager v5 09/16] ui: utils: add overrides for translatable notification fields/values

2024-04-15 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 www/manager6/Utils.js | 12 
 1 file changed, 12 insertions(+)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index d4b5f3e6..c46ec4df 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -2052,6 +2052,18 @@ Ext.define('PVE.Utils', {
zfscreate: [gettext('ZFS Storage'), gettext('Create')],
zfsremove: ['ZFS Pool', gettext('Remove')],
});
+
+   Proxmox.Utils.overrideNotificationFieldName({
+   'backup-job': gettext('Backup job ID'),
+   'replication-job': gettext('Replication 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 v5 06/16] vzdump: apt: notification: do not include domain in 'hostname' field

2024-04-15 Thread Lukas Wagner
 - 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 54121ec2..71c83581 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -354,7 +354,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 88f42962..c24ff38e 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -487,10 +487,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,
@@ -501,7 +500,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->{'backup-job'} = $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 docs v5 14/16] notification: clarify that 'hostname' does not include the domain

2024-04-15 Thread Lukas Wagner
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 widget-toolkit v5 10/16] notification: matcher: match-field: show known fields/values

2024-04-15 Thread Lukas Wagner
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 | 300 +-
 2 files changed, 256 insertions(+), 56 deletions(-)

diff --git a/src/data/model/NotificationConfig.js 
b/src/data/model/NotificationConfig.js
index e8ebf28..a2c365b 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', 'internal', 'field'],
+idProperty: 'value',
+});
diff --git a/src/window/NotificationMatcherEdit.js 
b/src/window/NotificationMatcherEdit.js
index e717ad7..34ed573 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.Notifi

[pve-devel] [PATCH widget-toolkit v5 13/16] notification: matcher: move match-severity fields to panel

2024-04-15 Thread Lukas Wagner
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 75eee42..874140b 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 docs v5 16/16] notifications: match-field 'exact'-mode can now match multiple values

2024-04-15 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 notifications.adoc | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/notifications.adoc b/notifications.adoc
index 0eeed6a..912b048 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 v5 07/16] api: replication: include 'hostname' field for notifications

2024-04-15 Thread Lukas Wagner
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 703640f5..192b8af3 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -142,6 +142,8 @@ my sub _handle_job_err {
 my $metadata_fields = {
type => "replication",
"replication-job" => $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 v5 03/16] ui: dc: backup: send 'id' property when starting a backup job manually

2024-04-15 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 www/manager6/dc/Backup.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 70903bdc..4beb84c0 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -569,7 +569,6 @@ Ext.define('PVE.dc.BackupView', {
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



[pve-devel] [PATCH manager v5 02/16] api: jobs: vzdump: pass job 'id' parameter

2024-04-15 Thread Lukas Wagner
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 f66fc740..6bc0b792 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -52,6 +52,14 @@ __PACKAGE__->register_method ({
 parameters => {
additionalProperties => 0,
properties => PVE::VZDump::Common::json_config_properties({
+   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 => 64,
+   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..2f7c72ab 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, $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->{id} = $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 152eb3e5..88f42962 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -453,6 +453,7 @@ sub send_notification {
 my ($self, $tasklist, $total_time, $err, $detail_pre, $detail_post) = @_;
 
 my $opts = $self->{opts};
+my $job_id = $opts->{id};
 my $mailto = $opts->{mailto};
 my $cmdline = $self->{cmdline};
 my $policy = $opts->{mailnotification} // 'always';
@@ -499,12 +500,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->{'backup-job'} = $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 manager v5 01/16] api: notifications: add 'smtp' to target index

2024-04-15 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 PVE/API2/Cluster/Notifications.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Cluster/Notifications.pm 
b/PVE/API2/Cluster/Notifications.pm
index 7047f0b1..68fdda2a 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -107,6 +107,7 @@ __PACKAGE__->register_method ({
my $result = [
{ name => 'gotify' },
{ name => 'sendmail' },
+   { name => 'smtp' },
];
 
return $result;
@@ -143,7 +144,7 @@ __PACKAGE__->register_method ({
'type' => {
description => 'Type of the target.',
type  => 'string',
-   enum => [qw(sendmail gotify)],
+   enum => [qw(sendmail gotify smtp)],
},
'comment' => {
description => 'Comment',
-- 
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 v5 05/16] api: replication: add 'replication-job' to notification metadata

2024-04-15 Thread Lukas Wagner
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 0dc944c9..703640f5 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -140,8 +140,8 @@ my sub _handle_job_err {
 };
 
 my $metadata_fields = {
-   # TODO: Add job-id?
type => "replication",
+   "replication-job" => $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 v5 04/16] ui: dc: backup: allow to set custom job id in advanced settings

2024-04-15 Thread Lukas Wagner
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 |  4 
 www/manager6/dc/Backup.js | 11 +++
 2 files changed, 15 insertions(+)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 287d651a..d4b5f3e6 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1952,6 +1952,10 @@ Ext.define('PVE.Utils', {
 singleton: true,
 constructor: function() {
var me = this;
+
+   // Same regex as 'pve-configid
+   me.CONFIGID_RE = /^[A-Za-z][A-Za-z0-9_-]+$/;
+
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 4beb84c0..5b6f6688 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -397,6 +397,17 @@ Ext.define('PVE.dc.BackupEdit', {
},
],
advancedColumn1: [
+   {
+   xtype: 'pmxDisplayEditField',
+   fieldLabel: gettext('Job ID'),
+   emptyText: gettext('Autogenerate'),
+   name: 'id',
+   allowBlank: true,
+   regex: PVE.Utils.CONFIGID_RE,
+   cbind: {
+   editable: '{isCreate}',
+   },
+   },
{
xtype: 'proxmoxcheckbox',
fieldLabel: gettext('Repeat missed'),
-- 
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/manager/widget-toolkit v5 00/16] notifications: notification metadata matching improvements

2024-04-15 Thread Lukas Wagner
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:
- backup-job: ID of the backup job (set for backups, unless they are 
'ad-hoc' backups)
- replication-job: ID of the replication job
  - 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)

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)
  - widget-toolkit relies on a new API endpoint provided by pve-manager
  - the changes in the backend for matching multiple values in "exact"
mode have already been rolled out as of libpve-rs-perl 0.8.8
--> this means that libpve-notify-perl (which pulls in libpve-rs-perl)
must depend on libpve-rs-perl 0.8.8 at minimum - otherwise
the notification stack cannot understand the comma-separated
array of matched values

Changelog:
  - 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-manager:

Lukas Wagner (9):
  api: notifications: add 'smtp' to target index
  api: jobs: vzdump: pass job 'id' parameter
  ui: dc: backup: send 'id' property when starting a backup job manually
  ui: dc: backup: allow to set custom job id in  advanced settings
  api: replication: add 'replication-job' 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

 PVE/API2/APT.pm   |   3 +-
 PVE/API2/Cluster/Notifications.pm | 155 +-
 PVE/API2/Replication.pm   |   4 +-
 PVE/API2/VZDump.pm|   8 ++
 PVE/Jobs/VZDump.pm|   4 +-
 PVE/VZDump.pm |  14 +--
 www/manager6/Utils.js |  16 +++
 www/manager6/dc/Backup.js |  12 ++-
 8 files changed, 204 insertions(+), 12 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-severity fields to panel

 src/data/model/NotificationConfig.js  |  12 +
 src/window/NotificationMatcherEdit.js | 613 ++
 2 files changed, 441 insertions(+), 184 deletions(-)


pve-docs:

Lukas Wagner (3):
  notification: clarify that 'hostname' does not include the domain
  notifications: describe new notification metadata fields
  notifications: match-field 'exact'-mode can now match multiple values

 notifications.adoc | 39 ++-
 1 file changed, 18 insertions(+), 21 deletions(-)


Summary over all repositories:
  11 files changed, 663 insertions(+), 217 deletions(-)

-- 
Generated by git-murpp 0.7.1


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