* [pve-devel] [PATCH installer v2 1/5] fix #5250: install: config: add new `btrfs_opts` with `compress` config option
2024-08-13 16:15 [pve-devel] [PATCH installer v2 0/5] fix #5250: add btrfs `compress` mount option support Christoph Heiss
@ 2024-08-13 16:15 ` Christoph Heiss
2024-08-13 16:15 ` [pve-devel] [PATCH installer v2 2/5] fix #5250: install: write btrfs `compress` option to fstab Christoph Heiss
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Heiss @ 2024-08-13 16:15 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
Proxmox/Install/Config.pm | 15 ++++++++++
proxmox-auto-installer/src/utils.rs | 1 +
proxmox-installer-common/src/options.rs | 31 +++++++++++++++++++++
proxmox-installer-common/src/setup.rs | 21 ++++++++++++--
proxmox-tui-installer/src/setup.rs | 2 ++
proxmox-tui-installer/src/views/bootdisk.rs | 5 ++--
6 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/Proxmox/Install/Config.pm b/Proxmox/Install/Config.pm
index ae70093..7e67f69 100644
--- a/Proxmox/Install/Config.pm
+++ b/Proxmox/Install/Config.pm
@@ -79,6 +79,9 @@ my sub init_cfg {
copies => 1,
arc_max => Proxmox::Install::RunEnv::default_zfs_arc_max(), # in MiB
},
+ btrfs_opts => {
+ compress => 'off',
+ },
# TODO: single disk selection config
target_hd => undef,
disk_selection => {},
@@ -173,6 +176,18 @@ sub get_zfs_opt {
return defined($k) ? $zfs_opts->{$k} : $zfs_opts;
}
+sub set_btrfs_opt {
+ my ($k, $v) = @_;
+ my $opts = get('btrfs_opts');
+ croak "unknown btrfs opts key '$k'" if !exists($opts->{$k});
+ $opts->{$k} = $v;
+}
+sub get_btrfs_opt {
+ my ($k) = @_;
+ my $opts = get('btrfs_opts');
+ return defined($k) ? $opts->{$k} : $opts;
+}
+
sub set_target_hd { set_key('target_hd', $_[0]); }
sub get_target_hd { return get('target_hd'); }
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 45ad222..ae7dbbd 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -340,6 +340,7 @@ pub fn parse_answer(
minfree: None,
maxvz: None,
zfs_opts: None,
+ btrfs_opts: None,
target_hd: None,
disk_selection: BTreeMap::new(),
existing_storage_auto_rename: 1,
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 9375ded..d99e26d 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -102,10 +102,40 @@ impl LvmBootdiskOptions {
}
}
+/// See the accompanying mount option in btrfs(5).
+#[derive(Copy, Clone, Debug, Default, Deserialize, Eq, PartialEq)]
+#[serde(rename_all(deserialize = "lowercase"))]
+pub enum BtrfsCompressOption {
+ On,
+ #[default]
+ Off,
+ Zlib,
+ Lzo,
+ Zstd,
+}
+
+impl fmt::Display for BtrfsCompressOption {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ write!(f, "{}", format!("{self:?}").to_lowercase())
+ }
+}
+
+impl From<&BtrfsCompressOption> for String {
+ fn from(value: &BtrfsCompressOption) -> Self {
+ value.to_string()
+ }
+}
+
+pub const BTRFS_COMPRESS_OPTIONS: &[BtrfsCompressOption] = {
+ use BtrfsCompressOption::*;
+ &[On, Off, Zlib, Lzo, Zstd]
+};
+
#[derive(Clone, Debug)]
pub struct BtrfsBootdiskOptions {
pub disk_size: f64,
pub selected_disks: Vec<usize>,
+ pub compress: BtrfsCompressOption,
}
impl BtrfsBootdiskOptions {
@@ -115,6 +145,7 @@ impl BtrfsBootdiskOptions {
Self {
disk_size: disk.size,
selected_disks: (0..disks.len()).collect(),
+ compress: BtrfsCompressOption::default(),
}
}
}
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index e4e609b..d7f1c47 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -13,8 +13,8 @@ use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
use crate::{
options::{
- BtrfsRaidLevel, Disk, FsType, ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption,
- ZfsRaidLevel,
+ BtrfsBootdiskOptions, BtrfsCompressOption, BtrfsRaidLevel, Disk, FsType,
+ ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel,
},
utils::CidrAddress,
};
@@ -221,6 +221,20 @@ impl From<ZfsBootdiskOptions> for InstallZfsOption {
}
}
+#[derive(Debug, Deserialize, Serialize)]
+pub struct InstallBtrfsOption {
+ #[serde(serialize_with = "serialize_as_display")]
+ pub compress: BtrfsCompressOption,
+}
+
+impl From<BtrfsBootdiskOptions> for InstallBtrfsOption {
+ fn from(opts: BtrfsBootdiskOptions) -> Self {
+ InstallBtrfsOption {
+ compress: opts.compress,
+ }
+ }
+}
+
pub fn read_json<T: for<'de> Deserialize<'de>, P: AsRef<Path>>(path: P) -> Result<T, String> {
let file = File::open(path).map_err(|err| err.to_string())?;
let reader = BufReader::new(file);
@@ -475,6 +489,9 @@ pub struct InstallConfig {
#[serde(skip_serializing_if = "Option::is_none")]
pub zfs_opts: Option<InstallZfsOption>,
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub btrfs_opts: Option<InstallBtrfsOption>,
+
#[serde(
serialize_with = "serialize_disk_opt",
skip_serializing_if = "Option::is_none",
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index ee4e1c7..70b8e28 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -18,6 +18,7 @@ impl From<InstallerOptions> for InstallConfig {
minfree: None,
maxvz: None,
zfs_opts: None,
+ btrfs_opts: None,
target_hd: None,
disk_selection: BTreeMap::new(),
existing_storage_auto_rename: 0,
@@ -66,6 +67,7 @@ impl From<InstallerOptions> for InstallConfig {
}
AdvancedBootdiskOptions::Btrfs(btrfs) => {
config.hdsize = btrfs.disk_size;
+ config.btrfs_opts = Some(btrfs.clone().into());
for (i, disk) in options.bootdisk.disks.iter().enumerate() {
config
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 1e22105..8db33dd 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -19,8 +19,8 @@ use proxmox_installer_common::{
check_zfs_raid_config,
},
options::{
- AdvancedBootdiskOptions, BootdiskOptions, BtrfsBootdiskOptions, Disk, FsType,
- LvmBootdiskOptions, ZfsBootdiskOptions, ZFS_CHECKSUM_OPTIONS, ZFS_COMPRESS_OPTIONS,
+ AdvancedBootdiskOptions, BootdiskOptions, BtrfsBootdiskOptions, BtrfsCompressOption, Disk,
+ FsType, LvmBootdiskOptions, ZfsBootdiskOptions, ZFS_CHECKSUM_OPTIONS, ZFS_COMPRESS_OPTIONS,
},
setup::{BootType, ProductConfig, ProxmoxProduct, RuntimeInfo},
};
@@ -602,6 +602,7 @@ impl BtrfsBootdiskOptionsView {
BtrfsBootdiskOptions {
disk_size,
selected_disks,
+ compress: BtrfsCompressOption::default(),
},
))
}
--
2.45.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pve-devel] [PATCH installer v2 2/5] fix #5250: install: write btrfs `compress` option to fstab
2024-08-13 16:15 [pve-devel] [PATCH installer v2 0/5] fix #5250: add btrfs `compress` mount option support Christoph Heiss
2024-08-13 16:15 ` [pve-devel] [PATCH installer v2 1/5] fix #5250: install: config: add new `btrfs_opts` with `compress` config option Christoph Heiss
@ 2024-08-13 16:15 ` Christoph Heiss
2024-08-13 16:15 ` [pve-devel] [PATCH installer v2 3/5] fix #5250: proxinstall: expose new btrfs `compress` option Christoph Heiss
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Heiss @ 2024-08-13 16:15 UTC (permalink / raw)
To: pve-devel
`compress` instead of `compress-force` is used, as the latter can have
unindented (performance) implications, as the name implies. That would
be neither expected by users nor should such a decision made without the
user explicitly opting for it.
Others do the same, e.g. the installer for RedHat/Fedora systems (aka.
Anaconda) opts for `compress` too.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
* moved some mount setup code here from next patch
Proxmox/Install.pm | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index fa2702c..5c64c3d 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -1065,7 +1065,16 @@ sub extract_data {
die "unable to detect FS UUID" if !defined($fsuuid);
- $fstab .= "UUID=$fsuuid / btrfs defaults 0 1\n";
+ my $btrfs_opts = Proxmox::Install::Config::get_btrfs_opt();
+
+ my $mountopts = 'defaults';
+ if ($btrfs_opts->{compress} eq 'on') {
+ $mountopts .= ',compress';
+ } elsif ($btrfs_opts->{compress} ne 'off') {
+ $mountopts .= ",compress=$btrfs_opts->{compress}";
+ }
+
+ $fstab .= "UUID=$fsuuid / btrfs $mountopts 0 1\n";
} else {
my $root_mountopt = $fssetup->{$filesys}->{root_mountopt} || 'defaults';
$fstab .= "$rootdev / $filesys ${root_mountopt} 0 1\n";
--
2.45.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pve-devel] [PATCH installer v2 3/5] fix #5250: proxinstall: expose new btrfs `compress` option
2024-08-13 16:15 [pve-devel] [PATCH installer v2 0/5] fix #5250: add btrfs `compress` mount option support Christoph Heiss
2024-08-13 16:15 ` [pve-devel] [PATCH installer v2 1/5] fix #5250: install: config: add new `btrfs_opts` with `compress` config option Christoph Heiss
2024-08-13 16:15 ` [pve-devel] [PATCH installer v2 2/5] fix #5250: install: write btrfs `compress` option to fstab Christoph Heiss
@ 2024-08-13 16:15 ` Christoph Heiss
2024-08-13 16:15 ` [pve-devel] [PATCH installer v2 4/5] fix #5250: tui: " Christoph Heiss
2024-08-13 16:15 ` [pve-devel] [PATCH installer v2 5/5] fix #5250: auto-installer: " Christoph Heiss
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Heiss @ 2024-08-13 16:15 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
* moved some mount option setup code to previous patch
proxinstall | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/proxinstall b/proxinstall
index 12f3eaa..c7776f0 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1160,6 +1160,21 @@ my $create_raid_advanced_grid = sub {
my $create_btrfs_raid_advanced_grid = sub {
my ($hdsize_btn) = @_;
my $labeled_widgets = [];
+
+ my $combo_compress = Gtk3::ComboBoxText->new();
+ $combo_compress->set_tooltip_text("btrfs compression algorithm for boot volume");
+ my $comp_opts = ["on", "off", "zlib", "lzo", "zstd"];
+ foreach my $opt (@$comp_opts) {
+ $combo_compress->append($opt, $opt);
+ }
+ my $compress = Proxmox::Install::Config::get_btrfs_opt('compress') // 'off';
+ $combo_compress->set_active_id($compress);
+ $combo_compress->signal_connect (changed => sub {
+ my $w = shift;
+ Proxmox::Install::Config::set_btrfs_opt('compress', $w->get_active_text());
+ });
+ push @$labeled_widgets, ['compress', $combo_compress];
+
push @$labeled_widgets, ['hdsize', $hdsize_btn, 'GB'];
return $create_label_widget_grid->($labeled_widgets);;
};
--
2.45.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pve-devel] [PATCH installer v2 4/5] fix #5250: tui: expose new btrfs `compress` option
2024-08-13 16:15 [pve-devel] [PATCH installer v2 0/5] fix #5250: add btrfs `compress` mount option support Christoph Heiss
` (2 preceding siblings ...)
2024-08-13 16:15 ` [pve-devel] [PATCH installer v2 3/5] fix #5250: proxinstall: expose new btrfs `compress` option Christoph Heiss
@ 2024-08-13 16:15 ` Christoph Heiss
2024-08-13 16:15 ` [pve-devel] [PATCH installer v2 5/5] fix #5250: auto-installer: " Christoph Heiss
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Heiss @ 2024-08-13 16:15 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* rebased on master
proxmox-tui-installer/src/views/bootdisk.rs | 37 +++++++++++++--------
1 file changed, 24 insertions(+), 13 deletions(-)
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 8db33dd..625374e 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -19,8 +19,9 @@ use proxmox_installer_common::{
check_zfs_raid_config,
},
options::{
- AdvancedBootdiskOptions, BootdiskOptions, BtrfsBootdiskOptions, BtrfsCompressOption, Disk,
- FsType, LvmBootdiskOptions, ZfsBootdiskOptions, ZFS_CHECKSUM_OPTIONS, ZFS_COMPRESS_OPTIONS,
+ AdvancedBootdiskOptions, BootdiskOptions, BtrfsBootdiskOptions, Disk, FsType,
+ LvmBootdiskOptions, ZfsBootdiskOptions, BTRFS_COMPRESS_OPTIONS, ZFS_CHECKSUM_OPTIONS,
+ ZFS_COMPRESS_OPTIONS,
},
setup::{BootType, ProductConfig, ProxmoxProduct, RuntimeInfo},
};
@@ -573,12 +574,23 @@ struct BtrfsBootdiskOptionsView {
impl BtrfsBootdiskOptionsView {
fn new(runinfo: &RuntimeInfo, options: &BtrfsBootdiskOptions) -> Self {
- let view = MultiDiskOptionsView::new(
- &runinfo.disks,
- &options.selected_disks,
- FormView::new().child("hdsize", DiskSizeEditView::new().content(options.disk_size)),
- )
- .top_panel(TextView::new("Btrfs integration is a technology preview!").center());
+ let inner = FormView::new()
+ .child(
+ "compress",
+ SelectView::new()
+ .popup()
+ .with_all(BTRFS_COMPRESS_OPTIONS.iter().map(|o| (o.to_string(), *o)))
+ .selected(
+ BTRFS_COMPRESS_OPTIONS
+ .iter()
+ .position(|o| *o == options.compress)
+ .unwrap_or_default(),
+ ),
+ )
+ .child("hdsize", DiskSizeEditView::new().content(options.disk_size));
+
+ let view = MultiDiskOptionsView::new(&runinfo.disks, &options.selected_disks, inner)
+ .top_panel(TextView::new("Btrfs integration is a technology preview!").center());
Self { view }
}
@@ -592,17 +604,16 @@ impl BtrfsBootdiskOptionsView {
fn get_values(&mut self) -> Option<(Vec<Disk>, BtrfsBootdiskOptions)> {
let (disks, selected_disks) = self.view.get_disks_and_selection()?;
- let disk_size = self
- .view
- .get_options_view()?
- .get_value::<DiskSizeEditView, _>(0)?;
+ let view = self.view.get_options_view()?;
+ let compress = view.get_value::<SelectView<_>, _>(0)?;
+ let disk_size = view.get_value::<DiskSizeEditView, _>(1)?;
Some((
disks,
BtrfsBootdiskOptions {
disk_size,
selected_disks,
- compress: BtrfsCompressOption::default(),
+ compress,
},
))
}
--
2.45.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pve-devel] [PATCH installer v2 5/5] fix #5250: auto-installer: expose new btrfs `compress` option
2024-08-13 16:15 [pve-devel] [PATCH installer v2 0/5] fix #5250: add btrfs `compress` mount option support Christoph Heiss
` (3 preceding siblings ...)
2024-08-13 16:15 ` [pve-devel] [PATCH installer v2 4/5] fix #5250: tui: " Christoph Heiss
@ 2024-08-13 16:15 ` Christoph Heiss
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Heiss @ 2024-08-13 16:15 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* squashed in separate tests patch
proxmox-auto-installer/src/answer.rs | 6 ++++-
proxmox-auto-installer/src/utils.rs | 6 ++++-
.../tests/resources/parse_answer/btrfs.json | 24 +++++++++++++++++++
.../tests/resources/parse_answer/btrfs.toml | 17 +++++++++++++
4 files changed, 51 insertions(+), 2 deletions(-)
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/btrfs.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml
diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
index d691da1..fd0ed0d 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -1,6 +1,9 @@
use clap::ValueEnum;
use proxmox_installer_common::{
- options::{BtrfsRaidLevel, FsType, ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel},
+ options::{
+ BtrfsCompressOption, BtrfsRaidLevel, FsType, ZfsChecksumOption, ZfsCompressOption,
+ ZfsRaidLevel,
+ },
utils::{CidrAddress, Fqdn},
};
use serde::{Deserialize, Serialize};
@@ -270,6 +273,7 @@ pub struct LvmOptions {
pub struct BtrfsOptions {
pub hdsize: Option<f64>,
pub raid: Option<BtrfsRaidLevel>,
+ pub compress: Option<BtrfsCompressOption>,
}
#[derive(Clone, Deserialize, Serialize, Debug, PartialEq)]
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index ae7dbbd..9e86053 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -11,7 +11,8 @@ use crate::{
use proxmox_installer_common::{
options::{FsType, NetworkOptions, ZfsChecksumOption, ZfsCompressOption},
setup::{
- InstallConfig, InstallRootPassword, InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo,
+ InstallBtrfsOption, InstallConfig, InstallRootPassword, InstallZfsOption, LocaleInfo,
+ RuntimeInfo, SetupInfo,
},
};
use serde::{Deserialize, Serialize};
@@ -394,6 +395,9 @@ pub fn parse_answer(
config.hdsize = btrfs
.hdsize
.unwrap_or(runtime_info.disks[first_selected_disk].size);
+ config.btrfs_opts = Some(InstallBtrfsOption {
+ compress: btrfs.compress.unwrap_or_default(),
+ })
}
}
Ok(config)
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/btrfs.json b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.json
new file mode 100644
index 0000000..0330a38
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.json
@@ -0,0 +1,24 @@
+{
+ "autoreboot": 1,
+ "cidr": "192.168.1.114/24",
+ "country": "at",
+ "dns": "192.168.1.254",
+ "domain": "testinstall",
+ "disk_selection": {
+ "6": "6",
+ "7": "7"
+ },
+ "filesys": "btrfs (RAID1)",
+ "gateway": "192.168.1.1",
+ "hdsize": 80.0,
+ "existing_storage_auto_rename": 1,
+ "hostname": "pveauto",
+ "keymap": "de",
+ "mailto": "mail@no.invalid",
+ "mngmt_nic": "eno1",
+ "root_password": { "plain": "123456" },
+ "timezone": "Europe/Vienna",
+ "btrfs_opts": {
+ "compress": "zlib"
+ }
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml
new file mode 100644
index 0000000..8fcd27d
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml
@@ -0,0 +1,17 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root_password = "123456"
+
+[network]
+source = "from-dhcp"
+
+[disk-setup]
+filesystem = "btrfs"
+btrfs.raid = "raid1"
+btrfs.compress = "zlib"
+btrfs.hdsize = 80
+disk_list = ["sda", "sdb"]
--
2.45.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread