The old incoming migration is running in main thread and default
gcontext. With the new qio_channel_add_watch_full() we can now let it
run in the thread's own gcontext (if there is one).
Currently this patch does nothing alone. But when any of the incoming
migration is run in another iothread (e.g., the upcoming migrate-recover
command), this patch will bind the incoming logic to the iothread
instead of the main thread (which may already get page faulted and
hanged).
RDMA is not considered for now since it's not even using the QIO APIs at
all.
CC: Juan Quintela <quintela@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/exec.c | 11 ++++++-----
migration/fd.c | 11 ++++++-----
migration/socket.c | 12 +++++++-----
3 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/migration/exec.c b/migration/exec.c
index 0bc5a427dd..f401fc005e 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -55,6 +55,7 @@ void exec_start_incoming_migration(const char *command, Error **errp)
{
QIOChannel *ioc;
const char *argv[] = { "/bin/sh", "-c", command, NULL };
+ GSource *source;
trace_migration_exec_incoming(command);
ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
@@ -65,9 +66,9 @@ void exec_start_incoming_migration(const char *command, Error **errp)
}
qio_channel_set_name(ioc, "migration-exec-incoming");
- qio_channel_add_watch(ioc,
- G_IO_IN,
- exec_accept_incoming_migration,
- NULL,
- NULL);
+ source = qio_channel_add_watch_full(ioc, G_IO_IN,
+ exec_accept_incoming_migration,
+ NULL, NULL,
+ g_main_context_get_thread_default());
+ g_source_unref(source);
}
diff --git a/migration/fd.c b/migration/fd.c
index cd06182d1e..9c593eb3ff 100644
--- a/migration/fd.c
+++ b/migration/fd.c
@@ -55,6 +55,7 @@ void fd_start_incoming_migration(const char *infd, Error **errp)
{
QIOChannel *ioc;
int fd;
+ GSource *source;
fd = strtol(infd, NULL, 0);
trace_migration_fd_incoming(fd);
@@ -66,9 +67,9 @@ void fd_start_incoming_migration(const char *infd, Error **errp)
}
qio_channel_set_name(QIO_CHANNEL(ioc), "migration-fd-incoming");
- qio_channel_add_watch(ioc,
- G_IO_IN,
- fd_accept_incoming_migration,
- NULL,
- NULL);
+ source = qio_channel_add_watch_full(ioc, G_IO_IN,
+ fd_accept_incoming_migration,
+ NULL, NULL,
+ g_main_context_get_thread_default());
+ g_source_unref(source);
}
diff --git a/migration/socket.c b/migration/socket.c
index e090097077..82c330083c 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -164,6 +164,7 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
Error **errp)
{
QIOChannelSocket *listen_ioc = qio_channel_socket_new();
+ GSource *source;
qio_channel_set_name(QIO_CHANNEL(listen_ioc),
"migration-socket-listener");
@@ -173,11 +174,12 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
return;
}
- qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
- G_IO_IN,
- socket_accept_incoming_migration,
- listen_ioc,
- (GDestroyNotify)object_unref);
+ source = qio_channel_add_watch_full(QIO_CHANNEL(listen_ioc), G_IO_IN,
+ socket_accept_incoming_migration,
+ listen_ioc,
+ (GDestroyNotify)object_unref,
+ g_main_context_get_thread_default());
+ g_source_unref(source);
}
void tcp_start_incoming_migration(const char *host_port, Error **errp)
--
2.14.3
On Wed, Feb 28, 2018 at 01:06:23PM +0800, Peter Xu wrote:
> The old incoming migration is running in main thread and default
> gcontext. With the new qio_channel_add_watch_full() we can now let it
> run in the thread's own gcontext (if there is one).
>
> Currently this patch does nothing alone. But when any of the incoming
> migration is run in another iothread (e.g., the upcoming migrate-recover
> command), this patch will bind the incoming logic to the iothread
> instead of the main thread (which may already get page faulted and
> hanged).
>
> RDMA is not considered for now since it's not even using the QIO APIs at
> all.
Errm, yes, it is.
struct QIOChannelRDMA {
QIOChannel parent;
RDMAContext *rdma;
QEMUFile *file;
size_t len;
bool blocking; /* XXX we don't actually honour this yet */
};
....
>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> CC: Laurent Vivier <lvivier@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> migration/exec.c | 11 ++++++-----
> migration/fd.c | 11 ++++++-----
> migration/socket.c | 12 +++++++-----
> 3 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/migration/exec.c b/migration/exec.c
> index 0bc5a427dd..f401fc005e 100644
> --- a/migration/exec.c
> +++ b/migration/exec.c
> @@ -55,6 +55,7 @@ void exec_start_incoming_migration(const char *command, Error **errp)
> {
> QIOChannel *ioc;
> const char *argv[] = { "/bin/sh", "-c", command, NULL };
> + GSource *source;
>
> trace_migration_exec_incoming(command);
> ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
> @@ -65,9 +66,9 @@ void exec_start_incoming_migration(const char *command, Error **errp)
> }
>
> qio_channel_set_name(ioc, "migration-exec-incoming");
> - qio_channel_add_watch(ioc,
> - G_IO_IN,
> - exec_accept_incoming_migration,
> - NULL,
> - NULL);
> + source = qio_channel_add_watch_full(ioc, G_IO_IN,
> + exec_accept_incoming_migration,
> + NULL, NULL,
> + g_main_context_get_thread_default());
> + g_source_unref(source);
> }
> diff --git a/migration/fd.c b/migration/fd.c
> index cd06182d1e..9c593eb3ff 100644
> --- a/migration/fd.c
> +++ b/migration/fd.c
> @@ -55,6 +55,7 @@ void fd_start_incoming_migration(const char *infd, Error **errp)
> {
> QIOChannel *ioc;
> int fd;
> + GSource *source;
>
> fd = strtol(infd, NULL, 0);
> trace_migration_fd_incoming(fd);
> @@ -66,9 +67,9 @@ void fd_start_incoming_migration(const char *infd, Error **errp)
> }
>
> qio_channel_set_name(QIO_CHANNEL(ioc), "migration-fd-incoming");
> - qio_channel_add_watch(ioc,
> - G_IO_IN,
> - fd_accept_incoming_migration,
> - NULL,
> - NULL);
> + source = qio_channel_add_watch_full(ioc, G_IO_IN,
> + fd_accept_incoming_migration,
> + NULL, NULL,
> + g_main_context_get_thread_default());
> + g_source_unref(source);
> }
> diff --git a/migration/socket.c b/migration/socket.c
> index e090097077..82c330083c 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -164,6 +164,7 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
> Error **errp)
> {
> QIOChannelSocket *listen_ioc = qio_channel_socket_new();
> + GSource *source;
>
> qio_channel_set_name(QIO_CHANNEL(listen_ioc),
> "migration-socket-listener");
> @@ -173,11 +174,12 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
> return;
> }
>
> - qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
> - G_IO_IN,
> - socket_accept_incoming_migration,
> - listen_ioc,
> - (GDestroyNotify)object_unref);
> + source = qio_channel_add_watch_full(QIO_CHANNEL(listen_ioc), G_IO_IN,
> + socket_accept_incoming_migration,
> + listen_ioc,
> + (GDestroyNotify)object_unref,
> + g_main_context_get_thread_default());
> + g_source_unref(source);
> }
>
> void tcp_start_incoming_migration(const char *host_port, Error **errp)
> --
> 2.14.3
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Feb 28, 2018 at 09:10:58AM +0000, Daniel P. Berrangé wrote:
> On Wed, Feb 28, 2018 at 01:06:23PM +0800, Peter Xu wrote:
> > The old incoming migration is running in main thread and default
> > gcontext. With the new qio_channel_add_watch_full() we can now let it
> > run in the thread's own gcontext (if there is one).
> >
> > Currently this patch does nothing alone. But when any of the incoming
> > migration is run in another iothread (e.g., the upcoming migrate-recover
> > command), this patch will bind the incoming logic to the iothread
> > instead of the main thread (which may already get page faulted and
> > hanged).
> >
> > RDMA is not considered for now since it's not even using the QIO APIs at
> > all.
>
> Errm, yes, it is.
>
> struct QIOChannelRDMA {
> QIOChannel parent;
> RDMAContext *rdma;
> QEMUFile *file;
> size_t len;
> bool blocking; /* XXX we don't actually honour this yet */
> };
> ....
Ah, you are right. :)
I should say that "it's not using QIO watch framework" since it's
using qemu_set_fd_handler() so it's always on main thread.
Thanks,
--
Peter Xu
* Peter Xu (peterx@redhat.com) wrote:
> The old incoming migration is running in main thread and default
> gcontext. With the new qio_channel_add_watch_full() we can now let it
> run in the thread's own gcontext (if there is one).
>
> Currently this patch does nothing alone. But when any of the incoming
> migration is run in another iothread (e.g., the upcoming migrate-recover
> command), this patch will bind the incoming logic to the iothread
> instead of the main thread (which may already get page faulted and
> hanged).
Does this make any difference to the Postcopy listener thread, which
takes over reading from the main thread once in postcopy mode?
(See savevm.c:postcopy_ram_listen_thread).
Dave
> RDMA is not considered for now since it's not even using the QIO APIs at
> all.
>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> CC: Laurent Vivier <lvivier@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> migration/exec.c | 11 ++++++-----
> migration/fd.c | 11 ++++++-----
> migration/socket.c | 12 +++++++-----
> 3 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/migration/exec.c b/migration/exec.c
> index 0bc5a427dd..f401fc005e 100644
> --- a/migration/exec.c
> +++ b/migration/exec.c
> @@ -55,6 +55,7 @@ void exec_start_incoming_migration(const char *command, Error **errp)
> {
> QIOChannel *ioc;
> const char *argv[] = { "/bin/sh", "-c", command, NULL };
> + GSource *source;
>
> trace_migration_exec_incoming(command);
> ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
> @@ -65,9 +66,9 @@ void exec_start_incoming_migration(const char *command, Error **errp)
> }
>
> qio_channel_set_name(ioc, "migration-exec-incoming");
> - qio_channel_add_watch(ioc,
> - G_IO_IN,
> - exec_accept_incoming_migration,
> - NULL,
> - NULL);
> + source = qio_channel_add_watch_full(ioc, G_IO_IN,
> + exec_accept_incoming_migration,
> + NULL, NULL,
> + g_main_context_get_thread_default());
> + g_source_unref(source);
> }
> diff --git a/migration/fd.c b/migration/fd.c
> index cd06182d1e..9c593eb3ff 100644
> --- a/migration/fd.c
> +++ b/migration/fd.c
> @@ -55,6 +55,7 @@ void fd_start_incoming_migration(const char *infd, Error **errp)
> {
> QIOChannel *ioc;
> int fd;
> + GSource *source;
>
> fd = strtol(infd, NULL, 0);
> trace_migration_fd_incoming(fd);
> @@ -66,9 +67,9 @@ void fd_start_incoming_migration(const char *infd, Error **errp)
> }
>
> qio_channel_set_name(QIO_CHANNEL(ioc), "migration-fd-incoming");
> - qio_channel_add_watch(ioc,
> - G_IO_IN,
> - fd_accept_incoming_migration,
> - NULL,
> - NULL);
> + source = qio_channel_add_watch_full(ioc, G_IO_IN,
> + fd_accept_incoming_migration,
> + NULL, NULL,
> + g_main_context_get_thread_default());
> + g_source_unref(source);
> }
> diff --git a/migration/socket.c b/migration/socket.c
> index e090097077..82c330083c 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -164,6 +164,7 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
> Error **errp)
> {
> QIOChannelSocket *listen_ioc = qio_channel_socket_new();
> + GSource *source;
>
> qio_channel_set_name(QIO_CHANNEL(listen_ioc),
> "migration-socket-listener");
> @@ -173,11 +174,12 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
> return;
> }
>
> - qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
> - G_IO_IN,
> - socket_accept_incoming_migration,
> - listen_ioc,
> - (GDestroyNotify)object_unref);
> + source = qio_channel_add_watch_full(QIO_CHANNEL(listen_ioc), G_IO_IN,
> + socket_accept_incoming_migration,
> + listen_ioc,
> + (GDestroyNotify)object_unref,
> + g_main_context_get_thread_default());
> + g_source_unref(source);
> }
>
> void tcp_start_incoming_migration(const char *host_port, Error **errp)
> --
> 2.14.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Wed, Feb 28, 2018 at 05:43:50PM +0000, Dr. David Alan Gilbert wrote: > * Peter Xu (peterx@redhat.com) wrote: > > The old incoming migration is running in main thread and default > > gcontext. With the new qio_channel_add_watch_full() we can now let it > > run in the thread's own gcontext (if there is one). > > > > Currently this patch does nothing alone. But when any of the incoming > > migration is run in another iothread (e.g., the upcoming migrate-recover > > command), this patch will bind the incoming logic to the iothread > > instead of the main thread (which may already get page faulted and > > hanged). > > Does this make any difference to the Postcopy listener thread, which > takes over reading from the main thread once in postcopy mode? > (See savevm.c:postcopy_ram_listen_thread). It should not. It should only affect when use sends a "migrate-recover" with "run-oob=true". The rest should be the same as before. And since the postcopy ram load thread is a standalone thread with its own initial thread stack (so it's not really in a gmainloop), I can hardly tell how that can be affected since it'll always use its own thread stack. Or, have I missed anything? -- Peter Xu
* Peter Xu (peterx@redhat.com) wrote: > On Wed, Feb 28, 2018 at 05:43:50PM +0000, Dr. David Alan Gilbert wrote: > > * Peter Xu (peterx@redhat.com) wrote: > > > The old incoming migration is running in main thread and default > > > gcontext. With the new qio_channel_add_watch_full() we can now let it > > > run in the thread's own gcontext (if there is one). > > > > > > Currently this patch does nothing alone. But when any of the incoming > > > migration is run in another iothread (e.g., the upcoming migrate-recover > > > command), this patch will bind the incoming logic to the iothread > > > instead of the main thread (which may already get page faulted and > > > hanged). > > > > Does this make any difference to the Postcopy listener thread, which > > takes over reading from the main thread once in postcopy mode? > > (See savevm.c:postcopy_ram_listen_thread). > > It should not. It should only affect when use sends a > "migrate-recover" with "run-oob=true". The rest should be the same as > before. And since the postcopy ram load thread is a standalone thread > with its own initial thread stack (so it's not really in a gmainloop), > I can hardly tell how that can be affected since it'll always use its > own thread stack. OK, I think that's the bit I was worrying about; just since it was another thread that ended up reading from the fd originally handled by the incoming side main thread. Dave > Or, have I missed anything? > > -- > Peter Xu -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
© 2016 - 2025 Red Hat, Inc.