]> git.proxmox.com Git - pve-installer.git/log
pve-installer.git
8 months agosys/command: double wait frequency and send SIGKILL once after 0.5s
Thomas Lamprecht [Mon, 26 Feb 2024 19:40:14 +0000 (20:40 +0100)]
sys/command: double wait frequency and send SIGKILL once after 0.5s

100 ms is quite plenty, while we would be better of using a event
based wait, i.e., dropping the WNOHANG, that would also mean handling
the time out via alarm, EINTR checking and quite a bit other stuff
making this more convoluted, so for now just go faster..

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
8 months agosys: command: wait for process exit with sub-second granularity
Christoph Heiss [Mon, 26 Feb 2024 16:50:43 +0000 (17:50 +0100)]
sys: command: wait for process exit with sub-second granularity

Using full seconds as a granularity for sleeping between waitpid()'s is
way too much and unnecessarily slows down the installation a lot. Most
processes take a few moments after closing their stdin/stdout to
actually exit fully, which means that we would sleep a second in most
cases.

Lower it to 0.1 second, which immensely improves the situation.

Some values for comparison; tui-installer on the same bog-standard
2-core, SeaBIOS, ext4, virtio VM (roughly averaged over multiple runs):

  * 8.0 ISO (baseline): ~2:30 min
  * w/o patch: ~9:00 min
  * w/  patch: ~2:30 min

Values measured are from pressing the 'Install' button until the
autoreboot dialog (aka. install finished) popped up.

Fixes: 152bbef ("sys: command: factor out kill() + waitpid() from run_command()")
Reported-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Reported-by: Filip Schauer <f.schauer@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agobump version to 8.1.9
Thomas Lamprecht [Mon, 26 Feb 2024 14:11:49 +0000 (15:11 +0100)]
bump version to 8.1.9

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
8 months agorun command: use explicit return undef in closures on call sites
Thomas Lamprecht [Mon, 26 Feb 2024 13:38:11 +0000 (14:38 +0100)]
run command: use explicit return undef in closures on call sites

To avoid a misinterpretation of the auto-return value:

> In the absence of an explicit return, a subroutine, eval, or do FILE
> automatically returns the value of the last expression evaluated.
-- https://perldoc.perl.org/functions/return

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
8 months agorun command: avoid using 1 as special value
Thomas Lamprecht [Mon, 26 Feb 2024 13:34:09 +0000 (14:34 +0100)]
run command: avoid using 1 as special value

In Perl, the last expression of a block (e.g. of a method, eval) gets
returned if there's no explicit return statement. Quite often that is
truthy, i.e., 1.

As that was chosen as the special value for the CMD_FINISHED flag it
had quite a few false positives, causing weird effects and
installation failure.

Reserve that overly problematic value and chose 2 as new CMD_FINISHED
value, albeit it could be better to signal this even more explicitly,
like with a structured hash reference, but for now this is a good stop
gap.

Fixes: 23c5fbe ("sys: command: allow terminating the process early from log subroutine")
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
8 months agobump version to 8.1.8
Thomas Lamprecht [Sat, 24 Feb 2024 15:30:12 +0000 (16:30 +0100)]
bump version to 8.1.8

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
8 months agotests: code-style and error handling fixes for ui2stdio
Thomas Lamprecht [Sat, 24 Feb 2024 17:17:10 +0000 (18:17 +0100)]
tests: code-style and error handling fixes for ui2stdio

use modern calling style, avoid duplicate use of Test::More module,
handle fork error more visible, handle pipe creation errors and do
that all in one commit as it's just a test and I don't care.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
8 months agotui: install progress: add tests for UI^2 stdio protocol
Christoph Heiss [Wed, 6 Dec 2023 11:34:55 +0000 (12:34 +0100)]
tui: install progress: add tests for UI^2 stdio protocol

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agobuildsys: setup proper test environment for testsuite
Christoph Heiss [Wed, 6 Dec 2023 11:34:54 +0000 (12:34 +0100)]
buildsys: setup proper test environment for testsuite

Some test to come will need a proper environment.

`prepare-test-env` can also be generally useful while developing to
quickly set up a new test environment as needed.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agotest: add tests for UI^2 stdio protocol
Christoph Heiss [Wed, 6 Dec 2023 11:34:53 +0000 (12:34 +0100)]
test: add tests for UI^2 stdio protocol

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agostdio connected UI: drop perl prototype definitions
Thomas Lamprecht [Sat, 24 Feb 2024 16:56:26 +0000 (17:56 +0100)]
stdio connected UI: drop perl prototype definitions

The prototypes where completely circumvented by calling those two
methods by reference via &, and that probably happened as the send_msg
one was just wrong, it forced scalar context for the second parameter,
while that was a list (or well hash, but the difference there can be
blurry).

