Re: [pve-devel] [PATCH docs] asciidoc: add clickable anchor link to all headings
Ping. Patch still applies. On Fri, May 10, 2024 at 03:16:18PM +0200, Christoph Heiss wrote: > Works the same as in our PBS documentation and is generally common for > documentations. > > Very useful for linking specific sections of the documentation in other > places. Previously, this always had to be done by getting the correct > anchor from the HTML directly via e.g. browser devtools. > > Signed-off-by: Christoph Heiss > --- > This patch should also pretty much also apply to pmg-docs, although I > did not explicitly test that yet. > > asciidoc/pve-docs.css | 34 ++ > asciidoc/pve-html.conf | 22 +- > 2 files changed, 51 insertions(+), 5 deletions(-) > > diff --git a/asciidoc/pve-docs.css b/asciidoc/pve-docs.css > index 19c176e..9ddf57c 100644 > --- a/asciidoc/pve-docs.css > +++ b/asciidoc/pve-docs.css > @@ -1,6 +1,7 @@ > :root { > /* pre-defined colors */ > --pdt-grey-950: hsl(0deg, 0%, 95%); > +--pdt-grey-750: hsl(0deg, 0%, 75%); > --pdt-grey-400: hsl(0deg, 0%, 40%); > --pdt-grey-250: hsl(0deg, 0%, 25%); > --pdt-grey-150: hsl(0deg, 0%, 15%); > @@ -41,6 +42,39 @@ h6 { >font-size: 1.0em; > } > > +/* Support for heading anchor links */ > +h3 { > +border-bottom: unset; > +} > + > +h3 > span { > +display: inline-block; > +border-bottom: 2px solid silver; > +} > + > +a.headerlink { > +color: var(--pdt-grey-750); > +padding: 0 4px; > +text-decoration: none; > +visibility: hidden; > +} > + > +/* add it as an pseudo-element, so that it does not show up in the ToC */ > +a.headerlink::after { > +content: '\00b6'; > +text-decoration: none; > +} > + > +h1:hover > a.headerlink, > +h2:hover > a.headerlink, > +h3:hover > a.headerlink, > +h4:hover > a.headerlink, > +h5:hover > a.headerlink, > +h6:hover > a.headerlink { > + visibility: visible; > +} > + > +/* Dark mode theme */ > @media screen and (prefers-color-scheme: dark) { > :root { > color-scheme: dark; > diff --git a/asciidoc/pve-html.conf b/asciidoc/pve-html.conf > index 6e895e6..396a5e7 100644 > --- a/asciidoc/pve-html.conf > +++ b/asciidoc/pve-html.conf > @@ -505,7 +505,10 @@ bodydata=| > > [sect1] > > -{numbered?{sectnum} }{title} > + > +{numbered?{sectnum} }{title} > +{id? } > + > > | > > @@ -513,25 +516,34 @@ bodydata=| > > [sect2] > > -{numbered?{sectnum} }{title} > + > +{numbered?{sectnum} }{title} > +{id? } > + > | > > > [sect3] > > -{numbered?{sectnum} }{title} > +{numbered?{sectnum} }{title} > +{id? } > + > | > > > [sect4] > > -{title} > +{title} > +{id? } > + > | > > > [sect5] > > -{title} > +{title} > +{id? } > + > | > > > -- > 2.44.0 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer v2 0/4] assistant: clean up glob patterns & regexes
Ping. Still applies cleanly. On Mon, May 13, 2024 at 11:49:08AM +0200, Christoph Heiss wrote: > The proxmox-auto-install-assistant uses > - glob patterns for disk matching, which can be pre-compiled for > efficiency > - regexes for udev property matching, which can be simplified by some > simple prefix matching & splitting on = > > The latter also significantly reduces binary size due to the removing > the regex dependency, for details see patch #4. > > Overall no functional changes in this series. > > v1: https://lists.proxmox.com/pipermail/pve-devel/2024-May/063802.html > > Changes v1 -> v2: > * rebased on latest master > > Christoph Heiss (4): > tree-wide: run rustfmt, fix clippy warnings > assistant: drop unused `log` dependency > assistant: pre-compile ignored block device patterns > assistant: avoid regex for simple prefix matching > > proxmox-auto-install-assistant/Cargo.toml | 2 - > proxmox-auto-install-assistant/src/main.rs| 75 --- > proxmox-auto-installer/tests/parse-answer.rs | 14 ++-- > .../src/fetch_plugins/partition.rs| 10 +-- > 4 files changed, 45 insertions(+), 56 deletions(-) > > -- > 2.44.0 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 1/4] tui: fix some comment typos
Signed-off-by: Christoph Heiss --- proxmox-tui-installer/src/views/bootdisk.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs index f6fdb31..1c7b2a4 100644 --- a/proxmox-tui-installer/src/views/bootdisk.rs +++ b/proxmox-tui-installer/src/views/bootdisk.rs @@ -29,7 +29,7 @@ use proxmox_installer_common::{ /// https://openzfs.github.io/openzfs-docs/Performance%20and%20Tuning/Module%20Parameters.html#zfs-arc-max const ZFS_ARC_MIN_SIZE_MIB: usize = 64; // MiB -/// Convience wrapper when needing to take a (interior-mutable) reference to `BootdiskOptions`. +/// Convenience wrapper when needing to take a (interior-mutable) reference to `BootdiskOptions`. /// Interior mutability is safe for this case, as it is completely single-threaded. pub type BootdiskOptionsRef = Rc>; @@ -165,9 +165,9 @@ impl AdvancedBootdiskOptionsView { Self { view } } -/// Called when a new filesystem type is choosen by the user. +/// Called when a new filesystem type is chosen by the user. /// It first creates the inner (filesystem-specific) options view according to the selected -/// filesytem type. +/// filesystem type. /// Further, it replaces the (outer) bootdisk selector in the main dialog, either with a /// selector for LVM configurations or a simple label displaying the chosen RAID for ZFS and /// Btrfs configurations. -- 2.44.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 4/4] tui: bootdisk: use tabbed view for disk options on small screens
It's currently only activated for small (<=80 columns) displays, to make disk selection a lot more usable in these cases. This mostly affects serial console installation, but possibly also installations using a virtual screen via IPMI/BMC. Signed-off-by: Christoph Heiss --- proxmox-tui-installer/src/views/bootdisk.rs | 239 1 file changed, 147 insertions(+), 92 deletions(-) diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs index 29a2854..1e22105 100644 --- a/proxmox-tui-installer/src/views/bootdisk.rs +++ b/proxmox-tui-installer/src/views/bootdisk.rs @@ -4,12 +4,12 @@ use cursive::{ view::{Nameable, Resizable, ViewWrapper}, views::{ Button, Dialog, DummyView, LinearLayout, NamedView, PaddedView, Panel, ScrollView, -SelectView, TextView, +SelectView, TextView, ViewRef, }, -Cursive, View, +Cursive, Vec2, View, }; -use super::{DiskSizeEditView, FormView, IntegerEditView}; +use super::{DiskSizeEditView, FormView, IntegerEditView, TabbedView}; use crate::options::FS_TYPES; use crate::InstallerState; @@ -67,11 +67,15 @@ impl BootdiskOptionsView { let runinfo = runinfo.clone(); let options = advanced_options.clone(); move |siv| { -siv.add_layer(advanced_options_view( -, -options.clone(), -product_conf.clone(), -)); +let mut view = +advanced_options_view(, options.clone(), product_conf.clone()); + +// Pre-compute the child's layout, since it might depend on the size. Without this, +// the view will be empty until focused. +// The screen size never changes in our case, so this is completely OK. +view.layout(siv.screen_size()); + +siv.add_layer(view); } })); @@ -193,6 +197,7 @@ impl AdvancedBootdiskOptionsView { .unwrap_or_else(|| runinfo.disks[0].clone()); // Update the (inner) options view +let screen_size = siv.screen_size(); siv.call_on_name("advanced-bootdisk-options-dialog", |view: Dialog| { if let Some(AdvancedBootdiskOptionsView { view }) = view.get_content_mut().downcast_mut() @@ -203,7 +208,7 @@ impl AdvancedBootdiskOptionsView { view.add_child(LvmBootdiskOptionsView::new_with_defaults( _lvm_disk, _conf, -)); +)) } FsType::Zfs(_) => view.add_child(ZfsBootdiskOptionsView::new_with_defaults( , @@ -213,6 +218,11 @@ impl AdvancedBootdiskOptionsView { view.add_child(BtrfsBootdiskOptionsView::new_with_defaults()) } } + +// Pre-compute the child's layout, since it might depend on the size. Without this, +// the view will be empty until focused. +// The screen size never changes in our case, so this is completely OK. +view.layout(screen_size); } }); @@ -373,11 +383,98 @@ impl ViewWrapper for LvmBootdiskOptionsView { struct MultiDiskOptionsView { view: LinearLayout, +layout_data: Option<(Vec, Vec, T)>, phantom: PhantomData, } impl MultiDiskOptionsView { +const DISK_FORM_VIEW_ID: &'static str = "multidisk-disk-form"; + fn new(avail_disks: &[Disk], selected_disks: &[usize], options_view: T) -> Self { +Self { +view: LinearLayout::vertical().child(DummyView).child(DummyView), +layout_data: Some((avail_disks.to_vec(), selected_disks.to_vec(), options_view)), +phantom: PhantomData, +} +} + +fn top_panel(mut self, view: impl View) -> Self { +self.view.remove_child(0); +self.view.insert_child(0, Panel::new(view)); +self +} + +fn get_options_view( self) -> Option<> { +let inner = self.view.get_child(1)?; + +if let Some(view) = inner.downcast_ref::() { +view.get_child(2)? +.downcast_ref::()? +.get_child(2)? +.downcast_ref::() +} else if let Some(view) = inner.downcast_ref::() { +view.get(1)? +.downcast_ref::>() +.map(|v| v.get_inner()) +} else { +None +} +} + +fn get_disk_form( self) -> Option> { +let inner = self.view.get_child_mut(1)?; + +let view = if let Some(view) = inner.downcast_mut::() { +view.get_child_mut(0)? +.downcast_mut::()?
[pve-devel] [PATCH installer 2/4] tui: bootdisk: align btrfs dialog interface with zfs equivalent
Signed-off-by: Christoph Heiss --- proxmox-tui-installer/src/views/bootdisk.rs | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs index 1c7b2a4..29a2854 100644 --- a/proxmox-tui-installer/src/views/bootdisk.rs +++ b/proxmox-tui-installer/src/views/bootdisk.rs @@ -158,7 +158,7 @@ impl AdvancedBootdiskOptionsView { view.add_child(ZfsBootdiskOptionsView::new(runinfo, zfs, _conf)) } AdvancedBootdiskOptions::Btrfs(btrfs) => { -view.add_child(BtrfsBootdiskOptionsView::new(, btrfs)) +view.add_child(BtrfsBootdiskOptionsView::new(runinfo, btrfs)) } }; @@ -210,7 +210,7 @@ impl AdvancedBootdiskOptionsView { _conf, )), FsType::Btrfs(_) => { - view.add_child(BtrfsBootdiskOptionsView::new_with_defaults()) + view.add_child(BtrfsBootdiskOptionsView::new_with_defaults()) } } } @@ -520,9 +520,9 @@ struct BtrfsBootdiskOptionsView { } impl BtrfsBootdiskOptionsView { -fn new(disks: &[Disk], options: ) -> Self { +fn new(runinfo: , options: ) -> Self { let view = MultiDiskOptionsView::new( -disks, +, _disks, FormView::new().child("hdsize", DiskSizeEditView::new().content(options.disk_size)), ) @@ -531,8 +531,11 @@ impl BtrfsBootdiskOptionsView { Self { view } } -fn new_with_defaults(disks: &[Disk]) -> Self { -Self::new(disks, ::defaults_from(disks)) +fn new_with_defaults(runinfo: ) -> Self { +Self::new( +runinfo, +::defaults_from(), +) } fn get_values( self) -> Option<(Vec, BtrfsBootdiskOptions)> { -- 2.44.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 3/4] tui: views: add new TabbedView component
Add a tabbed view component, for usage in the advanced disk options dialog when selecting ZFS or Btrfs (for now). Works pretty much the same as its GUI counterpart, as much as that is possible. Signed-off-by: Christoph Heiss --- proxmox-tui-installer/src/views/mod.rs| 3 + .../src/views/tabbed_view.rs | 196 ++ 2 files changed, 199 insertions(+) create mode 100644 proxmox-tui-installer/src/views/tabbed_view.rs diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs index 3244e76..a098d1f 100644 --- a/proxmox-tui-installer/src/views/mod.rs +++ b/proxmox-tui-installer/src/views/mod.rs @@ -15,6 +15,9 @@ pub use bootdisk::*; mod install_progress; pub use install_progress::*; +mod tabbed_view; +pub use tabbed_view::*; + mod table_view; pub use table_view::*; diff --git a/proxmox-tui-installer/src/views/tabbed_view.rs b/proxmox-tui-installer/src/views/tabbed_view.rs new file mode 100644 index 000..2aeea6a --- /dev/null +++ b/proxmox-tui-installer/src/views/tabbed_view.rs @@ -0,0 +1,196 @@ +use std::borrow::{Borrow, BorrowMut}; + +use cursive::{ +direction::Direction, +event::{AnyCb, Event, EventResult, Key}, +theme::{ColorStyle, PaletteColor}, +utils::{markup::StyledString, span::SpannedStr}, +view::{CannotFocus, IntoBoxedView, Selector, ViewNotFound}, +Printer, Vec2, View, +}; + +pub struct TabbedView { +/// All tab views in format (name, view) +views: Vec<(String, Box)>, +/// Currently active tab index +current: usize, +/// Whether the tab bar has focus currently +bar_has_focus: bool, +} + +impl TabbedView { +/// Creates a view with multiple tabs. +pub fn new() -> Self { +Self { +views: vec![], +current: 0, +bar_has_focus: false, +} +} + +/// Adds a tab to the view. The `name` is the string displayed at the top. +/// +/// Chainable variant. +pub fn tab(mut self, name: , view: V) -> Self +where +V: 'static + IntoBoxedView, +{ +assert!(!name.is_empty()); +self.views.push((name.to_owned(), view.into_boxed_view())); +self +} + +/// Returns a reference to the specified tab content view. +pub fn get(, index: usize) -> Option< View> { +self.views.get(index).map(|(_, view)| view.borrow()) +} + +/// Returns a mutable reference to the specified tab content view. +pub fn get_mut( self, index: usize) -> Option< dyn View> { +self.views.get_mut(index).map(|(_, view)| view.borrow_mut()) +} + +/// Draws the border around the tab view and the name header for each tab. +fn draw_border(, p: ) { +let names_len: usize = self.views.iter().map(|(name, _)| name.len() + 2).sum(); +let tabbar_width = names_len + self.views.len() + 1; + +let top_border_width = (p.output_size.x - tabbar_width) / 2; +p.print_box((0, 0), p.output_size, false); + +self.print_tab_names(p.offset((top_border_width, 0))); +} + +/// Draws all tab names with appropriate highlighting, depending on its state, +/// to the specified printer `p` at `(0, 0)`. +fn print_tab_names(, p: Printer) { +let mut pos = Vec2::zero(); +for (index, name) in self.views.iter().map(|(name, _)| name).enumerate() { +p.print(pos, "| "); +pos.x += 2; + +if index == self.current { +self.print_active_tab_name(name, p.offset(pos)); +} else { +p.print(pos, name); +} + +pos.x += name.len(); +p.print(pos, " "); +pos.x += 1; + +p.print(pos, "|"); +} +} + +/// Draws the active tab name to the printer `p`, with its highlighting +/// additionally depending upon whether the tab bar currently has focus or not. +fn print_active_tab_name(, name: , p: Printer) { +let background = if self.bar_has_focus { +PaletteColor::Highlight +} else { +PaletteColor::HighlightInactive +}; + +p.print_styled( +(0, 0), +SpannedStr::from(::styled( +name, +ColorStyle::new(PaletteColor::HighlightText, background), +)), +) +} +} + +impl View for TabbedView { +fn draw(, printer: ) { +self.draw_border(printer); + +if let Some(view) = self.get(self.current) { +view.draw(((1, 1)).focused(!self.bar_has_focus)); +} +} + +fn layout( self, size: Vec2) { +for (_, view) in self.views.iter_mut() { +view.layout(size.saturating_sub((2, 2))); +} +} + +fn required_size( self, constraint: Vec2) -> Vec2 { +if let Some(view) = self.get_mut(self.current) { +view.required_size
[pve-devel] [PATCH installer 0/4] tui: make disk options view tabbed on small screens
This adds a tabbed view component, for usage in the advanced disk options dialog when selecting ZFS or Btrfs. Works pretty much the same as its GUI counterpart, as much as that is possible. It's currently only activated for small (<=80 columns) displays, to make disk selection a lot more usable in these cases. This mostly affects serial console installation, but possibly also installations using a virtual screen via IPMI/BMC. Testing can be done using the `stty` to set specific terminal sizes, e.g. `stty columns 80 rows 24` for a standard VT100-spec terminal. This componont/view may also be made the default for the advanced disk options dialog, to align the TUI with it GUI in more cases - I'm open for discussion on that. Would also simplify the code a lot, so there are certainly other benefits to it as well. Christoph Heiss (4): tui: fix some comment typos tui: bootdisk: align btrfs dialog interface with zfs equivalent tui: views: add new TabbedView component tui: bootdisk: use tabbed view for disk options on small screens proxmox-tui-installer/src/views/bootdisk.rs | 260 +++--- proxmox-tui-installer/src/views/mod.rs| 3 + .../src/views/tabbed_view.rs | 196 + 3 files changed, 358 insertions(+), 101 deletions(-) create mode 100644 proxmox-tui-installer/src/views/tabbed_view.rs -- 2.44.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH stable-7 container 2/2] setup: unlink default netplan configuration even with Ubuntu >= 23.04
From: Fiona Ebner It seems like commit 02d9462 ("setup: enable systemd-networkd via preset for ubuntu 23.04+") also resulted in the default netplan configuration no longer being unlinked. That should still happen, even if systemd-networkd is now enabled via preset. Otherwise, the network configuration created by Proxmox VE is not ordered before the one generated by netplan and thus not applied by systemd-networkd. Reported in the community forum: https://forum.proxmox.com/threads/145848/post-658058 Fixes: 02d9462 ("setup: enable systemd-networkd via preset for ubuntu 23.04+") Signed-off-by: Fiona Ebner (cherry picked from commit dfcbad017361d4e3ded20af573fbaeacc05231eb) Signed-off-by: Christoph Heiss --- src/PVE/LXC/Setup/Ubuntu.pm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/PVE/LXC/Setup/Ubuntu.pm b/src/PVE/LXC/Setup/Ubuntu.pm index cea8ef5..897eab9 100644 --- a/src/PVE/LXC/Setup/Ubuntu.pm +++ b/src/PVE/LXC/Setup/Ubuntu.pm @@ -73,7 +73,9 @@ sub template_fixup { '/etc/systemd/system/multi-user.target.wants/systemd-networkd.service'); $self->ct_symlink('/lib/systemd/system/systemd-networkd.socket', '/etc/systemd/system/socket.target.wants/systemd-networkd.socket'); +} +if ($version >= '17.10') { # unlink default netplan lxc config $self->ct_unlink('/etc/netplan/10-lxc.yaml'); } -- 2.44.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH stable-7 container 0/2] setup: cherry-pick ubuntu 24.04 support
This backports the series to support Ubuntu 24.04 "Noble" CTs [0]. They could be cherry-picked cleanly. Tested on a up-to-date PVE 7.4 system (running on `pvetest`), template for Ubuntu 24.04 (`ubuntu-24.04-standard_24.04-2_amd64.tar.zst`) was copied from a 8.2 host for testing. Tested creating new CT from that template, using either a static IP or DHCP. Then working around with the system a bit, everything seems to work just fine. [0] https://lists.proxmox.com/pipermail/pve-devel/2024-April/063766.html Fiona Ebner (2): setup: support Ubuntu 24.04 Noble setup: unlink default netplan configuration even with Ubuntu >= 23.04 src/PVE/LXC/Setup/Ubuntu.pm | 3 +++ 1 file changed, 3 insertions(+) -- 2.44.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH stable-7 container 1/2] setup: support Ubuntu 24.04 Noble
From: Fiona Ebner Minimally tested, that an upgrade from an existing 23.04 container works, there still is network and no obviously bad messages in the container's journal. Reported in the community forum: https://forum.proxmox.com/threads/145848/ Signed-off-by: Fiona Ebner (cherry picked from commit 3d800f832c25e4bf2435d88ab190fd8e681a67b1) Signed-off-by: Christoph Heiss --- src/PVE/LXC/Setup/Ubuntu.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/src/PVE/LXC/Setup/Ubuntu.pm b/src/PVE/LXC/Setup/Ubuntu.pm index 905cacb..cea8ef5 100644 --- a/src/PVE/LXC/Setup/Ubuntu.pm +++ b/src/PVE/LXC/Setup/Ubuntu.pm @@ -12,6 +12,7 @@ use PVE::LXC::Setup::Debian; use base qw(PVE::LXC::Setup::Debian); my $known_versions = { +'24.04' => 1, # noble '23.10' => 1, # mantic '23.04' => 1, # lunar '22.10' => 1, # kinetic -- 2.44.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 7/7] proxinstall: use email regex from HTML specification for validation
That regex should be a lot more accurate in what it allows - if it's good enough for the HTML spec, it should be for us too. Signed-off-by: Christoph Heiss --- Proxmox/Makefile | 1 + Proxmox/Sys.pm | 9 + proxinstall | 3 ++- 3 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 Proxmox/Sys.pm diff --git a/Proxmox/Makefile b/Proxmox/Makefile index 9561d9b..6f9eea8 100644 --- a/Proxmox/Makefile +++ b/Proxmox/Makefile @@ -12,6 +12,7 @@ PERL_MODULES=\ Install/RunEnv.pm \ Install/StorageConfig.pm \ Log.pm \ +Sys.pm \ Sys/Block.pm \ Sys/Command.pm \ Sys/File.pm \ diff --git a/Proxmox/Sys.pm b/Proxmox/Sys.pm new file mode 100644 index 000..afc6780 --- /dev/null +++ b/Proxmox/Sys.pm @@ -0,0 +1,9 @@ +package Proxmox::Sys; + +use strict; +use warnings; + +# The HTML specification actually gives a "blessed" regex for email addresses: +# https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address +# Using that /should/ cover all possible cases that are encountered in the wild. +our $EMAIL_RE = '^[a-zA-Z0-9.!#$%&\'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$'; diff --git a/proxinstall b/proxinstall index a6a4cfb..79b0c76 100755 --- a/proxinstall +++ b/proxinstall @@ -33,6 +33,7 @@ my $iso_env = Proxmox::Install::ISOEnv::get(); use Proxmox::Install; use Proxmox::Install::Config; +use Proxmox::Sys; use Proxmox::Sys::Block qw(get_cached_disks); use Proxmox::Sys::Command qw(syscmd); use Proxmox::Sys::File qw(file_read_all file_write_all); @@ -733,7 +734,7 @@ sub create_password_view { } my $t3 = $eme->get_text; - if ($t3 !~ m/^[\w\+\-\~]+(\.[\w\+\-\~]+)*@[a-zA-Z0-9\-]+(\.[a-zA-Z0-9\-]+)*$/) { + if ($t3 !~ m/$Proxmox::Sys::EMAIL_RE/) { Proxmox::UI::message("Email does not look like a valid address (user\@domain.tld)"); $eme->grab_focus(); return; -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 5/7] tui: use email regex from HTML specification for validation
That regex should be a lot more accurate in what it allows - if it's good enough for the HTML spec, it should be for us too. Signed-off-by: Christoph Heiss --- proxmox-installer-common/Cargo.toml | 1 + proxmox-installer-common/src/options.rs | 29 - proxmox-tui-installer/Cargo.toml| 1 - proxmox-tui-installer/src/main.rs | 18 +-- 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml index 70f828a..e151b0e 100644 --- a/proxmox-installer-common/Cargo.toml +++ b/proxmox-installer-common/Cargo.toml @@ -8,6 +8,7 @@ exclude = [ "build", "debian" ] homepage = "https://www.proxmox.com; [dependencies] +anyhow.workspace = true regex = "1.7" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs index e77914b..1962f87 100644 --- a/proxmox-installer-common/src/options.rs +++ b/proxmox-installer-common/src/options.rs @@ -1,5 +1,8 @@ +use anyhow::{bail, Result}; +use regex::Regex; use serde::Deserialize; use std::net::{IpAddr, Ipv4Addr}; +use std::sync::OnceLock; use std::{cmp, fmt}; use crate::setup::{ @@ -327,6 +330,8 @@ impl TimezoneOptions { } } +const EMAIL_DEFAULT_PLACEHOLDER: = "mail@example.invalid"; + #[derive(Clone, Debug)] pub struct PasswordOptions { pub email: String, @@ -336,7 +341,7 @@ pub struct PasswordOptions { impl Default for PasswordOptions { fn default() -> Self { Self { -email: "mail@example.invalid".to_string(), +email: EMAIL_DEFAULT_PLACEHOLDER.to_owned(), root_password: String::new(), } } @@ -418,6 +423,28 @@ impl NetworkOptions { } } +/// Validates an email address using the regex for elements +/// as defined in the HTML specification [0]. +/// Using that /should/ cover all possible cases that are encountered in the wild. +/// +/// It additionally checks whether the email our default email placeholder value. +/// +/// [0] https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address +pub fn email_validate(email: ) -> Result<()> { +static RE: OnceLock = OnceLock::new(); +let re = RE.get_or_init(|| { + Regex::new(r"^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$").unwrap() +}); + +if !re.is_match(email) { +bail!("Email does not look like a valid address (u...@domain.tld)") +} else if email == EMAIL_DEFAULT_PLACEHOLDER { +bail!("Invalid (default) email address") +} + +Ok(()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/proxmox-tui-installer/Cargo.toml b/proxmox-tui-installer/Cargo.toml index fc653f0..d3087d8 100644 --- a/proxmox-tui-installer/Cargo.toml +++ b/proxmox-tui-installer/Cargo.toml @@ -11,5 +11,4 @@ homepage = "https://www.proxmox.com; cursive = { version = "0.20.0", default-features = false, features = ["termion-backend"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" -regex = "1.7" proxmox-installer-common = { path = "../proxmox-installer-common" } diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs index a24fb0b..67dd479 100644 --- a/proxmox-tui-installer/src/main.rs +++ b/proxmox-tui-installer/src/main.rs @@ -13,13 +13,11 @@ use cursive::{ Cursive, CursiveRunnable, ScreenId, View, XY, }; -use regex::Regex; - mod options; use options::InstallerOptions; use proxmox_installer_common::{ -options::{BootdiskOptions, NetworkOptions, PasswordOptions, TimezoneOptions}, +options::{email_validate, BootdiskOptions, NetworkOptions, PasswordOptions, TimezoneOptions}, setup::{installer_setup, LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo}, utils::Fqdn, }; @@ -448,18 +446,12 @@ fn password_dialog(siv: Cursive) -> InstallerView { .get_value::(2) .ok_or("failed to retrieve email")?; -let email_regex = - Regex::new(r"^[\w\+\-\~]+(\.[\w\+\-\~]+)*@[a-zA-Z0-9\-]+(\.[a-zA-Z0-9\-]+)*$") -.unwrap(); - if root_password.len() < 5 { -Err("password too short, must be at least 5 characters long") +Err("password too short, must be at least 5 characters long".to_owned()) } else if root_password != confirm_password { -Err("passwords do not match") -} else if email == "mail@example.invalid" { -
[pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation
This uses the regex for elements as defined in the HTML spec to validate emails in the installer. That regex should be a lot more robust than our current on and cover basically all cases of email addresses that might be encountered in the while. Also, patch #6 adds validation for the auto-installer `global.mailto` option too, while at it. Patches #1 through #4 are just small cleanups/fixes and may be applied independently. [0] https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address Christoph Heiss (7): cargo: convert `anyhow` to workspace dependency tui: fix new clippy lint auto-installer: drop some unneeded `pub` modifiers auto-installer: print full anyhow message on failure tui: use email regex from HTML specification for validation auto-installer: validate `global.mailto` answer option proxinstall: use email regex from HTML specification for validation Cargo.toml| 2 ++ Proxmox/Makefile | 1 + Proxmox/Sys.pm| 9 ++ proxinstall | 3 +- proxmox-auto-install-assistant/Cargo.toml | 2 +- proxmox-auto-installer/Cargo.toml | 2 +- .../src/bin/proxmox-auto-installer.rs | 2 +- proxmox-auto-installer/src/utils.rs | 17 ++- proxmox-chroot/Cargo.toml | 2 +- proxmox-fetch-answer/Cargo.toml | 2 +- proxmox-installer-common/Cargo.toml | 1 + proxmox-installer-common/src/options.rs | 29 ++- proxmox-tui-installer/Cargo.toml | 1 - proxmox-tui-installer/src/main.rs | 22 +- 14 files changed, 65 insertions(+), 30 deletions(-) create mode 100644 Proxmox/Sys.pm -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 4/7] auto-installer: print full anyhow message on failure
Otherwise, context info is simply lost -- which might contain valuable information for the user. Signed-off-by: Christoph Heiss --- proxmox-auto-installer/src/bin/proxmox-auto-installer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs index 9fcec1e..ef42dc6 100644 --- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs +++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs @@ -86,7 +86,7 @@ fn main() -> ExitCode { match run_installation(, , _info, _info, _info) { Ok(_) => info!("Installation done."), Err(err) => { -error!("Installation failed: {err}"); +error!("Installation failed: {err:#}"); return exit_failure(answer.global.reboot_on_error); } } -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 6/7] auto-installer: validate `global.mailto` answer option
Signed-off-by: Christoph Heiss --- proxmox-auto-installer/src/utils.rs | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs index f752924..7241eec 100644 --- a/proxmox-auto-installer/src/utils.rs +++ b/proxmox-auto-installer/src/utils.rs @@ -1,4 +1,4 @@ -use anyhow::{bail, Context as _, Result}; +use anyhow::{bail, Context, Result}; use clap::ValueEnum; use glob::Pattern; use log::info; @@ -9,7 +9,7 @@ use crate::{ udevinfo::UdevInfo, }; use proxmox_installer_common::{ -options::{FsType, NetworkOptions, ZfsChecksumOption, ZfsCompressOption}, +options::{email_validate, FsType, NetworkOptions, ZfsChecksumOption, ZfsCompressOption}, setup::{InstallConfig, InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo}, }; use serde::{Deserialize, Serialize}; @@ -272,7 +272,10 @@ fn get_first_selected_disk(config: ) -> usize { .expect("could not parse key to usize") } -fn verify_locale_settings(answer: , locales: ) -> Result<()> { +fn verify_global_settings(answer: , locales: ) -> Result<()> { +info!("Verifying global settings"); +email_validate().with_context(|| answer.global.mailto.clone())?; + info!("Verifying locale settings"); if !locales .countries @@ -315,7 +318,7 @@ pub fn parse_answer( let network_settings = get_network_settings(answer, udev_info, runtime_info, setup_info)?; -verify_locale_settings(answer, locales)?; +verify_global_settings(answer, locales)?; let mut config = InstallConfig { autoreboot: 1_usize, -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 3/7] auto-installer: drop some unneeded `pub` modifiers
Signed-off-by: Christoph Heiss --- proxmox-auto-installer/src/utils.rs | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs index 202ad41..f752924 100644 --- a/proxmox-auto-installer/src/utils.rs +++ b/proxmox-auto-installer/src/utils.rs @@ -14,7 +14,7 @@ use proxmox_installer_common::{ }; use serde::{Deserialize, Serialize}; -pub fn get_network_settings( +fn get_network_settings( answer: , udev_info: , runtime_info: , @@ -143,7 +143,7 @@ pub fn get_matched_udev_indexes( Ok(matches) } -pub fn set_disks( +fn set_disks( answer: , udev_info: , runtime_info: , @@ -261,7 +261,7 @@ fn set_selected_disks( Ok(()) } -pub fn get_first_selected_disk(config: ) -> usize { +fn get_first_selected_disk(config: ) -> usize { config .disk_selection .iter() @@ -272,7 +272,7 @@ pub fn get_first_selected_disk(config: ) -> usize { .expect("could not parse key to usize") } -pub fn verify_locale_settings(answer: , locales: ) -> Result<()> { +fn verify_locale_settings(answer: , locales: ) -> Result<()> { info!("Verifying locale settings"); if !locales .countries -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 1/7] cargo: convert `anyhow` to workspace dependency
Signed-off-by: Christoph Heiss --- Cargo.toml| 2 ++ proxmox-auto-install-assistant/Cargo.toml | 2 +- proxmox-auto-installer/Cargo.toml | 2 +- proxmox-chroot/Cargo.toml | 2 +- proxmox-fetch-answer/Cargo.toml | 2 +- 5 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1e730ce..ec1deaf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,3 +9,5 @@ members = [ "proxmox-tui-installer", ] +[workspace.dependencies] +anyhow = "1.0" diff --git a/proxmox-auto-install-assistant/Cargo.toml b/proxmox-auto-install-assistant/Cargo.toml index eaca7f8..b30a157 100644 --- a/proxmox-auto-install-assistant/Cargo.toml +++ b/proxmox-auto-install-assistant/Cargo.toml @@ -11,7 +11,7 @@ exclude = [ "build", "debian" ] homepage = "https://www.proxmox.com; [dependencies] -anyhow = "1.0" +anyhow.workspace = true clap = { version = "4.0", features = ["derive"] } glob = "0.3" log = "0.4.20" diff --git a/proxmox-auto-installer/Cargo.toml b/proxmox-auto-installer/Cargo.toml index 4f54439..efb0fe3 100644 --- a/proxmox-auto-installer/Cargo.toml +++ b/proxmox-auto-installer/Cargo.toml @@ -11,7 +11,7 @@ exclude = [ "build", "debian" ] homepage = "https://www.proxmox.com; [dependencies] -anyhow = "1.0" +anyhow.workspace = true clap = { version = "4.0", features = ["derive"] } glob = "0.3" log = "0.4.20" diff --git a/proxmox-chroot/Cargo.toml b/proxmox-chroot/Cargo.toml index 43b96ff..6247baa 100644 --- a/proxmox-chroot/Cargo.toml +++ b/proxmox-chroot/Cargo.toml @@ -8,7 +8,7 @@ exclude = [ "build", "debian" ] homepage = "https://www.proxmox.com; [dependencies] -anyhow = "1.0" +anyhow.workspace = true clap = { version = "4.0", features = ["derive"] } nix = "0.26.1" proxmox-installer-common = { path = "../proxmox-installer-common" } diff --git a/proxmox-fetch-answer/Cargo.toml b/proxmox-fetch-answer/Cargo.toml index 964682a..881979b 100644 --- a/proxmox-fetch-answer/Cargo.toml +++ b/proxmox-fetch-answer/Cargo.toml @@ -13,7 +13,7 @@ homepage = "https://www.proxmox.com; # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -anyhow = "1.0" +anyhow.workspace = true hex = "0.4" log = "0.4.20" native-tls = "0.2" -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer v3 0/3] expose zfs arc size setting for all products
v4: https://lists.proxmox.com/pipermail/pve-devel/2024-May/063957.html ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer v4 3/3] proxinstall: expose arc size setting for zfs bootdisks for all products
For non-PVE products, simply use the ZFS defaults (aka. 50%) and leave unset, if the user never touches that setting. Signed-off-by: Christoph Heiss --- Changes v3 -> v4: * no changes Changes v2 -> v3: * rework based on Maximilano's suggestion using Gtk3::Adjustment Changes v1 -> v2: * add some more explanatory comments Signed-off-by: Christoph Heiss --- Proxmox/Install/RunEnv.pm | 3 ++- proxinstall | 26 +- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm index 7eaf96a..8322603 100644 --- a/Proxmox/Install/RunEnv.pm +++ b/Proxmox/Install/RunEnv.pm @@ -329,7 +329,8 @@ our $ZFS_ARC_SYSMEM_PERCENTAGE = 0.1; # use 10% of available system memory by de # Calculates the default upper limit for the ZFS ARC size. # Returns the default ZFS maximum ARC size in MiB. sub default_zfs_arc_max { -# Use ZFS default on non-PVE +# For products other the PVE, just let ZFS decide on its own. Setting `0` +# causes the installer to skip writing the `zfs_arc_max` module parameter. return 0 if Proxmox::Install::ISOEnv::get('product') ne 'pve'; my $default_mib = get('total_memory') * $ZFS_ARC_SYSMEM_PERCENTAGE; diff --git a/proxinstall b/proxinstall index a6a4cfb..bc0e1e4 100755 --- a/proxinstall +++ b/proxinstall @@ -1138,20 +1138,20 @@ my $create_raid_advanced_grid = sub { $spinbutton_copies->set_value($copies); push @$labeled_widgets, ['copies', $spinbutton_copies]; -if ($iso_env->{product} eq 'pve') { - my $total_memory = Proxmox::Install::RunEnv::get('total_memory'); +my $total_memory = Proxmox::Install::RunEnv::get('total_memory'); +my $arc_max = Proxmox::Install::Config::get_zfs_opt('arc_max') || ($total_memory * 0.5); + +my $arc_max_adjustment = Gtk3::Adjustment->new( + $arc_max, $Proxmox::Install::RunEnv::ZFS_ARC_MIN_SIZE_MIB, + $total_memory, 1, 10, 0); +my $spinbutton_arc_max = Gtk3::SpinButton->new($arc_max_adjustment, 1, 0); +$spinbutton_arc_max->set_tooltip_text('Maximum ARC size in megabytes'); +$spinbutton_arc_max->signal_connect('value-changed' => sub { + my $w = shift; + Proxmox::Install::Config::set_zfs_opt('arc_max', $w->get_value_as_int()); +}); - my $spinbutton_arc_max = Gtk3::SpinButton->new_with_range( - $Proxmox::Install::RunEnv::ZFS_ARC_MIN_SIZE_MIB, $total_memory, 1); - $spinbutton_arc_max->set_tooltip_text('Maximum ARC size in megabytes'); - $spinbutton_arc_max->signal_connect('value-changed' => sub { - my $w = shift; - Proxmox::Install::Config::set_zfs_opt('arc_max', $w->get_value_as_int()); - }); - my $arc_max = Proxmox::Install::Config::get_zfs_opt('arc_max'); - $spinbutton_arc_max->set_value($arc_max); - push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB']; -} +push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB']; push @$labeled_widgets, ['hdsize', $hdsize_btn, 'GB']; return $create_label_widget_grid->($labeled_widgets);; -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer v4 2/3] tui: expose arc size setting for zfs bootdisks for all products
For non-PVE products, simply use the ZFS defaults (aka. 50%) and leave unset, if the user never touches that setting. Signed-off-by: Christoph Heiss --- Changes v3 -> v4: * rebased on latest master, trivial conflict Changes v2 -> v3: * no changes Changes v1 -> v2: * use new placeholder functionality for non-PVE products Signed-off-by: Christoph Heiss --- proxmox-installer-common/src/options.rs | 3 +- proxmox-tui-installer/src/views/bootdisk.rs | 42 - proxmox-tui-installer/src/views/mod.rs | 1 - 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs index e77914b..0534ace 100644 --- a/proxmox-installer-common/src/options.rs +++ b/proxmox-installer-common/src/options.rs @@ -214,7 +214,8 @@ impl ZfsBootdiskOptions { /// The default ZFS maximum ARC size in MiB for this system. fn default_zfs_arc_max(product: ProxmoxProduct, total_memory: usize) -> usize { if product != ProxmoxProduct::PVE { -// Use ZFS default for non-PVE +// For products other the PVE, just let ZFS decide on its own. Setting `0` +// causes the installer to skip writing the `zfs_arc_max` module parameter. 0 } else { ((total_memory as f64) / 10.) diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs index f6fdb31..3d9be50 100644 --- a/proxmox-tui-installer/src/views/bootdisk.rs +++ b/proxmox-tui-installer/src/views/bootdisk.rs @@ -564,7 +564,18 @@ impl ZfsBootdiskOptionsView { options: , product_conf: , ) -> Self { -let is_pve = product_conf.product == ProxmoxProduct::PVE; +let arc_max_view = { +let view = IntegerEditView::new_with_suffix("MiB").max_value(runinfo.total_memory); + +// For PVE "force" the default value, for other products place the recommended value +// only in the placeholder. This causes for the latter to not write the module option +// if the value is never modified by the user. +if product_conf.product == ProxmoxProduct::PVE { +view.content(options.arc_max) +} else { +view.placeholder(runinfo.total_memory / 2) +} +}; let inner = FormView::new() .child("ashift", IntegerEditView::new().content(options.ashift)) @@ -596,13 +607,7 @@ impl ZfsBootdiskOptionsView { "copies", IntegerEditView::new().content(options.copies).max_value(3), ) -.child_conditional( -is_pve, -"ARC max size", -IntegerEditView::new_with_suffix("MiB") -.max_value(runinfo.total_memory) -.content(options.arc_max), -) +.child("ARC max size", arc_max_view) .child("hdsize", DiskSizeEditView::new().content(options.disk_size)); let view = MultiDiskOptionsView::new(, _disks, inner) @@ -624,21 +629,22 @@ impl ZfsBootdiskOptionsView { fn get_values( self) -> Option<(Vec, ZfsBootdiskOptions)> { let (disks, selected_disks) = self.view.get_disks_and_selection()?; let view = self.view.inner_mut()?; -let has_arc_max = view.len() >= 6; -let disk_size_index = if has_arc_max { 5 } else { 4 }; let ashift = view.get_value::(0)?; let compress = view.get_value::, _>(1)?; let checksum = view.get_value::, _>(2)?; let copies = view.get_value::(3)?; -let disk_size = view.get_value::(disk_size_index)?; - -let arc_max = if has_arc_max { -view.get_value::(4)? -.max(ZFS_ARC_MIN_SIZE_MIB) -} else { -0 // use built-in ZFS default value -}; +let disk_size = view.get_value::(5)?; + +// If a value is set, return that and clamp it to at least [`ZFS_ARC_MIN_SIZE_MIB`]. +// +// Otherwise, if no value was set or an error occured return `0`. The former simply means +// that the placeholder value is still there. +let arc_max = view +.get_child::(4)? +.get_content_maybe() +.map_or(Ok(0), |v| v.map(|v| v.max(ZFS_ARC_MIN_SIZE_MIB))) +.unwrap_or(0); Some(( disks, diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs index 5e5f4fb..eab258c 100644 --- a/proxmox-tui-installer/src/views/mod.rs +++ b/proxmox-tui-installer/src/views/mod.rs @@ -93,7 +93,6 @@ impl NumericEditView { /// /// # Arguments /// `placeholder` - The placeholder value to set for this view. -#[allow(unused)] pub fn placeholder(mut self, pl
[pve-devel] [PATCH installer v4 1/3] tui: NumericEditView: add optional placeholder value
Enables to add an optional placeholder value to `NumericEditView`, which will be displayed in a different (darker) color and not returned by `.get_content*()`. Can be used for having default values in the TUI, but with different handling in the back. Signed-off-by: Christoph Heiss --- Changes v3 -> v4: * no changes Changes v2 -> v3: * when empty & focused, do not show the placeholder value at all Changes v1 -> v2: * new patch Signed-off-by: Christoph Heiss --- proxmox-tui-installer/src/views/mod.rs | 66 -- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs index 3244e76..5e5f4fb 100644 --- a/proxmox-tui-installer/src/views/mod.rs +++ b/proxmox-tui-installer/src/views/mod.rs @@ -2,9 +2,10 @@ use std::{net::IpAddr, rc::Rc, str::FromStr}; use cursive::{ event::{Event, EventResult}, +theme::BaseColor, view::{Resizable, ViewWrapper}, views::{EditView, LinearLayout, NamedView, ResizedView, SelectView, TextView}, -Rect, Vec2, View, +Printer, Rect, Vec2, View, }; use proxmox_installer_common::utils::CidrAddress; @@ -24,6 +25,7 @@ pub use timezone::*; pub struct NumericEditView { view: LinearLayout, max_value: Option, +placeholder: Option, max_content_width: Option, allow_empty: bool, } @@ -36,6 +38,7 @@ impl NumericEditView { Self { view, max_value: None, +placeholder: None, max_content_width: None, allow_empty: false, } @@ -54,6 +57,7 @@ impl NumericEditView { Self { view, max_value: None, +placeholder: None, max_content_width: None, allow_empty: false, } @@ -84,15 +88,42 @@ impl NumericEditView { self } +/// Sets a placeholder value for this view. Implies `allow_empty(true)`. +/// Implies `allow_empty(true)`. +/// +/// # Arguments +/// `placeholder` - The placeholder value to set for this view. +#[allow(unused)] +pub fn placeholder(mut self, placeholder: T) -> Self { +self.placeholder = Some(placeholder); +self.allow_empty(true) +} + +/// Returns the current value of the view. If a placeholder is defined and +/// no value is currently set, the placeholder value is returned. +/// +/// **This should only be called when `allow_empty = false` or a placeholder +/// is set.** pub fn get_content() -> Result::Err> { -assert!(!self.allow_empty); -self.inner().get_content().parse() +let content = self.inner().get_content(); + +if content.is_empty() { +if let Some(placeholder) = self.placeholder { +return Ok(placeholder); +} +} + +assert!(!(self.allow_empty && self.placeholder.is_none())); +content.parse() } +/// Returns the current value of the view, or [`None`] if the view is +/// currently empty. pub fn get_content_maybe() -> Option::Err>> { let content = self.inner().get_content(); + if !content.is_empty() { -Some(self.inner().get_content().parse()) +Some(content.parse()) } else { None } @@ -157,6 +188,25 @@ impl NumericEditView { std::mem::swap(self.inner_mut(), inner); self } + +/// Generic `wrap_draw()` implementation for [`ViewWrapper`]. +/// +/// # Arguments +/// * `printer` - The [`Printer`] to draw to the base view. +fn wrap_draw_inner(, printer: ) { +self.view.draw(printer); + +if self.inner().get_content().is_empty() && !printer.focused { +if let Some(placeholder) = self.placeholder { +let placeholder = placeholder.to_string(); + +printer.with_color( +(BaseColor::Blue.light(), BaseColor::Blue.dark()).into(), +|printer| printer.print((0, 0), ), +); +} +} +} } pub type FloatEditView = NumericEditView; @@ -165,6 +215,10 @@ pub type IntegerEditView = NumericEditView; impl ViewWrapper for FloatEditView { cursive::wrap_impl!(self.view: LinearLayout); +fn wrap_draw(, printer: ) { +self.wrap_draw_inner(printer); +} + fn wrap_on_event( self, event: Event) -> EventResult { let original = self.inner_mut().get_content(); @@ -204,6 +258,10 @@ impl FloatEditView { impl ViewWrapper for IntegerEditView { cursive::wrap_impl!(self.view: LinearLayout); +fn wrap_draw(, printer: ) { +self.wrap_draw_inner(printer); +} + fn wrap_on_event( self, event: Event) -> EventResult { let original = self.inner_mut().get_content(); -- 2.44.0 ___ pve
[pve-devel] [PATCH installer v4 0/3] expose zfs arc size setting for all products
As suggested by Thomas, leaves the ZFS default if the user never touches the setting in the installer (i.e. not writing a modprobe file). See also the discussion in v1 [0]. [0] https://lists.proxmox.com/pipermail/pve-devel/2024-February/061659.html Testing --- Tested the installation of PVE and PBS (PMG is handled the same as PBS), with each once letting the arc size setting untouched and once setting it to some specific value. Also checked for each whether the correct default value was displayed. Afterwards, checked that for PVE the module parameter was always written to /etc/modprobe.d/, for PBS that it was only written in case it was explicitly set. Previous revisions -- v3: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062976.html v2: https://lists.proxmox.com/pipermail/pve-devel/2024-February/061667.html v1: https://lists.proxmox.com/pipermail/pve-devel/2023-November/060898.html Changes v3 -> v4: * rebased on latest master Changes v2 -> v3: * tui: when empty & focused, do not show the placeholder value at all * gui: rework using Gtk3::Adjustment based on Maximilanos suggestion Changes v1 -> v2: * rebased on latest master * add placeholder functionality for arc max size in TUI * "emulate" placeholder functionality in GTK on best-effort basis Christoph Heiss (3): tui: NumericEditView: add optional placeholder value tui: expose arc size setting for zfs bootdisks for all products proxinstall: expose arc size setting for zfs bootdisks for all products Proxmox/Install/RunEnv.pm | 3 +- proxinstall | 26 - proxmox-installer-common/src/options.rs | 3 +- proxmox-tui-installer/src/views/bootdisk.rs | 45 -- proxmox-tui-installer/src/views/mod.rs | 65 +++-- 5 files changed, 105 insertions(+), 37 deletions(-) -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer 6/7] auto-installer: add new `global.root_password_hashed` answer option
On Thu, May 23, 2024 at 02:19:34PM +0200, Christoph Heiss wrote: > This allows user to specify the root password in a hashed format, > generated using e.g. mkpasswd(1), instead of plaintext. > > Signed-off-by: Christoph Heiss > --- > proxmox-auto-installer/src/answer.rs | 3 ++- > proxmox-auto-installer/src/utils.rs | 4 > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/proxmox-auto-installer/src/answer.rs > b/proxmox-auto-installer/src/answer.rs > index aab7198..d691da1 100644 > --- a/proxmox-auto-installer/src/answer.rs > +++ b/proxmox-auto-installer/src/answer.rs > @@ -26,7 +26,8 @@ pub struct Global { > pub keyboard: KeyboardLayout, > pub mailto: String, > pub timezone: String, > -pub root_password: String, > +pub root_password: Option, > +pub root_password_hashed: Option, > #[serde(default)] > pub reboot_on_error: bool, > #[serde(default)] > diff --git a/proxmox-auto-installer/src/utils.rs > b/proxmox-auto-installer/src/utils.rs > index f1425b0..2b9930d 100644 > --- a/proxmox-auto-installer/src/utils.rs > +++ b/proxmox-auto-installer/src/utils.rs > @@ -319,6 +319,10 @@ pub fn parse_answer( > > verify_locale_settings(answer, locales)?; > > +if answer.global.root_password.is_some() == > answer.global.root_password_hashed.is_some() { > +bail!("`root_password` and `root_password_hashed` cannot be set at > the same time"); > +} > + > let mut config = InstallConfig { > autoreboot: 1_usize, > filesys: filesystem, Seems a part of the diff went missing, probably while rebasing. There should be also @@ -342,7 +342,7 @@ pub fn parse_answer( root_password: InstallRootPassword { plain: answer.global.root_password.clone(), -hashed: None, +hashed: answer.global.root_password_hashed.clone(), }, mailto: answer.global.mailto.clone(), root_ssh_keys: answer.global.root_ssh_keys.clone(), I'll send a v2 soon, although I'll let it bake on the mailing list a bit first, in case there is some (other) feedback. > -- > 2.44.0 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 6/7] auto-installer: add new `global.root_password_hashed` answer option
This allows user to specify the root password in a hashed format, generated using e.g. mkpasswd(1), instead of plaintext. Signed-off-by: Christoph Heiss --- proxmox-auto-installer/src/answer.rs | 3 ++- proxmox-auto-installer/src/utils.rs | 4 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs index aab7198..d691da1 100644 --- a/proxmox-auto-installer/src/answer.rs +++ b/proxmox-auto-installer/src/answer.rs @@ -26,7 +26,8 @@ pub struct Global { pub keyboard: KeyboardLayout, pub mailto: String, pub timezone: String, -pub root_password: String, +pub root_password: Option, +pub root_password_hashed: Option, #[serde(default)] pub reboot_on_error: bool, #[serde(default)] diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs index f1425b0..2b9930d 100644 --- a/proxmox-auto-installer/src/utils.rs +++ b/proxmox-auto-installer/src/utils.rs @@ -319,6 +319,10 @@ pub fn parse_answer( verify_locale_settings(answer, locales)?; +if answer.global.root_password.is_some() == answer.global.root_password_hashed.is_some() { +bail!("`root_password` and `root_password_hashed` cannot be set at the same time"); +} + let mut config = InstallConfig { autoreboot: 1_usize, filesys: filesystem, -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 5/7] auto-installer: adapt to new `root_password` plain/hashed setup option
Signed-off-by: Christoph Heiss --- proxmox-auto-installer/src/utils.rs | 9 +++-- .../tests/resources/parse_answer/disk_match.json | 2 +- .../tests/resources/parse_answer/disk_match_all.json | 2 +- .../tests/resources/parse_answer/disk_match_any.json | 2 +- .../tests/resources/parse_answer/minimal.json| 2 +- .../tests/resources/parse_answer/nic_matching.json | 2 +- .../tests/resources/parse_answer/specific_nic.json | 2 +- .../tests/resources/parse_answer/zfs.json| 2 +- 8 files changed, 14 insertions(+), 9 deletions(-) diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs index 202ad41..f1425b0 100644 --- a/proxmox-auto-installer/src/utils.rs +++ b/proxmox-auto-installer/src/utils.rs @@ -10,7 +10,9 @@ use crate::{ }; use proxmox_installer_common::{ options::{FsType, NetworkOptions, ZfsChecksumOption, ZfsCompressOption}, -setup::{InstallConfig, InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo}, +setup::{ +InstallConfig, InstallRootPassword, InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo, +}, }; use serde::{Deserialize, Serialize}; @@ -334,7 +336,10 @@ pub fn parse_answer( timezone: answer.global.timezone.clone(), keymap: answer.global.keyboard.to_string(), -password: answer.global.root_password.clone(), +root_password: InstallRootPassword { +plain: answer.global.root_password.clone(), +hashed: None, +}, mailto: answer.global.mailto.clone(), root_ssh_keys: answer.global.root_ssh_keys.clone(), diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match.json b/proxmox-auto-installer/tests/resources/parse_answer/disk_match.json index 3a117b6..a0942cc 100644 --- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match.json +++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match.json @@ -18,7 +18,7 @@ "keymap": "de", "mailto": "mail@no.invalid", "mngmt_nic": "eno1", - "password": "123456", + "root_password": { "plain": "123456" }, "timezone": "Europe/Vienna", "zfs_opts": { "arc_max": 2048, diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json index 5325fc3..a7324f1 100644 --- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json +++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json @@ -15,7 +15,7 @@ "keymap": "de", "mailto": "mail@no.invalid", "mngmt_nic": "eno1", - "password": "123456", + "root_password": { "plain": "123456" }, "timezone": "Europe/Vienna", "zfs_opts": { "arc_max": 2048, diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json index 18e22d1..8e13496 100644 --- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json +++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json @@ -22,7 +22,7 @@ "keymap": "de", "mailto": "mail@no.invalid", "mngmt_nic": "eno1", - "password": "123456", + "root_password": { "plain": "123456" }, "timezone": "Europe/Vienna", "zfs_opts": { "arc_max": 2048, diff --git a/proxmox-auto-installer/tests/resources/parse_answer/minimal.json b/proxmox-auto-installer/tests/resources/parse_answer/minimal.json index bb72713..7a6bfd5 100644 --- a/proxmox-auto-installer/tests/resources/parse_answer/minimal.json +++ b/proxmox-auto-installer/tests/resources/parse_answer/minimal.json @@ -12,7 +12,7 @@ "keymap": "de", "mailto": "mail@no.invalid", "mngmt_nic": "eno1", - "password": "123456", + "root_password": { "plain": "123456" }, "target_hd": "/dev/sda", "timezone": "Europe/Vienna" } diff --git a/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json b/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json index de94165..e1c9d12 100644 --- a/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json +++ b/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json @@ -12,7 +12,7 @@ "keymap": "de", "mailto": "mail@no.invalid", "mngm
[pve-devel] [PATCH installer 7/7] auto-installer: add test for hashed root password option
Signed-off-by: Christoph Heiss --- .../parse_answer/hashed_root_password.json| 20 +++ .../parse_answer/hashed_root_password.toml| 14 + 2 files changed, 34 insertions(+) create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.json create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml diff --git a/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.json b/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.json new file mode 100644 index 000..19c51ba --- /dev/null +++ b/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.json @@ -0,0 +1,20 @@ +{ + "autoreboot": 1, + "cidr": "192.168.1.114/24", + "country": "at", + "dns": "192.168.1.254", + "domain": "testinstall", + "filesys": "ext4", + "gateway": "192.168.1.1", + "hdsize": 223.57088470458984, + "lvm_auto_rename": 1, + "hostname": "pveauto", + "keymap": "de", + "mailto": "mail@no.invalid", + "mngmt_nic": "eno1", + "root_password": { +"hashed": "$y$j9T$VgMv8lsz/TEvzesCZU3xD.$SK.h4QW51Jr/EmjuaTz5Bt4kYiX2Iezz6omzoqVEwj9" + }, + "target_hd": "/dev/sda", + "timezone": "Europe/Vienna" +} diff --git a/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml b/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml new file mode 100644 index 000..ed4d2e5 --- /dev/null +++ b/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml @@ -0,0 +1,14 @@ +[global] +keyboard = "de" +country = "at" +fqdn = "pveauto.testinstall" +mailto = "mail@no.invalid" +timezone = "Europe/Vienna" +root_password_hashed = "$y$j9T$VgMv8lsz/TEvzesCZU3xD.$SK.h4QW51Jr/EmjuaTz5Bt4kYiX2Iezz6omzoqVEwj9" + +[network] +source = "from-dhcp" + +[disk-setup] +filesystem = "ext4" +disk_list = ["sda"] -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 0/7] auto-installer: add option for providing hashed root password
This series adds a new answer option `global.root_password_hashed` for the auto-installer, enabling administrators to specify the root password of the new installation in a hashed format - as generated by e.g. mkpasswd(1) - instead of plain-text. Administrators/users might want to avoid passing along a plain-text password with the different answer-fetching methods supported by the auto-installer, for obvious reasons. While this of course does not provide full security, sending a hashed password might still be preferred by administrators over plain text. Tested by installing using the GUI and TUI (to ensure no regressions happend) and using the auto-installer, once with `root_password` set (again testing for potential regressions) and once with `global.root_password_hashed` set instead, testing the new functionality. First two patches are small cleanups and may be applied independently. Christoph Heiss (7): common: move `PasswordOptions` type to tui crate tui-installer: remove `Debug` implementation for password options low-level: change root password option to contain either plaintext or hash tui-installer: adapt to new `root_password` plain/hashed setup option auto-installer: adapt to new `root_password` plain/hashed setup option auto-installer: add new `global.root_password_hashed` answer option auto-installer: add test for hashed root password option Proxmox/Install.pm| 25 --- Proxmox/Install/Config.pm | 20 --- proxinstall | 4 +-- proxmox-auto-installer/src/answer.rs | 3 ++- proxmox-auto-installer/src/utils.rs | 13 -- .../resources/parse_answer/disk_match.json| 2 +- .../parse_answer/disk_match_all.json | 2 +- .../parse_answer/disk_match_any.json | 2 +- .../parse_answer/hashed_root_password.json| 20 +++ .../parse_answer/hashed_root_password.toml| 14 +++ .../tests/resources/parse_answer/minimal.json | 2 +- .../resources/parse_answer/nic_matching.json | 2 +- .../resources/parse_answer/specific_nic.json | 2 +- .../tests/resources/parse_answer/zfs.json | 2 +- proxmox-installer-common/src/options.rs | 15 --- proxmox-installer-common/src/setup.rs | 12 +++-- proxmox-tui-installer/src/main.rs | 4 +-- proxmox-tui-installer/src/options.rs | 20 --- proxmox-tui-installer/src/setup.rs| 10 ++-- 19 files changed, 132 insertions(+), 42 deletions(-) create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.json create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 3/7] low-level: change root password option to contain either plaintext or hash
A hashed password can be created e.g. using the `mkpasswd(1)`. This then will allow the auto-installer to pass along a already-hashed password from the user, instead of simple plaintext. Signed-off-by: Christoph Heiss --- Proxmox/Install.pm| 25 ++--- Proxmox/Install/Config.pm | 20 +--- proxinstall | 4 ++-- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm index c0f8955..bcf8ba7 100644 --- a/Proxmox/Install.pm +++ b/Proxmox/Install.pm @@ -621,6 +621,27 @@ sub prepare_grub_efi_boot_esp { die "failed to prepare EFI boot using Grub on '$espdev': $err" if $err; } +my sub setup_root_password { +my ($targetdir) = @_; + +my $plain = Proxmox::Install::Config::get_root_password('plain'); +my $hashed = Proxmox::Install::Config::get_root_password('hashed'); + +die "root password must be set!\n" + if !defined($plain) && !defined($hashed); + +die "plain and hashed root password cannot be set at the same time!\n" + if defined($plain) && defined($hashed); + +if (defined($plain)) { + my $octets = encode("utf-8", $plain); + run_command("chroot $targetdir /usr/sbin/chpasswd", undef, "root:$octets\n"); +} elsif (defined($hashed)) { + my $octets = encode("utf-8", $hashed); + run_command("chroot $targetdir /usr/sbin/chpasswd --encrypted", undef, "root:$octets\n"); +} +} + sub extract_data { my $iso_env = Proxmox::Install::ISOEnv::get(); my $run_env = Proxmox::Install::RunEnv::get(); @@ -1269,9 +1290,7 @@ _EOD diversion_remove($targetdir, "/sbin/start-stop-daemon"); - # set root password - my $octets = encode("utf-8", Proxmox::Install::Config::get_password()); - run_command("chroot $targetdir /usr/sbin/chpasswd", undef, "root:$octets\n"); + setup_root_password($targetdir); # set root ssh keys my $ssh_keys = Proxmox::Install::Config::get_root_ssh_keys(); diff --git a/Proxmox/Install/Config.pm b/Proxmox/Install/Config.pm index ecd8a74..0313fd9 100644 --- a/Proxmox/Install/Config.pm +++ b/Proxmox/Install/Config.pm @@ -90,7 +90,7 @@ my sub init_cfg { keymap => 'en-us', # root credentials & details - password => undef, + root_password => undef, mailto => 'mail@example.invalid', root_ssh_keys => [], @@ -196,8 +196,22 @@ sub get_timezone { return get('timezone'); } sub set_keymap { set_key('keymap', $_[0]); } sub get_keymap { return get('keymap'); } -sub set_password { set_key('password', $_[0]); } -sub get_password { return get('password'); } +sub set_root_password { +my ($key) = @_; +croak "unknown root password option '$key'" + if $key ne 'plain' && $key ne 'hashed'; + +set_key('root_password', { $_[0] => $_[1] }); +} + +sub get_root_password { +my ($key) = @_; +croak "unknown root password option '$key'" + if $key ne 'plain' && $key ne 'hashed'; + +my $password = get('root_password'); +return defined($password->{$key}) ? $password->{$key} : undef; +} sub set_mailto { set_key('mailto', $_[0]); } sub get_mailto { return get('mailto'); } diff --git a/proxinstall b/proxinstall index a6a4cfb..12f3eaa 100755 --- a/proxinstall +++ b/proxinstall @@ -674,7 +674,7 @@ sub create_password_view { cleanup_view(); -my $password = Proxmox::Install::Config::get_password(); +my $password = Proxmox::Install::Config::get_root_password('plain'); my $grid = &$create_basic_grid(); $gtk_state->{inbox}->pack_start($grid, 0, 0, 0); @@ -745,7 +745,7 @@ sub create_password_view { return; } - Proxmox::Install::Config::set_password($t1); + Proxmox::Install::Config::set_root_password('plain', $t1); Proxmox::Install::Config::set_mailto($t3); $step_number++; -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 4/7] tui-installer: adapt to new `root_password` plain/hashed setup option
Signed-off-by: Christoph Heiss --- proxmox-installer-common/src/setup.rs | 12 ++-- proxmox-tui-installer/src/setup.rs| 10 -- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs index 64d05af..ef92eb7 100644 --- a/proxmox-installer-common/src/setup.rs +++ b/proxmox-installer-common/src/setup.rs @@ -424,6 +424,14 @@ impl Interface { } } +#[derive(Clone, Deserialize, Serialize)] +pub struct InstallRootPassword { +#[serde(skip_serializing_if = "Option::is_none")] +pub plain: Option, +#[serde(skip_serializing_if = "Option::is_none")] +pub hashed: Option, +} + pub fn spawn_low_level_installer(test_mode: bool) -> io::Result { let (path, args, envs): (, &[], Vec<(, )>) = if test_mode { ( @@ -444,7 +452,7 @@ pub fn spawn_low_level_installer(test_mode: bool) -> io::Result } /// See Proxmox::Install::Config -#[derive(Debug, Deserialize, Serialize)] +#[derive(Deserialize, Serialize)] pub struct InstallConfig { pub autoreboot: usize, @@ -485,7 +493,7 @@ pub struct InstallConfig { pub timezone: String, pub keymap: String, -pub password: String, +pub root_password: InstallRootPassword, pub mailto: String, #[serde(skip_serializing_if = "Vec::is_empty")] pub root_ssh_keys: Vec, diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs index 8c01e42..ee6e65c 100644 --- a/proxmox-tui-installer/src/setup.rs +++ b/proxmox-tui-installer/src/setup.rs @@ -1,7 +1,10 @@ use std::collections::BTreeMap; use crate::options::InstallerOptions; -use proxmox_installer_common::{options::AdvancedBootdiskOptions, setup::InstallConfig}; +use proxmox_installer_common::{ +options::AdvancedBootdiskOptions, +setup::{InstallConfig, InstallRootPassword}, +}; impl From for InstallConfig { fn from(options: InstallerOptions) -> Self { @@ -23,7 +26,10 @@ impl From for InstallConfig { timezone: options.timezone.timezone, keymap: options.timezone.kb_layout, -password: options.password.root_password, +root_password: InstallRootPassword { +plain: Some(options.password.root_password), +hashed: None, +}, mailto: options.password.email, root_ssh_keys: vec![], -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 2/7] tui-installer: remove `Debug` implementation for password options
So we never accidentally show/log the password somewhere. Need to drop it from `InstallerOptions` in turn too, but it's never used currently anyway. Signed-off-by: Christoph Heiss --- proxmox-tui-installer/src/options.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs index 6335762..19992ca 100644 --- a/proxmox-tui-installer/src/options.rs +++ b/proxmox-tui-installer/src/options.rs @@ -24,7 +24,7 @@ pub const FS_TYPES: &[FsType] = { ] }; -#[derive(Clone, Debug)] +#[derive(Clone)] pub struct PasswordOptions { pub email: String, pub root_password: String, @@ -39,7 +39,7 @@ impl Default for PasswordOptions { } } -#[derive(Clone, Debug)] +#[derive(Clone)] pub struct InstallerOptions { pub bootdisk: BootdiskOptions, pub timezone: TimezoneOptions, -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 1/7] common: move `PasswordOptions` type to tui crate
It's only used internally there anyway, so make it slightly less confusing. Signed-off-by: Christoph Heiss --- proxmox-installer-common/src/options.rs | 15 --- proxmox-tui-installer/src/main.rs | 4 ++-- proxmox-tui-installer/src/options.rs| 18 -- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs index e77914b..9375ded 100644 --- a/proxmox-installer-common/src/options.rs +++ b/proxmox-installer-common/src/options.rs @@ -327,21 +327,6 @@ impl TimezoneOptions { } } -#[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, diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs index 4fb7afd..dd600c6 100644 --- a/proxmox-tui-installer/src/main.rs +++ b/proxmox-tui-installer/src/main.rs @@ -16,10 +16,10 @@ use cursive::{ use regex::Regex; mod options; -use options::InstallerOptions; +use options::{InstallerOptions, PasswordOptions}; use proxmox_installer_common::{ -options::{BootdiskOptions, NetworkOptions, PasswordOptions, TimezoneOptions}, +options::{BootdiskOptions, NetworkOptions, TimezoneOptions}, setup::{installer_setup, LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo}, utils::Fqdn, }; diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs index 73fbf2a..6335762 100644 --- a/proxmox-tui-installer/src/options.rs +++ b/proxmox-tui-installer/src/options.rs @@ -2,8 +2,7 @@ use crate::SummaryOption; use proxmox_installer_common::{ options::{ -BootdiskOptions, BtrfsRaidLevel, FsType, NetworkOptions, PasswordOptions, TimezoneOptions, -ZfsRaidLevel, +BootdiskOptions, BtrfsRaidLevel, FsType, NetworkOptions, TimezoneOptions, ZfsRaidLevel, }, setup::LocaleInfo, }; @@ -25,6 +24,21 @@ pub const FS_TYPES: &[FsType] = { ] }; +#[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)] pub struct InstallerOptions { pub bootdisk: BootdiskOptions, -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 4/6] low-level: stdio: fix: make progress text properly optional
.. such that receivers can differentiate between these two cases more clearly. Sometimes, the `progress` sub does not get passed a text (on purpose!), just updating the progress ratio. This would cause log messages to be written out which could indicate missing text and look rather weird. Signed-off-by: Christoph Heiss --- Proxmox/UI/StdIO.pm | 8 +--- .../src/bin/proxmox-auto-installer.rs | 6 +- proxmox-installer-common/src/setup.rs | 2 +- .../src/views/install_progress.rs | 19 +-- test/ui2-stdio.pl | 2 +- 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/Proxmox/UI/StdIO.pm b/Proxmox/UI/StdIO.pm index 2838fcb..ec9eac9 100644 --- a/Proxmox/UI/StdIO.pm +++ b/Proxmox/UI/StdIO.pm @@ -70,9 +70,11 @@ sub display_html { sub progress { my ($self, $ratio, $text) = @_; -$text = '' if !defined($text); - -send_msg('progress', ratio => $ratio, text => $text); +if (defined($text)) { + send_msg('progress', ratio => $ratio, text => $text); +} else { + send_msg('progress', ratio => $ratio); +} } sub process_events { diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs index 9fc328c..96a48bd 100644 --- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs +++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs @@ -172,7 +172,11 @@ fn run_installation( } LowLevelMessage::Progress { ratio, text } => { let percentage = ratio * 100.; -info!("progress {percentage:>5.1} % - {text}"); +if let Some(text) = text { +info!("progress {percentage:>5.1} % - {text}"); +} else { +info!("progress {percentage:>5.1} %"); +} } LowLevelMessage::Finished { state, message } => { if state == "err" { diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs index 0c62bc3..d145fc5 100644 --- a/proxmox-installer-common/src/setup.rs +++ b/proxmox-installer-common/src/setup.rs @@ -578,6 +578,6 @@ pub enum LowLevelMessage { }, Progress { ratio: f32, -text: String, +text: Option, }, } diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs index 11b12f0..629472a 100644 --- a/proxmox-tui-installer/src/views/install_progress.rs +++ b/proxmox-tui-installer/src/views/install_progress.rs @@ -127,11 +127,18 @@ impl InstallProgressView { }), LowLevelMessage::Progress { ratio, text } => { counter.set((ratio * 100.).floor() as usize); -cb_sink.send(Box::new(move |siv| { -siv.call_on_name(Self::PROGRESS_TEXT_VIEW_ID, |v: TextView| { -v.set_content(text); -}); -})) +if let Some(text) = text { +cb_sink.send(Box::new(move |siv| { +siv.call_on_name( +Self::PROGRESS_TEXT_VIEW_ID, +|v: TextView| { +v.set_content(text); +}, +); +})) +} else { +Ok(()) +} } LowLevelMessage::Finished { state, message } => { counter.set(100); @@ -300,7 +307,7 @@ mod tests { next_msg(), Some(LowLevelMessage::Progress { ratio: (i as f32) / 1000., -text: format!("foo {i}"), +text: Some(format!("foo {i}")), }), ); } diff --git a/test/ui2-stdio.pl b/test/ui2-stdio.pl index 01f323d..eebb5c9 100755 --- a/test/ui2-stdio.pl +++ b/test/ui2-stdio.pl @@ -81,7 +81,7 @@ if ($child_pid) { 'should get 20% done progress message'); is_deeply( - $next_msg->(), { type => 'progress', ratio => 0.2, text => '' }, + $next_msg->(), { type => 'progress', ratio => 0.2, }, 'should get progress continuation message'); is_deeply( -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 5/6] low-level: add proper message to 100% progress ratio update
Signed-off-by: Christoph Heiss --- Proxmox/Install.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm index c0f8955..af857ca 100644 --- a/Proxmox/Install.pm +++ b/Proxmox/Install.pm @@ -1331,7 +1331,7 @@ _EOD my $err = $@; -update_progress(1, 0, 1, ""); +update_progress(1, 0, 1, "installation finished"); print STDERR $err if $err && $err ne "\n"; -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 2/6] auto, tui: move low-level installer message struct to common crate
Signed-off-by: Christoph Heiss --- .../src/bin/proxmox-auto-installer.rs | 8 ++-- proxmox-auto-installer/src/utils.rs | 23 - proxmox-installer-common/src/setup.rs | 23 + .../src/views/install_progress.rs | 48 +-- 4 files changed, 38 insertions(+), 64 deletions(-) diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs index 9fcec1e..e607a9c 100644 --- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs +++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs @@ -8,14 +8,12 @@ use std::{ }; use proxmox_installer_common::setup::{ -installer_setup, read_json, spawn_low_level_installer, LocaleInfo, RuntimeInfo, SetupInfo, +installer_setup, read_json, spawn_low_level_installer, LocaleInfo, LowLevelMessage, +RuntimeInfo, SetupInfo, }; use proxmox_auto_installer::{ -answer::Answer, -log::AutoInstLogger, -udevinfo::UdevInfo, -utils::{parse_answer, LowLevelMessage}, +answer::Answer, log::AutoInstLogger, udevinfo::UdevInfo, utils::parse_answer, }; static LOGGER: AutoInstLogger = AutoInstLogger; diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs index 202ad41..7f3688d 100644 --- a/proxmox-auto-installer/src/utils.rs +++ b/proxmox-auto-installer/src/utils.rs @@ -380,26 +380,3 @@ pub fn parse_answer( } Ok(config) } - -#[derive(Clone, Debug, Deserialize, PartialEq)] -#[serde(tag = "type", rename_all = "lowercase")] -pub enum LowLevelMessage { -#[serde(rename = "message")] -Info { -message: String, -}, -Error { -message: String, -}, -Prompt { -query: String, -}, -Finished { -state: String, -message: String, -}, -Progress { -ratio: f32, -text: String, -}, -} diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs index 64d05af..0c62bc3 100644 --- a/proxmox-installer-common/src/setup.rs +++ b/proxmox-installer-common/src/setup.rs @@ -558,3 +558,26 @@ where _ => Err(de::Error::custom("could not find file system: {de_fs}")), } } + +#[derive(Clone, Debug, Deserialize, PartialEq)] +#[serde(tag = "type", rename_all = "lowercase")] +pub enum LowLevelMessage { +#[serde(rename = "message")] +Info { +message: String, +}, +Error { +message: String, +}, +Prompt { +query: String, +}, +Finished { +state: String, +message: String, +}, +Progress { +ratio: f32, +text: String, +}, +} diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs index 6453426..11b12f0 100644 --- a/proxmox-tui-installer/src/views/install_progress.rs +++ b/proxmox-tui-installer/src/views/install_progress.rs @@ -4,7 +4,6 @@ use cursive::{ views::{Dialog, DummyView, LinearLayout, PaddedView, ProgressBar, TextView}, CbSink, Cursive, }; -use serde::Deserialize; use std::{ fs::File, io::{BufRead, BufReader, Write}, @@ -14,7 +13,7 @@ use std::{ }; use crate::{abort_install_button, prompt_dialog, InstallerState}; -use proxmox_installer_common::setup::{spawn_low_level_installer, InstallConfig}; +use proxmox_installer_common::setup::{spawn_low_level_installer, InstallConfig, LowLevelMessage}; pub struct InstallProgressView { view: PaddedView, @@ -105,7 +104,7 @@ impl InstallProgressView { continue; } -let msg = match serde_json::from_str::() { +let msg = match serde_json::from_str::() { Ok(msg) => msg, Err(err) => { // Not a fatal error, so don't abort the installation by returning @@ -116,17 +115,17 @@ impl InstallProgressView { }; let result = match msg.clone() { -UiMessage::Info { message } => cb_sink.send(Box::new(|siv| { +LowLevelMessage::Info { message } => cb_sink.send(Box::new(|siv| { siv.add_layer(Dialog::info(message).title("Information")); })), -UiMessage::Error { message } => cb_sink.send(Box::new(|siv| { +LowLevelMessage::Error { message } => cb_sink.send(Box::new(|siv| { siv.add_layer(Dialog::info(message).title("Error")); })), -UiMessage::Prompt { query } => cb_sink.send({ +LowLevelMessage::Prompt { query } => cb_sink.send({ let writer = writer.clone(); Box::new(move |si
[pve-devel] [PATCH installer 3/6] auto: log non-json low-level messages into separate file
Same as the TUI does, as introduced in [0]. [0] 22de6e5 ("tui: install_progress: write low-level non-JSON messages to separate file") Signed-off-by: Christoph Heiss --- .../src/bin/proxmox-auto-installer.rs | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs index e607a9c..9fc328c 100644 --- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs +++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs @@ -2,6 +2,7 @@ use anyhow::{bail, format_err, Result}; use log::{error, info, LevelFilter}; use std::{ env, +fs::File, io::{BufRead, BufReader, Write}, path::PathBuf, process::ExitCode, @@ -136,15 +137,29 @@ fn run_installation( .map_err(|err| format_err!("failed to serialize install config: {err}"))?; writeln!(writer).map_err(|err| format_err!("failed to write install config: {err}"))?; +let mut lowlevel_log = File::create("/tmp/install-low-level.log") +.map_err(|err| format_err!("failed to open low-level installer logfile: {err}"))?; + for line in reader.lines() { let line = match line { Ok(line) => line, Err(_) => break, }; + +// The low-level installer also spews the output of any command it runs on its +// stdout. Use a very simple heuricstic to determine whether it is actually JSON +// or not. +if !line.starts_with('{') || !line.ends_with('}') { +let _ = writeln!(lowlevel_log, "{}", line); +continue; +} + let msg = match serde_json::from_str::() { Ok(msg) => msg, -Err(_) => { +Err(err) => { // Not a fatal error, so don't abort the installation by returning +eprintln!("low-level installer: error while parsing message: '{err}'"); +eprintln!("original message was: '{line}'"); continue; } }; -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 6/6] auto: avoid printing unnecessary progress update lines
In case the progress message did not contain any text and the percentage did not change, don't print another (useless) line. Signed-off-by: Christoph Heiss --- proxmox-auto-installer/src/bin/proxmox-auto-installer.rs | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs index 96a48bd..a72181f 100644 --- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs +++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs @@ -140,6 +140,8 @@ fn run_installation( let mut lowlevel_log = File::create("/tmp/install-low-level.log") .map_err(|err| format_err!("failed to open low-level installer logfile: {err}"))?; +let mut last_progress_percentage = 0.; + for line in reader.lines() { let line = match line { Ok(line) => line, @@ -174,8 +176,10 @@ fn run_installation( let percentage = ratio * 100.; if let Some(text) = text { info!("progress {percentage:>5.1} % - {text}"); -} else { +last_progress_percentage = percentage; +} else if (percentage - last_progress_percentage) > 0.1 { info!("progress {percentage:>5.1} %"); +last_progress_percentage = percentage; } } LowLevelMessage::Finished { state, message } => { -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 1/6] test: ui2-stdio: fix multi-process testing
Previously, if something failed in the child, the overall test would still be successful and exit with `0`. Signed-off-by: Christoph Heiss --- test/ui2-stdio.pl | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/ui2-stdio.pl b/test/ui2-stdio.pl index ae29b79..01f323d 100755 --- a/test/ui2-stdio.pl +++ b/test/ui2-stdio.pl @@ -4,7 +4,6 @@ use strict; use warnings; use JSON qw(from_json); -use Test::More; use Proxmox::Log; use Proxmox::UI; @@ -16,10 +15,11 @@ $parent_writer->autoflush(1); $child_writer->autoflush(1); $child_reader->autoflush(1); - my $child_pid = fork() // die "fork failed - $!\n"; if ($child_pid) { +eval 'use Test::More tests => 3;'; + # parent, the hypothetical low-level installer close($parent_reader); close($parent_writer); @@ -45,8 +45,11 @@ if ($child_pid) { Proxmox::UI::progress(1, 0, 1, 'done'); waitpid($child_pid, 0); +is($?, 0); # check child exit status done_testing(); } else { +eval 'use Test::More tests => 10;'; + # child, e.g. the TUI close($child_reader); close($child_writer); @@ -90,6 +93,5 @@ if ($child_pid) { 'should get 100% done progress message'); done_testing(); -exit(0); } -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 0/6] small, overall install progress improvements
This series tries to improve upon some small things around the installation progress reporting, esp. in the auto-installer, as well as some small fixes & code-deduplication. Christoph Heiss (6): test: ui2-stdio: fix multi-process testing auto, tui: move low-level installer message struct to common crate auto: log non-json low-level messages into separate file low-level: stdio: fix: make progress text properly optional low-level: add proper message to 100% progress ratio update auto: avoid printing unnecessary progress update lines Proxmox/Install.pm| 2 +- Proxmox/UI/StdIO.pm | 8 ++- .../src/bin/proxmox-auto-installer.rs | 35 -- proxmox-auto-installer/src/utils.rs | 23 --- proxmox-installer-common/src/setup.rs | 23 +++ .../src/views/install_progress.rs | 67 +++ test/ui2-stdio.pl | 10 +-- 7 files changed, 88 insertions(+), 80 deletions(-) -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 1/3] low-level: add zfs module for retrieving importable zpool info
Signed-off-by: Christoph Heiss --- Proxmox/Makefile | 1 + Proxmox/Sys/ZFS.pm| 43 ++ test/Makefile | 6 + test/zfs-get-pool-list.pl | 49 +++ 4 files changed, 99 insertions(+) create mode 100644 Proxmox/Sys/ZFS.pm create mode 100755 test/zfs-get-pool-list.pl diff --git a/Proxmox/Makefile b/Proxmox/Makefile index 9561d9b..035626b 100644 --- a/Proxmox/Makefile +++ b/Proxmox/Makefile @@ -17,6 +17,7 @@ PERL_MODULES=\ Sys/File.pm \ Sys/Net.pm \ Sys/Udev.pm \ +Sys/ZFS.pm \ UI.pm \ UI/Base.pm \ UI/Gtk3.pm \ diff --git a/Proxmox/Sys/ZFS.pm b/Proxmox/Sys/ZFS.pm new file mode 100644 index 000..4c732ca --- /dev/null +++ b/Proxmox/Sys/ZFS.pm @@ -0,0 +1,43 @@ +package Proxmox::Sys::ZFS; + +use strict; +use warnings; + +use Proxmox::Sys::Command qw(run_command); + +use base qw(Exporter); +our @EXPORT_OK = qw(get_exported_pools); + +my sub parse_pool_list { +my ($fh) = @_; + +my @pools; +my $pool = {}; # last found pool in output + +while (my $line = <$fh>) { + if ($line =~ /^\s+pool: (.+)$/) { + push @pools, $pool if %$pool; + $pool = { name => $1 }; + next; + } + + next if !%$pool; + + if ($line =~ /^\s*(id|state|status|action): (.+)$/) { + chomp($pool->{$1} = $2); + next; + } +} + +push @pools, $pool if %$pool; +return \@pools; +} + +sub get_exported_pools { +my $raw = run_command(['zpool', 'import']); +open (my $fh, '<', \$raw) or die 'failed to open in-memory stream'; + +return parse_pool_list($fh); +} + +1; diff --git a/test/Makefile b/test/Makefile index 99bf14e..2d9decc 100644 --- a/test/Makefile +++ b/test/Makefile @@ -2,6 +2,7 @@ all: export PERLLIB=.. +# test-zfs-get-pool-list is not run by default, due to requiring root access .PHONY: check check: test-zfs-arc-max test-run-command test-parse-fqdn test-ui2-stdio @@ -9,6 +10,7 @@ check: test-zfs-arc-max test-run-command test-parse-fqdn test-ui2-stdio test-zfs-arc-max: ./zfs-arc-max.pl +.PHONY: test-run-command test-run-command: ./run-command.pl @@ -19,3 +21,7 @@ test-parse-fqdn: .PHONY: test-ui2-stdio test-ui2-stdio: ./ui2-stdio.pl + +.PHONY: test-zfs-get-pool-list +test-zfs-get-pool-list: + ./zfs-get-pool-list.pl diff --git a/test/zfs-get-pool-list.pl b/test/zfs-get-pool-list.pl new file mode 100755 index 000..072da2f --- /dev/null +++ b/test/zfs-get-pool-list.pl @@ -0,0 +1,49 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use File::Temp; +use Test::More tests => 6; + +use Proxmox::Sys::Command qw(syscmd run_command); +use Proxmox::Sys::ZFS; +use Proxmox::UI; + +my $log_file = File::Temp->new(); +Proxmox::Log::init($log_file->filename); + +Proxmox::UI::init_stdio(); + +our $POOL_PREFIX = 'pve-installer'; +my $pools = { 'test-pool1' => {}, 'test-pool2' => {} }; + +foreach (keys %$pools) { +my $f = File::Temp->new(); +print "$_: $f\n"; + +syscmd("truncate -s 1G $f"); +my $dev = run_command("losetup --find --show $f"); + +syscmd("zpool create $POOL_PREFIX-$_ $dev"); +syscmd("zpool export $POOL_PREFIX-$_"); + +$pools->{$_} = { + file => $f, + dev => $dev, +}; +} + +my $exported = Proxmox::Sys::ZFS::get_exported_pools(); +while (my ($name, $info) = each %$pools) { +my ($p) = grep { $_->{name} eq "$POOL_PREFIX-$name" } @$exported; +ok(defined($p), "pool $name was found"); +is($p->{state}, 'ONLINE', "pool $name is online"); +is($p->{action}, 'The pool can be imported using its name or numeric identifier.', + "pool $name can be imported"); +} + +keys %$pools; +while (my ($name, $info) = each %$pools) { +syscmd("losetup --detach $info->{dev}"); +} -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 2/3] low-level: install: split out random disk uid generation
Signed-off-by: Christoph Heiss --- Proxmox/Install.pm | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm index c0f8955..c86a574 100644 --- a/Proxmox/Install.pm +++ b/Proxmox/Install.pm @@ -169,6 +169,13 @@ sub btrfs_create { syscmd($cmd); } +# no high randomnes properties, but this is only for the cases where +# we either have multiple volume groups or zpools from multiple old PVE disks, +# or we have a disk with both a "$vgname" and "$vgname-old" ... +sub random_short_uid { +return sprintf "%08X", rand(0x); +} + sub zfs_create_rpool { my ($vdev, $pool_name, $root_volume_name) = @_; @@ -376,12 +383,7 @@ sub ask_existing_vg_rename_or_abort { for my $vg_uuid (keys %$duplicate_vgs) { my $vg = $duplicate_vgs->{$vg_uuid}; - - # no high randomnes properties, but this is only for the cases where - # we either have multiple "$vgname" vgs from multiple old PVE disks, or - # we have a disk with both a "$vgname" and "$vgname-old"... - my $short_uid = sprintf "%08X", rand(0x); - $vg->{new_vgname} = "$vgname-OLD-$short_uid"; + $vg->{new_vgname} = "$vgname-OLD-" . random_short_uid(); } my $response_ok = Proxmox::Install::Config::get_lvm_auto_rename(); -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 0/3] add check/rename for already-existing ZFS rpool
Pretty straight forward overall, implements a check for an exising `rpool` on the system and ask the user whether they would like to rename it, much in the same way as it works for VGs already. Without this, the installer would silently create a second (and thus conflicting) `rpool` and cause a boot failure after the installation, since it does not know which pool to import exactly. Christoph Heiss (3): proxmox: add zfs module for retrieving importable zpool info low-level: install: split out random disk uid generation low-level: install: check for already-existing `rpool` on install Proxmox/Install.pm| 47 - Proxmox/Makefile | 1 + Proxmox/Sys/ZFS.pm| 43 ++ test/Makefile | 6 + test/zfs-get-pool-list.pl | 49 +++ 5 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 Proxmox/Sys/ZFS.pm create mode 100755 test/zfs-get-pool-list.pl -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 3/3] low-level: install: check for already-existing `rpool` on install
.. in the same manner as the detection for LVM works. zpools can only be renamed by importing them with a new name, so unfortunaly the import-export dance is needed. Signed-off-by: Christoph Heiss --- Proxmox/Install.pm | 33 + 1 file changed, 33 insertions(+) diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm index c86a574..531ef1c 100644 --- a/Proxmox/Install.pm +++ b/Proxmox/Install.pm @@ -16,6 +16,7 @@ use Proxmox::Install::StorageConfig; use Proxmox::Sys::Block qw(get_cached_disks wipe_disk partition_bootable_disk); use Proxmox::Sys::Command qw(run_command syscmd); use Proxmox::Sys::File qw(file_read_firstline file_write_all); +use Proxmox::Sys::ZFS; use Proxmox::UI; # TODO: move somewhere better? @@ -176,9 +177,41 @@ sub random_short_uid { return sprintf "%08X", rand(0x); } +sub zfs_ask_existing_zpool_rename { +my ($pool_name) = @_; + +# At this point, no pools should be imported/active +my $exported_pools = Proxmox::Sys::ZFS::get_exported_pools(); + +foreach (@$exported_pools) { + next if $_->{name} ne $pool_name || $_->{state} ne 'ONLINE'; + my $renamed_pool = "$_->{name}-OLD-" . random_short_uid(); + + my $response_ok = Proxmox::UI::prompt( + "A ZFS pool named '$_->{name}' (id $_->{id}) already exists on the system.\n\n" . + "Do you want to rename the pool to '$renamed_pool' before continuing\n" . + "or cancel the installation?" + ); + + # Import the pool using its id, as that is unique and works even if there are + # multiple zpools with the same name. + if ($response_ok) { + syscmd("zpool import -f $_->{id} $renamed_pool") == 0 || + die "failed to import zfs pool '$_->{name}' with new name '$renamed_pool'\n"; + syscmd("zpool export $renamed_pool") == 0 || + warn "failed to export renamed zfs pool '$renamed_pool'\n"; + } else { + warn "Canceled installation as requested by user, due to already existing ZFS pool '$pool_name'\n"; + die "\n"; # causes abort without re-showing an error dialogue + } +} +} + sub zfs_create_rpool { my ($vdev, $pool_name, $root_volume_name) = @_; +zfs_ask_existing_zpool_rename($pool_name); + my $iso_env = Proxmox::Install::ISOEnv::get(); my $zfs_opts = Proxmox::Install::Config::get_zfs_opt(); -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer 3/6] fix #5250: proxinstall: expose new btrfs `compress` option
On Mon, May 13, 2024 at 02:13:52PM +0200, Christoph Heiss wrote: > Signed-off-by: Christoph Heiss > --- > Proxmox/Install.pm | 7 +-- > proxinstall| 15 +++ > 2 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm > index f3bc5aa..60f38e5 100644 > --- a/Proxmox/Install.pm > +++ b/Proxmox/Install.pm > @@ -1014,8 +1014,11 @@ sub extract_data { > my $btrfs_opts = Proxmox::Install::Config::get_btrfs_opt(); > > my $mountopts = 'defaults'; > - $mountopts .= ",compress=$btrfs_opts->{compress}" > - if $btrfs_opts->{compress} ne 'off'; > + if ($btrfs_opts->{compress} eq 'on') { > + $mountopts .= ',compress'; > + } elsif ($btrfs_opts->{compress} ne 'off') { > + $mountopts .= ",compress=$btrfs_opts->{compress}"; > + } That was supposed to be squashed into the previous patch, whoops! I'll send a v2 soon if nothing else comes up, sorry for the noise. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 5/6] fix #5250: auto-installer: expose new btrfs `compress` option
Signed-off-by: Christoph Heiss --- proxmox-auto-installer/src/answer.rs | 6 +- proxmox-auto-installer/src/utils.rs | 7 ++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs index aab7198..add36a3 100644 --- a/proxmox-auto-installer/src/answer.rs +++ b/proxmox-auto-installer/src/answer.rs @@ -1,6 +1,9 @@ use clap::ValueEnum; use proxmox_installer_common::{ -options::{BtrfsRaidLevel, FsType, ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel}, +options::{ +BtrfsCompressOption, BtrfsRaidLevel, FsType, ZfsChecksumOption, ZfsCompressOption, +ZfsRaidLevel, +}, utils::{CidrAddress, Fqdn}, }; use serde::{Deserialize, Serialize}; @@ -269,6 +272,7 @@ pub struct LvmOptions { pub struct BtrfsOptions { pub hdsize: Option, pub raid: Option, +pub compress: Option, } #[derive(Clone, Deserialize, Serialize, Debug, PartialEq)] diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs index 30b6196..b08c617 100644 --- a/proxmox-auto-installer/src/utils.rs +++ b/proxmox-auto-installer/src/utils.rs @@ -10,7 +10,9 @@ use crate::{ }; use proxmox_installer_common::{ options::{FsType, NetworkOptions, ZfsChecksumOption, ZfsCompressOption}, -setup::{InstallConfig, InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo}, +setup::{ +InstallBtrfsOption, InstallConfig, InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo, +}, }; use serde::{Deserialize, Serialize}; @@ -377,6 +379,9 @@ pub fn parse_answer( config.hdsize = btrfs .hdsize .unwrap_or(runtime_info.disks[first_selected_disk].size); +config.btrfs_opts = Some(InstallBtrfsOption { +compress: btrfs.compress.unwrap_or_default(), +}) } } Ok(config) -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 4/6] fix #5250: tui: expose new btrfs `compress` option
Signed-off-by: Christoph Heiss --- proxmox-tui-installer/src/views/bootdisk.rs | 34 +++-- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs index 107fc9c..7a34d63 100644 --- a/proxmox-tui-installer/src/views/bootdisk.rs +++ b/proxmox-tui-installer/src/views/bootdisk.rs @@ -19,8 +19,9 @@ use proxmox_installer_common::{ check_zfs_raid_config, }, options::{ -AdvancedBootdiskOptions, BootdiskOptions, BtrfsBootdiskOptions, BtrfsCompressOption, Disk, -FsType, LvmBootdiskOptions, ZfsBootdiskOptions, ZFS_CHECKSUM_OPTIONS, ZFS_COMPRESS_OPTIONS, +AdvancedBootdiskOptions, BootdiskOptions, BtrfsBootdiskOptions, Disk, FsType, +LvmBootdiskOptions, ZfsBootdiskOptions, BTRFS_COMPRESS_OPTIONS, ZFS_CHECKSUM_OPTIONS, +ZFS_COMPRESS_OPTIONS, }, setup::{BootType, ProductConfig, ProxmoxProduct, RuntimeInfo}, }; @@ -521,12 +522,23 @@ struct BtrfsBootdiskOptionsView { impl BtrfsBootdiskOptionsView { fn new(disks: &[Disk], options: ) -> Self { -let view = MultiDiskOptionsView::new( -disks, -_disks, -FormView::new().child("hdsize", DiskSizeEditView::new().content(options.disk_size)), -) -.top_panel(TextView::new("Btrfs integration is a technology preview!").center()); +let inner = FormView::new() +.child( +"compress", +SelectView::new() +.popup() +.with_all(BTRFS_COMPRESS_OPTIONS.iter().map(|o| (o.to_string(), *o))) +.selected( +BTRFS_COMPRESS_OPTIONS +.iter() +.position(|o| *o == options.compress) +.unwrap_or_default(), +), +) +.child("hdsize", DiskSizeEditView::new().content(options.disk_size)); + +let view = MultiDiskOptionsView::new(disks, _disks, inner) +.top_panel(TextView::new("Btrfs integration is a technology preview!").center()); Self { view } } @@ -537,14 +549,16 @@ impl BtrfsBootdiskOptionsView { fn get_values( self) -> Option<(Vec, BtrfsBootdiskOptions)> { let (disks, selected_disks) = self.view.get_disks_and_selection()?; -let disk_size = self.view.inner_mut()?.get_value::(0)?; +let view = self.view.inner_mut()?; +let disk_size = view.get_value::(1)?; +let compress = view.get_value::, _>(0)?; Some(( disks, BtrfsBootdiskOptions { disk_size, selected_disks, -compress: BtrfsCompressOption::default(), +compress, }, )) } -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 0/6] fix #5250: add btrfs `compress` mount option support
Pretty much as it says on the tin, this adds the `compress` mount option for Btrfs much in the same way as the ZFS equivalent. In regards to the discussion in #5250 - `compress` is used. For the detailed why, see commit #2. Christoph Heiss (6): fix #5250: install: config: add new `btrfs_opts` with `compress` config option fix #5250: install: write btrfs `compress` option to fstab fix #5250: proxinstall: expose new btrfs `compress` option fix #5250: tui: expose new btrfs `compress` option fix #5250: auto-installer: expose new btrfs `compress` option auto-installer: add btrfs answer parsing tests Proxmox/Install.pm| 11 ++- Proxmox/Install/Config.pm | 15 + proxinstall | 15 + proxmox-auto-installer/src/answer.rs | 6 +++- proxmox-auto-installer/src/utils.rs | 8 - .../tests/resources/parse_answer/btrfs.json | 24 ++ .../tests/resources/parse_answer/btrfs.toml | 17 ++ proxmox-installer-common/src/options.rs | 31 +++ proxmox-installer-common/src/setup.rs | 21 +++-- proxmox-tui-installer/src/setup.rs| 2 ++ proxmox-tui-installer/src/views/bootdisk.rs | 31 ++- 11 files changed, 168 insertions(+), 13 deletions(-) create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/btrfs.json create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 1/6] fix #5250: install: config: add new `btrfs_opts` with `compress` config option
Signed-off-by: Christoph Heiss --- Proxmox/Install/Config.pm | 15 ++ proxmox-auto-installer/src/utils.rs | 1 + proxmox-installer-common/src/options.rs | 31 + proxmox-installer-common/src/setup.rs | 21 -- proxmox-tui-installer/src/setup.rs | 2 ++ proxmox-tui-installer/src/views/bootdisk.rs | 5 ++-- 6 files changed, 71 insertions(+), 4 deletions(-) diff --git a/Proxmox/Install/Config.pm b/Proxmox/Install/Config.pm index ecd8a74..09edf11 100644 --- a/Proxmox/Install/Config.pm +++ b/Proxmox/Install/Config.pm @@ -79,6 +79,9 @@ my sub init_cfg { copies => 1, arc_max => Proxmox::Install::RunEnv::default_zfs_arc_max(), # in MiB }, + btrfs_opts => { + compress => 'off', + }, # TODO: single disk selection config target_hd => undef, disk_selection => {}, @@ -173,6 +176,18 @@ sub get_zfs_opt { return defined($k) ? $zfs_opts->{$k} : $zfs_opts; } +sub set_btrfs_opt { +my ($k, $v) = @_; +my $opts = get('btrfs_opts'); +croak "unknown btrfs opts key '$k'" if !exists($opts->{$k}); +$opts->{$k} = $v; +} +sub get_btrfs_opt { +my ($k) = @_; +my $opts = get('btrfs_opts'); +return defined($k) ? $opts->{$k} : $opts; +} + sub set_target_hd { set_key('target_hd', $_[0]); } sub get_target_hd { return get('target_hd'); } diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs index 202ad41..30b6196 100644 --- a/proxmox-auto-installer/src/utils.rs +++ b/proxmox-auto-installer/src/utils.rs @@ -326,6 +326,7 @@ pub fn parse_answer( minfree: None, maxvz: None, zfs_opts: None, +btrfs_opts: None, target_hd: None, disk_selection: BTreeMap::new(), lvm_auto_rename: 1, diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs index e77914b..972b66c 100644 --- a/proxmox-installer-common/src/options.rs +++ b/proxmox-installer-common/src/options.rs @@ -102,10 +102,40 @@ impl LvmBootdiskOptions { } } +/// See the accompanying mount option in btrfs(5). +#[derive(Copy, Clone, Debug, Default, Deserialize, Eq, PartialEq)] +#[serde(rename_all(deserialize = "lowercase"))] +pub enum BtrfsCompressOption { +On, +#[default] +Off, +Zlib, +Lzo, +Zstd, +} + +impl fmt::Display for BtrfsCompressOption { +fn fmt(, f: fmt::Formatter) -> fmt::Result { +write!(f, "{}", format!("{self:?}").to_lowercase()) +} +} + +impl From<> for String { +fn from(value: ) -> Self { +value.to_string() +} +} + +pub const BTRFS_COMPRESS_OPTIONS: &[BtrfsCompressOption] = { +use BtrfsCompressOption::*; +&[On, Off, Zlib, Lzo, Zstd] +}; + #[derive(Clone, Debug)] pub struct BtrfsBootdiskOptions { pub disk_size: f64, pub selected_disks: Vec, +pub compress: BtrfsCompressOption, } impl BtrfsBootdiskOptions { @@ -115,6 +145,7 @@ impl BtrfsBootdiskOptions { Self { disk_size: disk.size, selected_disks: (0..disks.len()).collect(), +compress: BtrfsCompressOption::default(), } } } diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs index 64d05af..81f3533 100644 --- a/proxmox-installer-common/src/setup.rs +++ b/proxmox-installer-common/src/setup.rs @@ -13,8 +13,8 @@ use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; use crate::{ options::{ -BtrfsRaidLevel, Disk, FsType, ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption, -ZfsRaidLevel, +BtrfsBootdiskOptions, BtrfsCompressOption, BtrfsRaidLevel, Disk, FsType, +ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel, }, utils::CidrAddress, }; @@ -220,6 +220,20 @@ impl From for InstallZfsOption { } } +#[derive(Debug, Deserialize, Serialize)] +pub struct InstallBtrfsOption { +#[serde(serialize_with = "serialize_as_display")] +pub compress: BtrfsCompressOption, +} + +impl From for InstallBtrfsOption { +fn from(opts: BtrfsBootdiskOptions) -> Self { +InstallBtrfsOption { +compress: opts.compress, +} +} +} + pub fn read_json Deserialize<'de>, P: AsRef>(path: P) -> Result { let file = File::open(path).map_err(|err| err.to_string())?; let reader = BufReader::new(file); @@ -466,6 +480,9 @@ pub struct InstallConfig { #[serde(skip_serializing_if = "Option::is_none")] pub zfs_opts: Option, +#[serde(skip_serializing_if = "Option::is_none")] +pub btrfs_opts: Option, + #[serde( serialize_with = "serialize_disk_opt", skip_serializing_if = "Option::is_none", diff --git a/proxmox-tui-installe
[pve-devel] [PATCH installer 6/6] auto-installer: add btrfs answer parsing tests
Signed-off-by: Christoph Heiss --- .../tests/resources/parse_answer/btrfs.json | 24 +++ .../tests/resources/parse_answer/btrfs.toml | 17 + 2 files changed, 41 insertions(+) create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/btrfs.json create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml diff --git a/proxmox-auto-installer/tests/resources/parse_answer/btrfs.json b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.json new file mode 100644 index 000..d6cc30d --- /dev/null +++ b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.json @@ -0,0 +1,24 @@ +{ + "autoreboot": 1, + "cidr": "192.168.1.114/24", + "country": "at", + "dns": "192.168.1.254", + "domain": "testinstall", + "disk_selection": { +"6": "6", +"7": "7" + }, + "lvm_auto_rename": 1, + "filesys": "btrfs (RAID1)", + "gateway": "192.168.1.1", + "hdsize": 80.0, + "hostname": "pveauto", + "keymap": "de", + "mailto": "mail@no.invalid", + "mngmt_nic": "eno1", + "password": "123456", + "timezone": "Europe/Vienna", + "btrfs_opts": { +"compress": "zlib" + } +} diff --git a/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml new file mode 100644 index 000..8fcd27d --- /dev/null +++ b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml @@ -0,0 +1,17 @@ +[global] +keyboard = "de" +country = "at" +fqdn = "pveauto.testinstall" +mailto = "mail@no.invalid" +timezone = "Europe/Vienna" +root_password = "123456" + +[network] +source = "from-dhcp" + +[disk-setup] +filesystem = "btrfs" +btrfs.raid = "raid1" +btrfs.compress = "zlib" +btrfs.hdsize = 80 +disk_list = ["sda", "sdb"] -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 2/6] fix #5250: install: write btrfs `compress` option to fstab
`compress` instead of `compress-force` is used, as the latter can have unindented (performance) implications, as the name implies. That would be neither expected by users nor should such a decision made without the user explicitly opting for it. Others do the same, e.g. the installer for RedHat/Fedora systems (aka. Anaconda) opts for `compress` also. And if Fedora uses it for their systems, it's fine for us too. Signed-off-by: Christoph Heiss --- Proxmox/Install.pm | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm index c0f8955..f3bc5aa 100644 --- a/Proxmox/Install.pm +++ b/Proxmox/Install.pm @@ -1011,7 +1011,13 @@ sub extract_data { die "unable to detect FS UUID" if !defined($fsuuid); - $fstab .= "UUID=$fsuuid / btrfs defaults 0 1\n"; + my $btrfs_opts = Proxmox::Install::Config::get_btrfs_opt(); + + my $mountopts = 'defaults'; + $mountopts .= ",compress=$btrfs_opts->{compress}" + if $btrfs_opts->{compress} ne 'off'; + + $fstab .= "UUID=$fsuuid / btrfs $mountopts 0 1\n"; } else { my $root_mountopt = $fssetup->{$filesys}->{root_mountopt} || 'defaults'; $fstab .= "$rootdev / $filesys ${root_mountopt} 0 1\n"; -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 3/6] fix #5250: proxinstall: expose new btrfs `compress` option
Signed-off-by: Christoph Heiss --- Proxmox/Install.pm | 7 +-- proxinstall| 15 +++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm index f3bc5aa..60f38e5 100644 --- a/Proxmox/Install.pm +++ b/Proxmox/Install.pm @@ -1014,8 +1014,11 @@ sub extract_data { my $btrfs_opts = Proxmox::Install::Config::get_btrfs_opt(); my $mountopts = 'defaults'; - $mountopts .= ",compress=$btrfs_opts->{compress}" - if $btrfs_opts->{compress} ne 'off'; + if ($btrfs_opts->{compress} eq 'on') { + $mountopts .= ',compress'; + } elsif ($btrfs_opts->{compress} ne 'off') { + $mountopts .= ",compress=$btrfs_opts->{compress}"; + } $fstab .= "UUID=$fsuuid / btrfs $mountopts 0 1\n"; } else { diff --git a/proxinstall b/proxinstall index a6a4cfb..7180fe6 100755 --- a/proxinstall +++ b/proxinstall @@ -1160,6 +1160,21 @@ my $create_raid_advanced_grid = sub { my $create_btrfs_raid_advanced_grid = sub { my ($hdsize_btn) = @_; my $labeled_widgets = []; + +my $combo_compress = Gtk3::ComboBoxText->new(); +$combo_compress->set_tooltip_text("btrfs compression algorithm for boot volume"); +my $comp_opts = ["on", "off", "zlib", "lzo", "zstd"]; +foreach my $opt (@$comp_opts) { + $combo_compress->append($opt, $opt); +} +my $compress = Proxmox::Install::Config::get_btrfs_opt('compress') // 'off'; +$combo_compress->set_active_id($compress); +$combo_compress->signal_connect (changed => sub { + my $w = shift; + Proxmox::Install::Config::set_btrfs_opt('compress', $w->get_active_text()); +}); +push @$labeled_widgets, ['compress', $combo_compress]; + push @$labeled_widgets, ['hdsize', $hdsize_btn, 'GB']; return $create_label_widget_grid->($labeled_widgets);; }; -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer 0/4] assistant: clean up glob patterns & regexes
On Mon, May 13, 2024 at 11:18:51AM +0200, Aaron Lauterer wrote: > it seems these patches don't apply anymore. could you please do a rebase on > current master? Sure, thanks for the notice! Seems 1a01a01 ("assistant: keep prepared iso bootable on uefi with flash drives") "broke" it, due to an import statement change :') v2: https://lists.proxmox.com/pipermail/pve-devel/2024-May/063833.html > > On 2024-05-07 15:21, Christoph Heiss wrote: > > The proxmox-auto-install-assistant uses > >- glob patterns for disk matching, which can be pre-compiled for > > efficiency > >- regexes for udev property matching, which can be simplified by some > > simple prefix matching & splitting on = > > > > The latter also significantly reduces binary size due to the removing > > the regex dependency, for details see patch #4. > > > > Overall no functional changes in this series. > > > > Christoph Heiss (4): > >tree-wide: run rustfmt, fix clippy warnings > >assistant: drop unused `log` dependency > >assistant: pre-compile ignored block device patterns > >assistant: avoid regex for simple prefix matching > > > > proxmox-auto-install-assistant/Cargo.toml | 2 - > > proxmox-auto-install-assistant/src/main.rs| 75 --- > > proxmox-auto-installer/tests/parse-answer.rs | 14 ++-- > > .../src/fetch_plugins/partition.rs| 10 +-- > > 4 files changed, 45 insertions(+), 56 deletions(-) > > > > -- > > 2.44.0 > > > > > > > > ___ > > pve-devel mailing list > > pve-devel@lists.proxmox.com > > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer v2 4/4] assistant: avoid regex for simple prefix matching
udev properties are very easy to parse and can be done by doing a line-based scan and matching the prefix, splitting once for properties. Avoids the use of regexes and signicantly reduces binary size by about -38%(!). Tested by comparing the output of `proxmox-auto-install-assistant device-info`, running it before and after the changes. Stripped binary size for release builds: before: 3103104 bytes ~ 2.96MiB after: 1935744 bytes ~ 1.85MiB textdata bss dec hex filename 2906765 187144 537 3094446 2f37ae proxmox-auto-install-assistant-before 1871090 55736 497 1927323 1d689b proxmox-auto-install-assistant-after No functional changes. Signed-off-by: Christoph Heiss --- Changes v1 -> v2: * rebased on latest master proxmox-auto-install-assistant/Cargo.toml | 1 - proxmox-auto-install-assistant/src/main.rs | 57 +- 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/proxmox-auto-install-assistant/Cargo.toml b/proxmox-auto-install-assistant/Cargo.toml index 0286c80..766b445 100644 --- a/proxmox-auto-install-assistant/Cargo.toml +++ b/proxmox-auto-install-assistant/Cargo.toml @@ -15,7 +15,6 @@ anyhow = "1.0" clap = { version = "4.0", features = ["derive"] } glob = "0.3" proxmox-auto-installer = { path = "../proxmox-auto-installer" } -regex = "1.7" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" toml = "0.7" diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs index 790dbc7..7c0b0c6 100644 --- a/proxmox-auto-install-assistant/src/main.rs +++ b/proxmox-auto-install-assistant/src/main.rs @@ -1,7 +1,6 @@ use anyhow::{bail, format_err, Result}; use clap::{Args, Parser, Subcommand, ValueEnum}; use glob::Pattern; -use regex::Regex; use serde::Serialize; use std::{ collections::BTreeMap, @@ -439,13 +438,9 @@ fn get_disks() -> Result>> { Pattern::new("sr[0-9]*")?, ]; -// compile Regex here once and not inside the loop -let re_disk = Regex::new(r"(?m)^E: DEVTYPE=disk")?; -let re_cdrom = Regex::new(r"(?m)^E: ID_CDROM")?; -let re_iso9660 = Regex::new(r"(?m)^E: ID_FS_TYPE=iso9660")?; - -let re_name = Regex::new(r"(?m)^N: (.*)$")?; -let re_props = Regex::new(r"(?m)^E: ([^=]+)=(.*)$")?; +const PROP_DEVTYP_PREFIX: = "E: DEVTYPE="; +const PROP_CDROM: = "E: ID_CDROM"; +const PROP_ISO9660_FS: = "E: ID_FS_TYPE=iso9660"; let mut disks: BTreeMap> = BTreeMap::new(); @@ -467,30 +462,27 @@ fn get_disks() -> Result>> { } }; -if !re_disk.is_match() { -continue 'outer; -}; -if re_cdrom.is_match() { -continue 'outer; -}; -if re_iso9660.is_match() { -continue 'outer; -}; - let mut name = filename; -if let Some(cap) = re_name.captures() { -if let Some(res) = cap.get(1) { -name = String::from(res.as_str()); +let mut udev_props: BTreeMap = BTreeMap::new(); +for line in output.lines() { +if let Some(prop) = line.strip_prefix(PROP_DEVTYP_PREFIX) { +if prop != "disk" { +continue 'outer; +} } -} -let mut udev_props: BTreeMap = BTreeMap::new(); +if line.starts_with(PROP_CDROM) || line.starts_with(PROP_ISO9660_FS) { +continue 'outer; +} -for line in output.lines() { -if let Some(caps) = re_props.captures(line) { -let key = String::from(caps.get(1).unwrap().as_str()); -let value = String::from(caps.get(2).unwrap().as_str()); -udev_props.insert(key, value); +if let Some(prop) = line.strip_prefix("N: ") { +name = prop.to_owned(); +}; + +if let Some(prop) = line.strip_prefix("E: ") { +if let Some((key, val)) = prop.split_once('=') { +udev_props.insert(key.to_owned(), val.to_owned()); +} } } @@ -500,7 +492,6 @@ fn get_disks() -> Result>> { } fn get_nics() -> Result>> { -let re_props = Regex::new(r"(?m)^E: (.*)=(.*)$")?; let mut nics: BTreeMap> = BTreeMap::new(); let links = get_nic_list()?; @@ -518,10 +509,10 @@ fn get_nics() -> Result>> { let mut udev_props: BTreeMap = BTreeMap::new(); for line in output.lines() { -if let Some(caps) = re_props.captures(line) { -let key = String::from(caps.get(1).unwrap().as_str()); -let value = St
[pve-devel] [PATCH installer v2 3/4] assistant: pre-compile ignored block device patterns
No functional changes. Signed-off-by: Christoph Heiss --- Changes v1 -> v2: * no changes proxmox-auto-install-assistant/src/main.rs | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs index 1447175..790dbc7 100644 --- a/proxmox-auto-install-assistant/src/main.rs +++ b/proxmox-auto-install-assistant/src/main.rs @@ -430,13 +430,13 @@ fn get_iso_uuid(iso: ) -> Result { } fn get_disks() -> Result>> { -let unwantend_block_devs = vec![ -"ram[0-9]*", -"loop[0-9]*", -"md[0-9]*", -"dm-*", -"fd[0-9]*", -"sr[0-9]*", +let unwanted_block_devs = [ +Pattern::new("ram[0-9]*")?, +Pattern::new("loop[0-9]*")?, +Pattern::new("md[0-9]*")?, +Pattern::new("dm-*")?, +Pattern::new("fd[0-9]*")?, +Pattern::new("sr[0-9]*")?, ]; // compile Regex here once and not inside the loop @@ -453,8 +453,8 @@ fn get_disks() -> Result>> { let entry = entry.unwrap(); let filename = entry.file_name().into_string().unwrap(); -for p in _block_devs { -if Pattern::new(p)?.matches() { +for p in _block_devs { +if p.matches() { continue 'outer; } } -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer v2 2/4] assistant: drop unused `log` dependency
No functional changes. Signed-off-by: Christoph Heiss --- Changes v1 -> v2: * no changes proxmox-auto-install-assistant/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/proxmox-auto-install-assistant/Cargo.toml b/proxmox-auto-install-assistant/Cargo.toml index eaca7f8..0286c80 100644 --- a/proxmox-auto-install-assistant/Cargo.toml +++ b/proxmox-auto-install-assistant/Cargo.toml @@ -14,7 +14,6 @@ homepage = "https://www.proxmox.com; anyhow = "1.0" clap = { version = "4.0", features = ["derive"] } glob = "0.3" -log = "0.4.20" proxmox-auto-installer = { path = "../proxmox-auto-installer" } regex = "1.7" serde = { version = "1.0", features = ["derive"] } -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer v2 0/4] assistant: clean up glob patterns & regexes
The proxmox-auto-install-assistant uses - glob patterns for disk matching, which can be pre-compiled for efficiency - regexes for udev property matching, which can be simplified by some simple prefix matching & splitting on = The latter also significantly reduces binary size due to the removing the regex dependency, for details see patch #4. Overall no functional changes in this series. v1: https://lists.proxmox.com/pipermail/pve-devel/2024-May/063802.html Changes v1 -> v2: * rebased on latest master Christoph Heiss (4): tree-wide: run rustfmt, fix clippy warnings assistant: drop unused `log` dependency assistant: pre-compile ignored block device patterns assistant: avoid regex for simple prefix matching proxmox-auto-install-assistant/Cargo.toml | 2 - proxmox-auto-install-assistant/src/main.rs| 75 --- proxmox-auto-installer/tests/parse-answer.rs | 14 ++-- .../src/fetch_plugins/partition.rs| 10 +-- 4 files changed, 45 insertions(+), 56 deletions(-) -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer v2 1/4] tree-wide: run rustfmt, fix clippy warnings
No functional changes. Signed-off-by: Christoph Heiss --- Changes v1 -> v2: * no changes proxmox-auto-installer/tests/parse-answer.rs | 14 +++--- .../src/fetch_plugins/partition.rs | 10 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs index 4014b6d..e77a769 100644 --- a/proxmox-auto-installer/tests/parse-answer.rs +++ b/proxmox-auto-installer/tests/parse-answer.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use serde_json::Value; use std::fs; @@ -24,9 +24,9 @@ fn get_answer(path: PathBuf) -> Result { Ok(answer) } -fn setup_test_basic(path: ) -> (SetupInfo, LocaleInfo, RuntimeInfo, UdevInfo) { +fn setup_test_basic(path: ) -> (SetupInfo, LocaleInfo, RuntimeInfo, UdevInfo) { let installer_info: SetupInfo = { -let mut path = path.clone(); +let mut path = path.to_path_buf(); path.push("iso-info.json"); read_json() @@ -35,7 +35,7 @@ fn setup_test_basic(path: ) -> (SetupInfo, LocaleInfo, RuntimeInfo, Udev }; let locale_info = { -let mut path = path.clone(); +let mut path = path.to_path_buf(); path.push("locales.json"); read_json() @@ -44,7 +44,7 @@ fn setup_test_basic(path: ) -> (SetupInfo, LocaleInfo, RuntimeInfo, Udev }; let mut runtime_info: RuntimeInfo = { -let mut path = path.clone(); +let mut path = path.to_path_buf(); path.push("run-env-info.json"); read_json() @@ -53,7 +53,7 @@ fn setup_test_basic(path: ) -> (SetupInfo, LocaleInfo, RuntimeInfo, Udev }; let udev_info: UdevInfo = { -let mut path = path.clone(); +let mut path = path.to_path_buf(); path.push("run-env-udev.json"); read_json() @@ -71,7 +71,7 @@ fn setup_test_basic(path: ) -> (SetupInfo, LocaleInfo, RuntimeInfo, Udev fn test_parse_answers() { let path = get_test_resource_path().unwrap(); let (setup_info, locales, runtime_info, udev_info) = setup_test_basic(); -let mut tests_path = path.clone(); +let mut tests_path = path; tests_path.push("parse_answer"); let test_dir = fs::read_dir(tests_path.clone()).unwrap(); diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs b/proxmox-fetch-answer/src/fetch_plugins/partition.rs index 0479c8f..7213493 100644 --- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs +++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs @@ -31,7 +31,7 @@ impl FetchFromPartition { } fn path_exists_logged(file_name: , search_path: ) -> Option { -let path = Path::new(search_path).join(_name); +let path = Path::new(search_path).join(file_name); info!("Testing partition search path {path:?}"); match path.try_exists() { Ok(true) => Some(path), @@ -51,14 +51,14 @@ fn path_exists_logged(file_name: , search_path: ) -> Option { fn scan_partlabels(partlabel: , search_path: ) -> Result { let partlabel_upper_case = partlabel.to_uppercase(); if let Some(path) = path_exists_logged(_upper_case, search_path) { -info!("Found partition with label '{partlabel_upper_case}'"); -return Ok(path); +info!("Found partition with label '{partlabel_upper_case}'"); +return Ok(path); } let partlabel_lower_case = partlabel.to_lowercase(); if let Some(path) = path_exists_logged(_lower_case, search_path) { -info!("Found partition with label '{partlabel_lower_case}'"); -return Ok(path); +info!("Found partition with label '{partlabel_lower_case}'"); +return Ok(path); } bail!("Could not detect upper or lower case labels for '{partlabel}'"); -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs] asciidoc: add clickable anchor link to all headings
Works the same as in our PBS documentation and is generally common for documentations. Very useful for linking specific sections of the documentation in other places. Previously, this always had to be done by getting the correct anchor from the HTML directly via e.g. browser devtools. Signed-off-by: Christoph Heiss --- This patch should also pretty much also apply to pmg-docs, although I did not explicitly test that yet. asciidoc/pve-docs.css | 34 ++ asciidoc/pve-html.conf | 22 +- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/asciidoc/pve-docs.css b/asciidoc/pve-docs.css index 19c176e..9ddf57c 100644 --- a/asciidoc/pve-docs.css +++ b/asciidoc/pve-docs.css @@ -1,6 +1,7 @@ :root { /* pre-defined colors */ --pdt-grey-950: hsl(0deg, 0%, 95%); +--pdt-grey-750: hsl(0deg, 0%, 75%); --pdt-grey-400: hsl(0deg, 0%, 40%); --pdt-grey-250: hsl(0deg, 0%, 25%); --pdt-grey-150: hsl(0deg, 0%, 15%); @@ -41,6 +42,39 @@ h6 { font-size: 1.0em; } +/* Support for heading anchor links */ +h3 { +border-bottom: unset; +} + +h3 > span { +display: inline-block; +border-bottom: 2px solid silver; +} + +a.headerlink { +color: var(--pdt-grey-750); +padding: 0 4px; +text-decoration: none; +visibility: hidden; +} + +/* add it as an pseudo-element, so that it does not show up in the ToC */ +a.headerlink::after { +content: '\00b6'; +text-decoration: none; +} + +h1:hover > a.headerlink, +h2:hover > a.headerlink, +h3:hover > a.headerlink, +h4:hover > a.headerlink, +h5:hover > a.headerlink, +h6:hover > a.headerlink { + visibility: visible; +} + +/* Dark mode theme */ @media screen and (prefers-color-scheme: dark) { :root { color-scheme: dark; diff --git a/asciidoc/pve-html.conf b/asciidoc/pve-html.conf index 6e895e6..396a5e7 100644 --- a/asciidoc/pve-html.conf +++ b/asciidoc/pve-html.conf @@ -505,7 +505,10 @@ bodydata=| [sect1] -{numbered?{sectnum} }{title} + +{numbered?{sectnum} }{title} +{id? } + | @@ -513,25 +516,34 @@ bodydata=| [sect2] -{numbered?{sectnum} }{title} + +{numbered?{sectnum} }{title} +{id? } + | [sect3] -{numbered?{sectnum} }{title} +{numbered?{sectnum} }{title} +{id? } + | [sect4] -{title} +{title} +{id? } + | [sect5] -{title} +{title} +{id? } + | -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 4/4] assistant: avoid regex for simple prefix matching
udev properties are very easy to parse and can be done by doing a line-based scan and matching the prefix, splitting once for properties. Avoids the use of regexes and signicantly reduces binary size by about -38%(!). Tested by comparing the output of `proxmox-auto-install-assistant device-info`, running it before and after the changes. Stripped binary size for release builds: before: 3103104 bytes ~ 2.96MiB after: 1935744 bytes ~ 1.85MiB textdata bss dec hex filename 2906765 187144 537 3094446 2f37ae proxmox-auto-install-assistant-before 1871090 55736 497 1927323 1d689b proxmox-auto-install-assistant-after No functional changes. Signed-off-by: Christoph Heiss --- proxmox-auto-install-assistant/Cargo.toml | 1 - proxmox-auto-install-assistant/src/main.rs | 57 +- 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/proxmox-auto-install-assistant/Cargo.toml b/proxmox-auto-install-assistant/Cargo.toml index 0286c80..766b445 100644 --- a/proxmox-auto-install-assistant/Cargo.toml +++ b/proxmox-auto-install-assistant/Cargo.toml @@ -15,7 +15,6 @@ anyhow = "1.0" clap = { version = "4.0", features = ["derive"] } glob = "0.3" proxmox-auto-installer = { path = "../proxmox-auto-installer" } -regex = "1.7" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" toml = "0.7" diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs index 906f144..828ba76 100644 --- a/proxmox-auto-install-assistant/src/main.rs +++ b/proxmox-auto-install-assistant/src/main.rs @@ -1,7 +1,6 @@ use anyhow::{bail, Result}; use clap::{Args, Parser, Subcommand, ValueEnum}; use glob::Pattern; -use regex::Regex; use serde::Serialize; use std::{ collections::BTreeMap, @@ -401,13 +400,9 @@ fn get_disks() -> Result>> { Pattern::new("sr[0-9]*")?, ]; -// compile Regex here once and not inside the loop -let re_disk = Regex::new(r"(?m)^E: DEVTYPE=disk")?; -let re_cdrom = Regex::new(r"(?m)^E: ID_CDROM")?; -let re_iso9660 = Regex::new(r"(?m)^E: ID_FS_TYPE=iso9660")?; - -let re_name = Regex::new(r"(?m)^N: (.*)$")?; -let re_props = Regex::new(r"(?m)^E: ([^=]+)=(.*)$")?; +const PROP_DEVTYP_PREFIX: = "E: DEVTYPE="; +const PROP_CDROM: = "E: ID_CDROM"; +const PROP_ISO9660_FS: = "E: ID_FS_TYPE=iso9660"; let mut disks: BTreeMap> = BTreeMap::new(); @@ -429,30 +424,27 @@ fn get_disks() -> Result>> { } }; -if !re_disk.is_match() { -continue 'outer; -}; -if re_cdrom.is_match() { -continue 'outer; -}; -if re_iso9660.is_match() { -continue 'outer; -}; - let mut name = filename; -if let Some(cap) = re_name.captures() { -if let Some(res) = cap.get(1) { -name = String::from(res.as_str()); +let mut udev_props: BTreeMap = BTreeMap::new(); +for line in output.lines() { +if let Some(prop) = line.strip_prefix(PROP_DEVTYP_PREFIX) { +if prop != "disk" { +continue 'outer; +} } -} -let mut udev_props: BTreeMap = BTreeMap::new(); +if line.starts_with(PROP_CDROM) || line.starts_with(PROP_ISO9660_FS) { +continue 'outer; +} -for line in output.lines() { -if let Some(caps) = re_props.captures(line) { -let key = String::from(caps.get(1).unwrap().as_str()); -let value = String::from(caps.get(2).unwrap().as_str()); -udev_props.insert(key, value); +if let Some(prop) = line.strip_prefix("N: ") { +name = prop.to_owned(); +}; + +if let Some(prop) = line.strip_prefix("E: ") { +if let Some((key, val)) = prop.split_once('=') { +udev_props.insert(key.to_owned(), val.to_owned()); +} } } @@ -462,7 +454,6 @@ fn get_disks() -> Result>> { } fn get_nics() -> Result>> { -let re_props = Regex::new(r"(?m)^E: (.*)=(.*)$")?; let mut nics: BTreeMap> = BTreeMap::new(); let links = get_nic_list()?; @@ -480,10 +471,10 @@ fn get_nics() -> Result>> { let mut udev_props: BTreeMap = BTreeMap::new(); for line in output.lines() { -if let Some(caps) = re_props.captures(line) { -let key = String::from(caps.get(1).unwrap().as_str()); -let value = String::from(caps.get(2).unwrap().as_str()); -ude
[pve-devel] [PATCH installer 0/4] assistant: clean up glob patterns & regexes
The proxmox-auto-install-assistant uses - glob patterns for disk matching, which can be pre-compiled for efficiency - regexes for udev property matching, which can be simplified by some simple prefix matching & splitting on = The latter also significantly reduces binary size due to the removing the regex dependency, for details see patch #4. Overall no functional changes in this series. Christoph Heiss (4): tree-wide: run rustfmt, fix clippy warnings assistant: drop unused `log` dependency assistant: pre-compile ignored block device patterns assistant: avoid regex for simple prefix matching proxmox-auto-install-assistant/Cargo.toml | 2 - proxmox-auto-install-assistant/src/main.rs| 75 --- proxmox-auto-installer/tests/parse-answer.rs | 14 ++-- .../src/fetch_plugins/partition.rs| 10 +-- 4 files changed, 45 insertions(+), 56 deletions(-) -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 2/4] assistant: drop unused `log` dependency
No functional changes. Signed-off-by: Christoph Heiss --- proxmox-auto-install-assistant/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/proxmox-auto-install-assistant/Cargo.toml b/proxmox-auto-install-assistant/Cargo.toml index eaca7f8..0286c80 100644 --- a/proxmox-auto-install-assistant/Cargo.toml +++ b/proxmox-auto-install-assistant/Cargo.toml @@ -14,7 +14,6 @@ homepage = "https://www.proxmox.com; anyhow = "1.0" clap = { version = "4.0", features = ["derive"] } glob = "0.3" -log = "0.4.20" proxmox-auto-installer = { path = "../proxmox-auto-installer" } regex = "1.7" serde = { version = "1.0", features = ["derive"] } -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 1/4] tree-wide: run rustfmt, fix clippy warnings
No functional changes. Signed-off-by: Christoph Heiss --- proxmox-auto-installer/tests/parse-answer.rs | 14 +++--- .../src/fetch_plugins/partition.rs | 10 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs index 4014b6d..e77a769 100644 --- a/proxmox-auto-installer/tests/parse-answer.rs +++ b/proxmox-auto-installer/tests/parse-answer.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use serde_json::Value; use std::fs; @@ -24,9 +24,9 @@ fn get_answer(path: PathBuf) -> Result { Ok(answer) } -fn setup_test_basic(path: ) -> (SetupInfo, LocaleInfo, RuntimeInfo, UdevInfo) { +fn setup_test_basic(path: ) -> (SetupInfo, LocaleInfo, RuntimeInfo, UdevInfo) { let installer_info: SetupInfo = { -let mut path = path.clone(); +let mut path = path.to_path_buf(); path.push("iso-info.json"); read_json() @@ -35,7 +35,7 @@ fn setup_test_basic(path: ) -> (SetupInfo, LocaleInfo, RuntimeInfo, Udev }; let locale_info = { -let mut path = path.clone(); +let mut path = path.to_path_buf(); path.push("locales.json"); read_json() @@ -44,7 +44,7 @@ fn setup_test_basic(path: ) -> (SetupInfo, LocaleInfo, RuntimeInfo, Udev }; let mut runtime_info: RuntimeInfo = { -let mut path = path.clone(); +let mut path = path.to_path_buf(); path.push("run-env-info.json"); read_json() @@ -53,7 +53,7 @@ fn setup_test_basic(path: ) -> (SetupInfo, LocaleInfo, RuntimeInfo, Udev }; let udev_info: UdevInfo = { -let mut path = path.clone(); +let mut path = path.to_path_buf(); path.push("run-env-udev.json"); read_json() @@ -71,7 +71,7 @@ fn setup_test_basic(path: ) -> (SetupInfo, LocaleInfo, RuntimeInfo, Udev fn test_parse_answers() { let path = get_test_resource_path().unwrap(); let (setup_info, locales, runtime_info, udev_info) = setup_test_basic(); -let mut tests_path = path.clone(); +let mut tests_path = path; tests_path.push("parse_answer"); let test_dir = fs::read_dir(tests_path.clone()).unwrap(); diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs b/proxmox-fetch-answer/src/fetch_plugins/partition.rs index 0479c8f..7213493 100644 --- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs +++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs @@ -31,7 +31,7 @@ impl FetchFromPartition { } fn path_exists_logged(file_name: , search_path: ) -> Option { -let path = Path::new(search_path).join(_name); +let path = Path::new(search_path).join(file_name); info!("Testing partition search path {path:?}"); match path.try_exists() { Ok(true) => Some(path), @@ -51,14 +51,14 @@ fn path_exists_logged(file_name: , search_path: ) -> Option { fn scan_partlabels(partlabel: , search_path: ) -> Result { let partlabel_upper_case = partlabel.to_uppercase(); if let Some(path) = path_exists_logged(_upper_case, search_path) { -info!("Found partition with label '{partlabel_upper_case}'"); -return Ok(path); +info!("Found partition with label '{partlabel_upper_case}'"); +return Ok(path); } let partlabel_lower_case = partlabel.to_lowercase(); if let Some(path) = path_exists_logged(_lower_case, search_path) { -info!("Found partition with label '{partlabel_lower_case}'"); -return Ok(path); +info!("Found partition with label '{partlabel_lower_case}'"); +return Ok(path); } bail!("Could not detect upper or lower case labels for '{partlabel}'"); -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 3/4] assistant: pre-compile ignored block device patterns
No functional changes. Signed-off-by: Christoph Heiss --- proxmox-auto-install-assistant/src/main.rs | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs index 0debd29..906f144 100644 --- a/proxmox-auto-install-assistant/src/main.rs +++ b/proxmox-auto-install-assistant/src/main.rs @@ -392,13 +392,13 @@ fn inject_file_to_iso(iso: , file: , location: ) -> Result<( } fn get_disks() -> Result>> { -let unwantend_block_devs = vec![ -"ram[0-9]*", -"loop[0-9]*", -"md[0-9]*", -"dm-*", -"fd[0-9]*", -"sr[0-9]*", +let unwanted_block_devs = [ +Pattern::new("ram[0-9]*")?, +Pattern::new("loop[0-9]*")?, +Pattern::new("md[0-9]*")?, +Pattern::new("dm-*")?, +Pattern::new("fd[0-9]*")?, +Pattern::new("sr[0-9]*")?, ]; // compile Regex here once and not inside the loop @@ -415,8 +415,8 @@ fn get_disks() -> Result>> { let entry = entry.unwrap(); let filename = entry.file_name().into_string().unwrap(); -for p in _block_devs { -if Pattern::new(p)?.matches() { +for p in _block_devs { +if p.matches() { continue 'outer; } } -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] auto-installer: move ssh keys setup to low-level installer
.. thereby, also fixing a accidental shell injection. Since run_cmd{,s}() is nowhere else used anymore, they can be removed too. Also mostly reverts commit 5878dc4ae "auto-installer: handle auto-reboot info messages directly" in the process too. Reported-by: Friedrich Weber Signed-off-by: Christoph Heiss --- Proxmox/Install.pm| 7 ++ Proxmox/Install/Config.pm | 4 ++ .../src/bin/proxmox-auto-installer.rs | 34 + proxmox-auto-installer/src/utils.rs | 70 ++- .../resources/parse_answer/disk_match.json| 2 +- .../parse_answer/disk_match_all.json | 2 +- .../parse_answer/disk_match_any.json | 2 +- .../tests/resources/parse_answer/minimal.json | 2 +- .../resources/parse_answer/nic_matching.json | 2 +- .../resources/parse_answer/specific_nic.json | 2 +- .../tests/resources/parse_answer/zfs.json | 2 +- proxmox-installer-common/src/setup.rs | 2 + proxmox-tui-installer/src/setup.rs| 1 + 13 files changed, 27 insertions(+), 105 deletions(-) diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm index e2f8ad9..dcbedb2 100644 --- a/Proxmox/Install.pm +++ b/Proxmox/Install.pm @@ -1271,6 +1271,13 @@ _EOD my $octets = encode("utf-8", Proxmox::Install::Config::get_password()); run_command("chroot $targetdir /usr/sbin/chpasswd", undef, "root:$octets\n"); + # set root ssh keys + my $ssh_keys = Proxmox::Install::Config::get_root_ssh_keys(); + if (scalar(@$ssh_keys) > 0) { + mkdir "$targetdir/root/.ssh"; + file_write_all("$targetdir/root/.ssh/authorized_keys", join("\n", @$ssh_keys)); + } + my $mailto = Proxmox::Install::Config::get_mailto(); if ($iso_env->{product} eq 'pmg') { # save admin email diff --git a/Proxmox/Install/Config.pm b/Proxmox/Install/Config.pm index 5ef3438..ecd8a74 100644 --- a/Proxmox/Install/Config.pm +++ b/Proxmox/Install/Config.pm @@ -92,6 +92,7 @@ my sub init_cfg { # root credentials & details password => undef, mailto => 'mail@example.invalid', + root_ssh_keys => [], # network related mngmt_nic => undef, @@ -201,6 +202,9 @@ sub get_password { return get('password'); } sub set_mailto { set_key('mailto', $_[0]); } sub get_mailto { return get('mailto'); } +sub set_root_ssh_keys { set_key('root_ssh_keys', $_[0]); } +sub get_root_ssh_keys { return get('root_ssh_keys'); } + sub set_mngmt_nic { set_key('mngmt_nic', $_[0]); } sub get_mngmt_nic { return get('mngmt_nic'); } diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs index 97b5746..2e7d20d 100644 --- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs +++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs @@ -5,8 +5,6 @@ use std::{ io::{BufRead, BufReader, Write}, path::PathBuf, process::ExitCode, -thread, -time::Duration, }; use proxmox_installer_common::setup::{ @@ -17,7 +15,7 @@ use proxmox_auto_installer::{ answer::Answer, log::AutoInstLogger, udevinfo::UdevInfo, -utils::{parse_answer, run_cmds, LowLevelMessage}, +utils::{parse_answer, LowLevelMessage}, }; static LOGGER: AutoInstLogger = AutoInstLogger; @@ -93,15 +91,8 @@ fn main() -> ExitCode { } } -run_postinstallation(); - // TODO: (optionally) do a HTTP post with basic system info, like host SSH public key(s) here -for secs in (0..=5).rev() { -info!("Installation finished - auto-rebooting in {secs} seconds .."); -thread::sleep(Duration::from_secs(1)); -} - ExitCode::SUCCESS } @@ -178,8 +169,7 @@ fn run_installation( if state == "err" { bail!("{message}"); } -// Do not print anything if the installation was successful, -// as we handle that here ourselves +info!("Finished: '{state}' {message}"); } }; } @@ -187,23 +177,3 @@ fn run_installation( }; inner().map_err(|err| format_err!("low level installer returned early: {err}")) } - -fn run_postinstallation(answer: ) { -if !answer.global.root_ssh_keys.is_empty() { -// FIXME: move handling this into the low-level installer and just pass in installation -// config, as doing parts of the installation/configuration here and parts in the -// low-level installer is not nice (seemingly spooky actions at a distance). -info!("Adding root ssh-keys to the installed system .."); -run_cmds( -"ssh-key-setup", -
[pve-devel] [PATCH installer] tui: update screen during installation only when necessary
This can significantly reduces CPU load and even speed up the installation a lot on single-core machines. While the latter may not be a realistic target for obvious reasons, lowering overall CPU usage is always a good thing. Also helps with flickering during the installation process quite a bit too. E.g. a test installation on a single-core VM goes down from 47:35 min w/o the patch to 2:26 min w/ the patch, a ~94%(!) decrease in time. Signed-off-by: Christoph Heiss --- proxmox-tui-installer/src/main.rs | 3 -- .../src/views/install_progress.rs | 32 +++ 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs index 2462a58..4fb7afd 100644 --- a/proxmox-tui-installer/src/main.rs +++ b/proxmox-tui-installer/src/main.rs @@ -664,9 +664,6 @@ fn summary_dialog(siv: Cursive) -> InstallerView { } fn install_progress_dialog(siv: Cursive) -> InstallerView { -// Ensure the screen is updated independently of keyboard events and such -siv.set_autorefresh(true); - let state = siv.user_data::().cloned().unwrap(); InstallerView::with_raw(, InstallProgressView::new(siv)) } diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs index 71af484..6453426 100644 --- a/proxmox-tui-installer/src/views/install_progress.rs +++ b/proxmox-tui-installer/src/views/install_progress.rs @@ -1,7 +1,7 @@ use cursive::{ utils::Counter, view::{Nameable, Resizable, ViewWrapper}, -views::{Dialog, DummyView, LinearLayout, PaddedView, ProgressBar, TextContent, TextView}, +views::{Dialog, DummyView, LinearLayout, PaddedView, ProgressBar, TextView}, CbSink, Cursive, }; use serde::Deserialize; @@ -21,15 +21,15 @@ pub struct InstallProgressView { } impl InstallProgressView { +const PROGRESS_TEXT_VIEW_ID: = "progress-text"; + pub fn new(siv: Cursive) -> Self { let cb_sink = siv.cb_sink().clone(); let state = siv.user_data::().unwrap(); -let progress_text = TextContent::new("starting the installation .."); let progress_task = { -let progress_text = progress_text.clone(); let state = state.clone(); -move |counter: Counter| Self::progress_task(counter, cb_sink, state, progress_text) +move |counter: Counter| Self::progress_task(counter, cb_sink, state) }; let progress_bar = ProgressBar::new().with_task(progress_task).full_width(); @@ -41,7 +41,11 @@ impl InstallProgressView { LinearLayout::vertical() .child(PaddedView::lrtb(1, 1, 0, 0, progress_bar)) .child(DummyView) -.child(TextView::new_with_content(progress_text).center()) +.child( +TextView::new("starting the installation ..") +.center() +.with_name(Self::PROGRESS_TEXT_VIEW_ID), +) .child(PaddedView::lrtb( 1, 1, @@ -54,12 +58,7 @@ impl InstallProgressView { Self { view } } -fn progress_task( -counter: Counter, -cb_sink: CbSink, -state: InstallerState, -progress_text: TextContent, -) { +fn progress_task(counter: Counter, cb_sink: CbSink, state: InstallerState) { let mut child = match spawn_low_level_installer(state.in_test_mode) { Ok(child) => child, Err(err) => { @@ -129,13 +128,18 @@ impl InstallProgressView { }), UiMessage::Progress { ratio, text } => { counter.set((ratio * 100.).floor() as usize); -progress_text.set_content(text); -Ok(()) +cb_sink.send(Box::new(move |siv| { +siv.call_on_name(Self::PROGRESS_TEXT_VIEW_ID, |v: TextView| { +v.set_content(text); +}); +})) } UiMessage::Finished { state, message } => { counter.set(100); -progress_text.set_content(message.to_owned()); cb_sink.send(Box::new(move |siv| { +siv.call_on_name(Self::PROGRESS_TEXT_VIEW_ID, |v: TextView| { +v.set_content(); +}); Self::prepare_for_reboot(siv, state == "ok", ); })) } -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] auto-installer: support UTC as timezone
Reported-by: Fiona Ebner Signed-off-by: Christoph Heiss --- proxmox-auto-installer/src/utils.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs index 02c27d8..1912e51 100644 --- a/proxmox-auto-installer/src/utils.rs +++ b/proxmox-auto-installer/src/utils.rs @@ -280,13 +280,16 @@ pub fn verify_locale_settings(answer: , locales: ) -> Result<( if !locales.kmap.keys().any(|i| i == ) { bail!("keyboard layout '{}' is not valid", ); } + if !locales .cczones .iter() .any(|(_, zones)| zones.contains()) +&& answer.global.timezone != "UTC" { bail!("timezone '{}' is not valid", ); } + Ok(()) } -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH docs-common 01/13] installation-media: move page from pve-docs here
Thanks for the review! On Fri, Apr 19, 2024 at 12:51:07PM +0200, Aaron Lauterer wrote: > > > On 2024-04-19 11:05, Christoph Heiss wrote: > > Small adaptions were necessary; mostly a s/{pve}/{product}/g and > > replacing the ISO URL with the {iso-url} variable. > > except there are still plenty of `{pve}`s in there? Oh right, that is a leftover from splitting the patch into to separate commits. I'll remove that with the next revision. Sorry for the confusion. > > another thing looking at this patch, how do we handle product specifics? > having a ton of variables that are set according to the product, might be > cumbersome. > > Most likely something like `ifdef:product-pve` and so forth would be useful. Depending on the amount of specifics, that's were splitting sections out into partials (into proxmox-docs-common) and then including them into the main page (in the product-specific docs) come into play. E.g. the installer page is a good example where I tried to apply this pattern. Some things like the installer flow and advanced options are sharable, there are still some sections that are specific to e.g. PVE. As for e.g. the `{pve}` macro, there is now simply a `{product}` (and `{product-short}` macro, which already handles most other, trivial differences. But overall, that is definitely a point to discuss further and improve upon incrementally as pages/sections are moved and pain points are discovered, IMO. > > some situations I spotted where we would probably need it in this patched > marked below: All of these should be fixed up/adapted in the next patch in the series. I've split them into two for review sake, where the first is a 1:1 copy and the next patch then adapts it. [..] > > + > > +ifdef::wiki[] > > +Boot your Server from the USB Flash Drive > > +~ > > + > > +Connect the USB flash drive to your server and make sure that booting from > > USB > > +is enabled (check your servers firmware settings). Then follow the steps > > in the > > +xref:chapter_installation[installation wizard]. > > Aligning chapter references will also be some work, especially if we want to > keep old direct links still working. That's a very good point, thanks for noticing! I'll definitely keep note of that, but probably would deal with that as we come to that. > > > + > > +endif::wiki[] > > -- > > 2.44.0 > > > > > > > > ___ > > pve-devel mailing list > > pve-devel@lists.proxmox.com > > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC PATCH docs 12/13] installation: use new 'installation-flow' partial from common docs
Signed-off-by: Christoph Heiss --- pve-installation.adoc | 172 +- 1 file changed, 1 insertion(+), 171 deletions(-) diff --git a/pve-installation.adoc b/pve-installation.adoc index f492ad0..2f4642e 100644 --- a/pve-installation.adoc +++ b/pve-installation.adoc @@ -52,177 +52,7 @@ The installer ISO image includes the following: * Web-based management interface -NOTE: All existing data on the selected drives will be removed during the -installation process. The installer does not add boot menu entries for other -operating systems. - -Please insert the xref:installation_prepare_media[prepared installation media] -(for example, USB flash drive or CD-ROM) and boot from it. - -TIP: Make sure that booting from the installation medium (for example, USB) is -enabled in your server's firmware settings. Secure boot needs to be disabled -when booting an installer prior to {pve} version 8.1. - -[thumbnail="screenshot/pve-grub-menu.png"] - -After choosing the correct entry (for example, 'Boot from USB') the {pve} menu -will be displayed, and one of the following options can be selected: - -Install {pve} (Graphical):: - -Starts the normal installation. - -TIP: It's possible to use the installation wizard with a keyboard only. Buttons -can be clicked by pressing the `ALT` key combined with the underlined character -from the respective button. For example, `ALT + N` to press a `Next` button. - -Install {pve} (Terminal UI):: - -Starts the terminal-mode installation wizard. It provides the same overall -installation experience as the graphical installer, but has generally better -compatibility with very old and very new hardware. - -Install {pve} (Terminal UI, Serial Console):: - -Starts the terminal-mode installation wizard, additionally setting up the Linux -kernel to use the (first) serial port of the machine for in- and output. This -can be used if the machine is completely headless and only has a serial console -available. - -[thumbnail="screenshot/pve-tui-installer.png"] - -Both modes use the same code base for the actual installation process to -benefit from more than a decade of bug fixes and ensure feature parity. - -TIP: The 'Terminal UI' option can be used in case the graphical installer does -not work correctly, due to e.g. driver issues. See also -xref:nomodeset_kernel_param[adding the `nomodeset` kernel parameter]. - -Advanced Options: Install {pve} (Graphical, Debug Mode):: - -Starts the installation in debug mode. A console will be opened at several -installation steps. This helps to debug the situation if something goes wrong. -To exit a debug console, press `CTRL-D`. This option can be used to boot a live -system with all basic tools available. You can use it, for example, to -xref:chapter_zfs[repair a degraded ZFS 'rpool'] or fix the -xref:sysboot[bootloader] for an existing {pve} setup. - -Advanced Options: Install {pve} (Terminal UI, Debug Mode):: - -Same as the graphical debug mode, but preparing the system to run the -terminal-based installer instead. - -Advanced Options: Install {pve} (Serial Console Debug Mode):: - -Same the terminal-based debug mode, but additionally sets up the Linux kernel to -use the (first) serial port of the machine for in- and output. - -Advanced Options: Rescue Boot:: - -With this option you can boot an existing installation. It searches all attached -hard disks. If it finds an existing installation, it boots directly into that -disk using the Linux kernel from the ISO. This can be useful if there are -problems with the bootloader (GRUB/`systemd-boot`) or the BIOS/UEFI is unable to -read the boot block from the disk. - -Advanced Options: Test Memory (memtest86+):: - -Runs `memtest86+`. This is useful to check if the memory is functional and free -of errors. Secure Boot must be turned off in the UEFI firmware setup utility to -run this option. - -You normally select *Install {pve} (Graphical)* to start the installation. - -[thumbnail="screenshot/pve-select-target-disk.png"] - -The first step is to read our EULA (End User License Agreement). Following this, -you can select the target hard disk(s) for the installation. - -CAUTION: By default, the whole server is used and all existing data is removed. -Make sure there is no important data on the server before proceeding with the -installation. - -The `Options` button lets you select the target file system, which -defaults to `ext4`. The installer uses LVM if you select -`ext4` or `xfs` as a file system, and offers additional options to -restrict LVM space (see xref:advanced_lvm_options[below]). - -{pve} can also be installed on ZFS. As ZFS offers several software RAID levels, -this is an option for systems that don't have a hardware RAID controller. The -target disks must be selected in the `Options` dialog. More ZFS specific -settings can be changed under xref:advanced_zfs_options[`Advanced Options`]. - -WARNING: ZFS on top of any hardware RAID i
[pve-devel] [RFC PATCH docs 13/13] installation: use new 'advanced-installation' partial from common docs
Signed-off-by: Christoph Heiss --- pve-installation.adoc | 126 +- 1 file changed, 1 insertion(+), 125 deletions(-) diff --git a/pve-installation.adoc b/pve-installation.adoc index 2f4642e..0a0dc76 100644 --- a/pve-installation.adoc +++ b/pve-installation.adoc @@ -78,131 +78,7 @@ web interface for further configuration. . Check your xref:chapter_pve_firewall[Firewall settings]. -[[advanced_lvm_options]] -Advanced LVM Configuration Options -~~ - -The installer creates a Volume Group (VG) called `pve`, and additional Logical -Volumes (LVs) called `root`, `data`, and `swap`, if `ext4` or `xfs` is used. To -control the size of these volumes use: - -`hdsize`:: - -Defines the total hard disk size to be used. This way you can reserve free space -on the hard disk for further partitioning (for example for an additional PV and -VG on the same hard disk that can be used for LVM storage). - -`swapsize`:: - -Defines the size of the `swap` volume. The default is the size of the installed -memory, minimum 4 GB and maximum 8 GB. The resulting value cannot be greater -than `hdsize/8`. -+ -NOTE: If set to `0`, no `swap` volume will be created. - -`maxroot`:: - -Defines the maximum size of the `root` volume, which stores the operation -system. The maximum limit of the `root` volume size is `hdsize/4`. - -`maxvz`:: - -Defines the maximum size of the `data` volume. The actual size of the `data` -volume is: -+ -`datasize = hdsize - rootsize - swapsize - minfree` -+ -Where `datasize` cannot be bigger than `maxvz`. -+ -NOTE: In case of LVM thin, the `data` pool will only be created if `datasize` is -bigger than 4GB. -+ -NOTE: If set to `0`, no `data` volume will be created and the storage -configuration will be adapted accordingly. - -`minfree`:: - -Defines the amount of free space that should be left in the LVM volume group -`pve`. With more than 128GB storage available, the default is 16GB, otherwise -`hdsize/8` will be used. -+ -NOTE: LVM requires free space in the VG for snapshot creation (not required for -lvmthin snapshots). - -[[advanced_zfs_options]] -Advanced ZFS Configuration Options -~~ -The installer creates the ZFS pool `rpool`, if ZFS is used. No swap space is -created but you can reserve some unpartitioned space on the install disks for -swap. You can also create a swap zvol after the installation, although this can -lead to problems (see xref:zfs_swap[ZFS swap notes]). - -`ashift`:: - -Defines the `ashift` value for the created pool. The `ashift` needs to be set at -least to the sector-size of the underlying disks (2 to the power of `ashift` is -the sector-size), or any disk which might be put in the pool (for example the -replacement of a defective disk). - -`compress`:: - -Defines whether compression is enabled for `rpool`. - -`checksum`:: - -Defines which checksumming algorithm should be used for `rpool`. - -`copies`:: - -Defines the `copies` parameter for `rpool`. Check the `zfs(8)` manpage for the -semantics, and why this does not replace redundancy on disk-level. - -`ARC max size`:: - -Defines the maximum size the ARC can grow to and thus limits the amount of -memory ZFS will use. See also the section on -xref:sysadmin_zfs_limit_memory_usage[how to limit ZFS memory usage] for more -details. - -`hdsize`:: - -Defines the total hard disk size to be used. This is useful to save free space -on the hard disk(s) for further partitioning (for example to create a -swap-partition). `hdsize` is only honored for bootable disks, that is only the -first disk or mirror for RAID0, RAID1 or RAID10, and all disks in RAID-Z[123]. - - -ZFS Performance Tips - - -ZFS works best with a lot of memory. If you intend to use ZFS make sure to have -enough RAM available for it. A good calculation is 4GB plus 1GB RAM for each TB -RAW disk space. - -ZFS can use a dedicated drive as write cache, called the ZFS Intent Log (ZIL). -Use a fast drive (SSD) for it. It can be added after installation with the -following command: - - -# zpool add log - - -[[nomodeset_kernel_param]] -Adding the `nomodeset` Kernel Parameter -~ - -Problems may arise on very old or very new hardware due to graphics drivers. If -the installation hangs during boot, you can try adding the `nomodeset` -parameter. This prevents the Linux kernel from loading any graphics drivers and -forces it to continue using the BIOS/UEFI-provided framebuffer. - -On the {pve} bootloader menu, navigate to 'Install {pve} (Terminal UI)' and -press `e` to edit the entry. Using the arrow keys, navigate to the line starting -with `linux`, move the cursor to the end of that line and add the -parameter `nomodeset`, separated by a space from the pre-existing last -parameter. - -Then press `Ctrl-X` or `F10` to boot the configuration. +include::proxmox-docs-common/partials/advanced-installation.adoc[] ifndef
[pve-devel] [RFC PATCH docs 07/13] gitmodules: add proxmox-docs-common
Signed-off-by: Christoph Heiss --- .gitmodules | 3 +++ proxmox-docs-common | 1 + 2 files changed, 4 insertions(+) create mode 100644 .gitmodules create mode 16 proxmox-docs-common diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 000..eff8adf --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "proxmox-docs-common"] + path = proxmox-docs-common + url = ../proxmox-docs-common diff --git a/proxmox-docs-common b/proxmox-docs-common new file mode 16 index 000..a042676 --- /dev/null +++ b/proxmox-docs-common @@ -0,0 +1 @@ +Subproject commit a042676729ea0b87a03844eb4c468028bb45f55d -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC PATCH docs-common 06/13] partials: advanced-installation: adapt from pve-docs
Signed-off-by: Christoph Heiss --- partials/advanced-installation.adoc | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/partials/advanced-installation.adoc b/partials/advanced-installation.adoc index ed2709f..271533b 100644 --- a/partials/advanced-installation.adoc +++ b/partials/advanced-installation.adoc @@ -2,9 +2,9 @@ Advanced LVM Configuration Options ~~ -The installer creates a Volume Group (VG) called `pve`, and additional Logical -Volumes (LVs) called `root`, `data`, and `swap`, if `ext4` or `xfs` is used. To -control the size of these volumes use: +The installer creates a Volume Group (VG) called +{product-short}+, and +additional Logical Volumes (LVs) called `root`, `data`, and `swap`, if `ext4` or +`xfs` is used. To control the size of these volumes use: `hdsize`:: @@ -43,8 +43,8 @@ configuration will be adapted accordingly. `minfree`:: Defines the amount of free space that should be left in the LVM volume group -`pve`. With more than 128GB storage available, the default is 16GB, otherwise -`hdsize/8` will be used. ++{product-short}+. With more than 128GB storage available, the default is 16GB, +otherwise `hdsize/8` will be used. + NOTE: LVM requires free space in the VG for snapshot creation (not required for lvmthin snapshots). @@ -109,16 +109,16 @@ following command: [[nomodeset_kernel_param]] Adding the `nomodeset` Kernel Parameter -~ +~~~ Problems may arise on very old or very new hardware due to graphics drivers. If the installation hangs during boot, you can try adding the `nomodeset` parameter. This prevents the Linux kernel from loading any graphics drivers and forces it to continue using the BIOS/UEFI-provided framebuffer. -On the {pve} bootloader menu, navigate to 'Install {pve} (Terminal UI)' and -press `e` to edit the entry. Using the arrow keys, navigate to the line starting -with `linux`, move the cursor to the end of that line and add the +On the {product} bootloader menu, navigate to 'Install {product} (Terminal UI)' +and press `e` to edit the entry. Using the arrow keys, navigate to the line +starting with `linux`, move the cursor to the end of that line and add the parameter `nomodeset`, separated by a space from the pre-existing last parameter. -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC PATCH docs 10/13] asciidoc: conf: add iso-url variable
Signed-off-by: Christoph Heiss --- asciidoc/asciidoc-pve.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/asciidoc/asciidoc-pve.conf b/asciidoc/asciidoc-pve.conf index 47139b8..faa190a 100644 --- a/asciidoc/asciidoc-pve.conf +++ b/asciidoc/asciidoc-pve.conf @@ -6,6 +6,7 @@ pve=Proxmox VE product=Proxmox VE product-short=pve pricing-url=https://proxmox.com/en/proxmox-virtual-environment/pricing +iso-url=https://proxmox.com/downloads/proxmox-virtual-environment website=https://www.proxmox.com/ forum-url=https://forum.proxmox.com/ forum=https://forum.proxmox.com/[Proxmox VE Community Forum] -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC PATCH docs-common 04/13] partials: installation-flow: adapt from pve-docs
A trivial s/{pve}/{product}/g again and a additional s/screenshot\/pve-/screenshot\//g to fix the screenshot paths. Signed-off-by: Christoph Heiss --- partials/installation-flow.adoc | 47 + 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/partials/installation-flow.adoc b/partials/installation-flow.adoc index b796676..9a7d1b3 100644 --- a/partials/installation-flow.adoc +++ b/partials/installation-flow.adoc @@ -7,14 +7,14 @@ Please insert the xref:installation_prepare_media[prepared installation media] TIP: Make sure that booting from the installation medium (for example, USB) is enabled in your server's firmware settings. Secure boot needs to be disabled -when booting an installer prior to {pve} version 8.1. +when booting an installer prior to {product} version 8.1. -[thumbnail="screenshot/pve-grub-menu.png"] +[thumbnail="screenshot/grub-menu.png"] -After choosing the correct entry (for example, 'Boot from USB') the {pve} menu -will be displayed, and one of the following options can be selected: +After choosing the correct entry (for example, 'Boot from USB') the {product} +menu will be displayed, and one of the following options can be selected: -Install {pve} (Graphical):: +Install {product} (Graphical):: Starts the normal installation. @@ -22,20 +22,20 @@ TIP: It's possible to use the installation wizard with a keyboard only. Buttons can be clicked by pressing the `ALT` key combined with the underlined character from the respective button. For example, `ALT + N` to press a `Next` button. -Install {pve} (Terminal UI):: +Install {product} (Terminal UI):: Starts the terminal-mode installation wizard. It provides the same overall installation experience as the graphical installer, but has generally better compatibility with very old and very new hardware. -Install {pve} (Terminal UI, Serial Console):: +Install {product} (Terminal UI, Serial Console):: Starts the terminal-mode installation wizard, additionally setting up the Linux kernel to use the (first) serial port of the machine for in- and output. This can be used if the machine is completely headless and only has a serial console available. -[thumbnail="screenshot/pve-tui-installer.png"] +[thumbnail="screenshot/tui-installer.png"] Both modes use the same code base for the actual installation process to benefit from more than a decade of bug fixes and ensure feature parity. @@ -44,21 +44,21 @@ TIP: The 'Terminal UI' option can be used in case the graphical installer does not work correctly, due to e.g. driver issues. See also xref:nomodeset_kernel_param[adding the `nomodeset` kernel parameter]. -Advanced Options: Install {pve} (Graphical, Debug Mode):: +Advanced Options: Install {product} (Graphical, Debug Mode):: Starts the installation in debug mode. A console will be opened at several installation steps. This helps to debug the situation if something goes wrong. To exit a debug console, press `CTRL-D`. This option can be used to boot a live system with all basic tools available. You can use it, for example, to xref:chapter_zfs[repair a degraded ZFS 'rpool'] or fix the -xref:sysboot[bootloader] for an existing {pve} setup. +xref:sysboot[bootloader] for an existing {product} setup. -Advanced Options: Install {pve} (Terminal UI, Debug Mode):: +Advanced Options: Install {product} (Terminal UI, Debug Mode):: Same as the graphical debug mode, but preparing the system to run the terminal-based installer instead. -Advanced Options: Install {pve} (Serial Console Debug Mode):: +Advanced Options: Install {product} (Serial Console Debug Mode):: Same the terminal-based debug mode, but additionally sets up the Linux kernel to use the (first) serial port of the machine for in- and output. @@ -77,9 +77,9 @@ Runs `memtest86+`. This is useful to check if the memory is functional and free of errors. Secure Boot must be turned off in the UEFI firmware setup utility to run this option. -You normally select *Install {pve} (Graphical)* to start the installation. +You normally select *Install {product} (Graphical)* to start the installation. -[thumbnail="screenshot/pve-select-target-disk.png"] +[thumbnail="screenshot/select-target-disk.png"] The first step is to read our EULA (End User License Agreement). Following this, you can select the target hard disk(s) for the installation. @@ -93,15 +93,16 @@ defaults to `ext4`. The installer uses LVM if you select `ext4` or `xfs` as a file system, and offers additional options to restrict LVM space (see xref:advanced_lvm_options[below]). -{pve} can also be installed on ZFS. As ZFS offers several software RAID levels, -this is an option for systems that don't have a hardware RAID controller. The -target disks must be selected in the `Options` dialog. More ZFS specific -settings can be changed under xref:advanced_zfs_options[`Advanced Options`]. +{product} can a
[pve-devel] [RFC PATCH docs 11/13] installation-media: move to common docs
Signed-off-by: Christoph Heiss --- pve-installation-media.adoc | 132 pve-installation.adoc | 2 +- 2 files changed, 1 insertion(+), 133 deletions(-) delete mode 100644 pve-installation-media.adoc diff --git a/pve-installation-media.adoc b/pve-installation-media.adoc deleted file mode 100644 index a1c9402..000 --- a/pve-installation-media.adoc +++ /dev/null @@ -1,132 +0,0 @@ -[[installation_prepare_media]] -Prepare Installation Media --- -ifdef::wiki[] -:pve-toplevel: -endif::wiki[] - -Download the installer ISO image from: {website}en/downloads/proxmox-virtual-environment/iso - -The {pve} installation media is a hybrid ISO image. It works in two ways: - -* An ISO image file ready to burn to a CD or DVD. - -* A raw sector (IMG) image file ready to copy to a USB flash drive (USB stick). - -Using a USB flash drive to install {pve} is the recommended way because it is -the faster option. - -Prepare a USB Flash Drive as Installation Medium - - -The flash drive needs to have at least 1 GB of storage available. - -NOTE: Do not use UNetbootin. It does not work with the {pve} installation image. - -IMPORTANT: Make sure that the USB flash drive is not mounted and does not -contain any important data. - - -Instructions for GNU/Linux -~~ - -On Unix-like operating system use the `dd` command to copy the ISO image to the -USB flash drive. First find the correct device name of the USB flash drive (see -below). Then run the `dd` command. - - -# dd bs=1M conv=fdatasync if=./proxmox-ve_*.iso of=/dev/XYZ - - -NOTE: Be sure to replace /dev/XYZ with the correct device name and adapt the -input filename ('if') path. - -CAUTION: Be very careful, and do not overwrite the wrong disk! - - -Find the Correct USB Device Name - -There are two ways to find out the name of the USB flash drive. The first one is -to compare the last lines of the `dmesg` command output before and after -plugging in the flash drive. The second way is to compare the output of the -`lsblk` command. Open a terminal and run: - - -# lsblk - - -Then plug in your USB flash drive and run the command again: - - -# lsblk - - -A new device will appear. This is the one you want to use. To be on the extra -safe side check if the reported size matches your USB flash drive. - - -Instructions for macOS -~~ - -Open the terminal (query Terminal in Spotlight). - -Convert the `.iso` file to `.dmg` format using the convert option of `hdiutil`, -for example: - - -# hdiutil convert proxmox-ve_*.iso -format UDRW -o proxmox-ve_*.dmg - - -TIP: macOS tends to automatically add '.dmg' to the output file name. - -To get the current list of devices run the command: - - -# diskutil list - - -Now insert the USB flash drive and run this command again to determine which -device node has been assigned to it. (e.g., /dev/diskX). - - -# diskutil list -# diskutil unmountDisk /dev/diskX - - -NOTE: replace X with the disk number from the last command. - - -# sudo dd if=proxmox-ve_*.dmg bs=1M of=/dev/rdiskX - - -NOTE: 'rdiskX', instead of 'diskX', in the last command is intended. It will -increase the write speed. - -Instructions for Windows - - -Using Etcher - - -Etcher works out of the box. Download Etcher from https://etcher.io. It will -guide you through the process of selecting the ISO and your USB flash drive. - -Using Rufus -^^^ - -Rufus is a more lightweight alternative, but you need to use the *DD mode* to -make it work. Download Rufus from https://rufus.ie/. Either install it or use -the portable version. Select the destination drive and the {pve} ISO file. - -IMPORTANT: Once you 'Start' you have to click 'No' on the dialog asking to -download a different version of GRUB. In the next dialog select the 'DD' mode. - -ifdef::wiki[] -Boot your Server from the USB Flash Drive -~ - -Connect the USB flash drive to your server and make sure that booting from USB -is enabled (check your servers firmware settings). Then follow the steps in the -xref:chapter_installation[installation wizard]. - -endif::wiki[] diff --git a/pve-installation.adoc b/pve-installation.adoc index 274d9ad..f492ad0 100644 --- a/pve-installation.adoc +++ b/pve-installation.adoc @@ -29,7 +29,7 @@ ifndef::wiki[] include::pve-system-requirements.adoc[] -include::pve-installation-media.adoc[] +include::proxmox-docs-common/installation-media.adoc[] endif::wiki[] -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC PATCH docs 09/13] images: strip `pve-` prefix from screenshots used in common docs
Signed-off-by: Christoph Heiss --- .../{pve-grub-menu.png => grub-menu.png} | Bin .../{pve-grub-menu.ppm => grub-menu.ppm} | Bin ...ve-install-summary.png => install-summary.png} | Bin ...ve-install-summary.ppm => install-summary.ppm} | Bin .../{pve-installation.png => installation.png}| Bin .../{pve-installation.ppm => installation.ppm}| Bin ...ve-select-location.png => select-location.png} | Bin ...ve-select-location.ppm => select-location.ppm} | Bin ...ect-target-disk.png => select-target-disk.png} | Bin ...ect-target-disk.ppm => select-target-disk.ppm} | Bin .../{pve-set-password.png => set-password.png}| Bin .../{pve-set-password.ppm => set-password.ppm}| Bin .../{pve-setup-network.png => setup-network.png} | Bin .../{pve-setup-network.ppm => setup-network.ppm} | Bin .../{pve-tui-installer.png => tui-installer.png} | Bin .../{pve-tui-installer.ppm => tui-installer.ppm} | Bin png-verify.pl | 14 -- 17 files changed, 8 insertions(+), 6 deletions(-) rename images/screenshot/{pve-grub-menu.png => grub-menu.png} (100%) rename images/screenshot/{pve-grub-menu.ppm => grub-menu.ppm} (100%) rename images/screenshot/{pve-install-summary.png => install-summary.png} (100%) rename images/screenshot/{pve-install-summary.ppm => install-summary.ppm} (100%) rename images/screenshot/{pve-installation.png => installation.png} (100%) rename images/screenshot/{pve-installation.ppm => installation.ppm} (100%) rename images/screenshot/{pve-select-location.png => select-location.png} (100%) rename images/screenshot/{pve-select-location.ppm => select-location.ppm} (100%) rename images/screenshot/{pve-select-target-disk.png => select-target-disk.png} (100%) rename images/screenshot/{pve-select-target-disk.ppm => select-target-disk.ppm} (100%) rename images/screenshot/{pve-set-password.png => set-password.png} (100%) rename images/screenshot/{pve-set-password.ppm => set-password.ppm} (100%) rename images/screenshot/{pve-setup-network.png => setup-network.png} (100%) rename images/screenshot/{pve-setup-network.ppm => setup-network.ppm} (100%) rename images/screenshot/{pve-tui-installer.png => tui-installer.png} (100%) rename images/screenshot/{pve-tui-installer.ppm => tui-installer.ppm} (100%) diff --git a/images/screenshot/pve-grub-menu.png b/images/screenshot/grub-menu.png similarity index 100% rename from images/screenshot/pve-grub-menu.png rename to images/screenshot/grub-menu.png diff --git a/images/screenshot/pve-grub-menu.ppm b/images/screenshot/grub-menu.ppm similarity index 100% rename from images/screenshot/pve-grub-menu.ppm rename to images/screenshot/grub-menu.ppm diff --git a/images/screenshot/pve-install-summary.png b/images/screenshot/install-summary.png similarity index 100% rename from images/screenshot/pve-install-summary.png rename to images/screenshot/install-summary.png diff --git a/images/screenshot/pve-install-summary.ppm b/images/screenshot/install-summary.ppm similarity index 100% rename from images/screenshot/pve-install-summary.ppm rename to images/screenshot/install-summary.ppm diff --git a/images/screenshot/pve-installation.png b/images/screenshot/installation.png similarity index 100% rename from images/screenshot/pve-installation.png rename to images/screenshot/installation.png diff --git a/images/screenshot/pve-installation.ppm b/images/screenshot/installation.ppm similarity index 100% rename from images/screenshot/pve-installation.ppm rename to images/screenshot/installation.ppm diff --git a/images/screenshot/pve-select-location.png b/images/screenshot/select-location.png similarity index 100% rename from images/screenshot/pve-select-location.png rename to images/screenshot/select-location.png diff --git a/images/screenshot/pve-select-location.ppm b/images/screenshot/select-location.ppm similarity index 100% rename from images/screenshot/pve-select-location.ppm rename to images/screenshot/select-location.ppm diff --git a/images/screenshot/pve-select-target-disk.png b/images/screenshot/select-target-disk.png similarity index 100% rename from images/screenshot/pve-select-target-disk.png rename to images/screenshot/select-target-disk.png diff --git a/images/screenshot/pve-select-target-disk.ppm b/images/screenshot/select-target-disk.ppm similarity index 100% rename from images/screenshot/pve-select-target-disk.ppm rename to images/screenshot/select-target-disk.ppm diff --git a/images/screenshot/pve-set-password.png b/images/screenshot/set-password.png similarity index 100% rename from images/screenshot/pve-set-password.png rename to images/screenshot/set-password.png diff --git a/images/screenshot/pve-set-password.ppm b/images/screenshot/set-password.ppm similarity index 100% rename from images/screenshot/pve-set-password.ppm rename to images/screenshot/set-password
[pve-devel] [RFC PATCH docs-common 05/13] partials: add advanced installation hints from pve-docs
Signed-off-by: Christoph Heiss --- partials/advanced-installation.adoc | 125 1 file changed, 125 insertions(+) create mode 100644 partials/advanced-installation.adoc diff --git a/partials/advanced-installation.adoc b/partials/advanced-installation.adoc new file mode 100644 index 000..ed2709f --- /dev/null +++ b/partials/advanced-installation.adoc @@ -0,0 +1,125 @@ +[[advanced_lvm_options]] +Advanced LVM Configuration Options +~~ + +The installer creates a Volume Group (VG) called `pve`, and additional Logical +Volumes (LVs) called `root`, `data`, and `swap`, if `ext4` or `xfs` is used. To +control the size of these volumes use: + +`hdsize`:: + +Defines the total hard disk size to be used. This way you can reserve free space +on the hard disk for further partitioning (for example for an additional PV and +VG on the same hard disk that can be used for LVM storage). + +`swapsize`:: + +Defines the size of the `swap` volume. The default is the size of the installed +memory, minimum 4 GB and maximum 8 GB. The resulting value cannot be greater +than `hdsize/8`. ++ +NOTE: If set to `0`, no `swap` volume will be created. + +`maxroot`:: + +Defines the maximum size of the `root` volume, which stores the operation +system. The maximum limit of the `root` volume size is `hdsize/4`. + +`maxvz`:: + +Defines the maximum size of the `data` volume. The actual size of the `data` +volume is: ++ +`datasize = hdsize - rootsize - swapsize - minfree` ++ +Where `datasize` cannot be bigger than `maxvz`. ++ +NOTE: In case of LVM thin, the `data` pool will only be created if `datasize` is +bigger than 4GB. ++ +NOTE: If set to `0`, no `data` volume will be created and the storage +configuration will be adapted accordingly. + +`minfree`:: + +Defines the amount of free space that should be left in the LVM volume group +`pve`. With more than 128GB storage available, the default is 16GB, otherwise +`hdsize/8` will be used. ++ +NOTE: LVM requires free space in the VG for snapshot creation (not required for +lvmthin snapshots). + +[[advanced_zfs_options]] +Advanced ZFS Configuration Options +~~ +The installer creates the ZFS pool `rpool`, if ZFS is used. No swap space is +created but you can reserve some unpartitioned space on the install disks for +swap. You can also create a swap zvol after the installation, although this can +lead to problems (see xref:zfs_swap[ZFS swap notes]). + +`ashift`:: + +Defines the `ashift` value for the created pool. The `ashift` needs to be set at +least to the sector-size of the underlying disks (2 to the power of `ashift` is +the sector-size), or any disk which might be put in the pool (for example the +replacement of a defective disk). + +`compress`:: + +Defines whether compression is enabled for `rpool`. + +`checksum`:: + +Defines which checksumming algorithm should be used for `rpool`. + +`copies`:: + +Defines the `copies` parameter for `rpool`. Check the `zfs(8)` manpage for the +semantics, and why this does not replace redundancy on disk-level. + +`ARC max size`:: + +Defines the maximum size the ARC can grow to and thus limits the amount of +memory ZFS will use. See also the section on +xref:sysadmin_zfs_limit_memory_usage[how to limit ZFS memory usage] for more +details. + +`hdsize`:: + +Defines the total hard disk size to be used. This is useful to save free space +on the hard disk(s) for further partitioning (for example to create a +swap-partition). `hdsize` is only honored for bootable disks, that is only the +first disk or mirror for RAID0, RAID1 or RAID10, and all disks in RAID-Z[123]. + + +ZFS Performance Tips + + +ZFS works best with a lot of memory. If you intend to use ZFS make sure to have +enough RAM available for it. A good calculation is 4GB plus 1GB RAM for each TB +RAW disk space. + +ZFS can use a dedicated drive as write cache, called the ZFS Intent Log (ZIL). +Use a fast drive (SSD) for it. It can be added after installation with the +following command: + + +# zpool add log + + +[[nomodeset_kernel_param]] +Adding the `nomodeset` Kernel Parameter +~ + +Problems may arise on very old or very new hardware due to graphics drivers. If +the installation hangs during boot, you can try adding the `nomodeset` +parameter. This prevents the Linux kernel from loading any graphics drivers and +forces it to continue using the BIOS/UEFI-provided framebuffer. + +On the {pve} bootloader menu, navigate to 'Install {pve} (Terminal UI)' and +press `e` to edit the entry. Using the arrow keys, navigate to the line starting +with `linux`, move the cursor to the end of that line and add the +parameter `nomodeset`, separated by a space from the pre-existing last +parameter. + +Then press `Ctrl-X` or `F10` to boot the configuration. -- 2.44.0 ___ pve-devel mailing list pve-devel
[pve-devel] [RFC PATCH docs 08/13] scan-adoc-refs: enable building pages from proxmox-docs-common/ subdir
Signed-off-by: Christoph Heiss --- Makefile | 6 -- asciidoc/asciidoc-pve.conf | 2 ++ pve-doc-generator.mk.in| 6 ++ scan-adoc-refs | 25 - 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 801a2a3..5b14e3d 100644 --- a/Makefile +++ b/Makefile @@ -26,9 +26,11 @@ all: index.html verify-images: for i in ./images/screenshot/*.png; do ./png-verify.pl $$i; done -ADOC_SOURCES_GUESS=$(filter-out %-synopsis.adoc %-opts.adoc %-table.adoc, $(wildcard *.adoc)) +ADOC_WILDCARD_SOURCES := *.adoc proxmox-docs-common/*.adoc proxmox-docs-common/partials/*.adoc +ADOC_SOURCES_GUESS=$(filter-out %-synopsis.adoc %-opts.adoc %-table.adoc, $(wildcard $(ADOC_WILDCARD_SOURCES))) + .pve-doc-depends link-refs.json: $(ADOC_SOURCES_GUESS) scan-adoc-refs - ./scan-adoc-refs *.adoc --depends .pve-doc-depends.tmp > link-refs.json.tmp + ./scan-adoc-refs $(ADOC_WILDCARD_SOURCES) --depends .pve-doc-depends.tmp > link-refs.json.tmp @cmp --quiet .pve-doc-depends .pve-doc-depends.tmp || mv .pve-doc-depends.tmp .pve-doc-depends @cmp --quiet link-refs.json link-refs.json.tmp || mv link-refs.json.tmp link-refs.json diff --git a/asciidoc/asciidoc-pve.conf b/asciidoc/asciidoc-pve.conf index 0c28298..47139b8 100644 --- a/asciidoc/asciidoc-pve.conf +++ b/asciidoc/asciidoc-pve.conf @@ -3,6 +3,8 @@ proxmoxGmbh=Proxmox Server Solutions GmbH copyright=Proxmox Server Solutions GmbH pve=Proxmox VE +product=Proxmox VE +product-short=pve pricing-url=https://proxmox.com/en/proxmox-virtual-environment/pricing website=https://www.proxmox.com/ forum-url=https://forum.proxmox.com/ diff --git a/pve-doc-generator.mk.in b/pve-doc-generator.mk.in index 77fd4f6..a733966 100644 --- a/pve-doc-generator.mk.in +++ b/pve-doc-generator.mk.in @@ -71,9 +71,15 @@ endif %-plain.html: %.adoc ${PVE_COMMON_DOC_SOURCES} ${ASCIIDOC_PVE} compile-wiki -o $@ $*.adoc +%-plain.html: proxmox-docs-common/%.adoc ${PVE_COMMON_DOC_SOURCES} + ${ASCIIDOC_PVE} compile-wiki -o $@ proxmox-docs-common/$*.adoc + chapter-%.html: %.adoc ${PVE_COMMON_DOC_SOURCES} ${ASCIIDOC_PVE} compile-chapter -o $@ $*.adoc +chapter-%.html: proxmox-docs-common/%.adoc ${PVE_COMMON_DOC_SOURCES} + ${ASCIIDOC_PVE} compile-chapter -o $@ proxmox-docs-common/$*.adoc + %.1: %.adoc %.1-synopsis.adoc ${PVE_COMMON_DOC_SOURCES} ${ASCIIDOC_PVE} compile-man -o $@ $*.adoc test -z "$${PVE_DOC_INSTANTVIEW}" || man -l $@ diff --git a/scan-adoc-refs b/scan-adoc-refs index 9252701..fd74c20 100755 --- a/scan-adoc-refs +++ b/scan-adoc-refs @@ -5,6 +5,8 @@ use warnings; use Getopt::Long; use IO::File; use JSON; +use Cwd qw(abs_path); +use File::Spec; use Data::Dumper; @@ -19,6 +21,7 @@ my $environments = { wiki => 1, manvolnum => 1, pvelogo => 0, # ignore +pve => 0, }; my $fileinfo = {}; @@ -76,6 +79,12 @@ sub pop_environment { sub register_include { my ($filename, $include_filename, $env_list) = @_; +# get containing directory of the file that includes .. +my $reldir = (File::Spec->splitpath($filename))[1]; +# .. and resolve the included filename relative to that +$include_filename = abs_path(File::Spec->join($reldir, $include_filename)) + if $reldir; + foreach my $e (@$env_list) { $fileinfo->{include}->{$e}->{$filename}->{$include_filename} = 1; } @@ -99,6 +108,7 @@ sub register_title { # fixme: what about other macros? $title =~ s/\{pve\}/Proxmox VE/g; +$title =~ s/\{product\}/Proxmox VE/g; $title =~ s!http://\S+\[(.*?)\]!$1!g; # register document title (onyl once) @@ -269,23 +279,19 @@ foreach my $e (@$start_env) { my $toplevel_hash = $fileinfo->{toplevel}->{$e}; foreach my $fn (sort keys %$toplevel_hash) { my $mansection = $fileinfo->{mansection}->{manvolnum}->{$fn}; + my $realfn = $fn; + $realfn =~ s/^proxmox-docs-common\///; + $realfn =~ s/\.adoc$//; + if ($e eq 'wiki') { - my $realfn = $fn; - $realfn =~ s/\.adoc$//; if (defined($mansection) && ($mansection == 5)) { $realfn .= ".$mansection"; } $realfn = "$realfn-plain.html"; - $fileinfo->{outfile}->{$e}->{$fn} = $realfn; } elsif ($e eq 'manvolnum') { - my $realfn = $fn; - $realfn =~ s/\.adoc$//; die "toplevel file '$fn' is not marked as manual page!" if !$mansection; $realfn .= ".$mansection"; - $fileinfo->{outfile}->{$e}->{$fn} = $realfn; } elsif ($e eq 'default') { - my $realfn = $fn; - $realfn =~ s/\.adoc$//; if (defined($mansection) && ($mansection == 5)) { $realfn .= ".$mansecti
[pve-devel] [RFC PATCH docs-common 03/13] partials: add installation flow from pve-docs
Signed-off-by: Christoph Heiss --- partials/installation-flow.adoc | 170 1 file changed, 170 insertions(+) create mode 100644 partials/installation-flow.adoc diff --git a/partials/installation-flow.adoc b/partials/installation-flow.adoc new file mode 100644 index 000..b796676 --- /dev/null +++ b/partials/installation-flow.adoc @@ -0,0 +1,170 @@ +NOTE: All existing data on the selected drives will be removed during the +installation process. The installer does not add boot menu entries for other +operating systems. + +Please insert the xref:installation_prepare_media[prepared installation media] +(for example, USB flash drive or CD-ROM) and boot from it. + +TIP: Make sure that booting from the installation medium (for example, USB) is +enabled in your server's firmware settings. Secure boot needs to be disabled +when booting an installer prior to {pve} version 8.1. + +[thumbnail="screenshot/pve-grub-menu.png"] + +After choosing the correct entry (for example, 'Boot from USB') the {pve} menu +will be displayed, and one of the following options can be selected: + +Install {pve} (Graphical):: + +Starts the normal installation. + +TIP: It's possible to use the installation wizard with a keyboard only. Buttons +can be clicked by pressing the `ALT` key combined with the underlined character +from the respective button. For example, `ALT + N` to press a `Next` button. + +Install {pve} (Terminal UI):: + +Starts the terminal-mode installation wizard. It provides the same overall +installation experience as the graphical installer, but has generally better +compatibility with very old and very new hardware. + +Install {pve} (Terminal UI, Serial Console):: + +Starts the terminal-mode installation wizard, additionally setting up the Linux +kernel to use the (first) serial port of the machine for in- and output. This +can be used if the machine is completely headless and only has a serial console +available. + +[thumbnail="screenshot/pve-tui-installer.png"] + +Both modes use the same code base for the actual installation process to +benefit from more than a decade of bug fixes and ensure feature parity. + +TIP: The 'Terminal UI' option can be used in case the graphical installer does +not work correctly, due to e.g. driver issues. See also +xref:nomodeset_kernel_param[adding the `nomodeset` kernel parameter]. + +Advanced Options: Install {pve} (Graphical, Debug Mode):: + +Starts the installation in debug mode. A console will be opened at several +installation steps. This helps to debug the situation if something goes wrong. +To exit a debug console, press `CTRL-D`. This option can be used to boot a live +system with all basic tools available. You can use it, for example, to +xref:chapter_zfs[repair a degraded ZFS 'rpool'] or fix the +xref:sysboot[bootloader] for an existing {pve} setup. + +Advanced Options: Install {pve} (Terminal UI, Debug Mode):: + +Same as the graphical debug mode, but preparing the system to run the +terminal-based installer instead. + +Advanced Options: Install {pve} (Serial Console Debug Mode):: + +Same the terminal-based debug mode, but additionally sets up the Linux kernel to +use the (first) serial port of the machine for in- and output. + +Advanced Options: Rescue Boot:: + +With this option you can boot an existing installation. It searches all attached +hard disks. If it finds an existing installation, it boots directly into that +disk using the Linux kernel from the ISO. This can be useful if there are +problems with the bootloader (GRUB/`systemd-boot`) or the BIOS/UEFI is unable to +read the boot block from the disk. + +Advanced Options: Test Memory (memtest86+):: + +Runs `memtest86+`. This is useful to check if the memory is functional and free +of errors. Secure Boot must be turned off in the UEFI firmware setup utility to +run this option. + +You normally select *Install {pve} (Graphical)* to start the installation. + +[thumbnail="screenshot/pve-select-target-disk.png"] + +The first step is to read our EULA (End User License Agreement). Following this, +you can select the target hard disk(s) for the installation. + +CAUTION: By default, the whole server is used and all existing data is removed. +Make sure there is no important data on the server before proceeding with the +installation. + +The `Options` button lets you select the target file system, which +defaults to `ext4`. The installer uses LVM if you select +`ext4` or `xfs` as a file system, and offers additional options to +restrict LVM space (see xref:advanced_lvm_options[below]). + +{pve} can also be installed on ZFS. As ZFS offers several software RAID levels, +this is an option for systems that don't have a hardware RAID controller. The +target disks must be selected in the `Options` dialog. More ZFS specific +settings can be changed under xref:advanced_zfs_options[`Advanced Options`]. + +WARNING: ZFS on top of any hardware RAID is not supported
[pve-devel] [RFC PATCH docs{, -common} 0/13] introduce common documentation base
tl;dr: Introduce a separate repository for shared documentation between all three products. This proposes the introduction of a - aptly named - proxmox-docs-common repo, which can be used for all documentation not specific to a single product. Marked RFC to gather feedback from other people. After talking to some people off-list, there definitely is the need for something like this in the long run. For now, this is pve-docs only, with only one page and two partials (see also "Organization"). But it shows the purpose and general approach well enough to gather some first feedback. Motivation == A lot of pages and sections esp. from pve-docs apply to all three products. To name a few; package repos, (firmware) updates, network config, ZFS host administration, bootloader etc. etc. Further, the documentation for the auto-installer from Aaron (now in the wiki) and for the notification system from Lukas would also be good fit for proxmox-docs-common. The plan would be to gradually move them all into proxmox-docs-common as appropriate, thus de-duplicating and having them available on all three products without the headaches of copying & keeping them in sync. Most of the common documentation is now done via the wiki, but this approach would allow "codifying" these things - generating wiki pages from our documentation is a solved problem already. Organization Pretty simple; all files in the repo root of proxmox-docs-common represent "full pages", i.e. starting with a level-1 heading, much like in pve-docs. These must still each be included somewhere, e.g. for pve-docs the appropriate subpage or in `pve-admin-guide.adoc`. Additionally, there is the `partials/` folder, which contains exactly that - these are sections of pages (aka. with no level-1 heading) that can be shared, but where the whole page might might not be suitable for sharing. These can be then included as usual using include::proxmox-docs-common/partials/foo.adoc[] The distinction is only for organization in the repo itself, but otherwise has no special/hidden meaning. So this can easily be changed/dropped if desired. Future works - Integrating into pmg-docs - Integrating ReStructuredText - Integrating into proxmox-backup documentation - Moving more pages and sections into it, of course - lots to be done there - Unifying the `scan-adoc-refs` tool from {pve,pmg}-docs in the common repo AsciiDoc vs. ReStructuredText = Currently, this is AsciiDoc only. My proposal would be to keep all exisiting AsciiDoc pages as such and only adapt as necessary. Since it is possible to automatically convert between AsciiDoc and ReStructuredText, this means we can reuse existing AsciiDoc-written pages/partials in PBS and write new pages in ReStructuredText. The latter is not included in this series yet, but would be a follow-up in the future, to keep this initial change as simple as possible. [ Of course, one could also think about mechanically converting all AsciiDoc-written pages to ReStructuredText, but that's a whole different story and harder to get right. ] Diffstats = Looks a bit scary, but the bulk of it are simply renames & pulling sections out into different files. For the latter, there is always a commit for copying the content 1:1 and one adapting small things as necessary. pve-docs: Christoph Heiss (7): gitmodules: add proxmox-docs-common scan-adoc-refs: enable building pages from proxmox-docs-common/ subdir images: strip `pve-` prefix from screenshots used in common docs asciidoc: conf: add iso-url variable installation-media: move to common docs installation: use new 'installation-flow' partial from common docs installation: use new 'advanced-installation' partial from common docs .gitmodules | 3 + Makefile | 6 +- asciidoc/asciidoc-pve.conf| 3 + .../{pve-grub-menu.png => grub-menu.png} | Bin .../{pve-grub-menu.ppm => grub-menu.ppm} | Bin ...nstall-summary.png => install-summary.png} | Bin ...nstall-summary.ppm => install-summary.ppm} | Bin ...{pve-installation.png => installation.png} | Bin ...{pve-installation.ppm => installation.ppm} | Bin ...elect-location.png => select-location.png} | Bin ...elect-location.ppm => select-location.ppm} | Bin ...target-disk.png => select-target-disk.png} | Bin ...target-disk.ppm => select-target-disk.ppm} | Bin ...{pve-set-password.png => set-password.png} | Bin ...{pve-set-password.ppm => set-password.ppm} | Bin ...ve-setup-network.png => setup-network.png} | Bin ...ve-setup-network.ppm => setup-network.ppm} | Bin ...ve-tui-installer.png => tui-installer.png} | Bin ...ve-tui-installer.ppm => tui-installer.ppm} | Bin png-verify.pl | 14 +- proxmox-docs-common
[pve-devel] [RFC PATCH docs-common 01/13] installation-media: move page from pve-docs here
Small adaptions were necessary; mostly a s/{pve}/{product}/g and replacing the ISO URL with the {iso-url} variable. Signed-off-by: Christoph Heiss --- installation-media.adoc | 132 1 file changed, 132 insertions(+) create mode 100644 installation-media.adoc diff --git a/installation-media.adoc b/installation-media.adoc new file mode 100644 index 000..a1c9402 --- /dev/null +++ b/installation-media.adoc @@ -0,0 +1,132 @@ +[[installation_prepare_media]] +Prepare Installation Media +-- +ifdef::wiki[] +:pve-toplevel: +endif::wiki[] + +Download the installer ISO image from: {website}en/downloads/proxmox-virtual-environment/iso + +The {pve} installation media is a hybrid ISO image. It works in two ways: + +* An ISO image file ready to burn to a CD or DVD. + +* A raw sector (IMG) image file ready to copy to a USB flash drive (USB stick). + +Using a USB flash drive to install {pve} is the recommended way because it is +the faster option. + +Prepare a USB Flash Drive as Installation Medium + + +The flash drive needs to have at least 1 GB of storage available. + +NOTE: Do not use UNetbootin. It does not work with the {pve} installation image. + +IMPORTANT: Make sure that the USB flash drive is not mounted and does not +contain any important data. + + +Instructions for GNU/Linux +~~ + +On Unix-like operating system use the `dd` command to copy the ISO image to the +USB flash drive. First find the correct device name of the USB flash drive (see +below). Then run the `dd` command. + + +# dd bs=1M conv=fdatasync if=./proxmox-ve_*.iso of=/dev/XYZ + + +NOTE: Be sure to replace /dev/XYZ with the correct device name and adapt the +input filename ('if') path. + +CAUTION: Be very careful, and do not overwrite the wrong disk! + + +Find the Correct USB Device Name + +There are two ways to find out the name of the USB flash drive. The first one is +to compare the last lines of the `dmesg` command output before and after +plugging in the flash drive. The second way is to compare the output of the +`lsblk` command. Open a terminal and run: + + +# lsblk + + +Then plug in your USB flash drive and run the command again: + + +# lsblk + + +A new device will appear. This is the one you want to use. To be on the extra +safe side check if the reported size matches your USB flash drive. + + +Instructions for macOS +~~ + +Open the terminal (query Terminal in Spotlight). + +Convert the `.iso` file to `.dmg` format using the convert option of `hdiutil`, +for example: + + +# hdiutil convert proxmox-ve_*.iso -format UDRW -o proxmox-ve_*.dmg + + +TIP: macOS tends to automatically add '.dmg' to the output file name. + +To get the current list of devices run the command: + + +# diskutil list + + +Now insert the USB flash drive and run this command again to determine which +device node has been assigned to it. (e.g., /dev/diskX). + + +# diskutil list +# diskutil unmountDisk /dev/diskX + + +NOTE: replace X with the disk number from the last command. + + +# sudo dd if=proxmox-ve_*.dmg bs=1M of=/dev/rdiskX + + +NOTE: 'rdiskX', instead of 'diskX', in the last command is intended. It will +increase the write speed. + +Instructions for Windows + + +Using Etcher + + +Etcher works out of the box. Download Etcher from https://etcher.io. It will +guide you through the process of selecting the ISO and your USB flash drive. + +Using Rufus +^^^ + +Rufus is a more lightweight alternative, but you need to use the *DD mode* to +make it work. Download Rufus from https://rufus.ie/. Either install it or use +the portable version. Select the destination drive and the {pve} ISO file. + +IMPORTANT: Once you 'Start' you have to click 'No' on the dialog asking to +download a different version of GRUB. In the next dialog select the 'DD' mode. + +ifdef::wiki[] +Boot your Server from the USB Flash Drive +~ + +Connect the USB flash drive to your server and make sure that booting from USB +is enabled (check your servers firmware settings). Then follow the steps in the +xref:chapter_installation[installation wizard]. + +endif::wiki[] -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC PATCH docs-common 02/13] installation-media: adapt for common usage
Small adaptions were necessary; mostly a trivial s/{pve}/{product}/g and replacing the ISO URL with the {iso-url} variable. Signed-off-by: Christoph Heiss --- installation-media.adoc | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/installation-media.adoc b/installation-media.adoc index a1c9402..bcef51c 100644 --- a/installation-media.adoc +++ b/installation-media.adoc @@ -1,27 +1,30 @@ [[installation_prepare_media]] Prepare Installation Media -- +ifdef::pve[] ifdef::wiki[] :pve-toplevel: endif::wiki[] +endif::pve[] -Download the installer ISO image from: {website}en/downloads/proxmox-virtual-environment/iso +Download the installer ISO image from: {iso-url} -The {pve} installation media is a hybrid ISO image. It works in two ways: +The {product} installation media is a hybrid ISO image. It works in two ways: * An ISO image file ready to burn to a CD or DVD. * A raw sector (IMG) image file ready to copy to a USB flash drive (USB stick). -Using a USB flash drive to install {pve} is the recommended way because it is -the faster option. +Using a USB flash drive to install {product} is the recommended way because it +is the faster option. Prepare a USB Flash Drive as Installation Medium The flash drive needs to have at least 1 GB of storage available. -NOTE: Do not use UNetbootin. It does not work with the {pve} installation image. +NOTE: Do not use UNetbootin. It does not work with the {product} installation +image. IMPORTANT: Make sure that the USB flash drive is not mounted and does not contain any important data. @@ -35,7 +38,7 @@ USB flash drive. First find the correct device name of the USB flash drive (see below). Then run the `dd` command. -# dd bs=1M conv=fdatasync if=./proxmox-ve_*.iso of=/dev/XYZ +# dd bs=1M conv=fdatasync if=./proxmox-*.iso of=/dev/XYZ NOTE: Be sure to replace /dev/XYZ with the correct device name and adapt the @@ -74,7 +77,7 @@ Convert the `.iso` file to `.dmg` format using the convert option of `hdiutil`, for example: -# hdiutil convert proxmox-ve_*.iso -format UDRW -o proxmox-ve_*.dmg +# hdiutil convert proxmox-*.iso -format UDRW -o proxmox-ve_*.dmg TIP: macOS tends to automatically add '.dmg' to the output file name. @@ -96,7 +99,7 @@ device node has been assigned to it. (e.g., /dev/diskX). NOTE: replace X with the disk number from the last command. -# sudo dd if=proxmox-ve_*.dmg bs=1M of=/dev/rdiskX +# sudo dd if=proxmox-*.dmg bs=1M of=/dev/rdiskX NOTE: 'rdiskX', instead of 'diskX', in the last command is intended. It will @@ -116,7 +119,7 @@ Using Rufus Rufus is a more lightweight alternative, but you need to use the *DD mode* to make it work. Download Rufus from https://rufus.ie/. Either install it or use -the portable version. Select the destination drive and the {pve} ISO file. +the portable version. Select the destination drive and the {product} ISO file. IMPORTANT: Once you 'Start' you have to click 'No' on the dialog asking to download a different version of GRUB. In the next dialog select the 'DD' mode. -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer v6 36/36] autoinst-helper: add prepare-iso subcommand
Just quick three notes inline; nits other than the crate thing. Did not review in depth, LGTM overall tho. On Wed, Apr 17, 2024 at 02:31:08PM +0200, Aaron Lauterer wrote: [..] > diff --git a/proxmox-autoinst-helper/Cargo.toml > b/proxmox-autoinst-helper/Cargo.toml > index 2a88c0f..75399e0 100644 > --- a/proxmox-autoinst-helper/Cargo.toml > +++ b/proxmox-autoinst-helper/Cargo.toml > @@ -19,3 +19,4 @@ serde_json = "1.0" > toml = "0.7" > log = "0.4.20" > regex = "1.7" > +which = "4.2.5" Misses the debian/control entry, but see also below. [..] > > +fn prepare_iso(args: ) -> Result<()> { > +check_prepare_requirements(args)?; > + > +if args.install_mode == AutoInstModes::Included && > args.answer_file.is_none() { > +bail!("Missing path to answer file needed for 'direct' install > mode."); > +} > +if args.install_mode == AutoInstModes::Included && > args.cert_fingerprint.is_some() { > +bail!("No certificate fingerprint needed for direct install mode. > Drop the parameter!"); > +} > +if args.install_mode == AutoInstModes::Included && args.url.is_some() { > +bail!("No URL needed for direct install mode. Drop the parameter!"); > +} if args.install_mode == AutoInstModes::Included { if args.answer_file.is_none() { bail!("Missing path to answer file needed for 'direct' install mode."); } if args.cert_fingerprint.is_some() { bail!("No certificate fingerprint needed for direct install mode. Drop the parameter!"); } if args.url.is_some() { bail!("No URL needed for direct install mode. Drop the parameter!"); } } else if (args.install_mode == AutoInstModes::Partition) { .. } .. maybe, to avoid the repeated condition? (The resulting visual grouping is also nice) > +if args.answer_file.is_some() && args.install_mode != > AutoInstModes::Included { > +bail!("Set '-i', '--install-mode' to 'included' to place the answer > file directly in the ISO."); > +} > +if args.install_mode == AutoInstModes::Partition && > args.cert_fingerprint.is_some() { > +bail!("No certificate fingerprint needed for partition install mode. > Drop the parameter!"); > +} > +if args.install_mode == AutoInstModes::Partition && args.url.is_some() { > +bail!("No URL needed for partition install mode. Drop the > parameter!"); > +} > + [..] > + > fn get_disks() -> Result>> { > let unwantend_block_devs = vec![ > "ram[0-9]*", > @@ -335,3 +510,53 @@ fn get_udev_properties(path: ) -> Result > { > } > Ok(String::from_utf8(udev_output.stdout)?) > } > + > +fn parse_answer(path: ) -> Result { > +let mut file = match fs::File::open(path) { > +Ok(file) => file, > +Err(err) => bail!("Opening answer file '{}' failed: {err}", > path.display()), > +}; > +let mut contents = String::new(); > +if let Err(err) = file.read_to_string( contents) { > +bail!("Reading from file '{}' failed: {err}", path.display()); > +} There is also std::fs::read_to_string() for exactly that; and would avoid the whole open/close dance :^) (Seems I missed that when reviewing the patch that introduced validate_answer()) > +match toml::from_str() { > +Ok(answer) => { > +println!("The file was parsed successfully, no syntax errors > found!"); > +Ok(answer) > +} > +Err(err) => bail!("Error parsing answer file: {err}"), > +} > +} > + > +fn check_prepare_requirements(args: ) -> Result<()> { > +match which("xorriso") { Do we really need _yet another_ crate dependency for that? Below is a check / bail! anyway when running the command proper. And if we really want a explicit check beforehand, I'd just do something like fn which(name: ) -> Result<()> { match Command::new(name).output() { Ok(_) => Ok(()), Err(err) => Err(err.into()), } } > +Ok(_) => (), > +Err(_) => bail!("Could not find 'xorriso'. Please install it and try > again"), > +} > + > +match Path::try_exists() { > +Ok(true) => (), > +Ok(false) => bail!("Source file does not exist."), > +Err(_) => bail!("Source file does not exist."), > +} > + > +match Command::new("xorriso") > +.arg("-dev") > +.arg() > +.arg("-find") > +.arg(PROXMOX_ISO_FLAG) > +.stderr(Stdio::null()) > +.stdout(Stdio::null()) > +.status() > +{ > +Ok(v) => { > +if !v.success() { > +bail!("The source ISO file is not able to be installed > automatically. Please try a more current one."); > +} > +} > +Err(_) => bail!("Could not run 'xorriso'. Please install it."), > +}; > + > +Ok(()) > +} > -- > 2.39.2 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com >
Re: [pve-devel] [PATCH installer v2 0/3] expose zfs arc size setting for all products
v3: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062976.html On Wed, Feb 07, 2024 at 03:28:11PM +0100, Christoph Heiss wrote: > As suggested by Thomas, leaves the ZFS default if the user never touches > the setting in the installer (i.e. not writing a modprobe file). > See also the discussion in v1 [0]. > > [0] https://lists.proxmox.com/pipermail/pve-devel/2024-February/061659.html > > Testing > --- > Tested the installation of PVE and PBS, with each once letting the arc > size setting untouched and once setting it to some specific value. > Afterwards, checked that for PVE the module parameter was always written > to /etc/modprobe.d/, for PBS that it was only written in case it was > set. > > Previous revisions > -- > v1: https://lists.proxmox.com/pipermail/pve-devel/2023-November/060898.html > > Changes v1 -> v2: > * rebased on latest master > * add placeholder functionality for arc max size in TUI > * "emulate" placeholder functionality in GTK on best-effort basis > > Christoph Heiss (3): > tui: NumericEditView: add optional placeholder value > tui: expose arc size setting for zfs bootdisks for all products > proxinstall: expose arc size setting for zfs bootdisks for all > products > > Proxmox/Install/RunEnv.pm | 3 +- > proxinstall | 37 +++ > proxmox-installer-common/src/options.rs | 3 +- > proxmox-tui-installer/src/views/bootdisk.rs | 48 -- > proxmox-tui-installer/src/views/mod.rs | 69 +++-- > 5 files changed, 124 insertions(+), 36 deletions(-) > > -- > 2.42.0 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer v3 1/3] tui: NumericEditView: add optional placeholder value
Enables to add an optional placeholder value to `NumericEditView`, which will be displayed in a different (darker) color and not returned by `.get_content*()`. Can be used for having default values in the TUI, but with different handling in the back. Signed-off-by: Christoph Heiss --- Changes v2 -> v3: * when empty & focused, do not show the placeholder value at all Changes v1 -> v2: * new patch proxmox-tui-installer/src/views/mod.rs | 66 -- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs index 3244e76..5e5f4fb 100644 --- a/proxmox-tui-installer/src/views/mod.rs +++ b/proxmox-tui-installer/src/views/mod.rs @@ -2,9 +2,10 @@ use std::{net::IpAddr, rc::Rc, str::FromStr}; use cursive::{ event::{Event, EventResult}, +theme::BaseColor, view::{Resizable, ViewWrapper}, views::{EditView, LinearLayout, NamedView, ResizedView, SelectView, TextView}, -Rect, Vec2, View, +Printer, Rect, Vec2, View, }; use proxmox_installer_common::utils::CidrAddress; @@ -24,6 +25,7 @@ pub use timezone::*; pub struct NumericEditView { view: LinearLayout, max_value: Option, +placeholder: Option, max_content_width: Option, allow_empty: bool, } @@ -36,6 +38,7 @@ impl NumericEditView { Self { view, max_value: None, +placeholder: None, max_content_width: None, allow_empty: false, } @@ -54,6 +57,7 @@ impl NumericEditView { Self { view, max_value: None, +placeholder: None, max_content_width: None, allow_empty: false, } @@ -84,15 +88,42 @@ impl NumericEditView { self } +/// Sets a placeholder value for this view. Implies `allow_empty(true)`. +/// Implies `allow_empty(true)`. +/// +/// # Arguments +/// `placeholder` - The placeholder value to set for this view. +#[allow(unused)] +pub fn placeholder(mut self, placeholder: T) -> Self { +self.placeholder = Some(placeholder); +self.allow_empty(true) +} + +/// Returns the current value of the view. If a placeholder is defined and +/// no value is currently set, the placeholder value is returned. +/// +/// **This should only be called when `allow_empty = false` or a placeholder +/// is set.** pub fn get_content() -> Result::Err> { -assert!(!self.allow_empty); -self.inner().get_content().parse() +let content = self.inner().get_content(); + +if content.is_empty() { +if let Some(placeholder) = self.placeholder { +return Ok(placeholder); +} +} + +assert!(!(self.allow_empty && self.placeholder.is_none())); +content.parse() } +/// Returns the current value of the view, or [`None`] if the view is +/// currently empty. pub fn get_content_maybe() -> Option::Err>> { let content = self.inner().get_content(); + if !content.is_empty() { -Some(self.inner().get_content().parse()) +Some(content.parse()) } else { None } @@ -157,6 +188,25 @@ impl NumericEditView { std::mem::swap(self.inner_mut(), inner); self } + +/// Generic `wrap_draw()` implementation for [`ViewWrapper`]. +/// +/// # Arguments +/// * `printer` - The [`Printer`] to draw to the base view. +fn wrap_draw_inner(, printer: ) { +self.view.draw(printer); + +if self.inner().get_content().is_empty() && !printer.focused { +if let Some(placeholder) = self.placeholder { +let placeholder = placeholder.to_string(); + +printer.with_color( +(BaseColor::Blue.light(), BaseColor::Blue.dark()).into(), +|printer| printer.print((0, 0), ), +); +} +} +} } pub type FloatEditView = NumericEditView; @@ -165,6 +215,10 @@ pub type IntegerEditView = NumericEditView; impl ViewWrapper for FloatEditView { cursive::wrap_impl!(self.view: LinearLayout); +fn wrap_draw(, printer: ) { +self.wrap_draw_inner(printer); +} + fn wrap_on_event( self, event: Event) -> EventResult { let original = self.inner_mut().get_content(); @@ -204,6 +258,10 @@ impl FloatEditView { impl ViewWrapper for IntegerEditView { cursive::wrap_impl!(self.view: LinearLayout); +fn wrap_draw(, printer: ) { +self.wrap_draw_inner(printer); +} + fn wrap_on_event( self, event: Event) -> EventResult { let original = self.inner_mut().get_content(); -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer v3 2/3] tui: expose arc size setting for zfs bootdisks for all products
For non-PVE products, simply use the ZFS defaults (aka. 50%) and leave unset, if the user never touches that setting. Signed-off-by: Christoph Heiss --- Changes v2 -> v3: * no changes Changes v1 -> v2: * use new placeholder functionality for non-PVE products proxmox-installer-common/src/options.rs | 3 +- proxmox-tui-installer/src/views/bootdisk.rs | 45 - proxmox-tui-installer/src/views/mod.rs | 1 - 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs index 1aa8f65..a210142 100644 --- a/proxmox-installer-common/src/options.rs +++ b/proxmox-installer-common/src/options.rs @@ -205,7 +205,8 @@ impl ZfsBootdiskOptions { /// The default ZFS maximum ARC size in MiB for this system. fn default_zfs_arc_max(product: ProxmoxProduct, total_memory: usize) -> usize { if product != ProxmoxProduct::PVE { -// Use ZFS default for non-PVE +// For products other the PVE, just let ZFS decide on its own. Setting `0` +// causes the installer to skip writing the `zfs_arc_max` module parameter. 0 } else { ((total_memory as f64) / 10.) diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs index 7e13e91..3d9be50 100644 --- a/proxmox-tui-installer/src/views/bootdisk.rs +++ b/proxmox-tui-installer/src/views/bootdisk.rs @@ -564,7 +564,18 @@ impl ZfsBootdiskOptionsView { options: , product_conf: , ) -> Self { -let is_pve = product_conf.product == ProxmoxProduct::PVE; +let arc_max_view = { +let view = IntegerEditView::new_with_suffix("MiB").max_value(runinfo.total_memory); + +// For PVE "force" the default value, for other products place the recommended value +// only in the placeholder. This causes for the latter to not write the module option +// if the value is never modified by the user. +if product_conf.product == ProxmoxProduct::PVE { +view.content(options.arc_max) +} else { +view.placeholder(runinfo.total_memory / 2) +} +}; let inner = FormView::new() .child("ashift", IntegerEditView::new().content(options.ashift)) @@ -592,14 +603,11 @@ impl ZfsBootdiskOptionsView { .unwrap_or_default(), ), ) -.child("copies", IntegerEditView::new().content(options.copies).max_value(3)) -.child_conditional( -is_pve, -"ARC max size", -IntegerEditView::new_with_suffix("MiB") -.max_value(runinfo.total_memory) -.content(options.arc_max), +.child( +"copies", +IntegerEditView::new().content(options.copies).max_value(3), ) +.child("ARC max size", arc_max_view) .child("hdsize", DiskSizeEditView::new().content(options.disk_size)); let view = MultiDiskOptionsView::new(, _disks, inner) @@ -621,21 +629,22 @@ impl ZfsBootdiskOptionsView { fn get_values( self) -> Option<(Vec, ZfsBootdiskOptions)> { let (disks, selected_disks) = self.view.get_disks_and_selection()?; let view = self.view.inner_mut()?; -let has_arc_max = view.len() >= 6; -let disk_size_index = if has_arc_max { 5 } else { 4 }; let ashift = view.get_value::(0)?; let compress = view.get_value::, _>(1)?; let checksum = view.get_value::, _>(2)?; let copies = view.get_value::(3)?; -let disk_size = view.get_value::(disk_size_index)?; - -let arc_max = if has_arc_max { -view.get_value::(4)? -.max(ZFS_ARC_MIN_SIZE_MIB) -} else { -0 // use built-in ZFS default value -}; +let disk_size = view.get_value::(5)?; + +// If a value is set, return that and clamp it to at least [`ZFS_ARC_MIN_SIZE_MIB`]. +// +// Otherwise, if no value was set or an error occured return `0`. The former simply means +// that the placeholder value is still there. +let arc_max = view +.get_child::(4)? +.get_content_maybe() +.map_or(Ok(0), |v| v.map(|v| v.max(ZFS_ARC_MIN_SIZE_MIB))) +.unwrap_or(0); Some(( disks, diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs index 5e5f4fb..eab258c 100644 --- a/proxmox-tui-installer/src/views/mod.rs +++ b/proxmox-tui-installer/src/views/mod.rs @@ -93,7 +93,6 @@ impl NumericEditView { /// /// # Arguments /// `placeholder` - The placeholder value to set for this view. -#
[pve-devel] [PATCH installer v3 3/3] proxinstall: expose arc size setting for zfs bootdisks for all products
For non-PVE products, simply use the ZFS defaults (aka. 50%) and leave unset, if the user never touches that setting. Signed-off-by: Christoph Heiss --- Changes v2 -> v3: * rework based on Maximilano's suggestion using Gtk3::Adjustment Changes v1 -> v2: * add some more explanatory comments Proxmox/Install/RunEnv.pm | 3 ++- proxinstall | 26 +- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm index 25b6bb3..4e24da6 100644 --- a/Proxmox/Install/RunEnv.pm +++ b/Proxmox/Install/RunEnv.pm @@ -319,7 +319,8 @@ our $ZFS_ARC_SYSMEM_PERCENTAGE = 0.1; # use 10% of available system memory by de # Calculates the default upper limit for the ZFS ARC size. # Returns the default ZFS maximum ARC size in MiB. sub default_zfs_arc_max { -# Use ZFS default on non-PVE +# For products other the PVE, just let ZFS decide on its own. Setting `0` +# causes the installer to skip writing the `zfs_arc_max` module parameter. return 0 if Proxmox::Install::ISOEnv::get('product') ne 'pve'; my $default_mib = get('total_memory') * $ZFS_ARC_SYSMEM_PERCENTAGE; diff --git a/proxinstall b/proxinstall index a6a4cfb..bc0e1e4 100755 --- a/proxinstall +++ b/proxinstall @@ -1138,20 +1138,20 @@ my $create_raid_advanced_grid = sub { $spinbutton_copies->set_value($copies); push @$labeled_widgets, ['copies', $spinbutton_copies]; -if ($iso_env->{product} eq 'pve') { - my $total_memory = Proxmox::Install::RunEnv::get('total_memory'); +my $total_memory = Proxmox::Install::RunEnv::get('total_memory'); +my $arc_max = Proxmox::Install::Config::get_zfs_opt('arc_max') || ($total_memory * 0.5); + +my $arc_max_adjustment = Gtk3::Adjustment->new( + $arc_max, $Proxmox::Install::RunEnv::ZFS_ARC_MIN_SIZE_MIB, + $total_memory, 1, 10, 0); +my $spinbutton_arc_max = Gtk3::SpinButton->new($arc_max_adjustment, 1, 0); +$spinbutton_arc_max->set_tooltip_text('Maximum ARC size in megabytes'); +$spinbutton_arc_max->signal_connect('value-changed' => sub { + my $w = shift; + Proxmox::Install::Config::set_zfs_opt('arc_max', $w->get_value_as_int()); +}); - my $spinbutton_arc_max = Gtk3::SpinButton->new_with_range( - $Proxmox::Install::RunEnv::ZFS_ARC_MIN_SIZE_MIB, $total_memory, 1); - $spinbutton_arc_max->set_tooltip_text('Maximum ARC size in megabytes'); - $spinbutton_arc_max->signal_connect('value-changed' => sub { - my $w = shift; - Proxmox::Install::Config::set_zfs_opt('arc_max', $w->get_value_as_int()); - }); - my $arc_max = Proxmox::Install::Config::get_zfs_opt('arc_max'); - $spinbutton_arc_max->set_value($arc_max); - push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB']; -} +push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB']; push @$labeled_widgets, ['hdsize', $hdsize_btn, 'GB']; return $create_label_widget_grid->($labeled_widgets);; -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer v3 0/3] expose zfs arc size setting for all products
As suggested by Thomas, leaves the ZFS default if the user never touches the setting in the installer (i.e. not writing a modprobe file). See also the discussion in v1 [0]. [0] https://lists.proxmox.com/pipermail/pve-devel/2024-February/061659.html Testing --- Tested the installation of PVE and PBS (PMG is handled the same as PBS), with each once letting the arc size setting untouched and once setting it to some specific value. Also checked for each whether the correct default value was displayed. Afterwards, checked that for PVE the module parameter was always written to /etc/modprobe.d/, for PBS that it was only written in case it was explicitly set. Previous revisions -- v2: https://lists.proxmox.com/pipermail/pve-devel/2024-February/061667.html v1: https://lists.proxmox.com/pipermail/pve-devel/2023-November/060898.html Changes v2 -> v3: * tui: when empty & focused, do not show the placeholder value at all * gui: rework using Gtk3::Adjustment based on Maximilanos suggestion Changes v1 -> v2: * rebased on latest master * add placeholder functionality for arc max size in TUI * "emulate" placeholder functionality in GTK on best-effort basis Christoph Heiss (3): tui: NumericEditView: add optional placeholder value tui: expose arc size setting for zfs bootdisks for all products proxinstall: expose arc size setting for zfs bootdisks for all products Proxmox/Install/RunEnv.pm | 3 +- proxinstall | 26 - proxmox-installer-common/src/options.rs | 3 +- proxmox-tui-installer/src/views/bootdisk.rs | 45 -- proxmox-tui-installer/src/views/mod.rs | 65 +++-- 5 files changed, 105 insertions(+), 37 deletions(-) -- 2.42.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer v2 3/3] proxinstall: expose arc size setting for zfs bootdisks for all products
Thanks for the review! On Fri, Feb 23, 2024 at 04:37:16PM +0100, Maximiliano Sandoval wrote: > > Some comments bellow. > > Christoph Heiss writes: > > > For non-PVE products, simply use the ZFS defaults (aka. 50%) and leave > > unset, if the user never touches that setting. > > > > Signed-off-by: Christoph Heiss > > --- > > ... > > ... > > + > > +# GTKs SpinButton does not support a placeholder value, unfortunaly. > > That means we also > > There is a typo on unfortunately. What exactly do you mean by a > placeholder value? Like the initial value? If so see comment bellow. Placeholder as in the `Gtk.Entry:placeholder-text` [0]. Effectively a separate value that is displayed to the user if the field is empty. [0] https://docs.gtk.org/gtk3/property.Entry.placeholder-text.html > > > +# have to set the value normally for non-PVE products, where the ZFS > > default should be used, > > +# in case the user does not explicitly set a value. > > +# But due to the signal-based nature of GTK, as long as the user never > > touches the spinbutton, > > +# `Proxmox::Install::Config::set_zfs_opt('arc_max', ..)` never gets > > called, thus the default > > +# value (which is 0 for non-PVE) won't get overwritten and no modprobe > > file is written. > > +if ($arc_max > 0) { > > $spinbutton_arc_max->set_value($arc_max); > > - push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB']; > > +} else { > > + # .. but we need to display the "real" value to the user > > + $spinbutton_arc_max->set_value($total_memory * 0.5); > > } > > > > +# We need to connect the signal afterwards, to avoid triggering it > > using ->set_value() above. > > > > Alternatively one could init the spin button with the correct values > e.g. > > my $total_memory = Proxmox::Install::RunEnv::get('total_memory'); > my $arc_max = Proxmox::Install::Config::get_zfs_opt('arc_max'); > my $arc_max_value = $total_memory * 0.5; > $arc_max_value = $arc_max if $arc_max > 0; > my $arc_max_adjusment = Gtk3::Adjustment->new($arc_max_value, > $Proxmox::Install::RunEnv::ZFS_ARC_MIN_SIZE_MIB, $total_memory, 1, 10, 0); > my $spinbutton_arc_max = Gtk3::SpinButton->new($arc_max_adjusment, 1, 0); > $spinbutton_arc_max->set_tooltip_text('Maximum ARC size in megabytes'); > $spinbutton_arc_max->signal_connect('value-changed' => sub { > my $w = shift; > Proxmox::Install::Config::set_zfs_opt('arc_max', > $w->get_value_as_int()); > }); > > Here the numerical values 1, 10, 0, and 1, 0 come from a quick > inspection of the source for gtk_spin_button_new_with_range and > gtk_spin_button_new (in the docs [1, 2] there is a [src] link next to > the Description header). Well, after testing, this does not really create the wanted behaviour for PBS and PMG. For these two products, we want to display the ZFS default value, used when the `zfs_arc_max` module parameter is unset (aka. 50% of memory). But the widget itself should return 0 (aka what Proxmox::Install::RunEnv::default_zfs_arc_max() returns) in case the user never changed it, such that we can skip writing the file (resulting in the previous behaviour). That's why the "workaround" was needed to support that. > > [1] https://docs.gtk.org/gtk3/ctor.SpinButton.new.html > [2] https://docs.gtk.org/gtk3/ctor.SpinButton.new_with_range.html > > > +$spinbutton_arc_max->signal_connect('value-changed' => sub { > > + my $w = shift; > > + Proxmox::Install::Config::set_zfs_opt('arc_max', > > $w->get_value_as_int()); > > +}); > > + > > +push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB']; > > + > > push @$labeled_widgets, ['hdsize', $hdsize_btn, 'GB']; > > return $create_label_widget_grid->($labeled_widgets);; > > }; > > > -- > Maximiliano > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v4 installer 31/30 follow-up] auto-installer: answer: deny unknown fields
LGTM, does exactly what is says on the tin. Tested it using both the `proxmox-autoinst-helper validate-answer` tool and trying to boot the auto-installer itself with a bogus answer file. So please consider this also: Reviewed-by: Christoph Heiss Tested-by: Christoph Heiss On Fri, Apr 05, 2024 at 04:25:07PM +0200, Aaron Lauterer wrote: > This way, serde will throw errors if fields are not known. > > This can help to reduce frustration if one might think to have set an > option, but for example a small type has happened. Yeah, that's kinda how I discovered that, wondering why a certain option did not get applied :^) > > Signed-off-by: Aaron Lauterer > --- > Since Christoph mentioned it I tried to implement it. Tested quickly > with the proxmox-autoinst-helper tool. > > proxmox-auto-installer/src/answer.rs | 16 +++- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/proxmox-auto-installer/src/answer.rs > b/proxmox-auto-installer/src/answer.rs > index 94cebb3..57c2602 100644 > --- a/proxmox-auto-installer/src/answer.rs > +++ b/proxmox-auto-installer/src/answer.rs > @@ -10,7 +10,7 @@ use std::{collections::BTreeMap, net::IpAddr}; > /// storing them in a HashMap > > #[derive(Clone, Deserialize, Debug)] > -#[serde(rename_all = "kebab-case")] > +#[serde(rename_all = "kebab-case", deny_unknown_fields)] > pub struct Answer { > pub global: Global, > pub network: Network, > @@ -19,6 +19,7 @@ pub struct Answer { > } > > #[derive(Clone, Deserialize, Debug)] > +#[serde(deny_unknown_fields)] > pub struct Global { > pub country: String, > pub fqdn: Fqdn, > @@ -33,6 +34,7 @@ pub struct Global { > } > > #[derive(Clone, Deserialize, Debug)] > +#[serde(deny_unknown_fields)] > struct NetworkInAnswer { > #[serde(default)] > pub use_dhcp: bool, > @@ -43,7 +45,7 @@ struct NetworkInAnswer { > } > > #[derive(Clone, Deserialize, Debug)] > -#[serde(try_from = "NetworkInAnswer")] > +#[serde(try_from = "NetworkInAnswer", deny_unknown_fields)] > pub struct Network { > pub network_settings: NetworkSettings, > } > @@ -97,6 +99,7 @@ pub struct NetworkManual { > } > > #[derive(Clone, Debug, Deserialize)] > +#[serde(deny_unknown_fields)] > pub struct DiskSetup { > pub filesystem: Filesystem, > #[serde(default)] > @@ -109,7 +112,7 @@ pub struct DiskSetup { > } > > #[derive(Clone, Debug, Deserialize)] > -#[serde(try_from = "DiskSetup")] > +#[serde(try_from = "DiskSetup", deny_unknown_fields)] > pub struct Disks { > pub fs_type: FsType, > pub disk_selection: DiskSelection, > @@ -207,14 +210,14 @@ pub enum DiskSelection { > Filter(BTreeMap), > } > #[derive(Clone, Deserialize, Debug, PartialEq, ValueEnum)] > -#[serde(rename_all = "lowercase")] > +#[serde(rename_all = "lowercase", deny_unknown_fields)] > pub enum FilterMatch { > Any, > All, > } > > #[derive(Clone, Deserialize, Serialize, Debug, PartialEq)] > -#[serde(rename_all = "lowercase")] > +#[serde(rename_all = "lowercase", deny_unknown_fields)] > pub enum Filesystem { > Ext4, > Xfs, > @@ -223,6 +226,7 @@ pub enum Filesystem { > } > > #[derive(Clone, Copy, Default, Deserialize, Debug)] > +#[serde(deny_unknown_fields)] > pub struct ZfsOptions { > pub raid: Option, > pub ashift: Option, > @@ -234,6 +238,7 @@ pub struct ZfsOptions { > } > > #[derive(Clone, Copy, Default, Deserialize, Serialize, Debug)] > +#[serde(deny_unknown_fields)] > pub struct LvmOptions { > pub hdsize: Option, > pub swapsize: Option, > @@ -243,6 +248,7 @@ pub struct LvmOptions { > } > > #[derive(Clone, Copy, Default, Deserialize, Debug)] > +#[serde(deny_unknown_fields)] > pub struct BtrfsOptions { > pub hdsize: Option, > pub raid: Option, > -- > 2.39.2 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer v4 00/30] add automated/unattended installation
I've tested mostly the same things as for v3 [0], to confirm nothing broke since that: - Using a few different values for `global` options - Install on ext4, xfs, Btrfs RAID1 and ZFS RAID10 (with different values in multiple runs) - Using DHCP and static IP - Fetching answer from a partition - Fetching answer from a HTTP source, getting the URL through DHCP or DNS - Trying out the `proxmox-autoinst-helper` tool for assembling udev rules and validating files. - Using the `post_command` to create some files in the newly installed system. - Tested with PVE, PMG and PBS, each w/ BIOS & UEFI (latter also w/ SB) One small thing I noticed: unknown/undefined options in the answer file are currently silently ignored - in the installer as well as by `proxmox-autoinst-helper validate-answer`. Something to implement in the future though definitely, but for now IMHO a rather mundane issue. Really just noting it here for reference. I can also confirm now that a small bug I found in [0] is now fixed, such that LVM configurations only allows a single disk now. The other things from [0] (and more) were also talked over again with Aaron directly, off-list. Also quickly skimmed over the actual changes again, looks fine overall. At least nothing to really note of; that would impact functionality and aren't some low-hanging fruit for the future (as e.g. noted above). So please consider this whole series: Tested-by: Christoph Heiss Reviewed-by: Christoph Heiss [0] https://lists.proxmox.com/pipermail/pve-devel/2024-April/062485.html On Thu, Apr 04, 2024 at 04:48:32PM +0200, Aaron Lauterer wrote: > This patch series adds the possibility to do an automated / unattended > installation of Proxmox VE. > > The overall idea is that we will have a dedicated ISO for the unattended > installation. It should be configured in such a way that it will start > the installation without any user interaction. Therefore, the GRUB > config should automatically start it (after a timeout). > > The information for the installer that is usually gathered interactively > from the user is provided via an `answer.toml` file. > > The answer file allows to select disks and the network card via filters. > > The installer also allows to run custom commands pre and post > installation. This should give users plenty of possibilities to either > further customize/prepare the installation or integrate it into a larger > automated installation setup. > For example, one could issue HTTP requests to signal the status and > progress of the installation. > > When the installer is called with 'proxauto' in the kernel cmdline, the > 'proxmox-fetch-answer' binary is called. It tries to find the answer > file and once found, will start the 'proxmox-auto-installer' binary and > pass the contents to it via stdin. > > The auto-installer then parses the answer file and determines what > parameters need to be passed to the low-level installer. For example, > which disks and NIC to use, network IP settings and so forth. > > The current status reporting of the actual installation is kept rather > simple. > > Both binaries log into the tmp directory. > > There is a third binary, the 'proxmox-autoinst-helper'. It provides a > few subcommands, from the help: > answerValidate if an answer file is formatted correctly > device-match Test which devices the given filter matches against > device-info Show device information that can be used for filters > identifiers Show identifiers for the current machine. This information is > part of the POST request to fetch an answer file > > The fetch-answer binary is trying to get an answer file. It does so by > first searching for a partition/FS labeled `proxmoxinst`, or all upper > case, and an `answer.toml` in there. This could be provided by another > USB flash drive. > If that is not successful, the next step is to send an HTTP POST request > to a URL to get the TOML contents in return. A POST request was chosen > because we also send information to identify the host in JSON format. > > The question then is, where to get that URL from. Right now, there are > two options implemented. The first is looking for a custom DHCP option > and the second is querying for a TXT record in the `proxmoxinst` > subdomain of the search domain. > > It is possible to provide a SHA256 fingerprint of the SSL cert used by > the answer server. The safest option is to place a > `cert_fingerprint.txt` file in the same `proxmoxinst` partition as where > you alternatively would place the `answer.toml`. > If that is not found, then it can be provided by a second custom DHCP > option or placed as TXT record in the subdomain `proxmoxinst-fp`. > > This patch series now also separates the 3 binaries into their own > crate. The 'proxmox-
[pve-devel] [PATCH installer] html: pbs: fix missing in template after feature list
This adds an empty line between the feature list and the "more information" paragraph, which looks a lot better. The exact same is already present in the HTML template for both other products, probably a simple oversight. Signed-off-by: Christoph Heiss --- html/pbs/extract1-license.htm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/html/pbs/extract1-license.htm b/html/pbs/extract1-license.htm index 9ceae4f..cbaf9de 100644 --- a/html/pbs/extract1-license.htm +++ b/html/pbs/extract1-license.htm @@ -21,7 +21,7 @@ - Deduplication - Incremental backups - Remote sync -- Easy management +- Easy management For more information, visit www.proxmox.com or the Proxmox Backup Server project page. -- 2.43.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v3 00/30] add automated/unattended installation
On Tue, Apr 02, 2024 at 04:55:11PM +0200, Aaron Lauterer wrote: [..] > > > > - While trying out different configurations, I wondered if for the > >network something like this would be better for static IPs: > > > > [network.manual] > > cidr = ".." > > dns = ".." > > [..] > > > >.. keeping the `network.use_dhcp` option as before. Would simplify > >some checks now and provide good future-proofing for any new options > >that might get added. > > > >Thereby basically modelling > >`proxmox_auto_installer::answer::NetworkSettings` enum nearly 1:1 to > >the TOML config. > > okay, so that in the DHPC case, it could be > [network] > use_dhcp = true > > and in the manual case, either > [network] > manual.cidr = "…" > manual.dns = "…" > > and so forth, or, to keep it simpler, like your example with > [network.manual] defining the overall manual key. Yeah, exactly. > > This will make it slightly more elaborate to document, as we need to dig > deeper into how TOML works and that there are multiple ways to define the > same hierarchy. But it could be worth it to keep the definition cleaner. > > Some more feedback in that regard might be useful, especially since changing > the format later on will be, as you described it, a PITA :) Feel free though to not block this series on further feedback for this! :^) Just came to mind while pondering over this and trying different settings - but doesn't change anything wrt. functionality really. IMO we can change/break the answer file format at least with a new major release later on, so it's not completely set in stone after all. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v3 00/30] add automated/unattended installation
Rebuild an (PVE) ISO to contain the new auto-installer with an appropriate GRUB entry setting `proxauto` on the kernel commandline. What I've tried: - Using a few different values for `global` options - Install on ext4, xfs, Btrfs RAID1 and ZFS RAID1 (with different values in multiple runs) - Using DHCP and static IP - Fetching answer from a HTTP source, getting the URL from DHCP - Trying out the `proxmox-autoinst-helper` tool for assembling udev rules and validating files. - Using the `post_command` to create some files in the newly installed system. I didn't play around all that extensively the udev filters. Some notes: - When using ext4 or xfs as filesystem, `disk_list` happily takes multiple disks (but really only installs on the first disk, of course). Should probably be another sanity check there. - As for the `proxmox-autoinst-helper answer` command, might `validate` or `validate-answer` be a better name? `answer` alone seems a bit confusing at first, as if e.g. the tool would give me an answer file or similar. - `{pre,post}_command` in the answer file should be spelled plural, as it is an array of commands after all. - While trying out different configurations, I wondered if for the network something like this would be better for static IPs: [network.manual] cidr = ".." dns = ".." [..] .. keeping the `network.use_dhcp` option as before. Would simplify some checks now and provide good future-proofing for any new options that might get added. Thereby basically modelling `proxmox_auto_installer::answer::NetworkSettings` enum nearly 1:1 to the TOML config. Nitpicking at this point, I know, but changing the answer format afterwards would be a quite a PITA :^) Overall very nice! and Tested-by: Christoph Heiss in any case. On Thu, Mar 28, 2024 at 02:49:58PM +0100, Aaron Lauterer wrote: > This patch series adds the possibility to do an automated / unattended > installation of Proxmox VE. > > The overall idea is that we will have a dedicated ISO for the unattended > installation. It should be configured in such a way that it will start > the installation without any user interaction. > Though the integration in the installation environmend isn't part of > this patch series. > > The information for the installer that is usually gathered interactively > from the user is provided via an `answer.toml` file. > > The answer file allows to select disks and the network card via filters. > > The installer also allows to run custom commands pre and post > installation. This should give users plenty of possibilities to either > further customize/prepare the installation or integrate it into a larger > automated installation setup. > For example, one could issue HTTP requests to signal the status and > progress of the installation. > > When the installer is called with 'proxauto' in the kernel cmdline, the > 'proxmox-fetch-answer' binary is called. It tries to find the answer > file and once found, will start the 'proxmox-auto-installer' binary and > pass the contents to it via stdin. > > The auto-installer then parses the answer file and determines what > parameters need to be passed to the low-level installer. For example, > which disks and NIC to use, network IP settings and so forth. > > The current status reporting of the actual installation is kept rather > simple. > > Both binaries log into the tmp directory. > > There is a third binary, the 'proxmox-autoinst-helper'. It provides a > few subcommands, from the help: > answerValidate if an answer file is formatted correctly > device-match Test which devices the given filter matches against > device-info Show device information that can be used for filters > identifiers Show identifiers for the current machine. This information is > part of the POST request to fetch an answer file > > The fetch-answer binary is trying to get an answer file. It does so by > first searching for a partition/FS labeled `proxmoxinst`, or all upper > case, and an `answer.toml` in there. This could be provided by another > USB flash drive. > If that is not successful, the next step is to send an HTTP POST request > to a URL to get the TOML contents in return. A POST request was chosen > because we also send information to identify the host in JSON format. > > The question then is, where to get that URL from. Right now, there are > two options implemented. The first is looking for a custom DHCP option > and the second is querying for a TXT record in the `proxmoxinst` > subdomain of the search domain. > > It is possible to provide a SHA256 fingerprint of the SSL cert used by > the answer server. The safest option is to place a > `cert_fingerprint.txt` file in the same `proxmoxinst` partition as where > you
Re: [pve-devel] [PATCH v3 15/30] auto-installer: add fetch answer binary
Two typos ;) And some small nits On Thu, Mar 28, 2024 at 02:50:13PM +0100, Aaron Lauterer wrote: [..] > diff --git a/proxmox-auto-installer/src/bin/proxmox-fetch-answer.rs > b/proxmox-auto-installer/src/bin/proxmox-fetch-answer.rs > new file mode 100644 > index 000..9e89a3c > --- /dev/null > +++ b/proxmox-auto-installer/src/bin/proxmox-fetch-answer.rs [..] > +fn main() -> ExitCode { > +if let Err(err) = init_log() { > +panic!("could not initilize logging: {err}"); ^ typo And for consistency sake, capitalize the first word here too as done everywhere else? > +} > + > +info!("Fetching answer file"); > +let answer = match fetch_answer() { > +Ok(answer) => answer, > +Err(err) => { > +error!("Aborting: {}", err); > +return ExitCode::FAILURE; > +} > +}; > + > +let mut child = match Command::new("proxmox-auto-installer") > +.stdout(Stdio::inherit()) > +.stdin(Stdio::piped()) > +.stderr(Stdio::null()) > +.spawn() > +{ > +Ok(child) => child, > +Err(err) => panic!("Failed to start automatic installation: {}", > err), Very much a nit; but above always inline format-strings (i.e. "{err}") are used, from here on out mostly explicit parameters. Maybe unify them and use the former consistenly where possible? E.g. below in `utils.rs` are some more instances > +}; > + > +let mut stdin = child.stdin.take().expect("Failed to open stdin"); > +std::thread::spawn(move || { > +stdin > +.write_all(answer.as_bytes()) > +.expect("Failed to write to stdin"); > +}); [..] > diff --git a/proxmox-auto-installer/src/fetch_plugins/utils.rs > b/proxmox-auto-installer/src/fetch_plugins/utils.rs > new file mode 100644 > index 000..82cd3e0 > --- /dev/null > +++ b/proxmox-auto-installer/src/fetch_plugins/utils.rs > @@ -0,0 +1,90 @@ > +use anyhow::{bail, Result}; > +use log::{info, warn}; > +use std::{ > +fs::create_dir_all, > +path::{Path, PathBuf}, > +process::Command, > +}; > + > +/// Searches for upper and lower case existence of the partlabel in the > search_path > +/// > +/// # Arguemnts > +/// * `partlabel_lower` - Partition Label in lower case > +/// * `search_path` - Path where to search for the partiiton label > +/// search_path: String Stray last line? > +pub fn scan_partlabels(partlabel_lower: , search_path: ) -> > Result { > +let partlabel = partlabel_lower.to_uppercase(); > +let path = Path::new(search_path).join(partlabel.clone()); ^ Can be `` > +match path.try_exists() { > +Ok(true) => { > +info!("Found partition with label '{}'", partlabel); > +return Ok(path); > +} > +Ok(false) => info!("Did not detect partition with label '{}'", > partlabel), > +Err(err) => info!("Encountered issue, accessing '{}': {}", > path.display(), err), > +} > + > +let partlabel = partlabel_lower.to_lowercase(); Since you explicitly convert it to lowercase anyway here, I'd say name `partlabel_lower` does not make much sense (anymore)? > +let path = Path::new(search_path).join(partlabel.clone()); ^ Same here > +match path.try_exists() { > +Ok(true) => { > +info!("Found partition with label '{}'", partlabel); > +return Ok(path); > +} > +Ok(false) => info!("Did not detect partition with label '{}'", > partlabel), > +Err(err) => info!("Encountered issue, accessing '{}': {}", > path.display(), err), > +} > +bail!( > +"Could not detect upper or lower case labels for '{}'", > +partlabel_lower > +); > +} > + [..] > +/// Tries to unmount the specified path. Will warn on errors, but not fail. > +/// > +/// # Arguemnts > +/// * `target_path` - path to unmount > +pub fn umount_part(target_path: ) -> Result<()> { > +info!("Unmounting partitiona at {target_path}"); ^^ typo > +match Command::new("umount").arg(target_path).output() { > +Ok(output) => { > +if output.status.success() { > +Ok(()) > +} else { > +warn!("Error unmounting: {}", > String::from_utf8(output.stderr)?); > +Ok(()) > +} > +} > +Err(err) => { > +warn!("Error unmounting: {}", err); > +Ok(()) > +} > +} > +} ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v3 27/30] common: add deserializer for FsType
On Thu, Mar 28, 2024 at 02:50:25PM +0100, Aaron Lauterer wrote: > Signed-off-by: Aaron Lauterer > --- [..] > diff --git a/proxmox-installer-common/src/setup.rs > b/proxmox-installer-common/src/setup.rs > index 8432a2c..743e7a9 100644 > --- a/proxmox-installer-common/src/setup.rs > +++ b/proxmox-installer-common/src/setup.rs [..] > @@ -471,3 +472,39 @@ where > > serializer.collect_str(value) > } > + > +pub fn deserialize_fs_type<'de, D>(deserializer: D) -> Result D::Error> > +where > +D: Deserializer<'de>, > +{ > +let de_fs: String = Deserialize::deserialize(deserializer)?; > + > +let fs; > +let mut raid: Option = None; > +let re_raid = Regex::new(r"^(?P.*) > \((?P.*)\)$").map_err(de::Error::custom)?; > +match re_raid.captures(de_fs.as_str()) { > +Some(caps) => { > +fs = caps.name("fs").unwrap().as_str().to_lowercase(); > +raid = Some(caps.name("raid").unwrap().as_str().to_lowercase()); > +}, > +None => fs = de_fs, > +} (nit I guess?) Instead of using a regex here (which also bloats the binary considerably), I'd IMO prefer a dumb `match` like in serialize_fstype() above. Would be much easier to read & reason about, at least. (and probably less code in total as well) > + > +match fs { > +t if t == "zfs" => { > +let raidlevel: ZfsRaidLevel = > Deserialize::deserialize(serde_json::Value::String(raid.unwrap())) > +.map_err(de::Error::custom)?; > +Ok(FsType::Zfs(raidlevel)) > +} > +t if t == "btrfs" => { > +let raidlevel: BtrfsRaidLevel = > Deserialize::deserialize(serde_json::Value::String(raid.unwrap())) > +.map_err(de::Error::custom)?; > +Ok(FsType::Btrfs(raidlevel)) > +} > +t => { > +let fstype: FsType = > Deserialize::deserialize(serde_json::Value::String(t)) > +.map_err(de::Error::custom)?; > +Ok(fstype) > +} > +} > +} > -- > 2.39.2 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v3 09/30] auto-installer: add answer file definition
Mostly just some comments regarding the struct (member) definitions, to make them (and their accompanying) checks a bit simpler. On Thu, Mar 28, 2024 at 02:50:07PM +0100, Aaron Lauterer wrote: > Signed-off-by: Aaron Lauterer > --- [..] > diff --git a/proxmox-auto-installer/src/answer.rs > b/proxmox-auto-installer/src/answer.rs > new file mode 100644 > index 000..96e5608 > --- /dev/null > +++ b/proxmox-auto-installer/src/answer.rs > @@ -0,0 +1,256 @@ [..] > +#[derive(Clone, Deserialize, Debug)] > +pub struct Global { > +pub country: String, > +pub fqdn: Fqdn, > +pub keyboard: String, > +pub mailto: String, > +pub timezone: String, > +pub password: String, > +pub pre_command: Option>, > +pub post_command: Option>, > +pub reboot_on_error: Option, How about using #[serde(default)] pub reboot_on_error: bool, here? Would make checks using this a bit simpler as well. > +} > + > +#[derive(Clone, Deserialize, Debug)] > +struct NetworkInAnswer { > +pub use_dhcp: Option, Same here as for `reboot_on_error`. > +pub cidr: Option, > +pub dns: Option, > +pub gateway: Option, > +// use BTreeMap to have keys sorted Since this comment appears multiple times in this, how about moving this into a module-level comment, explaining it there with a full sentence? > +pub filter: Option>, > +} > + [..] > + > +#[derive(Clone, Debug)] > +pub enum NetworkSettings { > +Dhcp(bool), `Dhcp` as variant without the `bool` should work too. At least nothing seems to exlicitly match on this variant from a quick grep. > +Manual(NetworkManual), > +} > + > +#[derive(Clone, Debug)] > +pub struct NetworkManual { > +pub cidr: CidrAddress, > +pub dns: IpAddr, > +pub gateway: IpAddr, > +// use BTreeMap to have keys sorted > +pub filter: BTreeMap, > +} > + > +#[derive(Clone, Deserialize, Debug)] > +pub struct DiskSetup { > +pub filesystem: Filesystem, > +pub disk_list: Option>, Could this be a #[serde(default)] pub disk_list: Vec, as well? Both a missing & an empty list are invalid, right? > +// use BTreeMap to have keys sorted > +pub filter: Option>, > +pub filter_match: Option, > +pub zfs: Option, > +pub lvm: Option, > +pub btrfs: Option, > +} > + [..] > + > +impl TryFrom for Disks { > +type Error = &'static str; > + > +fn try_from(source: DiskSetup) -> Result { > +if source.disk_list.is_none() && source.filter.is_none() { > +return Err("Need either 'disk_list' or 'filter' set"); > +} > +if source.disk_list.clone().is_some_and(|v| v.is_empty()) { nit: .as_ref() works here as well and would avoid a allocation. > +return Err("'disk_list' cannot be empty"); > +} > +if source.disk_list.is_some() && source.filter.is_some() { > +return Err("Cannot use both, 'disk_list' and 'filter'"); > +} > + > +let disk_selection = match source.disk_list { > +Some(disk_list) => DiskSelection::Selection(disk_list), > +None => DiskSelection::Filter(source.filter.unwrap()), > +}; > + > +// TODO: improve checks for foreign FS options. E.g. less verbose > and handling new FS types > +// automatically > +let fs_options; > +let fs = match source.filesystem { This could be a let (fs, fs_options) = match source.filesystem { .. > +Filesystem::Xfs => { > +if source.zfs.is_some() || source.btrfs.is_some() { > +return Err("make sure only 'lvm' options are set"); > +} > +fs_options = > FsOptions::LVM(source.lvm.unwrap_or(LvmOptions::default())); > +FsType::Xfs .. and then here instead (FsType::Xfs, FsOptions::LVM(source.lvm.unwrap_or(LvmOptions::default( as well for all the below cases, of course. > +} > +Filesystem::Ext4 => { > +if source.zfs.is_some() || source.btrfs.is_some() { > +return Err("make sure only 'lvm' options are set"); > +} > +fs_options = > FsOptions::LVM(source.lvm.unwrap_or(LvmOptions::default())); > +FsType::Ext4 > +} > +Filesystem::Zfs => { > +if source.lvm.is_some() || source.btrfs.is_some() { > +return Err("make sure only 'zfs' options are set"); > +} > +if source.zfs.is_none() || source.zfs.is_some_and(|v| > v.raid.is_none()) { > +return Err("ZFS raid level 'zfs.raid' must be set"); > +} > +fs_options = FsOptions::ZFS(source.zfs.unwrap()); > +FsType::Zfs(source.zfs.unwrap().raid.unwrap()) > +} > +Filesystem::Btrfs => { > +if source.zfs.is_some() || source.lvm.is_some() { > +
[pve-devel] [PATCH docs] installation: fix volume group name
This slipped through while taking the wording from PMG. Fixes: 67d2d94 ("installation: lvm-options: improve & align wording with pmg-docs") Signed-off-by: Christoph Heiss --- Probably have been staring at the docs too long to not notice this earlier .. pve-installation.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pve-installation.adoc b/pve-installation.adoc index 72cbc39..274d9ad 100644 --- a/pve-installation.adoc +++ b/pve-installation.adoc @@ -293,7 +293,7 @@ configuration will be adapted accordingly. `minfree`:: Defines the amount of free space that should be left in the LVM volume group -`pmg`. With more than 128GB storage available, the default is 16GB, otherwise +`pve`. With more than 128GB storage available, the default is 16GB, otherwise `hdsize/8` will be used. + NOTE: LVM requires free space in the VG for snapshot creation (not required for -- 2.43.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] run env: use default error message if country detection failed with empty string
Bit of perl fun again. $err from detect_country_tracing_to() can be empty string under certain circumstances (according to a forum post [0]). The // operator evaluates an empty as true, thus `warn` receives an empty string to and just prints Warning: something wrong at /usr/share/perl5/proxmox/Install/RunEnv.pm line 305 Which isn't particular helpful. Use the || operator instead, that evaluates an empty string as false and thus would fall back to the generic error message. A minimal reproducer/example for completeness sake: #!/usr/bin/env perl use strict; use warnings; warn ('' // "unable to detect country\n"); warn ('' || "unable to detect country\n"); gives Warning: something's wrong at ./test.pl line 5. unable to detect country [0] https://forum.proxmox.com/threads/blank-screen-while-installing.143928/ Signed-off-by: Christoph Heiss --- Proxmox/Install/RunEnv.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm index 25b6bb3..39505d0 100644 --- a/Proxmox/Install/RunEnv.pm +++ b/Proxmox/Install/RunEnv.pm @@ -302,7 +302,7 @@ sub query_installation_environment : prototype() { if (defined($country)) { $output->{country} = $country; } else { - warn ($err // "unable to detect country\n"); + warn ($err || "unable to detect country\n"); } return $output; -- 2.43.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs] installation: update link to installing on top of Debian to bookworm version
Seems this just was forgotten, Buster is quite old at this point. Signed-off-by: Christoph Heiss --- pve-installation.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pve-installation.adoc b/pve-installation.adoc index d123cd4..2c034d6 100644 --- a/pve-installation.adoc +++ b/pve-installation.adoc @@ -401,7 +401,7 @@ See Also * link:/wiki/Prepare_Installation_Media[Prepare Installation Media] -* link:/wiki/Install_Proxmox_VE_on_Debian_Buster[Install Proxmox VE on Debian Buster] +* link:/wiki/Install_Proxmox_VE_on_Debian_12_Bookworm[Install Proxmox VE on Debian 12 Bookworm] * link:/wiki/System_Requirements[System Requirements] -- 2.43.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v3 1/5] package-repos: improve wording partly based on pmg-docs
Some sentences are phrased better and more expansive in pmg-docs, so take them from there and adapt them as needed. Signed-off-by: Christoph Heiss --- Changes v1 -> v2: * deduplicate email/changelog paragraph, move to top * deduplicate enterprise repo introduction sentence * very slightly reword some phrases Changes v2 -> v3: * fixed small typo (thanks, Sterzy!) pve-package-repos.adoc | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/pve-package-repos.adoc b/pve-package-repos.adoc index b0c2a95..065323c 100644 --- a/pve-package-repos.adoc +++ b/pve-package-repos.adoc @@ -8,6 +8,10 @@ endif::wiki[] {pve} uses http://en.wikipedia.org/wiki/Advanced_Packaging_Tool[APT] as its package management tool like any other Debian-based system. +{pve} automatically checks for package updates on a daily basis. The `root@pam` +user is notified via email about available updates. From the GUI, the +'Changelog' button can be used to see more details about an selected update. + Repositories in {pve} ~ @@ -57,21 +61,18 @@ deb http://security.debian.org/debian-security bookworm-security main contrib {pve} Enterprise Repository ~~~ -This is the default, stable, and recommended repository, available for all {pve} -subscription users. It contains the most stable packages and is suitable for -production use. The `pve-enterprise` repository is enabled by default: +This is the recommended repository and available for all {pve} subscription +users. It contains the most stable packages and is suitable for production use. +The `pve-enterprise` repository is enabled by default: .File `/etc/apt/sources.list.d/pve-enterprise.list` deb https://enterprise.proxmox.com/debian/pve bookworm pve-enterprise -The `root@pam` user is notified via email about available updates. Click the -'Changelog' button in the GUI to see more details about the selected update. - -You need a valid subscription key to access the `pve-enterprise` repository. -Different support levels are available. Further details can be found at -{pricing-url}. +Please note that you need a valid subscription key to access the +`pve-enterprise` repository. We offer different support levels, which you can +find further details about at {pricing-url}. NOTE: You can disable this repository by commenting out the above line using a `#` (at the start of the line). This prevents error messages if your host does @@ -82,9 +83,10 @@ repository in that case. {pve} No-Subscription Repository -This is the recommended repository for testing and non-production use. Its -packages are not as heavily tested and validated. You don't need a subscription key -to access the `pve-no-subscription` repository. +As the name suggests, you do not need a subscription key to access +this repository. It can be used for testing and non-production +use. It's not recommended to use this on production servers, as these +packages are not always as heavily tested and validated. We recommend to configure this repository in `/etc/apt/sources.list`. -- 2.43.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v3 4/5] installation: zfs-options: improve & align wording with pmg-docs
These changes are the result of basically "diffing" both documentations, choosing the better prased/sounding sections. Some wording were also slightly changed as necessary to further improve them. The equivalent changes will be done for pmg-docs too, to ensure they are really the same in the end. Signed-off-by: Christoph Heiss --- Changes v1 -> v2: * none Changes v2 -> v3: * none pve-installation.adoc | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pve-installation.adoc b/pve-installation.adoc index eaa800f..f4a898b 100644 --- a/pve-installation.adoc +++ b/pve-installation.adoc @@ -294,10 +294,10 @@ lvmthin snapshots). [[advanced_zfs_options]] Advanced ZFS Configuration Options ~~ -The installer creates the ZFS pool `rpool`. No swap space is created but you can -reserve some unpartitioned space on the install disks for swap. You can also -create a swap zvol after the installation, although this can lead to problems. -(see <>). +The installer creates the ZFS pool `rpool`, if ZFS is used. No swap space is +created but you can reserve some unpartitioned space on the install disks for +swap. You can also create a swap zvol after the installation, although this can +lead to problems (see xref:zfs_swap[ZFS swap notes]). `ashift`:: -- 2.43.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel