From patchwork Mon Feb 11 16:01:45 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Hajnoczi X-Patchwork-Id: 219631 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 71C522C02F4 for ; Tue, 12 Feb 2013 03:02:16 +1100 (EST) Received: from localhost ([::1]:57353 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U4vpe-0006DV-JX for incoming@patchwork.ozlabs.org; Mon, 11 Feb 2013 11:02:14 -0500 Received: from eggs.gnu.org ([208.118.235.92]:44037) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U4vpS-0006C7-3t for qemu-devel@nongnu.org; Mon, 11 Feb 2013 11:02:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1U4vpJ-0001vj-OG for qemu-devel@nongnu.org; Mon, 11 Feb 2013 11:02:02 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34377) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U4vpJ-0001vY-Ff for qemu-devel@nongnu.org; Mon, 11 Feb 2013 11:01:53 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r1BG1qYI024773 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 11 Feb 2013 11:01:52 -0500 Received: from localhost (ovpn-112-38.ams2.redhat.com [10.36.112.38]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r1BG1o9M009471; Mon, 11 Feb 2013 11:01:51 -0500 From: Stefan Hajnoczi To: Date: Mon, 11 Feb 2013 17:01:45 +0100 Message-Id: <1360598505-5512-1-git-send-email-stefanha@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Kevin Wolf , Paolo Bonzini , Anthony Liguori , Stefan Hajnoczi , Juan Quintela Subject: [Qemu-devel] [PATCH for-1.4] migration: restrict scope of incoming fd read handler X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org The incoming migration is processed in a coroutine and uses an fd read handler to enter the yielded coroutine when data becomes available. The read handler was set too broadly, so that spurious coroutine entries were be triggered if other coroutine users yielded (like the block layer's bdrv_write() function). Install the fd read only only when yielding for more data to become available. This prevents spurious coroutine entries which break code that assumes only a specific set of places can re-enter the coroutine. This patch fixes crashes in block/raw-posix.c that are triggered with "migrate -b" when qiov becomes a dangling pointer due to a spurious coroutine entry that frees qiov early. Signed-off-by: Stefan Hajnoczi --- Note: this patch replaces "[PATCH for-1.4 stable] block: handle spurious coroutine entries". It's an alternative fix that I hope Kevin will like more. migration.c | 8 -------- savevm.c | 34 ++++++++++++++++++++++++++++++---- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/migration.c b/migration.c index c4589b1..b1ebb01 100644 --- a/migration.c +++ b/migration.c @@ -95,7 +95,6 @@ static void process_incoming_migration_co(void *opaque) int ret; ret = qemu_loadvm_state(f); - qemu_set_fd_handler(qemu_get_fd(f), NULL, NULL, NULL); qemu_fclose(f); if (ret < 0) { fprintf(stderr, "load of migration failed\n"); @@ -115,12 +114,6 @@ static void process_incoming_migration_co(void *opaque) } } -static void enter_migration_coroutine(void *opaque) -{ - Coroutine *co = opaque; - qemu_coroutine_enter(co, NULL); -} - void process_incoming_migration(QEMUFile *f) { Coroutine *co = qemu_coroutine_create(process_incoming_migration_co); @@ -128,7 +121,6 @@ void process_incoming_migration(QEMUFile *f) assert(fd != -1); socket_set_nonblock(fd); - qemu_set_fd_handler(fd, enter_migration_coroutine, NULL, co); qemu_coroutine_enter(co, f); } diff --git a/savevm.c b/savevm.c index 4eb29b2..0b6724d 100644 --- a/savevm.c +++ b/savevm.c @@ -140,6 +140,34 @@ typedef struct QEMUFileSocket QEMUFile *file; } QEMUFileSocket; +typedef struct { + Coroutine *co; + int fd; +} FDYieldUntilData; + +static void fd_coroutine_enter(void *opaque) +{ + FDYieldUntilData *data = opaque; + qemu_set_fd_handler(data->fd, NULL, NULL, NULL); + qemu_coroutine_enter(data->co, NULL); +} + +/** + * Yield until a file descriptor becomes readable + * + * Note that this function clobbers the handlers for the file descriptor. + */ +static void coroutine_fn yield_until_fd_readable(int fd) +{ + FDYieldUntilData data; + + assert(qemu_in_coroutine()); + data.co = qemu_coroutine_self(); + data.fd = fd; + qemu_set_fd_handler(fd, fd_coroutine_enter, NULL, &data); + qemu_coroutine_yield(); +} + static int socket_get_fd(void *opaque) { QEMUFileSocket *s = opaque; @@ -158,8 +186,7 @@ static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size) break; } if (socket_error() == EAGAIN) { - assert(qemu_in_coroutine()); - qemu_coroutine_yield(); + yield_until_fd_readable(s->fd); } else if (socket_error() != EINTR) { break; } @@ -205,8 +232,7 @@ static int stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size) break; } if (errno == EAGAIN) { - assert(qemu_in_coroutine()); - qemu_coroutine_yield(); + yield_until_fd_readable(fileno(fp)); } else if (errno != EINTR) { break; }