| Message ID | 20210804212724.07e411d6@gecko.fritz.box |
|---|---|
| State | New |
| Headers | show |
| Series | multifd: Implement yank for multifd send side | expand |
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
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
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 */
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(-)