qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: qemu-devel@nongnu.org
Cc: berrange@redhat.com, mreitz@redhat.com, pbonzini@redhat.com,
	ehabkost@redhat.com, ldoktor@redhat.com, eblake@redhat.com
Subject: [Qemu-devel] [PATCH v6 2/2] vl: fix use of --daemonize with --preconfig
Date: Thu,  7 Jun 2018 14:00:09 +0200	[thread overview]
Message-ID: <1528372809-175770-1-git-send-email-imammedo@redhat.com> (raw)
In-Reply-To: <20180606135051.GR7451@localhost.localdomain>

When using --daemonize, the initial lead process will fork a child and
then wait to be notified that setup is complete via a pipe, before it
exits.  When using --preconfig there is an extra call to main_loop()
before the notification is done from os_setup_post(). Thus the parent
process won't exit until the mgmt application connects to the monitor
and tells QEMU to leave the RUN_STATE_PRECONFIG. The mgmt application
won't connect to the monitor until daemonizing has completed though.

This is a chicken and egg problem, leading to deadlock at startup.

The only viable way to fix this is to call os_setup_post() before
the early main_loop() call when --preconfig is used. This has the
downside that any errors from this point onwards won't be handled
well by the mgmt application, because it will think QEMU has started
successfully, so not be expecting an abrupt exit. Moving as much user
input validation as possible to before the main_loop() call might help,
but mgmt application should stop assuming that QEMU has started
successfuly and use other means to collect errors from QEMU (logfile).

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v5:
  * use original Daniel's patch [1], but addapt it to apply on top of
    "[PATCH v3 1/2] cli: Don't run early event loop if no  --preconfig was specified"
    with extra comment and massage commit message a little bit.
v6:
  * hide os_setup_post_done flag inside of os_setup_post() as it was in v4

CC: berrange@redhat.com
CC: mreitz@redhat.com
CC: pbonzini@redhat.com
CC: ehabkost@redhat.com
CC: ldoktor@redhat.com
CC: eblake@redhat.com
---
 os-posix.c | 6 ++++++
 vl.c       | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/os-posix.c b/os-posix.c
index 9ce6f74..0246195 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -309,8 +309,14 @@ void os_daemonize(void)
 
 void os_setup_post(void)
 {
+    static bool os_setup_post_done;
     int fd = 0;
 
+    if (os_setup_post_done) {
+        return;
+    }
+    os_setup_post_done = true;
+
     if (daemonize) {
         if (chdir("/")) {
             error_report("not able to chdir to /: %s", strerror(errno));
diff --git a/vl.c b/vl.c
index fa44138..457ff2a 100644
--- a/vl.c
+++ b/vl.c
@@ -4578,6 +4578,12 @@ int main(int argc, char **argv, char **envp)
     parse_numa_opts(current_machine);
 
     /* do monitor/qmp handling at preconfig state if requested */
+    if (!preconfig_exit_requested && is_daemonized()) {
+        /* signal parent QEMU to exit, libvirt treats it as a sign
+         * that monitor socket is ready to accept connections
+         */
+        os_setup_post();
+    }
     main_loop();
 
     /* from here on runstate is RUN_STATE_PRELAUNCH */
-- 
2.7.4

  reply	other threads:[~2018-06-07 12:00 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-05 14:00 [Qemu-devel] [PATCH v3 0/2] fix -nodefaults and -daemonize regressions caused by --preconfig introduction Igor Mammedov
2018-06-05 14:00 ` [Qemu-devel] [PATCH v3 1/2] cli: Don't run early event loop if no --preconfig was specified Igor Mammedov
2018-06-05 18:12   ` Eduardo Habkost
2018-06-06  7:22     ` Igor Mammedov
2018-06-11 17:34       ` Eduardo Habkost
2018-06-05 14:00 ` [Qemu-devel] [PATCH v3 2/2] vl: fix use of --daemonize with --preconfig Igor Mammedov
2018-06-05 15:13   ` Eric Blake
2018-06-05 15:28     ` [Qemu-devel] [PATCH v4 " Igor Mammedov
2018-06-05 18:30   ` [Qemu-devel] [PATCH v3 " Eduardo Habkost
2018-06-06  8:34     ` Igor Mammedov
2018-06-06  8:37     ` [Qemu-devel] [PATCH v5 " Igor Mammedov
2018-06-06 13:50       ` Eduardo Habkost
2018-06-07 12:00         ` Igor Mammedov [this message]
2018-06-08 13:21           ` [Qemu-devel] [PATCH v6 " Eduardo Habkost
2018-06-11 13:16             ` Igor Mammedov
2018-06-11 19:06               ` Eduardo Habkost
2018-06-11 21:29                 ` Igor Mammedov
2018-06-11 22:36                   ` Eduardo Habkost
2018-06-12  9:17                     ` [Qemu-devel] [libvirt] " Michal Privoznik
2018-06-12 12:42                       ` Igor Mammedov
2018-06-12 12:50                         ` Daniel P. Berrangé
2018-06-13 14:17                           ` Eduardo Habkost
2018-06-13 14:23                             ` Daniel P. Berrangé
2018-06-13 17:09                               ` Eduardo Habkost
2018-06-14 12:32                                 ` Igor Mammedov
2018-06-12 13:04                         ` Michal Privoznik
2018-06-12 13:10                           ` Peter Krempa
2018-06-12 13:17                           ` Daniel P. Berrangé
2018-06-06  8:55 ` [Qemu-devel] [PATCH v3 0/2] fix -nodefaults and -daemonize regressions caused by --preconfig introduction no-reply

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=1528372809-175770-1-git-send-email-imammedo@redhat.com \
    --to=imammedo@redhat.com \
    --cc=berrange@redhat.com \
    --cc=eblake@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=ldoktor@redhat.com \
    --cc=mreitz@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).