Re: [Qemu-devel] [PATCH 08/18] migration-local: introduce qemu_fopen_local()

["Michael R. Hines" <mrhines@linux.vnet.ibm.com>]

On 08/21/2013 03:18 AM, Lei Li wrote:
> Introduce read/write backend of QEMUFileLocal used by localhost
> migration. The unix domain socket will be replaced by PIPE with
> vmsplice mechanism.
>
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>   Makefile.objs     |    1 +
>   migration-local.c |  211 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 212 insertions(+), 0 deletions(-)
>   create mode 100644 migration-local.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index f46a4cd..30670cc 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -54,6 +54,7 @@ common-obj-y += migration.o migration-tcp.o
>   common-obj-$(CONFIG_RDMA) += migration-rdma.o
>   common-obj-y += qemu-char.o #aio.o
>   common-obj-y += block-migration.o
> +common-obj-y += migration-local.o
>   common-obj-y += page_cache.o xbzrle.o
>
>   common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
> diff --git a/migration-local.c b/migration-local.c
> new file mode 100644
> index 0000000..93190fd
> --- /dev/null
> +++ b/migration-local.c
> @@ -0,0 +1,211 @@
> +/*
> + * QEMU localhost migration
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.
> + *
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "config-host.h"
> +#include "qemu-common.h"
> +#include "migration/migration.h"
> +#include "exec/cpu-common.h"
> +#include "config.h"
> +#include "exec/cpu-all.h"
> +#include "monitor/monitor.h"
> +#include "migration/qemu-file.h"
> +#include "qemu/iov.h"
> +#include "sysemu/arch_init.h"
> +#include "sysemu/sysemu.h"
> +#include "block/block.h"
> +#include "qemu/sockets.h"
> +#include "migration/block.h"
> +#include "qemu/thread.h"
> +#include "qmp-commands.h"
> +#include "trace.h"
> +#include "qemu/osdep.h"
> +
> +//#define DEBUG_MIGRATION_LOCAL
> +
> +#ifdef DEBUG_MIGRATION_LOCAL
> +#define DPRINTF(fmt, ...) \
> +    do { printf("migration-local: " fmt, ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF(fmt, ...) \
> +    do { } while (0)
> +#endif
> +
> +/*
> + * Interface for the local migration.
> + */
> +typedef struct QEMUFileLocal {
> +    QEMUFile *file;
> +    int fd;
> +    int state;
> +
> +    /*
> +     * This is the last block from where we have sent data
> +     * for local migration
> +     */
> +    RAMBlock *last_block_sent;
> +} QEMUFileLocal;
> +
> +
> +static int qemu_local_get_buffer(void *opaque, uint8_t *buf,
> +                                 int64_t pos, int size)
> +{
> +    QEMUFileLocal *s = opaque;
> +    ssize_t len;
> +
> +    for (;;) {
> +        len = qemu_recv(s->fd, buf, size, 0);
> +        if (len != -1) {
> +            break;
> +        }
> +        if (socket_error() == EAGAIN) {
> +            yield_until_fd_readable(s->fd);
> +        } else if (socket_error() != EINTR) {
> +            break;
> +        }
> +    }
> +
> +    if (len == -1) {
> +        len = -socket_error();
> +    }
> +    return len;
> +}
> +

This looks like a line-for-line copy of socket_get_buffer()......

Since you're just going to end up replacing this with vmsplice(),
could you just call socket_get_buffer() temporarily until
your next patch is ready?

> +static int qemu_local_get_fd(void *opaque)
> +{
> +    QEMUFileLocal *s = opaque;
> +
> +    return s->fd;
> +}
> +
> +static int qemu_local_close(void *opaque)
> +{
> +    QEMUFileLocal *s = opaque;
> +
> +    closesocket(s->fd);
> +    g_free(s);
> +
> +    return 0;
> +}
> +
> +static size_t qemu_local_put_buffer(void *opaque, struct iovec *iov,
> +                                    int iovcnt, int64_t pos)
> +{
> +    QEMUFileLocal *s = opaque;
> +    ssize_t len;
> +    ssize_t size = iov_size(iov, iovcnt);
> +
> +    len = iov_send(s->fd, iov, iovcnt, 0, size);
> +    if (len < size) {
> +        len = -socket_error();
> +    }
> +
> +    return len;
> +}
> +
> +static size_t local_save_page(QEMUFile *f, RAMBlock *block,
> +                              ram_addr_t offset, int flags)
> +{
> +    MemoryRegion *mr = block->mr;
> +    uint8_t *p;
> +
> +    p = memory_region_get_ram_ptr(mr) + offset;
> +
> +    if (buffer_find_nonzero_offset(p, TARGET_PAGE_SIZE)) {
> +        qemu_put_be64(f, offset | flags | RAM_SAVE_FLAG_COMPRESS);
> +        if (!flags) {
> +            qemu_put_byte(f, strlen(block->idstr));
> +            qemu_put_buffer(f, (uint8_t *)block->idstr,
> +                            strlen(block->idstr));
> +        }
> +        qemu_put_byte(f, *p);
> +        return 0;
> +    }
> +
> +    qemu_put_be64(f, offset | flags | RAM_SAVE_FLAG_PAGE);
> +    if (!flags) {
> +        qemu_put_byte(f, strlen(block->idstr));
> +        qemu_put_buffer(f, (uint8_t *)block->idstr,
> +                        strlen(block->idstr));
> +    }
> +    qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
> +
> +    return TARGET_PAGE_SIZE;
> +}
> +
> +static size_t qemu_local_ram_save(QEMUFile *f, void *opaque,
> +                                  ram_addr_t block_offset, ram_addr_t offset,
> +                                  size_t size, int *bytes_sent)
> +{
> +    QEMUFileLocal *s = opaque;
> +    uint64_t current_addr = block_offset + offset;
> +    RAMBlock *block = qemu_get_ram_block(current_addr);
> +    MemoryRegion *mr = block->mr;

RAMBlock structs are not visible outside of exec.c and arch_init.c,
how did you do this?
> +    void *ram;
> +    int ret;
> +    int cont;
> +
> +    ret = qemu_file_get_error(f);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    qemu_fflush(f);
> +
> +    cont = (block == s->last_block_sent) ? RAM_SAVE_FLAG_CONTINUE : 0;
> +
> +    ram = memory_region_get_ram_ptr(mr) + offset;
> +    s->last_block_sent = block;
> +
> +    *bytes_sent = local_save_page(f, block, offset, cont);
> +    if (!bytes_sent || *bytes_sent < 0) {
> +        return RAM_SAVE_CONTROL_DELAYED;
> +    }
> +
RAM_SAVE_CONTROL_DELAYED is only if you have *not* finished moving the 
bytes.

If you've finished moving the bytes, then you should return zero.

> +    /* DONTNEED the RAM page that has already been copied. */
> +    qemu_madvise(ram, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
> +

This should be ram_handle_compressed().

> +    return 0;
> +}
> +
> +const QEMUFileOps local_read_ops = {
> +    .get_fd     = qemu_local_get_fd,
> +    .get_buffer = qemu_local_get_buffer,
> +    .close      = qemu_local_close,
> +};
> +
> +const QEMUFileOps local_write_ops = {
> +    .get_fd             = qemu_local_get_fd,
> +    .writev_buffer      = qemu_local_put_buffer,
> +    .close              = qemu_local_close,
> +    .save_page          = qemu_local_ram_save,
> +};
> +
> +static void *qemu_fopen_local(int fd, const char *mode)
> +{
> +    QEMUFileLocal *s;
> +
> +    if (qemu_file_mode_is_not_valid(mode)) {
> +        return NULL;
> +    }
> +
> +    s = g_malloc0(sizeof(QEMUFileLocal));
> +    s->fd = fd;
> +
> +    if (mode[0] == 'w') {
> +        qemu_set_block(s->fd);
> +        s->file = qemu_fopen_ops(s, &local_write_ops);
> +    } else {
> +        s->file = qemu_fopen_ops(s, &local_read_ops);
> +    }
> +
> +    return s->file;
> +}