From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54514) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RFoy3-000205-LF for qemu-devel@nongnu.org; Mon, 17 Oct 2011 11:19:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RFoxz-0003hd-Gl for qemu-devel@nongnu.org; Mon, 17 Oct 2011 11:19:07 -0400 Received: from mail-gy0-f173.google.com ([209.85.160.173]:53146) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RFoxz-0003hX-7M for qemu-devel@nongnu.org; Mon, 17 Oct 2011 11:19:03 -0400 Received: by gyg10 with SMTP id 10so3834785gyg.4 for ; Mon, 17 Oct 2011 08:19:02 -0700 (PDT) Message-ID: <4E9C4763.5010403@codemonkey.ws> Date: Mon, 17 Oct 2011 10:18:59 -0500 From: Anthony Liguori MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 25/36] migration: Our release callback was just free List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org On 10/11/2011 05:00 AM, Juan Quintela wrote: > We called it from a single place, and always with state != > MIG_STATE_ACTIVE. Just remove the whole callback. For users of the > notifier, notice that this is exactly the case where they don't care, > we are just freeing the state from previous failed migration (it can't > be a sucessful one, otherwise we would not be running on that machine > in the first place). > > Signed-off-by: Juan Quintela > --- > migration.c | 19 +------------------ > migration.h | 1 - > 2 files changed, 1 insertions(+), 19 deletions(-) > > diff --git a/migration.c b/migration.c > index a8e936e..689464d 100644 > --- a/migration.c > +++ b/migration.c > @@ -123,10 +123,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data) > goto free_migrate_state; > } > > - if (current_migration) { > - current_migration->release(current_migration); > - } So ->release() calls migrate_fd_release() which if (s->state == MIG_STATE_ACTIVE) changes the state to MIG_STATE_CANCELLED and calls migrate_fd_cleanup(). This path is triggered *if* you start a migration while another is active. Now you just free() the old migration and never bother cancelling it (and calling migrate_fd_cleanup). Regards, Anthony Liguori > - > + g_free(current_migration); > current_migration = s; > notifier_list_notify(&migration_state_notifiers, NULL); > return 0; > @@ -416,19 +413,6 @@ static void migrate_fd_cancel(MigrationState *s) > migrate_fd_cleanup(s); > } > > -static void migrate_fd_release(MigrationState *s) > -{ > - > - DPRINTF("releasing state\n"); > - > - if (s->state == MIG_STATE_ACTIVE) { > - s->state = MIG_STATE_CANCELLED; > - notifier_list_notify(&migration_state_notifiers, NULL); > - migrate_fd_cleanup(s); > - } > - g_free(s); > -} > - > static void migrate_fd_wait_for_unfreeze(void *opaque) > { > MigrationState *s = opaque; > @@ -511,7 +495,6 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit, > > s->cancel = migrate_fd_cancel; > s->get_status = migrate_fd_get_status; > - s->release = migrate_fd_release; > s->blk = blk; > s->shared = inc; > s->mon = NULL; > diff --git a/migration.h b/migration.h > index 3165140..1cdb539 100644 > --- a/migration.h > +++ b/migration.h > @@ -40,7 +40,6 @@ struct MigrationState > int (*write)(MigrationState *s, const void *buff, size_t size); > void (*cancel)(MigrationState *s); > int (*get_status)(MigrationState *s); > - void (*release)(MigrationState *s); > void *opaque; > int blk; > int shared;