[v1] multifd: Implement yank for multifd send side

[PATCH] multifd: Implement yank for multifd send side

Posted by Lukas Straub 4 years, 6 months ago

When introducing yank functionality in the migration code I forgot
to cover the multifd send side.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
---

@Leonardo Could you check if this fixes your issue?

 migration/multifd.c | 6 +++++-
 migration/multifd.h | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index 377da78f5b..5a4f158f3c 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -546,6 +546,9 @@ void multifd_save_cleanup(void)
         MultiFDSendParams *p = &multifd_send_state->params[i];
         Error *local_err = NULL;
 
+        if (p->registered_yank) {
+            migration_ioc_unregister_yank(p->c);
+        }
         socket_send_channel_destroy(p->c);
         p->c = NULL;
         qemu_mutex_destroy(&p->mutex);
@@ -813,7 +816,8 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
                 return false;
             }
         } else {
-            /* update for tls qio channel */
+            migration_ioc_register_yank(ioc);
+            p->registered_yank = true;
             p->c = ioc;
             qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
                                    QEMU_THREAD_JOINABLE);
diff --git a/migration/multifd.h b/migration/multifd.h
index 8d6751f5ed..16c4d112d1 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -85,6 +85,8 @@ typedef struct {
     bool running;
     /* should this thread finish */
     bool quit;
+    /* is the yank function registered */
+    bool registered_yank;
     /* thread has work to do */
     int pending_job;
     /* array of pages to sent */
-- 
2.32.0

Re: [PATCH] multifd: Implement yank for multifd send side

Posted by Leonardo Bras Soares Passos 4 years, 5 months ago

Hello Lukas,

On Wed, Aug 4, 2021 at 4:27 PM Lukas Straub <lukasstraub2@web.de> wrote:
>
> When introducing yank functionality in the migration code I forgot
> to cover the multifd send side.
>
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> ---
>
> @Leonardo Could you check if this fixes your issue?

In the same scenario I was testing my previous patch,
(http://patchwork.ozlabs.org/project/qemu-devel/patch/20210730074043.54260-1-leobras@redhat.com/)
this patch also seems to fix the issue .
(https://bugzilla.redhat.com/show_bug.cgi?id=1970337).


>
>  migration/multifd.c | 6 +++++-
>  migration/multifd.h | 2 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 377da78f5b..5a4f158f3c 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -546,6 +546,9 @@ void multifd_save_cleanup(void)
>          MultiFDSendParams *p = &multifd_send_state->params[i];
>          Error *local_err = NULL;
>
> +        if (p->registered_yank) {
> +            migration_ioc_unregister_yank(p->c);
> +        }
>          socket_send_channel_destroy(p->c);
>          p->c = NULL;
>          qemu_mutex_destroy(&p->mutex);
> @@ -813,7 +816,8 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
>                  return false;
>              }
>          } else {
> -            /* update for tls qio channel */
> +            migration_ioc_register_yank(ioc);
> +            p->registered_yank = true;
>              p->c = ioc;
>              qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
>                                     QEMU_THREAD_JOINABLE);
> diff --git a/migration/multifd.h b/migration/multifd.h
> index 8d6751f5ed..16c4d112d1 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -85,6 +85,8 @@ typedef struct {
>      bool running;
>      /* should this thread finish */
>      bool quit;
> +    /* is the yank function registered */
> +    bool registered_yank;
>      /* thread has work to do */
>      int pending_job;
>      /* array of pages to sent */
> --
> 2.32.0

Re: [PATCH] multifd: Implement yank for multifd send side

Posted by Leonardo Bras Soares Passos 4 years, 5 months ago

On Mon, Aug 16, 2021 at 2:44 AM Leonardo Bras Soares Passos
<leobras@redhat.com> wrote:
>
> Hello Lukas,
>
> On Wed, Aug 4, 2021 at 4:27 PM Lukas Straub <lukasstraub2@web.de> wrote:
> >
> > When introducing yank functionality in the migration code I forgot
> > to cover the multifd send side.
> >
> > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > ---
> >
> > @Leonardo Could you check if this fixes your issue?
>
> In the same scenario I was testing my previous patch,
> (http://patchwork.ozlabs.org/project/qemu-devel/patch/20210730074043.54260-1-leobras@redhat.com/)
> this patch also seems to fix the issue .
> (https://bugzilla.redhat.com/show_bug.cgi?id=1970337).

Regarding this single test:
Tested-by: Leonardo Bras <leobras@redhat.com>

I am by no means a yank or migration expert, but the change seems to
make sense based on what I could learn in the above BZ.

So, FWIW:
Reviewed-by: Leonardo Bras <leobras@redhat.com>

Although I think it would be great if a more experienced person could
also review.

Best regards,
Leonardo Bras



>
>
> >
> >  migration/multifd.c | 6 +++++-
> >  migration/multifd.h | 2 ++
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/migration/multifd.c b/migration/multifd.c
> > index 377da78f5b..5a4f158f3c 100644
> > --- a/migration/multifd.c
> > +++ b/migration/multifd.c
> > @@ -546,6 +546,9 @@ void multifd_save_cleanup(void)
> >          MultiFDSendParams *p = &multifd_send_state->params[i];
> >          Error *local_err = NULL;
> >
> > +        if (p->registered_yank) {
> > +            migration_ioc_unregister_yank(p->c);
> > +        }
> >          socket_send_channel_destroy(p->c);
> >          p->c = NULL;
> >          qemu_mutex_destroy(&p->mutex);
> > @@ -813,7 +816,8 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
> >                  return false;
> >              }
> >          } else {
> > -            /* update for tls qio channel */
> > +            migration_ioc_register_yank(ioc);
> > +            p->registered_yank = true;
> >              p->c = ioc;
> >              qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
> >                                     QEMU_THREAD_JOINABLE);
> > diff --git a/migration/multifd.h b/migration/multifd.h
> > index 8d6751f5ed..16c4d112d1 100644
> > --- a/migration/multifd.h
> > +++ b/migration/multifd.h
> > @@ -85,6 +85,8 @@ typedef struct {
> >      bool running;
> >      /* should this thread finish */
> >      bool quit;
> > +    /* is the yank function registered */
> > +    bool registered_yank;
> >      /* thread has work to do */
> >      int pending_job;
> >      /* array of pages to sent */
> > --
> > 2.32.0