public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH 00/12] installer: add crate for common code
@ 2023-10-25 15:59 Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 01/12] add proxmox-installer-common crate Aaron Lauterer
                   ` (14 more replies)
  0 siblings, 15 replies; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 15:59 UTC (permalink / raw)
  To: pve-devel

since work on the auto installer is happenning in parallel, now would be
a good point to move commonly used code into its own crate. Otherwise
the auto-installer will always have to play catch up with the ongoing
development of the tui installer.

I tried to split up the commits as much as possible, but there are two
larger ones, copying most the code over to the new repo and making the
switch. The former because it is difficult to pull apart the parts that
belong together. The latter needed to be this big as most local
occurences needed to be removed at the same time to avoid dependency
conflicts.

One last things that is missing, is the "InstallConfig" struct.
This should also be part of the common crate, but I need to look further
into how to make it possible that it can be created from structs of each
crate (tui, auto) as implementing a ::From can only be done within the
crate where the struct lives IIUC.

This series depends on the patches by Christoph to remove the global
unsafe setup info, version 2 [0]. Without those patches applied first,
this series will not apply.

[0] https://lists.proxmox.com/pipermail/pve-devel/2023-October/059628.html

Aaron Lauterer (12):
  add proxmox-installer-common crate
  common: copy common code from tui-installer
  common: utils: add dependency for doc test
  common: make InstallZfsOption public
  common: disk_checks: make functions public
  tui-installer: add dependency for new common crate
  tui: switch to common crate
  tui: remove now unused utils.rs
  common: add installer_setup method
  common: document installer_setup method
  tui: use installer_setup from common cate
  tui: remove unused read_json function

 Cargo.toml                                    |   1 +
 proxmox-installer-common/Cargo.toml           |  12 +
 proxmox-installer-common/src/disk_checks.rs   | 237 ++++++++++
 proxmox-installer-common/src/lib.rs           |   4 +
 proxmox-installer-common/src/options.rs       | 387 +++++++++++++++++
 proxmox-installer-common/src/setup.rs         | 368 ++++++++++++++++
 .../src/utils.rs                              |   1 +
 proxmox-tui-installer/Cargo.toml              |   1 +
 proxmox-tui-installer/src/main.rs             |  51 +--
 proxmox-tui-installer/src/options.rs          | 403 +-----------------
 proxmox-tui-installer/src/setup.rs            | 324 +-------------
 proxmox-tui-installer/src/system.rs           |   2 +-
 proxmox-tui-installer/src/views/bootdisk.rs   | 248 +----------
 proxmox-tui-installer/src/views/mod.rs        |   2 +-
 proxmox-tui-installer/src/views/timezone.rs   |   4 +-
 15 files changed, 1054 insertions(+), 991 deletions(-)
 create mode 100644 proxmox-installer-common/Cargo.toml
 create mode 100644 proxmox-installer-common/src/disk_checks.rs
 create mode 100644 proxmox-installer-common/src/lib.rs
 create mode 100644 proxmox-installer-common/src/options.rs
 create mode 100644 proxmox-installer-common/src/setup.rs
 rename {proxmox-tui-installer => proxmox-installer-common}/src/utils.rs (99%)

-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pve-devel] [PATCH 01/12] add proxmox-installer-common crate
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
@ 2023-10-25 16:00 ` Aaron Lauterer
  2023-10-27 10:59   ` Christoph Heiss
  2023-10-25 16:00 ` [pve-devel] [PATCH 02/12] common: copy common code from tui-installer Aaron Lauterer
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 16:00 UTC (permalink / raw)
  To: pve-devel

It will be used for code shared among the different crates in the
installer. For now between the TUI installer and the upcoming auto
installer.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 Cargo.toml                          |  1 +
 proxmox-installer-common/Cargo.toml | 10 ++++++++++
 proxmox-installer-common/src/lib.rs |  0
 3 files changed, 11 insertions(+)
 create mode 100644 proxmox-installer-common/Cargo.toml
 create mode 100644 proxmox-installer-common/src/lib.rs

diff --git a/Cargo.toml b/Cargo.toml
index fd151ba..c1bd578 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,5 +1,6 @@
 [workspace]
 members = [
+    "proxmox-installer-common",
     "proxmox-tui-installer",
 ]
 
diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml
new file mode 100644
index 0000000..b8762e8
--- /dev/null
+++ b/proxmox-installer-common/Cargo.toml
@@ -0,0 +1,10 @@
+[package]
+name = "proxmox-installer-common"
+version = "0.1.0"
+edition = "2021"
+authors = [ "Aaron Lauterer <a.lauterer@proxmox.com>" ]
+license = "AGPL-3"
+exclude = [ "build", "debian" ]
+homepage = "https://www.proxmox.com"
+
+[dependencies]
diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
new file mode 100644
index 0000000..e69de29
-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pve-devel] [PATCH 02/12] common: copy common code from tui-installer
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 01/12] add proxmox-installer-common crate Aaron Lauterer
@ 2023-10-25 16:00 ` Aaron Lauterer
  2023-10-27 11:14   ` Christoph Heiss
  2023-10-25 16:00 ` [pve-devel] [PATCH 03/12] common: utils: add dependency for doc test Aaron Lauterer
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 16:00 UTC (permalink / raw)
  To: pve-devel

Copy code that is common to its own crate.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-installer-common/Cargo.toml         |   2 +
 proxmox-installer-common/src/disk_checks.rs | 237 ++++++++++++
 proxmox-installer-common/src/lib.rs         |   4 +
 proxmox-installer-common/src/options.rs     | 387 ++++++++++++++++++++
 proxmox-installer-common/src/setup.rs       | 330 +++++++++++++++++
 proxmox-installer-common/src/utils.rs       | 268 ++++++++++++++
 6 files changed, 1228 insertions(+)
 create mode 100644 proxmox-installer-common/src/disk_checks.rs
 create mode 100644 proxmox-installer-common/src/options.rs
 create mode 100644 proxmox-installer-common/src/setup.rs
 create mode 100644 proxmox-installer-common/src/utils.rs

diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml
index b8762e8..bde5457 100644
--- a/proxmox-installer-common/Cargo.toml
+++ b/proxmox-installer-common/Cargo.toml
@@ -8,3 +8,5 @@ exclude = [ "build", "debian" ]
 homepage = "https://www.proxmox.com"
 
 [dependencies]
+serde = { version = "1.0", features = ["derive"] }
+serde_json = "1.0"
diff --git a/proxmox-installer-common/src/disk_checks.rs b/proxmox-installer-common/src/disk_checks.rs
new file mode 100644
index 0000000..15b5928
--- /dev/null
+++ b/proxmox-installer-common/src/disk_checks.rs
@@ -0,0 +1,237 @@
+use std::collections::HashSet;
+
+use crate::options::{BtrfsRaidLevel, Disk, ZfsRaidLevel};
+use crate::setup::BootType;
+
+/// Checks a list of disks for duplicate entries, using their index as key.
+///
+/// # Arguments
+///
+/// * `disks` - A list of disks to check for duplicates.
+fn check_for_duplicate_disks(disks: &[Disk]) -> Result<(), &Disk> {
+    let mut set = HashSet::new();
+
+    for disk in disks {
+        if !set.insert(&disk.index) {
+            return Err(disk);
+        }
+    }
+
+    Ok(())
+}
+
+/// Simple wrapper which returns an descriptive error if the list of disks is too short.
+///
+/// # Arguments
+///
+/// * `disks` - A list of disks to check the lenght of.
+/// * `min` - Minimum number of disks
+fn check_raid_min_disks(disks: &[Disk], min: usize) -> Result<(), String> {
+    if disks.len() < min {
+        Err(format!("Need at least {min} disks"))
+    } else {
+        Ok(())
+    }
+}
+
+/// Checks all disks for legacy BIOS boot compatibility and reports an error as appropriate. 4Kn
+/// disks are generally broken with legacy BIOS and cannot be booted from.
+///
+/// # Arguments
+///
+/// * `runinfo` - `RuntimeInfo` instance of currently running system
+/// * `disks` - List of disks designated as bootdisk targets.
+fn check_disks_4kn_legacy_boot(boot_type: BootType, disks: &[Disk]) -> Result<(), &str> {
+    let is_blocksize_4096 = |disk: &Disk| disk.block_size.map(|s| s == 4096).unwrap_or(false);
+
+    if boot_type == BootType::Bios && disks.iter().any(is_blocksize_4096) {
+        return Err("Booting from 4Kn drive in legacy BIOS mode is not supported.");
+    }
+
+    Ok(())
+}
+
+/// Checks whether a user-supplied ZFS RAID setup is valid or not, such as disk sizes andminimum
+/// number of disks.
+///
+/// # Arguments
+///
+/// * `level` - The targeted ZFS RAID level by the user.
+/// * `disks` - List of disks designated as RAID targets.
+fn check_zfs_raid_config(level: ZfsRaidLevel, disks: &[Disk]) -> Result<(), String> {
+    // See also Proxmox/Install.pm:get_zfs_raid_setup()
+
+    let check_mirror_size = |disk1: &Disk, disk2: &Disk| {
+        if (disk1.size - disk2.size).abs() > disk1.size / 10. {
+            Err(format!(
+                "Mirrored disks must have same size:\n\n  * {disk1}\n  * {disk2}"
+            ))
+        } else {
+            Ok(())
+        }
+    };
+
+    match level {
+        ZfsRaidLevel::Raid0 => check_raid_min_disks(disks, 1)?,
+        ZfsRaidLevel::Raid1 => {
+            check_raid_min_disks(disks, 2)?;
+            for disk in disks {
+                check_mirror_size(&disks[0], disk)?;
+            }
+        }
+        ZfsRaidLevel::Raid10 => {
+            check_raid_min_disks(disks, 4)?;
+            // Pairs need to have the same size
+            for i in (0..disks.len()).step_by(2) {
+                check_mirror_size(&disks[i], &disks[i + 1])?;
+            }
+        }
+        // For RAID-Z: minimum disks number is level + 2
+        ZfsRaidLevel::RaidZ => {
+            check_raid_min_disks(disks, 3)?;
+            for disk in disks {
+                check_mirror_size(&disks[0], disk)?;
+            }
+        }
+        ZfsRaidLevel::RaidZ2 => {
+            check_raid_min_disks(disks, 4)?;
+            for disk in disks {
+                check_mirror_size(&disks[0], disk)?;
+            }
+        }
+        ZfsRaidLevel::RaidZ3 => {
+            check_raid_min_disks(disks, 5)?;
+            for disk in disks {
+                check_mirror_size(&disks[0], disk)?;
+            }
+        }
+    }
+
+    Ok(())
+}
+
+/// Checks whether a user-supplied Btrfs RAID setup is valid or not, such as minimum
+/// number of disks.
+///
+/// # Arguments
+///
+/// * `level` - The targeted Btrfs RAID level by the user.
+/// * `disks` - List of disks designated as RAID targets.
+fn check_btrfs_raid_config(level: BtrfsRaidLevel, disks: &[Disk]) -> Result<(), String> {
+    // See also Proxmox/Install.pm:get_btrfs_raid_setup()
+
+    match level {
+        BtrfsRaidLevel::Raid0 => check_raid_min_disks(disks, 1)?,
+        BtrfsRaidLevel::Raid1 => check_raid_min_disks(disks, 2)?,
+        BtrfsRaidLevel::Raid10 => check_raid_min_disks(disks, 4)?,
+    }
+
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn dummy_disk(index: usize) -> Disk {
+        Disk {
+            index: index.to_string(),
+            path: format!("/dev/dummy{index}"),
+            model: Some("Dummy disk".to_owned()),
+            size: 1024. * 1024. * 1024. * 8.,
+            block_size: Some(512),
+        }
+    }
+
+    fn dummy_disks(num: usize) -> Vec<Disk> {
+        (0..num).map(dummy_disk).collect()
+    }
+
+    #[test]
+    fn duplicate_disks() {
+        assert!(check_for_duplicate_disks(&dummy_disks(2)).is_ok());
+        assert_eq!(
+            check_for_duplicate_disks(&[
+                dummy_disk(0),
+                dummy_disk(1),
+                dummy_disk(2),
+                dummy_disk(2),
+                dummy_disk(3),
+            ]),
+            Err(&dummy_disk(2)),
+        );
+    }
+
+    #[test]
+    fn raid_min_disks() {
+        let disks = dummy_disks(10);
+
+        assert!(check_raid_min_disks(&disks[..1], 2).is_err());
+        assert!(check_raid_min_disks(&disks[..1], 1).is_ok());
+        assert!(check_raid_min_disks(&disks, 1).is_ok());
+    }
+
+    #[test]
+    fn bios_boot_compat_4kn() {
+        for i in 0..10 {
+            let mut disks = dummy_disks(10);
+            disks[i].block_size = Some(4096);
+
+            // Must fail if /any/ of the disks are 4Kn
+            assert!(check_disks_4kn_legacy_boot(BootType::Bios, &disks).is_err());
+            // For UEFI, we allow it for every configuration
+            assert!(check_disks_4kn_legacy_boot(BootType::Efi, &disks).is_ok());
+        }
+    }
+
+    #[test]
+    fn btrfs_raid() {
+        let disks = dummy_disks(10);
+
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid0, &[]).is_err());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid0, &disks[..1]).is_ok());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid0, &disks).is_ok());
+
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &[]).is_err());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks[..1]).is_err());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks[..2]).is_ok());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks).is_ok());
+
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &[]).is_err());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..3]).is_err());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..4]).is_ok());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks).is_ok());
+    }
+
+    #[test]
+    fn zfs_raid() {
+        let disks = dummy_disks(10);
+
+        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid0, &[]).is_err());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid0, &disks[..1]).is_ok());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid0, &disks).is_ok());
+
+        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid1, &[]).is_err());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid1, &disks[..2]).is_ok());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid1, &disks).is_ok());
+
+        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid10, &[]).is_err());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid10, &dummy_disks(4)).is_ok());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid10, &disks).is_ok());
+
+        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &[]).is_err());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &disks[..2]).is_err());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &disks[..3]).is_ok());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &disks).is_ok());
+
+        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &[]).is_err());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &disks[..3]).is_err());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &disks[..4]).is_ok());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &disks).is_ok());
+
+        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &[]).is_err());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &disks[..4]).is_err());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &disks[..5]).is_ok());
+        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &disks).is_ok());
+    }
+}
diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
index e69de29..f0093f5 100644
--- a/proxmox-installer-common/src/lib.rs
+++ b/proxmox-installer-common/src/lib.rs
@@ -0,0 +1,4 @@
+pub mod disk_checks;
+pub mod options;
+pub mod setup;
+pub mod utils;
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
new file mode 100644
index 0000000..185be2e
--- /dev/null
+++ b/proxmox-installer-common/src/options.rs
@@ -0,0 +1,387 @@
+use std::net::{IpAddr, Ipv4Addr};
+use std::{cmp, fmt};
+
+use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo};
+use crate::utils::{CidrAddress, Fqdn};
+
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum BtrfsRaidLevel {
+    Raid0,
+    Raid1,
+    Raid10,
+}
+
+impl fmt::Display for BtrfsRaidLevel {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use BtrfsRaidLevel::*;
+        match self {
+            Raid0 => write!(f, "RAID0"),
+            Raid1 => write!(f, "RAID1"),
+            Raid10 => write!(f, "RAID10"),
+        }
+    }
+}
+
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum ZfsRaidLevel {
+    Raid0,
+    Raid1,
+    Raid10,
+    RaidZ,
+    RaidZ2,
+    RaidZ3,
+}
+
+impl fmt::Display for ZfsRaidLevel {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use ZfsRaidLevel::*;
+        match self {
+            Raid0 => write!(f, "RAID0"),
+            Raid1 => write!(f, "RAID1"),
+            Raid10 => write!(f, "RAID10"),
+            RaidZ => write!(f, "RAIDZ-1"),
+            RaidZ2 => write!(f, "RAIDZ-2"),
+            RaidZ3 => write!(f, "RAIDZ-3"),
+        }
+    }
+}
+
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum FsType {
+    Ext4,
+    Xfs,
+    Zfs(ZfsRaidLevel),
+    Btrfs(BtrfsRaidLevel),
+}
+
+impl FsType {
+    pub fn is_btrfs(&self) -> bool {
+        matches!(self, FsType::Btrfs(_))
+    }
+}
+
+impl fmt::Display for FsType {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use FsType::*;
+        match self {
+            Ext4 => write!(f, "ext4"),
+            Xfs => write!(f, "XFS"),
+            Zfs(level) => write!(f, "ZFS ({level})"),
+            Btrfs(level) => write!(f, "Btrfs ({level})"),
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+pub struct LvmBootdiskOptions {
+    pub total_size: f64,
+    pub swap_size: Option<f64>,
+    pub max_root_size: Option<f64>,
+    pub max_data_size: Option<f64>,
+    pub min_lvm_free: Option<f64>,
+}
+
+impl LvmBootdiskOptions {
+    pub fn defaults_from(disk: &Disk) -> Self {
+        Self {
+            total_size: disk.size,
+            swap_size: None,
+            max_root_size: None,
+            max_data_size: None,
+            min_lvm_free: None,
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+pub struct BtrfsBootdiskOptions {
+    pub disk_size: f64,
+    pub selected_disks: Vec<usize>,
+}
+
+impl BtrfsBootdiskOptions {
+    /// This panics if the provided slice is empty.
+    pub fn defaults_from(disks: &[Disk]) -> Self {
+        let disk = &disks[0];
+        Self {
+            disk_size: disk.size,
+            selected_disks: (0..disks.len()).collect(),
+        }
+    }
+}
+
+#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
+pub enum ZfsCompressOption {
+    #[default]
+    On,
+    Off,
+    Lzjb,
+    Lz4,
+    Zle,
+    Gzip,
+    Zstd,
+}
+
+impl fmt::Display for ZfsCompressOption {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}", format!("{self:?}").to_lowercase())
+    }
+}
+
+impl From<&ZfsCompressOption> for String {
+    fn from(value: &ZfsCompressOption) -> Self {
+        value.to_string()
+    }
+}
+
+pub const ZFS_COMPRESS_OPTIONS: &[ZfsCompressOption] = {
+    use ZfsCompressOption::*;
+    &[On, Off, Lzjb, Lz4, Zle, Gzip, Zstd]
+};
+
+#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
+pub enum ZfsChecksumOption {
+    #[default]
+    On,
+    Off,
+    Fletcher2,
+    Fletcher4,
+    Sha256,
+}
+
+impl fmt::Display for ZfsChecksumOption {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}", format!("{self:?}").to_lowercase())
+    }
+}
+
+impl From<&ZfsChecksumOption> for String {
+    fn from(value: &ZfsChecksumOption) -> Self {
+        value.to_string()
+    }
+}
+
+pub const ZFS_CHECKSUM_OPTIONS: &[ZfsChecksumOption] = {
+    use ZfsChecksumOption::*;
+    &[On, Off, Fletcher2, Fletcher4, Sha256]
+};
+
+#[derive(Clone, Debug)]
+pub struct ZfsBootdiskOptions {
+    pub ashift: usize,
+    pub compress: ZfsCompressOption,
+    pub checksum: ZfsChecksumOption,
+    pub copies: usize,
+    pub disk_size: f64,
+    pub selected_disks: Vec<usize>,
+}
+
+impl ZfsBootdiskOptions {
+    /// This panics if the provided slice is empty.
+    pub fn defaults_from(disks: &[Disk]) -> Self {
+        let disk = &disks[0];
+        Self {
+            ashift: 12,
+            compress: ZfsCompressOption::default(),
+            checksum: ZfsChecksumOption::default(),
+            copies: 1,
+            disk_size: disk.size,
+            selected_disks: (0..disks.len()).collect(),
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+pub enum AdvancedBootdiskOptions {
+    Lvm(LvmBootdiskOptions),
+    Zfs(ZfsBootdiskOptions),
+    Btrfs(BtrfsBootdiskOptions),
+}
+
+#[derive(Clone, Debug, PartialEq)]
+pub struct Disk {
+    pub index: String,
+    pub path: String,
+    pub model: Option<String>,
+    pub size: f64,
+    pub block_size: Option<usize>,
+}
+
+impl fmt::Display for Disk {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        // TODO: Format sizes properly with `proxmox-human-byte` once merged
+        // https://lists.proxmox.com/pipermail/pbs-devel/2023-May/006125.html
+        f.write_str(&self.path)?;
+        if let Some(model) = &self.model {
+            // FIXME: ellipsize too-long names?
+            write!(f, " ({model})")?;
+        }
+        write!(f, " ({:.2} GiB)", self.size)
+    }
+}
+
+impl From<&Disk> for String {
+    fn from(value: &Disk) -> Self {
+        value.to_string()
+    }
+}
+
+impl cmp::Eq for Disk {}
+
+impl cmp::PartialOrd for Disk {
+    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
+        self.index.partial_cmp(&other.index)
+    }
+}
+
+impl cmp::Ord for Disk {
+    fn cmp(&self, other: &Self) -> cmp::Ordering {
+        self.index.cmp(&other.index)
+    }
+}
+
+#[derive(Clone, Debug)]
+pub struct BootdiskOptions {
+    pub disks: Vec<Disk>,
+    pub fstype: FsType,
+    pub advanced: AdvancedBootdiskOptions,
+}
+
+impl BootdiskOptions {
+    pub fn defaults_from(disk: &Disk) -> Self {
+        Self {
+            disks: vec![disk.clone()],
+            fstype: FsType::Ext4,
+            advanced: AdvancedBootdiskOptions::Lvm(LvmBootdiskOptions::defaults_from(disk)),
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+pub struct TimezoneOptions {
+    pub country: String,
+    pub timezone: String,
+    pub kb_layout: String,
+}
+
+impl TimezoneOptions {
+    pub fn defaults_from(runtime: &RuntimeInfo, locales: &LocaleInfo) -> Self {
+        let country = runtime.country.clone().unwrap_or_else(|| "at".to_owned());
+
+        let timezone = locales
+            .cczones
+            .get(&country)
+            .and_then(|zones| zones.get(0))
+            .cloned()
+            .unwrap_or_else(|| "UTC".to_owned());
+
+        let kb_layout = locales
+            .countries
+            .get(&country)
+            .and_then(|c| {
+                if c.kmap.is_empty() {
+                    None
+                } else {
+                    Some(c.kmap.clone())
+                }
+            })
+            .unwrap_or_else(|| "en-us".to_owned());
+
+        Self {
+            country,
+            timezone,
+            kb_layout,
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+pub struct PasswordOptions {
+    pub email: String,
+    pub root_password: String,
+}
+
+impl Default for PasswordOptions {
+    fn default() -> Self {
+        Self {
+            email: "mail@example.invalid".to_string(),
+            root_password: String::new(),
+        }
+    }
+}
+
+#[derive(Clone, Debug, PartialEq)]
+pub struct NetworkOptions {
+    pub ifname: String,
+    pub fqdn: Fqdn,
+    pub address: CidrAddress,
+    pub gateway: IpAddr,
+    pub dns_server: IpAddr,
+}
+
+impl NetworkOptions {
+    const DEFAULT_DOMAIN: &str = "example.invalid";
+
+    pub fn defaults_from(setup: &SetupInfo, network: &NetworkInfo) -> Self {
+        let mut this = Self {
+            ifname: String::new(),
+            fqdn: Self::construct_fqdn(network, setup.config.product.default_hostname()),
+            // Safety: The provided mask will always be valid.
+            address: CidrAddress::new(Ipv4Addr::UNSPECIFIED, 0).unwrap(),
+            gateway: Ipv4Addr::UNSPECIFIED.into(),
+            dns_server: Ipv4Addr::UNSPECIFIED.into(),
+        };
+
+        if let Some(ip) = network.dns.dns.first() {
+            this.dns_server = *ip;
+        }
+
+        if let Some(routes) = &network.routes {
+            let mut filled = false;
+            if let Some(gw) = &routes.gateway4 {
+                if let Some(iface) = network.interfaces.get(&gw.dev) {
+                    this.ifname = iface.name.clone();
+                    if let Some(addresses) = &iface.addresses {
+                        if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv4()) {
+                            this.gateway = gw.gateway;
+                            this.address = addr.clone();
+                            filled = true;
+                        }
+                    }
+                }
+            }
+            if !filled {
+                if let Some(gw) = &routes.gateway6 {
+                    if let Some(iface) = network.interfaces.get(&gw.dev) {
+                        if let Some(addresses) = &iface.addresses {
+                            if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv6()) {
+                                this.ifname = iface.name.clone();
+                                this.gateway = gw.gateway;
+                                this.address = addr.clone();
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
+        this
+    }
+
+    fn construct_fqdn(network: &NetworkInfo, default_hostname: &str) -> Fqdn {
+        let hostname = network.hostname.as_deref().unwrap_or(default_hostname);
+
+        let domain = network
+            .dns
+            .domain
+            .as_deref()
+            .unwrap_or(Self::DEFAULT_DOMAIN);
+
+        Fqdn::from(&format!("{hostname}.{domain}")).unwrap_or_else(|_| {
+            // Safety: This will always result in a valid FQDN, as we control & know
+            // the values of default_hostname (one of "pve", "pmg" or "pbs") and
+            // constant-defined DEFAULT_DOMAIN.
+            Fqdn::from(&format!("{}.{}", default_hostname, Self::DEFAULT_DOMAIN)).unwrap()
+        })
+    }
+}
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
new file mode 100644
index 0000000..a4947f1
--- /dev/null
+++ b/proxmox-installer-common/src/setup.rs
@@ -0,0 +1,330 @@
+use std::{
+    cmp,
+    collections::HashMap,
+    fmt,
+    fs::File,
+    io::BufReader,
+    net::IpAddr,
+    path::{Path, PathBuf},
+};
+
+use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
+
+use crate::{
+    options::{Disk, ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption},
+    utils::CidrAddress,
+};
+
+#[allow(clippy::upper_case_acronyms)]
+#[derive(Clone, Copy, Deserialize, PartialEq)]
+#[serde(rename_all = "lowercase")]
+pub enum ProxmoxProduct {
+    PVE,
+    PBS,
+    PMG,
+}
+
+impl ProxmoxProduct {
+    pub fn default_hostname(self) -> &'static str {
+        match self {
+            Self::PVE => "pve",
+            Self::PMG => "pmg",
+            Self::PBS => "pbs",
+        }
+    }
+}
+
+#[derive(Clone, Deserialize)]
+pub struct ProductConfig {
+    pub fullname: String,
+    pub product: ProxmoxProduct,
+    #[serde(deserialize_with = "deserialize_bool_from_int")]
+    pub enable_btrfs: bool,
+}
+
+#[derive(Clone, Deserialize)]
+pub struct IsoInfo {
+    pub release: String,
+    pub isorelease: String,
+}
+
+/// Paths in the ISO environment containing installer data.
+#[derive(Clone, Deserialize)]
+pub struct IsoLocations {
+    pub iso: PathBuf,
+}
+
+#[derive(Clone, Deserialize)]
+pub struct SetupInfo {
+    #[serde(rename = "product-cfg")]
+    pub config: ProductConfig,
+    #[serde(rename = "iso-info")]
+    pub iso_info: IsoInfo,
+    pub locations: IsoLocations,
+}
+
+#[derive(Clone, Deserialize)]
+pub struct CountryInfo {
+    pub name: String,
+    #[serde(default)]
+    pub zone: String,
+    pub kmap: String,
+}
+
+#[derive(Clone, Deserialize, Eq, PartialEq)]
+pub struct KeyboardMapping {
+    pub name: String,
+    #[serde(rename = "kvm")]
+    pub id: String,
+    #[serde(rename = "x11")]
+    pub xkb_layout: String,
+    #[serde(rename = "x11var")]
+    pub xkb_variant: String,
+}
+
+impl cmp::PartialOrd for KeyboardMapping {
+    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
+        self.name.partial_cmp(&other.name)
+    }
+}
+
+impl cmp::Ord for KeyboardMapping {
+    fn cmp(&self, other: &Self) -> cmp::Ordering {
+        self.name.cmp(&other.name)
+    }
+}
+
+#[derive(Clone, Deserialize)]
+pub struct LocaleInfo {
+    #[serde(deserialize_with = "deserialize_cczones_map")]
+    pub cczones: HashMap<String, Vec<String>>,
+    #[serde(rename = "country")]
+    pub countries: HashMap<String, CountryInfo>,
+    pub kmap: HashMap<String, KeyboardMapping>,
+}
+
+#[derive(Serialize)]
+struct InstallZfsOption {
+    ashift: usize,
+    #[serde(serialize_with = "serialize_as_display")]
+    compress: ZfsCompressOption,
+    #[serde(serialize_with = "serialize_as_display")]
+    checksum: ZfsChecksumOption,
+    copies: usize,
+}
+
+impl From<ZfsBootdiskOptions> for InstallZfsOption {
+    fn from(opts: ZfsBootdiskOptions) -> Self {
+        InstallZfsOption {
+            ashift: opts.ashift,
+            compress: opts.compress,
+            checksum: opts.checksum,
+            copies: opts.copies,
+        }
+    }
+}
+
+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);
+
+    serde_json::from_reader(reader).map_err(|err| format!("failed to parse JSON: {err}"))
+}
+
+fn deserialize_bool_from_int<'de, D>(deserializer: D) -> Result<bool, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let val: u32 = Deserialize::deserialize(deserializer)?;
+    Ok(val != 0)
+}
+
+fn deserialize_cczones_map<'de, D>(
+    deserializer: D,
+) -> Result<HashMap<String, Vec<String>>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let map: HashMap<String, HashMap<String, u32>> = Deserialize::deserialize(deserializer)?;
+
+    let mut result = HashMap::new();
+    for (cc, list) in map.into_iter() {
+        result.insert(cc, list.into_keys().collect());
+    }
+
+    Ok(result)
+}
+
+fn deserialize_disks_map<'de, D>(deserializer: D) -> Result<Vec<Disk>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let disks =
+        <Vec<(usize, String, f64, String, Option<usize>, String)>>::deserialize(deserializer)?;
+    Ok(disks
+        .into_iter()
+        .map(
+            |(index, device, size_mb, model, logical_bsize, _syspath)| Disk {
+                index: index.to_string(),
+                // Linux always reports the size of block devices in sectors, where one sector is
+                // defined as being 2^9 = 512 bytes in size.
+                // https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/blk_types.h?h=v6.4#n30
+                size: (size_mb * 512.) / 1024. / 1024. / 1024.,
+                block_size: logical_bsize,
+                path: device,
+                model: (!model.is_empty()).then_some(model),
+            },
+        )
+        .collect())
+}
+
+fn deserialize_cidr_list<'de, D>(deserializer: D) -> Result<Option<Vec<CidrAddress>>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    #[derive(Deserialize)]
+    struct CidrDescriptor {
+        address: String,
+        prefix: usize,
+        // family is implied anyway by parsing the address
+    }
+
+    let list: Vec<CidrDescriptor> = Deserialize::deserialize(deserializer)?;
+
+    let mut result = Vec::with_capacity(list.len());
+    for desc in list {
+        let ip_addr = desc
+            .address
+            .parse::<IpAddr>()
+            .map_err(|err| de::Error::custom(format!("{:?}", err)))?;
+
+        result.push(
+            CidrAddress::new(ip_addr, desc.prefix)
+                .map_err(|err| de::Error::custom(format!("{:?}", err)))?,
+        );
+    }
+
+    Ok(Some(result))
+}
+
+fn serialize_as_display<S, T>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
+where
+    S: Serializer,
+    T: fmt::Display,
+{
+    serializer.collect_str(value)
+}
+
+#[derive(Clone, Deserialize)]
+pub struct RuntimeInfo {
+    /// Whether is system was booted in (legacy) BIOS or UEFI mode.
+    pub boot_type: BootType,
+
+    /// Detected country if available.
+    pub country: Option<String>,
+
+    /// Maps devices to their information.
+    #[serde(deserialize_with = "deserialize_disks_map")]
+    pub disks: Vec<Disk>,
+
+    /// Network addresses, gateways and DNS info.
+    pub network: NetworkInfo,
+
+    /// Total memory of the system in MiB.
+    pub total_memory: usize,
+
+    /// Whether the CPU supports hardware-accelerated virtualization
+    #[serde(deserialize_with = "deserialize_bool_from_int")]
+    pub hvm_supported: bool,
+}
+
+#[derive(Copy, Clone, Eq, Deserialize, PartialEq)]
+#[serde(rename_all = "lowercase")]
+pub enum BootType {
+    Bios,
+    Efi,
+}
+
+#[derive(Clone, Deserialize)]
+pub struct NetworkInfo {
+    pub dns: Dns,
+    pub routes: Option<Routes>,
+
+    /// Maps devices to their configuration, if it has a usable configuration.
+    /// (Contains no entries for devices with only link-local addresses.)
+    #[serde(default)]
+    pub interfaces: HashMap<String, Interface>,
+
+    /// The hostname of this machine, if set by the DHCP server.
+    pub hostname: Option<String>,
+}
+
+#[derive(Clone, Deserialize)]
+pub struct Dns {
+    pub domain: Option<String>,
+
+    /// List of stringified IP addresses.
+    #[serde(default)]
+    pub dns: Vec<IpAddr>,
+}
+
+#[derive(Clone, Deserialize)]
+pub struct Routes {
+    /// Ipv4 gateway.
+    pub gateway4: Option<Gateway>,
+
+    /// Ipv6 gateway.
+    pub gateway6: Option<Gateway>,
+}
+
+#[derive(Clone, Deserialize)]
+pub struct Gateway {
+    /// Outgoing network device.
+    pub dev: String,
+
+    /// Stringified gateway IP address.
+    pub gateway: IpAddr,
+}
+
+#[derive(Clone, Deserialize)]
+#[serde(rename_all = "UPPERCASE")]
+pub enum InterfaceState {
+    Up,
+    Down,
+    #[serde(other)]
+    Unknown,
+}
+
+impl InterfaceState {
+    // avoid display trait as this is not the string representation for a serializer
+    pub fn render(&self) -> String {
+        match self {
+            Self::Up => "\u{25CF}",
+            Self::Down | Self::Unknown => " ",
+        }
+        .into()
+    }
+}
+
+#[derive(Clone, Deserialize)]
+pub struct Interface {
+    pub name: String,
+
+    pub index: usize,
+
+    pub mac: String,
+
+    pub state: InterfaceState,
+
+    #[serde(default)]
+    #[serde(deserialize_with = "deserialize_cidr_list")]
+    pub addresses: Option<Vec<CidrAddress>>,
+}
+
+impl Interface {
+    // avoid display trait as this is not the string representation for a serializer
+    pub fn render(&self) -> String {
+        format!("{} {}", self.state.render(), self.name)
+    }
+}
+
diff --git a/proxmox-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs
new file mode 100644
index 0000000..89349ed
--- /dev/null
+++ b/proxmox-installer-common/src/utils.rs
@@ -0,0 +1,268 @@
+use std::{
+    fmt,
+    net::{AddrParseError, IpAddr},
+    num::ParseIntError,
+    str::FromStr,
+};
+
+use serde::Deserialize;
+
+/// Possible errors that might occur when parsing CIDR addresses.
+#[derive(Debug)]
+pub enum CidrAddressParseError {
+    /// No delimiter for separating address and mask was found.
+    NoDelimiter,
+    /// The IP address part could not be parsed.
+    InvalidAddr(AddrParseError),
+    /// The mask could not be parsed.
+    InvalidMask(Option<ParseIntError>),
+}
+
+/// An IP address (IPv4 or IPv6), including network mask.
+///
+/// See the [`IpAddr`] type for more information how IP addresses are handled.
+/// The mask is appropriately enforced to be `0 <= mask <= 32` for IPv4 or
+/// `0 <= mask <= 128` for IPv6 addresses.
+///
+/// # Examples
+/// ```
+/// use std::net::{Ipv4Addr, Ipv6Addr};
+/// let ipv4 = CidrAddress::new(Ipv4Addr::new(192, 168, 0, 1), 24).unwrap();
+/// let ipv6 = CidrAddress::new(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0xc0a8, 1), 32).unwrap();
+///
+/// assert_eq!(ipv4.to_string(), "192.168.0.1/24");
+/// assert_eq!(ipv6.to_string(), "2001:db8::c0a8:1/32");
+/// ```
+#[derive(Clone, Debug, PartialEq)]
+pub struct CidrAddress {
+    addr: IpAddr,
+    mask: usize,
+}
+
+impl CidrAddress {
+    /// Constructs a new CIDR address.
+    ///
+    /// It fails if the mask is invalid for the given IP address.
+    pub fn new<T: Into<IpAddr>>(addr: T, mask: usize) -> Result<Self, CidrAddressParseError> {
+        let addr = addr.into();
+
+        if mask > mask_limit(&addr) {
+            Err(CidrAddressParseError::InvalidMask(None))
+        } else {
+            Ok(Self { addr, mask })
+        }
+    }
+
+    /// Returns only the IP address part of the address.
+    pub fn addr(&self) -> IpAddr {
+        self.addr
+    }
+
+    /// Returns `true` if this address is an IPv4 address, `false` otherwise.
+    pub fn is_ipv4(&self) -> bool {
+        self.addr.is_ipv4()
+    }
+
+    /// Returns `true` if this address is an IPv6 address, `false` otherwise.
+    pub fn is_ipv6(&self) -> bool {
+        self.addr.is_ipv6()
+    }
+
+    /// Returns only the mask part of the address.
+    pub fn mask(&self) -> usize {
+        self.mask
+    }
+}
+
+impl FromStr for CidrAddress {
+    type Err = CidrAddressParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let (addr, mask) = s
+            .split_once('/')
+            .ok_or(CidrAddressParseError::NoDelimiter)?;
+
+        let addr = addr.parse().map_err(CidrAddressParseError::InvalidAddr)?;
+
+        let mask = mask
+            .parse()
+            .map_err(|err| CidrAddressParseError::InvalidMask(Some(err)))?;
+
+        if mask > mask_limit(&addr) {
+            Err(CidrAddressParseError::InvalidMask(None))
+        } else {
+            Ok(Self { addr, mask })
+        }
+    }
+}
+
+impl fmt::Display for CidrAddress {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}/{}", self.addr, self.mask)
+    }
+}
+
+fn mask_limit(addr: &IpAddr) -> usize {
+    if addr.is_ipv4() {
+        32
+    } else {
+        128
+    }
+}
+
+/// Possible errors that might occur when parsing FQDNs.
+#[derive(Debug, Eq, PartialEq)]
+pub enum FqdnParseError {
+    MissingHostname,
+    NumericHostname,
+    InvalidPart(String),
+}
+
+impl fmt::Display for FqdnParseError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        use FqdnParseError::*;
+        match self {
+            MissingHostname => write!(f, "missing hostname part"),
+            NumericHostname => write!(f, "hostname cannot be purely numeric"),
+            InvalidPart(part) => write!(
+                f,
+                "FQDN must only consist of alphanumeric characters and dashes. Invalid part: '{part}'",
+            ),
+        }
+    }
+}
+
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct Fqdn {
+    parts: Vec<String>,
+}
+
+impl Fqdn {
+    pub fn from(fqdn: &str) -> Result<Self, FqdnParseError> {
+        let parts = fqdn
+            .split('.')
+            .map(ToOwned::to_owned)
+            .collect::<Vec<String>>();
+
+        for part in &parts {
+            if !Self::validate_single(part) {
+                return Err(FqdnParseError::InvalidPart(part.clone()));
+            }
+        }
+
+        if parts.len() < 2 {
+            Err(FqdnParseError::MissingHostname)
+        } else if parts[0].chars().all(|c| c.is_ascii_digit()) {
+            // Not allowed/supported on Debian systems.
+            Err(FqdnParseError::NumericHostname)
+        } else {
+            Ok(Self { parts })
+        }
+    }
+
+    pub fn host(&self) -> Option<&str> {
+        self.has_host().then_some(&self.parts[0])
+    }
+
+    pub fn domain(&self) -> String {
+        let parts = if self.has_host() {
+            &self.parts[1..]
+        } else {
+            &self.parts
+        };
+
+        parts.join(".")
+    }
+
+    /// Checks whether the FQDN has a hostname associated with it, i.e. is has more than 1 part.
+    fn has_host(&self) -> bool {
+        self.parts.len() > 1
+    }
+
+    fn validate_single(s: &String) -> bool {
+        !s.is_empty()
+            // First character must be alphanumeric
+            && s.chars()
+                .next()
+                .map(|c| c.is_ascii_alphanumeric())
+                .unwrap_or_default()
+            // .. last character as well,
+            && s.chars()
+                .last()
+                .map(|c| c.is_ascii_alphanumeric())
+                .unwrap_or_default()
+            // and anything between must be alphanumeric or -
+            && s.chars()
+                .skip(1)
+                .take(s.len().saturating_sub(2))
+                .all(|c| c.is_ascii_alphanumeric() || c == '-')
+    }
+}
+
+impl FromStr for Fqdn {
+    type Err = FqdnParseError;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        Self::from(value)
+    }
+}
+
+impl fmt::Display for Fqdn {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}", self.parts.join("."))
+    }
+}
+
+impl<'de> Deserialize<'de> for Fqdn {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        let s: String = Deserialize::deserialize(deserializer)?;
+        s.parse()
+            .map_err(|_| serde::de::Error::custom("invalid FQDN"))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn fqdn_construct() {
+        use FqdnParseError::*;
+        assert!(Fqdn::from("foo.example.com").is_ok());
+        assert!(Fqdn::from("foo-bar.com").is_ok());
+        assert!(Fqdn::from("a-b.com").is_ok());
+
+        assert_eq!(Fqdn::from("foo"), Err(MissingHostname));
+
+        assert_eq!(Fqdn::from("-foo.com"), Err(InvalidPart("-foo".to_owned())));
+        assert_eq!(Fqdn::from("foo-.com"), Err(InvalidPart("foo-".to_owned())));
+        assert_eq!(Fqdn::from("foo.com-"), Err(InvalidPart("com-".to_owned())));
+        assert_eq!(Fqdn::from("-o-.com"), Err(InvalidPart("-o-".to_owned())));
+
+        assert_eq!(Fqdn::from("123.com"), Err(NumericHostname));
+        assert!(Fqdn::from("foo123.com").is_ok());
+        assert!(Fqdn::from("123foo.com").is_ok());
+    }
+
+    #[test]
+    fn fqdn_parts() {
+        let fqdn = Fqdn::from("pve.example.com").unwrap();
+        assert_eq!(fqdn.host().unwrap(), "pve");
+        assert_eq!(fqdn.domain(), "example.com");
+        assert_eq!(
+            fqdn.parts,
+            &["pve".to_owned(), "example".to_owned(), "com".to_owned()]
+        );
+    }
+
+    #[test]
+    fn fqdn_display() {
+        assert_eq!(
+            Fqdn::from("foo.example.com").unwrap().to_string(),
+            "foo.example.com"
+        );
+    }
+}
-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pve-devel] [PATCH 03/12] common: utils: add dependency for doc test
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 01/12] add proxmox-installer-common crate Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 02/12] common: copy common code from tui-installer Aaron Lauterer
@ 2023-10-25 16:00 ` Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 04/12] common: make InstallZfsOption public Aaron Lauterer
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 16:00 UTC (permalink / raw)
  To: pve-devel

Was probably missed because it used to be in a binary crate where doc
tests aren't run automatically.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-installer-common/src/utils.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/proxmox-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs
index 89349ed..c038524 100644
--- a/proxmox-installer-common/src/utils.rs
+++ b/proxmox-installer-common/src/utils.rs
@@ -27,6 +27,7 @@ pub enum CidrAddressParseError {
 /// # Examples
 /// ```
 /// use std::net::{Ipv4Addr, Ipv6Addr};
