[4/9] qemu_qsb.diff

This patch adds support functions for operating on in memory sized file buffers.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 include/migration/qemu-file.h |   12 +++
 include/qemu-common.h         |   15 ++++
 util/qemu-file.c              |  184 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 211 insertions(+)

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 <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
> ---
>  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
> 
>

On 03/13/2013 05:11 PM, mdroth wrote:
> 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)?

The advantage of using the QEMUFile abstractions is that now you can 
build a visitor on top of it and read from buffers, sockets, BDRV's 
(later on), plain files, and whatever else you can hide underneath that 
interface. Back in 2011 when I initially wrote this code there at least 
was talk about using ASN.1 for migration, but this is nearly 2 years ago 
and it may never be done that way, so this was one driving force behind 
using QEMUFile inside the visitor. Besides that we later want to use the 
visitors for writing into virtual NVRAM, which we would build on top of 
a QEMUFile wrapping BDRVs. So there are some immediate advantages of 
using the common QEMUFile interface for reading and writing of data from 
different types of sources.

    Regards,
       Stefan

On Wed, Mar 13, 2013 at 05:28:56PM -0400, Stefan Berger wrote:
> On 03/13/2013 05:11 PM, mdroth wrote:
> >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)?
> 
> The advantage of using the QEMUFile abstractions is that now you can
> build a visitor on top of it and read from buffers, sockets, BDRV's
> (later on), plain files, and whatever else you can hide underneath
> that interface. Back in 2011 when I initially wrote this code there

Maybe a case can be made for making it a general utility library, but
I'm having a hard time thinking of any reasons that aren't specific to
migration, and I wonder if it's even necessary now that we have a
migration thread that can handle the rate-limiting/buffering
considerations.

But I'm not sure exactly why we decided to drop non-migration users, so
I think it's worth clarifying before we attempt to tether another
component to it.

Here's the thread I'm referencing:

https://lists.gnu.org/archive/html/qemu-devel/2011-09/msg02589.html

Juan, any background/input on this?

> that interface. Back in 2011 when I initially wrote this code there
> at least was talk about using ASN.1 for migration, but this is
> nearly 2 years ago and it may never be done that way, so this was
> one driving force behind using QEMUFile inside the visitor. Besides

Well, even back then goal was to abstract away direct calls to QEMUFile
and replace them with visitor calls, and then to provide legacy support
via a QEMUFile Visitor. A BER visitor could then be dropped in to
provide a BER migration protocol (how exactly this would be done was
somewhat of a ???, might've ended up layering on to of the
QEMUFile visitor anyway, but more out of pragmatism than anything else)

> that we later want to use the visitors for writing into virtual
> NVRAM, which we would build on top of a QEMUFile wrapping BDRVs. So
> there are some immediate advantages of using the common QEMUFile
> interface for reading and writing of data from different types of
> sources.

Can you describe the requirements for the BDRV wrapper a bit more?
Everything else seems reasonably doable via visitor internals but
maybe there's more to it I'm not considering.

> 
>    Regards,
>       Stefan
>

On Wed, Mar 13, 2013 at 05:41:33PM -0500, mdroth wrote:
> On Wed, Mar 13, 2013 at 05:28:56PM -0400, Stefan Berger wrote:
> > On 03/13/2013 05:11 PM, mdroth wrote:
> > >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)?
> > 
> > The advantage of using the QEMUFile abstractions is that now you can
> > build a visitor on top of it and read from buffers, sockets, BDRV's
> > (later on), plain files, and whatever else you can hide underneath
> > that interface. Back in 2011 when I initially wrote this code there
> 
> Maybe a case can be made for making it a general utility library, but
> I'm having a hard time thinking of any reasons that aren't specific to
> migration, and I wonder if it's even necessary now that we have a
> migration thread that can handle the rate-limiting/buffering
> considerations.
> 
> But I'm not sure exactly why we decided to drop non-migration users, so
> I think it's worth clarifying before we attempt to tether another
> component to it.
> 
> Here's the thread I'm referencing:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2011-09/msg02589.html
> 
> Juan, any background/input on this?

Sorry, forgot to CC Juan :)

> 
> > that interface. Back in 2011 when I initially wrote this code there
> > at least was talk about using ASN.1 for migration, but this is
> > nearly 2 years ago and it may never be done that way, so this was
> > one driving force behind using QEMUFile inside the visitor. Besides
> 
> Well, even back then goal was to abstract away direct calls to QEMUFile
> and replace them with visitor calls, and then to provide legacy support
> via a QEMUFile Visitor. A BER visitor could then be dropped in to
> provide a BER migration protocol (how exactly this would be done was
> somewhat of a ???, might've ended up layering on to of the
> QEMUFile visitor anyway, but more out of pragmatism than anything else)
> 
> > that we later want to use the visitors for writing into virtual
> > NVRAM, which we would build on top of a QEMUFile wrapping BDRVs. So
> > there are some immediate advantages of using the common QEMUFile
> > interface for reading and writing of data from different types of
> > sources.
> 
> Can you describe the requirements for the BDRV wrapper a bit more?
> Everything else seems reasonably doable via visitor internals but
> maybe there's more to it I'm not considering.
> 
> > 
> >    Regards,
> >       Stefan
> >