[RFC PATCH libvirt v1 0/3] Ensure full early console access with libvirt
Currently, early console output may be lost, e.g. if starting a guest with `virsh start --console` guest, which can make debugging of early failures very difficult (like zipl messages or disabled wait conditions happening early). This is because QEMU may emit serial console output before the libvirt console client starts to consume data from the pts. This can be prevented by starting the guest in paused state, connect to the console and then resume the guest. Note: There is still a problem in QEMU itself, see QEMU patch series `[PATCH] chardev/char-pty: Avoid losing bytes when the other side just (re-)connected` [1] [1] https://lists.gnu.org/archive/html/qemu-devel/2023-08/msg02725.html Marc Hartmayer (3): virsh: add `console --resume` support Improve `virsh start --console` behavior Improve `virsh create --console` behavior tools/virsh-console.c | 8 ++++++++ tools/virsh-console.h | 1 + tools/virsh-domain.c | 32 ++++++++++++++++++++++++++------ 3 files changed, 35 insertions(+), 6 deletions(-) base-commit: 3fd64fb0e236fc80ffa2cc977c0d471f11fc39bf -- 2.34.1
This patch adds the command line flag `--resume` to the `virsh console` command. This resumes a paused guest after connecting to the console. This might be handy since it's a "common" pattern to start a guest paused, connect to the console, and then resume it so as not to miss any console messages. Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> --- tools/virsh-console.c | 8 ++++++++ tools/virsh-console.h | 1 + tools/virsh-domain.c | 14 ++++++++++---- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/tools/virsh-console.c b/tools/virsh-console.c index 6bfb44a190ec..e44a070e7045 100644 --- a/tools/virsh-console.c +++ b/tools/virsh-console.c @@ -401,6 +401,7 @@ int virshRunConsole(vshControl *ctl, virDomainPtr dom, const char *dev_name, + const bool resume_domain, unsigned int flags) { virConsole *con = NULL; @@ -476,6 +477,13 @@ virshRunConsole(vshControl *ctl, goto cleanup; } + if (resume_domain) { + if (virDomainResume(dom) != 0) { + vshError(ctl, _("Failed to resume domain '%1$s'"), virDomainGetName(dom)); + goto cleanup; + } + } + while (!con->quit) { if (virCondWait(&con->cond, &con->parent.lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/tools/virsh-console.h b/tools/virsh-console.h index e89484d24bf4..2d00ed90cf4a 100644 --- a/tools/virsh-console.h +++ b/tools/virsh-console.h @@ -27,6 +27,7 @@ int virshRunConsole(vshControl *ctl, virDomainPtr dom, const char *dev_name, + const bool resume_domain, unsigned int flags); #endif /* !WIN32 */ diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 7abafe2ba30c..5c3c6d18aebf 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3012,6 +3012,10 @@ static const vshCmdOptDef opts_console[] = { .type = VSH_OT_BOOL, .help = N_("force console connection (disconnect already connected sessions)") }, + {.name = "resume", + .type = VSH_OT_BOOL, + .help = N_("resume a paused guest after connecting to console") + }, {.name = "safe", .type = VSH_OT_BOOL, .help = N_("only connect if safe console handling is supported") @@ -3022,6 +3026,7 @@ static const vshCmdOptDef opts_console[] = { static bool cmdRunConsole(vshControl *ctl, virDomainPtr dom, const char *name, + const bool resume_domain, unsigned int flags) { int state; @@ -3048,7 +3053,7 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, vshPrintExtra(ctl, " (Ctrl + %c)", priv->escapeChar[1]); vshPrintExtra(ctl, "\n"); fflush(stdout); - if (virshRunConsole(ctl, dom, name, flags) == 0) + if (virshRunConsole(ctl, dom, name, resume_domain, flags) == 0) return true; return false; @@ -3059,6 +3064,7 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom = NULL; bool force = vshCommandOptBool(cmd, "force"); + bool resume = vshCommandOptBool(cmd, "resume"); bool safe = vshCommandOptBool(cmd, "safe"); unsigned int flags = 0; const char *name = NULL; @@ -3074,7 +3080,7 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd) if (safe) flags |= VIR_DOMAIN_CONSOLE_SAFE; - return cmdRunConsole(ctl, dom, name, flags); + return cmdRunConsole(ctl, dom, name, resume, flags); } #endif /* WIN32 */ @@ -4136,7 +4142,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) vshPrintExtra(ctl, _("Domain '%1$s' started\n"), virDomainGetName(dom)); #ifndef WIN32 - if (console && !cmdRunConsole(ctl, dom, NULL, 0)) + if (console && !cmdRunConsole(ctl, dom, NULL, false, 0)) return false; #endif @@ -8232,7 +8238,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) virDomainGetName(dom), from); #ifndef WIN32 if (console) - cmdRunConsole(ctl, dom, NULL, 0); + cmdRunConsole(ctl, dom, NULL, false, 0); #endif return true; } -- 2.34.1
When starting a guest via libvirt (`virsh start --console`), early console output was missed because the guest was started first and then the console was attached. This patch changes this to the following sequence: 1. create a paused guest 2. attach the console 3. resume the guest Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> --- tools/virsh-domain.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 5c3c6d18aebf..3581161c6f53 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4065,6 +4065,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) g_autoptr(virshDomain) dom = NULL; #ifndef WIN32 bool console = vshCommandOptBool(cmd, "console"); + bool resume_domain = false; #endif unsigned int flags = VIR_DOMAIN_NONE; int rc; @@ -4083,8 +4084,14 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) if (virshFetchPassFdsList(ctl, cmd, &nfds, &fds) < 0) return false; - if (vshCommandOptBool(cmd, "paused")) + if (vshCommandOptBool(cmd, "paused")) { flags |= VIR_DOMAIN_START_PAUSED; +#ifndef WIN32 + } else if (console) { + flags |= VIR_DOMAIN_START_PAUSED; + resume_domain = true; +#endif + } if (vshCommandOptBool(cmd, "autodestroy")) flags |= VIR_DOMAIN_START_AUTODESTROY; if (vshCommandOptBool(cmd, "bypass-cache")) @@ -4142,7 +4149,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) vshPrintExtra(ctl, _("Domain '%1$s' started\n"), virDomainGetName(dom)); #ifndef WIN32 - if (console && !cmdRunConsole(ctl, dom, NULL, false, 0)) + if (console && !cmdRunConsole(ctl, dom, NULL, resume_domain, 0)) return false; #endif -- 2.34.1
On Mon, Sep 25, 2023 at 03:39:09PM +0200, Marc Hartmayer wrote:
When starting a guest via libvirt (`virsh start --console`), early console output was missed because the guest was started first and then the console was attached. This patch changes this to the following sequence:
1. create a paused guest 2. attach the console 3. resume the guest
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> --- tools/virsh-domain.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 5c3c6d18aebf..3581161c6f53 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4065,6 +4065,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) g_autoptr(virshDomain) dom = NULL; #ifndef WIN32 bool console = vshCommandOptBool(cmd, "console"); + bool resume_domain = false; #endif unsigned int flags = VIR_DOMAIN_NONE; int rc; @@ -4083,8 +4084,14 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) if (virshFetchPassFdsList(ctl, cmd, &nfds, &fds) < 0) return false;
- if (vshCommandOptBool(cmd, "paused")) + if (vshCommandOptBool(cmd, "paused")) { flags |= VIR_DOMAIN_START_PAUSED; +#ifndef WIN32 + } else if (console) { + flags |= VIR_DOMAIN_START_PAUSED; + resume_domain = true; +#endif
Hypervisor drivers are not required to support VIR_DOMAIN_START_PAUSED. So we need to detect the error code, and retry without that flag set as a fallback. Same in next patch.
+ } if (vshCommandOptBool(cmd, "autodestroy")) flags |= VIR_DOMAIN_START_AUTODESTROY; if (vshCommandOptBool(cmd, "bypass-cache")) @@ -4142,7 +4149,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) vshPrintExtra(ctl, _("Domain '%1$s' started\n"), virDomainGetName(dom)); #ifndef WIN32 - if (console && !cmdRunConsole(ctl, dom, NULL, false, 0)) + if (console && !cmdRunConsole(ctl, dom, NULL, resume_domain, 0)) return false; #endif
-- 2.34.1
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Sep 25, 2023 at 04:15 PM +0100, Daniel P. Berrangé <berrange@redhat.com> wrote:
On Mon, Sep 25, 2023 at 03:39:09PM +0200, Marc Hartmayer wrote:
When starting a guest via libvirt (`virsh start --console`), early console output was missed because the guest was started first and then the console was attached. This patch changes this to the following sequence:
1. create a paused guest 2. attach the console 3. resume the guest
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> --- tools/virsh-domain.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 5c3c6d18aebf..3581161c6f53 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4065,6 +4065,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) g_autoptr(virshDomain) dom = NULL; #ifndef WIN32 bool console = vshCommandOptBool(cmd, "console"); + bool resume_domain = false; #endif unsigned int flags = VIR_DOMAIN_NONE; int rc; @@ -4083,8 +4084,14 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) if (virshFetchPassFdsList(ctl, cmd, &nfds, &fds) < 0) return false;
- if (vshCommandOptBool(cmd, "paused")) + if (vshCommandOptBool(cmd, "paused")) { flags |= VIR_DOMAIN_START_PAUSED; +#ifndef WIN32 + } else if (console) { + flags |= VIR_DOMAIN_START_PAUSED; + resume_domain = true; +#endif
Hypervisor drivers are not required to support VIR_DOMAIN_START_PAUSED.
So we need to detect the error code, and retry without that flag set as a fallback. Same in next patch.
Yep, makes sense - will change. Thanks for the feedback. […snip] -- Kind regards / Beste Grüße Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Tue, Sep 26, 2023 at 02:11:37PM +0200, Marc Hartmayer wrote:
On Mon, Sep 25, 2023 at 04:15 PM +0100, Daniel P. Berrangé <berrange@redhat.com> wrote:
On Mon, Sep 25, 2023 at 03:39:09PM +0200, Marc Hartmayer wrote:
When starting a guest via libvirt (`virsh start --console`), early console output was missed because the guest was started first and then the console was attached. This patch changes this to the following sequence:
1. create a paused guest 2. attach the console 3. resume the guest
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> --- tools/virsh-domain.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 5c3c6d18aebf..3581161c6f53 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4065,6 +4065,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) g_autoptr(virshDomain) dom = NULL; #ifndef WIN32 bool console = vshCommandOptBool(cmd, "console"); + bool resume_domain = false; #endif unsigned int flags = VIR_DOMAIN_NONE; int rc; @@ -4083,8 +4084,14 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) if (virshFetchPassFdsList(ctl, cmd, &nfds, &fds) < 0) return false;
- if (vshCommandOptBool(cmd, "paused")) + if (vshCommandOptBool(cmd, "paused")) { flags |= VIR_DOMAIN_START_PAUSED; +#ifndef WIN32 + } else if (console) { + flags |= VIR_DOMAIN_START_PAUSED; + resume_domain = true; +#endif
Hypervisor drivers are not required to support VIR_DOMAIN_START_PAUSED.
So we need to detect the error code, and retry without that flag set as a fallback. Same in next patch.
Yep, makes sense - will change. Thanks for the feedback.
It'll be VIR_ERR_INVALID_ARG btw for an unsupported flag. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
When starting a guest via libvirt (`virsh create --console`), early console output was missed because the guest was started first and then the console was attached. This patch changes this to the following sequence: 1. create a paused transient guest 2. attach the console 3. resume the guest Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> --- tools/virsh-domain.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 3581161c6f53..5a97d44190c4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8207,6 +8207,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) g_autofree char *buffer = NULL; #ifndef WIN32 bool console = vshCommandOptBool(cmd, "console"); + bool resume_domain = false; #endif unsigned int flags = 0; size_t nfds = 0; @@ -8222,8 +8223,14 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) if (virshFetchPassFdsList(ctl, cmd, &nfds, &fds) < 0) return false; - if (vshCommandOptBool(cmd, "paused")) + if (vshCommandOptBool(cmd, "paused")) { flags |= VIR_DOMAIN_START_PAUSED; +#ifndef WIN32 + } else if (console) { + flags |= VIR_DOMAIN_START_PAUSED; + resume_domain = true; +#endif + } if (vshCommandOptBool(cmd, "autodestroy")) flags |= VIR_DOMAIN_START_AUTODESTROY; if (vshCommandOptBool(cmd, "validate")) @@ -8245,7 +8252,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) virDomainGetName(dom), from); #ifndef WIN32 if (console) - cmdRunConsole(ctl, dom, NULL, false, 0); + cmdRunConsole(ctl, dom, NULL, resume_domain, 0); #endif return true; } -- 2.34.1
participants (2)
-
Daniel P. Berrangé -
Marc Hartmayer