+/// use proxmox_installer_common::utils::CidrAddress;
 /// let ipv4 = CidrAddress::new(Ipv4Addr::new(192, 168, 0, 1), 24).unwrap();
 /// let ipv6 = CidrAddress::new(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0xc0a8, 1), 32).unwrap();
 ///
-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pve-devel] [PATCH 04/12] common: make InstallZfsOption public
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
                   ` (2 preceding siblings ...)
  2023-10-25 16:00 ` [pve-devel] [PATCH 03/12] common: utils: add dependency for doc test Aaron Lauterer
@ 2023-10-25 16:00 ` Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 05/12] common: disk_checks: make functions public Aaron Lauterer
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 16:00 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-installer-common/src/setup.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index a4947f1..a55f059 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -104,7 +104,7 @@ pub struct LocaleInfo {
 }
 
 #[derive(Serialize)]
-struct InstallZfsOption {
+pub struct InstallZfsOption {
     ashift: usize,
     #[serde(serialize_with = "serialize_as_display")]
     compress: ZfsCompressOption,
-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pve-devel] [PATCH 05/12] common: disk_checks: make functions public
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
                   ` (3 preceding siblings ...)
  2023-10-25 16:00 ` [pve-devel] [PATCH 04/12] common: make InstallZfsOption public Aaron Lauterer
@ 2023-10-25 16:00 ` Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 06/12] tui-installer: add dependency for new common crate Aaron Lauterer
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 16:00 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-installer-common/src/disk_checks.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/proxmox-installer-common/src/disk_checks.rs b/proxmox-installer-common/src/disk_checks.rs
index 15b5928..bcf2e21 100644
--- a/proxmox-installer-common/src/disk_checks.rs
+++ b/proxmox-installer-common/src/disk_checks.rs
@@ -8,7 +8,7 @@ use crate::setup::BootType;
 /// # Arguments
 ///
 /// * `disks` - A list of disks to check for duplicates.
