[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3] migration: incoming postcopy advise sanity c
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v3] migration: incoming postcopy advise sanity checks |
Date: |
Tue, 6 Feb 2018 11:27:52 +0000 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
* Greg Kurz (address@hidden) wrote:
> If postcopy-ram was set on the source but not on the destination,
> migration doesn't occur, the destination prints an error and boots
> the guest:
>
> qemu-system-ppc64: Expected vmdescription section, but got 0
>
> We end up with two running instances.
>
> This behaviour was introduced in 2.11 by commit 58110f0acb1a "migration:
> split common postcopy out of ram postcopy" to prepare ground for the
> upcoming dirty bitmap postcopy support. It adds a new case where the
> source may send an empty postcopy advise because dirty bitmap doesn't
> need to check page sizes like RAM postcopy does.
>
> If the source has enabled postcopy-ram, then it sends an advise with
> the page size values. If the destination hasn't enabled postcopy-ram,
> then loadvm_postcopy_handle_advise() leaves the page size values on
> the stream and returns. This confuses qemu_loadvm_state() later on
> and causes the destination to start execution.
>
> As discussed several times, postcopy-ram should be enabled both sides
> to be functional. This patch changes the destination to perform some
> extra checks on the advise length to ensure this is the case. Otherwise
> an error is returned and migration is aborted.
>
> Reported-by: Balamuruhan S <address@hidden>
> Signed-off-by: Greg Kurz <address@hidden>
> Reviewed-by: Daniel Henrique Barboza <address@hidden>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
and queued to replace v2.
Dave
> ---
> v3: - s/warn_report/error_report
>
> v2: - error out if postcopy-ram is enabled but the source hasn't sent the
> expected 16 byte advise
> - more descriptive message if postcopy-ram is disabled but the source
> has sent a 16 byte advise
> ---
> migration/savevm.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index b7908f62be3c..c74b18dcf317 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1376,7 +1376,8 @@ static int qemu_loadvm_state_main(QEMUFile *f,
> MigrationIncomingState *mis);
> * *might* happen - it might be skipped if precopy transferred everything
> * quickly.
> */
> -static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
> +static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
> + uint16_t len)
> {
> PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_ADVISE);
> uint64_t remote_pagesize_summary, local_pagesize_summary, remote_tps;
> @@ -1387,8 +1388,22 @@ static int
> loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
> return -1;
> }
>
> - if (!migrate_postcopy_ram()) {
> + switch (len) {
> + case 0:
> + if (migrate_postcopy_ram()) {
> + error_report("RAM postcopy is enabled but have 0 byte advise");
> + return -EINVAL;
> + }
> return 0;
> + case 8 + 8:
> + if (!migrate_postcopy_ram()) {
> + error_report("RAM postcopy is disabled but have 16 byte advise");
> + return -EINVAL;
> + }
> + break;
> + default:
> + error_report("CMD_POSTCOPY_ADVISE invalid length (%d)", len);
> + return -EINVAL;
> }
>
> if (!postcopy_ram_supported_by_host(mis)) {
> @@ -1807,7 +1822,7 @@ static int loadvm_process_command(QEMUFile *f)
> return loadvm_handle_cmd_packaged(mis);
>
> case MIG_CMD_POSTCOPY_ADVISE:
> - return loadvm_postcopy_handle_advise(mis);
> + return loadvm_postcopy_handle_advise(mis, len);
>
> case MIG_CMD_POSTCOPY_LISTEN:
> return loadvm_postcopy_handle_listen(mis);
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK