* [PATCH v1 0/2] Migration: fix missing iothread locking
@ 2021-10-05 8:07 Emanuele Giuseppe Esposito
2021-10-05 8:07 ` [PATCH v1 1/2] migration: block-dirty-bitmap: add missing qemu_mutex_lock_iothread Emanuele Giuseppe Esposito
2021-10-05 8:07 ` [PATCH v1 2/2] migration: add missing qemu_mutex_lock_iothread in migration_completion Emanuele Giuseppe Esposito
0 siblings, 2 replies; 6+ messages in thread
From: Emanuele Giuseppe Esposito @ 2021-10-05 8:07 UTC (permalink / raw)
To: qemu-block
Cc: Fam Zheng, Emanuele Giuseppe Esposito,
Vladimir Sementsov-Ogievskiy, Juan Quintela, Eric Blake,
Dr. David Alan Gilbert, qemu-devel, Stefan Hajnoczi,
Paolo Bonzini, John Snow
Some functions (in this case qemu_savevm_state_complete_postcopy() and
init_dirty_bitmap_migration()) assume and document that
qemu_mutex_lock_iothread() is hold.
This seems to have been forgotten in some places, and this series
aims to fix that.
Patch 1 was part of my RFC block layer series "block layer: split
block APIs in graph and I/O" but I decided to do a separate series
for these two bugs, as they are independent from the API split.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Emanuele Giuseppe Esposito (2):
migration: block-dirty-bitmap: add missing qemu_mutex_lock_iothread
migration: add missing qemu_mutex_lock_iothread in
migration_completion
migration/block-dirty-bitmap.c | 5 ++++-
migration/migration.c | 3 +++
2 files changed, 7 insertions(+), 1 deletion(-)
--
2.27.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 1/2] migration: block-dirty-bitmap: add missing qemu_mutex_lock_iothread
2021-10-05 8:07 [PATCH v1 0/2] Migration: fix missing iothread locking Emanuele Giuseppe Esposito
@ 2021-10-05 8:07 ` Emanuele Giuseppe Esposito
2021-11-02 9:11 ` Juan Quintela
2021-10-05 8:07 ` [PATCH v1 2/2] migration: add missing qemu_mutex_lock_iothread in migration_completion Emanuele Giuseppe Esposito
1 sibling, 1 reply; 6+ messages in thread
From: Emanuele Giuseppe Esposito @ 2021-10-05 8:07 UTC (permalink / raw)
To: qemu-block
Cc: Fam Zheng, Emanuele Giuseppe Esposito,
Vladimir Sementsov-Ogievskiy, Juan Quintela, Eric Blake,
Dr. David Alan Gilbert, qemu-devel, Stefan Hajnoczi,
Paolo Bonzini, John Snow
init_dirty_bitmap_migration assumes the iothread lock (BQL)
to be held, but instead it isn't.
Instead of adding the lock to qemu_savevm_state_setup(),
follow the same pattern as the other ->save_setup callbacks
and lock+unlock inside dirty_bitmap_save_setup().
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
migration/block-dirty-bitmap.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 35f5ef688d..9aba7d9c22 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -1215,7 +1215,10 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
{
DBMSaveState *s = &((DBMState *)opaque)->save;
SaveBitmapState *dbms = NULL;
+
+ qemu_mutex_lock_iothread();
if (init_dirty_bitmap_migration(s) < 0) {
+ qemu_mutex_unlock_iothread();
return -1;
}
@@ -1223,7 +1226,7 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
send_bitmap_start(f, s, dbms);
}
qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
-
+ qemu_mutex_unlock_iothread();
return 0;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v1 2/2] migration: add missing qemu_mutex_lock_iothread in migration_completion
2021-10-05 8:07 [PATCH v1 0/2] Migration: fix missing iothread locking Emanuele Giuseppe Esposito
2021-10-05 8:07 ` [PATCH v1 1/2] migration: block-dirty-bitmap: add missing qemu_mutex_lock_iothread Emanuele Giuseppe Esposito
@ 2021-10-05 8:07 ` Emanuele Giuseppe Esposito
2021-10-05 10:12 ` Dr. David Alan Gilbert
1 sibling, 1 reply; 6+ messages in thread
From: Emanuele Giuseppe Esposito @ 2021-10-05 8:07 UTC (permalink / raw)
To: qemu-block
Cc: Fam Zheng, Emanuele Giuseppe Esposito,
Vladimir Sementsov-Ogievskiy, Juan Quintela, Eric Blake,
Dr. David Alan Gilbert, qemu-devel, Stefan Hajnoczi,
Paolo Bonzini, John Snow
qemu_savevm_state_complete_postcopy assumes the iothread lock (BQL)
to be held, but instead it isn't.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
migration/migration.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/migration/migration.c b/migration/migration.c
index 041b8451a6..215d5281f2 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3182,7 +3182,10 @@ static void migration_completion(MigrationState *s)
} else if (s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) {
trace_migration_completion_postcopy_end();
+ qemu_mutex_lock_iothread();
qemu_savevm_state_complete_postcopy(s->to_dst_file);
+ qemu_mutex_unlock_iothread();
+
trace_migration_completion_postcopy_end_after_complete();
} else if (s->state == MIGRATION_STATUS_CANCELLING) {
goto fail;
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] migration: add missing qemu_mutex_lock_iothread in migration_completion
2021-10-05 8:07 ` [PATCH v1 2/2] migration: add missing qemu_mutex_lock_iothread in migration_completion Emanuele Giuseppe Esposito
@ 2021-10-05 10:12 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 6+ messages in thread
From: Dr. David Alan Gilbert @ 2021-10-05 10:12 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito, peterx
Cc: Fam Zheng, Vladimir Sementsov-Ogievskiy, qemu-block,
Juan Quintela, Eric Blake, qemu-devel, Stefan Hajnoczi,
Paolo Bonzini, John Snow
* Emanuele Giuseppe Esposito (eesposit@redhat.com) wrote:
> qemu_savevm_state_complete_postcopy assumes the iothread lock (BQL)
> to be held, but instead it isn't.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Interesting, I think you're right - and I think it's been missing it
from the start.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/migration.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 041b8451a6..215d5281f2 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3182,7 +3182,10 @@ static void migration_completion(MigrationState *s)
> } else if (s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) {
> trace_migration_completion_postcopy_end();
>
> + qemu_mutex_lock_iothread();
> qemu_savevm_state_complete_postcopy(s->to_dst_file);
> + qemu_mutex_unlock_iothread();
> +
> trace_migration_completion_postcopy_end_after_complete();
> } else if (s->state == MIGRATION_STATUS_CANCELLING) {
> goto fail;
> --
> 2.27.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] migration: block-dirty-bitmap: add missing qemu_mutex_lock_iothread
2021-10-05 8:07 ` [PATCH v1 1/2] migration: block-dirty-bitmap: add missing qemu_mutex_lock_iothread Emanuele Giuseppe Esposito
@ 2021-11-02 9:11 ` Juan Quintela
2021-11-02 9:15 ` Juan Quintela
0 siblings, 1 reply; 6+ messages in thread
From: Juan Quintela @ 2021-11-02 9:11 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito
Cc: Fam Zheng, Vladimir Sementsov-Ogievskiy, qemu-block, Eric Blake,
Dr. David Alan Gilbert, qemu-devel, Stefan Hajnoczi,
Paolo Bonzini, John Snow
Emanuele Giuseppe Esposito <eesposit@redhat.com> wrote:
> init_dirty_bitmap_migration assumes the iothread lock (BQL)
> to be held, but instead it isn't.
>
> Instead of adding the lock to qemu_savevm_state_setup(),
> follow the same pattern as the other ->save_setup callbacks
> and lock+unlock inside dirty_bitmap_save_setup().
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
queued.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] migration: block-dirty-bitmap: add missing qemu_mutex_lock_iothread
2021-11-02 9:11 ` Juan Quintela
@ 2021-11-02 9:15 ` Juan Quintela
0 siblings, 0 replies; 6+ messages in thread
From: Juan Quintela @ 2021-11-02 9:15 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito
Cc: Fam Zheng, Vladimir Sementsov-Ogievskiy, qemu-block, Eric Blake,
Dr. David Alan Gilbert, qemu-devel, Stefan Hajnoczi,
Paolo Bonzini, John Snow
Juan Quintela <quintela@redhat.com> wrote:
> Emanuele Giuseppe Esposito <eesposit@redhat.com> wrote:
>> init_dirty_bitmap_migration assumes the iothread lock (BQL)
>> to be held, but instead it isn't.
>>
>> Instead of adding the lock to qemu_savevm_state_setup(),
>> follow the same pattern as the other ->save_setup callbacks
>> and lock+unlock inside dirty_bitmap_save_setup().
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
And I realized that this patch is already on the tree, just not through
the migration tree.
Ignore the noise.
Later, Juan.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-02 9:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-05 8:07 [PATCH v1 0/2] Migration: fix missing iothread locking Emanuele Giuseppe Esposito
2021-10-05 8:07 ` [PATCH v1 1/2] migration: block-dirty-bitmap: add missing qemu_mutex_lock_iothread Emanuele Giuseppe Esposito
2021-11-02 9:11 ` Juan Quintela
2021-11-02 9:15 ` Juan Quintela
2021-10-05 8:07 ` [PATCH v1 2/2] migration: add missing qemu_mutex_lock_iothread in migration_completion Emanuele Giuseppe Esposito
2021-10-05 10:12 ` Dr. David Alan Gilbert
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.