| Message ID | 20170717134238.1966-2-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
* Juan Quintela (quintela@redhat.com) wrote: > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > migration/channel.c | 3 ++- > migration/channel.h | 2 +- > migration/exec.c | 6 ++++-- > migration/socket.c | 12 ++++++++---- > 4 files changed, 15 insertions(+), 8 deletions(-) > > diff --git a/migration/channel.c b/migration/channel.c > index 3b7252f..719055d 100644 > --- a/migration/channel.c > +++ b/migration/channel.c > @@ -19,7 +19,7 @@ > #include "qapi/error.h" > #include "io/channel-tls.h" > > -void migration_channel_process_incoming(QIOChannel *ioc) > +gboolean migration_channel_process_incoming(QIOChannel *ioc) > { > MigrationState *s = migrate_get_current(); > > @@ -39,6 +39,7 @@ void migration_channel_process_incoming(QIOChannel *ioc) > QEMUFile *f = qemu_fopen_channel_input(ioc); > migration_fd_process_incoming(f); > } > + return FALSE; /* unregister */ > } > > > diff --git a/migration/channel.h b/migration/channel.h > index e4b4057..72cbc9f 100644 > --- a/migration/channel.h > +++ b/migration/channel.h > @@ -18,7 +18,7 @@ > > #include "io/channel.h" > > -void migration_channel_process_incoming(QIOChannel *ioc); > +gboolean migration_channel_process_incoming(QIOChannel *ioc); Can you add a comment here that says what the return value means. Dave > void migration_channel_connect(MigrationState *s, > QIOChannel *ioc, > diff --git a/migration/exec.c b/migration/exec.c > index 08b599e..2827f15 100644 > --- a/migration/exec.c > +++ b/migration/exec.c > @@ -47,9 +47,11 @@ static gboolean exec_accept_incoming_migration(QIOChannel *ioc, > GIOCondition condition, > gpointer opaque) > { > - migration_channel_process_incoming(ioc); > + gboolean result; > + > + result = migration_channel_process_incoming(ioc); > object_unref(OBJECT(ioc)); > - return FALSE; /* unregister */ > + return result; > } > > void exec_start_incoming_migration(const char *command, Error **errp) > diff --git a/migration/socket.c b/migration/socket.c > index 757d382..6195596 100644 > --- a/migration/socket.c > +++ b/migration/socket.c > @@ -136,25 +136,29 @@ static gboolean socket_accept_incoming_migration(QIOChannel *ioc, > { > QIOChannelSocket *sioc; > Error *err = NULL; > + gboolean result; > > sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc), > &err); > if (!sioc) { > error_report("could not accept migration connection (%s)", > error_get_pretty(err)); > + result = FALSE; /* unregister */ > goto out; > } > > trace_migration_socket_incoming_accepted(); > > qio_channel_set_name(QIO_CHANNEL(sioc), "migration-socket-incoming"); > - migration_channel_process_incoming(QIO_CHANNEL(sioc)); > + result = migration_channel_process_incoming(QIO_CHANNEL(sioc)); > object_unref(OBJECT(sioc)); > > out: > - /* Close listening socket as its no longer needed */ > - qio_channel_close(ioc, NULL); > - return FALSE; /* unregister */ > + if (result == FALSE) { > + /* Close listening socket as its no longer needed */ > + qio_channel_close(ioc, NULL); > + } > + return result; > } > > > -- > 2.9.4 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Wed, Jul 19, 2017 at 04:01:10PM +0100, Dr. David Alan Gilbert wrote: > * Juan Quintela (quintela@redhat.com) wrote: > > Signed-off-by: Juan Quintela <quintela@redhat.com> > > --- > > migration/channel.c | 3 ++- > > migration/channel.h | 2 +- > > migration/exec.c | 6 ++++-- > > migration/socket.c | 12 ++++++++---- > > 4 files changed, 15 insertions(+), 8 deletions(-) > > > > diff --git a/migration/channel.c b/migration/channel.c > > index 3b7252f..719055d 100644 > > --- a/migration/channel.c > > +++ b/migration/channel.c > > @@ -19,7 +19,7 @@ > > #include "qapi/error.h" > > #include "io/channel-tls.h" > > > > -void migration_channel_process_incoming(QIOChannel *ioc) > > +gboolean migration_channel_process_incoming(QIOChannel *ioc) > > { > > MigrationState *s = migrate_get_current(); > > > > @@ -39,6 +39,7 @@ void migration_channel_process_incoming(QIOChannel *ioc) > > QEMUFile *f = qemu_fopen_channel_input(ioc); > > migration_fd_process_incoming(f); > > } > > + return FALSE; /* unregister */ > > } > > > > > > diff --git a/migration/channel.h b/migration/channel.h > > index e4b4057..72cbc9f 100644 > > --- a/migration/channel.h > > +++ b/migration/channel.h > > @@ -18,7 +18,7 @@ > > > > #include "io/channel.h" > > > > -void migration_channel_process_incoming(QIOChannel *ioc); > > +gboolean migration_channel_process_incoming(QIOChannel *ioc); > > Can you add a comment here that says what the return value means. And, looks like we have G_SOURCE_CONTINUE and G_SOURCE_REMOVE: https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html#G-SOURCE-CONTINUE:CAPS Maybe we can use them as well? I think the problem is that GSourceFunc's return code (which is a gboolean) is not clear enough.
On Thu, Jul 20, 2017 at 03:00:23PM +0800, Peter Xu wrote: > On Wed, Jul 19, 2017 at 04:01:10PM +0100, Dr. David Alan Gilbert wrote: > > * Juan Quintela (quintela@redhat.com) wrote: > > > Signed-off-by: Juan Quintela <quintela@redhat.com> > > > --- > > > migration/channel.c | 3 ++- > > > migration/channel.h | 2 +- > > > migration/exec.c | 6 ++++-- > > > migration/socket.c | 12 ++++++++---- > > > 4 files changed, 15 insertions(+), 8 deletions(-) > > > > > > diff --git a/migration/channel.c b/migration/channel.c > > > index 3b7252f..719055d 100644 > > > --- a/migration/channel.c > > > +++ b/migration/channel.c > > > @@ -19,7 +19,7 @@ > > > #include "qapi/error.h" > > > #include "io/channel-tls.h" > > > > > > -void migration_channel_process_incoming(QIOChannel *ioc) > > > +gboolean migration_channel_process_incoming(QIOChannel *ioc) > > > { > > > MigrationState *s = migrate_get_current(); > > > > > > @@ -39,6 +39,7 @@ void migration_channel_process_incoming(QIOChannel *ioc) > > > QEMUFile *f = qemu_fopen_channel_input(ioc); > > > migration_fd_process_incoming(f); > > > } > > > + return FALSE; /* unregister */ > > > } > > > > > > > > > diff --git a/migration/channel.h b/migration/channel.h > > > index e4b4057..72cbc9f 100644 > > > --- a/migration/channel.h > > > +++ b/migration/channel.h > > > @@ -18,7 +18,7 @@ > > > > > > #include "io/channel.h" > > > > > > -void migration_channel_process_incoming(QIOChannel *ioc); > > > +gboolean migration_channel_process_incoming(QIOChannel *ioc); > > > > Can you add a comment here that says what the return value means. > > And, looks like we have G_SOURCE_CONTINUE and G_SOURCE_REMOVE: > > https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html#G-SOURCE-CONTINUE:CAPS > > Maybe we can use them as well? Those are newer than our min required glib version, though we could add compat defines for them Regards, Daniel
"Daniel P. Berrange" <berrange@redhat.com> wrote: > On Thu, Jul 20, 2017 at 03:00:23PM +0800, Peter Xu wrote: >> On Wed, Jul 19, 2017 at 04:01:10PM +0100, Dr. David Alan Gilbert wrote: >> > * Juan Quintela (quintela@redhat.com) wrote: >> > > >> > > -void migration_channel_process_incoming(QIOChannel *ioc); >> > > +gboolean migration_channel_process_incoming(QIOChannel *ioc); >> > >> > Can you add a comment here that says what the return value means. Added comment for the two functions (in the .c file through) >> >> And, looks like we have G_SOURCE_CONTINUE and G_SOURCE_REMOVE: Used this ones, thanks. >> https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html#G-SOURCE-CONTINUE:CAPS >> >> Maybe we can use them as well? > > Those are newer than our min required glib version, though we could > add compat defines for them Added the compatibility macros. Thanks to all of you, Juan.
diff --git a/migration/channel.c b/migration/channel.c index 3b7252f..719055d 100644 --- a/migration/channel.c +++ b/migration/channel.c @@ -19,7 +19,7 @@ #include "qapi/error.h" #include "io/channel-tls.h" -void migration_channel_process_incoming(QIOChannel *ioc) +gboolean migration_channel_process_incoming(QIOChannel *ioc) { MigrationState *s = migrate_get_current(); @@ -39,6 +39,7 @@ void migration_channel_process_incoming(QIOChannel *ioc) QEMUFile *f = qemu_fopen_channel_input(ioc); migration_fd_process_incoming(f); } + return FALSE; /* unregister */ } diff --git a/migration/channel.h b/migration/channel.h index e4b4057..72cbc9f 100644 --- a/migration/channel.h +++ b/migration/channel.h @@ -18,7 +18,7 @@ #include "io/channel.h" -void migration_channel_process_incoming(QIOChannel *ioc); +gboolean migration_channel_process_incoming(QIOChannel *ioc); void migration_channel_connect(MigrationState *s, QIOChannel *ioc, diff --git a/migration/exec.c b/migration/exec.c index 08b599e..2827f15 100644 --- a/migration/exec.c +++ b/migration/exec.c @@ -47,9 +47,11 @@ static gboolean exec_accept_incoming_migration(QIOChannel *ioc, GIOCondition condition, gpointer opaque) { - migration_channel_process_incoming(ioc); + gboolean result; + + result = migration_channel_process_incoming(ioc); object_unref(OBJECT(ioc)); - return FALSE; /* unregister */ + return result; } void exec_start_incoming_migration(const char *command, Error **errp) diff --git a/migration/socket.c b/migration/socket.c index 757d382..6195596 100644 --- a/migration/socket.c +++ b/migration/socket.c @@ -136,25 +136,29 @@ static gboolean socket_accept_incoming_migration(QIOChannel *ioc, { QIOChannelSocket *sioc; Error *err = NULL; + gboolean result; sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc), &err); if (!sioc) { error_report("could not accept migration connection (%s)", error_get_pretty(err)); + result = FALSE; /* unregister */ goto out; } trace_migration_socket_incoming_accepted(); qio_channel_set_name(QIO_CHANNEL(sioc), "migration-socket-incoming"); - migration_channel_process_incoming(QIO_CHANNEL(sioc)); + result = migration_channel_process_incoming(QIO_CHANNEL(sioc)); object_unref(OBJECT(sioc)); out: - /* Close listening socket as its no longer needed */ - qio_channel_close(ioc, NULL); - return FALSE; /* unregister */ + if (result == FALSE) { + /* Close listening socket as its no longer needed */ + qio_channel_close(ioc, NULL); + } + return result; }
Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/channel.c | 3 ++- migration/channel.h | 2 +- migration/exec.c | 6 ++++-- migration/socket.c | 12 ++++++++---- 4 files changed, 15 insertions(+), 8 deletions(-)