* [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements
@ 2024-11-25 11:27 Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 1/6] auto, tui: move low-level installer message struct to common crate Christoph Heiss
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
To: pve-devel
This series tries to improve upon some small things around the
installation progress reporting, esp. in the auto-installer, as well as
some small fixes & code-deduplication.
History
=======
v1: https://lore.proxmox.com/pve-devel/20240516133945.1087246-1-c.heiss@proxmox.com/
Changes v1 -> v2:
* dropped already-applied patches
* rebased on latest master
* added new patch #6
Diffstat
========
Christoph Heiss (6):
auto, tui: move low-level installer message struct to common crate
auto: log non-json low-level messages into separate file
low-level: stdio: fix: make progress text properly optional
low-level: add proper message to 100% progress ratio update
auto: avoid printing unnecessary progress update lines
tui: tests: catch EOF from proxmox-low-level-installer early
Proxmox/Install.pm | 2 +-
Proxmox/UI/StdIO.pm | 8 +-
.../src/bin/proxmox-auto-installer.rs | 41 ++++++++--
proxmox-auto-installer/src/utils.rs | 23 ------
proxmox-installer-common/src/setup.rs | 23 ++++++
.../src/views/install_progress.rs | 76 ++++++++-----------
test/ui2-stdio.pl | 2 +-
7 files changed, 98 insertions(+), 77 deletions(-)
--
2.44.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH installer v2 1/6] auto, tui: move low-level installer message struct to common crate
2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
@ 2024-11-25 11:27 ` Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 2/6] auto: log non-json low-level messages into separate file Christoph Heiss
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
To: pve-devel
.. effectively de-duplicating the struct, which currently is defined in
both the auto-installer and the tui-installer separately.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
.../src/bin/proxmox-auto-installer.rs | 5 +-
proxmox-auto-installer/src/utils.rs | 23 ---------
proxmox-installer-common/src/setup.rs | 23 +++++++++
.../src/views/install_progress.rs | 50 +++++--------------
4 files changed, 39 insertions(+), 62 deletions(-)
diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index 4b9d73d..151694f 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -10,7 +10,8 @@ use std::{
use proxmox_installer_common::{
http,
setup::{
- installer_setup, read_json, spawn_low_level_installer, LocaleInfo, RuntimeInfo, SetupInfo,
+ installer_setup, read_json, spawn_low_level_installer, LocaleInfo, LowLevelMessage,
+ RuntimeInfo, SetupInfo,
},
FIRST_BOOT_EXEC_MAX_SIZE, FIRST_BOOT_EXEC_NAME, RUNTIME_DIR,
};
@@ -19,7 +20,7 @@ use proxmox_auto_installer::{
answer::{Answer, FirstBootHookInfo, FirstBootHookSourceMode},
log::AutoInstLogger,
udevinfo::UdevInfo,
- utils::{parse_answer, LowLevelMessage},
+ utils::parse_answer,
};
static LOGGER: AutoInstLogger = AutoInstLogger;
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index ea7176a..2be81ea 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -441,26 +441,3 @@ pub fn parse_answer(
Ok(config)
}
-
-#[derive(Clone, Debug, Deserialize, PartialEq)]
-#[serde(tag = "type", rename_all = "lowercase")]
-pub enum LowLevelMessage {
- #[serde(rename = "message")]
- Info {
- message: String,
- },
- Error {
- message: String,
- },
- Prompt {
- query: String,
- },
- Finished {
- state: String,
- message: String,
- },
- Progress {
- ratio: f32,
- text: String,
- },
-}
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 1a9a71c..e22ce4e 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -551,3 +551,26 @@ pub struct InstallConfig {
pub first_boot: InstallFirstBootSetup,
}
+
+#[derive(Clone, Debug, Deserialize, PartialEq)]
+#[serde(tag = "type", rename_all = "lowercase")]
+pub enum LowLevelMessage {
+ #[serde(rename = "message")]
+ Info {
+ message: String,
+ },
+ Error {
+ message: String,
+ },
+ Prompt {
+ query: String,
+ },
+ Finished {
+ state: String,
+ message: String,
+ },
+ Progress {
+ ratio: f32,
+ text: String,
+ },
+}
diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs
index 4b4a418..585877c 100644
--- a/proxmox-tui-installer/src/views/install_progress.rs
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -4,7 +4,6 @@ use cursive::{
views::{Dialog, DummyView, LinearLayout, PaddedView, ProgressBar, TextView},
CbSink, Cursive,
};
-use serde::Deserialize;
use std::{
fs::File,
io::{BufRead, BufReader, Write},
@@ -14,7 +13,7 @@ use std::{
};
use crate::{abort_install_button, prompt_dialog, InstallerState};
-use proxmox_installer_common::setup::{spawn_low_level_installer, InstallConfig};
+use proxmox_installer_common::setup::{spawn_low_level_installer, InstallConfig, LowLevelMessage};
pub struct InstallProgressView {
view: PaddedView<LinearLayout>,
@@ -105,7 +104,7 @@ impl InstallProgressView {
continue;
}
- let msg = match serde_json::from_str::<UiMessage>(&line) {
+ let msg = match serde_json::from_str::<LowLevelMessage>(&line) {
Ok(msg) => msg,
Err(err) => {
// Not a fatal error, so don't abort the installation by returning
@@ -116,17 +115,17 @@ impl InstallProgressView {
};
let result = match msg.clone() {
- UiMessage::Info { message } => cb_sink.send(Box::new(|siv| {
+ LowLevelMessage::Info { message } => cb_sink.send(Box::new(|siv| {
siv.add_layer(Dialog::info(message).title("Information"));
})),
- UiMessage::Error { message } => cb_sink.send(Box::new(|siv| {
+ LowLevelMessage::Error { message } => cb_sink.send(Box::new(|siv| {
siv.add_layer(Dialog::info(message).title("Error"));
})),
- UiMessage::Prompt { query } => cb_sink.send({
+ LowLevelMessage::Prompt { query } => cb_sink.send({
let writer = writer.clone();
Box::new(move |siv| Self::show_prompt(siv, &query, writer))
}),
- UiMessage::Progress { ratio, text } => {
+ LowLevelMessage::Progress { ratio, text } => {
counter.set((ratio * 100.).floor() as usize);
cb_sink.send(Box::new(move |siv| {
siv.call_on_name(Self::PROGRESS_TEXT_VIEW_ID, |v: &mut TextView| {
@@ -134,7 +133,7 @@ impl InstallProgressView {
});
}))
}
- UiMessage::Finished { state, message } => {
+ LowLevelMessage::Finished { state, message } => {
counter.set(100);
cb_sink.send(Box::new(move |siv| {
siv.call_on_name(Self::PROGRESS_TEXT_VIEW_ID, |v: &mut TextView| {
@@ -245,39 +244,16 @@ impl ViewWrapper for InstallProgressView {
cursive::wrap_impl!(self.view: PaddedView<LinearLayout>);
}
-#[derive(Clone, Debug, Deserialize, PartialEq)]
-#[serde(tag = "type", rename_all = "lowercase")]
-enum UiMessage {
- #[serde(rename = "message")]
- Info {
- message: String,
- },
- Error {
- message: String,
- },
- Prompt {
- query: String,
- },
- Finished {
- state: String,
- message: String,
- },
- Progress {
- ratio: f32,
- text: String,
- },
-}
-
#[cfg(test)]
mod tests {
use super::*;
use std::env;
- fn next_msg<R: BufRead>(reader: &mut R) -> Option<UiMessage> {
+ fn next_msg<R: BufRead>(reader: &mut R) -> Option<LowLevelMessage> {
let mut line = String::new();
reader.read_line(&mut line).expect("a line");
- match serde_json::from_str::<UiMessage>(&line) {
+ match serde_json::from_str::<LowLevelMessage>(&line) {
Ok(msg) => Some(msg),
Err(err) => {
eprintln!("invalid json: '{err}'");
@@ -310,7 +286,7 @@ mod tests {
assert_eq!(
next_msg(&mut reader),
- Some(UiMessage::Prompt {
+ Some(LowLevelMessage::Prompt {
query: "Reply anything?".to_owned()
}),
);
@@ -324,7 +300,7 @@ mod tests {
assert_eq!(
next_msg(&mut reader),
- Some(UiMessage::Info {
+ Some(LowLevelMessage::Info {
message: "Test Message - got ok".to_owned()
}),
);
@@ -332,7 +308,7 @@ mod tests {
for i in (1..=1000).step_by(3) {
assert_eq!(
next_msg(&mut reader),
- Some(UiMessage::Progress {
+ Some(LowLevelMessage::Progress {
ratio: (i as f32) / 1000.,
text: format!("foo {i}"),
}),
@@ -341,7 +317,7 @@ mod tests {
assert_eq!(
next_msg(&mut reader),
- Some(UiMessage::Finished {
+ Some(LowLevelMessage::Finished {
state: "ok".to_owned(),
message: "Installation finished - reboot now?".to_owned(),
}),
--
2.47.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH installer v2 2/6] auto: log non-json low-level messages into separate file
2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 1/6] auto, tui: move low-level installer message struct to common crate Christoph Heiss
@ 2024-11-25 11:27 ` Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 3/6] low-level: stdio: fix: make progress text properly optional Christoph Heiss
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
To: pve-devel
Same as the TUI does, as introduced in [0].
[0] 22de6e5 ("tui: install_progress: write low-level non-JSON messages to separate file")
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
.../src/bin/proxmox-auto-installer.rs | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index 151694f..2143838 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -1,7 +1,8 @@
use anyhow::{bail, format_err, Result};
use log::{error, info, LevelFilter};
use std::{
- env, fs,
+ env,
+ fs::{self, File},
io::{BufRead, BufReader, Write},
path::PathBuf,
process::ExitCode,
@@ -169,15 +170,29 @@ fn run_installation(
.map_err(|err| format_err!("failed to serialize install config: {err}"))?;
writeln!(writer).map_err(|err| format_err!("failed to write install config: {err}"))?;
+ let mut lowlevel_log = File::create("/tmp/install-low-level.log")
+ .map_err(|err| format_err!("failed to open low-level installer logfile: {err}"))?;
+
for line in reader.lines() {
let line = match line {
Ok(line) => line,
Err(_) => break,
};
+
+ // The low-level installer also spews the output of any command it runs on its
+ // stdout. Use a very simple heuricstic to determine whether it is actually JSON
+ // or not.
+ if !line.starts_with('{') || !line.ends_with('}') {
+ let _ = writeln!(lowlevel_log, "{}", line);
+ continue;
+ }
+
let msg = match serde_json::from_str::<LowLevelMessage>(&line) {
Ok(msg) => msg,
- Err(_) => {
+ Err(err) => {
// Not a fatal error, so don't abort the installation by returning
+ eprintln!("low-level installer: error while parsing message: '{err}'");
+ eprintln!(" original message was: '{line}'");
continue;
}
};
--
2.47.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH installer v2 3/6] low-level: stdio: fix: make progress text properly optional
2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 1/6] auto, tui: move low-level installer message struct to common crate Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 2/6] auto: log non-json low-level messages into separate file Christoph Heiss
@ 2024-11-25 11:27 ` Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 4/6] low-level: add proper message to 100% progress ratio update Christoph Heiss
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
To: pve-devel
.. such that receivers can differentiate between these two cases more
clearly.
Sometimes, the `progress` sub does not get passed a text (on purpose!),
just updating the progress ratio. This would cause log messages to be
written out which could indicate missing text and look rather weird.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
Proxmox/UI/StdIO.pm | 8 +++++---
.../src/bin/proxmox-auto-installer.rs | 6 +++++-
proxmox-installer-common/src/setup.rs | 2 +-
.../src/views/install_progress.rs | 19 +++++++++++++------
test/ui2-stdio.pl | 2 +-
5 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/Proxmox/UI/StdIO.pm b/Proxmox/UI/StdIO.pm
index 2838fcb..ec9eac9 100644
--- a/Proxmox/UI/StdIO.pm
+++ b/Proxmox/UI/StdIO.pm
@@ -70,9 +70,11 @@ sub display_html {
sub progress {
my ($self, $ratio, $text) = @_;
- $text = '' if !defined($text);
-
- send_msg('progress', ratio => $ratio, text => $text);
+ if (defined($text)) {
+ send_msg('progress', ratio => $ratio, text => $text);
+ } else {
+ send_msg('progress', ratio => $ratio);
+ }
}
sub process_events {
diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index 2143838..9d1dff4 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -205,7 +205,11 @@ fn run_installation(
}
LowLevelMessage::Progress { ratio, text } => {
let percentage = ratio * 100.;
- info!("progress {percentage:>5.1} % - {text}");
+ if let Some(text) = text {
+ info!("progress {percentage:>5.1} % - {text}");
+ } else {
+ info!("progress {percentage:>5.1} %");
+ }
}
LowLevelMessage::Finished { state, message } => {
if state == "err" {
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index e22ce4e..4b4920e 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -571,6 +571,6 @@ pub enum LowLevelMessage {
},
Progress {
ratio: f32,
- text: String,
+ text: Option<String>,
},
}
diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs
index 585877c..303da0e 100644
--- a/proxmox-tui-installer/src/views/install_progress.rs
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -127,11 +127,18 @@ impl InstallProgressView {
}),
LowLevelMessage::Progress { ratio, text } => {
counter.set((ratio * 100.).floor() as usize);
- cb_sink.send(Box::new(move |siv| {
- siv.call_on_name(Self::PROGRESS_TEXT_VIEW_ID, |v: &mut TextView| {
- v.set_content(text);
- });
- }))
+ if let Some(text) = text {
+ cb_sink.send(Box::new(move |siv| {
+ siv.call_on_name(
+ Self::PROGRESS_TEXT_VIEW_ID,
+ |v: &mut TextView| {
+ v.set_content(text);
+ },
+ );
+ }))
+ } else {
+ Ok(())
+ }
}
LowLevelMessage::Finished { state, message } => {
counter.set(100);
@@ -310,7 +317,7 @@ mod tests {
next_msg(&mut reader),
Some(LowLevelMessage::Progress {
ratio: (i as f32) / 1000.,
- text: format!("foo {i}"),
+ text: Some(format!("foo {i}")),
}),
);
}
diff --git a/test/ui2-stdio.pl b/test/ui2-stdio.pl
index 01f323d..eebb5c9 100755
--- a/test/ui2-stdio.pl
+++ b/test/ui2-stdio.pl
@@ -81,7 +81,7 @@ if ($child_pid) {
'should get 20% done progress message');
is_deeply(
- $next_msg->(), { type => 'progress', ratio => 0.2, text => '' },
+ $next_msg->(), { type => 'progress', ratio => 0.2, },
'should get progress continuation message');
is_deeply(
--
2.47.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH installer v2 4/6] low-level: add proper message to 100% progress ratio update
2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
` (2 preceding siblings ...)
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 3/6] low-level: stdio: fix: make progress text properly optional Christoph Heiss
@ 2024-11-25 11:27 ` Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 5/6] auto: avoid printing unnecessary progress update lines Christoph Heiss
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
Proxmox/Install.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index b891383..9074709 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -1460,7 +1460,7 @@ _EOD
my $err = $@;
- update_progress(1, 0, 1, "");
+ update_progress(1, 0, 1, "installation finished");
print STDERR $err if $err && $err ne "\n";
--
2.47.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH installer v2 5/6] auto: avoid printing unnecessary progress update lines
2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
` (3 preceding siblings ...)
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 4/6] low-level: add proper message to 100% progress ratio update Christoph Heiss
@ 2024-11-25 11:27 ` Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 6/6] tui: tests: catch EOF from proxmox-low-level-installer early Christoph Heiss
2024-11-25 22:25 ` [pve-devel] applied-series: [PATCH installer v2 0/6] small, overall install progress improvements Thomas Lamprecht
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
To: pve-devel
In case the progress message did not contain any text and the percentage
did not change, don't print another (useless) line.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* if the percentage was updated w/o any text, reuse the last message
.../src/bin/proxmox-auto-installer.rs | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index 9d1dff4..ece9a94 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -173,6 +173,9 @@ fn run_installation(
let mut lowlevel_log = File::create("/tmp/install-low-level.log")
.map_err(|err| format_err!("failed to open low-level installer logfile: {err}"))?;
+ let mut last_progress_percentage = 0.;
+ let mut last_progress_text = None;
+
for line in reader.lines() {
let line = match line {
Ok(line) => line,
@@ -207,8 +210,16 @@ fn run_installation(
let percentage = ratio * 100.;
if let Some(text) = text {
info!("progress {percentage:>5.1} % - {text}");
- } else {
- info!("progress {percentage:>5.1} %");
+ last_progress_percentage = percentage;
+ last_progress_text = Some(text);
+ } else if (percentage - last_progress_percentage) > 0.1 {
+ if let Some(text) = &last_progress_text {
+ info!("progress {percentage:>5.1} % - {text}");
+ } else {
+ info!("progress {percentage:>5.1} %");
+ }
+
+ last_progress_percentage = percentage;
}
}
LowLevelMessage::Finished { state, message } => {
--
2.47.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH installer v2 6/6] tui: tests: catch EOF from proxmox-low-level-installer early
2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
` (4 preceding siblings ...)
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 5/6] auto: avoid printing unnecessary progress update lines Christoph Heiss
@ 2024-11-25 11:27 ` Christoph Heiss
2024-11-25 22:25 ` [pve-devel] applied-series: [PATCH installer v2 0/6] small, overall install progress improvements Thomas Lamprecht
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
To: pve-devel
This can happen if e.g. not all required testdata is available, such as
.cd-info - in which case proxmox-low-level-installer will fail early
with
could not open CD info file '/.cd-info' - No such file or directory at Proxmox/Install/ISOEnv.pm line 95.
Currently, next_msg() would just be recursively called until the stack
overflowed in such a case.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* new patch
proxmox-tui-installer/src/views/install_progress.rs | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs
index 303da0e..d001451 100644
--- a/proxmox-tui-installer/src/views/install_progress.rs
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -258,7 +258,12 @@ mod tests {
fn next_msg<R: BufRead>(reader: &mut R) -> Option<LowLevelMessage> {
let mut line = String::new();
- reader.read_line(&mut line).expect("a line");
+
+ match reader.read_line(&mut line) {
+ Ok(0) => return None, /* reached EOF */
+ Err(err) => panic!("failed to read message: {err}"),
+ _ => {}
+ }
match serde_json::from_str::<LowLevelMessage>(&line) {
Ok(msg) => Some(msg),
--
2.47.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] applied-series: [PATCH installer v2 0/6] small, overall install progress improvements
2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
` (5 preceding siblings ...)
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 6/6] tui: tests: catch EOF from proxmox-low-level-installer early Christoph Heiss
@ 2024-11-25 22:25 ` Thomas Lamprecht
6 siblings, 0 replies; 8+ messages in thread
From: Thomas Lamprecht @ 2024-11-25 22:25 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
Am 25.11.24 um 12:27 schrieb Christoph Heiss:
> This series tries to improve upon some small things around the
> installation progress reporting, esp. in the auto-installer, as well as
> some small fixes & code-deduplication.
>
> History
> =======
>
> v1: https://lore.proxmox.com/pve-devel/20240516133945.1087246-1-c.heiss@proxmox.com/
>
> Changes v1 -> v2:
> * dropped already-applied patches
> * rebased on latest master
> * added new patch #6
>
> Diffstat
> ========
>
> Christoph Heiss (6):
> auto, tui: move low-level installer message struct to common crate
> auto: log non-json low-level messages into separate file
> low-level: stdio: fix: make progress text properly optional
> low-level: add proper message to 100% progress ratio update
> auto: avoid printing unnecessary progress update lines
> tui: tests: catch EOF from proxmox-low-level-installer early
>
> Proxmox/Install.pm | 2 +-
> Proxmox/UI/StdIO.pm | 8 +-
> .../src/bin/proxmox-auto-installer.rs | 41 ++++++++--
> proxmox-auto-installer/src/utils.rs | 23 ------
> proxmox-installer-common/src/setup.rs | 23 ++++++
> .../src/views/install_progress.rs | 76 ++++++++-----------
> test/ui2-stdio.pl | 2 +-
> 7 files changed, 98 insertions(+), 77 deletions(-)
>
applied series, thanks!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-11-25 22:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 1/6] auto, tui: move low-level installer message struct to common crate Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 2/6] auto: log non-json low-level messages into separate file Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 3/6] low-level: stdio: fix: make progress text properly optional Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 4/6] low-level: add proper message to 100% progress ratio update Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 5/6] auto: avoid printing unnecessary progress update lines Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 6/6] tui: tests: catch EOF from proxmox-low-level-installer early Christoph Heiss
2024-11-25 22:25 ` [pve-devel] applied-series: [PATCH installer v2 0/6] small, overall install progress improvements Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox