* [PATCH] multifd: Implement yank for multifd send side
@ 2021-08-04 19:27 Lukas Straub
2021-08-16 5:44 ` Leonardo Bras Soares Passos
0 siblings, 1 reply; 6+ messages in thread
From: Lukas Straub @ 2021-08-04 19:27 UTC (permalink / raw)
To: qemu-devel
Cc: Li Xiaohui, Leonardo Bras Soares Passos, Dr. David Alan Gilbert,
Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] multifd: Implement yank for multifd send side
2021-08-04 19:27 [PATCH] multifd: Implement yank for multifd send side Lukas Straub
@ 2021-08-16 5:44 ` Leonardo Bras Soares Passos
2021-08-17 17:55 ` Leonardo Bras Soares Passos
0 siblings, 1 reply; 6+ messages in thread
From: Leonardo Bras Soares Passos @ 2021-08-16 5:44 UTC (permalink / raw)
To: Lukas Straub
Cc: Li Xiaohui, Juan Quintela, qemu-devel, Dr. David Alan Gilbert
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] multifd: Implement yank for multifd send side
2021-08-16 5:44 ` Leonardo Bras Soares Passos
@ 2021-08-17 17:55 ` Leonardo Bras Soares Passos
0 siblings, 0 replies; 6+ messages in thread
From: Leonardo Bras Soares Passos @ 2021-08-17 17:55 UTC (permalink / raw)
To: Lukas Straub
Cc: Li Xiaohui, Juan Quintela, qemu-devel, Dr. David Alan Gilbert
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] multifd: Implement yank for multifd send side
@ 2021-09-01 15:58 Lukas Straub
2021-09-01 17:57 ` Peter Xu
2021-09-09 7:18 ` Juan Quintela
0 siblings, 2 replies; 6+ messages in thread
From: Lukas Straub @ 2021-09-01 15:58 UTC (permalink / raw)
To: qemu-devel
Cc: Leonardo Bras Soares Passos, Dr. David Alan Gilbert, Peter Xu,
Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]
When introducing yank functionality in the migration code I forgot
to cover the multifd send side.
Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Tested-by: Leonardo Bras <leobras@redhat.com>
Reviewed-by: Leonardo Bras <leobras@redhat.com>
---
-v2:
-add Tested-by and Reviewed-by tags
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] multifd: Implement yank for multifd send side
2021-09-01 15:58 Lukas Straub
@ 2021-09-01 17:57 ` Peter Xu
2021-09-09 7:18 ` Juan Quintela
1 sibling, 0 replies; 6+ messages in thread
From: Peter Xu @ 2021-09-01 17:57 UTC (permalink / raw)
To: Lukas Straub
Cc: Juan Quintela, qemu-devel, Leonardo Bras Soares Passos,
Dr. David Alan Gilbert
On Wed, Sep 01, 2021 at 05:58:57PM +0200, Lukas Straub 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>
> Tested-by: Leonardo Bras <leobras@redhat.com>
> Reviewed-by: Leonardo Bras <leobras@redhat.com>
> ---
>
> -v2:
> -add Tested-by and Reviewed-by tags
>
> 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
This is probably yet another case that I'm wondering whether we made unregister
of yank function/instance too strict so we should loose them.
Logically a remove/unregister function doesn't need to assert and crash qemu if
the entry doesn't exist at all. Then it's just something like "makes sure XXX
is removed", and noop if lookup failed. The extra fields do not really help us
from anything else..
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] multifd: Implement yank for multifd send side
2021-09-01 15:58 Lukas Straub
2021-09-01 17:57 ` Peter Xu
@ 2021-09-09 7:18 ` Juan Quintela
1 sibling, 0 replies; 6+ messages in thread
From: Juan Quintela @ 2021-09-09 7:18 UTC (permalink / raw)
To: Lukas Straub
Cc: Leonardo Bras Soares Passos, qemu-devel, Peter Xu,
Dr. David Alan Gilbert
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>
> Tested-by: Leonardo Bras <leobras@redhat.com>
> Reviewed-by: Leonardo Bras <leobras@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-09 7:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-04 19:27 [PATCH] multifd: Implement yank for multifd send side Lukas Straub
2021-08-16 5:44 ` Leonardo Bras Soares Passos
2021-08-17 17:55 ` Leonardo Bras Soares Passos
-- strict thread matches above, loose matches on Subject: below --
2021-09-01 15:58 Lukas Straub
2021-09-01 17:57 ` Peter Xu
2021-09-09 7:18 ` Juan Quintela
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).