-fn check_for_duplicate_disks(disks: &[Disk]) -> Result<(), &Disk> {
+pub fn check_for_duplicate_disks(disks: &[Disk]) -> Result<(), &Disk> {
     let mut set = HashSet::new();
 
     for disk in disks {
@@ -26,7 +26,7 @@ fn check_for_duplicate_disks(disks: &[Disk]) -> Result<(), &Disk> {
 ///
 /// * `disks` - A list of disks to check the lenght of.
 /// * `min` - Minimum number of disks
-fn check_raid_min_disks(disks: &[Disk], min: usize) -> Result<(), String> {
+pub fn check_raid_min_disks(disks: &[Disk], min: usize) -> Result<(), String> {
     if disks.len() < min {
         Err(format!("Need at least {min} disks"))
     } else {
@@ -41,7 +41,7 @@ fn check_raid_min_disks(disks: &[Disk], min: usize) -> Result<(), String> {
 ///
 /// * `runinfo` - `RuntimeInfo` instance of currently running system
 /// * `disks` - List of disks designated as bootdisk targets.
-fn check_disks_4kn_legacy_boot(boot_type: BootType, disks: &[Disk]) -> Result<(), &str> {
+pub fn check_disks_4kn_legacy_boot(boot_type: BootType, disks: &[Disk]) -> Result<(), &str> {
     let is_blocksize_4096 = |disk: &Disk| disk.block_size.map(|s| s == 4096).unwrap_or(false);
 
     if boot_type == BootType::Bios && disks.iter().any(is_blocksize_4096) {
@@ -58,7 +58,7 @@ fn check_disks_4kn_legacy_boot(boot_type: BootType, disks: &[Disk]) -> Result<()
 ///
 /// * `level` - The targeted ZFS RAID level by the user.
 /// * `disks` - List of disks designated as RAID targets.
-fn check_zfs_raid_config(level: ZfsRaidLevel, disks: &[Disk]) -> Result<(), String> {
+pub fn check_zfs_raid_config(level: ZfsRaidLevel, disks: &[Disk]) -> Result<(), String> {
     // See also Proxmox/Install.pm:get_zfs_raid_setup()
 
     let check_mirror_size = |disk1: &Disk, disk2: &Disk| {
@@ -117,7 +117,7 @@ fn check_zfs_raid_config(level: ZfsRaidLevel, disks: &[Disk]) -> Result<(), Stri
 ///
 /// * `level` - The targeted Btrfs RAID level by the user.
 /// * `disks` - List of disks designated as RAID targets.
-fn check_btrfs_raid_config(level: BtrfsRaidLevel, disks: &[Disk]) -> Result<(), String> {
+pub fn check_btrfs_raid_config(level: BtrfsRaidLevel, disks: &[Disk]) -> Result<(), String> {
     // See also Proxmox/Install.pm:get_btrfs_raid_setup()
 
     match level {
-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pve-devel] [PATCH 06/12] tui-installer: add dependency for new common crate
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
                   ` (4 preceding siblings ...)
  2023-10-25 16:00 ` [pve-devel] [PATCH 05/12] common: disk_checks: make functions public Aaron Lauterer
@ 2023-10-25 16:00 ` Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 07/12] tui: switch to " Aaron Lauterer
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 16:00 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-tui-installer/Cargo.toml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/proxmox-tui-installer/Cargo.toml b/proxmox-tui-installer/Cargo.toml
index 8a6eba8..fc653f0 100644
--- a/proxmox-tui-installer/Cargo.toml
+++ b/proxmox-tui-installer/Cargo.toml
@@ -12,3 +12,4 @@ cursive = { version = "0.20.0", default-features = false, features = ["termion-b
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
 regex = "1.7"
+proxmox-installer-common = { path = "../proxmox-installer-common" }
-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pve-devel] [PATCH 07/12] tui: switch to common crate
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
                   ` (5 preceding siblings ...)
  2023-10-25 16:00 ` [pve-devel] [PATCH 06/12] tui-installer: add dependency for new common crate Aaron Lauterer
@ 2023-10-25 16:00 ` Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 08/12] tui: remove now unused utils.rs Aaron Lauterer
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 16:00 UTC (permalink / raw)
  To: pve-devel

by switching dependencies and deleting doubled code to avoid ambiguities
within the same scope.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-tui-installer/src/main.rs           |  13 +-
 proxmox-tui-installer/src/options.rs        | 403 +-------------------
 proxmox-tui-installer/src/setup.rs          | 316 +--------------
 proxmox-tui-installer/src/system.rs         |   2 +-
 proxmox-tui-installer/src/views/bootdisk.rs | 248 +-----------
 proxmox-tui-installer/src/views/mod.rs      |   2 +-
 proxmox-tui-installer/src/views/timezone.rs |   4 +-
 7 files changed, 44 insertions(+), 944 deletions(-)

diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 81fe3ca..875a33a 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -28,16 +28,19 @@ use cursive::{
 use regex::Regex;
 
 mod options;
-use options::*;
+use options::InstallerOptions;
+
+use proxmox_installer_common::{
+    options::{BootdiskOptions, NetworkOptions, PasswordOptions, TimezoneOptions},
+    setup::{LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo},
+    utils::Fqdn,
+};
 
 mod setup;
-use setup::{InstallConfig, LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo};
+use setup::InstallConfig;
 
 mod system;
 
-mod utils;
-use utils::Fqdn;
-
 mod views;
 use views::{
     BootdiskOptionsView, CidrAddressEditView, FormView, TableView, TableViewItem,
diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs
index 85b39b8..221ad01 100644
--- a/proxmox-tui-installer/src/options.rs
+++ b/proxmox-tui-installer/src/options.rs
@@ -1,77 +1,12 @@
-use std::net::{IpAddr, Ipv4Addr};
-use std::{cmp, fmt};
-
-use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo};
-use crate::utils::{CidrAddress, Fqdn};
 use crate::SummaryOption;
 
-#[derive(Copy, Clone, Debug, Eq, PartialEq)]
-pub enum BtrfsRaidLevel {
-    Raid0,
-    Raid1,
-    Raid10,
-}
-
-impl fmt::Display for BtrfsRaidLevel {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use BtrfsRaidLevel::*;
-        match self {
-            Raid0 => write!(f, "RAID0"),
-            Raid1 => write!(f, "RAID1"),
-            Raid10 => write!(f, "RAID10"),
-        }
-    }
-}
-
-#[derive(Copy, Clone, Debug, Eq, PartialEq)]
-pub enum ZfsRaidLevel {
-    Raid0,
-    Raid1,
-    Raid10,
-    RaidZ,
-    RaidZ2,
-    RaidZ3,
-}
-
-impl fmt::Display for ZfsRaidLevel {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use ZfsRaidLevel::*;
-        match self {
-            Raid0 => write!(f, "RAID0"),
-            Raid1 => write!(f, "RAID1"),
-            Raid10 => write!(f, "RAID10"),
-            RaidZ => write!(f, "RAIDZ-1"),
-            RaidZ2 => write!(f, "RAIDZ-2"),
-            RaidZ3 => write!(f, "RAIDZ-3"),
-        }
-    }
-}
-
-#[derive(Copy, Clone, Debug, Eq, PartialEq)]
-pub enum FsType {
-    Ext4,
-    Xfs,
-    Zfs(ZfsRaidLevel),
-    Btrfs(BtrfsRaidLevel),
-}
-
-impl FsType {
-    pub fn is_btrfs(&self) -> bool {
-        matches!(self, FsType::Btrfs(_))
-    }
-}
-
-impl fmt::Display for FsType {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use FsType::*;
-        match self {
-            Ext4 => write!(f, "ext4"),
-            Xfs => write!(f, "XFS"),
-            Zfs(level) => write!(f, "ZFS ({level})"),
-            Btrfs(level) => write!(f, "Btrfs ({level})"),
-        }
-    }
-}
+use proxmox_installer_common::{
+    setup::LocaleInfo,
+    options::{
+        BootdiskOptions, BtrfsRaidLevel, FsType, NetworkOptions, TimezoneOptions,
+        PasswordOptions, ZfsRaidLevel,
+    },
+};
 
 pub const FS_TYPES: &[FsType] = {
     use FsType::*;
@@ -90,320 +25,6 @@ pub const FS_TYPES: &[FsType] = {
     ]
 };
 
-#[derive(Clone, Debug)]
-pub struct LvmBootdiskOptions {
-    pub total_size: f64,
-    pub swap_size: Option<f64>,
-    pub max_root_size: Option<f64>,
-    pub max_data_size: Option<f64>,
-    pub min_lvm_free: Option<f64>,
-}
-
-impl LvmBootdiskOptions {
-    pub fn defaults_from(disk: &Disk) -> Self {
-        Self {
-            total_size: disk.size,
-            swap_size: None,
-            max_root_size: None,
-            max_data_size: None,
-            min_lvm_free: None,
-        }
-    }
-}
-
-#[derive(Clone, Debug)]
-pub struct BtrfsBootdiskOptions {
-    pub disk_size: f64,
-    pub selected_disks: Vec<usize>,
-}
-
-impl BtrfsBootdiskOptions {
-    /// This panics if the provided slice is empty.
-    pub fn defaults_from(disks: &[Disk]) -> Self {
-        let disk = &disks[0];
-        Self {
-            disk_size: disk.size,
-            selected_disks: (0..disks.len()).collect(),
-        }
-    }
-}
-
-#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
-pub enum ZfsCompressOption {
-    #[default]
-    On,
-    Off,
-    Lzjb,
-    Lz4,
-    Zle,
-    Gzip,
-    Zstd,
-}
-
-impl fmt::Display for ZfsCompressOption {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", format!("{self:?}").to_lowercase())
-    }
-}
-
-impl From<&ZfsCompressOption> for String {
-    fn from(value: &ZfsCompressOption) -> Self {
-        value.to_string()
-    }
-}
-
-pub const ZFS_COMPRESS_OPTIONS: &[ZfsCompressOption] = {
-    use ZfsCompressOption::*;
-    &[On, Off, Lzjb, Lz4, Zle, Gzip, Zstd]
-};
-
-#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
-pub enum ZfsChecksumOption {
-    #[default]
-    On,
-    Off,
-    Fletcher2,
-    Fletcher4,
-    Sha256,
-}
-
-impl fmt::Display for ZfsChecksumOption {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", format!("{self:?}").to_lowercase())
-    }
-}
-
-impl From<&ZfsChecksumOption> for String {
-    fn from(value: &ZfsChecksumOption) -> Self {
-        value.to_string()
-    }
-}
-
-pub const ZFS_CHECKSUM_OPTIONS: &[ZfsChecksumOption] = {
-    use ZfsChecksumOption::*;
-    &[On, Off, Fletcher2, Fletcher4, Sha256]
-};
-
-#[derive(Clone, Debug)]
-pub struct ZfsBootdiskOptions {
-    pub ashift: usize,
-    pub compress: ZfsCompressOption,
-    pub checksum: ZfsChecksumOption,
-    pub copies: usize,
-    pub disk_size: f64,
-    pub selected_disks: Vec<usize>,
-}
-
-impl ZfsBootdiskOptions {
-    /// This panics if the provided slice is empty.
-    pub fn defaults_from(disks: &[Disk]) -> Self {
-        let disk = &disks[0];
-        Self {
-            ashift: 12,
-            compress: ZfsCompressOption::default(),
-            checksum: ZfsChecksumOption::default(),
-            copies: 1,
-            disk_size: disk.size,
-            selected_disks: (0..disks.len()).collect(),
-        }
-    }
-}
-
-#[derive(Clone, Debug)]
-pub enum AdvancedBootdiskOptions {
-    Lvm(LvmBootdiskOptions),
-    Zfs(ZfsBootdiskOptions),
-    Btrfs(BtrfsBootdiskOptions),
-}
-
-#[derive(Clone, Debug, PartialEq)]
-pub struct Disk {
-    pub index: String,
-    pub path: String,
-    pub model: Option<String>,
-    pub size: f64,
-    pub block_size: Option<usize>,
-}
-
-impl fmt::Display for Disk {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        // TODO: Format sizes properly with `proxmox-human-byte` once merged
-        // https://lists.proxmox.com/pipermail/pbs-devel/2023-May/006125.html
-        f.write_str(&self.path)?;
-        if let Some(model) = &self.model {
-            // FIXME: ellipsize too-long names?
-            write!(f, " ({model})")?;
-        }
-        write!(f, " ({:.2} GiB)", self.size)
-    }
-}
-
-impl From<&Disk> for String {
-    fn from(value: &Disk) -> Self {
-        value.to_string()
-    }
-}
-
-impl cmp::Eq for Disk {}
-
-impl cmp::PartialOrd for Disk {
-    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
-        self.index.partial_cmp(&other.index)
-    }
-}
-
-impl cmp::Ord for Disk {
-    fn cmp(&self, other: &Self) -> cmp::Ordering {
-        self.index.cmp(&other.index)
-    }
-}
-
-#[derive(Clone, Debug)]
-pub struct BootdiskOptions {
-    pub disks: Vec<Disk>,
-    pub fstype: FsType,
-    pub advanced: AdvancedBootdiskOptions,
-}
-
-impl BootdiskOptions {
-    pub fn defaults_from(disk: &Disk) -> Self {
-        Self {
-            disks: vec![disk.clone()],
-            fstype: FsType::Ext4,
-            advanced: AdvancedBootdiskOptions::Lvm(LvmBootdiskOptions::defaults_from(disk)),
-        }
-    }
-}
-
-#[derive(Clone, Debug)]
-pub struct TimezoneOptions {
-    pub country: String,
-    pub timezone: String,
-    pub kb_layout: String,
-}
-
-impl TimezoneOptions {
-    pub fn defaults_from(runtime: &RuntimeInfo, locales: &LocaleInfo) -> Self {
-        let country = runtime.country.clone().unwrap_or_else(|| "at".to_owned());
-
-        let timezone = locales
-            .cczones
-            .get(&country)
-            .and_then(|zones| zones.get(0))
-            .cloned()
-            .unwrap_or_else(|| "UTC".to_owned());
-
-        let kb_layout = locales
-            .countries
-            .get(&country)
-            .and_then(|c| {
-                if c.kmap.is_empty() {
-                    None
-                } else {
-                    Some(c.kmap.clone())
-                }
-            })
-            .unwrap_or_else(|| "en-us".to_owned());
-
-        Self {
-            country,
-            timezone,
-            kb_layout,
-        }
-    }
-}
-
-#[derive(Clone, Debug)]
-pub struct PasswordOptions {
-    pub email: String,
-    pub root_password: String,
-}
-
-impl Default for PasswordOptions {
-    fn default() -> Self {
-        Self {
-            email: "mail@example.invalid".to_string(),
-            root_password: String::new(),
-        }
-    }
-}
-
-#[derive(Clone, Debug, PartialEq)]
-pub struct NetworkOptions {
-    pub ifname: String,
-    pub fqdn: Fqdn,
-    pub address: CidrAddress,
-    pub gateway: IpAddr,
-    pub dns_server: IpAddr,
-}
-
-impl NetworkOptions {
-    const DEFAULT_DOMAIN: &str = "example.invalid";
-
-    pub fn defaults_from(setup: &SetupInfo, network: &NetworkInfo) -> Self {
-        let mut this = Self {
-            ifname: String::new(),
-            fqdn: Self::construct_fqdn(network, setup.config.product.default_hostname()),
-            // Safety: The provided mask will always be valid.
-            address: CidrAddress::new(Ipv4Addr::UNSPECIFIED, 0).unwrap(),
-            gateway: Ipv4Addr::UNSPECIFIED.into(),
-            dns_server: Ipv4Addr::UNSPECIFIED.into(),
-        };
-
-        if let Some(ip) = network.dns.dns.first() {
-            this.dns_server = *ip;
-        }
-
-        if let Some(routes) = &network.routes {
-            let mut filled = false;
-            if let Some(gw) = &routes.gateway4 {
-                if let Some(iface) = network.interfaces.get(&gw.dev) {
-                    this.ifname = iface.name.clone();
-                    if let Some(addresses) = &iface.addresses {
-                        if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv4()) {
-                            this.gateway = gw.gateway;
-                            this.address = addr.clone();
-                            filled = true;
-                        }
-                    }
-                }
-            }
-            if !filled {
-                if let Some(gw) = &routes.gateway6 {
-                    if let Some(iface) = network.interfaces.get(&gw.dev) {
-                        if let Some(addresses) = &iface.addresses {
-                            if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv6()) {
-                                this.ifname = iface.name.clone();
-                                this.gateway = gw.gateway;
-                                this.address = addr.clone();
-                            }
-                        }
-                    }
-                }
-            }
-        }
-
-        this
-    }
-
-    fn construct_fqdn(network: &NetworkInfo, default_hostname: &str) -> Fqdn {
-        let hostname = network.hostname.as_deref().unwrap_or(default_hostname);
-
-        let domain = network
-            .dns
-            .domain
-            .as_deref()
-            .unwrap_or(Self::DEFAULT_DOMAIN);
-
-        Fqdn::from(&format!("{hostname}.{domain}")).unwrap_or_else(|_| {
-            // Safety: This will always result in a valid FQDN, as we control & know
-            // the values of default_hostname (one of "pve", "pmg" or "pbs") and
-            // constant-defined DEFAULT_DOMAIN.
-            Fqdn::from(&format!("{}.{}", default_hostname, Self::DEFAULT_DOMAIN)).unwrap()
-        })
-    }
-}
-
 #[derive(Clone, Debug)]
 pub struct InstallerOptions {
     pub bootdisk: BootdiskOptions,
@@ -447,11 +68,15 @@ impl InstallerOptions {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::setup::{
-        Dns, Gateway, Interface, InterfaceState, IsoInfo, IsoLocations, NetworkInfo, ProductConfig,
-        ProxmoxProduct, Routes, SetupInfo,
+    use proxmox_installer_common::{
+        setup::{
+            Dns, Gateway, Interface, InterfaceState, IsoInfo, IsoLocations, NetworkInfo, ProductConfig,
+            ProxmoxProduct, Routes, SetupInfo,
+        },
+        utils::{CidrAddress, Fqdn},
     };
     use std::{collections::HashMap, path::PathBuf};
+    use std::net::{IpAddr, Ipv4Addr};
 
     fn dummy_setup_info() -> SetupInfo {
         SetupInfo {
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index 5575759..211a96b 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -1,131 +1,20 @@
 use std::{
-    cmp,
     collections::HashMap,
     fmt,
     fs::File,
     io::BufReader,
     net::IpAddr,
-    path::{Path, PathBuf},
+    path::Path,
 };
 
-use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
+use serde::{Deserialize, Serialize, Serializer};
 
-use crate::{
-    options::{
-        AdvancedBootdiskOptions, BtrfsRaidLevel, Disk, FsType, InstallerOptions,
-        ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel,
-    },
-    utils::CidrAddress,
-};
-
-#[allow(clippy::upper_case_acronyms)]
-#[derive(Clone, Copy, Deserialize, PartialEq)]
-#[serde(rename_all = "lowercase")]
-pub enum ProxmoxProduct {
-    PVE,
-    PBS,
-    PMG,
-}
-
-impl ProxmoxProduct {
-    pub fn default_hostname(self) -> &'static str {
-        match self {
-            Self::PVE => "pve",
-            Self::PMG => "pmg",
-            Self::PBS => "pbs",
-        }
-    }
-}
-
-#[derive(Clone, Deserialize)]
-pub struct ProductConfig {
-    pub fullname: String,
-    pub product: ProxmoxProduct,
-    #[serde(deserialize_with = "deserialize_bool_from_int")]
-    pub enable_btrfs: bool,
-}
-
-#[derive(Clone, Deserialize)]
-pub struct IsoInfo {
-    pub release: String,
-    pub isorelease: String,
-}
-
-/// Paths in the ISO environment containing installer data.
-#[derive(Clone, Deserialize)]
-pub struct IsoLocations {
-    pub iso: PathBuf,
-}
-
-#[derive(Clone, Deserialize)]
-pub struct SetupInfo {
-    #[serde(rename = "product-cfg")]
-    pub config: ProductConfig,
-    #[serde(rename = "iso-info")]
-    pub iso_info: IsoInfo,
-    pub locations: IsoLocations,
-}
-
-#[derive(Clone, Deserialize)]
-pub struct CountryInfo {
-    pub name: String,
-    #[serde(default)]
-    pub zone: String,
-    pub kmap: String,
-}
-
-#[derive(Clone, Deserialize, Eq, PartialEq)]
-pub struct KeyboardMapping {
-    pub name: String,
-    #[serde(rename = "kvm")]
-    pub id: String,
-    #[serde(rename = "x11")]
-    pub xkb_layout: String,
-    #[serde(rename = "x11var")]
-    pub xkb_variant: String,
-}
-
-impl cmp::PartialOrd for KeyboardMapping {
-    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
-        self.name.partial_cmp(&other.name)
-    }
-}
-
-impl cmp::Ord for KeyboardMapping {
-    fn cmp(&self, other: &Self) -> cmp::Ordering {
-        self.name.cmp(&other.name)
-    }
-}
-
-#[derive(Clone, Deserialize)]
-pub struct LocaleInfo {
-    #[serde(deserialize_with = "deserialize_cczones_map")]
-    pub cczones: HashMap<String, Vec<String>>,
-    #[serde(rename = "country")]
-    pub countries: HashMap<String, CountryInfo>,
-    pub kmap: HashMap<String, KeyboardMapping>,
-}
-
-#[derive(Serialize)]
-struct InstallZfsOption {
-    ashift: usize,
-    #[serde(serialize_with = "serialize_as_display")]
-    compress: ZfsCompressOption,
-    #[serde(serialize_with = "serialize_as_display")]
-    checksum: ZfsChecksumOption,
-    copies: usize,
-}
-
-impl From<ZfsBootdiskOptions> for InstallZfsOption {
-    fn from(opts: ZfsBootdiskOptions) -> Self {
-        InstallZfsOption {
-            ashift: opts.ashift,
-            compress: opts.compress,
-            checksum: opts.checksum,
-            copies: opts.copies,
-        }
-    }
-}
+use crate::options::InstallerOptions;
+use proxmox_installer_common::{
+        options::{AdvancedBootdiskOptions, BtrfsRaidLevel, Disk, FsType, ZfsRaidLevel},
+        setup::InstallZfsOption,
+        utils::CidrAddress,
+    };
 
 /// See Proxmox::Install::Config
 #[derive(Serialize)]
@@ -247,82 +136,6 @@ pub fn read_json<T: for<'de> Deserialize<'de>, P: AsRef<Path>>(path: P) -> Resul
     serde_json::from_reader(reader).map_err(|err| format!("failed to parse JSON: {err}"))
 }
 
-fn deserialize_bool_from_int<'de, D>(deserializer: D) -> Result<bool, D::Error>
-where
-    D: Deserializer<'de>,
-{
-    let val: u32 = Deserialize::deserialize(deserializer)?;
-    Ok(val != 0)
-}
-
-fn deserialize_cczones_map<'de, D>(
-    deserializer: D,
-) -> Result<HashMap<String, Vec<String>>, D::Error>
-where
-    D: Deserializer<'de>,
-{
-    let map: HashMap<String, HashMap<String, u32>> = Deserialize::deserialize(deserializer)?;
-
-    let mut result = HashMap::new();
-    for (cc, list) in map.into_iter() {
-        result.insert(cc, list.into_keys().collect());
-    }
-
-    Ok(result)
-}
-
-fn deserialize_disks_map<'de, D>(deserializer: D) -> Result<Vec<Disk>, D::Error>
-where
-    D: Deserializer<'de>,
-{
-    let disks =
-        <Vec<(usize, String, f64, String, Option<usize>, String)>>::deserialize(deserializer)?;
-    Ok(disks
-        .into_iter()
-        .map(
-            |(index, device, size_mb, model, logical_bsize, _syspath)| Disk {
-                index: index.to_string(),
-                // Linux always reports the size of block devices in sectors, where one sector is
-                // defined as being 2^9 = 512 bytes in size.
-                // https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/blk_types.h?h=v6.4#n30
-                size: (size_mb * 512.) / 1024. / 1024. / 1024.,
-                block_size: logical_bsize,
-                path: device,
-                model: (!model.is_empty()).then_some(model),
-            },
-        )
-        .collect())
-}
-
-fn deserialize_cidr_list<'de, D>(deserializer: D) -> Result<Option<Vec<CidrAddress>>, D::Error>
-where
-    D: Deserializer<'de>,
-{
-    #[derive(Deserialize)]
-    struct CidrDescriptor {
-        address: String,
-        prefix: usize,
-        // family is implied anyway by parsing the address
-    }
-
-    let list: Vec<CidrDescriptor> = Deserialize::deserialize(deserializer)?;
-
-    let mut result = Vec::with_capacity(list.len());
-    for desc in list {
-        let ip_addr = desc
-            .address
-            .parse::<IpAddr>()
-            .map_err(|err| de::Error::custom(format!("{:?}", err)))?;
-
-        result.push(
-            CidrAddress::new(ip_addr, desc.prefix)
-                .map_err(|err| de::Error::custom(format!("{:?}", err)))?,
-        );
-    }
-
-    Ok(Some(result))
-}
-
 fn serialize_disk_opt<S>(value: &Option<Disk>, serializer: S) -> Result<S::Ok, S::Error>
 where
     S: Serializer,
@@ -366,116 +179,3 @@ where
 {
     serializer.collect_str(value)
 }
-
-#[derive(Clone, Deserialize)]
-pub struct RuntimeInfo {
-    /// Whether is system was booted in (legacy) BIOS or UEFI mode.
-    pub boot_type: BootType,
-
-    /// Detected country if available.
-    pub country: Option<String>,
-
-    /// Maps devices to their information.
-    #[serde(deserialize_with = "deserialize_disks_map")]
-    pub disks: Vec<Disk>,
-
-    /// Network addresses, gateways and DNS info.
-    pub network: NetworkInfo,
-
-    /// Total memory of the system in MiB.
-    pub total_memory: usize,
-
-    /// Whether the CPU supports hardware-accelerated virtualization
-    #[serde(deserialize_with = "deserialize_bool_from_int")]
-    pub hvm_supported: bool,
-}
-
-#[derive(Copy, Clone, Eq, Deserialize, PartialEq)]
-#[serde(rename_all = "lowercase")]
-pub enum BootType {
-    Bios,
-    Efi,
-}
-
-#[derive(Clone, Deserialize)]
-pub struct NetworkInfo {
-    pub dns: Dns,
-    pub routes: Option<Routes>,
-
-    /// Maps devices to their configuration, if it has a usable configuration.
-    /// (Contains no entries for devices with only link-local addresses.)
-    #[serde(default)]
-    pub interfaces: HashMap<String, Interface>,
-
-    /// The hostname of this machine, if set by the DHCP server.
-    pub hostname: Option<String>,
-}
-
-#[derive(Clone, Deserialize)]
-pub struct Dns {
-    pub domain: Option<String>,
-
-    /// List of stringified IP addresses.
-    #[serde(default)]
-    pub dns: Vec<IpAddr>,
-}
-
-#[derive(Clone, Deserialize)]
-pub struct Routes {
-    /// Ipv4 gateway.
-    pub gateway4: Option<Gateway>,
-
-    /// Ipv6 gateway.
-    pub gateway6: Option<Gateway>,
-}
-
-#[derive(Clone, Deserialize)]
-pub struct Gateway {
-    /// Outgoing network device.
-    pub dev: String,
-
-    /// Stringified gateway IP address.
-    pub gateway: IpAddr,
-}
-
-#[derive(Clone, Deserialize)]
-#[serde(rename_all = "UPPERCASE")]
-pub enum InterfaceState {
-    Up,
-    Down,
-    #[serde(other)]
-    Unknown,
-}
-
-impl InterfaceState {
-    // avoid display trait as this is not the string representation for a serializer
-    pub fn render(&self) -> String {
-        match self {
-            Self::Up => "\u{25CF}",
-            Self::Down | Self::Unknown => " ",
-        }
-        .into()
-    }
-}
-
-#[derive(Clone, Deserialize)]
-pub struct Interface {
-    pub name: String,
-
-    pub index: usize,
-
-    pub mac: String,
-
-    pub state: InterfaceState,
-
-    #[serde(default)]
-    #[serde(deserialize_with = "deserialize_cidr_list")]
-    pub addresses: Option<Vec<CidrAddress>>,
-}
-
-impl Interface {
-    // avoid display trait as this is not the string representation for a serializer
-    pub fn render(&self) -> String {
-        format!("{} {}", self.state.render(), self.name)
-    }
-}
diff --git a/proxmox-tui-installer/src/system.rs b/proxmox-tui-installer/src/system.rs
index bbf13b8..d1675a9 100644
--- a/proxmox-tui-installer/src/system.rs
+++ b/proxmox-tui-installer/src/system.rs
@@ -1,6 +1,6 @@
 use std::{fs::OpenOptions, io::Write, process::Command};
 
-use crate::setup::KeyboardMapping;
+use proxmox_installer_common::setup::KeyboardMapping;
 
 pub fn set_keyboard_layout(kmap: &KeyboardMapping) -> Result<(), String> {
     Command::new("setxkbmap")
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index ba08c8b..38a6521 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -1,4 +1,4 @@
-use std::{cell::RefCell, collections::HashSet, marker::PhantomData, rc::Rc};
+use std::{cell::RefCell, marker::PhantomData, rc::Rc};
 
 use cursive::{
     view::{Nameable, Resizable, ViewWrapper},
@@ -10,15 +10,18 @@ use cursive::{
 };
 
 use super::{DiskSizeEditView, FormView, IntegerEditView};
-use crate::{
+use crate::options::FS_TYPES;
+use crate::InstallerState;
+
+use proxmox_installer_common::{
     options::{
-        AdvancedBootdiskOptions, BootdiskOptions, BtrfsBootdiskOptions, BtrfsRaidLevel, Disk,
-        FsType, LvmBootdiskOptions, ZfsBootdiskOptions, ZfsRaidLevel, FS_TYPES,
+        AdvancedBootdiskOptions, BootdiskOptions, BtrfsBootdiskOptions, Disk,
+        FsType, LvmBootdiskOptions, ZfsBootdiskOptions,
         ZFS_CHECKSUM_OPTIONS, ZFS_COMPRESS_OPTIONS,
     },
-    setup::{BootType, ProductConfig},
+    setup::{BootType, ProductConfig, ProxmoxProduct},
+    disk_checks::{check_btrfs_raid_config, check_for_duplicate_disks, check_disks_4kn_legacy_boot, check_zfs_raid_config},
 };
-use crate::{setup::ProxmoxProduct, InstallerState};
 
 pub struct BootdiskOptionsView {
     view: LinearLayout,
@@ -619,236 +622,3 @@ fn advanced_options_view(
     .with_name("advanced-bootdisk-options-dialog")
     .max_size((120, 40))
 }
-
-/// Checks a list of disks for duplicate entries, using their index as key.
-///
-/// # Arguments
-///
-/// * `disks` - A list of disks to check for duplicates.
-fn check_for_duplicate_disks(disks: &[Disk]) -> Result<(), &Disk> {
-    let mut set = HashSet::new();
-
-    for disk in disks {
-        if !set.insert(&disk.index) {
-            return Err(disk);
-        }
-    }
-
-    Ok(())
-}
-
-/// Simple wrapper which returns an descriptive error if the list of disks is too short.
-///
-/// # Arguments
-///
-/// * `disks` - A list of disks to check the lenght of.
-/// * `min` - Minimum number of disks
-fn check_raid_min_disks(disks: &[Disk], min: usize) -> Result<(), String> {
-    if disks.len() < min {
-        Err(format!("Need at least {min} disks"))
-    } else {
-        Ok(())
-    }
-}
-
-/// Checks all disks for legacy BIOS boot compatibility and reports an error as appropriate. 4Kn
-/// disks are generally broken with legacy BIOS and cannot be booted from.
-///
-/// # Arguments
-///
-/// * `runinfo` - `RuntimeInfo` instance of currently running system
-/// * `disks` - List of disks designated as bootdisk targets.
-fn check_disks_4kn_legacy_boot(boot_type: BootType, disks: &[Disk]) -> Result<(), &str> {
-    let is_blocksize_4096 = |disk: &Disk| disk.block_size.map(|s| s == 4096).unwrap_or(false);
-
-    if boot_type == BootType::Bios && disks.iter().any(is_blocksize_4096) {
-        return Err("Booting from 4Kn drive in legacy BIOS mode is not supported.");
-    }
-
-    Ok(())
-}
-
-/// Checks whether a user-supplied ZFS RAID setup is valid or not, such as disk sizes andminimum
-/// number of disks.
-///
-/// # Arguments
-///
-/// * `level` - The targeted ZFS RAID level by the user.
-/// * `disks` - List of disks designated as RAID targets.
-fn check_zfs_raid_config(level: ZfsRaidLevel, disks: &[Disk]) -> Result<(), String> {
-    // See also Proxmox/Install.pm:get_zfs_raid_setup()
-
-    let check_mirror_size = |disk1: &Disk, disk2: &Disk| {
-        if (disk1.size - disk2.size).abs() > disk1.size / 10. {
-            Err(format!(
-                "Mirrored disks must have same size:\n\n  * {disk1}\n  * {disk2}"
-            ))
-        } else {
-            Ok(())
-        }
-    };
-
-    match level {
-        ZfsRaidLevel::Raid0 => check_raid_min_disks(disks, 1)?,
-        ZfsRaidLevel::Raid1 => {
-            check_raid_min_disks(disks, 2)?;
-            for disk in disks {
-                check_mirror_size(&disks[0], disk)?;
-            }
-        }
-        ZfsRaidLevel::Raid10 => {
-            check_raid_min_disks(disks, 4)?;
-            // Pairs need to have the same size
-            for i in (0..disks.len()).step_by(2) {
-                check_mirror_size(&disks[i], &disks[i + 1])?;
-            }
-        }
-        // For RAID-Z: minimum disks number is level + 2
-        ZfsRaidLevel::RaidZ => {
-            check_raid_min_disks(disks, 3)?;
-            for disk in disks {
-                check_mirror_size(&disks[0], disk)?;
-            }
-        }
-        ZfsRaidLevel::RaidZ2 => {
-            check_raid_min_disks(disks, 4)?;
-            for disk in disks {
-                check_mirror_size(&disks[0], disk)?;
-            }
-        }
-        ZfsRaidLevel::RaidZ3 => {
-            check_raid_min_disks(disks, 5)?;
-            for disk in disks {
-                check_mirror_size(&disks[0], disk)?;
-            }
-        }
-    }
-
-    Ok(())
-}
-
-/// Checks whether a user-supplied Btrfs RAID setup is valid or not, such as minimum
-/// number of disks.
-///
-/// # Arguments
-///
-/// * `level` - The targeted Btrfs RAID level by the user.
-/// * `disks` - List of disks designated as RAID targets.
-fn check_btrfs_raid_config(level: BtrfsRaidLevel, disks: &[Disk]) -> Result<(), String> {
-    // See also Proxmox/Install.pm:get_btrfs_raid_setup()
-
-    match level {
-        BtrfsRaidLevel::Raid0 => check_raid_min_disks(disks, 1)?,
-        BtrfsRaidLevel::Raid1 => check_raid_min_disks(disks, 2)?,
-        BtrfsRaidLevel::Raid10 => check_raid_min_disks(disks, 4)?,
-    }
-
-    Ok(())
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    fn dummy_disk(index: usize) -> Disk {
-        Disk {
-            index: index.to_string(),
-            path: format!("/dev/dummy{index}"),
-            model: Some("Dummy disk".to_owned()),
-            size: 1024. * 1024. * 1024. * 8.,
-            block_size: Some(512),
-        }
-    }
-
-    fn dummy_disks(num: usize) -> Vec<Disk> {
-        (0..num).map(dummy_disk).collect()
-    }
-
-    #[test]
-    fn duplicate_disks() {
-        assert!(check_for_duplicate_disks(&dummy_disks(2)).is_ok());
-        assert_eq!(
-            check_for_duplicate_disks(&[
-                dummy_disk(0),
-                dummy_disk(1),
-                dummy_disk(2),
-                dummy_disk(2),
-                dummy_disk(3),
-            ]),
-            Err(&dummy_disk(2)),
-        );
-    }
-
-    #[test]
-    fn raid_min_disks() {
-        let disks = dummy_disks(10);
-
-        assert!(check_raid_min_disks(&disks[..1], 2).is_err());
-        assert!(check_raid_min_disks(&disks[..1], 1).is_ok());
-        assert!(check_raid_min_disks(&disks, 1).is_ok());
-    }
-
-    #[test]
-    fn bios_boot_compat_4kn() {
-        for i in 0..10 {
-            let mut disks = dummy_disks(10);
-            disks[i].block_size = Some(4096);
-
-            // Must fail if /any/ of the disks are 4Kn
-            assert!(check_disks_4kn_legacy_boot(BootType::Bios, &disks).is_err());
-            // For UEFI, we allow it for every configuration
-            assert!(check_disks_4kn_legacy_boot(BootType::Efi, &disks).is_ok());
-        }
-    }
-
-    #[test]
-    fn btrfs_raid() {
-        let disks = dummy_disks(10);
-
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid0, &[]).is_err());
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid0, &disks[..1]).is_ok());
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid0, &disks).is_ok());
-
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &[]).is_err());
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks[..1]).is_err());
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks[..2]).is_ok());
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks).is_ok());
-
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &[]).is_err());
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..3]).is_err());
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..4]).is_ok());
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks).is_ok());
-    }
-
-    #[test]
-    fn zfs_raid() {
-        let disks = dummy_disks(10);
-
-        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid0, &[]).is_err());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid0, &disks[..1]).is_ok());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid0, &disks).is_ok());
-
-        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid1, &[]).is_err());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid1, &disks[..2]).is_ok());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid1, &disks).is_ok());
-
-        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid10, &[]).is_err());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid10, &dummy_disks(4)).is_ok());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid10, &disks).is_ok());
-
-        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &[]).is_err());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &disks[..2]).is_err());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &disks[..3]).is_ok());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &disks).is_ok());
-
-        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &[]).is_err());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &disks[..3]).is_err());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &disks[..4]).is_ok());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &disks).is_ok());
-
-        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &[]).is_err());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &disks[..4]).is_err());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &disks[..5]).is_ok());
-        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &disks).is_ok());
-    }
-}
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index aa24fa4..aabae0e 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -7,7 +7,7 @@ use cursive::{
     Rect, Vec2, View,
 };
 
