qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Thomas Huth" <thuth@redhat.com>,
	"Daniel P . Berrangé" <berrange@redhat.com>
Subject: [Qemu-devel] [PULL 6/7] io/channel-command: Do not kill the child process after closing the pipe
Date: Thu, 15 Feb 2018 17:50:43 +0000	[thread overview]
Message-ID: <20180215175044.8159-7-berrange@redhat.com> (raw)
In-Reply-To: <20180215175044.8159-1-berrange@redhat.com>

From: Thomas Huth <thuth@redhat.com>

We are currently facing some migration failure on s390x when running
certain avocado-vt tests, e.g. when running the test
type_specific.io-github-autotest-qemu.migrate.with_reboot.exec.gzip_exec.
This test is using 'migrate -d "exec:nc localhost 5200"' for the migration.
The problem is detected at the receiving side, where the migration stream
apparently ends too early. However, the cause for the problem is at the
sending side: After writing the migration stream into the pipe to netcat,
the source QEMU calls qio_channel_command_close() which closes the pipe
and immediately (!) kills the child process afterwards (via the function
qio_channel_command_abort()). So if the  sending netcat did not read the
final bytes from the pipe yet, or  if it did not manage to send out all
its buffers yet, it is killed before the whole migration stream is passed
to the destination side.

QEMU can not know how much time is required by the child process to send
over all migration data, so we should not kill it, neither directly nor
after a delay. Let's simply wait for the child process to exit gracefully
instead (this was also the behaviour of pclose() that was used in "exec:"
migration before the QIOChannel rework).

Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 io/channel-command.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/io/channel-command.c b/io/channel-command.c
index 319c5ed50c..3e7eb17eff 100644
--- a/io/channel-command.c
+++ b/io/channel-command.c
@@ -301,6 +301,9 @@ static int qio_channel_command_close(QIOChannel *ioc,
 {
     QIOChannelCommand *cioc = QIO_CHANNEL_COMMAND(ioc);
     int rv = 0;
+#ifndef WIN32
+    pid_t wp;
+#endif
 
     /* We close FDs before killing, because that
      * gives a better chance of clean shutdown
@@ -315,11 +318,18 @@ static int qio_channel_command_close(QIOChannel *ioc,
         rv = -1;
     }
     cioc->writefd = cioc->readfd = -1;
+
 #ifndef WIN32
-    if (qio_channel_command_abort(cioc, errp) < 0) {
+    do {
+        wp = waitpid(cioc->pid, NULL, 0);
+    } while (wp == (pid_t)-1 && errno == EINTR);
+    if (wp == (pid_t)-1) {
+        error_setg_errno(errp, errno, "Failed to wait for pid %llu",
+                         (unsigned long long)cioc->pid);
         return -1;
     }
 #endif
+
     if (rv < 0) {
         error_setg_errno(errp, errno, "%s",
                          "Unable to close command");
-- 
2.14.3

  parent reply	other threads:[~2018-02-15 17:51 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-15 17:50 [Qemu-devel] [PULL 0/7] Qio next patches Daniel P. Berrangé
2018-02-15 17:50 ` [Qemu-devel] [PULL 1/7] io: fix QIONetListener memory leak Daniel P. Berrangé
2018-02-15 17:50 ` [Qemu-devel] [PULL 2/7] io/channel-websock: handle continuous reads without any data Daniel P. Berrangé
2018-02-15 17:50 ` [Qemu-devel] [PULL 3/7] io: Fix QIOChannelFile when creating and opening read-write Daniel P. Berrangé
2018-02-15 17:50 ` [Qemu-devel] [PULL 4/7] io: Don't call close multiple times in QIOChannelFile Daniel P. Berrangé
2018-02-15 17:50 ` [Qemu-devel] [PULL 5/7] io: Add /dev/fdset/ support to QIOChannelFile Daniel P. Berrangé
2018-02-15 17:50 ` Daniel P. Berrangé [this message]
2018-02-15 17:50 ` [Qemu-devel] [PULL 7/7] allow to build with older sed Daniel P. Berrangé
2018-02-15 19:43 ` [Qemu-devel] [PULL 0/7] Qio next patches Peter Maydell
2018-02-16  8:52   ` Daniel P. Berrangé
2018-02-16 12:51 ` Peter Maydell

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=20180215175044.8159-7-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /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).