[PATCH 0/2] virsh: Improve console functionality

*** APRIL BLURBS *** Andrea Bolognani (2): virsh: Show 'connected to console' message later virsh: Display more empathy towards inconsolable VMs :( tools/virsh-console.c | 24 +++++++++++++++++------- tools/virsh-domain.c | 7 ------- 2 files changed, 17 insertions(+), 14 deletions(-) -- 2.44.0

Right now, we display the message before actually attempting to connect to the VM console. That operation, however, can fail for a number of reasons: for example, is the VM doesn't have a serial device, the output ends up looking like $ virsh console cirros Connected to domain 'cirros' Escape character is ^] (Ctrl + ]) error: internal error: cannot find character device <null> The initial message is misleading. Change things so that it's only printed if we actually successfully connected to the VM console. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tools/virsh-console.c | 19 +++++++++++++------ tools/virsh-domain.c | 7 ------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tools/virsh-console.c b/tools/virsh-console.c index 7c561a11f3..27f2d09f5f 100644 --- a/tools/virsh-console.c +++ b/tools/virsh-console.c @@ -418,12 +418,6 @@ virshRunConsole(vshControl *ctl, sigemptyset(&sighandler.sa_mask); - /* Put STDIN into raw mode so that stuff typed does not echo to the screen - * (the TTY reads will result in it being echoed back already), and also - * ensure Ctrl-C, etc is blocked, and misc other bits */ - if (vshTTYMakeRaw(ctl, true) < 0) - goto resettty; - if (!(con = virConsoleNew())) goto resettty; @@ -447,6 +441,19 @@ virshRunConsole(vshControl *ctl, if (virDomainOpenConsole(dom, dev_name, con->st, flags) < 0) goto cleanup; + vshPrintExtra(ctl, _("Connected to domain '%1$s'\n"), virDomainGetName(dom)); + vshPrintExtra(ctl, _("Escape character is %1$s"), priv->escapeChar); + if (priv->escapeChar[0] == '^') + vshPrintExtra(ctl, " (Ctrl + %c)", priv->escapeChar[1]); + vshPrintExtra(ctl, "\n"); + fflush(stdout); + + /* Put STDIN into raw mode so that stuff typed does not echo to the screen + * (the TTY reads will result in it being echoed back already), and also + * ensure Ctrl-C, etc is blocked, and misc other bits */ + if (vshTTYMakeRaw(ctl, true) < 0) + goto cleanup; + virObjectRef(con); if ((con->stdinWatch = virEventAddHandle(STDIN_FILENO, VIR_EVENT_HANDLE_READABLE, diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cd37828660..148b78e878 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2987,7 +2987,6 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, unsigned int flags) { int state; - virshControl *priv = ctl->privData; if ((state = virshDomainState(ctl, dom, NULL)) < 0) { vshError(ctl, "%s", _("Unable to get domain status")); @@ -3004,12 +3003,6 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, return false; } - vshPrintExtra(ctl, _("Connected to domain '%1$s'\n"), virDomainGetName(dom)); - vshPrintExtra(ctl, _("Escape character is %1$s"), priv->escapeChar); - if (priv->escapeChar[0] == '^') - vshPrintExtra(ctl, " (Ctrl + %c)", priv->escapeChar[1]); - vshPrintExtra(ctl, "\n"); - fflush(stdout); if (virshRunConsole(ctl, dom, name, resume_domain, flags) == 0) return true; -- 2.44.0

On Mon, Apr 01, 2024 at 15:58:30 +0200, Andrea Bolognani wrote:
Right now, we display the message before actually attempting to connect to the VM console. That operation, however, can fail for a number of reasons: for example, is the VM doesn't have a serial device, the output ends up looking like
$ virsh console cirros Connected to domain 'cirros' Escape character is ^] (Ctrl + ]) error: internal error: cannot find character device <null>
The initial message is misleading. Change things so that it's only printed if we actually successfully connected to the VM console.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tools/virsh-console.c | 19 +++++++++++++------ tools/virsh-domain.c | 7 ------- 2 files changed, 13 insertions(+), 13 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

After attempting, and failing, to console a sad VM, we get back to the user with a fairly sterile message such as error: internal error: character device serial0 is not using a PTY That doesn't properly communicate the extent of our regret for having been unable to cheer up the poor VM despite our best efforts. Try to put this sentiment into words; to further carry the message, also include a tasteful ASCII rendition of our sorrow. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tools/virsh-console.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/virsh-console.c b/tools/virsh-console.c index 27f2d09f5f..db67e3f974 100644 --- a/tools/virsh-console.c +++ b/tools/virsh-console.c @@ -438,8 +438,11 @@ virshRunConsole(vshControl *ctl, if (!con->st) goto cleanup; - if (virDomainOpenConsole(dom, dev_name, con->st, flags) < 0) + if (virDomainOpenConsole(dom, dev_name, con->st, flags) < 0) { + vshError(ctl, _("Domain '%1$s' is utterly inconsolable :("), + virDomainGetName(dom)); goto cleanup; + } vshPrintExtra(ctl, _("Connected to domain '%1$s'\n"), virDomainGetName(dom)); vshPrintExtra(ctl, _("Escape character is %1$s"), priv->escapeChar); -- 2.44.0

On Mon, Apr 01, 2024 at 15:58:31 +0200, Andrea Bolognani wrote:
After attempting, and failing, to console a sad VM, we get back to the user with a fairly sterile message such as
error: internal error: character device serial0 is not using a PTY
That doesn't properly communicate the extent of our regret for having been unable to cheer up the poor VM despite our best efforts.
Try to put this sentiment into words; to further carry the message, also include a tasteful ASCII rendition of our sorrow.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tools/virsh-console.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-console.c b/tools/virsh-console.c index 27f2d09f5f..db67e3f974 100644 --- a/tools/virsh-console.c +++ b/tools/virsh-console.c @@ -438,8 +438,11 @@ virshRunConsole(vshControl *ctl, if (!con->st) goto cleanup;
- if (virDomainOpenConsole(dom, dev_name, con->st, flags) < 0) + if (virDomainOpenConsole(dom, dev_name, con->st, flags) < 0) { + vshError(ctl, _("Domain '%1$s' is utterly inconsolable :("),
Should we finally switch to proper emoji?
+ virDomainGetName(dom));
Laughed-at-by: Peter Krempa <pkrempa@redhat.com>
participants (2)
-
Andrea Bolognani
-
Peter Krempa