Re: [Qemu-devel] [PATCH 4/9] qemu_qsb.diff

[mdroth]

On Wed, Mar 13, 2013 at 01:56:23PM -0500, Joel Schopp wrote:
> This patch adds support functions for operating on in memory sized file 
> buffers.

There's been some past refactorings to remove non-migration users of
QEMUFile, and AFAIK that's still the case today. QEMUFile satisfies
funky requirements like rate-limiting, buffering, etc that were specific
to migration.

IIUC all we want here is an abstraction on top of write()/memcpy(),
and access to qemu_{put|get}_be* utility functions.

Have you considered rolling those abstractions in the visitor
implementations as opposed to extending QEMUFile, and using
be*_to_cpus/cpus_to_be* helpers directly instead (like block/qcow2.c
does, for example)?

> 
> Signed-off-by: Stefan Berger <address@hidden>
> Signed-off-by: Joel Schopp <address@hidden>
> ---
>  include/migration/qemu-file.h |   12 +++
>  include/qemu-common.h         |   15 ++++
>  util/qemu-file.c              |  184 
> +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 211 insertions(+)
> 
> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> index 07d8362..2bc77b1 100644
> --- a/include/migration/qemu-file.h
> +++ b/include/migration/qemu-file.h
> @@ -24,6 +24,8 @@
>  #ifndef QEMU_FILE_H
>  #define QEMU_FILE_H 1
> 
> +#include <stdint.h>
> +
>  /* This function writes a chunk of data to a file at the given position.
>   * The pos argument can be ignored if the file is only being used for
>   * streaming.  The handler should try to write all of the data it can.
> @@ -58,6 +60,14 @@ typedef struct QEMUFileOps {
>      QEMUFileGetFD *get_fd;
>  } QEMUFileOps;
> 
> +struct QEMUSizedBuffer {
> +    unsigned char *buffer;
> +    uint64_t size;
> +    uint64_t used;
> +};
> +
> +typedef struct QEMUSizedBuffer QEMUSizedBuffer;
> +
>  QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
>  QEMUFile *qemu_fopen(const char *filename, const char *mode);
>  QEMUFile *qemu_fdopen(int fd, const char *mode);
> @@ -71,6 +81,8 @@ void qemu_put_byte(QEMUFile *f, int v);
>  int qemu_read_bytes(QEMUFile *f, uint8_t *buf, int size);
>  int qemu_peek_bytes(QEMUFile *f, uint8_t *buf, int size, size_t offset);
>  int qemu_write_bytes(QEMUFile *f, const uint8_t *buf, int size);
> +QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input);
> +const QEMUSizedBuffer *qemu_buf_get(QEMUFile *f);
> 
>  static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
>  {
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 5e13708..de1cdc0 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -442,4 +442,19 @@ int64_t pow2floor(int64_t value);
>  int uleb128_encode_small(uint8_t *out, uint32_t n);
>  int uleb128_decode_small(const uint8_t *in, uint32_t *n);
> 
> +/* QEMU Sized Buffer */
> +#include "include/migration/qemu-file.h"
> +QEMUSizedBuffer *qsb_create(const uint8_t *buffer, uint64_t len);
> +QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *);
> +void qsb_free(QEMUSizedBuffer *);
> +uint64_t qsb_set_length(QEMUSizedBuffer *qsb, uint64_t length);
> +uint64_t qsb_get_length(const QEMUSizedBuffer *qsb);
> +const unsigned char *qsb_get_buffer(const QEMUSizedBuffer *, int64_t pos);
> +int qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t *buf,
> +                 int64_t pos, int size);
> +int qsb_append_qsb(QEMUSizedBuffer *dest, const QEMUSizedBuffer *src);
> +int qsb_append(QEMUSizedBuffer *dest, const uint8_t *buffer, uint64_t len);
> +
> +
> +
>  #endif
> diff --git a/util/qemu-file.c b/util/qemu-file.c
> index e698713..4442dcc 100644
> --- a/util/qemu-file.c
> +++ b/util/qemu-file.c
> @@ -710,3 +710,187 @@ int qemu_write_bytes(QEMUFile *f, const uint8_t *buf, 
> int size)
> 
>      return size;
>  }
> +
> +
> +QEMUSizedBuffer *qsb_create(const uint8_t *buffer, uint64_t len)
> +{
> +    QEMUSizedBuffer *qsb;
> +    uint64_t alloc_len;
> +
> +    alloc_len = (len > 1024) ? len : 1024;
> +
> +    qsb = g_new0(QEMUSizedBuffer, 1);
> +    if (!qsb) {
> +        return NULL;
> +    }
> +
> +    qsb->buffer = g_malloc(alloc_len);
> +    if (!qsb->buffer) {
> +        g_free(qsb);
> +        return NULL;
> +    }
> +    qsb->size = alloc_len;
> +
> +    if (buffer) {
> +        memcpy(qsb->buffer, buffer, len);
> +        qsb->used = len;
> +    }
> +
> +    return qsb;
> +}
> +
> +void qsb_free(QEMUSizedBuffer *qsb)
> +{
> +    if (!qsb) {
> +        return;
> +    }
> +    g_free(qsb->buffer);
> +    g_free(qsb);
> +}
> +
> +uint64_t qsb_get_length(const QEMUSizedBuffer *qsb)
> +{
> +    return qsb->used;
> +}
> +
> +uint64_t qsb_set_length(QEMUSizedBuffer *qsb, uint64_t new_len)
> +{
> +    if (new_len <= qsb->size) {
> +        qsb->used = new_len;
> +    } else {
> +        qsb->used = qsb->size;
> +    }
> +    return qsb->used;
> +}
> +
> +const unsigned char *qsb_get_buffer(const QEMUSizedBuffer *qsb, int64_t pos)
> +{
> +    if (pos < qsb->used) {
> +        return &qsb->buffer[pos];
> +    }
> +    return NULL;
> +}
> +
> +int qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t *buf,
> +                 int64_t pos, int size)
> +{
> +    if (pos + size > qsb->size) {
> +        qsb->buffer = g_realloc(qsb->buffer, pos + size + 1024);
> +        if (qsb->buffer == NULL) {
> +            return -ENOMEM;
> +        }
> +        qsb->size = pos + size;
> +    }
> +    memcpy(&qsb->buffer[pos], buf, size);
> +    if (pos + size > qsb->used) {
> +        qsb->used = pos + size;
> +    }
> +
> +    return size;
> +}
> +
> +int qsb_append_qsb(QEMUSizedBuffer *dest, const QEMUSizedBuffer *src)
> +{
> +    return qsb_write_at(dest, qsb_get_buffer(src, 0),
> +                        qsb_get_length(dest), qsb_get_length(src));
> +}
> +
> +int qsb_append(QEMUSizedBuffer *dest, const uint8_t *buf, uint64_t len)
> +{
> +    return qsb_write_at(dest, buf,
> +                        qsb_get_length(dest), len);
> +}
> +
> +QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *in)
> +{
> +    return qsb_create(qsb_get_buffer(in, 0),
> +                      qsb_get_length(in));
> +}
> +
> +typedef struct QEMUBuffer {
> +    QEMUSizedBuffer *qsb;
> +    QEMUFile *file;
> +} QEMUBuffer;
> +
> +static int buf_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
> +{
> +    QEMUBuffer *s = opaque;
> +    ssize_t len = qsb_get_length(s->qsb) - pos;
> +
> +    if (len <= 0) {
> +        return 0;
> +    }
> +
> +    if (len > size) {
> +        len = size;
> +    }
> +    memcpy(buf, qsb_get_buffer(s->qsb, pos), len);
> +
> +    return len;
> +}
> +
> +static int buf_put_buffer(void *opaque, const uint8_t *buf,
> +                          int64_t pos, int size)
> +{
> +    QEMUBuffer *s = opaque;
> +
> +    return qsb_write_at(s->qsb, buf, pos, size);
> +}
> +
> +static int buf_close(void *opaque)
> +{
> +    QEMUBuffer *s = opaque;
> +
> +    qsb_free(s->qsb);
> +
> +    g_free(s);
> +
> +    return 0;
> +}
> +
> +const QEMUSizedBuffer *qemu_buf_get(QEMUFile *f)
> +{
> +    QEMUBuffer *p;
> +
> +    qemu_fflush(f);
> +
> +    p = (QEMUBuffer *)f->opaque;
> +
> +    return p->qsb;
> +}
> +
> +static const QEMUFileOps buf_read_ops = {
> +    .get_buffer = buf_get_buffer,
> +    .close =      buf_close
> +};
> +
> +static const QEMUFileOps buf_write_ops = {
> +    .put_buffer = buf_put_buffer,
> +    .close =      buf_close
> +};
> +
> +QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input)
> +{
> +    QEMUBuffer *s;
> +
> +    if (mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] != 0) {
> +        fprintf(stderr, "qemu_bufopen: Argument validity check failed\n");
> +        return NULL;
> +    }
> +
> +    s = g_malloc0(sizeof(QEMUBuffer));
> +    if (mode[0] == 'r') {
> +        s->qsb = input;
> +    }
> +
> +    if (s->qsb == NULL) {
> +        s->qsb = qsb_create(NULL, 0);
> +    }
> +
> +    if (mode[0] == 'r') {
> +        s->file = qemu_fopen_ops(s, &buf_read_ops);
> +    } else {
> +        s->file = qemu_fopen_ops(s, &buf_write_ops);
> +    }
> +    return s->file;
> +}
> -- 
> 1.7.10.4
> 
>