-use crate::utils::CidrAddress;
+use proxmox_installer_common::utils::CidrAddress;
 
 mod bootdisk;
 pub use bootdisk::*;
diff --git a/proxmox-tui-installer/src/views/timezone.rs b/proxmox-tui-installer/src/views/timezone.rs
index 6732286..77fbb10 100644
--- a/proxmox-tui-installer/src/views/timezone.rs
+++ b/proxmox-tui-installer/src/views/timezone.rs
@@ -6,9 +6,11 @@ use cursive::{
 
 use super::FormView;
 use crate::{
+    system, InstallerState,
+};
+use proxmox_installer_common::{
     options::TimezoneOptions,
     setup::{KeyboardMapping, LocaleInfo},
-    system, InstallerState,
 };
 
 pub struct TimezoneOptionsView {
-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pve-devel] [PATCH 08/12] tui: remove now unused utils.rs
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
                   ` (6 preceding siblings ...)
  2023-10-25 16:00 ` [pve-devel] [PATCH 07/12] tui: switch to " Aaron Lauterer
@ 2023-10-25 16:00 ` Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 09/12] common: add installer_setup method Aaron Lauterer
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 16:00 UTC (permalink / raw)
  To: pve-devel

all it did moved to the common crate

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-tui-installer/src/utils.rs | 268 -----------------------------
 1 file changed, 268 deletions(-)
 delete mode 100644 proxmox-tui-installer/src/utils.rs

diff --git a/proxmox-tui-installer/src/utils.rs b/proxmox-tui-installer/src/utils.rs
deleted file mode 100644
index 89349ed..0000000
--- a/proxmox-tui-installer/src/utils.rs
+++ /dev/null
@@ -1,268 +0,0 @@
-use std::{
-    fmt,
-    net::{AddrParseError, IpAddr},
-    num::ParseIntError,
-    str::FromStr,
-};
-
-use serde::Deserialize;
-
-/// Possible errors that might occur when parsing CIDR addresses.
-#[derive(Debug)]
-pub enum CidrAddressParseError {
-    /// No delimiter for separating address and mask was found.
-    NoDelimiter,
-    /// The IP address part could not be parsed.
-    InvalidAddr(AddrParseError),
-    /// The mask could not be parsed.
-    InvalidMask(Option<ParseIntError>),
-}
-
-/// An IP address (IPv4 or IPv6), including network mask.
-///
-/// See the [`IpAddr`] type for more information how IP addresses are handled.
-/// The mask is appropriately enforced to be `0 <= mask <= 32` for IPv4 or
-/// `0 <= mask <= 128` for IPv6 addresses.
-///
-/// # Examples
-/// ```
-/// use std::net::{Ipv4Addr, Ipv6Addr};
-/// let ipv4 = CidrAddress::new(Ipv4Addr::new(192, 168, 0, 1), 24).unwrap();
-/// let ipv6 = CidrAddress::new(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0xc0a8, 1), 32).unwrap();
-///
-/// assert_eq!(ipv4.to_string(), "192.168.0.1/24");
-/// assert_eq!(ipv6.to_string(), "2001:db8::c0a8:1/32");
-/// ```
-#[derive(Clone, Debug, PartialEq)]
-pub struct CidrAddress {
-    addr: IpAddr,
-    mask: usize,
-}
-
-impl CidrAddress {
-    /// Constructs a new CIDR address.
-    ///
-    /// It fails if the mask is invalid for the given IP address.
-    pub fn new<T: Into<IpAddr>>(addr: T, mask: usize) -> Result<Self, CidrAddressParseError> {
-        let addr = addr.into();
-
-        if mask > mask_limit(&addr) {
-            Err(CidrAddressParseError::InvalidMask(None))
-        } else {
-            Ok(Self { addr, mask })
-        }
-    }
-
-    /// Returns only the IP address part of the address.
-    pub fn addr(&self) -> IpAddr {
-        self.addr
-    }
-
-    /// Returns `true` if this address is an IPv4 address, `false` otherwise.
-    pub fn is_ipv4(&self) -> bool {
-        self.addr.is_ipv4()
-    }
-
-    /// Returns `true` if this address is an IPv6 address, `false` otherwise.
-    pub fn is_ipv6(&self) -> bool {
-        self.addr.is_ipv6()
-    }
-
-    /// Returns only the mask part of the address.
-    pub fn mask(&self) -> usize {
-        self.mask
-    }
-}
-
-impl FromStr for CidrAddress {
-    type Err = CidrAddressParseError;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let (addr, mask) = s
-            .split_once('/')
-            .ok_or(CidrAddressParseError::NoDelimiter)?;
-
-        let addr = addr.parse().map_err(CidrAddressParseError::InvalidAddr)?;
-
-        let mask = mask
-            .parse()
-            .map_err(|err| CidrAddressParseError::InvalidMask(Some(err)))?;
-
-        if mask > mask_limit(&addr) {
-            Err(CidrAddressParseError::InvalidMask(None))
-        } else {
-            Ok(Self { addr, mask })
-        }
-    }
-}
-
-impl fmt::Display for CidrAddress {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}/{}", self.addr, self.mask)
-    }
-}
-
-fn mask_limit(addr: &IpAddr) -> usize {
-    if addr.is_ipv4() {
-        32
-    } else {
-        128
-    }
-}
-
-/// Possible errors that might occur when parsing FQDNs.
-#[derive(Debug, Eq, PartialEq)]
-pub enum FqdnParseError {
-    MissingHostname,
-    NumericHostname,
-    InvalidPart(String),
-}
-
-impl fmt::Display for FqdnParseError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        use FqdnParseError::*;
-        match self {
-            MissingHostname => write!(f, "missing hostname part"),
-            NumericHostname => write!(f, "hostname cannot be purely numeric"),
-            InvalidPart(part) => write!(
-                f,
-                "FQDN must only consist of alphanumeric characters and dashes. Invalid part: '{part}'",
-            ),
-        }
-    }
-}
-
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub struct Fqdn {
-    parts: Vec<String>,
-}
-
-impl Fqdn {
-    pub fn from(fqdn: &str) -> Result<Self, FqdnParseError> {
-        let parts = fqdn
-            .split('.')
-            .map(ToOwned::to_owned)
-            .collect::<Vec<String>>();
-
-        for part in &parts {
-            if !Self::validate_single(part) {
-                return Err(FqdnParseError::InvalidPart(part.clone()));
-            }
-        }
-
-        if parts.len() < 2 {
-            Err(FqdnParseError::MissingHostname)
-        } else if parts[0].chars().all(|c| c.is_ascii_digit()) {
-            // Not allowed/supported on Debian systems.
-            Err(FqdnParseError::NumericHostname)
-        } else {
-            Ok(Self { parts })
-        }
-    }
-
-    pub fn host(&self) -> Option<&str> {
-        self.has_host().then_some(&self.parts[0])
-    }
-
-    pub fn domain(&self) -> String {
-        let parts = if self.has_host() {
-            &self.parts[1..]
-        } else {
-            &self.parts
-        };
-
-        parts.join(".")
-    }
-
-    /// Checks whether the FQDN has a hostname associated with it, i.e. is has more than 1 part.
-    fn has_host(&self) -> bool {
-        self.parts.len() > 1
-    }
-
-    fn validate_single(s: &String) -> bool {
-        !s.is_empty()
-            // First character must be alphanumeric
-            && s.chars()
-                .next()
-                .map(|c| c.is_ascii_alphanumeric())
-                .unwrap_or_default()
-            // .. last character as well,
-            && s.chars()
-                .last()
-                .map(|c| c.is_ascii_alphanumeric())
-                .unwrap_or_default()
-            // and anything between must be alphanumeric or -
-            && s.chars()
-                .skip(1)
-                .take(s.len().saturating_sub(2))
-                .all(|c| c.is_ascii_alphanumeric() || c == '-')
-    }
-}
-
-impl FromStr for Fqdn {
-    type Err = FqdnParseError;
-
-    fn from_str(value: &str) -> Result<Self, Self::Err> {
-        Self::from(value)
-    }
-}
-
-impl fmt::Display for Fqdn {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", self.parts.join("."))
-    }
-}
-
-impl<'de> Deserialize<'de> for Fqdn {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de>,
-    {
-        let s: String = Deserialize::deserialize(deserializer)?;
-        s.parse()
-            .map_err(|_| serde::de::Error::custom("invalid FQDN"))
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn fqdn_construct() {
-        use FqdnParseError::*;
-        assert!(Fqdn::from("foo.example.com").is_ok());
-        assert!(Fqdn::from("foo-bar.com").is_ok());
-        assert!(Fqdn::from("a-b.com").is_ok());
-
-        assert_eq!(Fqdn::from("foo"), Err(MissingHostname));
-
-        assert_eq!(Fqdn::from("-foo.com"), Err(InvalidPart("-foo".to_owned())));
-        assert_eq!(Fqdn::from("foo-.com"), Err(InvalidPart("foo-".to_owned())));
-        assert_eq!(Fqdn::from("foo.com-"), Err(InvalidPart("com-".to_owned())));
-        assert_eq!(Fqdn::from("-o-.com"), Err(InvalidPart("-o-".to_owned())));
-
-        assert_eq!(Fqdn::from("123.com"), Err(NumericHostname));
-        assert!(Fqdn::from("foo123.com").is_ok());
-        assert!(Fqdn::from("123foo.com").is_ok());
-    }
-
-    #[test]
-    fn fqdn_parts() {
-        let fqdn = Fqdn::from("pve.example.com").unwrap();
-        assert_eq!(fqdn.host().unwrap(), "pve");
-        assert_eq!(fqdn.domain(), "example.com");
-        assert_eq!(
-            fqdn.parts,
-            &["pve".to_owned(), "example".to_owned(), "com".to_owned()]
-        );
-    }
-
-    #[test]
-    fn fqdn_display() {
-        assert_eq!(
-            Fqdn::from("foo.example.com").unwrap().to_string(),
-            "foo.example.com"
-        );
-    }
-}
-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pve-devel] [PATCH 09/12] common: add installer_setup method
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
                   ` (7 preceding siblings ...)
  2023-10-25 16:00 ` [pve-devel] [PATCH 08/12] tui: remove now unused utils.rs Aaron Lauterer
@ 2023-10-25 16:00 ` Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 10/12] common: document " Aaron Lauterer
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 16:00 UTC (permalink / raw)
  To: pve-devel

moved over from the TUI installer

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-installer-common/src/setup.rs | 37 +++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index a55f059..34b00cb 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -103,6 +103,43 @@ pub struct LocaleInfo {
     pub kmap: HashMap<String, KeyboardMapping>,
 }
 
+pub fn installer_setup(in_test_mode: bool) -> Result<(SetupInfo, LocaleInfo, RuntimeInfo), String> {
+    let base_path = if in_test_mode { "./testdir" } else { "/" };
+    let mut path = PathBuf::from(base_path);
+
+    path.push("run");
+    path.push("proxmox-installer");
+
+    let installer_info: SetupInfo = {
+        let mut path = path.clone();
+        path.push("iso-info.json");
+
+        read_json(&path).map_err(|err| format!("Failed to retrieve setup info: {err}"))?
+    };
+
+    let locale_info = {
+        let mut path = path.clone();
+        path.push("locales.json");
+
+        read_json(&path).map_err(|err| format!("Failed to retrieve locale info: {err}"))?
+    };
+
+    let mut runtime_info: RuntimeInfo = {
+        let mut path = path.clone();
+        path.push("run-env-info.json");
+
+        read_json(&path)
+            .map_err(|err| format!("Failed to retrieve runtime environment info: {err}"))?
+    };
+
+    runtime_info.disks.sort();
+    if runtime_info.disks.is_empty() {
+        Err("The installer could not find any supported hard disks.".to_owned())
+    } else {
+        Ok((installer_info, locale_info, runtime_info))
+    }
+}
+
 #[derive(Serialize)]
 pub struct InstallZfsOption {
     ashift: usize,
-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pve-devel] [PATCH 10/12] common: document installer_setup method
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
                   ` (8 preceding siblings ...)
  2023-10-25 16:00 ` [pve-devel] [PATCH 09/12] common: add installer_setup method Aaron Lauterer
@ 2023-10-25 16:00 ` Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 11/12] tui: use installer_setup from common cate Aaron Lauterer
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 16:00 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-installer-common/src/setup.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 34b00cb..3ef05ae 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -103,6 +103,7 @@ pub struct LocaleInfo {
     pub kmap: HashMap<String, KeyboardMapping>,
 }
 
+/// Fetches basic information needed for the installer which is required to work
 pub fn installer_setup(in_test_mode: bool) -> Result<(SetupInfo, LocaleInfo, RuntimeInfo), String> {
     let base_path = if in_test_mode { "./testdir" } else { "/" };
     let mut path = PathBuf::from(base_path);
-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pve-devel] [PATCH 11/12] tui: use installer_setup from common cate
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
                   ` (9 preceding siblings ...)
  2023-10-25 16:00 ` [pve-devel] [PATCH 10/12] common: document " Aaron Lauterer
@ 2023-10-25 16:00 ` Aaron Lauterer
  2023-10-25 16:00 ` [pve-devel] [PATCH 12/12] tui: remove unused read_json function Aaron Lauterer
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 16:00 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-tui-installer/src/main.rs | 40 +------------------------------
 1 file changed, 1 insertion(+), 39 deletions(-)

diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 875a33a..3216868 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -5,7 +5,6 @@ use std::{
     env,
     io::{BufRead, BufReader, Write},
     net::IpAddr,
-    path::PathBuf,
     str::FromStr,
     sync::{Arc, Mutex},
     thread,
@@ -32,7 +31,7 @@ use options::InstallerOptions;
 
 use proxmox_installer_common::{
     options::{BootdiskOptions, NetworkOptions, PasswordOptions, TimezoneOptions},
-    setup::{LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo},
+    setup::{installer_setup, LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo},
     utils::Fqdn,
 };
 
@@ -197,43 +196,6 @@ fn main() {
     siv.run();
 }
 
-fn installer_setup(in_test_mode: bool) -> Result<(SetupInfo, LocaleInfo, RuntimeInfo), String> {
-    let base_path = if in_test_mode { "./testdir" } else { "/" };
-    let mut path = PathBuf::from(base_path);
-
-    path.push("run");
-    path.push("proxmox-installer");
-
-    let installer_info: SetupInfo = {
-        let mut path = path.clone();
-        path.push("iso-info.json");
-
-        setup::read_json(&path).map_err(|err| format!("Failed to retrieve setup info: {err}"))?
-    };
-
-    let locale_info = {
-        let mut path = path.clone();
-        path.push("locales.json");
-
-        setup::read_json(&path).map_err(|err| format!("Failed to retrieve locale info: {err}"))?
-    };
-
-    let mut runtime_info: RuntimeInfo = {
-        let mut path = path.clone();
-        path.push("run-env-info.json");
-
-        setup::read_json(&path)
-            .map_err(|err| format!("Failed to retrieve runtime environment info: {err}"))?
-    };
-
-    runtime_info.disks.sort();
-    if runtime_info.disks.is_empty() {
-        Err("The installer could not find any supported hard disks.".to_owned())
-    } else {
-        Ok((installer_info, locale_info, runtime_info))
-    }
-}
-
 /// Anything that can be done late in the setup and will not result in fatal errors.
 fn installer_setup_late(siv: &mut Cursive) {
     let state = siv.user_data::<InstallerState>().cloned().unwrap();
-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pve-devel] [PATCH 12/12] tui: remove unused read_json function
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
                   ` (10 preceding siblings ...)
  2023-10-25 16:00 ` [pve-devel] [PATCH 11/12] tui: use installer_setup from common cate Aaron Lauterer
@ 2023-10-25 16:00 ` Aaron Lauterer
  2023-10-27 11:06   ` Christoph Heiss
  2023-10-27 11:39 ` [pve-devel] [PATCH installer] buildsys: copy over `proxmox-installer-common` crate to build directory Christoph Heiss
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-25 16:00 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-tui-installer/src/setup.rs | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index 211a96b..efcabed 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -1,13 +1,10 @@
 use std::{
     collections::HashMap,
     fmt,
-    fs::File,
-    io::BufReader,
     net::IpAddr,
-    path::Path,
 };
 
-use serde::{Deserialize, Serialize, Serializer};
+use serde::{Serialize, Serializer};
 
 use crate::options::InstallerOptions;
 use proxmox_installer_common::{
@@ -129,13 +126,6 @@ impl From<InstallerOptions> for InstallConfig {
     }
 }
 
-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);
-
-    serde_json::from_reader(reader).map_err(|err| format!("failed to parse JSON: {err}"))
-}
-
 fn serialize_disk_opt<S>(value: &Option<Disk>, serializer: S) -> Result<S::Ok, S::Error>
 where
     S: Serializer,
-- 
2.39.2





^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [pve-devel] [PATCH 01/12] add proxmox-installer-common crate
  2023-10-25 16:00 ` [pve-devel] [PATCH 01/12] add proxmox-installer-common crate Aaron Lauterer
@ 2023-10-27 10:59   ` Christoph Heiss
  0 siblings, 0 replies; 20+ messages in thread
From: Christoph Heiss @ 2023-10-27 10:59 UTC (permalink / raw)
  To: Aaron Lauterer; +Cc: Proxmox VE development discussion


The .deb fails to build with this patch applied,
`proxmox-installer-common/` must also be copied to the build directory
(see `$(BUILDIR)` target in the Makefile).

On Wed, Oct 25, 2023 at 06:00:00PM +0200, Aaron Lauterer wrote:
>
> It will be used for code shared among the different crates in the
> installer. For now between the TUI installer and the upcoming auto
> installer.
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  Cargo.toml                          |  1 +
>  proxmox-installer-common/Cargo.toml | 10 ++++++++++
>  proxmox-installer-common/src/lib.rs |  0
>  3 files changed, 11 insertions(+)
>  create mode 100644 proxmox-installer-common/Cargo.toml
>  create mode 100644 proxmox-installer-common/src/lib.rs
>
> diff --git a/Cargo.toml b/Cargo.toml
> index fd151ba..c1bd578 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -1,5 +1,6 @@
>  [workspace]
>  members = [
> +    "proxmox-installer-common",
>      "proxmox-tui-installer",
>  ]
>
> diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml
> new file mode 100644
> index 0000000..b8762e8
> --- /dev/null
> +++ b/proxmox-installer-common/Cargo.toml
> @@ -0,0 +1,10 @@
> +[package]
> +name = "proxmox-installer-common"
> +version = "0.1.0"
> +edition = "2021"
> +authors = [ "Aaron Lauterer <a.lauterer@proxmox.com>" ]
> +license = "AGPL-3"
> +exclude = [ "build", "debian" ]
> +homepage = "https://www.proxmox.com"
> +
> +[dependencies]
> diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
> new file mode 100644
> index 0000000..e69de29
> --
> 2.39.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] 20+ messages in thread

* Re: [pve-devel] [PATCH 12/12] tui: remove unused read_json function
  2023-10-25 16:00 ` [pve-devel] [PATCH 12/12] tui: remove unused read_json function Aaron Lauterer
@ 2023-10-27 11:06   ` Christoph Heiss
  0 siblings, 0 replies; 20+ messages in thread
From: Christoph Heiss @ 2023-10-27 11:06 UTC (permalink / raw)
  To: Aaron Lauterer; +Cc: Proxmox VE development discussion


Could be squased into the previous patch IMO if you do send a v2, but
not a blocker either for me if not.

In any case:

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>

On Wed, Oct 25, 2023 at 06:00:11PM +0200, Aaron Lauterer wrote:
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  proxmox-tui-installer/src/setup.rs | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
> index 211a96b..efcabed 100644
> --- a/proxmox-tui-installer/src/setup.rs
> +++ b/proxmox-tui-installer/src/setup.rs
> @@ -1,13 +1,10 @@
>  use std::{
>      collections::HashMap,
>      fmt,
> -    fs::File,
> -    io::BufReader,
>      net::IpAddr,
> -    path::Path,
>  };
>
> -use serde::{Deserialize, Serialize, Serializer};
> +use serde::{Serialize, Serializer};
>
>  use crate::options::InstallerOptions;
>  use proxmox_installer_common::{
> @@ -129,13 +126,6 @@ impl From<InstallerOptions> for InstallConfig {
>      }
>  }
>
> -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);
> -
> -    serde_json::from_reader(reader).map_err(|err| format!("failed to parse JSON: {err}"))
> -}
> -
>  fn serialize_disk_opt<S>(value: &Option<Disk>, serializer: S) -> Result<S::Ok, S::Error>
>  where
>      S: Serializer,
> --
> 2.39.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] 20+ messages in thread

* Re: [pve-devel] [PATCH 02/12] common: copy common code from tui-installer
  2023-10-25 16:00 ` [pve-devel] [PATCH 02/12] common: copy common code from tui-installer Aaron Lauterer
@ 2023-10-27 11:14   ` Christoph Heiss
  0 siblings, 0 replies; 20+ messages in thread
From: Christoph Heiss @ 2023-10-27 11:14 UTC (permalink / raw)
  To: Aaron Lauterer; +Cc: Proxmox VE development discussion


Simple code move/copy, so LGTM.

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>

On Wed, Oct 25, 2023 at 06:00:01PM +0200, Aaron Lauterer wrote:
>
> Copy code that is common to its own crate.
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  proxmox-installer-common/Cargo.toml         |   2 +
>  proxmox-installer-common/src/disk_checks.rs | 237 ++++++++++++
>  proxmox-installer-common/src/lib.rs         |   4 +
>  proxmox-installer-common/src/options.rs     | 387 ++++++++++++++++++++
>  proxmox-installer-common/src/setup.rs       | 330 +++++++++++++++++
>  proxmox-installer-common/src/utils.rs       | 268 ++++++++++++++
>  6 files changed, 1228 insertions(+)
>  create mode 100644 proxmox-installer-common/src/disk_checks.rs
>  create mode 100644 proxmox-installer-common/src/options.rs
>  create mode 100644 proxmox-installer-common/src/setup.rs
>  create mode 100644 proxmox-installer-common/src/utils.rs
>
> diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml
> index b8762e8..bde5457 100644
> --- a/proxmox-installer-common/Cargo.toml
> +++ b/proxmox-installer-common/Cargo.toml
> @@ -8,3 +8,5 @@ exclude = [ "build", "debian" ]
>  homepage = "https://www.proxmox.com"
>
>  [dependencies]
> +serde = { version = "1.0", features = ["derive"] }
> +serde_json = "1.0"
> diff --git a/proxmox-installer-common/src/disk_checks.rs b/proxmox-installer-common/src/disk_checks.rs
> new file mode 100644
> index 0000000..15b5928
> --- /dev/null
> +++ b/proxmox-installer-common/src/disk_checks.rs
> @@ -0,0 +1,237 @@
> +use std::collections::HashSet;
> +
> +use crate::options::{BtrfsRaidLevel, Disk, ZfsRaidLevel};
> +use crate::setup::BootType;
> +
> +/// Checks a list of disks for duplicate entries, using their index as key.
> +///
> +/// # Arguments
> +///
> +/// * `disks` - A list of disks to check for duplicates.
> +fn check_for_duplicate_disks(disks: &[Disk]) -> Result<(), &Disk> {
> +    let mut set = HashSet::new();
> +
> +    for disk in disks {
> +        if !set.insert(&disk.index) {
> +            return Err(disk);
> +        }
> +    }
> +
> +    Ok(())
> +}
> +
> +/// Simple wrapper which returns an descriptive error if the list of disks is too short.
> +///
> +/// # Arguments
> +///
> +/// * `disks` - A list of disks to check the lenght of.
> +/// * `min` - Minimum number of disks
> +fn check_raid_min_disks(disks: &[Disk], min: usize) -> Result<(), String> {
> +    if disks.len() < min {
> +        Err(format!("Need at least {min} disks"))
> +    } else {
> +        Ok(())
> +    }
> +}
> +
> +/// Checks all disks for legacy BIOS boot compatibility and reports an error as appropriate. 4Kn
> +/// disks are generally broken with legacy BIOS and cannot be booted from.
> +///
> +/// # Arguments
> +///
> +/// * `runinfo` - `RuntimeInfo` instance of currently running system
> +/// * `disks` - List of disks designated as bootdisk targets.
> +fn check_disks_4kn_legacy_boot(boot_type: BootType, disks: &[Disk]) -> Result<(), &str> {
> +    let is_blocksize_4096 = |disk: &Disk| disk.block_size.map(|s| s == 4096).unwrap_or(false);
> +
> +    if boot_type == BootType::Bios && disks.iter().any(is_blocksize_4096) {
> +        return Err("Booting from 4Kn drive in legacy BIOS mode is not supported.");
> +    }
> +
> +    Ok(())
> +}
> +
> +/// Checks whether a user-supplied ZFS RAID setup is valid or not, such as disk sizes andminimum
> +/// number of disks.
> +///
> +/// # Arguments
> +///
> +/// * `level` - The targeted ZFS RAID level by the user.
> +/// * `disks` - List of disks designated as RAID targets.
> +fn check_zfs_raid_config(level: ZfsRaidLevel, disks: &[Disk]) -> Result<(), String> {
> +    // See also Proxmox/Install.pm:get_zfs_raid_setup()
> +
> +    let check_mirror_size = |disk1: &Disk, disk2: &Disk| {
> +        if (disk1.size - disk2.size).abs() > disk1.size / 10. {
> +            Err(format!(
> +                "Mirrored disks must have same size:\n\n  * {disk1}\n  * {disk2}"
> +            ))
> +        } else {
> +            Ok(())
> +        }
> +    };
> +
> +    match level {
> +        ZfsRaidLevel::Raid0 => check_raid_min_disks(disks, 1)?,
> +        ZfsRaidLevel::Raid1 => {
> +            check_raid_min_disks(disks, 2)?;
> +            for disk in disks {
> +                check_mirror_size(&disks[0], disk)?;
> +            }
> +        }
> +        ZfsRaidLevel::Raid10 => {
> +            check_raid_min_disks(disks, 4)?;
> +            // Pairs need to have the same size
> +            for i in (0..disks.len()).step_by(2) {
> +                check_mirror_size(&disks[i], &disks[i + 1])?;
> +            }
> +        }
> +        // For RAID-Z: minimum disks number is level + 2
> +        ZfsRaidLevel::RaidZ => {
> +            check_raid_min_disks(disks, 3)?;
> +            for disk in disks {
> +                check_mirror_size(&disks[0], disk)?;
> +            }
> +        }
> +        ZfsRaidLevel::RaidZ2 => {
> +            check_raid_min_disks(disks, 4)?;
> +            for disk in disks {
> +                check_mirror_size(&disks[0], disk)?;
> +            }
> +        }
> +        ZfsRaidLevel::RaidZ3 => {
> +            check_raid_min_disks(disks, 5)?;
> +            for disk in disks {
> +                check_mirror_size(&disks[0], disk)?;
> +            }
> +        }
> +    }
> +
> +    Ok(())
> +}
> +
> +/// Checks whether a user-supplied Btrfs RAID setup is valid or not, such as minimum
> +/// number of disks.
> +///
> +/// # Arguments
> +///
> +/// * `level` - The targeted Btrfs RAID level by the user.
> +/// * `disks` - List of disks designated as RAID targets.
> +fn check_btrfs_raid_config(level: BtrfsRaidLevel, disks: &[Disk]) -> Result<(), String> {
> +    // See also Proxmox/Install.pm:get_btrfs_raid_setup()
> +
> +    match level {
> +        BtrfsRaidLevel::Raid0 => check_raid_min_disks(disks, 1)?,
> +        BtrfsRaidLevel::Raid1 => check_raid_min_disks(disks, 2)?,
> +        BtrfsRaidLevel::Raid10 => check_raid_min_disks(disks, 4)?,
> +    }
> +
> +    Ok(())
> +}
> +
> +#[cfg(test)]
> +mod tests {
> +    use super::*;
> +
> +    fn dummy_disk(index: usize) -> Disk {
> +        Disk {
> +            index: index.to_string(),
> +            path: format!("/dev/dummy{index}"),
> +            model: Some("Dummy disk".to_owned()),
> +            size: 1024. * 1024. * 1024. * 8.,
> +            block_size: Some(512),
> +        }
> +    }
> +
> +    fn dummy_disks(num: usize) -> Vec<Disk> {
> +        (0..num).map(dummy_disk).collect()
> +    }
> +
> +    #[test]
> +    fn duplicate_disks() {
> +        assert!(check_for_duplicate_disks(&dummy_disks(2)).is_ok());
> +        assert_eq!(
> +            check_for_duplicate_disks(&[
> +                dummy_disk(0),
> +                dummy_disk(1),
> +                dummy_disk(2),
> +                dummy_disk(2),
> +                dummy_disk(3),
> +            ]),
> +            Err(&dummy_disk(2)),
> +        );
> +    }
> +
> +    #[test]
> +    fn raid_min_disks() {
> +        let disks = dummy_disks(10);
> +
> +        assert!(check_raid_min_disks(&disks[..1], 2).is_err());
> +        assert!(check_raid_min_disks(&disks[..1], 1).is_ok());
> +        assert!(check_raid_min_disks(&disks, 1).is_ok());
> +    }
> +
> +    #[test]
> +    fn bios_boot_compat_4kn() {
> +        for i in 0..10 {
> +            let mut disks = dummy_disks(10);
> +            disks[i].block_size = Some(4096);
> +
> +            // Must fail if /any/ of the disks are 4Kn
> +            assert!(check_disks_4kn_legacy_boot(BootType::Bios, &disks).is_err());
> +            // For UEFI, we allow it for every configuration
> +            assert!(check_disks_4kn_legacy_boot(BootType::Efi, &disks).is_ok());
> +        }
> +    }
> +
> +    #[test]
> +    fn btrfs_raid() {
> +        let disks = dummy_disks(10);
> +
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid0, &[]).is_err());
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid0, &disks[..1]).is_ok());
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid0, &disks).is_ok());
> +
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &[]).is_err());
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks[..1]).is_err());
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks[..2]).is_ok());
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks).is_ok());
> +
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &[]).is_err());
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..3]).is_err());
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..4]).is_ok());
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks).is_ok());
> +    }
> +
> +    #[test]
> +    fn zfs_raid() {
> +        let disks = dummy_disks(10);
> +
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid0, &[]).is_err());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid0, &disks[..1]).is_ok());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid0, &disks).is_ok());
> +
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid1, &[]).is_err());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid1, &disks[..2]).is_ok());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid1, &disks).is_ok());
> +
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid10, &[]).is_err());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid10, &dummy_disks(4)).is_ok());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::Raid10, &disks).is_ok());
> +
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &[]).is_err());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &disks[..2]).is_err());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &disks[..3]).is_ok());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &disks).is_ok());
> +
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &[]).is_err());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &disks[..3]).is_err());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &disks[..4]).is_ok());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &disks).is_ok());
> +
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &[]).is_err());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &disks[..4]).is_err());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &disks[..5]).is_ok());
> +        assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &disks).is_ok());
> +    }
> +}
> diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
> index e69de29..f0093f5 100644
> --- a/proxmox-installer-common/src/lib.rs
> +++ b/proxmox-installer-common/src/lib.rs
> @@ -0,0 +1,4 @@
> +pub mod disk_checks;
> +pub mod options;
> +pub mod setup;
> +pub mod utils;
> diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
> new file mode 100644
> index 0000000..185be2e
> --- /dev/null
> +++ b/proxmox-installer-common/src/options.rs
> @@ -0,0 +1,387 @@
> +use std::net::{IpAddr, Ipv4Addr};
> +use std::{cmp, fmt};
> +
> +use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo};
> +use crate::utils::{CidrAddress, Fqdn};
> +
> +#[derive(Copy, Clone, Debug, Eq, PartialEq)]
> +pub enum BtrfsRaidLevel {
> +    Raid0,
> +    Raid1,
> +    Raid10,
> +}
> +
> +impl fmt::Display for BtrfsRaidLevel {
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        use BtrfsRaidLevel::*;
> +        match self {
> +            Raid0 => write!(f, "RAID0"),
> +            Raid1 => write!(f, "RAID1"),
> +            Raid10 => write!(f, "RAID10"),
> +        }
> +    }
> +}
> +
> +#[derive(Copy, Clone, Debug, Eq, PartialEq)]
> +pub enum ZfsRaidLevel {
> +    Raid0,
> +    Raid1,
> +    Raid10,
> +    RaidZ,
> +    RaidZ2,
> +    RaidZ3,
> +}
> +
> +impl fmt::Display for ZfsRaidLevel {
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        use ZfsRaidLevel::*;
> +        match self {
> +            Raid0 => write!(f, "RAID0"),
> +            Raid1 => write!(f, "RAID1"),
> +            Raid10 => write!(f, "RAID10"),
> +            RaidZ => write!(f, "RAIDZ-1"),
> +            RaidZ2 => write!(f, "RAIDZ-2"),
> +            RaidZ3 => write!(f, "RAIDZ-3"),
> +        }
> +    }
> +}
> +
> +#[derive(Copy, Clone, Debug, Eq, PartialEq)]
> +pub enum FsType {
> +    Ext4,
> +    Xfs,
> +    Zfs(ZfsRaidLevel),
> +    Btrfs(BtrfsRaidLevel),
> +}
> +
> +impl FsType {
> +    pub fn is_btrfs(&self) -> bool {
> +        matches!(self, FsType::Btrfs(_))
> +    }
> +}
> +
> +impl fmt::Display for FsType {
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        use FsType::*;
> +        match self {
> +            Ext4 => write!(f, "ext4"),
> +            Xfs => write!(f, "XFS"),
> +            Zfs(level) => write!(f, "ZFS ({level})"),
> +            Btrfs(level) => write!(f, "Btrfs ({level})"),
> +        }
> +    }
> +}
> +
> +#[derive(Clone, Debug)]
> +pub struct LvmBootdiskOptions {
> +    pub total_size: f64,
> +    pub swap_size: Option<f64>,
> +    pub max_root_size: Option<f64>,
> +    pub max_data_size: Option<f64>,
> +    pub min_lvm_free: Option<f64>,
> +}
> +
> +impl LvmBootdiskOptions {
> +    pub fn defaults_from(disk: &Disk) -> Self {
> +        Self {
> +            total_size: disk.size,
> +            swap_size: None,
> +            max_root_size: None,
> +            max_data_size: None,
> +            min_lvm_free: None,
> +        }
> +    }
> +}
> +
> +#[derive(Clone, Debug)]
> +pub struct BtrfsBootdiskOptions {
> +    pub disk_size: f64,
> +    pub selected_disks: Vec<usize>,
> +}
> +
> +impl BtrfsBootdiskOptions {
> +    /// This panics if the provided slice is empty.
> +    pub fn defaults_from(disks: &[Disk]) -> Self {
> +        let disk = &disks[0];
> +        Self {
> +            disk_size: disk.size,
> +            selected_disks: (0..disks.len()).collect(),
> +        }
> +    }
> +}
> +
> +#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
> +pub enum ZfsCompressOption {
> +    #[default]
> +    On,
> +    Off,
> +    Lzjb,
> +    Lz4,
> +    Zle,
> +    Gzip,
> +    Zstd,
> +}
> +
> +impl fmt::Display for ZfsCompressOption {
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        write!(f, "{}", format!("{self:?}").to_lowercase())
> +    }
> +}
> +
> +impl From<&ZfsCompressOption> for String {
> +    fn from(value: &ZfsCompressOption) -> Self {
> +        value.to_string()
> +    }
> +}
> +
> +pub const ZFS_COMPRESS_OPTIONS: &[ZfsCompressOption] = {
> +    use ZfsCompressOption::*;
> +    &[On, Off, Lzjb, Lz4, Zle, Gzip, Zstd]
> +};
> +
> +#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
> +pub enum ZfsChecksumOption {
> +    #[default]
> +    On,
> +    Off,
> +    Fletcher2,
> +    Fletcher4,
> +    Sha256,
> +}
> +
> +impl fmt::Display for ZfsChecksumOption {
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        write!(f, "{}", format!("{self:?}").to_lowercase())
> +    }
> +}
> +
> +impl From<&ZfsChecksumOption> for String {
> +    fn from(value: &ZfsChecksumOption) -> Self {
> +        value.to_string()
> +    }
> +}
> +
> +pub const ZFS_CHECKSUM_OPTIONS: &[ZfsChecksumOption] = {
> +    use ZfsChecksumOption::*;
> +    &[On, Off, Fletcher2, Fletcher4, Sha256]
> +};
> +
> +#[derive(Clone, Debug)]
> +pub struct ZfsBootdiskOptions {
> +    pub ashift: usize,
> +    pub compress: ZfsCompressOption,
> +    pub checksum: ZfsChecksumOption,
> +    pub copies: usize,
> +    pub disk_size: f64,
> +    pub selected_disks: Vec<usize>,
> +}
> +
> +impl ZfsBootdiskOptions {
> +    /// This panics if the provided slice is empty.
> +    pub fn defaults_from(disks: &[Disk]) -> Self {
> +        let disk = &disks[0];
> +        Self {
> +            ashift: 12,
> +            compress: ZfsCompressOption::default(),
> +            checksum: ZfsChecksumOption::default(),
> +            copies: 1,
> +            disk_size: disk.size,
> +            selected_disks: (0..disks.len()).collect(),
> +        }
> +    }
> +}
> +
> +#[derive(Clone, Debug)]
> +pub enum AdvancedBootdiskOptions {
> +    Lvm(LvmBootdiskOptions),
> +    Zfs(ZfsBootdiskOptions),
> +    Btrfs(BtrfsBootdiskOptions),
> +}
> +
> +#[derive(Clone, Debug, PartialEq)]
> +pub struct Disk {
> +    pub index: String,
> +    pub path: String,
> +    pub model: Option<String>,
> +    pub size: f64,
> +    pub block_size: Option<usize>,
> +}
> +
> +impl fmt::Display for Disk {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        // TODO: Format sizes properly with `proxmox-human-byte` once merged
> +        // https://lists.proxmox.com/pipermail/pbs-devel/2023-May/006125.html
> +        f.write_str(&self.path)?;
> +        if let Some(model) = &self.model {
> +            // FIXME: ellipsize too-long names?
> +            write!(f, " ({model})")?;
> +        }
> +        write!(f, " ({:.2} GiB)", self.size)
> +    }
> +}
> +
> +impl From<&Disk> for String {
> +    fn from(value: &Disk) -> Self {
> +        value.to_string()
> +    }
> +}
> +
> +impl cmp::Eq for Disk {}
> +
> +impl cmp::PartialOrd for Disk {
> +    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
> +        self.index.partial_cmp(&other.index)
> +    }
> +}
> +
> +impl cmp::Ord for Disk {
> +    fn cmp(&self, other: &Self) -> cmp::Ordering {
> +        self.index.cmp(&other.index)
> +    }
> +}
> +
> +#[derive(Clone, Debug)]
> +pub struct BootdiskOptions {
> +    pub disks: Vec<Disk>,
> +    pub fstype: FsType,
> +    pub advanced: AdvancedBootdiskOptions,
> +}
> +
> +impl BootdiskOptions {
> +    pub fn defaults_from(disk: &Disk) -> Self {
> +        Self {
> +            disks: vec![disk.clone()],
> +            fstype: FsType::Ext4,
> +            advanced: AdvancedBootdiskOptions::Lvm(LvmBootdiskOptions::defaults_from(disk)),
> +        }
> +    }
> +}
> +
> +#[derive(Clone, Debug)]
> +pub struct TimezoneOptions {
> +    pub country: String,
> +    pub timezone: String,
> +    pub kb_layout: String,
> +}
> +
> +impl TimezoneOptions {
> +    pub fn defaults_from(runtime: &RuntimeInfo, locales: &LocaleInfo) -> Self {
> +        let country = runtime.country.clone().unwrap_or_else(|| "at".to_owned());
> +
> +        let timezone = locales
> +            .cczones
> +            .get(&country)
> +            .and_then(|zones| zones.get(0))
> +            .cloned()
> +            .unwrap_or_else(|| "UTC".to_owned());
> +
> +        let kb_layout = locales
> +            .countries
> +            .get(&country)
> +            .and_then(|c| {
> +                if c.kmap.is_empty() {
> +                    None
> +                } else {
> +                    Some(c.kmap.clone())
> +                }
> +            })
> +            .unwrap_or_else(|| "en-us".to_owned());
> +
> +        Self {
> +            country,
> +            timezone,
> +            kb_layout,
> +        }
> +    }
> +}
> +
> +#[derive(Clone, Debug)]
> +pub struct PasswordOptions {
> +    pub email: String,
> +    pub root_password: String,
> +}
> +
> +impl Default for PasswordOptions {
> +    fn default() -> Self {
> +        Self {
> +            email: "mail@example.invalid".to_string(),
> +            root_password: String::new(),
> +        }
> +    }
> +}
> +
> +#[derive(Clone, Debug, PartialEq)]
> +pub struct NetworkOptions {
> +    pub ifname: String,
> +    pub fqdn: Fqdn,
> +    pub address: CidrAddress,
> +    pub gateway: IpAddr,
> +    pub dns_server: IpAddr,
> +}
> +
> +impl NetworkOptions {
> +    const DEFAULT_DOMAIN: &str = "example.invalid";
> +
> +    pub fn defaults_from(setup: &SetupInfo, network: &NetworkInfo) -> Self {
> +        let mut this = Self {
> +            ifname: String::new(),
> +            fqdn: Self::construct_fqdn(network, setup.config.product.default_hostname()),
> +            // Safety: The provided mask will always be valid.
> +            address: CidrAddress::new(Ipv4Addr::UNSPECIFIED, 0).unwrap(),
> +            gateway: Ipv4Addr::UNSPECIFIED.into(),
> +            dns_server: Ipv4Addr::UNSPECIFIED.into(),
> +        };
> +
> +        if let Some(ip) = network.dns.dns.first() {
> +            this.dns_server = *ip;
> +        }
> +
> +        if let Some(routes) = &network.routes {
> +            let mut filled = false;
> +            if let Some(gw) = &routes.gateway4 {
> +                if let Some(iface) = network.interfaces.get(&gw.dev) {
> +                    this.ifname = iface.name.clone();
> +                    if let Some(addresses) = &iface.addresses {
> +                        if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv4()) {
> +                            this.gateway = gw.gateway;
> +                            this.address = addr.clone();
> +                            filled = true;
> +                        }
> +                    }
> +                }
> +            }
> +            if !filled {
> +                if let Some(gw) = &routes.gateway6 {
> +                    if let Some(iface) = network.interfaces.get(&gw.dev) {
> +                        if let Some(addresses) = &iface.addresses {
> +                            if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv6()) {
> +                                this.ifname = iface.name.clone();
> +                                this.gateway = gw.gateway;
> +                                this.address = addr.clone();
> +                            }
> +                        }
> +                    }
> +                }
> +            }
> +        }
> +
> +        this
> +    }
> +
> +    fn construct_fqdn(network: &NetworkInfo, default_hostname: &str) -> Fqdn {
> +        let hostname = network.hostname.as_deref().unwrap_or(default_hostname);
> +
> +        let domain = network
> +            .dns
> +            .domain
> +            .as_deref()
> +            .unwrap_or(Self::DEFAULT_DOMAIN);
> +
> +        Fqdn::from(&format!("{hostname}.{domain}")).unwrap_or_else(|_| {
> +            // Safety: This will always result in a valid FQDN, as we control & know
> +            // the values of default_hostname (one of "pve", "pmg" or "pbs") and
> +            // constant-defined DEFAULT_DOMAIN.
> +            Fqdn::from(&format!("{}.{}", default_hostname, Self::DEFAULT_DOMAIN)).unwrap()
> +        })
> +    }
> +}
> diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
> new file mode 100644
> index 0000000..a4947f1
> --- /dev/null
> +++ b/proxmox-installer-common/src/setup.rs
> @@ -0,0 +1,330 @@
> +use std::{
> +    cmp,
> +    collections::HashMap,
> +    fmt,
> +    fs::File,
> +    io::BufReader,
> +    net::IpAddr,
> +    path::{Path, PathBuf},
> +};
> +
> +use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
> +
> +use crate::{
> +    options::{Disk, ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption},
> +    utils::CidrAddress,
> +};
> +
> +#[allow(clippy::upper_case_acronyms)]
> +#[derive(Clone, Copy, Deserialize, PartialEq)]
> +#[serde(rename_all = "lowercase")]
> +pub enum ProxmoxProduct {
> +    PVE,
> +    PBS,
> +    PMG,
> +}
> +
> +impl ProxmoxProduct {
> +    pub fn default_hostname(self) -> &'static str {
> +        match self {
> +            Self::PVE => "pve",
> +            Self::PMG => "pmg",
> +            Self::PBS => "pbs",
> +        }
> +    }
> +}
> +
> +#[derive(Clone, Deserialize)]
> +pub struct ProductConfig {
> +    pub fullname: String,
> +    pub product: ProxmoxProduct,
> +    #[serde(deserialize_with = "deserialize_bool_from_int")]
> +    pub enable_btrfs: bool,
> +}
> +
> +#[derive(Clone, Deserialize)]
> +pub struct IsoInfo {
> +    pub release: String,
> +    pub isorelease: String,
> +}
> +
> +/// Paths in the ISO environment containing installer data.
> +#[derive(Clone, Deserialize)]
> +pub struct IsoLocations {
> +    pub iso: PathBuf,
> +}
> +
> +#[derive(Clone, Deserialize)]
> +pub struct SetupInfo {
> +    #[serde(rename = "product-cfg")]
> +    pub config: ProductConfig,
> +    #[serde(rename = "iso-info")]
> +    pub iso_info: IsoInfo,
> +    pub locations: IsoLocations,
> +}
> +
> +#[derive(Clone, Deserialize)]
> +pub struct CountryInfo {
> +    pub name: String,
> +    #[serde(default)]
> +    pub zone: String,
> +    pub kmap: String,
> +}
> +
> +#[derive(Clone, Deserialize, Eq, PartialEq)]
> +pub struct KeyboardMapping {
> +    pub name: String,
> +    #[serde(rename = "kvm")]
> +    pub id: String,
> +    #[serde(rename = "x11")]
> +    pub xkb_layout: String,
> +    #[serde(rename = "x11var")]
> +    pub xkb_variant: String,
> +}
> +
> +impl cmp::PartialOrd for KeyboardMapping {
> +    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
> +        self.name.partial_cmp(&other.name)
> +    }
> +}
> +
> +impl cmp::Ord for KeyboardMapping {
> +    fn cmp(&self, other: &Self) -> cmp::Ordering {
> +        self.name.cmp(&other.name)
> +    }
> +}
> +
> +#[derive(Clone, Deserialize)]
> +pub struct LocaleInfo {
> +    #[serde(deserialize_with = "deserialize_cczones_map")]
> +    pub cczones: HashMap<String, Vec<String>>,
> +    #[serde(rename = "country")]
> +    pub countries: HashMap<String, CountryInfo>,
> +    pub kmap: HashMap<String, KeyboardMapping>,
> +}
> +
> +#[derive(Serialize)]
> +struct InstallZfsOption {
> +    ashift: usize,
> +    #[serde(serialize_with = "serialize_as_display")]
> +    compress: ZfsCompressOption,
> +    #[serde(serialize_with = "serialize_as_display")]
> +    checksum: ZfsChecksumOption,
> +    copies: usize,
> +}
> +
> +impl From<ZfsBootdiskOptions> for InstallZfsOption {
> +    fn from(opts: ZfsBootdiskOptions) -> Self {
> +        InstallZfsOption {
> +            ashift: opts.ashift,
> +            compress: opts.compress,
> +            checksum: opts.checksum,
> +            copies: opts.copies,
> +        }
> +    }
> +}
> +
> +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);
> +
> +    serde_json::from_reader(reader).map_err(|err| format!("failed to parse JSON: {err}"))
> +}
> +
> +fn deserialize_bool_from_int<'de, D>(deserializer: D) -> Result<bool, D::Error>
> +where
> +    D: Deserializer<'de>,
> +{
> +    let val: u32 = Deserialize::deserialize(deserializer)?;
> +    Ok(val != 0)
> +}
> +
> +fn deserialize_cczones_map<'de, D>(
> +    deserializer: D,
> +) -> Result<HashMap<String, Vec<String>>, D::Error>
> +where
> +    D: Deserializer<'de>,
> +{
> +    let map: HashMap<String, HashMap<String, u32>> = Deserialize::deserialize(deserializer)?;
> +
> +    let mut result = HashMap::new();
> +    for (cc, list) in map.into_iter() {
> +        result.insert(cc, list.into_keys().collect());
> +    }
> +
> +    Ok(result)
> +}
> +
> +fn deserialize_disks_map<'de, D>(deserializer: D) -> Result<Vec<Disk>, D::Error>
> +where
> +    D: Deserializer<'de>,
> +{
> +    let disks =
> +        <Vec<(usize, String, f64, String, Option<usize>, String)>>::deserialize(deserializer)?;
> +    Ok(disks
> +        .into_iter()
> +        .map(
> +            |(index, device, size_mb, model, logical_bsize, _syspath)| Disk {
> +                index: index.to_string(),
> +                // Linux always reports the size of block devices in sectors, where one sector is
> +                // defined as being 2^9 = 512 bytes in size.
> +                // https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/blk_types.h?h=v6.4#n30
> +                size: (size_mb * 512.) / 1024. / 1024. / 1024.,
> +                block_size: logical_bsize,
> +                path: device,
> +                model: (!model.is_empty()).then_some(model),
> +            },
> +        )
> +        .collect())
> +}
> +
> +fn deserialize_cidr_list<'de, D>(deserializer: D) -> Result<Option<Vec<CidrAddress>>, D::Error>
> +where
> +    D: Deserializer<'de>,
> +{
> +    #[derive(Deserialize)]
> +    struct CidrDescriptor {
> +        address: String,
> +        prefix: usize,
> +        // family is implied anyway by parsing the address
> +    }
> +
> +    let list: Vec<CidrDescriptor> = Deserialize::deserialize(deserializer)?;
> +
> +    let mut result = Vec::with_capacity(list.len());
> +    for desc in list {
> +        let ip_addr = desc
> +            .address
> +            .parse::<IpAddr>()
> +            .map_err(|err| de::Error::custom(format!("{:?}", err)))?;
> +
> +        result.push(
> +            CidrAddress::new(ip_addr, desc.prefix)
> +                .map_err(|err| de::Error::custom(format!("{:?}", err)))?,
> +        );
> +    }
> +
> +    Ok(Some(result))
> +}
> +
> +fn serialize_as_display<S, T>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
> +where
> +    S: Serializer,
> +    T: fmt::Display,
> +{
> +    serializer.collect_str(value)
> +}
> +
> +#[derive(Clone, Deserialize)]
> +pub struct RuntimeInfo {
> +    /// Whether is system was booted in (legacy) BIOS or UEFI mode.
> +    pub boot_type: BootType,
> +
> +    /// Detected country if available.
> +    pub country: Option<String>,
> +
> +    /// Maps devices to their information.
> +    #[serde(deserialize_with = "deserialize_disks_map")]
> +    pub disks: Vec<Disk>,
> +
> +    /// Network addresses, gateways and DNS info.
> +    pub network: NetworkInfo,
> +
> +    /// Total memory of the system in MiB.
> +    pub total_memory: usize,
> +
> +    /// Whether the CPU supports hardware-accelerated virtualization
> +    #[serde(deserialize_with = "deserialize_bool_from_int")]
> +    pub hvm_supported: bool,
> +}
> +
> +#[derive(Copy, Clone, Eq, Deserialize, PartialEq)]
> +#[serde(rename_all = "lowercase")]
> +pub enum BootType {
> +    Bios,
> +    Efi,
> +}
> +
> +#[derive(Clone, Deserialize)]
> +pub struct NetworkInfo {
> +    pub dns: Dns,
> +    pub routes: Option<Routes>,
> +
> +    /// Maps devices to their configuration, if it has a usable configuration.
> +    /// (Contains no entries for devices with only link-local addresses.)
> +    #[serde(default)]
> +    pub interfaces: HashMap<String, Interface>,
> +
> +    /// The hostname of this machine, if set by the DHCP server.
> +    pub hostname: Option<String>,
> +}
> +
> +#[derive(Clone, Deserialize)]
> +pub struct Dns {
> +    pub domain: Option<String>,
> +
> +    /// List of stringified IP addresses.
> +    #[serde(default)]
> +    pub dns: Vec<IpAddr>,
> +}
> +
> +#[derive(Clone, Deserialize)]
> +pub struct Routes {
> +    /// Ipv4 gateway.
> +    pub gateway4: Option<Gateway>,
> +
> +    /// Ipv6 gateway.
> +    pub gateway6: Option<Gateway>,
> +}
> +
> +#[derive(Clone, Deserialize)]
> +pub struct Gateway {
> +    /// Outgoing network device.
> +    pub dev: String,
> +
> +    /// Stringified gateway IP address.
> +    pub gateway: IpAddr,
> +}
> +
> +#[derive(Clone, Deserialize)]
> +#[serde(rename_all = "UPPERCASE")]
> +pub enum InterfaceState {
> +    Up,
> +    Down,
> +    #[serde(other)]
> +    Unknown,
> +}
> +
> +impl InterfaceState {
> +    // avoid display trait as this is not the string representation for a serializer
> +    pub fn render(&self) -> String {
> +        match self {
> +            Self::Up => "\u{25CF}",
> +            Self::Down | Self::Unknown => " ",
> +        }
> +        .into()
> +    }
> +}
> +
> +#[derive(Clone, Deserialize)]
> +pub struct Interface {
> +    pub name: String,
> +
> +    pub index: usize,
> +
> +    pub mac: String,
> +
> +    pub state: InterfaceState,
> +
> +    #[serde(default)]
> +    #[serde(deserialize_with = "deserialize_cidr_list")]
> +    pub addresses: Option<Vec<CidrAddress>>,
> +}
> +
> +impl Interface {
> +    // avoid display trait as this is not the string representation for a serializer
> +    pub fn render(&self) -> String {
> +        format!("{} {}", self.state.render(), self.name)
> +    }
> +}
> +
> diff --git a/proxmox-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs
> new file mode 100644
> index 0000000..89349ed
> --- /dev/null
> +++ b/proxmox-installer-common/src/utils.rs
> @@ -0,0 +1,268 @@
> +use std::{
> +    fmt,
> +    net::{AddrParseError, IpAddr},
> +    num::ParseIntError,
> +    str::FromStr,
> +};
> +
> +use serde::Deserialize;
> +
> +/// Possible errors that might occur when parsing CIDR addresses.
> +#[derive(Debug)]
> +pub enum CidrAddressParseError {
> +    /// No delimiter for separating address and mask was found.
> +    NoDelimiter,
> +    /// The IP address part could not be parsed.
> +    InvalidAddr(AddrParseError),
> +    /// The mask could not be parsed.
> +    InvalidMask(Option<ParseIntError>),
> +}
> +
> +/// An IP address (IPv4 or IPv6), including network mask.
> +///
> +/// See the [`IpAddr`] type for more information how IP addresses are handled.
> +/// The mask is appropriately enforced to be `0 <= mask <= 32` for IPv4 or
> +/// `0 <= mask <= 128` for IPv6 addresses.
> +///
> +/// # Examples
> +/// ```
> +/// use std::net::{Ipv4Addr, Ipv6Addr};
> +/// let ipv4 = CidrAddress::new(Ipv4Addr::new(192, 168, 0, 1), 24).unwrap();
> +/// let ipv6 = CidrAddress::new(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0xc0a8, 1), 32).unwrap();
> +///
> +/// assert_eq!(ipv4.to_string(), "192.168.0.1/24");
> +/// assert_eq!(ipv6.to_string(), "2001:db8::c0a8:1/32");
> +/// ```
> +#[derive(Clone, Debug, PartialEq)]
> +pub struct CidrAddress {
> +    addr: IpAddr,
> +    mask: usize,
> +}
> +
> +impl CidrAddress {
> +    /// Constructs a new CIDR address.
> +    ///
> +    /// It fails if the mask is invalid for the given IP address.
> +    pub fn new<T: Into<IpAddr>>(addr: T, mask: usize) -> Result<Self, CidrAddressParseError> {
> +        let addr = addr.into();
> +
> +        if mask > mask_limit(&addr) {
> +            Err(CidrAddressParseError::InvalidMask(None))
> +        } else {
> +            Ok(Self { addr, mask })
> +        }
> +    }
> +
> +    /// Returns only the IP address part of the address.
> +    pub fn addr(&self) -> IpAddr {
> +        self.addr
> +    }
> +
> +    /// Returns `true` if this address is an IPv4 address, `false` otherwise.
> +    pub fn is_ipv4(&self) -> bool {
> +        self.addr.is_ipv4()
> +    }
> +
> +    /// Returns `true` if this address is an IPv6 address, `false` otherwise.
> +    pub fn is_ipv6(&self) -> bool {
> +        self.addr.is_ipv6()
> +    }
> +
> +    /// Returns only the mask part of the address.
> +    pub fn mask(&self) -> usize {
> +        self.mask
> +    }
> +}
> +
> +impl FromStr for CidrAddress {
> +    type Err = CidrAddressParseError;
> +
> +    fn from_str(s: &str) -> Result<Self, Self::Err> {
> +        let (addr, mask) = s
> +            .split_once('/')
> +            .ok_or(CidrAddressParseError::NoDelimiter)?;
> +
> +        let addr = addr.parse().map_err(CidrAddressParseError::InvalidAddr)?;
> +
> +        let mask = mask
> +            .parse()
> +            .map_err(|err| CidrAddressParseError::InvalidMask(Some(err)))?;
> +
> +        if mask > mask_limit(&addr) {
> +            Err(CidrAddressParseError::InvalidMask(None))
> +        } else {
> +            Ok(Self { addr, mask })
> +        }
> +    }
> +}
> +
> +impl fmt::Display for CidrAddress {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        write!(f, "{}/{}", self.addr, self.mask)
> +    }
> +}
> +
> +fn mask_limit(addr: &IpAddr) -> usize {
> +    if addr.is_ipv4() {
> +        32
> +    } else {
> +        128
> +    }
> +}
> +
> +/// Possible errors that might occur when parsing FQDNs.
> +#[derive(Debug, Eq, PartialEq)]
> +pub enum FqdnParseError {
> +    MissingHostname,
> +    NumericHostname,
> +    InvalidPart(String),
> +}
> +
> +impl fmt::Display for FqdnParseError {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        use FqdnParseError::*;
> +        match self {
> +            MissingHostname => write!(f, "missing hostname part"),
> +            NumericHostname => write!(f, "hostname cannot be purely numeric"),
> +            InvalidPart(part) => write!(
> +                f,
> +                "FQDN must only consist of alphanumeric characters and dashes. Invalid part: '{part}'",
> +            ),
> +        }
> +    }
> +}
> +
> +#[derive(Clone, Debug, Eq, PartialEq)]
> +pub struct Fqdn {
> +    parts: Vec<String>,
> +}
> +
> +impl Fqdn {
> +    pub fn from(fqdn: &str) -> Result<Self, FqdnParseError> {
> +        let parts = fqdn
> +            .split('.')
> +            .map(ToOwned::to_owned)
> +            .collect::<Vec<String>>();
> +
> +        for part in &parts {
> +            if !Self::validate_single(part) {
> +                return Err(FqdnParseError::InvalidPart(part.clone()));
> +            }
> +        }
> +
> +        if parts.len() < 2 {
> +            Err(FqdnParseError::MissingHostname)
> +        } else if parts[0].chars().all(|c| c.is_ascii_digit()) {
> +            // Not allowed/supported on Debian systems.
> +            Err(FqdnParseError::NumericHostname)
> +        } else {
> +            Ok(Self { parts })
> +        }
> +    }
> +
> +    pub fn host(&self) -> Option<&str> {
> +        self.has_host().then_some(&self.parts[0])
> +    }
> +
> +    pub fn domain(&self) -> String {
> +        let parts = if self.has_host() {
> +            &self.parts[1..]
> +        } else {
> +            &self.parts
> +        };
> +
> +        parts.join(".")
> +    }
> +
> +    /// Checks whether the FQDN has a hostname associated with it, i.e. is has more than 1 part.
> +    fn has_host(&self) -> bool {
> +        self.parts.len() > 1
> +    }
> +
> +    fn validate_single(s: &String) -> bool {
> +        !s.is_empty()
> +            // First character must be alphanumeric
> +            && s.chars()
> +                .next()
> +                .map(|c| c.is_ascii_alphanumeric())
> +                .unwrap_or_default()
> +            // .. last character as well,
> +            && s.chars()
> +                .last()
> +                .map(|c| c.is_ascii_alphanumeric())
> +                .unwrap_or_default()
> +            // and anything between must be alphanumeric or -
> +            && s.chars()
> +                .skip(1)
> +                .take(s.len().saturating_sub(2))
> +                .all(|c| c.is_ascii_alphanumeric() || c == '-')
> +    }
> +}
> +
> +impl FromStr for Fqdn {
> +    type Err = FqdnParseError;
> +
> +    fn from_str(value: &str) -> Result<Self, Self::Err> {
> +        Self::from(value)
> +    }
> +}
> +
> +impl fmt::Display for Fqdn {
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        write!(f, "{}", self.parts.join("."))
> +    }
> +}
> +
> +impl<'de> Deserialize<'de> for Fqdn {
> +    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
> +    where
> +        D: serde::Deserializer<'de>,
> +    {
> +        let s: String = Deserialize::deserialize(deserializer)?;
> +        s.parse()
> +            .map_err(|_| serde::de::Error::custom("invalid FQDN"))
> +    }
> +}
> +
> +#[cfg(test)]
> +mod tests {
> +    use super::*;
> +
> +    #[test]
> +    fn fqdn_construct() {
> +        use FqdnParseError::*;
> +        assert!(Fqdn::from("foo.example.com").is_ok());
> +        assert!(Fqdn::from("foo-bar.com").is_ok());
> +        assert!(Fqdn::from("a-b.com").is_ok());
> +
> +        assert_eq!(Fqdn::from("foo"), Err(MissingHostname));
> +
> +        assert_eq!(Fqdn::from("-foo.com"), Err(InvalidPart("-foo".to_owned())));
> +        assert_eq!(Fqdn::from("foo-.com"), Err(InvalidPart("foo-".to_owned())));
> +        assert_eq!(Fqdn::from("foo.com-"), Err(InvalidPart("com-".to_owned())));
> +        assert_eq!(Fqdn::from("-o-.com"), Err(InvalidPart("-o-".to_owned())));
> +
> +        assert_eq!(Fqdn::from("123.com"), Err(NumericHostname));
> +        assert!(Fqdn::from("foo123.com").is_ok());
> +        assert!(Fqdn::from("123foo.com").is_ok());
> +    }
> +
> +    #[test]
> +    fn fqdn_parts() {
> +        let fqdn = Fqdn::from("pve.example.com").unwrap();
> +        assert_eq!(fqdn.host().unwrap(), "pve");
> +        assert_eq!(fqdn.domain(), "example.com");
> +        assert_eq!(
> +            fqdn.parts,
> +            &["pve".to_owned(), "example".to_owned(), "com".to_owned()]
> +        );
> +    }
> +
> +    #[test]
> +    fn fqdn_display() {
> +        assert_eq!(
> +            Fqdn::from("foo.example.com").unwrap().to_string(),
> +            "foo.example.com"
> +        );
> +    }
> +}
> --
> 2.39.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] 20+ messages in thread

