qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Michal Privoznik <mprivozn@redhat.com>
To: qemu-devel@nongnu.org
Cc: imammedo@redhat.com, ehabkost@redhat.com, eblake@redhat.com,
	pbonzini@redhat.com
Subject: [Qemu-devel] [PATCH] cli: Don't run early event loop if no --preconfig was specified
Date: Sat,  2 Jun 2018 12:34:52 +0200	[thread overview]
Message-ID: <ad910973c593c5ac2fed3a10ea958f7e9c12f82c.1527935663.git.mprivozn@redhat.com> (raw)

After 047f7038f586d215 it is possible for event loop to run two
times. First time whilst parsing command line options (the idea
is to bring up monitor early so that management applications can
tweak config before machine is initialized). And the second time
is after everything is set up (this is the usual place). In both
cases the event loop is called as main_loop_wait(nonblocking =
false) which causes the event loop to block until at least one
event occurred.

Now, consider that somebody (i.e. libvirt) calls us with
-daemonize. This operation is split in two steps. The main()
calls os_daemonize() which fork()-s and then waits in read()
until child notifies it via write():

/qemu.git $ ./x86_64-softmmu/qemu-system-x86_64 -S -daemonize \
  -no-user-config -nodefaults -nographic

  main():                child:
    os_daemonize():
      read(pipe[0])

                           main_loop():
                             main_loop_wait(false)

                           os_setup_post():
                             write(pipe[1])

                           main_loop():
                             main_loop_wait(false)

Here it can be clearly seen that main() does not exit until an
event occurs, but at the same time nobody will touch the monitor
socket until their exec("qemu-system-*") finishes. So the whole
thing deadlocks.

The solution is to not call main_loop() unless --preconfig was
specified (in which case caller knows they must connect to the
socket before exec() finishes).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 vl.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 70f090c823..cde2934c40 100644
--- a/vl.c
+++ b/vl.c
@@ -4469,8 +4469,13 @@ int main(int argc, char **argv, char **envp)
     }
     parse_numa_opts(current_machine);
 
-    /* do monitor/qmp handling at preconfig state if requested */
-    main_loop();
+    if (preconfig_exit_requested) {
+        runstate_set(RUN_STATE_PRELAUNCH);
+        preconfig_exit_requested = false;
+    } else {
+        /* do monitor/qmp handling at preconfig state if requested */
+        main_loop();
+    }
 
     /* from here on runstate is RUN_STATE_PRELAUNCH */
     machine_run_board_init(current_machine);
-- 
2.16.4

             reply	other threads:[~2018-06-02 10:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-02 10:34 Michal Privoznik [this message]
2018-06-04 10:32 ` [Qemu-devel] [PATCH] cli: Don't run early event loop if no --preconfig was specified Daniel P. Berrangé
2018-06-05  0:41   ` Eduardo Habkost
2018-06-05 11:43     ` Igor Mammedov
2018-06-05 12:06       ` Igor Mammedov
2018-06-04 11:44 ` Igor Mammedov

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=ad910973c593c5ac2fed3a10ea958f7e9c12f82c.1527935663.git.mprivozn@redhat.com \
    --to=mprivozn@redhat.com \
    --cc=eblake@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=pbonzini@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).