* [pve-devel] [PATCH installer v2 1/3] tui: NumericEditView: add optional placeholder value
2024-02-07 14:28 [pve-devel] [PATCH installer v2 0/3] expose zfs arc size setting for all products Christoph Heiss
@ 2024-02-07 14:28 ` Christoph Heiss
2024-02-07 14:28 ` [pve-devel] [PATCH installer v2 2/3] tui: expose arc size setting for zfs bootdisks for all products Christoph Heiss
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Christoph Heiss @ 2024-02-07 14:28 UTC (permalink / raw)
To: pve-devel
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 <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* new patch
proxmox-tui-installer/src/views/mod.rs | 70 ++++++++++++++++++++++++--
1 file changed, 66 insertions(+), 4 deletions(-)
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 3244e76..299517e 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<T> {
view: LinearLayout,
max_value: Option<T>,
+ placeholder: Option<T>,
max_content_width: Option<usize>,
allow_empty: bool,
}
@@ -36,6 +38,7 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
Self {
view,
max_value: None,
+ placeholder: None,
max_content_width: None,
allow_empty: false,
}
@@ -54,6 +57,7 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
Self {
view,
max_value: None,
+ placeholder: None,
max_content_width: None,
allow_empty: false,
}
@@ -84,15 +88,42 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
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<T, <T as FromStr>::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<Result<T, <T as FromStr>::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,29 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
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() {
+ 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),
+ );
+
+ if printer.focused {
+ printer.print((placeholder.len(), 0), " ");
+ }
+ }
+ }
+ }
}
pub type FloatEditView = NumericEditView<f64>;
@@ -165,6 +219,10 @@ pub type IntegerEditView = NumericEditView<usize>;
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 +262,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.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [pve-devel] [PATCH installer v2 2/3] tui: expose arc size setting for zfs bootdisks for all products
2024-02-07 14:28 [pve-devel] [PATCH installer v2 0/3] expose zfs arc size setting for all products Christoph Heiss
2024-02-07 14:28 ` [pve-devel] [PATCH installer v2 1/3] tui: NumericEditView: add optional placeholder value Christoph Heiss
@ 2024-02-07 14:28 ` Christoph Heiss
2024-02-07 14:28 ` [pve-devel] [PATCH installer v2 3/3] proxinstall: " Christoph Heiss
2024-04-15 13:21 ` [pve-devel] [PATCH installer v2 0/3] expose zfs arc size setting " Christoph Heiss
3 siblings, 0 replies; 7+ messages in thread
From: Christoph Heiss @ 2024-02-07 14:28 UTC (permalink / raw)
To: pve-devel
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 <c.heiss@proxmox.com>
---
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 | 48 +++++++++++++--------
proxmox-tui-installer/src/views/mod.rs | 1 -
3 files changed, 32 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..b856342 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,25 @@ impl ZfsBootdiskOptionsView {
fn get_values(&mut self) -> Option<(Vec<Disk>, 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::<IntegerEditView, _>(0)?;
let compress = view.get_value::<SelectView<_>, _>(1)?;
let checksum = view.get_value::<SelectView<_>, _>(2)?;
let copies = view.get_value::<IntegerEditView, _>(3)?;
- let disk_size = view.get_value::<DiskSizeEditView, _>(disk_size_index)?;
-
- let arc_max = if has_arc_max {
- view.get_value::<IntegerEditView, _>(4)?
- .max(ZFS_ARC_MIN_SIZE_MIB)
- } else {
- 0 // use built-in ZFS default value
- };
+ let disk_size = view.get_value::<DiskSizeEditView, _>(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.
+ //
+ // TODO: Return an [`Result`] from here as well and propagate the [`Result::Err`] value
+ // instead of defaulting to `0`.
+ let arc_max = view
+ .get_child::<IntegerEditView>(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 299517e..9e27c5f 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -93,7 +93,6 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
///
/// # 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)
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [pve-devel] [PATCH installer v2 3/3] proxinstall: expose arc size setting for zfs bootdisks for all products
2024-02-07 14:28 [pve-devel] [PATCH installer v2 0/3] expose zfs arc size setting for all products Christoph Heiss
2024-02-07 14:28 ` [pve-devel] [PATCH installer v2 1/3] tui: NumericEditView: add optional placeholder value Christoph Heiss
2024-02-07 14:28 ` [pve-devel] [PATCH installer v2 2/3] tui: expose arc size setting for zfs bootdisks for all products Christoph Heiss
@ 2024-02-07 14:28 ` Christoph Heiss
2024-02-23 15:37 ` Maximiliano Sandoval
2024-04-15 13:21 ` [pve-devel] [PATCH installer v2 0/3] expose zfs arc size setting " Christoph Heiss
3 siblings, 1 reply; 7+ messages in thread
From: Christoph Heiss @ 2024-02-07 14:28 UTC (permalink / raw)
To: pve-devel
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 <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* add some more explanatory comments
* shuffle around code to support placeholder-like value
Proxmox/Install/RunEnv.pm | 3 ++-
proxinstall | 37 +++++++++++++++++++++++++------------
2 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index c393f67..2eb26f4 100644
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -316,7 +316,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 81dd368..ab89a08 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1167,21 +1167,34 @@ 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 $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');
+ my $total_memory = Proxmox::Install::RunEnv::get('total_memory');
+
+ 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');
+ my $arc_max = Proxmox::Install::Config::get_zfs_opt('arc_max');
+
+ # GTKs SpinButton does not support a placeholder value, unfortunaly. That means we also
+ # have to set the value normally for non-PVE products, where the ZFS default should be used,
+ # in case the user does not explicitly set a value.
+ # But due to the signal-based nature of GTK, as long as the user never touches the spinbutton,
+ # `Proxmox::Install::Config::set_zfs_opt('arc_max', ..)` never gets called, thus the default
+ # value (which is 0 for non-PVE) won't get overwritten and no modprobe file is written.
+ if ($arc_max > 0) {
$spinbutton_arc_max->set_value($arc_max);
- push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB'];
+ } else {
+ # .. but we need to display the "real" value to the user
+ $spinbutton_arc_max->set_value($total_memory * 0.5);
}
+ # We need to connect the signal afterwards, to avoid triggering it using ->set_value() above.
+ $spinbutton_arc_max->signal_connect('value-changed' => sub {
+ my $w = shift;
+ Proxmox::Install::Config::set_zfs_opt('arc_max', $w->get_value_as_int());
+ });
+
+ 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.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pve-devel] [PATCH installer v2 3/3] proxinstall: expose arc size setting for zfs bootdisks for all products
2024-02-07 14:28 ` [pve-devel] [PATCH installer v2 3/3] proxinstall: " Christoph Heiss
@ 2024-02-23 15:37 ` Maximiliano Sandoval
2024-04-12 10:26 ` Christoph Heiss
0 siblings, 1 reply; 7+ messages in thread
From: Maximiliano Sandoval @ 2024-02-23 15:37 UTC (permalink / raw)
To: Proxmox VE development discussion
Some comments bellow.
Christoph Heiss <c.heiss@proxmox.com> writes:
> 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 <c.heiss@proxmox.com>
> ---
> ...
> ...
> +
> + # GTKs SpinButton does not support a placeholder value, unfortunaly. That means we also
There is a typo on unfortunately. What exactly do you mean by a
placeholder value? Like the initial value? If so see comment bellow.
> + # have to set the value normally for non-PVE products, where the ZFS default should be used,
> + # in case the user does not explicitly set a value.
> + # But due to the signal-based nature of GTK, as long as the user never touches the spinbutton,
> + # `Proxmox::Install::Config::set_zfs_opt('arc_max', ..)` never gets called, thus the default
> + # value (which is 0 for non-PVE) won't get overwritten and no modprobe file is written.
> + if ($arc_max > 0) {
> $spinbutton_arc_max->set_value($arc_max);
> - push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB'];
> + } else {
> + # .. but we need to display the "real" value to the user
> + $spinbutton_arc_max->set_value($total_memory * 0.5);
> }
>
> + # We need to connect the signal afterwards, to avoid triggering it using ->set_value() above.
>
Alternatively one could init the spin button with the correct values
e.g.
my $total_memory = Proxmox::Install::RunEnv::get('total_memory');
my $arc_max = Proxmox::Install::Config::get_zfs_opt('arc_max');
my $arc_max_value = $total_memory * 0.5;
$arc_max_value = $arc_max if $arc_max > 0;
my $arc_max_adjusment = Gtk3::Adjustment->new($arc_max_value, $Proxmox::Install::RunEnv::ZFS_ARC_MIN_SIZE_MIB, $total_memory, 1, 10, 0);
my $spinbutton_arc_max = Gtk3::SpinButton->new($arc_max_adjusment, 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());
});
Here the numerical values 1, 10, 0, and 1, 0 come from a quick
inspection of the source for gtk_spin_button_new_with_range and
gtk_spin_button_new (in the docs [1, 2] there is a [src] link next to
the Description header).
[1] https://docs.gtk.org/gtk3/ctor.SpinButton.new.html
[2] https://docs.gtk.org/gtk3/ctor.SpinButton.new_with_range.html
> + $spinbutton_arc_max->signal_connect('value-changed' => sub {
> + my $w = shift;
> + Proxmox::Install::Config::set_zfs_opt('arc_max', $w->get_value_as_int());
> + });
> +
> + push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB'];
> +
> push @$labeled_widgets, ['hdsize', $hdsize_btn, 'GB'];
> return $create_label_widget_grid->($labeled_widgets);;
> };
--
Maximiliano
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pve-devel] [PATCH installer v2 3/3] proxinstall: expose arc size setting for zfs bootdisks for all products
2024-02-23 15:37 ` Maximiliano Sandoval
@ 2024-04-12 10:26 ` Christoph Heiss
0 siblings, 0 replies; 7+ messages in thread
From: Christoph Heiss @ 2024-04-12 10:26 UTC (permalink / raw)
To: Maximiliano Sandoval; +Cc: Proxmox VE development discussion
Thanks for the review!
On Fri, Feb 23, 2024 at 04:37:16PM +0100, Maximiliano Sandoval wrote:
>
> Some comments bellow.
>
> Christoph Heiss <c.heiss@proxmox.com> writes:
>
> > 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 <c.heiss@proxmox.com>
> > ---
> > ...
> > ...
> > +
> > + # GTKs SpinButton does not support a placeholder value, unfortunaly. That means we also
>
> There is a typo on unfortunately. What exactly do you mean by a
> placeholder value? Like the initial value? If so see comment bellow.
Placeholder as in the `Gtk.Entry:placeholder-text` [0]. Effectively a
separate value that is displayed to the user if the field is empty.
[0] https://docs.gtk.org/gtk3/property.Entry.placeholder-text.html
>
> > + # have to set the value normally for non-PVE products, where the ZFS default should be used,
> > + # in case the user does not explicitly set a value.
> > + # But due to the signal-based nature of GTK, as long as the user never touches the spinbutton,
> > + # `Proxmox::Install::Config::set_zfs_opt('arc_max', ..)` never gets called, thus the default
> > + # value (which is 0 for non-PVE) won't get overwritten and no modprobe file is written.
> > + if ($arc_max > 0) {
> > $spinbutton_arc_max->set_value($arc_max);
> > - push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB'];
> > + } else {
> > + # .. but we need to display the "real" value to the user
> > + $spinbutton_arc_max->set_value($total_memory * 0.5);
> > }
> >
> > + # We need to connect the signal afterwards, to avoid triggering it using ->set_value() above.
> >
>
> Alternatively one could init the spin button with the correct values
> e.g.
>
> my $total_memory = Proxmox::Install::RunEnv::get('total_memory');
> my $arc_max = Proxmox::Install::Config::get_zfs_opt('arc_max');
> my $arc_max_value = $total_memory * 0.5;
> $arc_max_value = $arc_max if $arc_max > 0;
> my $arc_max_adjusment = Gtk3::Adjustment->new($arc_max_value, $Proxmox::Install::RunEnv::ZFS_ARC_MIN_SIZE_MIB, $total_memory, 1, 10, 0);
> my $spinbutton_arc_max = Gtk3::SpinButton->new($arc_max_adjusment, 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());
> });
>
> Here the numerical values 1, 10, 0, and 1, 0 come from a quick
> inspection of the source for gtk_spin_button_new_with_range and
> gtk_spin_button_new (in the docs [1, 2] there is a [src] link next to
> the Description header).
Well, after testing, this does not really create the wanted behaviour
for PBS and PMG.
For these two products, we want to display the ZFS default value, used
when the `zfs_arc_max` module parameter is unset (aka. 50% of memory).
But the widget itself should return 0 (aka what
Proxmox::Install::RunEnv::default_zfs_arc_max() returns) in case the
user never changed it, such that we can skip writing the file (resulting
in the previous behaviour).
That's why the "workaround" was needed to support that.
>
> [1] https://docs.gtk.org/gtk3/ctor.SpinButton.new.html
> [2] https://docs.gtk.org/gtk3/ctor.SpinButton.new_with_range.html
>
> > + $spinbutton_arc_max->signal_connect('value-changed' => sub {
> > + my $w = shift;
> > + Proxmox::Install::Config::set_zfs_opt('arc_max', $w->get_value_as_int());
> > + });
> > +
> > + push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB'];
> > +
> > push @$labeled_widgets, ['hdsize', $hdsize_btn, 'GB'];
> > return $create_label_widget_grid->($labeled_widgets);;
> > };
>
>
> --
> Maximiliano
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH installer v2 0/3] expose zfs arc size setting for all products
2024-02-07 14:28 [pve-devel] [PATCH installer v2 0/3] expose zfs arc size setting for all products Christoph Heiss
` (2 preceding siblings ...)
2024-02-07 14:28 ` [pve-devel] [PATCH installer v2 3/3] proxinstall: " Christoph Heiss
@ 2024-04-15 13:21 ` Christoph Heiss
3 siblings, 0 replies; 7+ messages in thread
From: Christoph Heiss @ 2024-04-15 13:21 UTC (permalink / raw)
To: Proxmox VE development discussion
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
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread