Re: [Qemu-devel] [PATCH v9 00/56] Postcopy implementation

[Dr. David Alan Gilbert]

* Juan Quintela (address@hidden) wrote:
> "Dr. David Alan Gilbert (git)" <address@hidden> wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> >   This is the 9th cut of my version of postcopy.
> >
> > The userfaultfd linux kernel code is now in the upstream kernel
> > tree, and so 4.3 can be used without modification.
> >
> > This qemu series can be found at:
> > https://github.com/orbitfp7/qemu.git
> > on the wp3-postcopy-v9 tag
> >
> > Testing status:
> >   * Tested heavily on x86
> >   * Smoke tested on aarch64 (so it does work on different page sizes)
> >
> > This work has been partially funded by the EU Orbit project:
> >   see http://www.orbitproject.eu/about/
> >
> > v9
> >   Almost all of the changes are changes from review comments, and most are
> >   minor.
> >
> >   The following are new patches, mostly split out from earlier patches
> >   (The exception being adding userfaultfd.h header back in - which was in v5
> >    but we took out, but is needed again due to 1842bdfd)
> >
> >     04/56 Move page_size_init earlier
> >     08/56 qemu_ram_block_by_name
> >     12/56 Factor out host_from_stream_offset call and check
> >     15/56 Add Linux userfaultfd.h header back
> >     21/56 migration_is_setup_or_active
> >     30/56 migration_completion: Take current state
> >     34/56 Maintain unsentmap
> >
> >   The previous patches 03,10,13,36/54 went in upstream already.
> >
> >   Fix for assert using hotplug (Thanks Bharata for spotting that)
> >   Fix for migrate_cancel a second time
> >   Rework for migration_bitmap_rcu after Denis's deadlock fix
> >   Rework ram_load into a separate postcopy loop
> >   The 'sentmap' is now an 'unsentmap' - this saves a complement step at the 
> > end
> >      The unsentmap creation is now split into a separate patch
> >      The unsentmap is now stored in the RCU structure (although it can't
> >        really resize during the migrate)
> >   Fix for block migration
> >      still not a suggested combination.
> >   Minor:
> >      trace_savevm_send_open_return_path added
> >      split migration_is_active function into separate patch and made
> >         migration_is_setup_or_active
> >      move file reads into migrate_handle_advise and migrate_handle_packaged
> >      use of MIN in send-packaged-chunk
> >      migration_thread_started -> migration_thread_running
> >      updated qemu_get_buffer_in_place with Juan's version (and size_t'ified 
> > it)
> >      split the host_from_stream_offset change into a separate patch
> >        'ram_load: Factor out host_from_stream_offset call and check'
> >      split out ram_find_block_by_id into a separate patch and now
> >        called qemu_ram_block_by_name
> >      postcopy_discard_send_range etc now take start/length rather than 
> > start/end
> >        (also added another trace)
> >      split ram_save_host_page into ram_save_host_page/ram_save_target_page
> >      split host page cleanup function into a core that handles both passes
> >
> > Dave
> 
> I need the following patch to make things compile on 32bits, windows,
> etc.
> 
> I guess you are ok with that (I have integrated it in the appropiate places)

Thanks

Reviewed-by: Dr. David Alan Gilbert <address@hidden>

> Later, Juan.
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index fd51d79..58eb099 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1239,7 +1239,7 @@ static void migrate_handle_rp_req_pages(MigrationState 
> *ms, const char* rbname,
>      if (start & (our_host_ps-1) ||
>         (len & (our_host_ps-1))) {
>          error_report("%s: Misaligned page request, start: " RAM_ADDR_FMT
> -                     " len: " RAM_ADDR_FMT, __func__, start, len);
> +                     " len: %zd", __func__, start, len);
>          mark_source_rp_bad(ms);
>          return;
>      }

I don't quite get why that doesn't fail in my 32bit test build; but I checked 
again
and my build gives no warnings for it.

> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 23ceb75..22d6b18 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -641,37 +641,44 @@ int postcopy_ram_incoming_init(MigrationIncomingState 
> *mis, size_t ram_pages)
>  int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
>  {
>      assert(0);
> +    return -1;
>  }

Sigh, yes, Windows assert/abort's can return.

>  int postcopy_ram_discard_range(MigrationIncomingState *mis, uint8_t *start,
>                                 size_t length)
>  {
>      assert(0);
> +    return -1;
>  }
> 
>  int postcopy_ram_prepare_discard(MigrationIncomingState *mis)
>  {
>      assert(0);
> +    return -1;
>  }
> 
>  int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>  {
>      assert(0);
> +    return -1;
>  }
> 
>  int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from)
>  {
>      assert(0);
> +    return -1;
>  }
> 
>  int postcopy_place_page_zero(MigrationIncomingState *mis, void *host)
>  {
>      assert(0);
> +    return -1;
>  }
> 
>  void *postcopy_get_tmp_page(MigrationIncomingState *mis)
>  {
>      assert(0);
> +    return NULL;
>  }
> 
>  #endif
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK