All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] vl: pause option
@ 2020-11-02 15:50 Steve Sistare
  2020-11-04 19:26 ` Eric Blake
  0 siblings, 1 reply; 8+ messages in thread
From: Steve Sistare @ 2020-11-02 15:50 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Daniel P. Berrange, Dr. David Alan Gilbert,
	Steve Sistare

Provide the -pause command-line parameter and the QEMU_PAUSE environment
variable to pause QEMU during process startup using SIGSTOP and allow a
developer to attach a debugger, or observe the process using tools such as
strace.  Useful when the QEMU has been launched with some other entity, such
as libvirt.  QEMU_PAUSE is checked in a constructor at the highest priority,
and can be used to debug other constructors.  The -pause option is checked
later, during argument processing in main, but is useful if passing an
environment variable from a launcher to qemu is awkard.

Usage: qemu -pause, or QEMU_PAUSE=1
After attaching a debugger, send SIGCONT to the qemu process to continue.

Example:

$ QEMU_PAUSE=1 qemu-system-x86_64 ...
QEMU pid 18371 is stopped.
[1]+  Stopped
                                 $ gdb -p 18371
                                 (gdb)
$ kill -cont 18371
                                 (gdb) break rcu_init
                                 (gdb) continue
                                 Program received signal SIGCONT, Continued.
                                 (gdb) continue
                                 Breakpoint 1, rcu_init () at util/rcu.c:380

Thanks to Daniel P. Berrange <berrange@redhat.com> for suggesting SIGSTOP.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 qemu-options.hx | 14 ++++++++++++++
 softmmu/vl.c    | 23 +++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/qemu-options.hx b/qemu-options.hx
index 708583b..42edd70 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3668,6 +3668,20 @@ SRST
     option is experimental.
 ERST
 
+DEF("pause", 0, QEMU_OPTION_pause, \
+    "-pause          pause the qemu process in main using SIGSTOP.\n"
+    "                to pause earlier, before constructors are run, set the\n"
+    "                environment variable QEMU_PAUSE=1 before starting qemu.\n",
+    QEMU_ARCH_ALL)
+
+SRST
+``-pause``
+    Pause the qemu process in main using SIGSTOP.  This is useful for attaching
+    a debugger after QEMU has been launched by some other entity.  After
+    attaching, send SIGCONT to continue.  To pause earlier, before constructors
+    are run, set the environment variable QEMU_PAUSE=1 before starting qemu.
+ERST
+
 DEF("S", 0, QEMU_OPTION_S, \
     "-S              freeze CPU at startup (use 'c' to start execution)\n",
     QEMU_ARCH_ALL)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 4eb9d1f..aee1a96 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2829,6 +2829,24 @@ static void create_default_memdev(MachineState *ms, const char *path)
                             &error_fatal);
 }
 
+static __attribute__((constructor(101))) void maybe_pause(void)
+{
+    const char *pause = getenv("QEMU_PAUSE");
+
+    if (pause) {
+        if (!pause[0] || !strcmp(pause, "1")) {
+            printf("QEMU pid %d is stopped.  Send SIGCONT to continue.\n",
+                   getpid());
+            kill(getpid(), SIGSTOP);
+        } else if (strcmp(pause, "0")) {
+            fprintf(stderr, "error: QEMU_PAUSE bad value %s. Must be 1 or "
+                            "empty to enable, 0 or unset to disable.\n",
+                            pause);
+            exit(1);
+        }
+    }
+}
+
 void qemu_init(int argc, char **argv, char **envp)
 {
     int i;
@@ -3191,6 +3209,11 @@ void qemu_init(int argc, char **argv, char **envp)
             case QEMU_OPTION_gdb:
                 add_device_config(DEV_GDB, optarg);
                 break;
+            case QEMU_OPTION_pause:
+                printf("QEMU pid %d is stopped.  Send SIGCONT to continue.\n",
+                       getpid());
+                kill(getpid(), SIGSTOP);
+                break;
             case QEMU_OPTION_L:
                 if (is_help_option(optarg)) {
                     list_data_dirs = true;
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] vl: pause option
  2020-11-02 15:50 [PATCH V2] vl: pause option Steve Sistare
@ 2020-11-04 19:26 ` Eric Blake
  2020-11-04 20:39   ` Steven Sistare
  2020-11-04 21:40   ` Alex Bennée
  0 siblings, 2 replies; 8+ messages in thread
