qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Steven Sistare <steven.sistare@oracle.com>
To: "Alex Bennée" <alex.bennee@linaro.org>, "Eric Blake" <eblake@redhat.com>
Cc: "Daniel P. Berrange" <berrange@redhat.com>,
	qemu-devel@nongnu.org,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH V2] vl: pause option
Date: Thu, 5 Nov 2020 09:55:21 -0500	[thread overview]
Message-ID: <db5462f0-f96f-2498-3aa3-ec322462ba22@oracle.com> (raw)
In-Reply-To: <87imakby2n.fsf@linaro.org>

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


  reply	other threads:[~2020-11-05 14:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2020-11-05 15:07       ` Daniel P. Berrangé
2020-11-05 15:19         ` Steven Sistare
2021-01-06 20:32       ` Steven Sistare

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=db5462f0-f96f-2498-3aa3-ec322462ba22@oracle.com \
    --to=steven.sistare@oracle.com \
    --cc=alex.bennee@linaro.org \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).