From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:37110) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R62Z4-0007Qb-O7 for qemu-devel@nongnu.org; Tue, 20 Sep 2011 11:48:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R62Z3-0002yW-5F for qemu-devel@nongnu.org; Tue, 20 Sep 2011 11:48:54 -0400 Received: from mail-ww0-f53.google.com ([74.125.82.53]:33240) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R62Z2-0002xw-RU for qemu-devel@nongnu.org; Tue, 20 Sep 2011 11:48:53 -0400 Received: by wwg14 with SMTP id 14so831985wwg.10 for ; Tue, 20 Sep 2011 08:48:52 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <4E78B5E1.4080608@redhat.com> Date: Tue, 20 Sep 2011 17:48:49 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <2fb9e253266c4926a168168c854fdf5c68ccfca3.1316524908.git.quintela@redhat.com> In-Reply-To: <2fb9e253266c4926a168168c854fdf5c68ccfca3.1316524908.git.quintela@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/7] migration: only flush when there are no errors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org On 09/20/2011 03:24 PM, Juan Quintela wrote: > If we have one error while migrating, and then we issuse a > "migrate_cancel" command, guest hang. Fix it for flushing only when > migration is in MIG_STATE_ACTIVE. In case of error of cancellation, > don't flush. > > We had an infinite loop at buffered_close() > > while (!s->has_error && s->buffer_size) { > buffered_flush(s); > if (s->freeze_output) > s->wait_for_unfreeze(s); > } > > There was no errors, there were things to send, and connection was > broken. send() returns -EAGAIN, so we freezed output, but we > unfreeze_output and try again. > > Signed-off-by: Juan Quintela I don't like the idea of adding an extra argument to fix a bug elsewhere. I don't consider this even a safety net, since it is relying anyway on the migration code setting the new argument correctly: diff --git a/migration.c b/migration.c index 9a93e3b..15d001e 100644 --- a/migration.c +++ b/migration.c @@ -288,7 +288,7 @@ int migrate_fd_cleanup(FdMigrationState *s) if (s->file) { DPRINTF("closing file\n"); - if (qemu_fclose(s->file) != 0) { + if (qemu_fclose(s->file, s->state == MIG_STATE_ACTIVE) != 0) { ret = -1; } s->file = NULL; Dan's patch is the right fix, this one can be dropped altogether. If anything, you may consider making wait_for_unfreeze return an error code, and set has_error when wait_for_unfreeze returns an error. Alternatively, merge QEMUFile and BufferedFile's has_error flags, and use qemu_file_set_error when migration is canceled. Paolo