1. Patchew
  2. Libvirt
  3. [RFC PATCH libvirt v1 0/3] Ensure full early console access with libvirt
  4. View patch

[RFC PATCH libvirt v1 2/3] Improve `virsh start --console` behavior

Marc Hartmayer posted 3 patches 2 years, 4 months ago
There is a newer version of this series
[RFC PATCH libvirt v1 2/3] Improve `virsh start --console` behavior
Posted by Marc Hartmayer 2 years, 4 months ago 
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
Re: [RFC PATCH libvirt v1 2/3] Improve `virsh start --console` behavior
Posted by Daniel P. Berrangé 2 years, 4 months ago 
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 :|
Re: [RFC PATCH libvirt v1 2/3] Improve `virsh start --console` behavior
Posted by Marc Hartmayer 2 years, 4 months ago 
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
Re: [RFC PATCH libvirt v1 2/3] Improve `virsh start --console` behavior
Posted by Daniel P. Berrangé 2 years, 4 months ago 
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 :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.