| From 149b5fe290b6d8f52b87993d3d27fe5fca806173 Mon Sep 17 00:00:00 2001 |
| From: Kevin Wolf <kwolf@redhat.com> |
| Date: Fri, 19 May 2017 09:38:59 +0200 |
| Subject: [PATCH 24/27] migration: Unify block node activation error handling |
| |
| RH-Author: Kevin Wolf <kwolf@redhat.com> |
| Message-id: <1495186739-13659-3-git-send-email-kwolf@redhat.com> |
| Patchwork-id: 75377 |
| O-Subject: [RHEL-7.4 qemu-kvm-rhev PATCH v2 2/2] migration: Unify block node activation error handling |
| Bugzilla: 1452148 |
| RH-Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com> |
| RH-Acked-by: Laurent Vivier <lvivier@redhat.com> |
| RH-Acked-by: Max Reitz <mreitz@redhat.com> |
| |
| Migration code activates all block driver nodes on the destination when |
| the migration completes. It does so by calling |
| bdrv_invalidate_cache_all() and blk_resume_after_migration(). There is |
| one code path for precopy and one for postcopy migration, resulting in |
| four function calls, which used to have three different failure modes. |
| |
| This patch unifies the behaviour so that failure to activate all block |
| nodes is non-fatal, but the error message is logged and the VM isn't |
| automatically started. 'cont' will retry activating the block nodes. |
| |
| Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
| Reviewed-by: Eric Blake <eblake@redhat.com> |
| (cherry picked from commit ace21a58751824f9a3d399e332317233e880de3a) |
| Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> |
| |
| migration/migration.c | 16 +++++----------- |
| migration/savevm.c | 12 +++++------- |
| qmp.c | 18 +++++++++--------- |
| 3 files changed, 19 insertions(+), 27 deletions(-) |
| |
| diff --git a/migration/migration.c b/migration/migration.c |
| index fdc17bc..7a2ee01 100644 |
| |
| |
| @@ -340,20 +340,14 @@ static void process_incoming_migration_bh(void *opaque) |
| Error *local_err = NULL; |
| MigrationIncomingState *mis = opaque; |
| |
| - /* Make sure all file formats flush their mutable metadata */ |
| + /* Make sure all file formats flush their mutable metadata. |
| + * If we get an error here, just don't restart the VM yet. */ |
| bdrv_invalidate_cache_all(&local_err); |
| - if (local_err) { |
| - migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, |
| - MIGRATION_STATUS_FAILED); |
| - error_report_err(local_err); |
| - migrate_decompress_threads_join(); |
| - exit(EXIT_FAILURE); |
| + if (!local_err) { |
| + blk_resume_after_migration(&local_err); |
| } |
| - |
| - /* If we get an error here, just don't restart the VM yet. */ |
| - blk_resume_after_migration(&local_err); |
| if (local_err) { |
| - error_free(local_err); |
| + error_report_err(local_err); |
| local_err = NULL; |
| autostart = false; |
| } |
| diff --git a/migration/savevm.c b/migration/savevm.c |
| index 21d7985..6e2f9e7 100644 |
| |
| |
| @@ -1620,16 +1620,14 @@ static void loadvm_postcopy_handle_run_bh(void *opaque) |
| |
| qemu_announce_self(); |
| |
| - /* Make sure all file formats flush their mutable metadata */ |
| + /* Make sure all file formats flush their mutable metadata. |
| + * If we get an error here, just don't restart the VM yet. */ |
| bdrv_invalidate_cache_all(&local_err); |
| - if (local_err) { |
| - error_report_err(local_err); |
| + if (!local_err) { |
| + blk_resume_after_migration(&local_err); |
| } |
| - |
| - /* If we get an error here, just don't restart the VM yet. */ |
| - blk_resume_after_migration(&local_err); |
| if (local_err) { |
| - error_free(local_err); |
| + error_report_err(local_err); |
| local_err = NULL; |
| autostart = false; |
| } |
| diff --git a/qmp.c b/qmp.c |
| index a744e44..847b13f 100644 |
| |
| |
| @@ -196,15 +196,15 @@ void qmp_cont(Error **errp) |
| } |
| |
| /* Continuing after completed migration. Images have been inactivated to |
| - * allow the destination to take control. Need to get control back now. */ |
| - if (runstate_check(RUN_STATE_FINISH_MIGRATE) || |
| - runstate_check(RUN_STATE_POSTMIGRATE)) |
| - { |
| - bdrv_invalidate_cache_all(&local_err); |
| - if (local_err) { |
| - error_propagate(errp, local_err); |
| - return; |
| - } |
| + * allow the destination to take control. Need to get control back now. |
| + * |
| + * If there are no inactive block nodes (e.g. because the VM was just |
| + * paused rather than completing a migration), bdrv_inactivate_all() simply |
| + * doesn't do anything. */ |
| + bdrv_invalidate_cache_all(&local_err); |
| + if (local_err) { |
| + error_propagate(errp, local_err); |
| + return; |
| } |
| |
| blk_resume_after_migration(&local_err); |
| -- |
| 1.8.3.1 |
| |