From: Eric Blake @ 2020-11-04 19:26 UTC (permalink / raw)
  To: Steve Sistare, qemu-devel
  Cc: Alex Bennée, Daniel P. Berrange, Dr. David Alan Gilbert

On 11/2/20 9:50 AM, Steve Sistare wrote:
> Provide the -pause command-line parameter and the QEMU_PAUSE environment
> variable to pause QEMU during process startup using SIGSTOP and allow a
> developer to attach a debugger, or observe the process using tools such as
> strace.  Useful when the QEMU has been launched with some other entity, such
> as libvirt.  QEMU_PAUSE is checked in a constructor at the highest priority,
> and can be used to debug other constructors.  The -pause option is checked
> later, during argument processing in main, but is useful if passing an
> environment variable from a launcher to qemu is awkard.
> 
> Usage: qemu -pause, or QEMU_PAUSE=1
> After attaching a debugger, send SIGCONT to the qemu process to continue.

Changing behavior via a new environment variable is awkward.  What
happens, for example, if libvirt inherits this variable set, but is not
aware of its impact to alter how qemu starts up?  Can we get by with
ONLY a command line option?

Also, how does this option differ from what we already have with qemu
--preconfig?

> 
> Example:
> 
> $ QEMU_PAUSE=1 qemu-system-x86_64 ...
> QEMU pid 18371 is stopped.
> [1]+  Stopped
>                                  $ gdb -p 18371
>                                  (gdb)
> $ kill -cont 18371
>                                  (gdb) break rcu_init
>                                  (gdb) continue
>                                  Program received signal SIGCONT, Continued.
>                                  (gdb) continue
>                                  Breakpoint 1, rcu_init () at util/rcu.c:380
> 
> Thanks to Daniel P. Berrange <berrange@redhat.com> for suggesting SIGSTOP.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  qemu-options.hx | 14 ++++++++++++++
>  softmmu/vl.c    | 23 +++++++++++++++++++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 708583b..42edd70 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3668,6 +3668,20 @@ SRST
>      option is experimental.
>  ERST
>  
> +DEF("pause", 0, QEMU_OPTION_pause, \
> +    "-pause          pause the qemu process in main using SIGSTOP.\n"
> +    "                to pause earlier, before constructors are run, set the\n"
> +    "                environment variable QEMU_PAUSE=1 before starting qemu.\n",
> +    QEMU_ARCH_ALL)
> +
> +SRST
> +``-pause``
> +    Pause the qemu process in main using SIGSTOP.  This is useful for attaching
> +    a debugger after QEMU has been launched by some other entity.  After
> +    attaching, send SIGCONT to continue.  To pause earlier, before constructors
> +    are run, set the environment variable QEMU_PAUSE=1 before starting qemu.
> +ERST

Isn't it always possible to provide a wrapper qemu process to be invoked
by whatever third-party management app (like libvirt), where your
wrapper then starts the real qemu under gdb to begin with, rather than
having to hack qemu itself to have a special start-up mode?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] vl: pause option
  2020-11-04 19:26 ` Eric Blake
@ 2020-11-04 20:39   ` Steven Sistare
  2020-11-04 21:40   ` Alex Bennée
  1 sibling, 0 replies; 8+ messages in thread
From: Steven Sistare @ 2020-11-04 20:39 UTC (permalink / raw)
  To: Eric Blake, qemu-devel
  Cc: Alex Bennée, Daniel P. Berrange, Dr. David Alan Gilbert

On 11/4/2020 2:26 PM, Eric Blake wrote:
> On 11/2/20 9:50 AM, Steve Sistare wrote:
>> Provide the -pause command-line parameter and the QEMU_PAUSE environment
>> variable to pause QEMU during process startup using SIGSTOP and allow a
>> developer to attach a debugger, or observe the process using tools such as
>> strace.  Useful when the QEMU has been launched with some other entity, such
>> as libvirt.  QEMU_PAUSE is checked in a constructor at the highest priority,
>> and can be used to debug other constructors.  The -pause option is checked
>> later, during argument processing in main, but is useful if passing an
>> environment variable from a launcher to qemu is awkard.
>>
>> Usage: qemu -pause, or QEMU_PAUSE=1
>> After attaching a debugger, send SIGCONT to the qemu process to continue.
> 
> Changing behavior via a new environment variable is awkward.  What
> happens, for example, if libvirt inherits this variable set, but is not
> aware of its impact to alter how qemu starts up? 

The env var is intended to only be set by a developer.  The developer is responsible
for sending SIGCONT, not libvirt.  libvirt does not need to understand the semantics
of QEMU_PAUSE.

For libvirt, the developer would add this to the domain definition file, so it only
applies to that domain:

<qemu:commandline>
   <qemu:env name='QEMU_PAUSE' value='1'/>
</qemu:commandline>

>  Can we get by with ONLY a command line option?

The command line option is sufficient most of the time and I would be happy to get
at least that.  The env var is even better, and if I do not push it, someone else
will!

> Also, how does this option differ from what we already have with qemu
> --preconfig?

pause stops the qemu process earlier, so more can be debugged before it occurs.
With preconfig, qemu is still running its event loop and may respond to external
events such as monitor requests, which may be undesirable.

- Steve

>> Example:
>>
>> $ QEMU_PAUSE=1 qemu-system-x86_64 ...
>> QEMU pid 18371 is stopped.
>> [1]+  Stopped
>>                                  $ gdb -p 18371
>>                                  (gdb)
>> $ kill -cont 18371
>>                                  (gdb) break rcu_init
>>                                  (gdb) continue
>>                                  Program received signal SIGCONT, Continued.
>>                                  (gdb) continue
>>                                  Breakpoint 1, rcu_init () at util/rcu.c:380
>>
>> Thanks to Daniel P. Berrange <berrange@redhat.com> for suggesting SIGSTOP.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>  qemu-options.hx | 14 ++++++++++++++
>>  softmmu/vl.c    | 23 +++++++++++++++++++++++
>>  2 files changed, 37 insertions(+)
>>
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 708583b..42edd70 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -3668,6 +3668,20 @@ SRST
>>      option is experimental.
>>  ERST
>>  
>> +DEF("pause", 0, QEMU_OPTION_pause, \
>> +    "-pause          pause the qemu process in main using SIGSTOP.\n"
>> +    "                to pause earlier, before constructors are run, set the\n"
>> +    "                environment variable QEMU_PAUSE=1 before starting qemu.\n",
>> +    QEMU_ARCH_ALL)
>> +
>> +SRST
>> +``-pause``
>> +    Pause the qemu process in main using SIGSTOP.  This is useful for attaching
>> +    a debugger after QEMU has been launched by some other entity.  After
>> +    attaching, send SIGCONT to continue.  To pause earlier, before constructors
>> +    are run, set the environment variable QEMU_PAUSE=1 before starting qemu.
>> +ERST
> 
> Isn't it always possible to provide a wrapper qemu process to be invoked
> by whatever third-party management app (like libvirt), where your
> wrapper then starts the real qemu under gdb to begin with, rather than
> having to hack qemu itself to have a special start-up mode?
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] vl: pause option
  2020-11-04 19:26 ` Eric Blake
  2020-11-04 20:39   ` Steven Sistare
@ 2020-11-04 21:40   ` Alex Bennée
  2020-11-05 14:55     ` Steven Sistare
  1 sibling, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2020-11-04 21:40 UTC (permalink / raw)
  To: Eric Blake
  Cc: Daniel P. Berrange, Steve Sistare, qemu-devel, Dr. David Alan Gilbert


Eric Blake <eblake@redhat.com> writes:

> On 11/2/20 9:50 AM, Steve Sistare wrote:
>> Provide the -pause command-line parameter and the QEMU_PAUSE environment
>> variable to pause QEMU during process startup using SIGSTOP and allow a
>> developer to attach a debugger, or observe the process using tools such as
>> strace.  Useful when the QEMU has been launched with some other entity, such
>> as libvirt.  QEMU_PAUSE is checked in a constructor at the highest priority,
>> and can be used to debug other constructors.  The -pause option is checked
>> later, during argument processing in main, but is useful if passing an
>> environment variable from a launcher to qemu is awkard.
>> 
>> Usage: qemu -pause, or QEMU_PAUSE=1
>> After attaching a debugger, send SIGCONT to the qemu process to continue.
>
> Changing behavior via a new environment variable is awkward.  What
> happens, for example, if libvirt inherits this variable set, but is not
> aware of its impact to alter how qemu starts up?  Can we get by with
> ONLY a command line option?
>
> Also, how does this option differ from what we already have with qemu
> --preconfig?

In the original discussion:

  Subject: [PATCH V1 12/32] vl: pause option
  Date: Thu, 30 Jul 2020 08:14:16 -0700
  Message-Id: <1596122076-341293-13-git-send-email-steven.sistare@oracle.com>

it seems the idea was to stop qemu as early as possible for debugging
when launched by some other launcher which wasn't modifiable except by
pass through configuration to QEMU's command line.

<snip>

> Isn't it always possible to provide a wrapper qemu process to be invoked
> by whatever third-party management app (like libvirt), where your
> wrapper then starts the real qemu under gdb to begin with, rather than
> having to hack qemu itself to have a special start-up mode?

I agree - this feels like a bit of an over complication as a debug
helper. How many times can a failure to launch by some binary blob not
be debugged by either a gdb follow-fork or a copying of the command line
and running gdb --args?

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] vl: pause option
  2020-11-04 21:40   ` Alex Bennée