* [pve-devel] [PATCH installer] buildsys: copy over `proxmox-installer-common` crate to build directory
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
                   ` (11 preceding siblings ...)
  2023-10-25 16:00 ` [pve-devel] [PATCH 12/12] tui: remove unused read_json function Aaron Lauterer
@ 2023-10-27 11:39 ` Christoph Heiss
  2023-10-27 11:41 ` [pve-devel] [PATCH 00/12] installer: add crate for common code Christoph Heiss
  2023-11-02 19:05 ` [pve-devel] applied-series: " Thomas Lamprecht
  14 siblings, 0 replies; 20+ messages in thread
From: Christoph Heiss @ 2023-10-27 11:39 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 111fe4b..e792e0e 100644
--- a/Makefile
+++ b/Makefile
@@ -43,6 +43,7 @@ $(BUILDDIR):
 	  proxinstall \
 	  proxmox-low-level-installer \
 	  proxmox-tui-installer/ \
+	  proxmox-installer-common/ \
 	  spice-vdagent.sh \
 	  unconfigured.sh \
 	  xinitrc \
-- 
2.42.0





^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [pve-devel] [PATCH 00/12] installer: add crate for common code
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
                   ` (12 preceding siblings ...)
  2023-10-27 11:39 ` [pve-devel] [PATCH installer] buildsys: copy over `proxmox-installer-common` crate to build directory Christoph Heiss
@ 2023-10-27 11:41 ` Christoph Heiss
  2023-10-30  9:45   ` Aaron Lauterer
  2023-11-02 19:05 ` [pve-devel] applied-series: " Thomas Lamprecht
  14 siblings, 1 reply; 20+ messages in thread
