[pve-devel] applied: [PATCH] pve7to8: add check for systemd-boot presence where needed
Am 21/06/2023 um 19:35 schrieb Stoiko Ivanov: > since the package won't get installed for systems upgraded from 7 to 8 > we warn users who need systemd-boot - to be able to initialize new > ESPs - that they need to install it > > The check for package installation is based on existance of the > changelog, since the package information used in pve7to8 comes from > the API-modules, which limit it to the pve-relevant packages. > > tested in VMs with uefi and legacy mode, with existing > proxmox-boot-uuids both with and w/o systemd-boot being installed > > Signed-off-by: Stoiko Ivanov > --- > PVE/CLI/pve7to8.pm | 29 + > 1 file changed, 29 insertions(+) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH] pve7to8: add check for systemd-boot presence where needed
since the package won't get installed for systems upgraded from 7 to 8
we warn users who need systemd-boot - to be able to initialize new
ESPs - that they need to install it
The check for package installation is based on existance of the
changelog, since the package information used in pve7to8 comes from
the API-modules, which limit it to the pve-relevant packages.
tested in VMs with uefi and legacy mode, with existing
proxmox-boot-uuids both with and w/o systemd-boot being installed
Signed-off-by: Stoiko Ivanov
---
PVE/CLI/pve7to8.pm | 29 +
1 file changed, 29 insertions(+)
diff --git a/PVE/CLI/pve7to8.pm b/PVE/CLI/pve7to8.pm
index 29bb099d..712deb20 100644
--- a/PVE/CLI/pve7to8.pm
+++ b/PVE/CLI/pve7to8.pm
@@ -1229,6 +1229,34 @@ sub check_time_sync {
}
}
+sub check_bootloader {
+log_info("Checking bootloader configuration...");
+if (!$upgraded) {
+ log_skip("not yet upgraded, no need to check the presence of
systemd-boot");
+ return;
+}
+
+if (! -f "/etc/kernel/proxmox-boot-uuids") {
+ log_skip("proxmox-boot-tool not used for bootloader configuration");
+ return;
+}
+
+if (! -d "/sys/firmware/efi") {
+ log_skip("System booted in legacy-mode - no need for systemd-boot");
+ return;
+}
+
+if ( -f "/usr/share/doc/systemd-boot/changelog.Debian.gz") {
+ log_pass("systemd-boot is installed");
+} else {
+ log_warn(
+ "proxmox-boot-tool is used for bootloader configuration in uefi
mode"
+ . "but the separate systemd-boot package, existing in Debian
Bookworm is not installed"
+ . "initializing new ESPs will not work until the package is
installed"
+ );
+}
+}
+
sub check_misc {
print_header("MISCELLANEOUS CHECKS");
my $ssh_config = eval { PVE::Tools::file_get_contents('/root/.ssh/config')
};
@@ -1328,6 +1356,7 @@ sub check_misc {
check_lxcfs_fuse_version();
check_node_and_guest_configurations();
check_apt_repos();
+check_bootloader();
}
my sub colored_if {
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH manager] pve7to8: add reminder comment for noout_wanted variable
Am 21/06/2023 um 18:19 schrieb Fiona Ebner: > Signed-off-by: Fiona Ebner > --- > > Applies to the master branch. It's enough the have the reminder here, > stable would even need a different patch because of changed context. > > PVE/CLI/pve7to8.pm | 1 + > 1 file changed, 1 insertion(+) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH stable-7 manager] pve7to8: enable noout before upgrade
Am 21/06/2023 um 18:19 schrieb Fiona Ebner:
> Just like 3b776617 ("pve6to7: enable noout before upgrade") last time,
> it should be enabled in the stable branch to ensure users see the
> warning before upgrade.
>
> Signed-off-by: Fiona Ebner
> ---
>
> Intended only for the stable-7 branch.
>
> PVE/CLI/pve7to8.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
applied, thanks!
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] new installer: preseed support ?
Hi, Am 21/06/2023 um 17:56 schrieb DERUMIER, Alexandre: > I just see all the patches for the new installer (don't have tested it > yet). > > Any plan to add support for preseed file like debian for automatic > install ? > Yes, for the mid/long term we'd definitively like to provide some mechanism for this, and tbh. with the rework of the installer and how it handles the config to a single point of "truth", and how the new low-level installer (used for the Text-UI mode) works it will be possible to add quite a few ways (from simple json config lives on a separte partition of the installer image to some web-based varians with API and a password defined on the image, ...); just see the "start-session" command implementation [0] for what's required between actual installer and UI, it's really not that much anymore: [0]: https://git.proxmox.com/?p=pve-installer.git;a=blob;f=proxmox-low-level-installer;h=814961e5d5a4e73457aaa5df85e7b87e38708af7;hb=99a7fcf1fe110d67f5d0b6a010ae34cce17579a4#l88 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH stable-7 manager] pve7to8: enable noout before upgrade
Just like 3b776617 ("pve6to7: enable noout before upgrade") last time,
it should be enabled in the stable branch to ensure users see the
warning before upgrade.
Signed-off-by: Fiona Ebner
---
Intended only for the stable-7 branch.
PVE/CLI/pve7to8.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/PVE/CLI/pve7to8.pm b/PVE/CLI/pve7to8.pm
index 73ea85b0..56af2732 100644
--- a/PVE/CLI/pve7to8.pm
+++ b/PVE/CLI/pve7to8.pm
@@ -543,7 +543,7 @@ sub check_ceph {
log_warn("unable to determine overall Ceph daemon versions!");
} elsif (keys %$overall_versions == 1) {
log_pass("single running overall version detected for all Ceph
daemon types.");
- $noout_wanted = 0; # off post-upgrade, on pre-upgrade
+ $noout_wanted = 1; # off post-upgrade, on pre-upgrade
} elsif (keys $ceph_versions_simple->{overall}->%* != 1) {
log_warn("overall version mismatch detected, check 'ceph versions'
output for details!");
}
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager] pve7to8: add reminder comment for noout_wanted variable
Signed-off-by: Fiona Ebner
---
Applies to the master branch. It's enough the have the reminder here,
stable would even need a different patch because of changed context.
PVE/CLI/pve7to8.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/PVE/CLI/pve7to8.pm b/PVE/CLI/pve7to8.pm
index 6b51e98e..d988c715 100644
--- a/PVE/CLI/pve7to8.pm
+++ b/PVE/CLI/pve7to8.pm
@@ -542,6 +542,7 @@ sub check_ceph {
log_warn("unable to determine overall Ceph daemon versions!");
} elsif (keys %$overall_versions == 1) {
log_pass("single running overall version detected for all Ceph
daemon types.");
+ # TODO: needs to be set to 1 in the stable branch each time! - find
better solution?
$noout_wanted = 0; # off post-upgrade, on pre-upgrade
} elsif (keys $ceph_versions_simple->{overall}->%* != 1) {
log_warn("overall version mismatch detected, check 'ceph versions'
output for details!");
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH stable-7 manager 2/2] pve7to8: avoid confusing warning about required setting 'storage' for vzdump
Am 21/06/2023 um 17:02 schrieb Fiona Ebner: > It's required in the schema for notes-template and protected, but when > parsing vzdump.conf, it shouldn't matter whether the storage parameter > is set or not. > > The warning is ugly and users might interpret it as something that > needs to be acted upon for the upgrade: > parse error in '/etc/vzdump.conf' - 'storage': missing property - > 'notes-template' requires this property\nmissing property - 'protected' > requires this property > > Signed-off-by: Fiona Ebner > --- > > Intended for both master and stable-7. > > PVE/CLI/pve7to8.pm | 2 ++ > 1 file changed, 2 insertions(+) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH stable-7 manager 1/2] pve7to8: remove outdated warning about retention
Am 21/06/2023 um 17:02 schrieb Fiona Ebner: > It just talks about the default behavior since PVE 7. It's rather > confusing to mention this, because the behavior doesn't change anymore > in PVE 8. > > Signed-off-by: Fiona Ebner > --- > > Intended for both master and stable-7. > > PVE/CLI/pve7to8.pm | 13 - > 1 file changed, 13 deletions(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] new installer: preseed support ?
Hi, I just see all the patches for the new installer (don't have tested it yet). Any plan to add support for preseed file like debian for automatic install ? https://wiki.debian.org/DebianInstaller/Preseed ? Alexandre ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH pve-installer] tui: improve ipv6 handling
Am 21/06/2023 um 16:16 schrieb Stefan Hanreich: > * fix detection of ipv6 addresses > * change input for CIDR to allow integers up until 128 > > Signed-off-by: Stefan Hanreich > --- > proxmox-tui-installer/src/utils.rs | 2 +- > proxmox-tui-installer/src/views/mod.rs | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH 0/3] adapt to systemd-boot hooks in bookworm
Am 21/06/2023 um 16:32 schrieb Stoiko Ivanov: > This patchset addresses the change of shipping systemd-boot as separate > binary packge introduced with Debian Bookworm. > > The patches are mostly cosmetic in nature - since they silence warnings, > which look scary, but don't hurt functionality. > > The second patch should help users who upgrade from 7.X -> 8, as they > won't have systemd-boot installed automatically - so for them initializing > new ESPs will not work. > > Adding systemd-boot as Recommends to proxmox-kernel-helper should also > only help in case someone setup their system on plain Debian, with the > plan of incorporating proxmox-boot-tool into it later (by partitioning > accordingly) > > While I tested the patches - some review and consideration, especially > about potential pitfalls regarding the in place editing of the > hook-scripts would be very much appreciated! > > Stoiko Ivanov (3): > boot-tool: disarm upstream systemd-boot hookscripts > proxmox-boot: warn on missing systemd-boot package > d/control: add Recommends on systemd-boot > > debian/control | 1 + > src/bin/proxmox-boot-tool| 6 ++ > src/proxmox-boot/zz-proxmox-boot | 23 +++ > 3 files changed, 30 insertions(+) > applied, with two changes as talked off-list: - downgraded systemd-boot from recommends to suggests, as otherwise it can get pulled in on a lot of systems - changed the "grep -q "$marker" "$hookfile" && continue" to a if+then to avoid that this fails if the marker ins't found and grep exits with non-zero as we got `set -e` at the top. thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH stable-7 manager 2/2] pve7to8: avoid confusing warning about required setting 'storage' for vzdump
It's required in the schema for notes-template and protected, but when
parsing vzdump.conf, it shouldn't matter whether the storage parameter
is set or not.
The warning is ugly and users might interpret it as something that
needs to be acted upon for the upgrade:
parse error in '/etc/vzdump.conf' - 'storage': missing property -
'notes-template' requires this property\nmissing property - 'protected'
requires this property
Signed-off-by: Fiona Ebner
---
Intended for both master and stable-7.
PVE/CLI/pve7to8.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/PVE/CLI/pve7to8.pm b/PVE/CLI/pve7to8.pm
index 87dc1dc3..2c288a2a 100644
--- a/PVE/CLI/pve7to8.pm
+++ b/PVE/CLI/pve7to8.pm
@@ -613,6 +613,8 @@ sub check_backup_retention_settings {
eval {
my $confdesc = PVE::VZDump::Common::get_confdesc();
+ # vzdump.conf by itself doesn't need to honor any 'requires'
+ delete $confdesc->{$_}->{requires} for keys $confdesc->%*;
my $fn = "/etc/vzdump.conf";
my $raw = PVE::Tools::file_get_contents($fn);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH stable-7 manager 1/2] pve7to8: remove outdated warning about retention
It just talks about the default behavior since PVE 7. It's rather
confusing to mention this, because the behavior doesn't change anymore
in PVE 8.
Signed-off-by: Fiona Ebner
---
Intended for both master and stable-7.
PVE/CLI/pve7to8.pm | 13 -
1 file changed, 13 deletions(-)
diff --git a/PVE/CLI/pve7to8.pm b/PVE/CLI/pve7to8.pm
index 6b51e98e..87dc1dc3 100644
--- a/PVE/CLI/pve7to8.pm
+++ b/PVE/CLI/pve7to8.pm
@@ -608,8 +608,6 @@ sub check_backup_retention_settings {
my $pass = 1;
-my $node_has_retention;
-
my $maxfiles_msg = "parameter 'maxfiles' is deprecated with PVE 7.x and
will be removed in a " .
"future version, use 'prune-backups' instead.";
@@ -626,8 +624,6 @@ sub check_backup_retention_settings {
$pass = 0;
log_warn("$fn - $maxfiles_msg");
}
-
- $node_has_retention = defined($param->{maxfiles}) ||
defined($param->{'prune-backups'});
};
if (my $err = $@) {
$pass = 0;
@@ -643,15 +639,6 @@ sub check_backup_retention_settings {
$pass = 0;
log_warn("storage '$storeid' - $maxfiles_msg");
}
-
- next if !$scfg->{content}->{backup};
- next if defined($scfg->{maxfiles}) || defined($scfg->{'prune-backups'});
- next if $node_has_retention;
-
- log_info(
- "storage '$storeid' - no backup retention settings defined - by
default, since PVE 7.0"
- ." it will no longer keep only the last backup, but all backups"
- );
}
eval {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH] tui: focus next button by default
except the password dialog, since the user must provide input
to do that, we have to set the focus index on all relevant views
Signed-off-by: Dominik Csapak
---
not sure if this is the correct approach, also the extra parameter feels
slightly wrong, but didn't found a nicer way to do this
any errors from focusing will be ignrored, but that shouldn't happen
anyway until we add/remove buttons and the index changes
alternatively we could create a second 'new_with_focus_next' (or
'without') that gets called respectively, but also seems a bit weird for
that
proxmox-tui-installer/src/main.rs | 101 +++---
1 file changed, 52 insertions(+), 49 deletions(-)
diff --git a/proxmox-tui-installer/src/main.rs
b/proxmox-tui-installer/src/main.rs
index 20cb31b..5eaba7f 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -63,21 +63,21 @@ impl InstallerView {
state: ,
view: T,
next_cb: Box,
+focus_next: bool,
) -> Self {
-let inner = LinearLayout::vertical()
+let mut bbar = LinearLayout::horizontal()
+.child(abort_install_button())
+.child(DummyView.full_width())
+.child(Button::new("Previous", switch_to_prev_screen))
+.child(DummyView)
+.child(Button::new("Next", next_cb));
+let _ = bbar.set_focus_index(4); // ignore errors
+let mut inner = LinearLayout::vertical()
.child(PaddedView::lrtb(0, 0, 1, 1, view))
-.child(PaddedView::lrtb(
-1,
-1,
-0,
-0,
-LinearLayout::horizontal()
-.child(abort_install_button())
-.child(DummyView.full_width())
-.child(Button::new("Previous", switch_to_prev_screen))
-.child(DummyView)
-.child(Button::new("Next", next_cb)),
-));
+.child(PaddedView::lrtb(1, 1, 0, 0, bbar));
+if focus_next {
+let _ = inner.set_focus_index(1); // ignore errors
+}
Self::with_raw(state, inner)
}
@@ -378,7 +378,15 @@ fn get_eula() -> String {
fn license_dialog(siv: Cursive) -> InstallerView {
let state = siv.user_data::().unwrap();
-let inner = LinearLayout::vertical()
+let mut bbar = LinearLayout::horizontal()
+.child(abort_install_button())
+.child(DummyView.full_width())
+.child(Button::new("I agree", |siv| {
+switch_to_next_screen(siv, InstallerStep::Bootdisk,
_dialog)
+}));
+let _ = bbar.set_focus_index(2); // ignore errors
+
+let mut inner = LinearLayout::vertical()
.child(PaddedView::lrtb(
0,
0,
@@ -389,18 +397,9 @@ fn license_dialog(siv: Cursive) -> InstallerView {
.child(Panel::new(ScrollView::new(
TextView::new(get_eula()).center(),
)))
-.child(PaddedView::lrtb(
-1,
-1,
-1,
-0,
-LinearLayout::horizontal()
-.child(abort_install_button())
-.child(DummyView.full_width())
-.child(Button::new("I agree", |siv| {
-switch_to_next_screen(siv, InstallerStep::Bootdisk,
_dialog)
-})),
-));
+.child(PaddedView::lrtb(1, 1, 1, 0, bbar));
+
+let _ = inner.set_focus_index(2); // ignore errors
InstallerView::with_raw(state, inner)
}
@@ -428,6 +427,7 @@ fn bootdisk_dialog(siv: Cursive) -> InstallerView {
_ => siv.add_layer(Dialog::info("Invalid values")),
}
}),
+true,
)
}
@@ -453,6 +453,7 @@ fn timezone_dialog(siv: Cursive) -> InstallerView {
_ => siv.add_layer(Dialog::info("Invalid values")),
}
}),
+true,
)
}
@@ -518,6 +519,7 @@ fn password_dialog(siv: Cursive) -> InstallerView {
_ => siv.add_layer(Dialog::info("Invalid values")),
}
}),
+false,
)
}
@@ -613,6 +615,7 @@ fn network_dialog(siv: Cursive) -> InstallerView {
_ => siv.add_layer(Dialog::info("Invalid values")),
}
}),
+true,
)
}
@@ -643,7 +646,27 @@ impl TableViewItem for SummaryOption {
fn summary_dialog(siv: Cursive) -> InstallerView {
let state = siv.user_data::().unwrap();
-let inner = LinearLayout::vertical()
+let mut bbar = LinearLayout::horizontal()
+.child(abort_install_button())
+.child(DummyView.full_width())
+.child(Button::new("Previous", switch_to_prev_screen))
+.child(DummyView)
+.child(Button::new("Install", |siv| {
+let autoreboot = siv
+.find_name("reboot-after-install")
+.map(|v: ViewRef| v.is_checked())
+.unwrap_or_default();
+
+
Re: [pve-devel] [PATCH installer] tui: multiply the disk size back into bytes
Am 21/06/2023 um 16:00 schrieb Stefan Sterz: > previously the installer correctly divided the value when using them > for the `FloatEditView`, but forgot to multiply the value again when > retrieving it after editing. this commit fixes that > > Signed-off-by: Stefan Sterz > --- > tested this only locally and didn't build the installer completelly. > i am not sure if the installer handles this value correctly once it > is forwarded to the perl installer. if the perl installer expects > bytes here, it should work just fine, though. no it doesn't it expects Gigabyte in floats, see: https://git.proxmox.com/?p=pve-installer.git;a=commitdiff;h=9a2d64977f73cec225c407ff13765ef02e2ff9e9 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH 3/3] d/control: add Recommends on systemd-boot
systemd-boot is a separate binary package, and proxmox-boot-tool needs
it in the uefi-case as boot-loader for the ESPs
Not adding as Depends, because it is not strictly necessary for
proxmox-boot-tool (pinning is independent as is its use on legacy-boot
systems)
Signed-off-by: Stoiko Ivanov
---
debian/control | 1 +
1 file changed, 1 insertion(+)
diff --git a/debian/control b/debian/control
index 1e2309a..c5f1179 100644
--- a/debian/control
+++ b/debian/control
@@ -10,6 +10,7 @@ Architecture: all
Section: admin
Priority: optional
Depends: dosfstools, gdisk, systemd, udev, ${misc:Depends},
+Recommends: systemd-boot,
Breaks: proxmox-ve (<< 6.0-2~), pve-kernel-helper,
Replaces: proxmox-ve (<< 6.0-2~), pve-kernel-helper,
Provides: pve-kernel-helper,
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH 2/3] proxmox-boot: warn on missing systemd-boot package
With the shipping of systemd-boot as separate package, we cannot rely
on `bootctl` being present in all systems (e.g. currently all systems
upgraded from PVE 7 will not automatically pull systemd-boot in.
This patch adds a check for existence + warning with an explanation to
the only invocation of bootctl in the boot-tool codebase
Signed-off-by: Stoiko Ivanov
---
src/bin/proxmox-boot-tool | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/bin/proxmox-boot-tool b/src/bin/proxmox-boot-tool
index d41f921..913b0f6 100755
--- a/src/bin/proxmox-boot-tool
+++ b/src/bin/proxmox-boot-tool
@@ -153,6 +153,12 @@ init_bootloader() {
if [ -d /sys/firmware/efi ]; then
echo "Installing systemd-boot.."
mkdir -p "$esp_mp/$PMX_ESP_DIR"
+ if ! command -V bootctl >/dev/null 2>&1 ;
+ then
+ warn "E: bootctl is not available - make sure
systemd-boot is installed"
+ exit 1
+ fi
+
bootctl --graceful --path "$esp_mp" install
echo "Configuring systemd-boot.."
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH 0/3] adapt to systemd-boot hooks in bookworm
This patchset addresses the change of shipping systemd-boot as separate binary packge introduced with Debian Bookworm. The patches are mostly cosmetic in nature - since they silence warnings, which look scary, but don't hurt functionality. The second patch should help users who upgrade from 7.X -> 8, as they won't have systemd-boot installed automatically - so for them initializing new ESPs will not work. Adding systemd-boot as Recommends to proxmox-kernel-helper should also only help in case someone setup their system on plain Debian, with the plan of incorporating proxmox-boot-tool into it later (by partitioning accordingly) While I tested the patches - some review and consideration, especially about potential pitfalls regarding the in place editing of the hook-scripts would be very much appreciated! Stoiko Ivanov (3): boot-tool: disarm upstream systemd-boot hookscripts proxmox-boot: warn on missing systemd-boot package d/control: add Recommends on systemd-boot debian/control | 1 + src/bin/proxmox-boot-tool| 6 ++ src/proxmox-boot/zz-proxmox-boot | 23 +++ 3 files changed, 30 insertions(+) -- 2.30.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH 1/3] boot-tool: disarm upstream systemd-boot hookscripts
With Debian Bookworm systemd-boot is a separate binary-package,
instead of part of the main systemd package.
Since it's not installed by default, Debian-upstream has added
hook-scripts to the package, which manage kernel copying to the esp
(kernel-install).
The hookscripts print a warning if the ESP is not mounted at
$SYSTEMD_ESP_PATH or /boot/efi, /efi or /boot - through `bootctl
is-installed --quiet` [0,1].
This patch adds a function, which disables the hookscripts from
upstream if /etc/kernel/proxmox-boot-uuids is present.
It adds an explanation as marker and 'exit 0' on top of the script, so
that users know why the scripts were touched (e.g. when a new
systemd-boot hookscript version from upstream asks what to do with the
local modifications)
While editing shell-script hooks from other packages is quite brittle
it still seems like the best option, to support most use-cases
(including users, who don't use proxmox-boot-tool, but want to
manually install systemd-boot).
Alternatives considered:
* dpkg-divert for all hookscripts - sadly the Debian policy manual
warns against this
* adding Replaces: systemd-boot to d/control - afaict this would need
systemd-boot to also declare this for proxmox-kernel-helper [3]
Tested on 2 VMs installed with the 8.0 ISO (once with legacy once with
uefi boot)
[0]
https://github.com/systemd/systemd/blob/8a38b62f37189b071a30f208530ce5dc278e521e/src/shared/find-esp.c#L503
[1]
https://github.com/systemd/systemd/blob/8a38b62f37189b071a30f208530ce5dc278e521e/src/boot/bootctl.c#L90
[2] https://www.debian.org/doc/debian-policy/ap-pkg-diversions.html
[3] https://www.debian.org/doc/debian-policy/ch-relationships.html
Reported-by: Aaron Lauterer
Signed-off-by: Stoiko Ivanov
---
src/proxmox-boot/zz-proxmox-boot | 23 +++
1 file changed, 23 insertions(+)
diff --git a/src/proxmox-boot/zz-proxmox-boot b/src/proxmox-boot/zz-proxmox-boot
index c6c708c..c72f9ef 100755
--- a/src/proxmox-boot/zz-proxmox-boot
+++ b/src/proxmox-boot/zz-proxmox-boot
@@ -191,6 +191,27 @@ remove_old_kernels_legacy() {
}
+disable_systemd_boot_hook() {
+
+ if [ ! -f "${ESP_LIST}" ]; then
+ return
+ fi
+
+ marker="# This hookfile has been disabled by proxmox-boot-tool"
+ for hookfile in \
+ "/etc/initramfs/post-update.d/systemd-boot" \
+ "/etc/kernel/postinst.d/zz-systemd-boot" \
+ "/etc/kernel/postrm.d/zz-systemd-boot" ; \
+ do
+ grep -q "$marker" "$hookfile" && continue
+ warn " Disabling upstream hook $hookfile"
+ printf "#!/bin/sh\n\n%s\nexit 0\n" "$marker" >
"$hookfile.pbt.tmp"
+ cat "$hookfile" >> "$hookfile.pbt.tmp"
+ mv "$hookfile.pbt.tmp" "$hookfile"
+ done
+
+}
+
set -- $DEB_MAINT_PARAMS
mode="${1#\'}"
mode="${mode%\'}"
@@ -203,12 +224,14 @@ case $0:$mode in
reexec_in_mountns "$@"
BOOT_KVERS="$(boot_kernel_list "$@")"
update_esps
+ disable_systemd_boot_hook
;;
*/postrm.d/*:|*/postrm.d/*:remove)
reexec_in_mountns "$@"
# no newly installed kernel
BOOT_KVERS="$(boot_kernel_list)"
update_esps
+ disable_systemd_boot_hook
;;
esac
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-installer] tui: improve ipv6 handling
* fix detection of ipv6 addresses
* change input for CIDR to allow integers up until 128
Signed-off-by: Stefan Hanreich
---
proxmox-tui-installer/src/utils.rs | 2 +-
proxmox-tui-installer/src/views/mod.rs | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/proxmox-tui-installer/src/utils.rs
b/proxmox-tui-installer/src/utils.rs
index c4b7c03..3245fac 100644
--- a/proxmox-tui-installer/src/utils.rs
+++ b/proxmox-tui-installer/src/utils.rs
@@ -65,7 +65,7 @@ impl CidrAddress {
/// Returns `true` if this address is an IPv6 address, `false` otherwise.
pub fn is_ipv6() -> bool {
-self.addr.is_ipv4()
+self.addr.is_ipv6()
}
/// Returns only the mask part of the address.
diff --git a/proxmox-tui-installer/src/views/mod.rs
b/proxmox-tui-installer/src/views/mod.rs
index 4160b8f..dc58043 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -387,10 +387,10 @@ impl CidrAddressEditView {
fn mask_edit_view(content: usize) -> ResizedView {
IntegerEditView::new()
-.max_value(32)
-.max_content_width(2)
+.max_value(128)
+.max_content_width(3)
.content(content)
-.fixed_width(3)
+.fixed_width(4)
}
fn get_values() -> 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] [PATCH installer] tui: multiply the disk size back into bytes
previously the installer correctly divided the value when using them
for the `FloatEditView`, but forgot to multiply the value again when
retrieving it after editing. this commit fixes that
Signed-off-by: Stefan Sterz
---
tested this only locally and didn't build the installer completelly.
i am not sure if the installer handles this value correctly once it
is forwarded to the perl installer. if the perl installer expects
bytes here, it should work just fine, though.
proxmox-tui-installer/src/views/mod.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxmox-tui-installer/src/views/mod.rs
b/proxmox-tui-installer/src/views/mod.rs
index 94c3993..8503d82 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -234,7 +234,7 @@ impl DiskSizeEditView {
.flatten()
})
.flatten()
-.map(|val| val as u64)
+.map(|val| (val * 1024. * 1024. * 1024.) as u64)
}
}
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 storage] plugin: handle invalid storage types
Warn and skip if a storage with unknown storage type is encountered.
This might happen by manually editing the storage config.
Signed-off-by: Christian Ebner
---
Changes since v1:
- v1 contained the wrong diff
src/PVE/Storage/Plugin.pm | 4
1 file changed, 4 insertions(+)
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 9d3b1ae..732f27e 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -518,6 +518,10 @@ sub parse_config {
foreach my $storeid (keys %$ids) {
my $d = $ids->{$storeid};
my $type = $d->{type};
+ if (!$type) {
+ warn "invalid storage type for '$storeid'\n";
+ next;
+ }
my $def = $defaultData->{plugindata}->{$type};
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage] plugin: handle invalid storage types
Oops, I send the wrong patch file here, will send the correct one as v2.
Please ignore this one.
> On 21.06.2023 15:28 CEST Christian Ebner wrote:
>
>
> Warn and skip if a storage with unknown storage type is encountered.
> This might happen by manually editing the storage config.
>
> Signed-off-by: Christian Ebner
> ---
> src/PVE/Storage/Plugin.pm | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 9d3b1ae..7e7749d 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -520,6 +520,10 @@ sub parse_config {
> my $type = $d->{type};
>
> my $def = $defaultData->{plugindata}->{$type};
> + if (!$def) {
> + warn "invalid storage type for '$d'\n";
> + next;
> + }
>
> if ($def->{content}) {
> $d->{content} = $def->{content}->[1] if !$d->{content};
> --
> 2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH storage] plugin: handle invalid storage types
Warn and skip if a storage with unknown storage type is encountered.
This might happen by manually editing the storage config.
Signed-off-by: Christian Ebner
---
src/PVE/Storage/Plugin.pm | 4
1 file changed, 4 insertions(+)
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 9d3b1ae..7e7749d 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -520,6 +520,10 @@ sub parse_config {
my $type = $d->{type};
my $def = $defaultData->{plugindata}->{$type};
+ if (!$def) {
+ warn "invalid storage type for '$d'\n";
+ next;
+ }
if ($def->{content}) {
$d->{content} = $def->{content}->[1] if !$d->{content};
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] tui: verify email with basic regex
Note that the HTML5 standard [1] suggests
/^[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])?)*$/
as a regex, I am not sure whats the difference but it might be worth it to
check if there is any relevant difference. This is used by the validator crate
[2] for example.
[1] https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address
[2] https://docs.rs/validator/latest/validator/fn.validate_email.html
> On 21.06.2023 14:30 CEST Dominik Csapak wrote:
>
>
> regex copied from perl gui installer
>
> Signed-off-by: Dominik Csapak
> ---
> this needs librust-regex-dev as build-dependency
> proxmox-tui-installer/Cargo.toml | 1 +
> proxmox-tui-installer/src/main.rs | 8
> 2 files changed, 9 insertions(+)
>
> diff --git a/proxmox-tui-installer/Cargo.toml
> b/proxmox-tui-installer/Cargo.toml
> index 9d57b5b..5a50c69 100644
> --- a/proxmox-tui-installer/Cargo.toml
> +++ b/proxmox-tui-installer/Cargo.toml
> @@ -11,5 +11,6 @@ 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-sys = "0.5.0"
> diff --git a/proxmox-tui-installer/src/main.rs
> b/proxmox-tui-installer/src/main.rs
> index 77cfb63..2d048f0 100644
> --- a/proxmox-tui-installer/src/main.rs
> +++ b/proxmox-tui-installer/src/main.rs
> @@ -25,6 +25,8 @@ use cursive::{
> Cursive, CursiveRunnable, ScreenId, View, XY,
> };
>
> +use regex::Regex;
> +
> use proxmox_sys::linux::procfs;
>
> mod options;
> @@ -484,12 +486,18 @@ 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")
> } else if root_password != confirm_password {
> Err("passwords do not match")
> } else if email == "mail@example.invalid" {
> Err("invalid email address")
> +} else if !email_regex.is_match() {
> +Err("Email does not look like a valid address
> (u...@domain.tld)")
> } else {
> Ok(PasswordOptions {
> root_password,
> --
> 2.30.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 2/2] tui: verify email with basic regex
Am 21/06/2023 um 14:30 schrieb Dominik Csapak:
> regex copied from perl gui installer
>
> Signed-off-by: Dominik Csapak
> ---
> this needs librust-regex-dev as build-dependency
> proxmox-tui-installer/Cargo.toml | 1 +
> proxmox-tui-installer/src/main.rs | 8
> 2 files changed, 9 insertions(+)
>
> diff --git a/proxmox-tui-installer/Cargo.toml
> b/proxmox-tui-installer/Cargo.toml
> index 9d57b5b..5a50c69 100644
> --- a/proxmox-tui-installer/Cargo.toml
> +++ b/proxmox-tui-installer/Cargo.toml
> @@ -11,5 +11,6 @@ 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"
FYI: needs also an build-dependency entry in d/control to be complete, but can
fix
that on applying.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH 2/2] tui: verify email with basic regex
regex copied from perl gui installer
Signed-off-by: Dominik Csapak
---
this needs librust-regex-dev as build-dependency
proxmox-tui-installer/Cargo.toml | 1 +
proxmox-tui-installer/src/main.rs | 8
2 files changed, 9 insertions(+)
diff --git a/proxmox-tui-installer/Cargo.toml b/proxmox-tui-installer/Cargo.toml
index 9d57b5b..5a50c69 100644
--- a/proxmox-tui-installer/Cargo.toml
+++ b/proxmox-tui-installer/Cargo.toml
@@ -11,5 +11,6 @@ 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-sys = "0.5.0"
diff --git a/proxmox-tui-installer/src/main.rs
b/proxmox-tui-installer/src/main.rs
index 77cfb63..2d048f0 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -25,6 +25,8 @@ use cursive::{
Cursive, CursiveRunnable, ScreenId, View, XY,
};
+use regex::Regex;
+
use proxmox_sys::linux::procfs;
mod options;
@@ -484,12 +486,18 @@ 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")
} else if root_password != confirm_password {
Err("passwords do not match")
} else if email == "mail@example.invalid" {
Err("invalid email address")
+} else if !email_regex.is_match() {
+Err("Email does not look like a valid address
(u...@domain.tld)")
} else {
Ok(PasswordOptions {
root_password,
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH 1/2] tui: always use mail@example.invalid as default email address
like the gui installer
Signed-off-by: Dominik Csapak
---
proxmox-tui-installer/src/main.rs| 2 +-
proxmox-tui-installer/src/options.rs | 14 +++---
2 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/proxmox-tui-installer/src/main.rs
b/proxmox-tui-installer/src/main.rs
index 90da81d..77cfb63 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -179,7 +179,7 @@ fn main() {
options: InstallerOptions {
bootdisk: BootdiskOptions::defaults_from(_info.disks[0]),
timezone: TimezoneOptions::defaults_from(_info, ),
-password: PasswordOptions::defaults_from(_info),
+password: Default::default(),
network: NetworkOptions::from(_info.network),
autoreboot: false,
},
diff --git a/proxmox-tui-installer/src/options.rs
b/proxmox-tui-installer/src/options.rs
index 6855f4a..5f3d295 100644
--- a/proxmox-tui-installer/src/options.rs
+++ b/proxmox-tui-installer/src/options.rs
@@ -306,18 +306,10 @@ pub struct PasswordOptions {
pub root_password: String,
}
-impl PasswordOptions {
-pub fn defaults_from(info: ) -> Self {
-let domain = info
-.network
-.dns
-.domain
-.clone()
-// Safety: The provided default domain will always be valid.
-.unwrap_or_else(|| Fqdn::from("example.invalid").unwrap());
-
+impl Default for PasswordOptions {
+fn default() -> Self {
Self {
-email: format!("mail@{domain}"),
+email: "mail@example.invalid".to_string(),
root_password: String::new(),
}
}
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer] tui: Add a cancel button to Advanced bootdisk options
meh, this focuses first, before the Ok button, which is just makes the existing
non-ideal UX
They way I see it, changing the existing settings for a disk is quite a
high-commitment action. In such cases I would generally focus by default the button
associated to the negative action, so yes it will require an extra TAB to get to the
button but I think it makes sense considering the consequences of the
action.
Note that from a UX perspective the user cannot tell that the dialog starts with the
current values (since the dialog is obscuring the previous values) and that pressing
will have no consequences if no field has been changed, and in the case
they have changed the values they have no way to go back to the previous values if
they have forgotten them. Or at least that was my experience.
As with a cancel we really need to ensure that no callback has already changed
data, not sure for the TUI, but the GTK UI would need quite some extra handling
here
At the moment closing the advanced options in the GUI (it has its x button visible) will
just close the dialog, just as this patch. The only line that changes something before
the uses closes or activates the "Ok" button are
```
$fstypecb->signal_connect (changed => sub {
my $new_filesys = $fstypecb->get_active_text();
Proxmox::Install::Config::set_filesys($new_filesys);
&$switch_view();
});
```
I would argue that the config changes should only apply after the user confirms the changes by
activating the "Ok" button. As far as I understand the TUI installer at the moment won't
change anything, e.g. via some callback, until the user presses so adding a
button that just closes the dialog won't do any bad.
On 6/21/23 12:40, Thomas Lamprecht wrote:
Am 21/06/2023 um 11:16 schrieb Maximiliano Sandoval:
This matches the GUI installer which counts with a close (x) button.
Signed-off-by: Maximiliano Sandoval
---
proxmox-tui-installer/src/views/bootdisk.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs
b/proxmox-tui-installer/src/views/bootdisk.rs
index 3fdbe5b..eaf343d 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -456,6 +456,7 @@ fn advanced_options_view(disks: &[Disk], options:
Rc>)
&(*options).borrow(),
))
.title("Advanced bootdisk options")
+.dismiss_button("Cancel")
meh, this focuses first, before the Ok button, which is just makes the existing
non-ideal UX w.r.t. focus priority of buttons worse, so for now I rather have no
such button - user can simply press OK, which is also a bit easier to have a
clear understanding that the entered values are actually the ones then used.
As with a cancel we really need to ensure that no callback has already changed
data, not sure for the TUI, but the GTK UI would need quite some extra handling
here.
.button("Ok", {
let options_ref = options.clone();
move |siv| {
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied-series: [PATCH v5 qemu-server 0/7] migration: don't scan all storages, fail on aliases
Am 19.06.23 um 11:29 schrieb Aaron Lauterer: > This patch series changes the behavior during guest migrations: > > Don't scan all storages for potential images belonging to the guest. > Only migrate images referenced in the config. > This made it necessary to handle pending changes explicitly which had > been covered by the storage scan. > > We also added checks for any aliased volids and fail the migration if > detected. > > The qemu-server part consists of quite a lot more commits since we had > to change a few things and for a better history, they are mostly their > own patches. > > There is also a small patch for the documentation to add a hint that > aliased storages should be avoided. > > applied the series, thanks! Added follow-ups for my nits and found one issue with qcow2 cloudinit disk and pushed a fix (the check for with_snapshots was only done after the cdrom check which would return early). ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH manager] api: resource usb mapping: add missing proxyto_callback
Am 21/06/2023 um 12:05 schrieb Dominik Csapak: > i have added it to the pci api call, but forgot to add it for usb > otherwise adding a mapped usb device only works on the node where the > gui is connected to > > Signed-off-by: Dominik Csapak > --- > PVE/API2/Cluster/Mapping/USB.pm | 5 + > 1 file changed, 5 insertions(+) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH installer] gui: make abort button unsensitive on last panel
Am 21/06/2023 um 11:57 schrieb Maximiliano Sandoval: > It makes no sense from a UX point of view to abort an install that's > already finished. > > Signed-off-by: Maximiliano Sandoval > --- > proxinstall | 5 + > 1 file changed, 5 insertions(+) > > applied, thanks! (albeit it's a bit scary to press on reboot if one is testing just the GUI on their workstation, outside of the installer, luckily we just quit in that case ;-) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH pve-installer] tui: read correct input for DNS server address
Am 21/06/2023 um 12:46 schrieb Stefan Hanreich:
> Currently the TUI installer reads the value for the DNS server from
> the field for the gateway.
>
Many thanks for the patch-as-report approach, but Christoph already provided
this one after I noticed it off-list, and I pushed that (looks exactly the same)
already; sorry!
> Signed-off-by: Stefan Hanreich
> ---
> proxmox-tui-installer/src/main.rs | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/proxmox-tui-installer/src/main.rs
> b/proxmox-tui-installer/src/main.rs
> index 510807a..77818e7 100644
> --- a/proxmox-tui-installer/src/main.rs
> +++ b/proxmox-tui-installer/src/main.rs
> @@ -540,7 +540,7 @@ fn network_dialog(siv: Cursive) -> InstallerView {
> .map_err(|err| err.to_string())?;
>
> let dns_server = view
> -.get_value::(3)
> +.get_value::(4)
> .ok_or("failed to retrieve DNS server address")?
> .parse::()
> .map_err(|err| err.to_string())?;
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied-series: [PATCH-SERIES qemu-server/manager] restore PVE 7 default for cloudinit package upgrades
Am 21/06/2023 um 12:04 schrieb Fiona Ebner:
> Commit efa3355d ("fix #3428: cloudinit: add parameter for upgrade on
> boot") changed the default, but this is a breaking change. The bug
> report was only about making the option configurable.
>
> The commit doesn't give an explicit reason for why, and arguably,
> doing the upgrade is not an issue for most users. It also leads to a
> different cloud-init instance ID, because of the different setting,
> which in turn leads to ssh host key regeneration within the VM.
>
> Dependency bump manager -> qemu-server doesn't hurt to keep things
> aligned (at least in one direction).
>
> Thanks to Friedrich for catching the issue!
>
>
> qemu-server:
>
> Fiona Ebner (2):
> schema: cloudinit: document default for ciupgrade
> cloudinit: restore previous default for package upgrades
>
> PVE/QemuServer.pm | 3 ++-
> PVE/QemuServer/Cloudinit.pm | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
>
> manager:
>
> Fiona Ebner (1):
> ui: cloudinit: align default value for package upgrades with backend
> again
>
> www/manager6/qemu/CloudInit.js | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
applied series, thanks!
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-installer] tui: read correct input for DNS server address
Currently the TUI installer reads the value for the DNS server from
the field for the gateway.
Signed-off-by: Stefan Hanreich
---
proxmox-tui-installer/src/main.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxmox-tui-installer/src/main.rs
b/proxmox-tui-installer/src/main.rs
index 510807a..77818e7 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -540,7 +540,7 @@ fn network_dialog(siv: Cursive) -> InstallerView {
.map_err(|err| err.to_string())?;
let dns_server = view
-.get_value::(3)
+.get_value::(4)
.ok_or("failed to retrieve DNS server address")?
.parse::()
.map_err(|err| err.to_string())?;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer] tui: Add a cancel button to Advanced bootdisk options
Am 21/06/2023 um 11:16 schrieb Maximiliano Sandoval:
> This matches the GUI installer which counts with a close (x) button.
>
> Signed-off-by: Maximiliano Sandoval
> ---
> proxmox-tui-installer/src/views/bootdisk.rs | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/proxmox-tui-installer/src/views/bootdisk.rs
> b/proxmox-tui-installer/src/views/bootdisk.rs
> index 3fdbe5b..eaf343d 100644
> --- a/proxmox-tui-installer/src/views/bootdisk.rs
> +++ b/proxmox-tui-installer/src/views/bootdisk.rs
> @@ -456,6 +456,7 @@ fn advanced_options_view(disks: &[Disk], options:
> Rc>)
> &(*options).borrow(),
> ))
> .title("Advanced bootdisk options")
> +.dismiss_button("Cancel")
meh, this focuses first, before the Ok button, which is just makes the existing
non-ideal UX w.r.t. focus priority of buttons worse, so for now I rather have no
such button - user can simply press OK, which is also a bit easier to have a
clear understanding that the entered values are actually the ones then used.
As with a cancel we really need to ensure that no callback has already changed
data, not sure for the TUI, but the GTK UI would need quite some extra handling
here.
> .button("Ok", {
> let options_ref = options.clone();
> move |siv| {
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer] gui: make abort button focusable
Am 21/06/2023 um 12:02 schrieb Maximiliano Sandoval:
> GtkWiget:can-focus=false means that the user cannot use tab or key
> navigation to interact with this button.
>
that's not true, as Do the Alt + A combination works just fine here.
And not having it in the default focusing "group" is definitively wanted.
I'd rather keep it as is.
> Signed-off-by: Maximiliano Sandoval
> ---
> proxinstall | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/proxinstall b/proxinstall
> index 6d13892..f260a95 100755
> --- a/proxinstall
> +++ b/proxinstall
> @@ -178,7 +178,6 @@ sub create_main_window {
>
>
> my $abort = Gtk3::Button->new('_Abort');
> -$abort->set_can_focus(0);
> $cmdbox->pack_start($abort, 0, 0, 10);
> $abort->signal_connect(clicked => sub { app_quit(-1); });
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer] tui: Add a cancel button to Advanced bootdisk options
LGTM.
On Wed, Jun 21, 2023 at 11:16:09AM +0200, Maximiliano Sandoval wrote:
> This matches the GUI installer which counts with a close (x) button.
>
Reviewed-by: Christoph Heiss
> Signed-off-by: Maximiliano Sandoval
> ---
> proxmox-tui-installer/src/views/bootdisk.rs | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/proxmox-tui-installer/src/views/bootdisk.rs
> b/proxmox-tui-installer/src/views/bootdisk.rs
> index 3fdbe5b..eaf343d 100644
> --- a/proxmox-tui-installer/src/views/bootdisk.rs
> +++ b/proxmox-tui-installer/src/views/bootdisk.rs
> @@ -456,6 +456,7 @@ fn advanced_options_view(disks: &[Disk], options:
> Rc>)
> &(*options).borrow(),
> ))
> .title("Advanced bootdisk options")
> +.dismiss_button("Cancel")
> .button("Ok", {
> let options_ref = options.clone();
> move |siv| {
> --
> 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
[pve-devel] applied: [PATCH v2 installer 1/1] fix #4643: show a confirmation dialog when clicking abort
Am 21/06/2023 um 10:55 schrieb Maximiliano Sandoval: > Note that unlike the rest of the file, we connect to the response signal > instead of using Gtk3::Dialog->run, the reason is that run blocks the > main loop used by GTK and this undesirable to the point where > Gtk3::Dialog->run was removed for GTK 4. > > Signed-off-by: Maximiliano Sandoval > --- > proxinstall | 14 +- > 1 file changed, 13 insertions(+), 1 deletion(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager] api: resource usb mapping: add missing proxyto_callback
i have added it to the pci api call, but forgot to add it for usb
otherwise adding a mapped usb device only works on the node where the
gui is connected to
Signed-off-by: Dominik Csapak
---
PVE/API2/Cluster/Mapping/USB.pm | 5 +
1 file changed, 5 insertions(+)
diff --git a/PVE/API2/Cluster/Mapping/USB.pm b/PVE/API2/Cluster/Mapping/USB.pm
index 5495bce2..763d5c2b 100644
--- a/PVE/API2/Cluster/Mapping/USB.pm
+++ b/PVE/API2/Cluster/Mapping/USB.pm
@@ -15,6 +15,11 @@ __PACKAGE__->register_method ({
name => 'index',
path => '',
method => 'GET',
+# only proxy if we give the 'check-node' parameter
+proxyto_callback => sub {
+ my ($rpcenv, $proxyto, $param) = @_;
+ return $param->{'check-node'} // 'localhost';
+},
description => "List USB Hardware Mappings",
permissions => {
description => "Only lists entries where you have 'Mapping.Modify',
'Mapping.Use' or".
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager 1/1] ui: cloudinit: align default value for package upgrades with backend again
The default in Proxmox VE 7 was true and it was decided to keep that
and avoid a breaking change.
Signed-off-by: Fiona Ebner
---
www/manager6/qemu/CloudInit.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/www/manager6/qemu/CloudInit.js b/www/manager6/qemu/CloudInit.js
index 0e06d962..03d06d9c 100644
--- a/www/manager6/qemu/CloudInit.js
+++ b/www/manager6/qemu/CloudInit.js
@@ -290,7 +290,7 @@ Ext.define('PVE.qemu.CloudInit', {
header: gettext('Upgrade packages'),
iconCls: 'fa fa-archive',
renderer: Proxmox.Utils.format_boolean,
- defaultValue: '',
+ defaultValue: 1,
editor: {
xtype: 'proxmoxWindowEdit',
subject: gettext('Upgrade packages on boot'),
@@ -298,7 +298,7 @@ Ext.define('PVE.qemu.CloudInit', {
xtype: 'proxmoxcheckbox',
name: 'ciupgrade',
uncheckedValue: 0,
- defaultValue: 0,
+ value: 1, // serves as default value, using
defaultValue is not enough
fieldLabel: gettext('Upgrade packages'),
labelWidth: 140,
},
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH-SERIES qemu-server/manager] restore PVE 7 default for cloudinit package upgrades
Commit efa3355d ("fix #3428: cloudinit: add parameter for upgrade on
boot") changed the default, but this is a breaking change. The bug
report was only about making the option configurable.
The commit doesn't give an explicit reason for why, and arguably,
doing the upgrade is not an issue for most users. It also leads to a
different cloud-init instance ID, because of the different setting,
which in turn leads to ssh host key regeneration within the VM.
Dependency bump manager -> qemu-server doesn't hurt to keep things
aligned (at least in one direction).
Thanks to Friedrich for catching the issue!
qemu-server:
Fiona Ebner (2):
schema: cloudinit: document default for ciupgrade
cloudinit: restore previous default for package upgrades
PVE/QemuServer.pm | 3 ++-
PVE/QemuServer/Cloudinit.pm | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
manager:
Fiona Ebner (1):
ui: cloudinit: align default value for package upgrades with backend
again
www/manager6/qemu/CloudInit.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 1/2] schema: cloudinit: document default for ciupgrade
Signed-off-by: Fiona Ebner
---
PVE/QemuServer.pm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 0fa43a74..91aab21c 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -785,7 +785,8 @@ my $confdesc_cloudinit = {
ciupgrade => {
optional => 1,
type => 'boolean',
- description => 'cloud-init: do an automatic package upgrade after the
first boot.'
+ description => 'cloud-init: do an automatic package upgrade after the
first boot.',
+ default => 0,
},
cicustom => {
optional => 1,
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 2/2] cloudinit: restore previous default for package upgrades
Commit efa3355d ("fix #3428: cloudinit: add parameter for upgrade on
boot") changed the default, but this is a breaking change. The bug
report was only about making the option configurable.
The commit doesn't give an explicit reason for why, and arguably,
doing the upgrade is not an issue for most users. It also leads to a
different cloud-init instance ID, because of the different setting,
which in turn leads to ssh host key regeneration within the VM.
Reported-by: Friedrich Weber
Signed-off-by: Fiona Ebner
---
PVE/QemuServer.pm | 2 +-
PVE/QemuServer/Cloudinit.pm | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 91aab21c..c9c1936a 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -786,7 +786,7 @@ my $confdesc_cloudinit = {
optional => 1,
type => 'boolean',
description => 'cloud-init: do an automatic package upgrade after the
first boot.',
- default => 0,
+ default => 1,
},
cicustom => {
optional => 1,
diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
index cd5cc242..7449993a 100644
--- a/PVE/QemuServer/Cloudinit.pm
+++ b/PVE/QemuServer/Cloudinit.pm
@@ -148,7 +148,7 @@ sub cloudinit_userdata {
$content .= " - default\n";
}
-$content .= "package_upgrade: true\n" if $conf->{ciupgrade};
+$content .= "package_upgrade: true\n" if !defined($conf->{ciupgrade}) ||
$conf->{ciupgrade};
return $content;
}
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] gui: make abort button focusable
GtkWiget:can-focus=false means that the user cannot use tab or key
navigation to interact with this button.
Signed-off-by: Maximiliano Sandoval
---
proxinstall | 1 -
1 file changed, 1 deletion(-)
diff --git a/proxinstall b/proxinstall
index 6d13892..f260a95 100755
--- a/proxinstall
+++ b/proxinstall
@@ -178,7 +178,6 @@ sub create_main_window {
my $abort = Gtk3::Button->new('_Abort');
-$abort->set_can_focus(0);
$cmdbox->pack_start($abort, 0, 0, 10);
$abort->signal_connect(clicked => sub { app_quit(-1); });
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] gui: make abort button unsensitive on last panel
It makes no sense from a UX point of view to abort an install that's
already finished.
Signed-off-by: Maximiliano Sandoval
---
proxinstall | 5 +
1 file changed, 5 insertions(+)
diff --git a/proxinstall b/proxinstall
index 6d13892..bf88b31 100755
--- a/proxinstall
+++ b/proxinstall
@@ -213,6 +213,7 @@ sub create_main_window {
$gtk_state->{next_btn} = $next_btn;
$gtk_state->{progress_bar} = Gtk3::ProgressBar->new();
$gtk_state->{progress_status} = Gtk3::Label->new('');
+$gtk_state->{abort_btn} = $abort;
Proxmox::UI::init_gtk($gtk_state, $iso_env);
@@ -1522,6 +1523,10 @@ sub create_extract_view {
return $raw_html;
};
+# It does not make sense to Abort the install at this point, whether it
+# succeded or failed makes no difference.
+$gtk_state->{abort_btn}->set_sensitive(0);
+
if ($err) {
Proxmox::UI::display_html("fail.htm");
# suppress "empty" error as we got some case where the user choose to
abort on a prompt,
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] tui: Add a cancel button to Advanced bootdisk options
This matches the GUI installer which counts with a close (x) button.
Signed-off-by: Maximiliano Sandoval
---
proxmox-tui-installer/src/views/bootdisk.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs
b/proxmox-tui-installer/src/views/bootdisk.rs
index 3fdbe5b..eaf343d 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -456,6 +456,7 @@ fn advanced_options_view(disks: &[Disk], options:
Rc>)
&(*options).borrow(),
))
.title("Advanced bootdisk options")
+.dismiss_button("Cancel")
.button("Ok", {
let options_ref = options.clone();
move |siv| {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 installer 1/1] fix #4643: show a confirmation dialog when clicking abort
Note that unlike the rest of the file, we connect to the response signal
instead of using Gtk3::Dialog->run, the reason is that run blocks the
main loop used by GTK and this undesirable to the point where
Gtk3::Dialog->run was removed for GTK 4.
Signed-off-by: Maximiliano Sandoval
---
proxinstall | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/proxinstall b/proxinstall
index 6d13892..600d5a5 100755
--- a/proxinstall
+++ b/proxinstall
@@ -180,7 +180,19 @@ sub create_main_window {
my $abort = Gtk3::Button->new('_Abort');
$abort->set_can_focus(0);
$cmdbox->pack_start($abort, 0, 0, 10);
-$abort->signal_connect(clicked => sub { app_quit(-1); });
+$abort->signal_connect(clicked => sub {
+ my $msg = 'Abort Installation';
+ my $secondary_text = 'Are you sure you want to abort the installation?';
+ my $dialog = Gtk3::MessageDialog->new($window, 'modal', 'question',
'yes-no', $msg);
+ $dialog->format_secondary_text($secondary_text);
+ $dialog->signal_connect(response => sub {
+ my ($dialog, $response) = @_;
+
+ $dialog->close();
+ app_quit(-1) if $response eq 'yes';
+ });
+ $dialog->present();
+});
my $vbox2 = Gtk3::Box->new('vertical', 0);
$hbox->add($vbox2);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 installer 0/1] fix #4643: show a confirmation dialog when clicking abort
Differences from v1: - The abort dialog now counts with a title and body text that matches the TUI installer's dialog. Unlike the TUI installer the title text does not end in a question mark (`Abort Installation?`), this is intentional as question marks look odd when set in the title of a GTK message dialog. Maximiliano Sandoval (1): fix #4643: show a confirmation dialog when clicking abort proxinstall | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH manager v2 1/5] ui: resource map tree: make 'ok' status clearer
Am 21/06/2023 um 09:41 schrieb Dominik Csapak: > by changing into 'mapping matches host data' which indicates that the > configured values matches the host information > > also for the pci and usb map selectors > > Signed-off-by: Dominik Csapak > --- > www/manager6/form/PCIMapSelector.js | 2 +- > www/manager6/form/USBMapSelector.js | 2 +- > www/manager6/tree/ResourceMapTree.js | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > applied series, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v2 3/5] ui: pci/usb map edit: improve new host mappings dialog
by disallowing nodes to be selected where a mapping already exists
Signed-off-by: Dominik Csapak
---
changes from v1:
* dont' disable autoselect, the first allowed node will be selected
* for usb mappings too
www/manager6/window/PCIMapEdit.js | 9 -
www/manager6/window/USBMapEdit.js | 4
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/www/manager6/window/PCIMapEdit.js
b/www/manager6/window/PCIMapEdit.js
index 8c1a95e3..f243362b 100644
--- a/www/manager6/window/PCIMapEdit.js
+++ b/www/manager6/window/PCIMapEdit.js
@@ -58,7 +58,13 @@ Ext.define('PVE.window.PCIMapEditWindow', {
let me = this;
let view = me.getView();
me.originalMap = [...values.map];
- values.map = PVE.Parser.filterPropertyStringList(values.map, (e) =>
e.node === view.nodename);
+ let configuredNodes = [];
+ values.map = PVE.Parser.filterPropertyStringList(values.map, (e) =>
{
+ configuredNodes.push(e.node);
+ return e.node === view.nodename;
+ });
+
+ me.lookup('nodeselector').disallowedNodes = configuredNodes;
return values;
},
@@ -203,6 +209,7 @@ Ext.define('PVE.window.PCIMapEditWindow', {
name: 'node',
editConfig: {
xtype: 'pveNodeSelector',
+ reference: 'nodeselector',
},
cbind: {
editable: '{!nodename}',
diff --git a/www/manager6/window/USBMapEdit.js
b/www/manager6/window/USBMapEdit.js
index 80f8e785..f36f1d03 100644
--- a/www/manager6/window/USBMapEdit.js
+++ b/www/manager6/window/USBMapEdit.js
@@ -71,13 +71,16 @@ Ext.define('PVE.window.USBMapEditWindow', {
let me = this;
let view = me.getView();
me.originalMap = [...values.map];
+ let configuredNodes = [];
PVE.Parser.filterPropertyStringList(values.map, (e) => {
+ configuredNodes.push(e.node);
if (e.node === view.nodename) {
values = e;
}
return false;
});
+ me.lookup('nodeselector').disallowedNodes = configuredNodes;
if (values.path) {
values.usb = 'path';
}
@@ -145,6 +148,7 @@ Ext.define('PVE.window.USBMapEditWindow', {
name: 'node',
editConfig: {
xtype: 'pveNodeSelector',
+ reference: 'nodeselector',
},
cbind: {
editable: '{!nodename}',
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v2 1/5] ui: resource map tree: make 'ok' status clearer
by changing into 'mapping matches host data' which indicates that the
configured values matches the host information
also for the pci and usb map selectors
Signed-off-by: Dominik Csapak
---
www/manager6/form/PCIMapSelector.js | 2 +-
www/manager6/form/USBMapSelector.js | 2 +-
www/manager6/tree/ResourceMapTree.js | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/www/manager6/form/PCIMapSelector.js
b/www/manager6/form/PCIMapSelector.js
index 4dca62ea..1bc73ec0 100644
--- a/www/manager6/form/PCIMapSelector.js
+++ b/www/manager6/form/PCIMapSelector.js
@@ -48,7 +48,7 @@ Ext.define('PVE.form.PCIMapSelector', {
let me = this;
if (!Ext.isArray(value) || !value?.length) {
- return `
${gettext('Mapping OK')}`;
+ return `
${gettext('Mapping matches host data')}`;
}
let checks = [];
diff --git a/www/manager6/form/USBMapSelector.js
b/www/manager6/form/USBMapSelector.js
index 990ef30f..6a33754a 100644
--- a/www/manager6/form/USBMapSelector.js
+++ b/www/manager6/form/USBMapSelector.js
@@ -34,7 +34,7 @@ Ext.define('PVE.form.USBMapSelector', {
let me = this;
if (!Ext.isArray(value) || !value?.length) {
- return `
${gettext('Mapping OK')}`;
+ return `
${gettext('Mapping matches host data')}`;
}
let errors = [];
diff --git a/www/manager6/tree/ResourceMapTree.js
b/www/manager6/tree/ResourceMapTree.js
index df50b63a..02717042 100644
--- a/www/manager6/tree/ResourceMapTree.js
+++ b/www/manager6/tree/ResourceMapTree.js
@@ -228,7 +228,7 @@ Ext.define('PVE.tree.ResourceMapTree', {
} else {
let state = value ? 'good' : 'critical';
iconCls = PVE.Utils.get_health_icon(state, true);
- status = value ? gettext("OK") : record.data.errmsg ||
Proxmox.Utils.unknownText;
+ status = value ? gettext("Mapping matches host data") :
record.data.errmsg || Proxmox.Utils.unknownText;
}
return ` ${status}`;
},
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v2 5/5] ui: pci/usb mapping: rework mapping panel for better user experience
by removing the confusing buttons in the toolbar and adding them as
actions in an actioncolumn. There a only relevant actions are visible
and get a more expressive tooltip
with this, we now differentiate between 4 modes of the edit window:
* create a new mapping altogether
- shows all fields
* edit existing mapping on top level
- show only 'global' fields (comment, mdev), so no mappings
* add new host mapping
- shows nodeselector, mapping (and mdev, but disabled)
(informational only)
* edit existing host mapping
- show selected node (displayfield) mdev and mappings, but only
mappings are editable
we have to split the nodeselector into two fields, since the disabling
cbind does not pass through to the editconfig (and thus makes the form
invalid if we try that)
Signed-off-by: Dominik Csapak
---
changes from rfc:
* for usb mappings too, so they are consistent
* moved the action column to second place
* hide/disable add button when there are mappings for every node
* use a bit different class to hide the buttons, because otherwise
the grid cells would have the wrong height (since they would not
get the styling of the font-awesome icon)
www/css/ext6-pve.css | 5 +
www/manager6/tree/ResourceMapTree.js | 178 +--
www/manager6/window/PCIMapEdit.js| 40 --
www/manager6/window/USBMapEdit.js| 49 ++--
4 files changed, 183 insertions(+), 89 deletions(-)
diff --git a/www/css/ext6-pve.css b/www/css/ext6-pve.css
index 3af64255..edae462b 100644
--- a/www/css/ext6-pve.css
+++ b/www/css/ext6-pve.css
@@ -704,3 +704,8 @@ table.osds td:first-of-type {
.x-grid-item .x-item-disabled {
opacity: 0.3;
}
+
+.pmx-action-hidden:before {
+opacity: 0.0;
+cursor: default;
+}
diff --git a/www/manager6/tree/ResourceMapTree.js
b/www/manager6/tree/ResourceMapTree.js
index 02717042..4c476909 100644
--- a/www/manager6/tree/ResourceMapTree.js
+++ b/www/manager6/tree/ResourceMapTree.js
@@ -49,44 +49,89 @@ Ext.define('PVE.tree.ResourceMapTree', {
});
},
- addHost: function() {
+ add: function(_grid, _rI, _cI, _item, _e, rec) {
let me = this;
- me.edit(false);
+ if (rec.data.type !== 'entry') {
+ return;
+ }
+
+ me.openMapEditWindow(rec.data.name);
},
- edit: function(includeNodename = true) {
+ editDblClick: function() {
let me = this;
let view = me.getView();
let selection = view.getSelection();
- if (!selection || !selection.length) {
+ if (!selection || selection.length < 1) {
return;
}
- let rec = selection[0];
- if (!view.canConfigure || (rec.data.type === 'entry' &&
includeNodename)) {
+
+ me.edit(selection[0]);
+ },
+
+ editAction: function(_grid, _rI, _cI, _item, _e, rec) {
+ this.edit(rec);
+ },
+
+ edit: function(rec) {
+ let me = this;
+ if (rec.data.type === 'map') {
return;
}
+ me.openMapEditWindow(rec.data.name, rec.data.node, rec.data.type
=== 'entry');
+ },
+
+ openMapEditWindow: function(name, nodename, entryOnly) {
+ let me = this;
+ let view = me.getView();
+
Ext.create(view.editWindowClass, {
- url: `${view.baseUrl}/${rec.data.name}`,
+ url: `${view.baseUrl}/${name}`,
autoShow: true,
autoLoad: true,
- nodename: includeNodename ? rec.data.node : undefined,
- name: rec.data.name,
+ entryOnly,
+ nodename,
+ name,
listeners: {
destroy: () => me.load(),
},
});
},
- remove: function() {
+ remove: function(_grid, _rI, _cI, _item, _e, rec) {
let me = this;
+ let msg, id;
let view = me.getView();
- let selection = view.getSelection();
- if (!selection || !selection.length) {
- return;
+ let confirmMsg;
+ switch (rec.data.type) {
+ case 'entry':
+ msg = gettext("Are you sure you want to remove '{0}'");
+ confirmMsg = Ext.String.format(msg, rec.data.name);
+ break;
+ case 'node':
+ msg = gettext("Are you sure you want to remove '{0}'
entries for '{1}'");
+ confirmMsg = Ext.String.format(msg, rec.data.node,
rec.data.name);
+ break;
+ case 'map':
+ msg = gettext("Are you sure you want to remove '{0}' on
'{1}' for '{2}'");
+ id = rec.data[view.entryIdProperty];
+ confirmMsg = Ext.String.format(msg, id, rec.data.node,
rec.data.name);
+ break;
+
[pve-devel] [PATCH manager v2 2/5] ui: pci map edit: reintroduce warnings checks
they got lost in my last rebase/refactor.
the onLoadCallBack is used to check by the window if there are iommu
groups at all, and the checkIsolated function checks if the selected
ones are in a separate group (in regards to the other devices)
Signed-off-by: Dominik Csapak
---
factored out the store lookup
@fiona, i opted to get the selected values via getValue, because via the
store it's not really possible, and via the selector requires knowledge
of how the selector works internally (lookup the grid, get the selection)
www/manager6/form/MultiPCISelector.js | 5
www/manager6/window/PCIMapEdit.js | 41 +++
2 files changed, 46 insertions(+)
diff --git a/www/manager6/form/MultiPCISelector.js
b/www/manager6/form/MultiPCISelector.js
index e1ef691a..d4fb6364 100644
--- a/www/manager6/form/MultiPCISelector.js
+++ b/www/manager6/form/MultiPCISelector.js
@@ -8,6 +8,9 @@ Ext.define('PVE.form.MultiPCISelector', {
field: 'Ext.form.field.Field',
},
+// will be called after loading finished
+onLoadCallBack: Ext.emptyFn,
+
getValue: function() {
let me = this;
return me.value ?? [];
@@ -287,6 +290,8 @@ Ext.define('PVE.form.MultiPCISelector', {
me.callParent();
+ me.mon(me.getStore(), 'load', me.onLoadCallBack);
+
Proxmox.Utils.monStoreErrors(me, me.getStore(), true);
me.setNodename(nodename);
diff --git a/www/manager6/window/PCIMapEdit.js
b/www/manager6/window/PCIMapEdit.js
index 516678e0..8c1a95e3 100644
--- a/www/manager6/window/PCIMapEdit.js
+++ b/www/manager6/window/PCIMapEdit.js
@@ -70,6 +70,46 @@ Ext.define('PVE.window.PCIMapEditWindow', {
me.lookup('iommu_warning').setVisible(
records.every((val) => val.data.iommugroup === -1),
);
+
+ let value = me.lookup('pciselector').getValue();
+ me.checkIsolated(value);
+ },
+
+ checkIsolated: function(value) {
+ let me = this;
+
+ let store = me.lookup('pciselector').getStore();
+
+ let isIsolated = function(entry) {
+ let isolated = true;
+ let parsed = PVE.Parser.parsePropertyString(entry);
+ parsed.iommugroup = parseInt(parsed.iommugroup, 10);
+ if (!parsed.iommugroup) {
+ return isolated;
+ }
+ store.each(({ data }) => {
+ let isSubDevice = data.id.startsWith(parsed.path);
+ if (data.iommugroup === parsed.iommugroup && data.id !==
parsed.path && !isSubDevice) {
+ isolated = false;
+ return false;
+ }
+ return true;
+ });
+ return isolated;
+ };
+
+ let showWarning = false;
+ if (Ext.isArray(value)) {
+ for (const entry of value) {
+ if (!isIsolated(entry)) {
+ showWarning = true;
+ break;
+ }
+ }
+ } else {
+ showWarning = isIsolated(value);
+ }
+ me.lookup('group_warning').setVisible(showWarning);
},
mdevChange: function(mdevField, value) {
@@ -83,6 +123,7 @@ Ext.define('PVE.window.PCIMapEditWindow', {
pciChange: function(_field, value) {
let me = this;
me.lookup('multiple_warning').setVisible(Ext.isArray(value) &&
value.length > 1);
+ me.checkIsolated(value);
},
control: {
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v2 4/5] ui: pci map edit: fix typos in warnings and use gettexts
so they can be translated
Signed-off-by: Dominik Csapak
---
new in v2
www/manager6/window/PCIMapEdit.js | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/www/manager6/window/PCIMapEdit.js
b/www/manager6/window/PCIMapEdit.js
index f243362b..2b268719 100644
--- a/www/manager6/window/PCIMapEdit.js
+++ b/www/manager6/window/PCIMapEdit.js
@@ -163,8 +163,7 @@ Ext.define('PVE.window.PCIMapEditWindow', {
hidden: true,
columnWidth: 1,
padding: '0 0 10 0',
- value: 'No IOMMU detected, please activate it.' +
- 'See Documentation for further information.',
+ value: gettext('No IOMMU detected, please activate it. See
Documentation for further information.'),
userCls: 'pmx-hint',
},
{
@@ -173,8 +172,7 @@ Ext.define('PVE.window.PCIMapEditWindow', {
hidden: true,
columnWidth: 1,
padding: '0 0 10 0',
- value: 'When multiple devices are selected, the first free
one will be chosen' +
- ' on guest start.',
+ value: gettext('When multiple devices are selected, the
first free one will be chosen on guest start.'),
userCls: 'pmx-hint',
},
{
@@ -184,7 +182,7 @@ Ext.define('PVE.window.PCIMapEditWindow', {
columnWidth: 1,
padding: '0 0 10 0',
itemId: 'iommuwarning',
- value: 'The selected Device is not in a seperate IOMMU
group, make sure this is intended.',
+ value: gettext('A selected device is not in a separate
IOMMU group, make sure this is intended.'),
userCls: 'pmx-hint',
},
],
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH manager 1/2] api: ceph: remove deprecrated Pools path
Am 15/06/2023 um 09:39 schrieb Aaron Lauterer: > The replacement is Pool (singular). > > Signed-off-by: Aaron Lauterer > --- > PVE/API2/Ceph.pm | 7 - > PVE/API2/Ceph/Makefile | 1 - > PVE/API2/Ceph/Pools.pm | 766 - > 3 files changed, 774 deletions(-) > delete mode 100644 PVE/API2/Ceph/Pools.pm > > applied, with a follow-up for the API route endpoint directory index, dropping "config", "configdb" and s/pools/pool/; thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
