Re: [PATCH V2] vl: pause option

[Steven Sistare <steven.sistare@oracle.com>]

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