@ 2020-11-05 14:55     ` Steven Sistare
  2020-11-05 15:07       ` Daniel P. Berrangé
  2021-01-06 20:32       ` Steven Sistare
  0 siblings, 2 replies; 8+ messages in thread
From: Steven Sistare @ 2020-11-05 14:55 UTC (permalink / raw)
  To: Alex Bennée, Eric Blake
  Cc: Daniel P. Berrange, qemu-devel, Dr. David Alan Gilbert

On 11/4/2020 4:40 PM, Alex Bennée wrote:
> Eric Blake <eblake@redhat.com> writes:
>> On 11/2/20 9:50 AM, Steve Sistare wrote:
>>> Provide the -pause command-line parameter and the QEMU_PAUSE environment
>>> variable to pause QEMU during process startup using SIGSTOP and allow a
>>> developer to attach a debugger, or observe the process using tools such as
>>> strace.  Useful when the QEMU has been launched with some other entity, such
>>> as libvirt.  QEMU_PAUSE is checked in a constructor at the highest priority,
>>> and can be used to debug other constructors.  The -pause option is checked
>>> later, during argument processing in main, but is useful if passing an
>>> environment variable from a launcher to qemu is awkard.
>>>
>>> Usage: qemu -pause, or QEMU_PAUSE=1
>>> After attaching a debugger, send SIGCONT to the qemu process to continue.
>>
>> Changing behavior via a new environment variable is awkward.  What
>> happens, for example, if libvirt inherits this variable set, but is not
>> aware of its impact to alter how qemu starts up?  Can we get by with
>> ONLY a command line option?
>>
>> Also, how does this option differ from what we already have with qemu
>> --preconfig?
> 
> In the original discussion:
> 
>   Subject: [PATCH V1 12/32] vl: pause option
>   Date: Thu, 30 Jul 2020 08:14:16 -0700
>   Message-Id: <1596122076-341293-13-git-send-email-steven.sistare@oracle.com>
> 
> it seems the idea was to stop qemu as early as possible for debugging
> when launched by some other launcher which wasn't modifiable except by
> pass through configuration to QEMU's command line.
> 
> <snip>
> 
>> Isn't it always possible to provide a wrapper qemu process to be invoked
>> by whatever third-party management app (like libvirt), where your
>> wrapper then starts the real qemu under gdb to begin with, rather than
>> having to hack qemu itself to have a special start-up mode?
> 
> I agree - this feels like a bit of an over complication as a debug
> helper. How many times can a failure to launch by some binary blob not
> be debugged by either a gdb follow-fork or a copying of the command line
> and running gdb --args?

Follow fork is awkward and error prone when the launcher performs many forks before forking
qemu. gdb --args does not work when the launcher sets up an environment for qemu to run
in, pre-opening fd's being just one example.  For developers, often the "launcher" is a 
script that performs setup and passes the myriad qemu options.  Even in that case, it is
easier to add a flag or set an env var to enable debugging. The pause option is fast 
(for the user) and reliable.  

I have a new version of the patch that handles the signal more smoothly, so the handshake
with gdb is easier:

    $ QEMU_PAUSE=1 qemu-system-x86_64 ...
    QEMU pid 18371 is stopped.

                                     $ gdb -p 18371
                                     (gdb) break rcu_init
                                     (gdb) signal SIGCONT
                                     Breakpoint 1, rcu_init () at util/rcu.c:380

The implementation does not even send a signal to qemu, so the launcher is none the wiser:

static void pause_me(void)
{
    int sig;
    sigset_t set, oldset;
    sigemptyset(&set);
    sigaddset(&set, SIGCONT);
    printf("QEMU pid %d is stopped.  Send SIGCONT to continue.\n", getpid());
    sigprocmask(SIG_BLOCK, &set, &oldset);
    sigwait(&set, &sig);                          <-- PAUSES HERE
    sigprocmask(SIG_SETMASK, &oldset, 0);
}


I will post it if you are still open to the idea.  Please let me know.

- Steve


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] vl: pause option
  2020-11-05 14:55     ` Steven Sistare
@ 2020-11-05 15:07       ` Daniel P. Berrangé
  2020-11-05 15:19         ` Steven Sistare
  2021-01-06 20:32       ` Steven Sistare
  1 sibling, 1 reply; 8+ messages in thread
From: Daniel P. Berrangé @ 2020-11-05 15:07 UTC (permalink / raw)
  To: Steven Sistare; +Cc: Alex Bennée, qemu-devel, Dr. David Alan Gilbert

On Thu, Nov 05, 2020 at 09:55:21AM -0500, Steven Sistare wrote:
> On 11/4/2020 4:40 PM, Alex Bennée wrote:
> > Eric Blake <eblake@redhat.com> writes:
> >> On 11/2/20 9:50 AM, Steve Sistare wrote:
> >>> Provide the -pause command-line parameter and the QEMU_PAUSE environment
> >>> variable to pause QEMU during process startup using SIGSTOP and allow a
> >>> developer to attach a debugger, or observe the process using tools such as
> >>> strace.  Useful when the QEMU has been launched with some other entity, such
> >>> as libvirt.  QEMU_PAUSE is checked in a constructor at the highest priority,
> >>> and can be used to debug other constructors.  The -pause option is checked
> >>> later, during argument processing in main, but is useful if passing an
> >>> environment variable from a launcher to qemu is awkard.
> >>>
> >>> Usage: qemu -pause, or QEMU_PAUSE=1
> >>> After attaching a debugger, send SIGCONT to the qemu process to continue.
> >>
> >> Changing behavior via a new environment variable is awkward.  What
> >> happens, for example, if libvirt inherits this variable set, but is not
> >> aware of its impact to alter how qemu starts up?  Can we get by with
> >> ONLY a command line option?
> >>
> >> Also, how does this option differ from what we already have with qemu
> >> --preconfig?
> > 
> > In the original discussion:
> > 
> >   Subject: [PATCH V1 12/32] vl: pause option
> >   Date: Thu, 30 Jul 2020 08:14:16 -0700
> >   Message-Id: <1596122076-341293-13-git-send-email-steven.sistare@oracle.com>
> > 
> > it seems the idea was to stop qemu as early as possible for debugging
> > when launched by some other launcher which wasn't modifiable except by
> > pass through configuration to QEMU's command line.
> > 
> > <snip>
> > 
> >> Isn't it always possible to provide a wrapper qemu process to be invoked
> >> by whatever third-party management app (like libvirt), where your
> >> wrapper then starts the real qemu under gdb to begin with, rather than
> >> having to hack qemu itself to have a special start-up mode?
> > 
> > I agree - this feels like a bit of an over complication as a debug
> > helper. How many times can a failure to launch by some binary blob not
> > be debugged by either a gdb follow-fork or a copying of the command line
> > and running gdb --args?
> 
> Follow fork is awkward and error prone when the launcher performs many forks before forking
> qemu. gdb --args does not work when the launcher sets up an environment for qemu to run
> in, pre-opening fd's being just one example.  For developers, often the "launcher" is a 
> script that performs setup and passes the myriad qemu options.  Even in that case, it is
> easier to add a flag or set an env var to enable debugging. The pause option is fast 
> (for the user) and reliable.

What is your launcher ?  Are you using libvirt, or something else ?

If the goal is simply to make it easy to attach GDB right at the start of
QEMU execution, then could this feature be supported by the launcher itself
in an easier way.

eg, immediately before the execve(qemu, ....) syscall, the launcher can
simply send SIGSTOP to itself.  You can now fire up GDB, attach to it,
and be able to watch execution from the very momement of execve(),
which is even earlier than this patch allows for, and still avoids the
need to follow across forks. 


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 :|



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] vl: pause option
  2020-11-05 15:07       ` Daniel P. Berrangé