Anyhow, prototypes are not always of help, and can be a PITA with
side-effects too, and especially for such small modules it has not
that much use to declare them for privately-scoped methods, so just
drop them and fix the calling style.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
8 months agotui, ui: switch over to JSON-based protocol
Christoph Heiss [Wed, 6 Dec 2023 11:34:52 +0000 (12:34 +0100)]
tui, ui: switch over to JSON-based protocol

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agofqdn comparison: make more efficient
Thomas Lamprecht [Fri, 23 Feb 2024 16:22:18 +0000 (17:22 +0100)]
fqdn comparison: make more efficient

Compare lazily to always avoid to vector collections and if one of the
first parts mismatch some lower_case calls.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
8 months agofqdn comparison: expand test scope
Thomas Lamprecht [Fri, 23 Feb 2024 16:19:15 +0000 (17:19 +0100)]
fqdn comparison: expand test scope

Add some negative tests to ensure a `return true` (exaggerated)
refactoring won't pass the suite, and add one test where a and b is
the same, just to be sure.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
8 months agofix #5230: sys: net: properly escape FQDN regex
Christoph Heiss [Thu, 15 Feb 2024 12:39:38 +0000 (13:39 +0100)]
fix #5230: sys: net: properly escape FQDN regex

Due to interpolation, the \. sequence must be double-escaped.
Previously, this would result in a non-escaped dot, thus matching much
more liberally than it should.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
 [ TL: fix bug # reference in code comments ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
8 months agosys: net: do not allow overlong FQDNs as per RFCs and Debian spec
Christoph Heiss [Thu, 15 Feb 2024 12:39:37 +0000 (13:39 +0100)]
sys: net: do not allow overlong FQDNs as per RFCs and Debian spec

Debian limits labels to 63 characters each and the total length to 253
characters [0].

[0] https://manpages.debian.org/stable/manpages/hostname.7.en.html

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agoproxinstall: avoid open-coding FQDN sanity check
Christoph Heiss [Thu, 15 Feb 2024 12:39:36 +0000 (13:39 +0100)]
proxinstall: avoid open-coding FQDN sanity check

.. by moving it into its own subroutine. Makes the whole thing quite a
bit neater and easier to maintain.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agocommon: fqdn: implement case-insensitive comparison as per RFC 952
Christoph Heiss [Thu, 15 Feb 2024 12:39:35 +0000 (13:39 +0100)]
common: fqdn: implement case-insensitive comparison as per RFC 952

Multiple DNS-related RFCs (notably RFC 952, RFC 1035 and RFC 4343)
reinforce that FQDN must not be case-sensitive.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agocommon: fqdn: do not allow overlong FQDNs as per Debian spec
Christoph Heiss [Thu, 15 Feb 2024 12:39:34 +0000 (13:39 +0100)]
common: fqdn: do not allow overlong FQDNs as per Debian spec

Debian limits labels to 63 characters each and the total length to 253
characters [0].

While at it, reference all the RFCs that apply when parsing FQDNs.

[0] https://manpages.debian.org/stable/manpages/hostname.7.en.html

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agosys: wait a second after sending TERM signal before going for KILL
Thomas Lamprecht [Fri, 23 Feb 2024 14:47:38 +0000 (15:47 +0100)]
sys: wait a second after sending TERM signal before going for KILL

Graceful process termination can need a bit of time, so wait one
second between sending the (catchable) TERM signal and the
(uncatchable) KILL one.

Makes the code shorter as a side benefit.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
8 months agofix #4872: run env: use run_command() for country detection
Christoph Heiss [Tue, 13 Feb 2024 15:14:03 +0000 (16:14 +0100)]
fix #4872: run env: use run_command() for country detection

This fixes a rather longstanding issue [0][1] with the country
detection, in that it might get completely stuck and thus hangs the
installation.

This is due how Perl, signals and line reading interacts.

A minimal reproducer, how the installer currently works, looks like
this:
```
    #!/usr/bin/env perl

    use strict;
    use warnings;

    open (my $fh, '-|', 'sleep', '1000') or die;

    my $prev = alarm(2);
    eval {
local $SIG{ALRM} = sub { die "timed out!\n" };

my $line;
while (defined ($line = <$fh>)) {
    print "line: $line";
}
    };

    alarm($prev);
    close($fh);
```

One might expect that this times out after 2 seconds, as specified in
`alarm(2)`. The thruth is that `$line = <$fh>` apparently prevents the
signal to go through. This then causes the installer to hang there
indefinitely, if `traceroute` never progresses - which seems to happen
on lots of (weird) networks, as evidently can be seen in the forum [1].

Proxmox::Sys::Command::run_command() handles of these weird cases, takes
care of the nitty-gritty details and - most importantly - interacts
properly with SIGALRM, so just use that instead.

This _should_ really fix that issue, but reproducing it 1:1 as part of
the installation process is _very_ hard, basically pure luck. But
rewriting the reproducer using run_command (in the exact same way that
this patch rewrites detect_country_tracing_to()) fixes the issue there,
so it's the best we can probably do.

NB: This causes that the traceroute command is now printed to the log
(as run_command() logs that by default), which we could also hide e.g.
through another parameter if wanted.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=4872
[1] https://forum.proxmox.com/threads/proxmox-installation-trying-to-detect-country.134301/

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agosys: command: add option to not print process output to stdout
Christoph Heiss [Tue, 13 Feb 2024 15:14:02 +0000 (16:14 +0100)]
sys: command: add option to not print process output to stdout

If $noprint is set, the output of the command won't be printed to stdout
of the parent process.

Fully backwards-compatible again, only takes effect if the new argument
is actually specified.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agosys: command: allow terminating the process early from log subroutine
Christoph Heiss [Tue, 13 Feb 2024 15:14:01 +0000 (16:14 +0100)]
sys: command: allow terminating the process early from log subroutine

If the logging subroutine $func returns CMD_FINISHED after processing a
line, the running subprocess is killed early.
This mechanism can be used when e.g. only a certain part of the output
of a (long-running) command is needed, avoiding the extra time it would
take the command to finish properly.

This is done in a entirely backwards-compatible way, i.e. existing
usages don't need any modification.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agosys: command: handle EINTR in run_command()
Christoph Heiss [Tue, 13 Feb 2024 15:14:00 +0000 (16:14 +0100)]
sys: command: handle EINTR in run_command()

Previously, the I/O loop would continue endlessly until the subprocess
exited.
This explicit handling allows run_command() to be used with e.g.
alarm().

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agosys: command: factor out kill() + waitpid() from run_command()
Christoph Heiss [Tue, 13 Feb 2024 15:13:59 +0000 (16:13 +0100)]
sys: command: factor out kill() + waitpid() from run_command()

This moves the kill() + waitpid() combo into a separate subroutine,
avoiding open-coding that sequence. wait_for_process() also handles
properly unkillable process (e.g. in D-state) and avoids completely
locking up the installer in such cases. See [0].

For the latter case, a timeout exists (with a default of 5 seconds) in
which to wait for the process to exit after sending an optional
TERM/KILL signal.

Also while at it, add a few basic tests for run_command().

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

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agolow-level: initialize UI backend for 'dump-env' subcommand too
Christoph Heiss [Tue, 13 Feb 2024 15:13:58 +0000 (16:13 +0100)]
low-level: initialize UI backend for 'dump-env' subcommand too

Some detection routines might try to log things and call some
Proxmox::Ui functions all the way down, so just initialize it with the
stdio backend to avoid errors.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
8 months agogui: remove trailing spaces and colons
Maximiliano Sandoval [Tue, 21 Nov 2023 15:10:25 +0000 (16:10 +0100)]
gui: remove trailing spaces and colons

For consistency sake, all colons and trailing spaces in labels that were
followed with an entry were removed, this matches other panels such as
the password and country/timezone panels.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Tested-by: Lukas Wagner <l.wagner@proxmox.com>
8 months agogui: expand ip address Gtk3::Entry
Maximiliano Sandoval [Tue, 21 Nov 2023 15:10:20 +0000 (16:10 +0100)]
gui: expand ip address Gtk3::Entry

This accounts for the different layout set in the previous commit
9102da7 ("gui: use basic grid in the network panel")

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Tested-by: Lukas Wagner <l.wagner@proxmox.com>
8 months agogui: use basic grid in the network panel
Maximiliano Sandoval [Tue, 21 Nov 2023 15:10:24 +0000 (16:10 +0100)]
gui: use basic grid in the network panel

Using boxes causes the labels to not align correctly in certain
circumstances. In the following commits we replace the use of boxes with
grids and set the margins and spacing directly on the respective grid.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Tested-by: Lukas Wagner <l.wagner@proxmox.com>
8 months agogui: change margins in create_basic_grid
Maximiliano Sandoval [Tue, 21 Nov 2023 15:10:23 +0000 (16:10 +0100)]
gui: change margins in create_basic_grid

Previously the grids were inserted in a succession of boxes each with
its own set of margins and spacing. We define the margins now
exclusively in the grid and account for previous values.

Note that we match the top and bottom margins of the 'Target Harddisk'
panel which does not need to use a grid.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Tested-by: Lukas Wagner <l.wagner@proxmox.com>
8 months agogui: use basic grid in country/timezone panel
Maximiliano Sandoval [Tue, 21 Nov 2023 15:10:22 +0000 (16:10 +0100)]
gui: use basic grid in country/timezone panel

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Tested-by: Lukas Wagner <l.wagner@proxmox.com>
8 months agogui: use basic grid in password panel
Maximiliano Sandoval [Tue, 21 Nov 2023 15:10:21 +0000 (16:10 +0100)]
gui: use basic grid in password panel

The extra 10px margin on the email row was added to account for the
removed line:

    $vbox->pack_start($hbox3, 0, 0, 15);

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Tested-by: Lukas Wagner <l.wagner@proxmox.com>
8 months agogui: move create_basic_grid subroutine definition up
Maximiliano Sandoval [Tue, 21 Nov 2023 15:10:19 +0000 (16:10 +0100)]
gui: move create_basic_grid subroutine definition up

This will be used in future commits to create grids so we need it to be defined.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Tested-by: Tested-by: Lukas Wagner <l.wagner@proxmox.com>
9 months agobuildsys: handle installing multiple executables to /usr/bin
Aaron Lauterer [Tue, 23 Jan 2024 17:00:39 +0000 (18:00 +0100)]
buildsys: handle installing multiple executables to /usr/bin

Otherwise the build will fail once we define more than one USR_BIN
file.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
9 months agoui: stdio: log error if display_html() is called on stdio backend
Christoph Heiss [Wed, 6 Dec 2023 11:34:51 +0000 (12:34 +0100)]
ui: stdio: log error if display_html() is called on stdio backend

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
9 months agolow-level: align wording of finish message
Christoph Heiss [Wed, 6 Dec 2023 11:34:50 +0000 (12:34 +0100)]
low-level: align wording of finish message

The other case uses "Installation finished [..]", thus use the same
wording here too.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
9 months agoproxinstall, common: remove "off" as zfs checksum option
Christoph Heiss [Tue, 6 Feb 2024 13:12:26 +0000 (14:12 +0100)]
proxinstall, common: remove "off" as zfs checksum option

See also the thread at [0] for the initial discussion/idea.

Disabling checksums is considered an "extraordinarily bad idea" [1] (for
pretty obvious reason) and nobody should really ever use it.

Thus remove the option completely; just so that users cannot simply
disable checksum "for performance reasons" without knowing about the
implications of this.

As pointed out by Thomas, it can still be set to "off" after the
installation using the `zfs` tool, if really wanted.

[0] https://lists.proxmox.com/pipermail/pve-devel/2023-December/061188.html
[1] https://openzfs.github.io/openzfs-docs/Basic%20Concepts/Checksums.html#disabling-checksums

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
9 months agoproxinstall, common: remove deprecated fletcher2 as zfs checksum algorithm
Christoph Heiss [Fri, 22 Dec 2023 10:52:24 +0000 (11:52 +0100)]
proxinstall, common: remove deprecated fletcher2 as zfs checksum algorithm

Fletcher-2 has long been deprecated and should not be used anymore
[0][1], so we probably should not offer it anymore too. It's been
deprecated since at least over 3 years, beyond that it's hard to find
an exact date.

[0]: https://openzfs.github.io/openzfs-docs/Basic%20Concepts/Checksums.html#checksum-algorithms
[1]: https://people.freebsd.org/~asomers/fletcher.pdf

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agobump version to 8.1.7
Thomas Lamprecht [Thu, 23 Nov 2023 10:11:27 +0000 (11:11 +0100)]
bump version to 8.1.7

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agosource interfaces.d snippets by default
Thomas Lamprecht [Thu, 23 Nov 2023 10:11:01 +0000 (11:11 +0100)]
source interfaces.d snippets by default

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agobump version to 8.1.6
Thomas Lamprecht [Wed, 22 Nov 2023 12:30:13 +0000 (13:30 +0100)]
bump version to 8.1.6

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agogrub: install all efi binaries in fallback/default dir
Fabian Grünbichler [Wed, 22 Nov 2023 12:19:27 +0000 (13:19 +0100)]
grub: install all efi binaries in fallback/default dir

else this fails with secureboot, where the entry point must be shim and not
grub.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
12 months agobump version to 8.1.5
Thomas Lamprecht [Tue, 21 Nov 2023 13:22:35 +0000 (14:22 +0100)]
bump version to 8.1.5

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agoZFS: detect and handle secure boot
Fabian Grünbichler [Tue, 21 Nov 2023 13:13:19 +0000 (14:13 +0100)]
ZFS: detect and handle secure boot

and switch the ESP to grub if it is enabled.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
12 months agozfs: create dataset var-lib-vz for /var/lib/vz
Stoiko Ivanov [Tue, 21 Nov 2023 13:11:18 +0000 (14:11 +0100)]
zfs: create dataset var-lib-vz for /var/lib/vz

Creating rpool/var/lib/vz and all intermediate datasets causes a
service-failure of `var.mount` upon shutdown.

creating the dataset for /var/lib/vz directly at the rpool and setting
its mountpoint property seems the most robust way to address this.

The alternative approach of setting `canmount=off` on the `var`
dataset seems a bit dangerous (users setting a zfs property and
suddenly hiding their /var contents).

The only small downside to this approach is that the setting of the
mountpoint happens quite a bit after extracting the data - but this
would probably be better addressed with a refactoring of the
lowlevel-installer code (setting the zfs-pool up under /target and
getting rid of a few special cases)

Fixes: dd19d40ceac179ba18652f1d6c3e4c23f246af00
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
12 months agoserial installer: add serial config for grub to target system
Stoiko Ivanov [Tue, 21 Nov 2023 11:09:59 +0000 (12:09 +0100)]
serial installer: add serial config for grub to target system

Matching if a serial will be needed for grub is based on the target
commandline - the speed is also read from there. The unit is based
on the ttyS device - although I'd assume that this might not always
match up.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
12 months agocommon: enforce even number of disks for ZFS RAID-10
Christoph Heiss [Tue, 21 Nov 2023 10:45:51 +0000 (11:45 +0100)]
common: enforce even number of disks for ZFS RAID-10

An uneven number of disks otherwise causes a panic due to an
out-of-bounds array access in the loop below.

Reported-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: add missing argument for low-level installer test-session
Christoph Heiss [Tue, 21 Nov 2023 10:45:50 +0000 (11:45 +0100)]
tui: add missing argument for low-level installer test-session

This broke running the TUI installer in debug mode, does not effect
release builds in any way.

Fixes: 4b4dfa1 ("low level: testmode: take path to disk image instead of using /dev/null")
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: preserve autoreboot checkbox state when switching views
Christoph Heiss [Tue, 21 Nov 2023 10:45:49 +0000 (11:45 +0100)]
tui: preserve autoreboot checkbox state when switching views

Instead of reading the checkbox when continuing to the next screen, save
its toggle status to the installer state instead on change.

Reported-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: do not center EULA text
Christoph Heiss [Tue, 21 Nov 2023 10:45:48 +0000 (11:45 +0100)]
tui: do not center EULA text

Brings it in line with the GUI installer.

Reported-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agobump version to 8.1.4
Thomas Lamprecht [Fri, 17 Nov 2023 18:35:23 +0000 (19:35 +0100)]
bump version to 8.1.4

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agotui: fix interface sort order
Stoiko Ivanov [Fri, 17 Nov 2023 17:30:23 +0000 (18:30 +0100)]
tui: fix interface sort order

Currently, when multiple NICs are present in a system the TUI
sometimes selects the wrong interface (not the one that has the
default gateway/dhcp lease)

I assume this is due to HashMap's values yielding an iterator in
arbitrary order

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
 [ TL: avoid intermediate vector, reuse the SelectView's iter()]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agotui: bootdisk zfs config: add a maximum value to the `copies` option
Stefan Sterz [Fri, 17 Nov 2023 16:32:18 +0000 (17:32 +0100)]
tui: bootdisk zfs config: add a maximum value to the `copies` option

according to `man zfsprops` the copies option can only be 1, 2, or 3.
limit the field to 3 just like we do for the GTK based UI, as setting
higher options can't work anyway.

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
 [ TL: fleece in note that we already limit this in the GTK UI ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agotui: install progress: use ok/cancel as button text for installer prompt
Christoph Heiss [Fri, 17 Nov 2023 13:45:27 +0000 (14:45 +0100)]
tui: install progress: use ok/cancel as button text for installer prompt

The GTK installer/UI module in the low-level installer does the same.
Messages used with this are worded for this, using yes/no instead can be
quite confusing (e.g.
Proxmox::Install::ask_existing_vg_rename_or_abort())

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agoui: stdio: replace newlines with whitespaces in prompt messages
Christoph Heiss [Fri, 17 Nov 2023 13:45:26 +0000 (14:45 +0100)]
ui: stdio: replace newlines with whitespaces in prompt messages

The line-based protocol currently used cannot handle this properly, so
introduce this as a stop-gap measure - otherwise messages might be cut
off.

This makes it work for now, and the text is wrapped correctely for the
screen width in the TUI anyway - which is the only user of this so far.

Will be reworked properly later on.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
 [ TL: add fix-me comment ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agotui: fix changing between non-LVM and LVM filesystem in bootdisk chooser
Christoph Heiss [Fri, 17 Nov 2023 12:12:18 +0000 (13:12 +0100)]
tui: fix changing between non-LVM and LVM filesystem in bootdisk chooser

Happens due to a force-unwrap() under the false assumption that the
disk for LVM configurations always exists when switching to a LVM
filesystem.
This fails spectacularly with a panic when switching from e.g. Btrfs to
ext4 in the filesystem chooser.

Fixes: eda9fa0 ("fix #4856: tui: bootdisk: use correct defaults in advanced dialog")
Reported-by: Christian Ebner <c.ebner@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agobump version to 8.1.3
Thomas Lamprecht [Fri, 17 Nov 2023 06:39:27 +0000 (07:39 +0100)]
bump version to 8.1.3

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agogtk: show the unit for disk related options
Thomas Lamprecht [Fri, 17 Nov 2023 06:36:32 +0000 (07:36 +0100)]
gtk: show the unit for disk related options

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agogtk: labeled widget grid: allow adding optional suffix label
Thomas Lamprecht [Fri, 17 Nov 2023 06:35:42 +0000 (07:35 +0100)]
gtk: labeled widget grid: allow adding optional suffix label

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agogtk: rework helper for creating labeled widget grid
Thomas Lamprecht [Fri, 17 Nov 2023 06:01:04 +0000 (07:01 +0100)]
gtk: rework helper for creating labeled widget grid

A array of arrays makes using this now, and possibly extending it in
the future, quite a bit simpler.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agoget fqdn: code/naming style cleanups
Thomas Lamprecht [Fri, 17 Nov 2023 05:34:33 +0000 (06:34 +0100)]
get fqdn: code/naming style cleanups

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agorun env: do not store emtpy hostname
Stoiko Ivanov [Thu, 16 Nov 2023 19:59:47 +0000 (20:59 +0100)]
run env: do not store emtpy hostname

without this patch the hostname ends up as the empty string in
run-env-info.json, which results in a parse-error in the TUI code
(an empty string is not None, but still too short as hostname)

Minimally tested on a VM.

Fixes: bda1cdf ("run env: retrieve and store hostname from DHCP lease
 if available")
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
12 months agobump version to 8.1.2
Thomas Lamprecht [Thu, 16 Nov 2023 18:21:13 +0000 (19:21 +0100)]
bump version to 8.1.2

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agozfs: create /var/lib/vz only for PVE
Stoiko Ivanov [Thu, 16 Nov 2023 17:12:26 +0000 (18:12 +0100)]
zfs: create /var/lib/vz only for PVE

the recent patch to create /var/lib/vz as dedicated dataset, did so
for all our products - but this is only needed/wanted for PVE

moved the creation of the root-dataset above the creation of
rpool/data, so that the pve-specifics can remain in one if block.

Fixes: dd19d40ceac179ba18652f1d6c3e4c23f246af00
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
12 months agox: disable screen saver via blank time
Thomas Lamprecht [Thu, 16 Nov 2023 16:54:37 +0000 (17:54 +0100)]
x: disable screen saver via blank time

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agobump version to 8.1.1
Thomas Lamprecht [Thu, 16 Nov 2023 07:33:40 +0000 (08:33 +0100)]
bump version to 8.1.1

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agozfs: set acltype=posix for root-dataset
Stoiko Ivanov [Thu, 16 Nov 2023 15:00:41 +0000 (16:00 +0100)]
zfs: set acltype=posix for root-dataset

journald as a core component tries setting a ACL on the journal files
for (non-root) users and fails on our ZFS installs.
Resulting in dmesg being spammed with messages from journald upon each
journal-rotation for each user upon their first login.

This is also suggested by OpenZFS in their Debian guide for root on
ZFS:
https://openzfs.github.io/openzfs-docs/Getting%20Started/Debian/Debian%20Bookworm%20Root%20on%20ZFS.html

Tested by setting this on a machine of mine, where this has been
bugging for quite a while.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
12 months agofix #1410: zfs: create /var/lib/vz as separate dataset
Stoiko Ivanov [Thu, 16 Nov 2023 15:00:40 +0000 (16:00 +0100)]
fix #1410: zfs: create /var/lib/vz as separate dataset

this enables the users to set reservations on / separate from
/var/lib/vz - where backups, ISOs, and other data might fill the
complete pool.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
12 months agounconfigured.sh: set serial to a number to prevent warning
Stoiko Ivanov [Thu, 16 Nov 2023 15:00:39 +0000 (16:00 +0100)]
unconfigured.sh: set serial to a number to prevent warning

caught me off-guard while debugging other things - the message:
` [: : integer expression expected`
made me look a bit too long at the wrong place.

Fixes: a31259b1597447a0b431cd5c81a6db2bc80f1ddf
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
12 months agofix #4747: pass kernel cmdline parameters to target system
Stoiko Ivanov [Thu, 16 Nov 2023 15:00:38 +0000 (16:00 +0100)]
fix #4747: pass kernel cmdline parameters to target system

Parameters needed for booting during installation are best preserved
in the target cmdline as well - e.g. if you need a particular
cmdline switch for your system to boot at all - not having to add it
for the first boot of the installed system and manually adding it to
the bootloader config is an improvement.

This additionally enables us to drop the console parameter handling
for serial consoles (it is just one of the parameters to pass along).

Finally it fixes the regular expressions for the installer settings we
read from the cmdline (swapsize, maxroot,...) which were broken if
added as last entry.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
12 months agoserial install: fix console parameter parsing
Stoiko Ivanov [Tue, 14 Nov 2023 17:31:48 +0000 (18:31 +0100)]
serial install: fix console parameter parsing

The regex matching in Proxmox::Install::Config was blindly copied from
above - so the other parameters are also likely to not get recognized
if they are the last on the cmdline

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
12 months agod/control: add libgtk3-perl to B-D
Stoiko Ivanov [Tue, 14 Nov 2023 17:31:47 +0000 (18:31 +0100)]
d/control: add libgtk3-perl to B-D

With the first tests entering the installer in
9a0d66cb36d395a1186904132aed1d5dc33a0937

we now need libgtk3-perl during package-building with `make deb`

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
12 months agounconfigured.sh: set terminal size for serial install
Stoiko Ivanov [Thu, 22 Jun 2023 20:14:45 +0000 (22:14 +0200)]
unconfigured.sh: set terminal size for serial install

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
12 months agopass optional console parameter from installer to target
Stoiko Ivanov [Thu, 22 Jun 2023 20:08:48 +0000 (22:08 +0200)]
pass optional console parameter from installer to target

If an installation needs to provide a dedicated console parameter
(e.g. because it runs on the serial console) the target system most
likely will need the parameter too.

This patch adds the parameter to the kernel-commandline (in case zfs
is used for both grub and systemd)

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
12 months agobump version to 8.1.0
Thomas Lamprecht [Sat, 11 Nov 2023 13:32:42 +0000 (14:32 +0100)]
bump version to 8.1.0

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
12 months agolow-level, tui: count down auto-reboot timeout
Christoph Heiss [Fri, 10 Nov 2023 14:17:26 +0000 (15:17 +0100)]
low-level, tui: count down auto-reboot timeout

The GUI installer already has the same functionality, with this the TUI
installer gains the same. It is a nice touch anyway, primarily to
indicate to the user that the installer is not frozen or similar.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agolow-level: avoid open-coding config reading, parsing and merging
Christoph Heiss [Fri, 10 Nov 2023 14:17:25 +0000 (15:17 +0100)]
low-level: avoid open-coding config reading, parsing and merging

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: install_progress: handle errors in ui message loop more gracefully
Christoph Heiss [Fri, 10 Nov 2023 14:17:24 +0000 (15:17 +0100)]
tui: install_progress: handle errors in ui message loop more gracefully

This at least gives _some_ feedback to the user he can potentially
report or try to address, instead of a single, hardcoded message.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: install_progress: split out prompt logic into own function
Christoph Heiss [Fri, 10 Nov 2023 14:17:23 +0000 (15:17 +0100)]
tui: install_progress: split out prompt logic into own function

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: install_progress: split out reboot handling into own function
Christoph Heiss [Fri, 10 Nov 2023 14:17:22 +0000 (15:17 +0100)]
tui: install_progress: split out reboot handling into own function

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: install_progress: split out low-level installer spawing into own function
Christoph Heiss [Fri, 10 Nov 2023 14:17:21 +0000 (15:17 +0100)]
tui: install_progress: split out low-level installer spawing into own function

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: install_progress: move progress task into own function
Christoph Heiss [Fri, 10 Nov 2023 14:17:20 +0000 (15:17 +0100)]
tui: install_progress: move progress task into own function

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: move install progress dialog into own view module
Christoph Heiss [Fri, 10 Nov 2023 14:17:19 +0000 (15:17 +0100)]
tui: move install progress dialog into own view module

While at it, convert it to a proper `View`-impl, instead of a functional
component.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agofix #4856: tui: bootdisk: use correct defaults in advanced dialog
Christoph Heiss [Thu, 9 Nov 2023 09:40:56 +0000 (10:40 +0100)]
fix #4856: tui: bootdisk: use correct defaults in advanced dialog

The size of the install disk was set to the size of the first disk,
regardless of what disk was selected. This only happened if the advanced
options dialog was never opened, and only a disk was selected in the
main bootdisk dialog.

Properly solving this involved restructuring the LVM advanced bootdisk
dialog, to also hold the selected disks, like the ZFS and Btrfs dialogs.
In addition to that, the `BootdiskOptionsRef` needs quite some passing
around, to cover all the cases, since the dialog also needs to be
"reentrant-safe".

I tested (among other things):
  * Only select disk, don't open the advanced dialog, go to summary,
    then back to the bootdisk dialog -> selected disk should be kept
  * Select disk, open advanced dialog but leave everything as is, go to
    summary, then go back again -> selected disk should be kept
  * Same as previous, but change the "Total size" for the disk, go to
    summary and back -> selected disk and size should be kept
  * Same as previous, but additionally change filesystem to XFS -> disk,
    filesystem and size should be kept
  * Same as previous, but then create a ZFS RAID, go to summary & back,
    ZFS RAID should be kept with all parameters
  * etc ..

Further I also verified that the correct disk size(s) get written into
the setup structure for the low-level installer.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: bootdisk: refactor Rc<RefCell<..>> type into custom type
Christoph Heiss [Thu, 9 Nov 2023 09:40:55 +0000 (10:40 +0100)]
tui: bootdisk: refactor Rc<RefCell<..>> type into custom type

Will be used/passed around quite a lot of times due to future changes,
so simplify it a bit.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agolow level: testmode: take path to disk image instead of using /dev/null
Christoph Heiss [Thu, 9 Nov 2023 09:47:56 +0000 (10:47 +0100)]
low level: testmode: take path to disk image instead of using /dev/null

.. in exactly the same way GUI and TUI installer do, streamlining them.

Up until now, testing the TUI installer often involved hand-editing the
`run-env-info.json` to put some proper disk sizes > 0 in place. This
makes this process a lot easier.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: bootdisk: expose `arc_max` ZFS option for PVE installations
Christoph Heiss [Tue, 7 Nov 2023 12:20:54 +0000 (13:20 +0100)]
tui: bootdisk: expose `arc_max` ZFS option for PVE installations

To set the maximum value for arc_max accordingly, simply pass down
`RuntimeInfo` directly instead of the disks array to the views.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agocommon: add ZFS `arc_max` installer setup option
Christoph Heiss [Tue, 7 Nov 2023 12:20:53 +0000 (13:20 +0100)]
common: add ZFS `arc_max` installer setup option

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotest: add tests for zfs_arc_max calculations
Christoph Heiss [Tue, 7 Nov 2023 12:20:52 +0000 (13:20 +0100)]
test: add tests for zfs_arc_max calculations

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agoproxinstall: expose `arc_max` ZFS option for PVE installations
Christoph Heiss [Tue, 7 Nov 2023 12:20:51 +0000 (13:20 +0100)]
proxinstall: expose `arc_max` ZFS option for PVE installations

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agoinstall: use correct variable names in zfs_setup_module_conf()
Christoph Heiss [Tue, 7 Nov 2023 12:20:50 +0000 (13:20 +0100)]
install: use correct variable names in zfs_setup_module_conf()

That's what happens when you do some last-minute variable renaming and
trust that nothing broke ..

Fixes: 42aa2fa ("fix #4829: install: add new ZFS `arc_max` setup option")
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agorun env: remove debug print
Christoph Heiss [Tue, 7 Nov 2023 12:20:49 +0000 (13:20 +0100)]
run env: remove debug print

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: fix same "Administrator" typo in password and summary view
Christoph Heiss [Tue, 7 Nov 2023 12:23:40 +0000 (13:23 +0100)]
tui: fix same "Administrator" typo in password and summary view

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agoinstall: install correct grub metapackage for the current boot-mode
Stoiko Ivanov [Thu, 28 Sep 2023 14:05:33 +0000 (16:05 +0200)]
install: install correct grub metapackage for the current boot-mode

grub packages in debian split between:

* meta-packages, which handles (among other things) the reinstalling
  grub to the actual device/ESP in case of a version upgrade (grub-pc,
  grub-efi-amd64)

* bin-packages, which contain the actual boot-loaders

The bin-packages can coexist on a system, but the meta-package
conflict with each other (didn't check why, but I don't see a hard
conflict on a quick glance)

Currently our ISO installs grub-pc unconditionally (and both bin
packages, since we install the legacy bootloader also on uefi-booted
systems). This results in uefi-systems not getting a new grub
installed automatically upon upgrade.

Reported in our community-forum from users who upgraded to PVE 8.0,
and still run into an issue fixed in grub for bookworm:
https://forum.proxmox.com/threads/.123512/

Reproduced and analyzed by Friedrich.

This patch changes the installer, to install the meta-package fitting
for the boot-mode.

We do not set the debconf variable install_devices, because the
'install_devices' variable is only defined for 'grub-pc', and thus
(still) only set for that package/namespace.

Reported-by: Friedrich Weber <f.weber@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
12 months agoCorrect DNS IP check on management interface setup
Filip Schauer [Mon, 16 Oct 2023 10:20:06 +0000 (12:20 +0200)]
Correct DNS IP check on management interface setup

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
12 months agofix #4829: install: add new ZFS `arc_max` setup option
Christoph Heiss [Tue, 31 Oct 2023 12:10:56 +0000 (13:10 +0100)]
fix #4829: install: add new ZFS `arc_max` setup option

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: bootdisk: simplify product handling by passing the config directly
Christoph Heiss [Tue, 31 Oct 2023 12:10:55 +0000 (13:10 +0100)]
tui: bootdisk: simplify product handling by passing the config directly

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: views: add optional suffix label for `NumericEditView`s
Christoph Heiss [Tue, 31 Oct 2023 12:10:54 +0000 (13:10 +0100)]
tui: views: add optional suffix label for `NumericEditView`s

Most of the churn here is due to changing the inner view from an
`EditView` to a `LinearLayout`. Also prompted the introduction of two
small helpers .inner() and .inner_mut() to simplify things everywhere
else in the view.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agorun env: add comment for query_total_memory()
Christoph Heiss [Tue, 31 Oct 2023 12:10:53 +0000 (13:10 +0100)]
run env: add comment for query_total_memory()

This is mainly to explicitly document the unit of its return value.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
12 months agotui: fix clippy warning
Christoph Heiss [Fri, 3 Nov 2023 09:49:37 +0000 (10:49 +0100)]
tui: fix clippy warning

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