Re: [Qemu-devel] [PATCH v7 26/42] postcopy: Incoming initialisation

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <address@hidden> wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
>
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> Reviewed-by: David Gibson <address@hidden>
> ---
>  include/migration/migration.h    |   3 +
>  include/migration/postcopy-ram.h |  12 ++++
>  migration/postcopy-ram.c         | 116 
> +++++++++++++++++++++++++++++++++++++++
>  migration/ram.c                  |  11 ++++
>  migration/savevm.c               |   4 ++
>  trace-events                     |   2 +
>  6 files changed, 148 insertions(+)
>

qemu_hugepage_enable(host_addr, length)?

> +#ifdef MADV_NOHUGEPAGE
> +    if (madvise(host_addr, length, MADV_NOHUGEPAGE)) {
> +        error_report("%s: NOHUGEPAGE: %s", __func__, strerror(errno));
> +        return -1;
> +    }
> +#endif

qemu_hugepage_disable(host_addr, length)?
> +#ifdef MADV_HUGEPAGE
> +    if (madvise(host_addr, length, MADV_HUGEPAGE)) {
> +        error_report("%s HUGEPAGE: %s", __func__, strerror(errno));
> +        return -1;
> +    }
> +#endif
> +
> +    /*
> +     * We can also turn off userfault now since we should have all the
> +     * pages.   It can be useful to leave it on to debug postcopy
> +     * if you're not sure it's always getting every page.
> +     */

qemu_userfault_unregister(host_addr, length)?

> +    range_struct.start = (uintptr_t)host_addr;
> +    range_struct.len = length;
> +
> +    if (ioctl(mis->userfault_fd, UFFDIO_UNREGISTER, &range_struct)) {
> +        error_report("%s: userfault unregister %s", __func__, 
> strerror(errno));
> +
> +        return -1;
> +    }

>  
> +/*
> + * Allocate data structures etc needed by incoming migration with 
> postcopy-ram
> + * postcopy-ram's similarly names postcopy_ram_incoming_init does the work
> + */
> +int ram_postcopy_incoming_init(MigrationIncomingState *mis)
> +{
> +    size_t ram_pages = last_ram_offset() >> TARGET_PAGE_BITS;
> +
> +    return postcopy_ram_incoming_init(mis, ram_pages);
> +}
> +

ram_postocpy_incoming_init()
and
postcopy_ram_incoming_init()

ouch  Thinking about better names ....



>  static int ram_load(QEMUFile *f, void *opaque, int version_id)
>  {
>      int flags = 0, ret = 0;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index e6398dd..f4de52d 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1238,6 +1238,10 @@ static int 
> loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
>          return -1;
>      }
>  
> +    if (ram_postcopy_incoming_init(mis)) {
> +        return -1;
> +    }
> +

how/where we know that this is called soon enough?

>      postcopy_state_set(mis, POSTCOPY_INCOMING_ADVISE);
>  
>      return 0;
> diff --git a/trace-events b/trace-events
> index 5e8a120..2ffc1c6 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1498,7 +1498,9 @@ 
> rdma_start_outgoing_migration_after_rdma_source_init(void) ""
>  
>  # migration/postcopy-ram.c
>  postcopy_discard_send_finish(const char *ramblock, int nwords, int ncmds) 
> "%s mask words sent=%d in %d commands"
> +postcopy_cleanup_area(const char *ramblock, void *host_addr, size_t offset, 
> size_t length) "%s: %p offset=%zx length=%zx"
>  postcopy_ram_discard_range(void *start, void *end) "%p,%p"
> +postcopy_init_area(const char *ramblock, void *host_addr, size_t offset, 
> size_t length) "%s: %p offset=%zx length=%zx"

once here, if we have range names before, what about:

postcopy_ram_cleanup_range()
postcopy_ram_init_range()

And let the ram* functions the same?