@ 2020-11-05 15:19         ` Steven Sistare
  0 siblings, 0 replies; 8+ messages in thread
From: Steven Sistare @ 2020-11-05 15:19 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Alex Bennée, qemu-devel, Dr. David Alan Gilbert

On 11/5/2020 10:07 AM, Daniel P. Berrangé wrote:
> On Thu, Nov 05, 2020 at 09:55:21AM -0500, Steven Sistare wrote:
>> On 11/4/2020 4:40 PM, Alex Bennée wrote:
>>> Eric Blake <eblake@redhat.com> writes:
>>>> On 11/2/20 9:50 AM, Steve Sistare wrote:
>>>>> Provide the -pause command-line parameter and the QEMU_PAUSE environment
>>>>> variable to pause QEMU during process startup using SIGSTOP and allow a
>>>>> developer to attach a debugger, or observe the process using tools such as
>>>>> strace.  Useful when the QEMU has been launched with some other entity, such
>>>>> as libvirt.  QEMU_PAUSE is checked in a constructor at the highest priority,
>>>>> and can be used to debug other constructors.  The -pause option is checked
>>>>> later, during argument processing in main, but is useful if passing an
>>>>> environment variable from a launcher to qemu is awkard.
>>>>>
>>>>> Usage: qemu -pause, or QEMU_PAUSE=1
>>>>> After attaching a debugger, send SIGCONT to the qemu process to continue.
>>>>
>>>> Changing behavior via a new environment variable is awkward.  What
>>>> happens, for example, if libvirt inherits this variable set, but is not
>>>> aware of its impact to alter how qemu starts up?  Can we get by with
>>>> ONLY a command line option?
>>>>
>>>> Also, how does this option differ from what we already have with qemu
>>>> --preconfig?
>>>
>>> In the original discussion:
>>>
>>>   Subject: [PATCH V1 12/32] vl: pause option
>>>   Date: Thu, 30 Jul 2020 08:14:16 -0700
>>>   Message-Id: <1596122076-341293-13-git-send-email-steven.sistare@oracle.com>
>>>
>>> it seems the idea was to stop qemu as early as possible for debugging
>>> when launched by some other launcher which wasn't modifiable except by
>>> pass through configuration to QEMU's command line.
>>>
>>> <snip>
>>>
>>>> Isn't it always possible to provide a wrapper qemu process to be invoked
>>>> by whatever third-party management app (like libvirt), where your
>>>> wrapper then starts the real qemu under gdb to begin with, rather than
>>>> having to hack qemu itself to have a special start-up mode?
>>>
>>> I agree - this feels like a bit of an over complication as a debug
>>> helper. How many times can a failure to launch by some binary blob not
>>> be debugged by either a gdb follow-fork or a copying of the command line
>>> and running gdb --args?
>>
>> Follow fork is awkward and error prone when the launcher performs many forks before forking
>> qemu. gdb --args does not work when the launcher sets up an environment for qemu to run
>> in, pre-opening fd's being just one example.  For developers, often the "launcher" is a 
>> script that performs setup and passes the myriad qemu options.  Even in that case, it is
>> easier to add a flag or set an env var to enable debugging. The pause option is fast 
>> (for the user) and reliable.
> 
> What is your launcher ?  Are you using libvirt, or something else ?
> 
> If the goal is simply to make it easy to attach GDB right at the start of
> QEMU execution, then could this feature be supported by the launcher itself
> in an easier way.
> 
> eg, immediately before the execve(qemu, ....) syscall, the launcher can
> simply send SIGSTOP to itself.  You can now fire up GDB, attach to it,
> and be able to watch execution from the very momement of execve(),
> which is even earlier than this patch allows for, and still avoids the
> need to follow across forks. 

My launcher is part of our container environment.  But I also use libvirt, and scripts.
Putting the hook inside qemu makes it work for all environments.

The V2 patch uses your SIGSTOP idea (which you previously suggested to me, thanks), but I found that
by fiddling with signal masks, I could make the handshake easier for the user.  See the example in the 
V2 patch message, versus the example I just sent in my last email.

- Steve


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] vl: pause option
  2020-11-05 14:55     ` Steven Sistare
  2020-11-05 15:07       ` Daniel P. Berrangé
@ 2021-01-06 20:32       ` Steven Sistare
  1 sibling, 0 replies; 8+ messages in thread
From: Steven Sistare @ 2021-01-06 20:32 UTC (permalink / raw)
  To: Alex Bennée, Eric Blake
  Cc: Daniel P. Berrange, qemu-devel, Dr. David Alan Gilbert

On 11/5/2020 9:55 AM, Steven Sistare wrote:
> On 11/4/2020 4:40 PM, Alex Bennée wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>> On 11/2/20 9:50 AM, Steve Sistare wrote:
>>>> Provide the -pause command-line parameter and the QEMU_PAUSE environment
>>>> variable to pause QEMU during process startup using SIGSTOP and allow a
>>>> developer to attach a debugger, or observe the process using tools such as
>>>> strace.  Useful when the QEMU has been launched with some other entity, such
>>>> as libvirt.  QEMU_PAUSE is checked in a constructor at the highest priority,
>>>> and can be used to debug other constructors.  The -pause option is checked
>>>> later, during argument processing in main, but is useful if passing an
>>>> environment variable from a launcher to qemu is awkard.
>>>>
>>>> Usage: qemu -pause, or QEMU_PAUSE=1
>>>> After attaching a debugger, send SIGCONT to the qemu process to continue.
>>>
>>> Changing behavior via a new environment variable is awkward.  What
>>> happens, for example, if libvirt inherits this variable set, but is not
>>> aware of its impact to alter how qemu starts up?  Can we get by with
>>> ONLY a command line option?
>>>
>>> Also, how does this option differ from what we already have with qemu
>>> --preconfig?
>>
>> In the original discussion:
>>
>>   Subject: [PATCH V1 12/32] vl: pause option
>>   Date: Thu, 30 Jul 2020 08:14:16 -0700
>>   Message-Id: <1596122076-341293-13-git-send-email-steven.sistare@oracle.com>
>>
>> it seems the idea was to stop qemu as early as possible for debugging
>> when launched by some other launcher which wasn't modifiable except by
>> pass through configuration to QEMU's command line.
>>
>> <snip>
>>
>>> Isn't it always possible to provide a wrapper qemu process to be invoked
>>> by whatever third-party management app (like libvirt), where your
>>> wrapper then starts the real qemu under gdb to begin with, rather than
>>> having to hack qemu itself to have a special start-up mode?
>>
>> I agree - this feels like a bit of an over complication as a debug
>> helper. How many times can a failure to launch by some binary blob not
>> be debugged by either a gdb follow-fork or a copying of the command line
>> and running gdb --args?
> 
> Follow fork is awkward and error prone when the launcher performs many forks before forking
> qemu. gdb --args does not work when the launcher sets up an environment for qemu to run
> in, pre-opening fd's being just one example.  For developers, often the "launcher" is a 
> script that performs setup and passes the myriad qemu options.  Even in that case, it is
> easier to add a flag or set an env var to enable debugging. The pause option is fast 
> (for the user) and reliable.  
> 
> I have a new version of the patch that handles the signal more smoothly, so the handshake
> with gdb is easier:
> 
>     $ QEMU_PAUSE=1 qemu-system-x86_64 ...
>     QEMU pid 18371 is stopped.
> 
>                                      $ gdb -p 18371
>                                      (gdb) break rcu_init
>                                      (gdb) signal SIGCONT
>                                      Breakpoint 1, rcu_init () at util/rcu.c:380
> 
> The implementation does not even send a signal to qemu, so the launcher is none the wiser:
> 
> static void pause_me(void)
> {
>     int sig;
>     sigset_t set, oldset;
>     sigemptyset(&set);
>     sigaddset(&set, SIGCONT);
>     printf("QEMU pid %d is stopped.  Send SIGCONT to continue.\n", getpid());
>     sigprocmask(SIG_BLOCK, &set, &oldset);
>     sigwait(&set, &sig);                          <-- PAUSES HERE
>     sigprocmask(SIG_SETMASK, &oldset, 0);
> }
> 
> 
> I will post it if you are still open to the idea.  Please let me know.

I am posting a V3 version of the patch with the above change.  I have found it easy to
use and robust. See your inboxes shortly.

- Steve


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-01-06 20:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-02 15:50 [PATCH V2] vl: pause option Steve Sistare
2020-11-04 19:26 ` Eric Blake
2020-11-04 20:39   ` Steven Sistare
2020-11-04 21:40   ` Alex Bennée
2020-11-05 14:55     ` Steven Sistare
2020-11-05 15:07       ` Daniel P. Berrangé
2020-11-05 15:19         ` Steven Sistare
2021-01-06 20:32       ` Steven Sistare

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.