From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53036) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UYFIP-0004EP-I5 for qemu-devel@nongnu.org; Fri, 03 May 2013 08:41:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UYFIO-0000Vc-8L for qemu-devel@nongnu.org; Fri, 03 May 2013 08:41:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49948) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UYFIO-0000VR-0r for qemu-devel@nongnu.org; Fri, 03 May 2013 08:41:04 -0400 Date: Fri, 3 May 2013 14:40:55 +0200 From: Kevin Wolf Message-ID: <20130503124055.GI3171@dhcp-200-207.str.redhat.com> References: <4c63abcaa4ffbb1e36a27521812c780fe084731b.1366817130.git.phrdina@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4c63abcaa4ffbb1e36a27521812c780fe084731b.1366817130.git.phrdina@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 10/12] savevm: update error reporting of qemu_savevm_state() and related functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Hrdina Cc: xiawenc@linux.vnet.ibm.com, lcapitulino@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com Am 24.04.2013 um 17:32 hat Pavel Hrdina geschrieben: > Signed-off-by: Pavel Hrdina > --- > include/sysemu/sysemu.h | 7 ++++--- > migration.c | 6 +++--- > savevm.c | 38 +++++++++++++++++++------------------- > 3 files changed, 26 insertions(+), 25 deletions(-) > --- a/migration.c > +++ b/migration.c > @@ -508,7 +508,7 @@ static void *migration_thread(void *opaque) > bool old_vm_running = false; > > DPRINTF("beginning savevm\n"); > - qemu_savevm_state_begin(s->file, &s->params); > + qemu_savevm_state_begin(s->file, &s->params, NULL); > > while (s->state == MIG_STATE_ACTIVE) { > int64_t current_time; > @@ -519,7 +519,7 @@ static void *migration_thread(void *opaque) > pending_size = qemu_savevm_state_pending(s->file, max_size); > DPRINTF("pending size %lu max %lu\n", pending_size, max_size); > if (pending_size && pending_size >= max_size) { > - qemu_savevm_state_iterate(s->file); > + qemu_savevm_state_iterate(s->file, NULL); > } else { > DPRINTF("done iterating\n"); > qemu_mutex_lock_iothread(); > @@ -528,7 +528,7 @@ static void *migration_thread(void *opaque) > old_vm_running = runstate_is_running(); > vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); > qemu_file_set_rate_limit(s->file, INT_MAX); > - qemu_savevm_state_complete(s->file); > + qemu_savevm_state_complete(s->file, NULL); > qemu_mutex_unlock_iothread(); > if (!qemu_file_get_error(s->file)) { > migrate_finish_set_state(s, MIG_STATE_COMPLETED); Why is it okay to ignore any errors in this function? > diff --git a/savevm.c b/savevm.c > index 6458621..749d49d 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -1972,37 +1976,33 @@ void qemu_savevm_state_cancel(void) > } > } > > -static int qemu_savevm_state(QEMUFile *f) > +static void qemu_savevm_state(QEMUFile *f, Error **errp) > { > - int ret; > MigrationParams params = { > .blk = 0, > .shared = 0 > }; > > - if (qemu_savevm_state_blocked(NULL)) { > - return -EINVAL; > + if (qemu_savevm_state_blocked(errp)) { > + return; > } > > qemu_mutex_unlock_iothread(); > - qemu_savevm_state_begin(f, ¶ms); > + qemu_savevm_state_begin(f, ¶ms, errp); > qemu_mutex_lock_iothread(); You need to check errp here. > while (qemu_file_get_error(f) == 0) { > - if (qemu_savevm_state_iterate(f) > 0) { > + if (qemu_savevm_state_iterate(f, errp) > 0) { > break; > } And here as well. The problem is that you must not overwrite an error, therefore you need to explicitly free and ignore unused errors, and decide which one takes precedence. > } > > - ret = qemu_file_get_error(f); > - if (ret == 0) { > - qemu_savevm_state_complete(f); > - ret = qemu_file_get_error(f); > + if (!qemu_file_get_error(f)) { > + qemu_savevm_state_complete(f, errp); > } > - if (ret != 0) { > + if (qemu_file_get_error(f)) { > qemu_savevm_state_cancel(); > } > - return ret; > } Kevin