From: Christoph Heiss @ 2023-10-27 11:41 UTC (permalink / raw)
  To: Aaron Lauterer; +Cc: Proxmox VE development discussion


Had a lot of in-person discussions with Aaron over the last few weeks
over this.
There are no real functional changes here that would impact users,
merely a code move.

Each inidivdual patch (except #1, see my answer on that) builds cleanly
on top of current master. The one dependency (see below) is also already
applied. I also did some quick smoke testing to verify everything really
still works.

I will go over the the rest of the patches afterwards, but from a glance
there is nothing that I would consider a blocker.
Things like e.g. the `InstallConfig` stuff can be done separately as
followups.

Also considering this is a great code churn, I would rather have this
get applied sooner than later, such that all future work is done on top
of this.

The only "complaint" here is that the .deb package does not build with
this series (see my reply on #1). I will shortly send a quick follow-up
patch for that, so you don't have to necessarily re-spin the whole
series just for a single, trivial line.

LGTM; thus please consider the whole series:

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>

On Wed, Oct 25, 2023 at 05:59:59PM +0200, Aaron Lauterer wrote:
>
> since work on the auto installer is happenning in parallel, now would be
> a good point to move commonly used code into its own crate. Otherwise
> the auto-installer will always have to play catch up with the ongoing
> development of the tui installer.
>
> I tried to split up the commits as much as possible, but there are two
> larger ones, copying most the code over to the new repo and making the
> switch. The former because it is difficult to pull apart the parts that
> belong together. The latter needed to be this big as most local
> occurences needed to be removed at the same time to avoid dependency
> conflicts.
>
> One last things that is missing, is the "InstallConfig" struct.
> This should also be part of the common crate, but I need to look further
> into how to make it possible that it can be created from structs of each
> crate (tui, auto) as implementing a ::From can only be done within the
> crate where the struct lives IIUC.
>
> This series depends on the patches by Christoph to remove the global
> unsafe setup info, version 2 [0]. Without those patches applied first,
> this series will not apply.
>
> [0] https://lists.proxmox.com/pipermail/pve-devel/2023-October/059628.html
>
> Aaron Lauterer (12):
>   add proxmox-installer-common crate
>   common: copy common code from tui-installer
>   common: utils: add dependency for doc test
>   common: make InstallZfsOption public
>   common: disk_checks: make functions public
>   tui-installer: add dependency for new common crate
>   tui: switch to common crate
>   tui: remove now unused utils.rs
>   common: add installer_setup method
>   common: document installer_setup method
>   tui: use installer_setup from common cate
>   tui: remove unused read_json function
>
>  Cargo.toml                                    |   1 +
>  proxmox-installer-common/Cargo.toml           |  12 +
>  proxmox-installer-common/src/disk_checks.rs   | 237 ++++++++++
>  proxmox-installer-common/src/lib.rs           |   4 +
>  proxmox-installer-common/src/options.rs       | 387 +++++++++++++++++
>  proxmox-installer-common/src/setup.rs         | 368 ++++++++++++++++
>  .../src/utils.rs                              |   1 +
>  proxmox-tui-installer/Cargo.toml              |   1 +
>  proxmox-tui-installer/src/main.rs             |  51 +--
>  proxmox-tui-installer/src/options.rs          | 403 +-----------------
>  proxmox-tui-installer/src/setup.rs            | 324 +-------------
>  proxmox-tui-installer/src/system.rs           |   2 +-
>  proxmox-tui-installer/src/views/bootdisk.rs   | 248 +----------
>  proxmox-tui-installer/src/views/mod.rs        |   2 +-
>  proxmox-tui-installer/src/views/timezone.rs   |   4 +-
>  15 files changed, 1054 insertions(+), 991 deletions(-)
>  create mode 100644 proxmox-installer-common/Cargo.toml
>  create mode 100644 proxmox-installer-common/src/disk_checks.rs
>  create mode 100644 proxmox-installer-common/src/lib.rs
>  create mode 100644 proxmox-installer-common/src/options.rs
>  create mode 100644 proxmox-installer-common/src/setup.rs
>  rename {proxmox-tui-installer => proxmox-installer-common}/src/utils.rs (99%)
>
> --
> 2.39.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] 20+ messages in thread

* Re: [pve-devel] [PATCH 00/12] installer: add crate for common code
  2023-10-27 11:41 ` [pve-devel] [PATCH 00/12] installer: add crate for common code Christoph Heiss
@ 2023-10-30  9:45   ` Aaron Lauterer
  0 siblings, 0 replies; 20+ messages in thread
From: Aaron Lauterer @ 2023-10-30  9:45 UTC (permalink / raw)
  To: Christoph Heiss; +Cc: Proxmox VE development discussion

Sorry, I missed to patch the Makefile and thanks for sending a patch for that [0].
The Makefile has another bug that I encountered during the RFC phase of the auto installer [0]. I could include that already in a V2 or add it to the auto installer series where the problem actually starts to show.


[0] https://lists.proxmox.com/pipermail/pve-devel/2023-October/059676.html
[1] https://lists.proxmox.com/pipermail/pve-devel/2023-September/059015.html


On 10/27/23 13:41, Christoph Heiss wrote:
> 
> Had a lot of in-person discussions with Aaron over the last few weeks
> over this.
> There are no real functional changes here that would impact users,
> merely a code move.
> 
> Each inidivdual patch (except #1, see my answer on that) builds cleanly
> on top of current master. The one dependency (see below) is also already
> applied. I also did some quick smoke testing to verify everything really
> still works.
> 
> I will go over the the rest of the patches afterwards, but from a glance
> there is nothing that I would consider a blocker.
> Things like e.g. the `InstallConfig` stuff can be done separately as
> followups.
> 
> Also considering this is a great code churn, I would rather have this
> get applied sooner than later, such that all future work is done on top
> of this.
> 
> The only "complaint" here is that the .deb package does not build with
> this series (see my reply on #1). I will shortly send a quick follow-up
> patch for that, so you don't have to necessarily re-spin the whole
> series just for a single, trivial line.
> 
> LGTM; thus please consider the whole series:
> 
> Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
> Tested-by: Christoph Heiss <c.heiss@proxmox.com>
> 
> On Wed, Oct 25, 2023 at 05:59:59PM +0200, Aaron Lauterer wrote:
>>
>> since work on the auto installer is happenning in parallel, now would be
>> a good point to move commonly used code into its own crate. Otherwise
>> the auto-installer will always have to play catch up with the ongoing
>> development of the tui installer.
>>
>> I tried to split up the commits as much as possible, but there are two
>> larger ones, copying most the code over to the new repo and making the
>> switch. The former because it is difficult to pull apart the parts that
>> belong together. The latter needed to be this big as most local
>> occurences needed to be removed at the same time to avoid dependency
>> conflicts.
>>
>> One last things that is missing, is the "InstallConfig" struct.
>> This should also be part of the common crate, but I need to look further
>> into how to make it possible that it can be created from structs of each
>> crate (tui, auto) as implementing a ::From can only be done within the
>> crate where the struct lives IIUC.
>>
>> This series depends on the patches by Christoph to remove the global
>> unsafe setup info, version 2 [0]. Without those patches applied first,
>> this series will not apply.
>>
>> [0] https://lists.proxmox.com/pipermail/pve-devel/2023-October/059628.html
>>
>> Aaron Lauterer (12):
>>    add proxmox-installer-common crate
>>    common: copy common code from tui-installer
>>    common: utils: add dependency for doc test
>>    common: make InstallZfsOption public
>>    common: disk_checks: make functions public
>>    tui-installer: add dependency for new common crate
>>    tui: switch to common crate
>>    tui: remove now unused utils.rs
>>    common: add installer_setup method
>>    common: document installer_setup method
>>    tui: use installer_setup from common cate
>>    tui: remove unused read_json function
>>
>>   Cargo.toml                                    |   1 +
>>   proxmox-installer-common/Cargo.toml           |  12 +
>>   proxmox-installer-common/src/disk_checks.rs   | 237 ++++++++++
>>   proxmox-installer-common/src/lib.rs           |   4 +
>>   proxmox-installer-common/src/options.rs       | 387 +++++++++++++++++
>>   proxmox-installer-common/src/setup.rs         | 368 ++++++++++++++++
>>   .../src/utils.rs                              |   1 +
>>   proxmox-tui-installer/Cargo.toml              |   1 +
>>   proxmox-tui-installer/src/main.rs             |  51 +--
>>   proxmox-tui-installer/src/options.rs          | 403 +-----------------
>>   proxmox-tui-installer/src/setup.rs            | 324 +-------------
>>   proxmox-tui-installer/src/system.rs           |   2 +-
>>   proxmox-tui-installer/src/views/bootdisk.rs   | 248 +----------
>>   proxmox-tui-installer/src/views/mod.rs        |   2 +-
>>   proxmox-tui-installer/src/views/timezone.rs   |   4 +-
>>   15 files changed, 1054 insertions(+), 991 deletions(-)
>>   create mode 100644 proxmox-installer-common/Cargo.toml
>>   create mode 100644 proxmox-installer-common/src/disk_checks.rs
>>   create mode 100644 proxmox-installer-common/src/lib.rs
>>   create mode 100644 proxmox-installer-common/src/options.rs
>>   create mode 100644 proxmox-installer-common/src/setup.rs
>>   rename {proxmox-tui-installer => proxmox-installer-common}/src/utils.rs (99%)
>>
>> --
>> 2.39.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] 20+ messages in thread

* [pve-devel] applied-series: [PATCH 00/12] installer: add crate for common code
  2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
                   ` (13 preceding siblings ...)
  2023-10-27 11:41 ` [pve-devel] [PATCH 00/12] installer: add crate for common code Christoph Heiss
@ 2023-11-02 19:05 ` Thomas Lamprecht
  14 siblings, 0 replies; 20+ messages in thread
From: Thomas Lamprecht @ 2023-11-02 19:05 UTC (permalink / raw)
  To: Proxmox VE development discussion, Aaron Lauterer

On 25/10/2023 17:59, Aaron Lauterer wrote:
> since work on the auto installer is happenning in parallel, now would be
> a good point to move commonly used code into its own crate. Otherwise
> the auto-installer will always have to play catch up with the ongoing
> development of the tui installer.
> 
> I tried to split up the commits as much as possible, but there are two
> larger ones, copying most the code over to the new repo and making the
> switch. The former because it is difficult to pull apart the parts that
> belong together. The latter needed to be this big as most local
> occurences needed to be removed at the same time to avoid dependency
> conflicts.
> 
> One last things that is missing, is the "InstallConfig" struct.
> This should also be part of the common crate, but I need to look further
> into how to make it possible that it can be created from structs of each
> crate (tui, auto) as implementing a ::From can only be done within the
> crate where the struct lives IIUC.
> 
> This series depends on the patches by Christoph to remove the global
> unsafe setup info, version 2 [0]. Without those patches applied first,
> this series will not apply.
> 
> [0] https://lists.proxmox.com/pipermail/pve-devel/2023-October/059628.html
> 
> Aaron Lauterer (12):
>   add proxmox-installer-common crate
>   common: copy common code from tui-installer
>   common: utils: add dependency for doc test
>   common: make InstallZfsOption public
>   common: disk_checks: make functions public
>   tui-installer: add dependency for new common crate
>   tui: switch to common crate
>   tui: remove now unused utils.rs
>   common: add installer_setup method
>   common: document installer_setup method
>   tui: use installer_setup from common cate
>   tui: remove unused read_json function

applied series, with Christoph's build-sys fix squashed into your first
patch, and his R-b added, thanks!




^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-11-02 19:05 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-25 15:59 [pve-devel] [PATCH 00/12] installer: add crate for common code Aaron Lauterer
2023-10-25 16:00 ` [pve-devel] [PATCH 01/12] add proxmox-installer-common crate Aaron Lauterer
2023-10-27 10:59   ` Christoph Heiss
2023-10-25 16:00 ` [pve-devel] [PATCH 02/12] common: copy common code from tui-installer Aaron Lauterer
2023-10-27 11:14   ` Christoph Heiss
2023-10-25 16:00 ` [pve-devel] [PATCH 03/12] common: utils: add dependency for doc test Aaron Lauterer
2023-10-25 16:00 ` [pve-devel] [PATCH 04/12] common: make InstallZfsOption public Aaron Lauterer
2023-10-25 16:00 ` [pve-devel] [PATCH 05/12] common: disk_checks: make functions public Aaron Lauterer
2023-10-25 16:00 ` [pve-devel] [PATCH 06/12] tui-installer: add dependency for new common crate Aaron Lauterer
2023-10-25 16:00 ` [pve-devel] [PATCH 07/12] tui: switch to " Aaron Lauterer
2023-10-25 16:00 ` [pve-devel] [PATCH 08/12] tui: remove now unused utils.rs Aaron Lauterer
2023-10-25 16:00 ` [pve-devel] [PATCH 09/12] common: add installer_setup method Aaron Lauterer
2023-10-25 16:00 ` [pve-devel] [PATCH 10/12] common: document " Aaron Lauterer
2023-10-25 16:00 ` [pve-devel] [PATCH 11/12] tui: use installer_setup from common cate Aaron Lauterer
2023-10-25 16:00 ` [pve-devel] [PATCH 12/12] tui: remove unused read_json function Aaron Lauterer
2023-10-27 11:06   ` Christoph Heiss
2023-10-27 11:39 ` [pve-devel] [PATCH installer] buildsys: copy over `proxmox-installer-common` crate to build directory Christoph Heiss
2023-10-27 11:41 ` [pve-devel] [PATCH 00/12] installer: add crate for common code Christoph Heiss
2023-10-30  9:45   ` Aaron Lauterer
2023-11-02 19:05 ` [pve-devel] applied-series: " Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal