From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:43719) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RB5uZ-0006zO-Cu for qemu-devel@nongnu.org; Tue, 04 Oct 2011 10:24:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RB5uX-0006z4-TM for qemu-devel@nongnu.org; Tue, 04 Oct 2011 10:23:59 -0400 Received: from mail-iy0-f173.google.com ([209.85.210.173]:36734) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RB5uX-0006yz-NG for qemu-devel@nongnu.org; Tue, 04 Oct 2011 10:23:57 -0400 Received: by iagf6 with SMTP id f6so758321iag.4 for ; Tue, 04 Oct 2011 07:23:56 -0700 (PDT) Message-ID: <4E8B16F8.1050808@codemonkey.ws> Date: Tue, 04 Oct 2011 09:23:52 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1ecde435c002123860f03fa720f20ff9225fab1b.1316782367.git.quintela@redhat.com> In-Reply-To: <1ecde435c002123860f03fa720f20ff9225fab1b.1316782367.git.quintela@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 05/23] migration: Refactor MigrationState creation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org On 09/23/2011 07:56 AM, Juan Quintela wrote: > Signed-off-by: Juan Quintela > --- > migration-exec.c | 16 +--------------- > migration-fd.c | 16 +--------------- > migration-tcp.c | 15 +-------------- > migration-unix.c | 15 +-------------- > migration.c | 29 +++++++++++++++++++++++++---- > migration.h | 11 +++-------- > 6 files changed, 32 insertions(+), 70 deletions(-) > > diff --git a/migration-exec.c b/migration-exec.c > index 0ed5976..db11374 100644 > --- a/migration-exec.c > +++ b/migration-exec.c > @@ -71,7 +71,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon, > MigrationState *s; > FILE *f; > > - s = g_malloc0(sizeof(*s)); > + s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc); > > f = popen(command, "w"); > if (f == NULL) { > @@ -92,20 +92,6 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon, > s->close = exec_close; > s->get_error = file_errno; > s->write = file_write; > - s->cancel = migrate_fd_cancel; > - s->get_status = migrate_fd_get_status; > - s->release = migrate_fd_release; > - > - s->blk = blk; > - s->shared = inc; > - > - s->state = MIG_STATE_ACTIVE; > - s->mon = NULL; > - s->bandwidth_limit = bandwidth_limit; > - > - if (!detach) { > - migrate_fd_monitor_suspend(s, mon); > - } > > migrate_fd_connect(s); > return s; > diff --git a/migration-fd.c b/migration-fd.c > index e78fd4e..2235a2f 100644 > --- a/migration-fd.c > +++ b/migration-fd.c > @@ -59,7 +59,7 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon, > { > MigrationState *s; > > - s = g_malloc0(sizeof(*s)); > + s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc); > > s->fd = monitor_get_fd(mon, fdname); > if (s->fd == -1) { > @@ -75,20 +75,6 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon, > s->get_error = fd_errno; > s->write = fd_write; > s->close = fd_close; > - s->cancel = migrate_fd_cancel; > - s->get_status = migrate_fd_get_status; > - s->release = migrate_fd_release; > - > - s->blk = blk; > - s->shared = inc; > - > - s->state = MIG_STATE_ACTIVE; > - s->mon = NULL; > - s->bandwidth_limit = bandwidth_limit; > - > - if (!detach) { > - migrate_fd_monitor_suspend(s, mon); > - } > > migrate_fd_connect(s); > return s; > diff --git a/migration-tcp.c b/migration-tcp.c > index d6feb23..db8cea2 100644 > --- a/migration-tcp.c > +++ b/migration-tcp.c > @@ -89,21 +89,12 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon, > if (parse_host_port(&addr, host_port)< 0) > return NULL; > > - s = g_malloc0(sizeof(*s)); > + s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc); > > s->get_error = socket_errno; > s->write = socket_write; > s->close = tcp_close; > - s->cancel = migrate_fd_cancel; > - s->get_status = migrate_fd_get_status; > - s->release = migrate_fd_release; > > - s->blk = blk; > - s->shared = inc; > - > - s->state = MIG_STATE_ACTIVE; > - s->mon = NULL; > - s->bandwidth_limit = bandwidth_limit; > s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0); > if (s->fd == -1) { > g_free(s); > @@ -112,10 +103,6 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon, > > socket_set_nonblock(s->fd); > > - if (!detach) { > - migrate_fd_monitor_suspend(s, mon); > - } > - > do { > ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr)); > if (ret == -1) > diff --git a/migration-unix.c b/migration-unix.c > index 3b9017b..74c6dde 100644 > --- a/migration-unix.c > +++ b/migration-unix.c > @@ -88,21 +88,12 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon, > addr.sun_family = AF_UNIX; > snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path); > > - s = g_malloc0(sizeof(*s)); > + s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc); > > s->get_error = unix_errno; > s->write = unix_write; > s->close = unix_close; > - s->cancel = migrate_fd_cancel; > - s->get_status = migrate_fd_get_status; > - s->release = migrate_fd_release; > > - s->blk = blk; > - s->shared = inc; > - > - s->state = MIG_STATE_ACTIVE; > - s->mon = NULL; > - s->bandwidth_limit = bandwidth_limit; > s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0); > if (s->fd< 0) { > DPRINTF("Unable to open socket"); > @@ -125,10 +116,6 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon, > goto err_after_open; > } > > - if (!detach) { > - migrate_fd_monitor_suspend(s, mon); > - } > - > if (ret>= 0) > migrate_fd_connect(s); > > diff --git a/migration.c b/migration.c > index b27a322..990667e 100644 > --- a/migration.c > +++ b/migration.c > @@ -263,7 +263,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data) > > /* shared migration helpers */ > > -void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon) > +static void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon) > { > s->mon = mon; > if (monitor_suspend(mon) == 0) { > @@ -404,12 +404,12 @@ void migrate_fd_put_ready(void *opaque) > } > } > > -int migrate_fd_get_status(MigrationState *s) > +static int migrate_fd_get_status(MigrationState *s) > { > return s->state; > } > > -void migrate_fd_cancel(MigrationState *s) > +static void migrate_fd_cancel(MigrationState *s) > { > if (s->state != MIG_STATE_ACTIVE) > return; > @@ -423,7 +423,7 @@ void migrate_fd_cancel(MigrationState *s) > migrate_fd_cleanup(s); > } > > -void migrate_fd_release(MigrationState *s) > +static void migrate_fd_release(MigrationState *s) > { > > DPRINTF("releasing state\n"); > @@ -488,3 +488,24 @@ int get_migration_state(void) > return MIG_STATE_ERROR; > } > } > + > +MigrationState *migrate_create_state(Monitor *mon, int64_t bandwidth_limit, > + int detach, int blk, int inc) I think this would fit better if it were migrate_new() or migrate_open(). It definitely should be in the form noun_verb and not noun_verb_noun(). Regards, Anthony Liguori > +{ > + MigrationState *s = g_malloc0(sizeof(*s)); > + > + 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; > + s->bandwidth_limit = bandwidth_limit; > + s->state = MIG_STATE_ACTIVE; > + > + if (!detach) { > + migrate_fd_monitor_suspend(s, mon); > + } > + > + return s; > +} > diff --git a/migration.h b/migration.h > index e24efed..32462f7 100644 > --- a/migration.h > +++ b/migration.h > @@ -98,8 +98,6 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon, > int blk, > int inc); > > -void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon); > - > void migrate_fd_error(MigrationState *s); > > int migrate_fd_cleanup(MigrationState *s); > @@ -112,16 +110,13 @@ void migrate_fd_connect(MigrationState *s); > > void migrate_fd_put_ready(void *opaque); > > -int migrate_fd_get_status(MigrationState *mig_state); > - > -void migrate_fd_cancel(MigrationState *mig_state); > - > -void migrate_fd_release(MigrationState *mig_state); > - > void migrate_fd_wait_for_unfreeze(void *opaque); > > int migrate_fd_close(void *opaque); > > +MigrationState *migrate_create_state(Monitor *mon, int64_t bandwidth_limit, > + int detach, int blk, int inc); > + > void add_migration_state_change_notifier(Notifier *notify); > void remove_migration_state_change_notifier(Notifier *notify); > int get_migration_state(void);