| Message ID | 20230314171558.75941-1-dgilbert@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration/rdma: Fix return-path case | expand |
On Tue, Mar 14, 2023 at 05:15:58PM +0000, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > The RDMA code has return-path handling code, but it's only enabled > if postcopy is enabled; if the 'return-path' migration capability > is enabled, the return path is NOT setup but the core migration > code still tries to use it and breaks. > > Enable the RDMA return path if either postcopy or the return-path > capability is enabled. > > bz: https://bugzilla.redhat.com/show_bug.cgi?id=2063615 > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Acked-by: Peter Xu <peterx@redhat.com> > @@ -3373,7 +3373,8 @@ static int qemu_rdma_accept(RDMAContext *rdma) > * initialize the RDMAContext for return path for postcopy after first > * connection request reached. > */ > - if (migrate_postcopy() && !rdma->is_return_path) { > + if ((migrate_postcopy() || migrate_use_return_path()) > + && !rdma->is_return_path) { > rdma_return_path = qemu_rdma_data_init(rdma->host_port, NULL); > if (rdma_return_path == NULL) { > rdma_ack_cm_event(cm_event); It's not extremely clear to me yet on when we should use migrate_postcopy() and when to use migrate_postcopy_ram(). I think it's because I don't know enough on the dirty-bitmaps capability. Do we have some good documentation somewhere? Not much I get from the qapi doc.. # @dirty-bitmaps: If enabled, QEMU will migrate named dirty bitmaps. # (since 2.12) Thanks,
* Peter Xu (peterx@redhat.com) wrote: > On Tue, Mar 14, 2023 at 05:15:58PM +0000, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > The RDMA code has return-path handling code, but it's only enabled > > if postcopy is enabled; if the 'return-path' migration capability > > is enabled, the return path is NOT setup but the core migration > > code still tries to use it and breaks. > > > > Enable the RDMA return path if either postcopy or the return-path > > capability is enabled. > > > > bz: https://bugzilla.redhat.com/show_bug.cgi?id=2063615 > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > Acked-by: Peter Xu <peterx@redhat.com> > > > @@ -3373,7 +3373,8 @@ static int qemu_rdma_accept(RDMAContext *rdma) > > * initialize the RDMAContext for return path for postcopy after first > > * connection request reached. > > */ > > - if (migrate_postcopy() && !rdma->is_return_path) { > > + if ((migrate_postcopy() || migrate_use_return_path()) > > + && !rdma->is_return_path) { > > rdma_return_path = qemu_rdma_data_init(rdma->host_port, NULL); > > if (rdma_return_path == NULL) { > > rdma_ack_cm_event(cm_event); > > It's not extremely clear to me yet on when we should use migrate_postcopy() > and when to use migrate_postcopy_ram(). I think it's because I don't know > enough on the dirty-bitmaps capability. Do we have some good documentation > somewhere? Hmm that's probably a good point. > Not much I get from the qapi doc.. > > # @dirty-bitmaps: If enabled, QEMU will migrate named dirty bitmaps. > # (since 2.12) I don't know of any good docs; I think this is a blocks mechanism; I'm not even sure if it needs the return path. Dave > Thanks, > > -- > Peter Xu >
On 15/03/2023 01:15, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > The RDMA code has return-path handling code, but it's only enabled > if postcopy is enabled; if the 'return-path' migration capability > is enabled, the return path is NOT setup but the core migration > code still tries to use it and breaks. > > Enable the RDMA return path if either postcopy or the return-path > capability is enabled. > > bz: https://bugzilla.redhat.com/show_bug.cgi?id=2063615 > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> LGTM. Reviewed-by: Li Zhijian <lizhijian@fujitsu.com> > --- > migration/rdma.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/migration/rdma.c b/migration/rdma.c > index 288eadc2d2..9d70e9885b 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -3373,7 +3373,8 @@ static int qemu_rdma_accept(RDMAContext *rdma) > * initialize the RDMAContext for return path for postcopy after first > * connection request reached. > */ > - if (migrate_postcopy() && !rdma->is_return_path) { > + if ((migrate_postcopy() || migrate_use_return_path()) > + && !rdma->is_return_path) { > rdma_return_path = qemu_rdma_data_init(rdma->host_port, NULL); > if (rdma_return_path == NULL) { > rdma_ack_cm_event(cm_event); > @@ -3455,7 +3456,8 @@ static int qemu_rdma_accept(RDMAContext *rdma) > } > > /* Accept the second connection request for return path */ > - if (migrate_postcopy() && !rdma->is_return_path) { > + if ((migrate_postcopy() || migrate_use_return_path()) > + && !rdma->is_return_path) { > qemu_set_fd_handler(rdma->channel->fd, rdma_accept_incoming_migration, > NULL, > (void *)(intptr_t)rdma->return_path); > @@ -4192,7 +4194,7 @@ void rdma_start_outgoing_migration(void *opaque, > } > > /* RDMA postcopy need a separate queue pair for return path */ > - if (migrate_postcopy()) { > + if (migrate_postcopy() || migrate_use_return_path()) { > rdma_return_path = qemu_rdma_data_init(host_port, errp); > > if (rdma_return_path == NULL) {
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > The RDMA code has return-path handling code, but it's only enabled > if postcopy is enabled; if the 'return-path' migration capability > is enabled, the return path is NOT setup but the core migration > code still tries to use it and breaks. > > Enable the RDMA return path if either postcopy or the return-path > capability is enabled. > > bz: https://bugzilla.redhat.com/show_bug.cgi?id=2063615 > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> queued.
diff --git a/migration/rdma.c b/migration/rdma.c index 288eadc2d2..9d70e9885b 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -3373,7 +3373,8 @@ static int qemu_rdma_accept(RDMAContext *rdma) * initialize the RDMAContext for return path for postcopy after first * connection request reached. */ - if (migrate_postcopy() && !rdma->is_return_path) { + if ((migrate_postcopy() || migrate_use_return_path()) + && !rdma->is_return_path) { rdma_return_path = qemu_rdma_data_init(rdma->host_port, NULL); if (rdma_return_path == NULL) { rdma_ack_cm_event(cm_event); @@ -3455,7 +3456,8 @@ static int qemu_rdma_accept(RDMAContext *rdma) } /* Accept the second connection request for return path */ - if (migrate_postcopy() && !rdma->is_return_path) { + if ((migrate_postcopy() || migrate_use_return_path()) + && !rdma->is_return_path) { qemu_set_fd_handler(rdma->channel->fd, rdma_accept_incoming_migration, NULL, (void *)(intptr_t)rdma->return_path); @@ -4192,7 +4194,7 @@ void rdma_start_outgoing_migration(void *opaque, } /* RDMA postcopy need a separate queue pair for return path */ - if (migrate_postcopy()) { + if (migrate_postcopy() || migrate_use_return_path()) { rdma_return_path = qemu_rdma_data_init(host_port, errp); if (rdma_return_path == NULL) {