On Fri, Oct 03, 2014 at 06:47:46PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
>
> In postcopy, the destination guest is running at the same time
> as it's receiving pages; as we receive new pages we must put
> them into the guests address space atomically to avoid a running
> CPU accessing a partially written page.
>
> Use the helpers in postcopy-ram.c to map these pages.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> ---
> arch_init.c | 96
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 87 insertions(+), 9 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 2f4345a..0ba627b 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -1458,9 +1458,20 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr,
> void *host)
> return 0;
> }
>
> +/*
> + * Read a RAMBlock ID from the stream f, find the host address of the
> + * start of that block and add on 'offset'
> + *
> + * f: Stream to read from
> + * mis: MigrationIncomingState
> + * offset: Offset within the block
> + * flags: Page flags (mostly to see if it's a continuation of previous block)
> + * rb: Pointer to RAMBlock* that gets filled in with the RB we find
> + */
> static inline void *host_from_stream_offset(QEMUFile *f,
> + MigrationIncomingState *mis,
> ram_addr_t offset,
> - int flags)
> + int flags, RAMBlock **rb)
> {
> static RAMBlock *block = NULL;
> char id[256];
> @@ -1471,8 +1482,11 @@ static inline void *host_from_stream_offset(QEMUFile
> *f,
> error_report("Ack, bad migration stream!");
> return NULL;
> }
> + if (rb) {
> + *rb = block;
> + }
>
> - return memory_region_get_ram_ptr(block->mr) + offset;
> + goto gotit;
This is an ugly use of goto - it looks kind of like the exception
handling goto idiom, but it's not. I think it would be nicer to make
the code fragment after gotit into a helper function.
> }
>
> len = qemu_get_byte(f);
> @@ -1480,12 +1494,22 @@ static inline void *host_from_stream_offset(QEMUFile
> *f,
> id[len] = 0;
>
> QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> - if (!strncmp(id, block->idstr, sizeof(id)))
> - return memory_region_get_ram_ptr(block->mr) + offset;
> + if (!strncmp(id, block->idstr, sizeof(id))) {
> + if (rb) {
> + *rb = block;
> + }
> + goto gotit;
> + }
> }
>
> error_report("Can't find block %s!", id);
> return NULL;
> +
> +gotit:
> + postcopy_hook_early_receive(mis,
> + (offset + (*rb)->offset) >> TARGET_PAGE_BITS);
> + return memory_region_get_ram_ptr(block->mr) + offset;
> +
> }
>
> /*
> @@ -1515,6 +1539,13 @@ static int ram_load(QEMUFile *f, void *opaque, int
> version_id)
> ram_addr_t addr;
> int flags, ret = 0;
> static uint64_t seq_iter;
> + /*
> + * System is running in postcopy mode, page inserts to host memory must
> be
> + * atomic
> + */
> + MigrationIncomingState *mis = migration_incoming_get_current();
> + bool postcopy_running = mis->postcopy_ram_state >=
> + POSTCOPY_RAM_INCOMING_LISTENING;
>
> seq_iter++;
>
> @@ -1523,6 +1554,7 @@ static int ram_load(QEMUFile *f, void *opaque, int
> version_id)
> }
>
> while (!ret) {
> + RAMBlock *rb = 0; /* =0 needed to silence compiler */
> addr = qemu_get_be64(f);
>
> flags = addr & ~TARGET_PAGE_MASK;
> @@ -1570,7 +1602,7 @@ static int ram_load(QEMUFile *f, void *opaque, int
> version_id)
> void *host;
> uint8_t ch;
>
> - host = host_from_stream_offset(f, addr, flags);
> + host = host_from_stream_offset(f, mis, addr, flags, &rb);
> if (!host) {
> error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
> ret = -EINVAL;
> @@ -1578,20 +1610,66 @@ static int ram_load(QEMUFile *f, void *opaque, int
> version_id)
> }
>
> ch = qemu_get_byte(f);
> - ram_handle_compressed(host, ch, TARGET_PAGE_SIZE);
> + if (!postcopy_running) {
> + ram_handle_compressed(host, ch, TARGET_PAGE_SIZE);
> + } else {
> + if (!ch) {
> + ret = postcopy_place_zero_page(mis, host,
> + (addr + rb->offset) >> TARGET_PAGE_BITS);
> + } else {
> + void *tmp;
> + tmp = postcopy_get_tmp_page(mis, (addr + rb->offset) >>
> + TARGET_PAGE_BITS);
> +
> + if (!tmp) {
> + return -ENOMEM;
> + }
> + memset(tmp, ch, TARGET_PAGE_SIZE);
> + ret = postcopy_place_page(mis, host, tmp,
> + (addr + rb->offset) >> TARGET_PAGE_BITS);
> + }
> + if (ret) {
> + error_report("ram_load: Failure in postcopy compress @"
> + "%zx/%p;%s+%zx",
> + addr, host, rb->idstr, rb->offset);
> + return ret;
> + }
> + }
Might be nicer to fold this logic into ram_handle_compressed(), since
there's no obvious reason it should not be used for the postcopy path.
> } else if (flags & RAM_SAVE_FLAG_PAGE) {
> void *host;
>
> - host = host_from_stream_offset(f, addr, flags);
> + host = host_from_stream_offset(f, mis, addr, flags, &rb);
> if (!host) {
> error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
> ret = -EINVAL;
> break;
> }
>
> - qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
> + if (!postcopy_running) {
> + qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
> + } else {
> + void *tmp = postcopy_get_tmp_page(mis, (addr + rb->offset) >>
> + TARGET_PAGE_BITS);
> +
> + if (!tmp) {
> + return -ENOMEM;
> + }
> + qemu_get_buffer(f, tmp, TARGET_PAGE_SIZE);
> + ret = postcopy_place_page(mis, host, tmp,
> + (addr + rb->offset) >> TARGET_PAGE_BITS);
> + if (ret) {
> + error_report("ram_load: Failure in postcopy simple"
> + "@%zx/%p;%s+%zx",
> + addr, host, rb->idstr, rb->offset);
> + return ret;
> + }
> + }
> } else if (flags & RAM_SAVE_FLAG_XBZRLE) {
> - void *host = host_from_stream_offset(f, addr, flags);
> + if (postcopy_running) {
> + error_report("XBZRLE RAM block in postcopy mode @%zx\n",
> addr);
> + return -EINVAL;
> + }
Hrm, there doesn't seem like an inherent reason XBZRLE shouldn't be
possible in postcopy. Obviously a temporary buffer would be
necessary.
> + void *host = host_from_stream_offset(f, mis, addr, flags, &rb);
> if (!host) {
> error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
> ret = -EINVAL;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
pgp7vTB9BjStv.pgp
Description: PGP signature
