[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] migration: move bdrv_invalidate_cache_all o
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] migration: move bdrv_invalidate_cache_all of of coroutine context |
Date: |
Fri, 12 Feb 2016 12:50:36 +0000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
* Denis V. Lunev (address@hidden) wrote:
> There is a possibility to hit an assert in qcow2_get_specific_info that
> s->qcow_version is undefined. This happens when VM in starting from
> suspended state, i.e. it processes incoming migration, and in the same
> time 'info block' is called.
>
> The problem is that qcow2_invalidate_cache() closes the image and
> memset()s BDRVQcowState in the middle.
>
> The patch moves processing of bdrv_invalidate_cache_all out of
> coroutine context for postcopy migration to avoid that. This function
> is called with the following stack:
> process_incoming_migration_co
> qemu_loadvm_state
> qemu_loadvm_state_main
> loadvm_process_command
> loadvm_postcopy_handle_run
>
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Paolo Bonzini <address@hidden>
> CC: Juan Quintela <address@hidden>
> CC: Amit Shah <address@hidden>
I've tested my normal postcopy smoke test and it seems to work,
so
Tested-by: Dr. David Alan Gilbert <address@hidden>
however, can you add a comment before loadvm_postcopy_handle_run_bh
to explain why it's a bh, otherwise at some point someone might
put it back.
I'll admit to not really understanding what the difference is
between bh and coroutine context; I'd thought if it was all
in the main thread stuff was safe.
Dave
> ---
> migration/savevm.c | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 94f2894..8415fd9 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1496,18 +1496,10 @@ static int
> loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> return 0;
> }
>
> -/* After all discards we can start running and asking for pages */
> -static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
> +static void loadvm_postcopy_handle_run_bh(void *opaque)
> {
> - PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_RUNNING);
> Error *local_err = NULL;
>
> - trace_loadvm_postcopy_handle_run();
> - if (ps != POSTCOPY_INCOMING_LISTENING) {
> - error_report("CMD_POSTCOPY_RUN in wrong postcopy state (%d)", ps);
> - return -1;
> - }
> -
> /* TODO we should move all of this lot into postcopy_ram.c or a shared
> code
> * in migration.c
> */
> @@ -1519,7 +1511,6 @@ static int
> loadvm_postcopy_handle_run(MigrationIncomingState *mis)
> bdrv_invalidate_cache_all(&local_err);
> if (local_err) {
> error_report_err(local_err);
> - return -1;
> }
>
> trace_loadvm_postcopy_handle_run_cpu_sync();
> @@ -1534,6 +1525,22 @@ static int
> loadvm_postcopy_handle_run(MigrationIncomingState *mis)
> /* leave it paused and let management decide when to start the CPU */
> runstate_set(RUN_STATE_PAUSED);
> }
> +}
> +
> +/* After all discards we can start running and asking for pages */
> +static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
> +{
> + PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_RUNNING);
> + QEMUBH *bh;
> +
> + trace_loadvm_postcopy_handle_run();
> + if (ps != POSTCOPY_INCOMING_LISTENING) {
> + error_report("CMD_POSTCOPY_RUN in wrong postcopy state (%d)", ps);
> + return -1;
> + }
> +
> + bh = qemu_bh_new(loadvm_postcopy_handle_run_bh, NULL);
> + qemu_bh_schedule(bh);
>
> /* We need to finish reading the stream from the package
> * and also stop reading anything more from the stream that loaded the
> --
> 2.1.4
>
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH v2 0/2] move qcow2_invalidate_cache() out of coroutine context, Denis V. Lunev, 2016/02/12
- [Qemu-devel] [PATCH 1/2] migration: move bdrv_invalidate_cache_all of of coroutine context, Denis V. Lunev, 2016/02/12
- Re: [Qemu-devel] [PATCH v2 0/2] move qcow2_invalidate_cache() out of coroutine context, Denis V. Lunev, 2016/02/16
- Re: [Qemu-devel] [PATCH v2 0/2] move qcow2_invalidate_cache() out of coroutine context, Amit Shah, 2016/02/18
- Re: [Qemu-devel] [PATCH v2 0/2] move qcow2_invalidate_cache() out of coroutine context, Denis V. Lunev, 2016/02/22