Re: [PATCH] migration/rdma: Fix return-path case

2023-03-14 Thread Peter Xu
On Tue, Mar 14, 2023 at 05:15:58PM +, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> 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 

Acked-by: Peter Xu 

> @@ -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




Re: [PATCH] migration/rdma: Fix return-path case

2023-03-14 Thread Dr. David Alan Gilbert
* Peter Xu (pet...@redhat.com) wrote:
> On Tue, Mar 14, 2023 at 05:15:58PM +, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > 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 
> 
> Acked-by: Peter Xu 
> 
> > @@ -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
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK




Re: [PATCH] migration/rdma: Fix return-path case

2023-03-14 Thread lizhij...@fujitsu.com


On 15/03/2023 01:15, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> 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 

LGTM.

Reviewed-by: Li Zhijian 



> ---
>   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) {

Re: [PATCH] migration/rdma: Fix return-path case

2023-03-16 Thread Juan Quintela
"Dr. David Alan Gilbert (git)"  wrote:
> From: "Dr. David Alan Gilbert" 
>
> 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 

Reviewed-by: Juan Quintela 

queued.