qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 2/9] qapi_c_arrays.diff
  2013-03-13  3:09 [Qemu-devel] [PATCH 0/9 v2] Implement and test asn1 ber visitors Joel Schopp
@ 2013-03-13  3:09 ` Joel Schopp
  2013-03-13 12:08   ` Eric Blake
  0 siblings, 1 reply; 33+ messages in thread
From: Joel Schopp @ 2013-03-13  3:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joel Schopp, Paolo Bonzini, Michael Roth, Michael Tsirkin

Forward ported Mike's previously sent patch
(see http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg05782.html ) in my
series since it impelments a qapi array interface the ASN.1 BER visitor needs.

Generally these will be serialized into lists, but the
representation can be of any form so long as it can
be deserialized into a single-dimension C array.

Cc: Michael Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 include/qapi/visitor-impl.h |    4 ++++
 include/qapi/visitor.h      |    4 ++++
 qapi/qapi-visit-core.c      |   25 +++++++++++++++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 5159964..79fe039 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -34,6 +34,10 @@ struct Visitor
     void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
     void (*type_number)(Visitor *v, double *obj, const char *name,
                         Error **errp);
+    void (*start_carray)(Visitor *v, void **obj, const char *name,
+                        size_t elem_count, size_t elem_size, Error **errp);
+    void (*next_carray)(Visitor *v, Error **errp);
+    void (*end_carray)(Visitor *v, Error **errp);
 
     /* May be NULL */
     void (*start_optional)(Visitor *v, bool *present, const char *name,
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 1fef18c..49f411f 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -51,5 +51,9 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
+void visit_start_carray(Visitor *v, void **obj, const char *name,
+                       size_t elem_count, size_t elem_size, Error **errp);
+void visit_next_carray(Visitor *v, Error **errp);
+void visit_end_carray(Visitor *v, Error **errp);
 
 #endif
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 401ee6e..d9982f8 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -313,3 +313,28 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
     g_free(enum_str);
     *obj = value;
 }
+
+void visit_start_carray(Visitor *v, void **obj, const char *name,
+                        size_t elem_count, size_t elem_size, Error **errp)
+{
+    g_assert(v->start_carray);
+    if (!error_is_set(errp)) {
+        v->start_carray(v, obj, name, elem_count, elem_size, errp);
+    }
+}
+
+void visit_next_carray(Visitor *v, Error **errp)
+{
+    g_assert(v->next_carray);
+    if (!error_is_set(errp)) {
+        v->next_carray(v, errp);
+    }
+}
+
+void visit_end_carray(Visitor *v, Error **errp)
+{
+    g_assert(v->end_carray);
+    if (!error_is_set(errp)) {
+        v->end_carray(v, errp);
+    }
+}
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 2/9] qapi_c_arrays.diff
  2013-03-13  3:09 ` [Qemu-devel] [PATCH 2/9] qapi_c_arrays.diff Joel Schopp
@ 2013-03-13 12:08   ` Eric Blake
  0 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2013-03-13 12:08 UTC (permalink / raw)
  To: Joel Schopp; +Cc: Paolo Bonzini, Michael Tsirkin, qemu-devel, Michael Roth

[-- Attachment #1: Type: text/plain, Size: 1567 bytes --]

On 03/12/2013 09:09 PM, Joel Schopp wrote:
> Forward ported Mike's previously sent patch
> (see http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg05782.html ) in my
> series since it impelments a qapi array interface the ASN.1 BER visitor needs.

s/impelments/implements/

> 
> Generally these will be serialized into lists, but the
> representation can be of any form so long as it can
> be deserialized into a single-dimension C array.
> 

> +++ b/include/qapi/visitor-impl.h
> @@ -34,6 +34,10 @@ struct Visitor
>      void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
>      void (*type_number)(Visitor *v, double *obj, const char *name,
>                          Error **errp);
> +    void (*start_carray)(Visitor *v, void **obj, const char *name,
> +                        size_t elem_count, size_t elem_size, Error **errp);

Indentation looks off.

> +++ b/include/qapi/visitor.h
> @@ -51,5 +51,9 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>  void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
>  void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
>  void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
> +void visit_start_carray(Visitor *v, void **obj, const char *name,
> +                       size_t elem_count, size_t elem_size, Error **errp);

Indentation.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 0/9 v3] Implement and test asn1 ber visitors
@ 2013-03-13 18:56 Joel Schopp
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 1/9] qemu-file Joel Schopp
                   ` (8 more replies)
  0 siblings, 9 replies; 33+ messages in thread
From: Joel Schopp @ 2013-03-13 18:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joel Schopp

These patches implement asn1 ber visitors for encoding and decoding data. 

changed since v2:
Moved qemu-file.c to util/
Left the bdrv functions in savevm.c
Fixed a typo in the introduction to qapi c arrays patch
Fixed two indendations in the qapi c arrays patch

changed since v1:
Moved .c files into qapi directory
Moved .h files into include/qapi
Added sized buffer code
cleaned up Makefile changes to play nicer
Broke out patches for output/input visitors
New tests in tests/test-visitor-serialization.c
Fixed width integer encoding now marked with application specific bit
Removed qapi array code
Added Mike Roth's previously sent carray qapi patch 
misc small cleanups
forward ported to keep up with git HEAD
added real number support

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 1/9] qemu-file
  2013-03-13 18:56 [Qemu-devel] [PATCH 0/9 v3] Implement and test asn1 ber visitors Joel Schopp
@ 2013-03-13 18:56 ` Joel Schopp
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 2/9] qapi_c_arrays.diff Joel Schopp
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Joel Schopp @ 2013-03-13 18:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joel Schopp, Michael Tsirkin, Stefan Berger, Paolo Bonzini

This patch reorganizes qemu file operations to be in their own source file
instead of being lumped in savevm.c.  Besides being more logical for maintenance
it also makes it easier for future users of the file functions to add tests.

v4 move qemu-file.c into util/ , leave the bdrv functions where they are for now
v3 forward port to resolve conflicts
v2 forward port to resolve conflicts, strip trailing whitespace during move

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael Tsirkin <mst@redhat.com>
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 |    5 +
 savevm.c                      |  656 +--------------------------------------
 util/Makefile.objs            |    1 +
 util/qemu-file.c              |  682 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 689 insertions(+), 655 deletions(-)
 create mode 100644 util/qemu-file.c

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index df81261..b76a901 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -98,6 +98,11 @@ void qemu_file_reset_rate_limit(QEMUFile *f);
 void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
 int64_t qemu_file_get_rate_limit(QEMUFile *f);
 int qemu_file_get_error(QEMUFile *f);
+void qemu_file_set_error(QEMUFile *f, int ret);
+int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset);
+int qemu_peek_byte(QEMUFile *f, int offset);
+void qemu_file_skip(QEMUFile *f, int size);
+void qemu_fflush(QEMUFile *f);
 
 static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
 {
diff --git a/savevm.c b/savevm.c
index 147e2d2..18b7144 100644
--- a/savevm.c
+++ b/savevm.c
@@ -109,336 +109,7 @@ void qemu_announce_self(void)
 	qemu_announce_self_once(&timer);
 }
 
-/***********************************************************/
-/* savevm/loadvm support */
-
-#define IO_BUF_SIZE 32768
-
-struct QEMUFile {
-    const QEMUFileOps *ops;
-    void *opaque;
-    int is_write;
-
-    int64_t bytes_xfer;
-    int64_t xfer_limit;
-
-    int64_t pos; /* start of buffer when writing, end of buffer
-                    when reading */
-    int buf_index;
-    int buf_size; /* 0 when writing */
-    uint8_t buf[IO_BUF_SIZE];
-
-    int last_error;
-};
-
-typedef struct QEMUFileStdio
-{
-    FILE *stdio_file;
-    QEMUFile *file;
-} QEMUFileStdio;
-
-typedef struct QEMUFileSocket
-{
-    int fd;
-    QEMUFile *file;
-} QEMUFileSocket;
-
-typedef struct {
-    Coroutine *co;
-    int fd;
-} FDYieldUntilData;
-
-static void fd_coroutine_enter(void *opaque)
-{
-    FDYieldUntilData *data = opaque;
-    qemu_set_fd_handler(data->fd, NULL, NULL, NULL);
-    qemu_coroutine_enter(data->co, NULL);
-}
-
-/**
- * Yield until a file descriptor becomes readable
- *
- * Note that this function clobbers the handlers for the file descriptor.
- */
-static void coroutine_fn yield_until_fd_readable(int fd)
-{
-    FDYieldUntilData data;
-
-    assert(qemu_in_coroutine());
-    data.co = qemu_coroutine_self();
-    data.fd = fd;
-    qemu_set_fd_handler(fd, fd_coroutine_enter, NULL, &data);
-    qemu_coroutine_yield();
-}
-
-static int socket_get_fd(void *opaque)
-{
-    QEMUFileSocket *s = opaque;
-
-    return s->fd;
-}
-
-static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
-{
-    QEMUFileSocket *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;
-}
-
-static int socket_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
-{
-    QEMUFileSocket *s = opaque;
-    ssize_t len;
-
-    len = qemu_send_full(s->fd, buf, size, 0);
-    if (len < size) {
-        len = -socket_error();
-    }
-    return len;
-}
-
-static int socket_close(void *opaque)
-{
-    QEMUFileSocket *s = opaque;
-    closesocket(s->fd);
-    g_free(s);
-    return 0;
-}
-
-static int stdio_get_fd(void *opaque)
-{
-    QEMUFileStdio *s = opaque;
-
-    return fileno(s->stdio_file);
-}
-
-static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
-{
-    QEMUFileStdio *s = opaque;
-    return fwrite(buf, 1, size, s->stdio_file);
-}
-
-static int stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
-{
-    QEMUFileStdio *s = opaque;
-    FILE *fp = s->stdio_file;
-    int bytes;
-
-    for (;;) {
-        clearerr(fp);
-        bytes = fread(buf, 1, size, fp);
-        if (bytes != 0 || !ferror(fp)) {
-            break;
-        }
-        if (errno == EAGAIN) {
-            yield_until_fd_readable(fileno(fp));
-        } else if (errno != EINTR) {
-            break;
-        }
-    }
-    return bytes;
-}
-
-static int stdio_pclose(void *opaque)
-{
-    QEMUFileStdio *s = opaque;
-    int ret;
-    ret = pclose(s->stdio_file);
-    if (ret == -1) {
-        ret = -errno;
-    } else if (!WIFEXITED(ret) || WEXITSTATUS(ret) != 0) {
-        /* close succeeded, but non-zero exit code: */
-        ret = -EIO; /* fake errno value */
-    }
-    g_free(s);
-    return ret;
-}
-
-static int stdio_fclose(void *opaque)
-{
-    QEMUFileStdio *s = opaque;
-    int ret = 0;
-
-    if (s->file->ops->put_buffer) {
-        int fd = fileno(s->stdio_file);
-        struct stat st;
-
-        ret = fstat(fd, &st);
-        if (ret == 0 && S_ISREG(st.st_mode)) {
-            /*
-             * If the file handle is a regular file make sure the
-             * data is flushed to disk before signaling success.
-             */
-            ret = fsync(fd);
-            if (ret != 0) {
-                ret = -errno;
-                return ret;
-            }
-        }
-    }
-    if (fclose(s->stdio_file) == EOF) {
-        ret = -errno;
-    }
-    g_free(s);
-    return ret;
-}
-
-static const QEMUFileOps stdio_pipe_read_ops = {
-    .get_fd =     stdio_get_fd,
-    .get_buffer = stdio_get_buffer,
-    .close =      stdio_pclose
-};
-
-static const QEMUFileOps stdio_pipe_write_ops = {
-    .get_fd =     stdio_get_fd,
-    .put_buffer = stdio_put_buffer,
-    .close =      stdio_pclose
-};
-
-QEMUFile *qemu_popen_cmd(const char *command, const char *mode)
-{
-    FILE *stdio_file;
-    QEMUFileStdio *s;
-
-    stdio_file = popen(command, mode);
-    if (stdio_file == NULL) {
-        return NULL;
-    }
-
-    if (mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] != 0) {
-        fprintf(stderr, "qemu_popen: Argument validity check failed\n");
-        return NULL;
-    }
-
-    s = g_malloc0(sizeof(QEMUFileStdio));
-
-    s->stdio_file = stdio_file;
-
-    if(mode[0] == 'r') {
-        s->file = qemu_fopen_ops(s, &stdio_pipe_read_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &stdio_pipe_write_ops);
-    }
-    return s->file;
-}
-
-static const QEMUFileOps stdio_file_read_ops = {
-    .get_fd =     stdio_get_fd,
-    .get_buffer = stdio_get_buffer,
-    .close =      stdio_fclose
-};
-
-static const QEMUFileOps stdio_file_write_ops = {
-    .get_fd =     stdio_get_fd,
-    .put_buffer = stdio_put_buffer,
-    .close =      stdio_fclose
-};
-
-QEMUFile *qemu_fdopen(int fd, const char *mode)
-{
-    QEMUFileStdio *s;
-
-    if (mode == NULL ||
-	(mode[0] != 'r' && mode[0] != 'w') ||
-	mode[1] != 'b' || mode[2] != 0) {
-        fprintf(stderr, "qemu_fdopen: Argument validity check failed\n");
-        return NULL;
-    }
-
-    s = g_malloc0(sizeof(QEMUFileStdio));
-    s->stdio_file = fdopen(fd, mode);
-    if (!s->stdio_file)
-        goto fail;
-
-    if(mode[0] == 'r') {
-        s->file = qemu_fopen_ops(s, &stdio_file_read_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &stdio_file_write_ops);
-    }
-    return s->file;
-
-fail:
-    g_free(s);
-    return NULL;
-}
-
-static const QEMUFileOps socket_read_ops = {
-    .get_fd =     socket_get_fd,
-    .get_buffer = socket_get_buffer,
-    .close =      socket_close
-};
-
-static const QEMUFileOps socket_write_ops = {
-    .get_fd =     socket_get_fd,
-    .put_buffer = socket_put_buffer,
-    .close =      socket_close
-};
-
-QEMUFile *qemu_fopen_socket(int fd, const char *mode)
-{
-    QEMUFileSocket *s = g_malloc0(sizeof(QEMUFileSocket));
-
-    if (mode == NULL ||
-        (mode[0] != 'r' && mode[0] != 'w') ||
-        mode[1] != 'b' || mode[2] != 0) {
-        fprintf(stderr, "qemu_fopen: Argument validity check failed\n");
-        return NULL;
-    }
-
-    s->fd = fd;
-    if (mode[0] == 'w') {
-        socket_set_block(s->fd);
-        s->file = qemu_fopen_ops(s, &socket_write_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &socket_read_ops);
-    }
-    return s->file;
-}
-
-QEMUFile *qemu_fopen(const char *filename, const char *mode)
-{
-    QEMUFileStdio *s;
-
-    if (mode == NULL ||
-	(mode[0] != 'r' && mode[0] != 'w') ||
-	mode[1] != 'b' || mode[2] != 0) {
-        fprintf(stderr, "qemu_fopen: Argument validity check failed\n");
-        return NULL;
-    }
-
-    s = g_malloc0(sizeof(QEMUFileStdio));
-
-    s->stdio_file = fopen(filename, mode);
-    if (!s->stdio_file)
-        goto fail;
-    
-    if(mode[0] == 'w') {
-        s->file = qemu_fopen_ops(s, &stdio_file_write_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &stdio_file_read_ops);
-    }
-    return s->file;
-fail:
-    g_free(s);
-    return NULL;
-}
-
+/* Block */
 static int block_put_buffer(void *opaque, const uint8_t *buf,
                            int64_t pos, int size)
 {
@@ -473,331 +144,6 @@ static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
     return qemu_fopen_ops(bs, &bdrv_read_ops);
 }
 
-QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops)
-{
-    QEMUFile *f;
-
-    f = g_malloc0(sizeof(QEMUFile));
-
-    f->opaque = opaque;
-    f->ops = ops;
-    f->is_write = 0;
-    return f;
-}
-
-int qemu_file_get_error(QEMUFile *f)
-{
-    return f->last_error;
-}
-
-static void qemu_file_set_error(QEMUFile *f, int ret)
-{
-    if (f->last_error == 0) {
-        f->last_error = ret;
-    }
-}
-
-/** Flushes QEMUFile buffer
- *
- */
-static void qemu_fflush(QEMUFile *f)
-{
-    int ret = 0;
-
-    if (!f->ops->put_buffer) {
-        return;
-    }
-    if (f->is_write && f->buf_index > 0) {
-        ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index);
-        if (ret >= 0) {
-            f->pos += f->buf_index;
-        }
-        f->buf_index = 0;
-    }
-    if (ret < 0) {
-        qemu_file_set_error(f, ret);
-    }
-}
-
-static void qemu_fill_buffer(QEMUFile *f)
-{
-    int len;
-    int pending;
-
-    if (!f->ops->get_buffer)
-        return;
-
-    if (f->is_write)
-        abort();
-
-    pending = f->buf_size - f->buf_index;
-    if (pending > 0) {
-        memmove(f->buf, f->buf + f->buf_index, pending);
-    }
-    f->buf_index = 0;
-    f->buf_size = pending;
-
-    len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
-                        IO_BUF_SIZE - pending);
-    if (len > 0) {
-        f->buf_size += len;
-        f->pos += len;
-    } else if (len == 0) {
-        qemu_file_set_error(f, -EIO);
-    } else if (len != -EAGAIN)
-        qemu_file_set_error(f, len);
-}
-
-int qemu_get_fd(QEMUFile *f)
-{
-    if (f->ops->get_fd) {
-        return f->ops->get_fd(f->opaque);
-    }
-    return -1;
-}
-
-/** Closes the file
- *
- * Returns negative error value if any error happened on previous operations or
- * while closing the file. Returns 0 or positive number on success.
- *
- * The meaning of return value on success depends on the specific backend
- * being used.
- */
-int qemu_fclose(QEMUFile *f)
-{
-    int ret;
-    qemu_fflush(f);
-    ret = qemu_file_get_error(f);
-
-    if (f->ops->close) {
-        int ret2 = f->ops->close(f->opaque);
-        if (ret >= 0) {
-            ret = ret2;
-        }
-    }
-    /* If any error was spotted before closing, we should report it
-     * instead of the close() return value.
-     */
-    if (f->last_error) {
-        ret = f->last_error;
-    }
-    g_free(f);
-    return ret;
-}
-
-void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
-{
-    int l;
-
-    if (f->last_error) {
-        return;
-    }
-
-    if (f->is_write == 0 && f->buf_index > 0) {
-        fprintf(stderr,
-                "Attempted to write to buffer while read buffer is not empty\n");
-        abort();
-    }
-
-    while (size > 0) {
-        l = IO_BUF_SIZE - f->buf_index;
-        if (l > size)
-            l = size;
-        memcpy(f->buf + f->buf_index, buf, l);
-        f->is_write = 1;
-        f->buf_index += l;
-        f->bytes_xfer += l;
-        buf += l;
-        size -= l;
-        if (f->buf_index >= IO_BUF_SIZE) {
-            qemu_fflush(f);
-            if (qemu_file_get_error(f)) {
-                break;
-            }
-        }
-    }
-}
-
-void qemu_put_byte(QEMUFile *f, int v)
-{
-    if (f->last_error) {
-        return;
-    }
-
-    if (f->is_write == 0 && f->buf_index > 0) {
-        fprintf(stderr,
-                "Attempted to write to buffer while read buffer is not empty\n");
-        abort();
-    }
-
-    f->buf[f->buf_index++] = v;
-    f->is_write = 1;
-    if (f->buf_index >= IO_BUF_SIZE) {
-        qemu_fflush(f);
-    }
-}
-
-static void qemu_file_skip(QEMUFile *f, int size)
-{
-    if (f->buf_index + size <= f->buf_size) {
-        f->buf_index += size;
-    }
-}
-
-static int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
-{
-    int pending;
-    int index;
-
-    if (f->is_write) {
-        abort();
-    }
-
-    index = f->buf_index + offset;
-    pending = f->buf_size - index;
-    if (pending < size) {
-        qemu_fill_buffer(f);
-        index = f->buf_index + offset;
-        pending = f->buf_size - index;
-    }
-
-    if (pending <= 0) {
-        return 0;
-    }
-    if (size > pending) {
-        size = pending;
-    }
-
-    memcpy(buf, f->buf + index, size);
-    return size;
-}
-
-int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
-{
-    int pending = size;
-    int done = 0;
-
-    while (pending > 0) {
-        int res;
-
-        res = qemu_peek_buffer(f, buf, pending, 0);
-        if (res == 0) {
-            return done;
-        }
-        qemu_file_skip(f, res);
-        buf += res;
-        pending -= res;
-        done += res;
-    }
-    return done;
-}
-
-static int qemu_peek_byte(QEMUFile *f, int offset)
-{
-    int index = f->buf_index + offset;
-
-    if (f->is_write) {
-        abort();
-    }
-
-    if (index >= f->buf_size) {
-        qemu_fill_buffer(f);
-        index = f->buf_index + offset;
-        if (index >= f->buf_size) {
-            return 0;
-        }
-    }
-    return f->buf[index];
-}
-
-int qemu_get_byte(QEMUFile *f)
-{
-    int result;
-
-    result = qemu_peek_byte(f, 0);
-    qemu_file_skip(f, 1);
-    return result;
-}
-
-int64_t qemu_ftell(QEMUFile *f)
-{
-    qemu_fflush(f);
-    return f->pos;
-}
-
-int qemu_file_rate_limit(QEMUFile *f)
-{
-    if (qemu_file_get_error(f)) {
-        return 1;
-    }
-    if (f->xfer_limit > 0 && f->bytes_xfer > f->xfer_limit) {
-        return 1;
-    }
-    return 0;
-}
-
-int64_t qemu_file_get_rate_limit(QEMUFile *f)
-{
-    return f->xfer_limit;
-}
-
-void qemu_file_set_rate_limit(QEMUFile *f, int64_t limit)
-{
-    f->xfer_limit = limit;
-}
-
-void qemu_file_reset_rate_limit(QEMUFile *f)
-{
-    f->bytes_xfer = 0;
-}
-
-void qemu_put_be16(QEMUFile *f, unsigned int v)
-{
-    qemu_put_byte(f, v >> 8);
-    qemu_put_byte(f, v);
-}
-
-void qemu_put_be32(QEMUFile *f, unsigned int v)
-{
-    qemu_put_byte(f, v >> 24);
-    qemu_put_byte(f, v >> 16);
-    qemu_put_byte(f, v >> 8);
-    qemu_put_byte(f, v);
-}
-
-void qemu_put_be64(QEMUFile *f, uint64_t v)
-{
-    qemu_put_be32(f, v >> 32);
-    qemu_put_be32(f, v);
-}
-
-unsigned int qemu_get_be16(QEMUFile *f)
-{
-    unsigned int v;
-    v = qemu_get_byte(f) << 8;
-    v |= qemu_get_byte(f);
-    return v;
-}
-
-unsigned int qemu_get_be32(QEMUFile *f)
-{
-    unsigned int v;
-    v = qemu_get_byte(f) << 24;
-    v |= qemu_get_byte(f) << 16;
-    v |= qemu_get_byte(f) << 8;
-    v |= qemu_get_byte(f);
-    return v;
-}
-
-uint64_t qemu_get_be64(QEMUFile *f)
-{
-    uint64_t v;
-    v = (uint64_t)qemu_get_be32(f) << 32;
-    v |= qemu_get_be32(f);
-    return v;
-}
-
 
 /* timer */
 
diff --git a/util/Makefile.objs b/util/Makefile.objs
index cad5ce8..499cba9 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -9,3 +9,4 @@ util-obj-y += error.o qemu-error.o
 util-obj-$(CONFIG_POSIX) += compatfd.o
 util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o
 util-obj-y += qemu-option.o qemu-progress.o
+util-obj-y += qemu-file.o
\ No newline at end of file
diff --git a/util/qemu-file.c b/util/qemu-file.c
new file mode 100644
index 0000000..44b52b4
--- /dev/null
+++ b/util/qemu-file.c
@@ -0,0 +1,682 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu-common.h"
+#include "hw/hw.h"
+#include "block/block.h"
+#include "qemu/sockets.h"
+
+
+#define IO_BUF_SIZE 32768
+
+struct QEMUFile {
+    const QEMUFileOps *ops;
+    void *opaque;
+    int is_write;
+
+    int64_t bytes_xfer;
+    int64_t xfer_limit;
+
+    int64_t pos; /* start of buffer when writing, end of buffer
+                    when reading */
+    int buf_index;
+    int buf_size; /* 0 when writing */
+    uint8_t buf[IO_BUF_SIZE];
+
+    int last_error;
+};
+
+typedef struct QEMUFileStdio
+{
+    FILE *stdio_file;
+    QEMUFile *file;
+} QEMUFileStdio;
+
+typedef struct QEMUFileSocket
+{
+    int fd;
+    QEMUFile *file;
+} QEMUFileSocket;
+
+typedef struct {
+    Coroutine *co;
+    int fd;
+} FDYieldUntilData;
+
+static void fd_coroutine_enter(void *opaque)
+{
+    FDYieldUntilData *data = opaque;
+    qemu_set_fd_handler(data->fd, NULL, NULL, NULL);
+    qemu_coroutine_enter(data->co, NULL);
+}
+
+/**
+ * Yield until a file descriptor becomes readable
+ *
+ * Note that this function clobbers the handlers for the file descriptor.
+ */
+static void coroutine_fn yield_until_fd_readable(int fd)
+{
+    FDYieldUntilData data;
+
+    assert(qemu_in_coroutine());
+    data.co = qemu_coroutine_self();
+    data.fd = fd;
+    qemu_set_fd_handler(fd, fd_coroutine_enter, NULL, &data);
+    qemu_coroutine_yield();
+}
+
+static int socket_get_fd(void *opaque)
+{
+    QEMUFileSocket *s = opaque;
+
+    return s->fd;
+}
+
+static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+    QEMUFileSocket *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;
+}
+
+static int socket_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
+{
+    QEMUFileSocket *s = opaque;
+    ssize_t len;
+
+    len = qemu_send_full(s->fd, buf, size, 0);
+    if (len < size) {
+        len = -socket_error();
+    }
+    return len;
+}
+
+static int socket_close(void *opaque)
+{
+    QEMUFileSocket *s = opaque;
+    closesocket(s->fd);
+    g_free(s);
+    return 0;
+}
+
+static int stdio_get_fd(void *opaque)
+{
+    QEMUFileStdio *s = opaque;
+
+    return fileno(s->stdio_file);
+}
+
+static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
+{
+    QEMUFileStdio *s = opaque;
+    return fwrite(buf, 1, size, s->stdio_file);
+}
+
+static int stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+    QEMUFileStdio *s = opaque;
+    FILE *fp = s->stdio_file;
+    int bytes;
+
+    for (;;) {
+        clearerr(fp);
+        bytes = fread(buf, 1, size, fp);
+        if (bytes != 0 || !ferror(fp)) {
+            break;
+        }
+        if (errno == EAGAIN) {
+            yield_until_fd_readable(fileno(fp));
+        } else if (errno != EINTR) {
+            break;
+        }
+    }
+    return bytes;
+}
+
+static int stdio_pclose(void *opaque)
+{
+    QEMUFileStdio *s = opaque;
+    int ret;
+    ret = pclose(s->stdio_file);
+    if (ret == -1) {
+        ret = -errno;
+    } else if (!WIFEXITED(ret) || WEXITSTATUS(ret) != 0) {
+        /* close succeeded, but non-zero exit code: */
+        ret = -EIO; /* fake errno value */
+    }
+    g_free(s);
+    return ret;
+}
+
+static int stdio_fclose(void *opaque)
+{
+    QEMUFileStdio *s = opaque;
+    int ret = 0;
+
+    if (s->file->ops->put_buffer) {
+        int fd = fileno(s->stdio_file);
+        struct stat st;
+
+        ret = fstat(fd, &st);
+        if (ret == 0 && S_ISREG(st.st_mode)) {
+            /*
+             * If the file handle is a regular file make sure the
+             * data is flushed to disk before signaling success.
+             */
+            ret = fsync(fd);
+            if (ret != 0) {
+                ret = -errno;
+                return ret;
+            }
+        }
+    }
+    if (fclose(s->stdio_file) == EOF) {
+        ret = -errno;
+    }
+    g_free(s);
+    return ret;
+}
+
+static const QEMUFileOps stdio_pipe_read_ops = {
+    .get_fd =     stdio_get_fd,
+    .get_buffer = stdio_get_buffer,
+    .close =      stdio_pclose
+};
+
+static const QEMUFileOps stdio_pipe_write_ops = {
+    .get_fd =     stdio_get_fd,
+    .put_buffer = stdio_put_buffer,
+    .close =      stdio_pclose
+};
+
+QEMUFile *qemu_popen_cmd(const char *command, const char *mode)
+{
+    FILE *stdio_file;
+    QEMUFileStdio *s;
+
+    stdio_file = popen(command, mode);
+    if (stdio_file == NULL) {
+        return NULL;
+    }
+
+    if (mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] != 0) {
+        fprintf(stderr, "qemu_popen: Argument validity check failed\n");
+        return NULL;
+    }
+
+    s = g_malloc0(sizeof(QEMUFileStdio));
+
+    s->stdio_file = stdio_file;
+
+    if(mode[0] == 'r') {
+        s->file = qemu_fopen_ops(s, &stdio_pipe_read_ops);
+    } else {
+        s->file = qemu_fopen_ops(s, &stdio_pipe_write_ops);
+    }
+    return s->file;
+}
+
+static const QEMUFileOps stdio_file_read_ops = {
+    .get_fd =     stdio_get_fd,
+    .get_buffer = stdio_get_buffer,
+    .close =      stdio_fclose
+};
+
+static const QEMUFileOps stdio_file_write_ops = {
+    .get_fd =     stdio_get_fd,
+    .put_buffer = stdio_put_buffer,
+    .close =      stdio_fclose
+};
+
+QEMUFile *qemu_fdopen(int fd, const char *mode)
+{
+    QEMUFileStdio *s;
+
+    if (mode == NULL ||
+	(mode[0] != 'r' && mode[0] != 'w') ||
+	mode[1] != 'b' || mode[2] != 0) {
+        fprintf(stderr, "qemu_fdopen: Argument validity check failed\n");
+        return NULL;
+    }
+
+    s = g_malloc0(sizeof(QEMUFileStdio));
+    s->stdio_file = fdopen(fd, mode);
+    if (!s->stdio_file)
+        goto fail;
+
+    if(mode[0] == 'r') {
+        s->file = qemu_fopen_ops(s, &stdio_file_read_ops);
+    } else {
+        s->file = qemu_fopen_ops(s, &stdio_file_write_ops);
+    }
+    return s->file;
+
+fail:
+    g_free(s);
+    return NULL;
+}
+
+static const QEMUFileOps socket_read_ops = {
+    .get_fd =     socket_get_fd,
+    .get_buffer = socket_get_buffer,
+    .close =      socket_close
+};
+
+static const QEMUFileOps socket_write_ops = {
+    .get_fd =     socket_get_fd,
+    .put_buffer = socket_put_buffer,
+    .close =      socket_close
+};
+
+QEMUFile *qemu_fopen_socket(int fd, const char *mode)
+{
+    QEMUFileSocket *s = g_malloc0(sizeof(QEMUFileSocket));
+
+    if (mode == NULL ||
+        (mode[0] != 'r' && mode[0] != 'w') ||
+        mode[1] != 'b' || mode[2] != 0) {
+        fprintf(stderr, "qemu_fopen: Argument validity check failed\n");
+        return NULL;
+    }
+
+    s->fd = fd;
+    if (mode[0] == 'w') {
+        socket_set_block(s->fd);
+        s->file = qemu_fopen_ops(s, &socket_write_ops);
+    } else {
+        s->file = qemu_fopen_ops(s, &socket_read_ops);
+    }
+    return s->file;
+}
+
+QEMUFile *qemu_fopen(const char *filename, const char *mode)
+{
+    QEMUFileStdio *s;
+
+    if (mode == NULL ||
+	(mode[0] != 'r' && mode[0] != 'w') ||
+	mode[1] != 'b' || mode[2] != 0) {
+        fprintf(stderr, "qemu_fopen: Argument validity check failed\n");
+        return NULL;
+    }
+
+    s = g_malloc0(sizeof(QEMUFileStdio));
+
+    s->stdio_file = fopen(filename, mode);
+    if (!s->stdio_file)
+        goto fail;
+
+    if(mode[0] == 'w') {
+        s->file = qemu_fopen_ops(s, &stdio_file_write_ops);
+    } else {
+        s->file = qemu_fopen_ops(s, &stdio_file_read_ops);
+    }
+    return s->file;
+fail:
+    g_free(s);
+    return NULL;
+}
+
+
+QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops)
+{
+    QEMUFile *f;
+
+    f = g_malloc0(sizeof(QEMUFile));
+
+    f->opaque = opaque;
+    f->ops = ops;
+    f->is_write = 0;
+    return f;
+}
+
+int qemu_file_get_error(QEMUFile *f)
+{
+    return f->last_error;
+}
+
+void qemu_file_set_error(QEMUFile *f, int ret)
+{
+    if (f->last_error == 0) {
+        f->last_error = ret;
+    }
+}
+
+/** Flushes QEMUFile buffer
+ *
+ */
+void qemu_fflush(QEMUFile *f)
+{
+    int ret = 0;
+
+    if (!f->ops->put_buffer) {
+        return;
+    }
+    if (f->is_write && f->buf_index > 0) {
+        ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index);
+        if (ret >= 0) {
+            f->pos += f->buf_index;
+        }
+        f->buf_index = 0;
+    }
+    if (ret < 0) {
+        qemu_file_set_error(f, ret);
+    }
+}
+
+static void qemu_fill_buffer(QEMUFile *f)
+{
+    int len;
+    int pending;
+
+    if (!f->ops->get_buffer)
+        return;
+
+    if (f->is_write)
+        abort();
+
+    pending = f->buf_size - f->buf_index;
+    if (pending > 0) {
+        memmove(f->buf, f->buf + f->buf_index, pending);
+    }
+    f->buf_index = 0;
+    f->buf_size = pending;
+
+    len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
+                        IO_BUF_SIZE - pending);
+    if (len > 0) {
+        f->buf_size += len;
+        f->pos += len;
+    } else if (len == 0) {
+        qemu_file_set_error(f, -EIO);
+    } else if (len != -EAGAIN)
+        qemu_file_set_error(f, len);
+}
+
+int qemu_get_fd(QEMUFile *f)
+{
+    if (f->ops->get_fd) {
+        return f->ops->get_fd(f->opaque);
+    }
+    return -1;
+}
+
+/** Closes the file
+ *
+ * Returns negative error value if any error happened on previous operations or
+ * while closing the file. Returns 0 or positive number on success.
+ *
+ * The meaning of return value on success depends on the specific backend
+ * being used.
+ */
+int qemu_fclose(QEMUFile *f)
+{
+    int ret;
+    qemu_fflush(f);
+    ret = qemu_file_get_error(f);
+
+    if (f->ops->close) {
+        int ret2 = f->ops->close(f->opaque);
+        if (ret >= 0) {
+            ret = ret2;
+        }
+    }
+    /* If any error was spotted before closing, we should report it
+     * instead of the close() return value.
+     */
+    if (f->last_error) {
+        ret = f->last_error;
+    }
+    g_free(f);
+    return ret;
+}
+
+void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
+{
+    int l;
+
+    if (f->last_error) {
+        return;
+    }
+
+    if (f->is_write == 0 && f->buf_index > 0) {
+        fprintf(stderr,
+                "Attempted to write to buffer while read buffer is not empty\n");
+        abort();
+    }
+
+    while (size > 0) {
+        l = IO_BUF_SIZE - f->buf_index;
+        if (l > size)
+            l = size;
+        memcpy(f->buf + f->buf_index, buf, l);
+        f->is_write = 1;
+        f->buf_index += l;
+        f->bytes_xfer += l;
+        buf += l;
+        size -= l;
+        if (f->buf_index >= IO_BUF_SIZE) {
+            qemu_fflush(f);
+            if (qemu_file_get_error(f)) {
+                break;
+            }
+        }
+    }
+}
+
+void qemu_put_byte(QEMUFile *f, int v)
+{
+    if (f->last_error) {
+        return;
+    }
+
+    if (f->is_write == 0 && f->buf_index > 0) {
+        fprintf(stderr,
+                "Attempted to write to buffer while read buffer is not empty\n");
+        abort();
+    }
+
+    f->buf[f->buf_index++] = v;
+    f->is_write = 1;
+    if (f->buf_index >= IO_BUF_SIZE) {
+        qemu_fflush(f);
+    }
+}
+
+void qemu_file_skip(QEMUFile *f, int size)
+{
+    if (f->buf_index + size <= f->buf_size) {
+        f->buf_index += size;
+    }
+}
+
+int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
+{
+    int pending;
+    int index;
+
+    if (f->is_write) {
+        abort();
+    }
+
+    index = f->buf_index + offset;
+    pending = f->buf_size - index;
+    if (pending < size) {
+        qemu_fill_buffer(f);
+        index = f->buf_index + offset;
+        pending = f->buf_size - index;
+    }
+
+    if (pending <= 0) {
+        return 0;
+    }
+    if (size > pending) {
+        size = pending;
+    }
+
+    memcpy(buf, f->buf + index, size);
+    return size;
+}
+
+int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
+{
+    int pending = size;
+    int done = 0;
+
+    while (pending > 0) {
+        int res;
+
+        res = qemu_peek_buffer(f, buf, pending, 0);
+        if (res == 0) {
+            return done;
+        }
+        qemu_file_skip(f, res);
+        buf += res;
+        pending -= res;
+        done += res;
+    }
+    return done;
+}
+
+int qemu_peek_byte(QEMUFile *f, int offset)
+{
+    int index = f->buf_index + offset;
+
+    if (f->is_write) {
+        abort();
+    }
+
+    if (index >= f->buf_size) {
+        qemu_fill_buffer(f);
+        index = f->buf_index + offset;
+        if (index >= f->buf_size) {
+            return 0;
+        }
+    }
+    return f->buf[index];
+}
+
+int qemu_get_byte(QEMUFile *f)
+{
+    int result;
+
+    result = qemu_peek_byte(f, 0);
+    qemu_file_skip(f, 1);
+    return result;
+}
+
+int64_t qemu_ftell(QEMUFile *f)
+{
+    qemu_fflush(f);
+    return f->pos;
+}
+
+int qemu_file_rate_limit(QEMUFile *f)
+{
+    if (qemu_file_get_error(f)) {
+        return 1;
+    }
+    if (f->xfer_limit > 0 && f->bytes_xfer > f->xfer_limit) {
+        return 1;
+    }
+    return 0;
+}
+
+int64_t qemu_file_get_rate_limit(QEMUFile *f)
+{
+    return f->xfer_limit;
+}
+
+void qemu_file_set_rate_limit(QEMUFile *f, int64_t limit)
+{
+    f->xfer_limit = limit;
+}
+
+void qemu_file_reset_rate_limit(QEMUFile *f)
+{
+    f->bytes_xfer = 0;
+}
+
+void qemu_put_be16(QEMUFile *f, unsigned int v)
+{
+    qemu_put_byte(f, v >> 8);
+    qemu_put_byte(f, v);
+}
+
+void qemu_put_be32(QEMUFile *f, unsigned int v)
+{
+    qemu_put_byte(f, v >> 24);
+    qemu_put_byte(f, v >> 16);
+    qemu_put_byte(f, v >> 8);
+    qemu_put_byte(f, v);
+}
+
+void qemu_put_be64(QEMUFile *f, uint64_t v)
+{
+    qemu_put_be32(f, v >> 32);
+    qemu_put_be32(f, v);
+}
+
+unsigned int qemu_get_be16(QEMUFile *f)
+{
+    unsigned int v;
+    v = qemu_get_byte(f) << 8;
+    v |= qemu_get_byte(f);
+    return v;
+}
+
+unsigned int qemu_get_be32(QEMUFile *f)
+{
+    unsigned int v;
+    v = qemu_get_byte(f) << 24;
+    v |= qemu_get_byte(f) << 16;
+    v |= qemu_get_byte(f) << 8;
+    v |= qemu_get_byte(f);
+    return v;
+}
+
+uint64_t qemu_get_be64(QEMUFile *f)
+{
+    uint64_t v;
+    v = (uint64_t)qemu_get_be32(f) << 32;
+    v |= qemu_get_be32(f);
+    return v;
+}
+
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 2/9] qapi_c_arrays.diff
  2013-03-13 18:56 [Qemu-devel] [PATCH 0/9 v3] Implement and test asn1 ber visitors Joel Schopp
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 1/9] qemu-file Joel Schopp
@ 2013-03-13 18:56 ` Joel Schopp
  2013-03-13 19:11   ` Anthony Liguori
  2013-03-13 22:54   ` Stefan Berger
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 3/9] two new file wrappers Joel Schopp
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 33+ messages in thread
From: Joel Schopp @ 2013-03-13 18:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joel Schopp, Paolo Bonzini, Michael Roth, Michael Tsirkin

Forward ported Mike's previously sent patch
(see http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg05782.html ) in my
series since it implements a qapi array interface the ASN.1 BER visitor needs.

Generally these will be serialized into lists, but the
representation can be of any form so long as it can
be deserialized into a single-dimension C array.

Cc: Michael Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 include/qapi/visitor-impl.h |    4 ++++
 include/qapi/visitor.h      |    4 ++++
 qapi/qapi-visit-core.c      |   25 +++++++++++++++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 5159964..9d87f2d 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -34,6 +34,10 @@ struct Visitor
     void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
     void (*type_number)(Visitor *v, double *obj, const char *name,
                         Error **errp);
+    void (*start_carray)(Visitor *v, void **obj, const char *name,
+                         size_t elem_count, size_t elem_size, Error **errp);
+    void (*next_carray)(Visitor *v, Error **errp);
+    void (*end_carray)(Visitor *v, Error **errp);
 
     /* May be NULL */
     void (*start_optional)(Visitor *v, bool *present, const char *name,
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 1fef18c..74bddef 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -51,5 +51,9 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
+void visit_start_carray(Visitor *v, void **obj, const char *name,
+                        size_t elem_count, size_t elem_size, Error **errp);
+void visit_next_carray(Visitor *v, Error **errp);
+void visit_end_carray(Visitor *v, Error **errp);
 
 #endif
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 401ee6e..d9982f8 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -313,3 +313,28 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
     g_free(enum_str);
     *obj = value;
 }
+
+void visit_start_carray(Visitor *v, void **obj, const char *name,
+                        size_t elem_count, size_t elem_size, Error **errp)
+{
+    g_assert(v->start_carray);
+    if (!error_is_set(errp)) {
+        v->start_carray(v, obj, name, elem_count, elem_size, errp);
+    }
+}
+
+void visit_next_carray(Visitor *v, Error **errp)
+{
+    g_assert(v->next_carray);
+    if (!error_is_set(errp)) {
+        v->next_carray(v, errp);
+    }
+}
+
+void visit_end_carray(Visitor *v, Error **errp)
+{
+    g_assert(v->end_carray);
+    if (!error_is_set(errp)) {
+        v->end_carray(v, errp);
+    }
+}
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 3/9] two new file wrappers
  2013-03-13 18:56 [Qemu-devel] [PATCH 0/9 v3] Implement and test asn1 ber visitors Joel Schopp
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 1/9] qemu-file Joel Schopp
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 2/9] qapi_c_arrays.diff Joel Schopp
@ 2013-03-13 18:56 ` Joel Schopp
  2013-03-13 21:04   ` Eric Blake
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 4/9] qemu_qsb.diff Joel Schopp
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 33+ messages in thread
From: Joel Schopp @ 2013-03-13 18:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joel Schopp, Stefan Berger, Michael Tsirkin

Add a 3 very short file wrapper functions to make code that follows more
readable.  Also export an existing function that is currently static.

Cc: Michael Tsirkin <mst@redhat.com>
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 |    3 +++
 util/qemu-file.c              |   30 ++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index b76a901..07d8362 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -68,6 +68,9 @@ int qemu_fclose(QEMUFile *f);
 int64_t qemu_ftell(QEMUFile *f);
 void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
 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);
 
 static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
 {
diff --git a/util/qemu-file.c b/util/qemu-file.c
index 44b52b4..e698713 100644
--- a/util/qemu-file.c
+++ b/util/qemu-file.c
@@ -680,3 +680,33 @@ uint64_t qemu_get_be64(QEMUFile *f)
     return v;
 }
 
+int qemu_read_bytes(QEMUFile *f, uint8_t *buf, int size)
+{
+    if (qemu_file_get_error(f)) {
+        return -1;
+    }
+    return qemu_get_buffer(f, buf, size);
+}
+
+int qemu_peek_bytes(QEMUFile *f, uint8_t *buf, int size, size_t offset)
+{
+    if (qemu_file_get_error(f)) {
+        return -1;
+    }
+    return qemu_peek_buffer(f, buf, size, offset);
+}
+
+int qemu_write_bytes(QEMUFile *f, const uint8_t *buf, int size)
+{
+    if (qemu_file_get_error(f)) {
+        return -1;
+    }
+
+    qemu_put_buffer(f, buf, size);
+
+    if (qemu_file_get_error(f)) {
+        return -1;
+    }
+
+    return size;
+}
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 4/9] qemu_qsb.diff
  2013-03-13 18:56 [Qemu-devel] [PATCH 0/9 v3] Implement and test asn1 ber visitors Joel Schopp
                   ` (2 preceding siblings ...)
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 3/9] two new file wrappers Joel Schopp
@ 2013-03-13 18:56 ` Joel Schopp
  2013-03-13 21:11   ` mdroth
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 5/9] qapi_sized_buffer Joel Schopp
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 33+ messages in thread
From: Joel Schopp @ 2013-03-13 18:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joel Schopp, Stefan Berger

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(+)

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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-13 18:56 [Qemu-devel] [PATCH 0/9 v3] Implement and test asn1 ber visitors Joel Schopp
                   ` (3 preceding siblings ...)
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 4/9] qemu_qsb.diff Joel Schopp
@ 2013-03-13 18:56 ` Joel Schopp
  2013-03-13 20:52   ` mdroth
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 6/9] asn1_output-visitor.diff Joel Schopp
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 33+ messages in thread
From: Joel Schopp @ 2013-03-13 18:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joel Schopp, Stefan Berger, Michael Tsirkin

Add a sized buffer interface to qapi.

Cc: Michael Tsirkin <mst@redhat.com>
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 include/qapi/visitor-impl.h |    2 ++
 include/qapi/visitor.h      |    2 ++
 qapi/qapi-visit-core.c      |    8 ++++++++
 3 files changed, 12 insertions(+)

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 9d87f2d..dc0e25c 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -38,6 +38,8 @@ struct Visitor
                          size_t elem_count, size_t elem_size, Error **errp);
     void (*next_carray)(Visitor *v, Error **errp);
     void (*end_carray)(Visitor *v, Error **errp);
+    void (*type_sized_buffer)(Visitor *v, uint8_t **obj, size_t size,
+                              const char *name, Error **errp);
 
     /* May be NULL */
     void (*start_optional)(Visitor *v, bool *present, const char *name,
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 74bddef..7c7bb98 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -55,5 +55,7 @@ void visit_start_carray(Visitor *v, void **obj, const char *name,
                         size_t elem_count, size_t elem_size, Error **errp);
 void visit_next_carray(Visitor *v, Error **errp);
 void visit_end_carray(Visitor *v, Error **errp);
+void visit_type_sized_buffer(Visitor *v, uint8_t **obj, size_t len,
+                             const char *name, Error **errp);
 
 #endif
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index d9982f8..4b36a54 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -338,3 +338,11 @@ void visit_end_carray(Visitor *v, Error **errp)
         v->end_carray(v, errp);
     }
 }
+
+void visit_type_sized_buffer(Visitor *v, uint8_t **obj, size_t len,
+                             const char *name, Error **errp)
+{
+    if (!error_is_set(errp)) {
+        v->type_sized_buffer(v, obj, len, name, errp);
+    }
+}
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 6/9] asn1_output-visitor.diff
  2013-03-13 18:56 [Qemu-devel] [PATCH 0/9 v3] Implement and test asn1 ber visitors Joel Schopp
                   ` (4 preceding siblings ...)
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 5/9] qapi_sized_buffer Joel Schopp
@ 2013-03-13 18:56 ` Joel Schopp
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 7/9] asn1_input-visitor.diff Joel Schopp
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Joel Schopp @ 2013-03-13 18:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joel Schopp, Stefan Berger, Michael Tsirkin

Implement an output visitor for ASN.1 BER encoding.

Cc: Michael Tsirkin <mst@redhat.com>
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 configure                         |    2 +-
 include/qapi/ber-output-visitor.h |   28 ++
 include/qapi/ber.h                |  107 +++++++
 qapi/Makefile.objs                |    1 +
 qapi/ber-common.c                 |   86 ++++++
 qapi/ber-common.h                 |   29 ++
 qapi/ber-output-visitor.c         |  617 +++++++++++++++++++++++++++++++++++++
 7 files changed, 869 insertions(+), 1 deletion(-)
 create mode 100644 include/qapi/ber-output-visitor.h
 create mode 100644 include/qapi/ber.h
 create mode 100644 qapi/ber-common.c
 create mode 100644 qapi/ber-common.h
 create mode 100644 qapi/ber-output-visitor.c

diff --git a/configure b/configure
index a382ff2..46ab8f8 100755
--- a/configure
+++ b/configure
@@ -2830,7 +2830,7 @@ fi
 # Do we need libm
 cat > $TMPC << EOF
 #include <math.h>
-int main(void) { return isnan(sin(0.0)); }
+int main(void) { return isnan(nan("NAN")); }
 EOF
 if compile_prog "" "" ; then
   :
diff --git a/include/qapi/ber-output-visitor.h b/include/qapi/ber-output-visitor.h
new file mode 100644
index 0000000..bd7e198
--- /dev/null
+++ b/include/qapi/ber-output-visitor.h
@@ -0,0 +1,28 @@
+/*
+ * BER Output Visitor header
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *  Stefan Berger     <stefanb@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef BER_OUTPUT_VISITOR_H
+#define BER_OUTPUT_VISITOR_H
+
+#include "qapi/visitor.h"
+#include "qapi/ber.h"
+
+typedef struct BEROutputVisitor BEROutputVisitor;
+
+BEROutputVisitor *ber_output_visitor_new(QEMUFile *, BERTypePC mode);
+void ber_output_visitor_cleanup(BEROutputVisitor *v);
+
+Visitor *ber_output_get_visitor(BEROutputVisitor *v);
+
+#endif
diff --git a/include/qapi/ber.h b/include/qapi/ber.h
new file mode 100644
index 0000000..9314b28
--- /dev/null
+++ b/include/qapi/ber.h
@@ -0,0 +1,107 @@
+/*
+ * ASN.1 Basic Encoding Rules Common functions
+ *
+ * Copyright IBM, Corp. 2011, 2013
+ * Copyright Red Hat, Inc. 2011
+ *
+ * Authors:
+ *  Stefan Berger     <stefanb@us.ibm.com>
+ *  Michael Tsirkin   <mst@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+#ifndef QAPI_BER_H
+#define QAPI_BER_H
+
+/*
+ * This is a subset of BER for QEMU use.
+ * QEMU will use the DER encoding always with one extension from
+ * CER: SET and SEQUENCE types can have indefinite-length encoding
+ * if the encoding is not all immediately available.
+ *
+ * We assume that SET encodings can be available or not available,
+ * and that SEQUENCE encodings are available unless a SEQUENCE includes
+ * a non-available SET.
+ *
+ * The last is an extension to allow an arbitrarily large SET
+ * to be produced online without knowing the length in advance.
+ *
+ * All types used shall be universal, with explicit tagging, to simplify
+ * use by external tools.
+ */
+
+
+#define BER_TYPE_CLASS_SHIFT  6
+#define BER_TYPE_PC_SHIFT     5
+
+typedef enum ber_type_class {
+    BER_TYPE_CLASS_UNIVERSAL = 0x0 << BER_TYPE_CLASS_SHIFT,
+    BER_TYPE_CLASS_APPLICATION = 0x1 << BER_TYPE_CLASS_SHIFT,
+    BER_TYPE_CLASS_CONTENT_SPECIFIC = 0x2 << BER_TYPE_CLASS_SHIFT,
+    BER_TYPE_CLASS_PRIVATE = 0x3 << BER_TYPE_CLASS_SHIFT,
+    BER_TYPE_CLASS_MASK = 0x3 << BER_TYPE_CLASS_SHIFT /* Mask to get class */
+} BERTypeClass;
+
+/* P/C bit */
+typedef enum ber_type_p_c {
+    BER_TYPE_PRIMITIVE = 0x0 << BER_TYPE_PC_SHIFT,
+    BER_TYPE_CONSTRUCTED = 0x1 << BER_TYPE_PC_SHIFT,
+    BER_TYPE_P_C_MASK = 0x1 << BER_TYPE_PC_SHIFT /* Mask to get P/C bit */
+} BERTypePC;
+
+typedef enum ber_type_tag {
+    BER_TYPE_EOC              /*  P        0       0*/,
+    BER_TYPE_BOOLEAN          /*  P        1       1*/,
+    BER_TYPE_INTEGER          /*  P        2       2*/,
+    BER_TYPE_BIT_STRING       /*  P/C      3       3*/,
+    BER_TYPE_OCTET_STRING     /*  P/C      4       4*/,
+    BER_TYPE_NULL             /*  P        5       5*/,
+    BER_TYPE_OBJECT_ID        /*  P        6       6*/,
+    BER_TYPE_OBJECT_DESC      /*  P        7       7*/,
+    BER_TYPE_EXTERNAL         /*  C        8       8*/,
+    BER_TYPE_REAL             /*  P        9       9*/,
+    BER_TYPE_ENUMERATED       /*  P        10      A*/,
+    BER_TYPE_EMBEDDED         /*  C        11      B*/,
+    BER_TYPE_UTF8_STRING      /*  P/C      12      C*/,
+    BER_TYPE_RELATIVE_OID     /*  P        13      D*/,
+    BER_TYPE_UNUSED_0xE       /*                    */,
+    BER_TYPE_UNUSED_0xF       /*                    */,
+    BER_TYPE_SEQUENCE         /*  C        16      10*/,
+    BER_TYPE_SET              /*  C        17      11*/,
+    BER_TYPE_NUMERIC_STRING   /*  P/C      18      12*/,
+    BER_TYPE_PRINTABLE_STRING /*  P/C      19      13*/,
+    BER_TYPE_T61STRING        /*  P/C      20      14*/,
+    BER_TYPE_VIDEOTEX_STRING  /*  P/C      21      15*/,
+    BER_TYPE_IA5_STRING       /*  P/C      22      16*/,
+    BER_TYPE_UTCTIME          /*  P/C      23      17*/,
+    BER_TYPE_GENERALIZED_TIME /*  P/C      24      18*/,
+    BER_TYPE_GRAPHIC_STRING   /*  P/C      25      19*/,
+    BER_TYPE_VISIBLE_STRING   /*  P/C      26      1A*/,
+    BER_TYPE_GENERAL_STRING   /*  P/C      27      1B*/,
+    BER_TYPE_UNIVERSAL_STRING /*  P/C      28      1C*/,
+    BER_TYPE_CHARACTER_STRING /*  P/C      29      1D*/,
+    BER_TYPE_BMP_STRING       /*  P/C      30      1E*/,
+    BER_TYPE_LONG_FORM        /*  -        31      1F*/,
+    BER_TYPE_TAG_MASK = 0x1f /* Mask to get tag */,
+    BER_TYPE_CUSTOM_LIST = 0x20,
+} BERTypeTag;
+
+typedef enum ber_length {
+    /* Special length values */
+    BER_LENGTH_INDEFINITE = (0x1 << 7),
+    BER_LENGTH_RESERVED = 0xFF,
+    /* Anything else is either short or long */
+    BER_LENGTH_SHORT = (0x0 << 7),
+    BER_LENGTH_LONG = (0x1 << 7),
+    BER_LENGTH_SHORT_LONG_MASK = (0x1 << 7),
+    BER_LENGTH_MASK = 0x7F,
+} BERLength;
+
+const char *ber_type_to_str(uint8_t ber_type);
+const char *ber_type_pc_to_str(enum ber_type_class ber_type_flags);
+const char *ber_type_class_to_str(enum ber_type_class ber_type_flags);
+
+#endif /* QAPI_BER_H */
+
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 1f9c973..519e3ee 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
 util-obj-y += string-input-visitor.o string-output-visitor.o
 
 util-obj-y += opts-visitor.o
+util-obj-y += ber-common.o ber-output-visitor.o
diff --git a/qapi/ber-common.c b/qapi/ber-common.c
new file mode 100644
index 0000000..1053c41
--- /dev/null
+++ b/qapi/ber-common.c
@@ -0,0 +1,86 @@
+/*
+ * ASN.1 Basic Encoding Rules Common functions
+ *
+ * Copyright IBM, Corp. 2011, 2013
+ * Copyright Red Hat, Inc. 2011
+ *
+ * Authors:
+ *  Stefan Berger     <stefanb@us.ibm.com>
+ *  Michael Tsirkin   <mst@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include <stdint.h>
+
+#include "qapi/ber.h"
+
+static const char *ber_type_names[] = {
+    "BER_TYPE_EOC",
+    "BER_TYPE_BOOLEAN",
+    "BER_TYPE_INTEGER",
+    "BER_TYPE_BIT_STRING",
+    "BER_TYPE_OCTET_STRING",
+    "BER_TYPE_NULL",
+    "BER_TYPE_OBJECT_ID",
+    "BER_TYPE_OBJECT_DESC",
+    "BER_TYPE_EXTERNAL",
+    "BER_TYPE_REAL",
+    "BER_TYPE_ENUMERATED",
+    "BER_TYPE_EMBEDDED",
+    "BER_TYPE_UTF8_STRING",
+    "BER_TYPE_RELATIVE_OID",
+    "BER_TYPE_UNUSED_0xE",
+    "BER_TYPE_UNUSED_0xF",
+    "BER_TYPE_SEQUENCE",
+    "BER_TYPE_SET",
+    "BER_TYPE_NUMERIC_STRING",
+    "BER_TYPE_PRINTABLE_STRING",
+    "BER_TYPE_T61STRING",
+    "BER_TYPE_VIDEOTEX_STRING",
+    "BER_TYPE_IA5_STRING",
+    "BER_TYPE_UTCTIME",
+    "BER_TYPE_GENERALIZED_TIME",
+    "BER_TYPE_GRAPHIC_STRING",
+    "BER_TYPE_VISIBLE_STRING",
+    "BER_TYPE_GENERAL_STRING",
+    "BER_TYPE_UNIVERSAL_STRING",
+    "BER_TYPE_CHARACTER_STRING"
+    "BER_TYPE_BMP_STRING",
+    "BER_TYPE_LONG_FORM",
+};
+
+const char *ber_type_to_str(uint8_t ber_type)
+{
+    return ber_type_names[ber_type & BER_TYPE_TAG_MASK];
+}
+
+static const char *ber_pc_names[] = {
+    "BER_PRIMITIVE",
+    "BER_CONSTRUCTED"
+};
+
+const char *ber_type_pc_to_str(enum ber_type_class ber_type_flags)
+{
+    int idx = (ber_type_flags & BER_TYPE_P_C_MASK) >>
+               BER_TYPE_PC_SHIFT;
+
+    return ber_pc_names[idx];
+}
+
+static const char *ber_class_names[] = {
+    "BER_CLASS_UNIVERSAL",
+    "BER_CLASS_APPLICATION",
+    "BER_CLASS_CONTEXT",
+    "BER_CLASS_PRIVATE"
+};
+
+const char *ber_type_class_to_str(enum ber_type_class ber_type_flags)
+{
+    int idx = (ber_type_flags & BER_TYPE_CLASS_MASK) >>
+               BER_TYPE_CLASS_SHIFT;
+
+    return ber_class_names[idx];
+}
diff --git a/qapi/ber-common.h b/qapi/ber-common.h
new file mode 100644
index 0000000..c9fd2dd
--- /dev/null
+++ b/qapi/ber-common.h
@@ -0,0 +1,29 @@
+/*
+ * BER Visitor -- common code
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ *  Stefan Berger     <stefanb@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+#ifndef __QAPI_BER_COMMON_H__
+#define __QAPI_BER_COMMON_H__
+
+#include <stdint.h>
+
+#include "qemu/compiler.h"
+
+struct ieee754_buffer {
+    uint8_t type;
+    uint8_t length;
+    uint8_t first;
+    uint16_t exponent;
+    uint32_t mant_hi;
+    uint32_t mant_lo;
+} QEMU_PACKED;
+
+#endif /* __QAPI_BER_COMMON_H__ */
diff --git a/qapi/ber-output-visitor.c b/qapi/ber-output-visitor.c
new file mode 100644
index 0000000..f32469d
--- /dev/null
+++ b/qapi/ber-output-visitor.c
@@ -0,0 +1,617 @@
+/*
+ * BER Output Visitor
+ *
+ * Copyright IBM, Corp. 2011, 2013
+ * Copyright Red Hat, Inc. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *  Stefan Berger     <stefanb@us.ibm.com>
+ *  Michael Tsirkin   <mst@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include <math.h>
+#include <glib.h>
+
+#include "qemu-common.h"
+#include "qapi/ber-common.h"
+#include "qapi/ber-output-visitor.h"
+#include "qemu/queue.h"
+#include "qemu-common.h"
+#include "include/qapi/qmp/qerror.h"
+#include "hw/hw.h"
+#include "qapi/ber.h"
+#include "qapi/visitor-impl.h"
+
+
+#define CER_FRAGMENT_CHUNK_SIZE  1000
+
+/*#define BER_DEBUG*/
+
+typedef struct QStackEntry {
+    QEMUFile *qfile;
+    bool is_list_head;
+    QTAILQ_ENTRY(QStackEntry) node;
+} QStackEntry;
+
+typedef QTAILQ_HEAD(QStack, QStackEntry) QStack;
+
+struct BEROutputVisitor {
+    Visitor visitor;
+    QStack stack;
+    QEMUFile *qfile;
+
+    BERTypePC mode;
+};
+
+static BEROutputVisitor *to_aov(Visitor *v)
+{
+    return container_of(v, BEROutputVisitor, visitor);
+}
+
+static void ber_output_push(BEROutputVisitor *qov, QEMUFile *qfile,
+                            Error **errp)
+{
+    QStackEntry *e = g_malloc0(sizeof(*e));
+
+    e->qfile = qfile;
+    e->is_list_head = true;
+    QTAILQ_INSERT_HEAD(&qov->stack, e, node);
+}
+
+static QEMUFile *ber_output_pop(BEROutputVisitor *qov)
+{
+    QStackEntry *e = QTAILQ_FIRST(&qov->stack);
+    QEMUFile *qfile;
+
+    QTAILQ_REMOVE(&qov->stack, e, node);
+    qfile = e->qfile;
+    g_free(e);
+
+    return qfile;
+}
+
+static unsigned int ber_encode_type(uint8_t *buffer, uint32_t buflen,
+                                    enum ber_type_tag ber_type,
+                                    uint8_t ber_type_flags,
+                                    Error **errp)
+{
+    unsigned int idx = 0;
+
+    if (buflen < 1) {
+        error_set(errp, QERR_BUFFER_OVERRUN);
+        return 0;
+    }
+
+    if (ber_type > BER_TYPE_LONG_FORM) {
+        int byte = 4;
+        uint32_t mask = (0x7f << (7 * byte));
+        bool do_write = false;
+
+        buffer[0] = ber_type_flags | BER_TYPE_LONG_FORM;
+
+        while (byte >= 0) {
+            if (!do_write) {
+                if ((mask & ber_type)) {
+                    do_write = true;
+                    if (1 + byte + 1 > buflen) {
+                        error_set(errp, QERR_BUFFER_OVERRUN);
+                        return 0;
+                    }
+                }
+            }
+            if (do_write) {
+                buffer[1 + idx] = (ber_type >> (7 * byte)) & 0x7f;
+                if (byte > 0) {
+                    buffer[1 + idx] |= 0x80;
+                }
+                idx++;
+            }
+            byte--;
+            mask =  (0x7f << (7 * byte));
+        }
+    } else {
+        buffer[0] = ber_type | ber_type_flags;
+    }
+    return 1 + idx;
+}
+
+static unsigned int ber_encode_len(uint8_t *buffer, uint32_t buflen,
+                                   uint64_t len, Error **errp)
+{
+    uint64_t mask = 0xFF00000000000000ULL;
+    int shift =  64 - 8;
+    int c = 0;
+
+    if (len <= 0x7f && buflen >= 1) {
+        buffer[0] = len;
+        return 1;
+    }
+
+    while (mask && (mask & len) == 0) {
+        mask >>= 8;
+        shift -= 8;
+    }
+
+    while (shift >= 0) {
+        if (1 + c + 1 > buflen) {
+            error_set(errp, QERR_BUFFER_OVERRUN);
+            return 0;
+        }
+        buffer[1+c] = (len >> shift);
+        c++;
+        shift -= 8;
+    }
+
+    buffer[0] = BER_LENGTH_LONG | c;
+
+    return 1 + c;
+}
+
+static void ber_output_start_constructed(Visitor *v, uint32_t ber_type,
+                                         Error **errp)
+{
+    BEROutputVisitor *aov = to_aov(v);
+    uint8_t buf[20];
+    unsigned int tag_bytes_written;
+
+    switch (aov->mode) {
+    case BER_TYPE_PRIMITIVE:
+        ber_output_push(aov, aov->qfile, errp);
+        if (error_is_set(errp)) {
+            return;
+        }
+        aov->qfile = qemu_bufopen("w", NULL);
+        break;
+    case BER_TYPE_CONSTRUCTED:
+        ber_output_push(aov, aov->qfile, errp); /* needed for list support */
+        if (error_is_set(errp)) {
+            return;
+        }
+        tag_bytes_written = ber_encode_type(buf, sizeof(buf),
+                                            ber_type, BER_TYPE_CONSTRUCTED,
+                                            errp);
+        if (error_is_set(errp)) {
+            return;
+        }
+        buf[tag_bytes_written] = BER_LENGTH_INDEFINITE;
+        if (qemu_write_bytes(aov->qfile, buf, 1 + tag_bytes_written) !=
+            1 + tag_bytes_written) {
+            error_setg(errp, "QEMUFile error: Error while writing constructed type");
+            return;
+        }
+    }
+}
+
+static void ber_output_constructed_ber_close(BEROutputVisitor *aov,
+                                             uint32_t ber_type,
+                                             Error **errp)
+{
+    uint8_t buf[20];
+    const QEMUSizedBuffer *qsb;
+    uint64_t len;
+    unsigned int num_bytes, tag_bytes_written;
+    QEMUFile *qfile = ber_output_pop(aov);
+
+    tag_bytes_written = ber_encode_type(buf, sizeof(buf),
+                                        ber_type, BER_TYPE_CONSTRUCTED,
+                                        errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    qsb = qemu_buf_get(aov->qfile);
+    len = qsb_get_length(qsb);
+#ifdef BER_DEBUG
+    fprintf(stderr, "constructed type (0x%02x, %p) has length %ld bytes\n",
+            ber_type, aov->qfile, len);
+#endif
+
+    num_bytes = ber_encode_len(&buf[tag_bytes_written],
+                               sizeof(buf) - tag_bytes_written,
+                               len, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+    if (qemu_write_bytes(qfile, buf, tag_bytes_written + num_bytes) !=
+        tag_bytes_written + num_bytes ||
+        qemu_write_bytes(qfile, qsb_get_buffer(qsb, 0),
+                         qsb_get_length(qsb)) != qsb_get_length(qsb)) {
+        error_setg(errp, "QEMUFile error: Error while writing buffer");
+        return;
+    }
+
+    qemu_fclose(aov->qfile);
+    aov->qfile = qfile;
+    qemu_fflush(qfile);
+}
+
+static void ber_output_end_constructed(Visitor *v, uint32_t ber_type,
+                                       Error **errp)
+{
+    BEROutputVisitor *aov = to_aov(v);
+    uint8_t buf[20];
+
+#ifdef BER_DEBUG
+    fprintf(stderr, "end set/struct:\n");
+#endif
+
+    switch (aov->mode) {
+    case BER_TYPE_PRIMITIVE:
+        ber_output_constructed_ber_close(aov, ber_type, errp);
+        break;
+
+    case BER_TYPE_CONSTRUCTED:
+        ber_output_pop(aov);
+        buf[0] = BER_TYPE_EOC;
+        buf[1] = 0;
+        if (qemu_write_bytes(aov->qfile, buf, 2) != 2) {
+            error_setg(errp, "QEMUFile error: Error while writing buffer with BER_TYPE_EOC");
+            return;
+        }
+        break;
+    }
+}
+
+static void ber_output_start_struct(Visitor *v, void **obj, const char *kind,
+                                    const char *name, size_t unused,
+                                    Error **errp)
+{
+    ber_output_start_constructed(v, BER_TYPE_SEQUENCE, errp);
+}
+
+static void ber_output_end_struct(Visitor *v, Error **errp)
+{
+    ber_output_end_constructed(v, BER_TYPE_SEQUENCE, errp);
+}
+
+static void ber_output_start_carray(Visitor *v, void **obj,
+                                   const char *name, size_t elem_count,
+                                   size_t elem_size, Error **errp)
+{
+    ber_output_start_constructed(v, BER_TYPE_SET, errp);
+}
+
+static void ber_output_next_carray(Visitor *v, Error **errp)
+{
+    /* nothing to do here */
+}
+
+static void ber_output_end_carray(Visitor *v, Error **errp)
+{
+    ber_output_end_constructed(v, BER_TYPE_SET, errp);
+}
+
+static void ber_output_start_list(Visitor *v, const char *name,
+                                  Error **errp)
+{
+    ber_output_start_constructed(v, BER_TYPE_CUSTOM_LIST, errp);
+}
+
+static GenericList *ber_output_next_list(Visitor *v, GenericList **listp,
+                                         Error **errp)
+{
+    GenericList *list = *listp;
+    BEROutputVisitor *bov = to_aov(v);
+    QStackEntry *e = QTAILQ_FIRST(&bov->stack);
+
+    assert(e);
+    if (e->is_list_head) {
+        e->is_list_head = false;
+        return list;
+    }
+
+    return list ? list->next : NULL;
+}
+
+static void ber_output_end_list(Visitor *v, Error **errp)
+{
+    ber_output_end_constructed(v, BER_TYPE_CUSTOM_LIST, errp);
+}
+
+static void ber_output_fragment(Visitor *v, uint32_t ber_type,
+                                uint8_t *buffer,
+                                size_t buflen, Error **errp)
+{
+    uint32_t offset = 0;
+    bool fragmented = false;
+    uint32_t chunk;
+    unsigned int num_bytes, type_bytes;
+    uint8_t buf[20];
+    uint32_t chunk_size;
+    BEROutputVisitor *aov = to_aov(v);
+
+    switch (aov->mode) {
+    case BER_TYPE_CONSTRUCTED:
+        /* X.690 9.2 */
+        fragmented = (buflen > CER_FRAGMENT_CHUNK_SIZE);
+        chunk_size = 1000;
+        break;
+    case BER_TYPE_PRIMITIVE:
+        chunk_size = 0xffffffff;
+        break;
+    }
+
+    if (fragmented) {
+        ber_output_start_constructed(&aov->visitor, ber_type, errp);
+        if (error_is_set(errp)) {
+            return;
+        }
+    }
+
+    do {
+        chunk = (buflen - offset > chunk_size) ? chunk_size : buflen - offset;
+
+        type_bytes = ber_encode_type(buf, sizeof(buf), ber_type, 0,
+                                     errp);
+        if (error_is_set(errp)) {
+            return;
+        }
+        num_bytes = ber_encode_len(&buf[type_bytes], sizeof(buf) - type_bytes,
+                                   chunk, errp);
+        if (error_is_set(errp)) {
+            return;
+        }
+        if (qemu_write_bytes(aov->qfile, buf, type_bytes + num_bytes) !=
+            type_bytes + num_bytes ||
+            qemu_write_bytes(aov->qfile, &buffer[offset], chunk) != chunk) {
+            error_setg(errp, "QEMUFile error: Error while writing buffer");
+            return;
+        }
+        offset += chunk;
+    } while (offset < buflen);
+
+    if (fragmented) {
+        ber_output_end_constructed(&aov->visitor, ber_type, errp);
+    }
+}
+
+static void ber_output_int(Visitor *v, int64_t val, uint8_t maxnumbytes,
+                           Error **errp)
+{
+    uint8_t buf[20];
+    int shift =  (maxnumbytes - 1) * 8;
+    uint64_t mask = 0xFF80ULL << (shift - 8);
+    bool exp_zeros;
+    int c = 0;
+    BEROutputVisitor *aov = to_aov(v);
+
+#ifdef BER_DEBUG
+    fprintf(stderr, "Writing int 0x%lx (signed=%d, len=%d)\n",
+            val, is_signed, maxnumbytes);
+#endif
+
+    /*
+     * We encode ints with fixed-witdh so that they will use
+     * the same number of bytes indepent of their value.
+     * The 'universal' encoding would not encode a 32bit '0'
+     * with 4 bytes, so this is an application-specific encoding.
+     */
+    buf[0] = BER_TYPE_CLASS_APPLICATION | BER_TYPE_PRIMITIVE |
+             BER_TYPE_INTEGER;
+
+    if (maxnumbytes > 1) {
+        exp_zeros = ((mask & val) == 0) ? true : false;
+        while (mask != 0xFF) {
+            if (exp_zeros) {
+                if ((mask & val) != 0) {
+                    break;
+                }
+            } else {
+                if ((mask & val) != mask) {
+                    break;
+                }
+            }
+            shift -= 8;
+            mask >>= 8;
+        }
+    }
+
+    while (shift >= 0) {
+        buf[2+c] = (val >> shift);
+        c++;
+        shift -= 8;
+    }
+    buf[1] = c;
+
+    if (qemu_write_bytes(aov->qfile, buf, 1 + 1 + c) != 1 + 1 + c) {
+        error_setg(errp, "QEMUFile error: Error while writing integer");
+        return;
+    }
+}
+static void ber_output_type_int(Visitor *v, int64_t *obj, const char *name,
+                                Error **errp)
+{
+    ber_output_int(v, *obj, sizeof(*obj), errp);
+}
+
+static void ber_output_type_uint8(Visitor *v, uint8_t *obj,
+                                    const char *name, Error **errp)
+{
+    ber_output_int(v, *obj, sizeof(*obj), errp);
+}
+
+static void ber_output_type_uint16(Visitor *v, uint16_t *obj,
+                                     const char *name, Error **errp)
+{
+    ber_output_int(v, *obj, sizeof(*obj), errp);
+}
+
+static void ber_output_type_uint32(Visitor *v, uint32_t *obj,
+                                     const char *name, Error **errp)
+{
+    ber_output_int(v, *obj, sizeof(*obj), errp);
+}
+
+static void ber_output_type_uint64(Visitor *v, uint64_t *obj,
+                                     const char *name, Error **errp)
+{
+    ber_output_int(v, *obj, sizeof(*obj), errp);
+}
+
+static void ber_output_type_int8(Visitor *v, int8_t *obj,
+                                   const char *name, Error **errp)
+{
+    ber_output_int(v, (int64_t)*obj, sizeof(*obj), errp);
+}
+
+static void ber_output_type_int16(Visitor *v, int16_t *obj,
+                                    const char *name, Error **errp)
+{
+    ber_output_int(v, (int64_t)*obj, sizeof(*obj), errp);
+}
+
+static void ber_output_type_int32(Visitor *v, int32_t *obj,
+                                    const char *name, Error **errp)
+{
+    ber_output_int(v, (int64_t)*obj, sizeof(*obj), errp);
+}
+
+static void ber_output_type_int64(Visitor *v, int64_t *obj,
+                                    const char *name, Error **errp)
+{
+    ber_output_int(v, (int64_t)*obj, sizeof(*obj), errp);
+}
+
+static void ber_output_type_bool(Visitor *v, bool *obj, const char *name,
+                                 Error **errp)
+{
+    BEROutputVisitor *aov = to_aov(v);
+    bool b = 0;
+
+    switch (aov->mode) {
+    case BER_TYPE_PRIMITIVE:
+        b = *obj;
+        break;
+    case BER_TYPE_CONSTRUCTED:
+        b = (*obj) ? 0xff : 0;
+        break;
+    }
+    ber_output_fragment(v, BER_TYPE_BOOLEAN, (uint8_t *)&b, sizeof(b), errp);
+}
+
+static void ber_output_type_str(Visitor *v, char **obj, const char *name,
+                                Error **errp)
+{
+#ifdef BER_DEBUG
+    fprintf(stderr, "Writing string %s, len = 0x%02x\n", *obj,
+            (int)strlen(*obj));
+#endif
+    ber_output_fragment(v, BER_TYPE_IA5_STRING,
+                        (uint8_t *)*obj,
+                        *obj == NULL ? 0 : strlen(*obj), errp);
+}
+
+static void ber_output_sized_buffer(Visitor *v, uint8_t **obj,
+                                    size_t size, const char *name,
+                                    Error **errp)
+{
+    ber_output_fragment(v, BER_TYPE_OCTET_STRING,
+                        *obj, size, errp);
+}
+
+static void ber_output_type_number(Visitor *v, double *obj, const char *name,
+                                   Error **errp)
+{
+    BEROutputVisitor *aov = to_aov(v);
+    GDoubleIEEE754 num;
+    uint8_t first;
+    struct ieee754_buffer number;
+
+    /* encode it as fixed-width double */
+    number.type = BER_TYPE_CLASS_APPLICATION | BER_TYPE_PRIMITIVE |
+                  BER_TYPE_REAL;
+    number.length = sizeof(number) - offsetof(struct ieee754_buffer, first);
+
+    num.v_double = *obj;
+
+    if (isnan(*obj)) {
+        /* special encoding supported here; not found in spec. */
+        first = 0x42;
+    } else if (isinf(*obj)) {
+        /* spec. 8.5.8 */
+        if (num.mpn.sign) {
+            first = 0x41; /* -oo */
+        } else {
+            first = 0x40; /* +oo */
+        }
+    } else {
+        first = 0x80;
+        if (num.mpn.sign) {
+            first |= 0x40;
+        }
+        /* Base 2; 0 for scaling factor; 2nd and 3rd octet encode exp. */
+        first |= 0x1;
+    }
+
+    number.first = first;
+    number.exponent = cpu_to_be16(num.mpn.biased_exponent);
+    number.mant_hi = cpu_to_be32(num.mpn.mantissa_high);
+    number.mant_lo = cpu_to_be32(num.mpn.mantissa_low);
+
+    if (qemu_write_bytes(aov->qfile, (uint8_t *)&number, sizeof(number)) !=
+        sizeof(number)) {
+        error_setg(errp, "QEMUFile error: Error while writing double.");
+    }
+}
+
+void ber_output_visitor_cleanup(BEROutputVisitor *v)
+{
+    QStackEntry *e, *tmp;
+
+    QTAILQ_FOREACH_SAFE(e, &v->stack, node, tmp) {
+        QTAILQ_REMOVE(&v->stack, e, node);
+        if (e->qfile) {
+            qemu_fclose(e->qfile);
+        }
+        g_free(e);
+    }
+
+    g_free(v);
+}
+
+
+Visitor *ber_output_get_visitor(BEROutputVisitor *v)
+{
+    return &v->visitor;
+}
+
+BEROutputVisitor *ber_output_visitor_new(QEMUFile *qfile,
+                                         BERTypePC mode)
+{
+    BEROutputVisitor *v;
+
+    v = g_malloc0(sizeof(*v));
+
+    v->visitor.start_struct = ber_output_start_struct;
+    v->visitor.end_struct = ber_output_end_struct;
+    v->visitor.start_carray = ber_output_start_carray;
+    v->visitor.next_carray = ber_output_next_carray;
+    v->visitor.end_carray = ber_output_end_carray;
+    v->visitor.start_list = ber_output_start_list;
+    v->visitor.next_list = ber_output_next_list;
+    v->visitor.end_list = ber_output_end_list;
+    v->visitor.type_int = ber_output_type_int;
+    v->visitor.type_uint8 = ber_output_type_uint8;
+    v->visitor.type_uint16 = ber_output_type_uint16;
+    v->visitor.type_uint32 = ber_output_type_uint32;
+    v->visitor.type_uint64 = ber_output_type_uint64;
+    v->visitor.type_int8 = ber_output_type_int8;
+    v->visitor.type_int16 = ber_output_type_int16;
+    v->visitor.type_int32 = ber_output_type_int32;
+    v->visitor.type_int64 = ber_output_type_int64;
+    v->visitor.type_bool = ber_output_type_bool;
+    v->visitor.type_str = ber_output_type_str;
+    v->visitor.type_sized_buffer = ber_output_sized_buffer;
+    v->visitor.type_number = ber_output_type_number;
+
+    QTAILQ_INIT(&v->stack);
+    v->qfile = qfile;
+    v->mode = mode;
+
+    return v;
+}
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 7/9] asn1_input-visitor.diff
  2013-03-13 18:56 [Qemu-devel] [PATCH 0/9 v3] Implement and test asn1 ber visitors Joel Schopp
                   ` (5 preceding siblings ...)
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 6/9] asn1_output-visitor.diff Joel Schopp
@ 2013-03-13 18:56 ` Joel Schopp
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 8/9] asn1_test_visitor_serialization.diff Joel Schopp
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 9/9] update_maintainers.diff Joel Schopp
  8 siblings, 0 replies; 33+ messages in thread
From: Joel Schopp @ 2013-03-13 18:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joel Schopp, Stefan Berger, Michael Tsirkin

Implement an input visitor for ASN.1 BER encoding.

Cc: Michael Tsirkin <mst@redhat.com>
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 include/qapi/ber-input-visitor.h |   30 ++
 qapi/Makefile.objs               |    2 +-
 qapi/ber-input-visitor.c         | 1073 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 1104 insertions(+), 1 deletion(-)
 create mode 100644 include/qapi/ber-input-visitor.h
 create mode 100644 qapi/ber-input-visitor.c

diff --git a/include/qapi/ber-input-visitor.h b/include/qapi/ber-input-visitor.h
new file mode 100644
index 0000000..eaa3d0e
--- /dev/null
+++ b/include/qapi/ber-input-visitor.h
@@ -0,0 +1,30 @@
+/*
+ * BER Input Visitor header
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *  Stefan Berger     <stefanb@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef BER_INPUT_VISITOR_H
+#define BER_INPUT_VISITOR_H
+
+#include "qapi/visitor.h"
+
+typedef struct BERInputVisitor BERInputVisitor;
+
+BERInputVisitor *ber_input_visitor_new(QEMUFile *,
+                                       uint64_t max_allowd_buffer_size);
+void ber_input_visitor_cleanup(BERInputVisitor *v);
+uint64_t ber_input_get_parser_position(BERInputVisitor *v);
+uint64_t ber_input_get_largest_needed_buffer(BERInputVisitor *v);
+
+Visitor *ber_input_get_visitor(BERInputVisitor *v);
+
+#endif
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 519e3ee..f7f080a 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -3,4 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
 util-obj-y += string-input-visitor.o string-output-visitor.o
 
 util-obj-y += opts-visitor.o
-util-obj-y += ber-common.o ber-output-visitor.o
+util-obj-y += ber-common.o ber-output-visitor.o ber-input-visitor.o
diff --git a/qapi/ber-input-visitor.c b/qapi/ber-input-visitor.c
new file mode 100644
index 0000000..b17224f
--- /dev/null
+++ b/qapi/ber-input-visitor.c
@@ -0,0 +1,1073 @@
+/*
+ * BER Input Visitor
+ *
+ * Copyright IBM, Corp. 2011, 2013
+ * Copyright Red Hat, Inc. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *  Stefan Berger     <stefanb@us.ibm.com>
+ *  Michael Tsirkin   <mst@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include <math.h>
+#include <glib.h>
+
+#include "qemu-common.h"
+#include "qapi/ber-common.h"
+#include "qapi/ber-input-visitor.h"
+#include "qemu/queue.h"
+#include "qemu-common.h"
+#include "hw/hw.h"
+#include "qapi/ber.h"
+#include "include/qapi/qmp/qerror.h"
+#include "migration/qemu-file.h"
+#include "qapi/visitor-impl.h"
+
+#define AIV_STACK_SIZE 1024
+
+/* whether to allow the parsing of primitives that are fragmented */
+#define BER_ALLOW_FRAGMENTED_PRIMITIVES
+
+/* #define BER_DEBUG */
+
+typedef struct StackEntry {
+    uint64_t cur_pos;
+} StackEntry;
+
+struct BERInputVisitor {
+    Visitor visitor;
+    QEMUFile *qfile;
+    uint64_t cur_pos;
+    StackEntry stack[AIV_STACK_SIZE];
+    int nb_stack;
+    uint64_t max_allowed_buffer_size;
+    uint64_t largest_needed_buffer;
+};
+
+static BERInputVisitor *to_biv(Visitor *v)
+{
+    return container_of(v, BERInputVisitor, visitor);
+}
+
+static void ber_input_push(BERInputVisitor *aiv,
+                           uint64_t cur_pos, Error **errp)
+{
+    aiv->stack[aiv->nb_stack].cur_pos = cur_pos;
+    aiv->nb_stack++;
+
+    if (aiv->nb_stack >= AIV_STACK_SIZE) {
+        error_set(errp, QERR_BUFFER_OVERRUN);
+    }
+}
+
+static uint64_t ber_input_pop(BERInputVisitor *aiv, Error **errp)
+{
+    aiv->nb_stack--;
+
+    if (aiv->nb_stack < 0) {
+        error_set(errp, QERR_BUFFER_OVERRUN);
+        return 0;
+    }
+
+    return aiv->stack[aiv->nb_stack].cur_pos;
+}
+
+/*
+ * Read a type tag from the stream. Up-to 32 bit type tags are supported
+ * for reading and otherwise an error is returned. Anything larger than that
+ * would not be reasonable and could only be abused.
+ */
+static uint32_t ber_read_type(BERInputVisitor *aiv, uint8_t *ber_type_flags,
+                              Error **errp)
+{
+    uint32_t type;
+    uint8_t byte;
+    uint8_t ctr = 0;
+    char buf[128];
+
+    if (qemu_read_bytes(aiv->qfile, &byte, 1) != 1) {
+        error_setg(errp, "QEMUFile has an error, error was '%s'",
+                  "Error while reading type");
+        return 0;
+    }
+    aiv->cur_pos++;
+    type = byte;
+
+    *ber_type_flags = type & (BER_TYPE_P_C_MASK | BER_TYPE_CLASS_MASK);
+
+    if ((type & BER_TYPE_TAG_MASK) == BER_TYPE_LONG_FORM) {
+        type = 0;
+        while (true) {
+            type <<= 7;
+            if (qemu_read_bytes(aiv->qfile, &byte, 1) != 1) {
+                error_setg(errp, "QEMUFile has an error, error was '%s'",
+                          "Error while reading long type");
+                return 0;
+            }
+            aiv->cur_pos++;
+
+            type |= (byte & 0x7f);
+            if ((byte & 0x80) == 0) {
+                break;
+            }
+            ctr += 7; /* read 7 bits */
+            if (ctr >= (sizeof(type) * 8)) {
+                /* only support 32 bit length identifiers */
+                snprintf(buf, sizeof(buf),
+                         "type tag is larger than 32 bit (offset %" PRIu64
+                         ")", aiv->cur_pos);
+                error_setg(errp, "Data stream is invalid, error was '%s'", buf);
+                return 0;
+            }
+        }
+    } else {
+        type &= BER_TYPE_TAG_MASK;
+    }
+
+    return type;
+}
+
+static uint64_t ber_read_length(BERInputVisitor *aiv, bool *is_indefinite,
+                                Error **errp)
+{
+    uint8_t byte, c, int_len;
+    uint64_t len = 0;
+    QEMUFile *qfile = aiv->qfile;
+    unsigned char int_array[sizeof(len)];
+    char buf[128];
+
+    *is_indefinite = false;
+
+    if (qemu_read_bytes(qfile, &byte, 1) != 1) {
+        error_setg(errp, "QEMUFile has an error, error was '%s'",
+                  "Error while reading length indicator");
+        return ~0x0ULL;
+    }
+    aiv->cur_pos++;
+
+    if (byte == BER_LENGTH_INDEFINITE) {
+        *is_indefinite = true;
+        return ~0x0ULL;
+    }
+
+    if (!(byte & BER_LENGTH_LONG)) {
+        len = byte;
+    } else {
+        int_len = byte & BER_LENGTH_MASK;
+        if (int_len > sizeof(len)) {
+            snprintf(buf, sizeof(buf),
+                     "ASN.1 integer length field %d > %" PRIu64,
+                     int_len, sizeof(len));
+            /* Length can be up to 127 byte, but it seems
+             * safe to assume any input will be < 1TB in length. */
+            error_set(errp, QERR_INVALID_PARAMETER, buf);
+            return ~0x0ULL;
+        }
+        if (qemu_read_bytes(qfile, int_array, int_len) != int_len) {
+            error_setg(errp, "QEMUFile error: Error while reading length");
+            return ~0x0ULL;
+        }
+        for (c = 0; c < int_len; c++) {
+            len <<= 8;
+            len |= int_array[c];
+        }
+        aiv->cur_pos += int_len;
+    }
+
+    if (len > aiv->max_allowed_buffer_size) {
+        snprintf(buf, sizeof(buf),
+                 "Length indicator (%"PRIu64") in input byte stream "
+                 "exceeds maximum allowed length (%"PRIu64").",
+                 len, aiv->max_allowed_buffer_size);
+        error_setg(errp, "Data stream is invalid, error was '%s'", buf);
+        return ~0x0ULL;
+    }
+
+    if (len > aiv->largest_needed_buffer) {
+        aiv->largest_needed_buffer = len;
+    }
+
+    return len;
+}
+
+static uint64_t ber_peek_is_eoc(BERInputVisitor *biv, Error **errp)
+{
+    uint8_t buf[2];
+    QEMUFile *qfile = biv->qfile;
+
+    if (qemu_peek_bytes(qfile, buf, 2, 0) != 2) {
+        error_setg(errp, "QEMUFile has an error, error was '%s'",
+                   "Error while peeking for EOC");
+        return ~0x0ULL;
+    }
+
+    if (buf[0] == BER_TYPE_EOC && buf[1] == 0) {
+        return 1;
+    }
+
+    return 0;
+}
+
+static void ber_skip_bytes(BERInputVisitor *aiv, uint64_t to_skip,
+                           Error **errp)
+{
+    uint8_t buf[1024];
+    uint32_t skip;
+
+    /* skip length bytes */
+    while (to_skip > 0) {
+        skip = MIN(to_skip, sizeof(buf));
+        if (qemu_read_bytes(aiv->qfile, buf, skip) != skip) {
+            error_setg(errp, "QEMUFile error: Error while skipping over bytes");
+            return;
+        }
+        aiv->cur_pos += skip;
+        to_skip -= skip;
+    }
+}
+
+static void ber_skip_until_eoc(BERInputVisitor *aiv, Error **errp)
+{
+    uint32_t ber_type_tag;
+    uint64_t length;
+    bool is_indefinite;
+    uint8_t ber_type_flags;
+    uint64_t indefinite_nesting = 1;
+    char buf[128];
+
+    while (!error_is_set(errp)) {
+        ber_type_tag = ber_read_type(aiv, &ber_type_flags, errp);
+        if (error_is_set(errp)) {
+            return;
+        }
+
+        length = ber_read_length(aiv, &is_indefinite, errp);
+        if (error_is_set(errp)) {
+            return;
+        }
+        if (ber_type_tag == BER_TYPE_EOC) {
+            if (length) {
+                snprintf(buf, sizeof(buf),
+                         "ASN.1 EOC length field at offset %" PRIu64
+                         " is invalid", aiv->cur_pos);
+                error_set(errp, QERR_INVALID_PARAMETER, buf);
+                return;
+            }
+            if (!indefinite_nesting) {
+                snprintf(buf, sizeof(buf),
+                         "ASN.1 EOC at offset %" PRIu64
+                         "not within an indefinite length",
+                         aiv->cur_pos);
+                error_set(errp, QERR_INVALID_PARAMETER, buf);
+                return;
+            }
+#ifdef BER_DEBUG
+            fprintf(stderr, "found end! nesting=%" PRIdMAX
+                    ", pos=%" PRIu64 "\n",
+                    indefinite_nesting, aiv->cur_pos);
+#endif
+            if (!--indefinite_nesting) {
+                return;
+            }
+        }
+        if (is_indefinite) {
+            if ((ber_type_flags & BER_TYPE_P_C_MASK) == BER_TYPE_PRIMITIVE) {
+                snprintf(buf, sizeof(buf),
+                         "ASN.1 indefinite length in a primitive type "
+                         "at offset %" PRIu64,
+                         aiv->cur_pos);
+                error_set(errp, QERR_INVALID_PARAMETER, buf);
+                return;
+            }
+            if (indefinite_nesting == ~0x0ULL) {
+                snprintf(buf, sizeof(buf),
+                         "ASN.1 indefinite nesting level is too large "
+                         "(offset %" PRIu64 ")",
+                         aiv->cur_pos);
+                error_set(errp, QERR_INVALID_PARAMETER, buf);
+                return;
+            }
+            ++indefinite_nesting;
+        } else {
+#ifdef BER_DEBUG
+            fprintf(stderr, "skipping type '%s' of length "
+                    "%" PRIu64 " at %" PRIu64 ".\n",
+                    ber_type_to_str(ber_type_tag), length, aiv->cur_pos);
+#endif
+            ber_skip_bytes(aiv, length, errp);
+        }
+    }
+}
+
+static void ber_input_start_constructed(Visitor *v, uint32_t exp_ber_type,
+                                        uint8_t exp_ber_flags, void **obj,
+                                        const char *kind, const char *name,
+                                        size_t size, Error **errp)
+{
+    BERInputVisitor *aiv = to_biv(v);
+    uint32_t ber_type_tag;
+    uint8_t ber_type_flags;
+    int64_t len;
+    bool is_indefinite;
+    char buf[128];
+
+    ber_type_tag = ber_read_type(aiv, &ber_type_flags, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (ber_type_tag != exp_ber_type || ber_type_flags != exp_ber_flags) {
+        sprintf(buf, "%s at offset %" PRIu64,
+                ber_type_to_str(exp_ber_type), aiv->cur_pos);
+
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE,
+                  ber_type_to_str(ber_type_tag),
+                  buf);
+        return;
+    }
+
+    if ((ber_type_flags & BER_TYPE_P_C_MASK) == BER_TYPE_PRIMITIVE) {
+        snprintf(buf, sizeof(buf), "primitive type (%s)",
+                 ber_type_to_str(ber_type_tag));
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE,
+                  buf, "constructed type");
+        return;
+    }
+
+    len = ber_read_length(aiv, &is_indefinite, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (!is_indefinite) {
+#ifdef BER_DEBUG
+        fprintf(stderr, "structure/set len: %" PRIi64 "\n", len);
+#endif
+        ber_input_push(aiv, aiv->cur_pos + len, errp);
+    } else {
+#ifdef BER_DEBUG
+        fprintf(stderr, "indefinite length encoding!\n");
+#endif
+        ber_input_push(aiv, 0, errp);
+    }
+
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (size > 0 && *obj == NULL) {
+        *obj = g_malloc0(size);
+#ifdef BER_DEBUG
+        fprintf(stderr, "for type '%s' allocated buffer at %p, size = %zu\n",
+                ber_type_to_str(ber_type_tag), *obj, size);
+#endif
+    }
+}
+
+static void ber_input_end_constructed(Visitor *v, Error **errp)
+{
+    uint64_t new_pos;
+    BERInputVisitor *aiv = to_biv(v);
+
+    new_pos = ber_input_pop(aiv, errp);
+
+    if (new_pos != 0) {
+#ifdef BER_DEBUG
+        fprintf(stderr, "new_pos = %" PRIu64 "\n", new_pos);
+#endif
+        aiv->cur_pos = new_pos;
+    } else {
+#ifdef BER_DEBUG
+        fprintf(stderr, "searching for end...\n");
+        fprintf(stderr, "cur_pos = %" PRIu64 "\n", aiv->cur_pos);
+#endif
+        ber_skip_until_eoc(aiv, errp);
+    }
+}
+
+static void ber_input_start_struct(Visitor *v, void **obj, const char *kind,
+                                   const char *name, size_t size, Error **errp)
+{
+    ber_input_start_constructed(v, BER_TYPE_SEQUENCE, BER_TYPE_CONSTRUCTED,
+                                obj, kind, name, size, errp);
+}
+
+static void ber_input_end_struct(Visitor *v, Error **errp)
+{
+    ber_input_end_constructed(v, errp);
+}
+
+static void ber_input_start_carray(Visitor *v, void **obj,
+                                  const char *name, size_t elem_count,
+                                  size_t elem_size, Error **errp)
+{
+    ber_input_start_constructed(v, BER_TYPE_SET, BER_TYPE_CONSTRUCTED,
+                                obj, NULL, name,
+                                elem_count * elem_size, errp);
+}
+
+static void ber_input_next_carray(Visitor *v, Error **errp)
+{
+    /* nothing to do here */
+}
+
+static void ber_input_end_carray(Visitor *v, Error **errp)
+{
+    ber_input_end_constructed(v, errp);
+}
+
+static void ber_input_start_list(Visitor *v, const char *name,
+                                 Error **errp)
+{
+    void *obj = NULL;
+    ber_input_start_constructed(v, BER_TYPE_CUSTOM_LIST, BER_TYPE_CONSTRUCTED,
+                                obj, NULL, name, 0, errp);
+    g_free(obj);
+}
+
+static GenericList *ber_input_next_list(Visitor *v, GenericList **list,
+                                        Error **errp)
+{
+    BERInputVisitor *biv = to_biv(v);
+    GenericList *entry;
+    StackEntry *se = &biv->stack[biv->nb_stack - 1];
+
+    if (se->cur_pos == 0) {
+        /* indefinite lenght encoding is used */
+        se = &biv->stack[biv->nb_stack];
+        if (ber_peek_is_eoc(biv, errp) != 0) {
+            return NULL;
+        }
+    } else if (se->cur_pos <= biv->cur_pos) {
+        return NULL;
+    }
+
+    entry = g_malloc0(sizeof(*entry));
+    if (*list) {
+        (*list)->next = entry;
+    }
+
+    return entry;
+}
+
+static void ber_input_end_list(Visitor *v, Error **errp)
+{
+    ber_input_end_constructed(v, errp);
+}
+
+static void ber_input_integer(Visitor *v, uint8_t *obj, uint8_t maxbytes,
+                              Error **errp)
+{
+    BERInputVisitor *aiv = to_biv(v);
+    uint32_t ber_type_tag;
+    uint8_t ber_type_flags;
+    bool is_indefinite;
+    uint64_t len;
+    uint64_t val = 0;
+    unsigned char int_array[sizeof(val)];
+    int c;
+    char buf[128], buf2[128];
+
+#ifdef BER_DEBUG
+    fprintf(stderr, "reading int to %p\n", obj);
+#endif
+
+    ber_type_tag = ber_read_type(aiv, &ber_type_flags, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+#ifdef BER_DEBUG
+    fprintf(stderr, "%s: got type: 0x%02x, expected 0x%02x\n",
+            __func__, ber_type_tag, BER_TYPE_INTEGER);
+#endif
+
+    if (ber_type_tag != BER_TYPE_INTEGER ||
+        ber_type_flags != (BER_TYPE_CLASS_APPLICATION|BER_TYPE_PRIMITIVE)) {
+        snprintf(buf, sizeof(buf), "%s/%s/%s",
+                 ber_type_class_to_str(ber_type_flags),
+                 ber_type_pc_to_str(ber_type_flags),
+                 ber_type_to_str(ber_type_tag));
+        snprintf(buf2, sizeof(buf2), "%s/%s/%s",
+                 ber_type_class_to_str(BER_TYPE_CLASS_APPLICATION),
+                 ber_type_pc_to_str(BER_TYPE_PRIMITIVE),
+                 ber_type_to_str(BER_TYPE_INTEGER));
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE, buf, buf2);
+        return;
+    }
+
+    len = ber_read_length(aiv, &is_indefinite, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+#ifdef BER_DEBUG
+    fprintf(stderr, "pos: %" PRIu64 " int len: %" PRIi64 "\n",
+            aiv->cur_pos, len);
+#endif
+
+    if (is_indefinite) {
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                  "ASN.1 int indicator is indefinite",
+                  "[1..8]");
+        return;
+    }
+
+    if (maxbytes > sizeof(val)) {
+        snprintf(buf, sizeof(buf), "ASN.1 integers cannot have a length of "
+                 "%" PRIi32 " bytes", maxbytes);
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                  buf, "[1..8]");
+        return;
+    }
+
+    if (len > maxbytes) {
+        snprintf(buf, sizeof(buf), "ASN.1 integer length indicator %" PRIi64
+                 " is larger than expected (%u bytes)",
+                 len, maxbytes);
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                  buf, "[1..8]");
+        return;
+    }
+
+    if (qemu_read_bytes(aiv->qfile, int_array, len) != len) {
+        error_setg(errp, "QEMUFile error: Error while reading integer");
+        return;
+    }
+
+    for (c = 0; c < len ; c++) {
+        val <<= 8;
+        val |= int_array[c];
+        if (c == 0 && (val & 0x80) == 0x80) {
+            /* sign extend */
+            val |= 0xFFFFFFFFFFFFFF00ULL;
+        }
+    }
+    aiv->cur_pos += len;
+#ifdef BER_DEBUG
+    fprintf(stderr, "pos: %" PRIu64 " int: %" PRIu64 "\n", aiv->cur_pos, val);
+#endif
+
+    memcpy(obj, &val, maxbytes);
+}
+
+static void ber_input_type_int(Visitor *v, int64_t *obj, const char *name,
+                               Error **errp)
+{
+    ber_input_integer(v, (uint8_t *)obj, sizeof(*obj), errp);
+}
+
+static void ber_input_type_uint8(Visitor *v, uint8_t *obj,
+                                   const char *name, Error **errp)
+{
+    ber_input_integer(v, (uint8_t *)obj, sizeof(*obj), errp);
+}
+
+static void ber_input_type_uint16(Visitor *v, uint16_t *obj,
+                                    const char *name, Error **errp)
+{
+    ber_input_integer(v, (uint8_t *)obj, sizeof(*obj), errp);
+}
+
+static void ber_input_type_uint32(Visitor *v, uint32_t *obj,
+                                    const char *name, Error **errp)
+{
+    ber_input_integer(v, (uint8_t *)obj, sizeof(*obj), errp);
+}
+
+static void ber_input_type_uint64(Visitor *v, uint64_t *obj,
+                                    const char *name, Error **errp)
+{
+    ber_input_integer(v, (uint8_t *)obj, sizeof(*obj), errp);
+}
+
+static void ber_input_type_int8(Visitor *v, int8_t *obj,
+                                  const char *name, Error **errp)
+{
+    ber_input_integer(v, (uint8_t *)obj, sizeof(*obj), errp);
+}
+
+static void ber_input_type_int16(Visitor *v, int16_t *obj,
+                                   const char *name, Error **errp)
+{
+    ber_input_integer(v, (uint8_t *)obj, sizeof(*obj), errp);
+}
+
+static void ber_input_type_int32(Visitor *v, int32_t *obj,
+                                   const char *name, Error **errp)
+{
+    ber_input_integer(v, (uint8_t *)obj, sizeof(*obj), errp);
+}
+
+static void ber_input_type_int64(Visitor *v, int64_t *obj,
+                                   const char *name, Error **errp)
+{
+    ber_input_integer(v, (uint8_t *)obj, sizeof(*obj), errp);
+}
+
+static void ber_input_type_bool(Visitor *v, bool *obj, const char *name,
+                                Error **errp)
+{
+    BERInputVisitor *aiv = to_biv(v);
+    uint32_t ber_type_tag;
+    uint8_t ber_type_flags, byte;
+    bool is_indefinite;
+    uint64_t len;
+    char buf[128];
+
+    ber_type_tag = ber_read_type(aiv, &ber_type_flags, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (ber_type_tag != BER_TYPE_BOOLEAN || ber_type_flags != 0) {
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE,
+                  ber_type_to_str(ber_type_tag),
+                  ber_type_to_str(BER_TYPE_BOOLEAN));
+        return;
+    }
+    len = ber_read_length(aiv, &is_indefinite, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+#ifdef BER_DEBUG
+    fprintf(stderr, "pos: %" PRIu64 " bool len: %" PRIi64 "\n",
+            aiv->cur_pos, len);
+#endif
+
+    if (is_indefinite || len != 1) {
+        snprintf(buf, sizeof(buf),
+                 "ASN.1 bool length indicator at offset %" PRIu64
+                 " is indefinite or != 1",
+                 aiv->cur_pos);
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                  buf, "1");
+        return;
+    }
+    if (qemu_read_bytes(aiv->qfile, &byte, 1) != 1) {
+        error_setg(errp, "QEMUFile error: Error while reading boolean");
+        return;
+    }
+    aiv->cur_pos++;
+    *obj = byte;
+
+#ifdef BER_DEBUG
+    fprintf(stderr, "pos: %" PRIu64 " bool: %d\n", aiv->cur_pos, *obj);
+#endif
+}
+
+/* Function for recursive reading of fragmented primitives */
+static uint32_t ber_input_fragment(BERInputVisitor *aiv,
+                                   uint32_t exp_type_tag,
+                                   uint8_t exp_type_flags,
+                                   uint8_t **buffer, size_t *buffer_len,
+                                   bool may_realloc,
+                                   uint32_t offset, uint32_t nesting,
+                                   bool indefinite, uint64_t max_pos,
+                                   const char *name, Error **errp)
+{
+    uint32_t ber_type_tag;
+    uint8_t ber_type_flags;
+    uint32_t bytes_read = 0;
+    bool is_indefinite;
+    uint64_t len;
+    char buf[128];
+
+    assert((exp_type_flags & BER_TYPE_CONSTRUCTED) == BER_TYPE_PRIMITIVE);
+
+    ber_type_tag = ber_read_type(aiv, &ber_type_flags, errp);
+    if (error_is_set(errp)) {
+        return 0;
+    }
+
+    if (ber_type_tag != exp_type_tag) {
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE,
+                  ber_type_to_str(ber_type_tag & BER_TYPE_TAG_MASK),
+                  ber_type_to_str(exp_type_tag));
+        return 0;
+    }
+
+    if ((ber_type_flags & BER_TYPE_CONSTRUCTED)) {
+#ifndef BER_ALLOW_FRAGMENTED_PRIMITIVES
+        error_setg(errp, "Data stream is invalid, error was '%s'",
+                  "constructed encoding of primitive types is not supported");
+        goto err_exit;
+#else
+        if (nesting == 1) {
+            /* don't allow further nesting */
+            error_setg(errp, "Data stream is invalid, error was invalid nesting");
+            goto err_exit;
+        }
+        len = ber_read_length(aiv, &is_indefinite, errp);
+        if (error_is_set(errp)) {
+            goto err_exit;
+        }
+#ifdef BER_DEBUG
+        fprintf(stderr, "pos: %" PRIu64 " string len: %" PRIi64 "\n",
+                aiv->cur_pos, len);
+#endif
+
+        if (!is_indefinite) {
+            if ((*buffer) == NULL) {
+                /* allocate buffer once; due to the ASN.1 overhead it
+                 * will be bigger than what we need */
+                *buffer = g_malloc0(len);
+                *buffer_len = len;
+                may_realloc = false;
+            }
+        }
+#ifdef BER_DEBUG
+        fprintf(stderr, "recursing now to read constructed type.\n");
+        fprintf(stderr, "  is_indefinite: %d\n", is_indefinite);
+#endif
+        bytes_read += ber_input_fragment(aiv, exp_type_tag, exp_type_flags,
+                                         buffer, buffer_len, may_realloc,
+                                         offset, nesting + 1, is_indefinite,
+                                         aiv->cur_pos + len, name, errp);
+        return bytes_read;
+#endif
+    }
+
+    while (true) {
+        /* Would reading the length carry us beyond what we are allowed to
+         * read?
+         */
+        if (!indefinite &&
+            max_pos != 0 &&
+            aiv->cur_pos + 1 > max_pos) {
+            snprintf(buf, sizeof(buf),
+                     "data stream would cause parsing beyond "
+                     "allowed offset at %" PRIu64,
+                     max_pos);
+            /* input stream is malformed */
+            error_setg(errp, "Data stream is invalid, error was '%s'", buf);
+            goto err_exit;
+        }
+
+        /* not-constructed case */
+        len = ber_read_length(aiv, &is_indefinite, errp);
+        if (error_is_set(errp)) {
+            goto err_exit;
+        }
+#ifdef BER_DEBUG
+        fprintf(stderr, "pos: %" PRIu64 " string len: %" PRIi64 "\n",
+                    aiv->cur_pos, len);
+#endif
+        if (is_indefinite) {
+            snprintf(buf, sizeof(buf),
+                     "Got indefinite type length in primitive type (%s) at"
+                     "offset %" PRIu64,
+                     ber_type_to_str(ber_type_tag), aiv->cur_pos);
+            error_set(errp, QERR_INVALID_PARAMETER, buf);
+            goto err_exit;
+        }
+        /* if max_pos is not set, set it here */
+        if (!indefinite && max_pos == 0) {
+            max_pos = aiv->cur_pos + len;
+        }
+
+        /* Would reading the data carry us beyond what we are allowed to
+         * read ?
+         */
+        if (!indefinite && aiv->cur_pos + len > max_pos) {
+            /* input stream is malformed */
+            snprintf(buf, sizeof(buf),
+                     "data stream would cause parsing beyond "
+                     "allowed offset at %" PRIu64,
+                     max_pos);
+            error_setg(errp, "Data stream is invalid, error was '%s'", buf);
+            goto err_exit;
+        }
+
+        if (offset + len > *buffer_len) {
+            if (!may_realloc) {
+                snprintf(buf, sizeof(buf),
+                         "given buffer is too small (%lu < %"PRIu64")",
+                         (unsigned long)*buffer_len, offset + len);
+                error_setg(errp, "Data stream is invalid, error was '%s'", buf);
+            }
+            /* allocate one more byte for strings, set to 0 */
+            *buffer = g_realloc(*buffer, offset + len + 1);
+            *buffer_len = offset + len;
+            (*buffer)[offset+len] = 0;
+        }
+
+        if (qemu_read_bytes(aiv->qfile,
+                            &((uint8_t *)*buffer)[offset], len) != len) {
+            error_setg(errp, "QEMUFile error: Error while reading data");
+            goto err_exit;
+        }
+
+        offset += len;
+        bytes_read += len;
+
+        aiv->cur_pos += len;
+#ifdef BER_DEBUG
+        if (exp_type_tag == BER_TYPE_IA5_STRING) {
+            fprintf(stderr, "pos: %" PRIu64 " string: %.*s\n", aiv->cur_pos,
+                    offset, *buffer);
+        }
+#endif
+
+        if (nesting == 0) {
+            break;
+        }
+
+        /* indefinite length case: loop until we encounter EOC */
+        if (indefinite) {
+            ber_type_tag = ber_read_type(aiv, &ber_type_flags, errp);
+            if (error_is_set(errp)) {
+                goto err_exit;
+            }
+
+            if (ber_type_tag == BER_TYPE_EOC) {
+                uint8_t byte;
+                if (qemu_read_bytes(aiv->qfile, &byte, 1) != 1) {
+                    error_setg(errp, "QEMUFile error: Error while reading BER_TYPE_EOC length");
+                    goto err_exit;
+                }
+                aiv->cur_pos++;
+
+                if (byte != 0) {
+                    snprintf(buf, sizeof(buf),
+                             "ASN.1 EOC length field is invalid at offset "
+                             "%" PRIu64,
+                             aiv->cur_pos);
+                    error_set(errp, QERR_INVALID_PARAMETER, buf);
+                    goto err_exit;
+                }
+                return bytes_read;
+            }
+
+            if (ber_type_tag != exp_type_tag ||
+                ber_type_flags != exp_type_flags) {
+                snprintf(buf, sizeof(buf),
+                         "ASN.1 type field or flags are wrong. Found "
+                         "0x%x/%u, expected "
+                         "0x%x/%u at offset %" PRIu64,
+                         ber_type_tag, ber_type_flags,
+                         exp_type_tag, exp_type_flags,
+                         aiv->cur_pos);
+                error_set(errp, QERR_INVALID_PARAMETER, buf);
+                goto err_exit;
+            }
+            continue;
+        }
+
+        /* in definite length coding case; caller told us how far to read */
+        if (aiv->cur_pos == max_pos) {
+            return bytes_read;
+        }
+
+        ber_type_tag = ber_read_type(aiv, &ber_type_flags, errp);
+        if (error_is_set(errp)) {
+            goto err_exit;
+        }
+
+        if ((ber_type_flags & BER_TYPE_P_C_MASK) == BER_TYPE_CONSTRUCTED) {
+            error_set(errp, QERR_INVALID_PARAMETER_TYPE,
+                      "constructed BER type",
+                      ber_type_to_str(exp_type_tag));
+            goto err_exit;
+        }
+
+        if (ber_type_tag != exp_type_tag) {
+            error_set(errp, QERR_INVALID_PARAMETER_TYPE,
+                      ber_type_to_str(ber_type_tag & BER_TYPE_TAG_MASK),
+                      ber_type_to_str(exp_type_tag));
+            goto err_exit;
+        }
+    }
+    return bytes_read;
+
+err_exit:
+    if (may_realloc) {
+        g_free(*buffer);
+        *buffer = NULL;
+    }
+    return 0;
+}
+
+static void ber_input_type_str(Visitor *v, char **obj, const char *name,
+                               Error **errp)
+{
+    BERInputVisitor *aiv = to_biv(v);
+    size_t buffer_len = 0;
+
+    ber_input_fragment(aiv, BER_TYPE_IA5_STRING, 0,
+                       (uint8_t **)obj, &buffer_len, (*obj == NULL),
+                       0, 0, false, 0, name, errp);
+
+    if (!error_is_set(errp) && *obj == NULL) {
+        /* adjust NULL string to "" */
+        *obj = g_strdup("");
+    }
+}
+
+static void ber_input_sized_buffer(Visitor *v, uint8_t **obj, size_t len,
+                                   const char *name, Error **errp)
+{
+    BERInputVisitor *aiv = to_biv(v);
+
+    ber_input_fragment(aiv, BER_TYPE_OCTET_STRING, 0,
+                       (uint8_t **)obj, &len, (*obj == NULL),
+                       0, 0, false, 0, name, errp);
+
+#ifdef BER_DEBUG
+    fprintf(stderr, "pos: %" PRIu64 " data at: %p data:\n",
+            aiv->cur_pos, *obj);
+    int i;
+    for (i = 0; i < len; i++) {
+        fprintf(stderr, "%02x ", (*obj)[i]);
+        if ((i & 0xf) == 0xf) {
+            fprintf(stderr, "\n");
+        }
+    }
+    fprintf(stderr, "\n");
+#endif
+}
+
+static void ber_input_type_number(Visitor *v, double *obj, const char *name,
+                                  Error **errp)
+{
+    BERInputVisitor *aiv = to_biv(v);
+    uint32_t ber_type_tag;
+    uint8_t ber_type_flags;
+    uint32_t len;
+    bool is_indefinite;
+    char buf[128], buf2[128];
+    GDoubleIEEE754 num;
+    struct ieee754_buffer number;
+    size_t to_read;
+
+    ber_type_tag = ber_read_type(aiv, &ber_type_flags, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (ber_type_tag != BER_TYPE_REAL ||
+        ber_type_flags != (BER_TYPE_CLASS_APPLICATION|BER_TYPE_PRIMITIVE)) {
+        snprintf(buf, sizeof(buf), "%s/%s/%s",
+                 ber_type_class_to_str(ber_type_flags),
+                 ber_type_pc_to_str(ber_type_flags),
+                 ber_type_to_str(ber_type_tag));
+        snprintf(buf2, sizeof(buf2), "%s/%s/%s",
+                 ber_type_class_to_str(BER_TYPE_CLASS_APPLICATION),
+                 ber_type_pc_to_str(BER_TYPE_PRIMITIVE),
+                 ber_type_to_str(BER_TYPE_REAL));
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE, buf, buf2);
+        return;
+    }
+
+    len = ber_read_length(aiv, &is_indefinite, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    to_read = sizeof(number) - offsetof(struct ieee754_buffer, first);
+
+    if (len != to_read) {
+        snprintf(buf, sizeof(buf),
+                 "Length indicator in input byte stream "
+                 "of real has unexpected length %"PRIu32"; "
+                 "expected %" PRIu64,
+                 len, sizeof(buf));
+        error_set(errp, QERR_INVALID_PARAMETER, buf);
+        return;
+    }
+
+    if (is_indefinite) {
+        snprintf(buf, sizeof(buf),
+                 "ASN.1 indefinite length in a real type "
+                 "at offset %" PRIu64,
+                 aiv->cur_pos);
+        error_set(errp, QERR_INVALID_PARAMETER, buf);
+        return;
+    }
+
+    if (qemu_read_bytes(aiv->qfile, &number.first, to_read) != to_read) {
+        error_setg(errp, "QEMUFile error: Error while reading real");
+        return;
+    }
+
+    switch (number.first) {
+    case 0x42:
+        *obj = nan("NAN");
+        break;
+    case 0x41:
+    case 0x40:
+        num.mpn.sign = ((number.first & 0x1) != 0);
+        num.mpn.biased_exponent = ~0;
+        num.mpn.mantissa_low = 0;
+        num.mpn.mantissa_high = 0;
+        *obj = num.v_double;
+        break;
+    default:
+        num.mpn.sign = ((number.first & 0x40) != 0);
+        num.mpn.biased_exponent = be16_to_cpu(number.exponent);
+        num.mpn.mantissa_low = be32_to_cpu(number.mant_lo);
+        num.mpn.mantissa_high = be32_to_cpu(number.mant_hi);
+        *obj = num.v_double;
+    }
+}
+
+Visitor *ber_input_get_visitor(BERInputVisitor *v)
+{
+    return &v->visitor;
+}
+
+uint64_t ber_input_get_parser_position(BERInputVisitor *v)
+{
+    return v->cur_pos;
+}
+
+uint64_t ber_input_get_largest_needed_buffer(BERInputVisitor *v)
+{
+    return v->largest_needed_buffer;
+}
+
+void ber_input_visitor_cleanup(BERInputVisitor *v)
+{
+    g_free(v);
+}
+
+BERInputVisitor *ber_input_visitor_new(QEMUFile *qfile,
+                                       uint64_t max_allowed_buffer_size)
+{
+    BERInputVisitor *v;
+
+    v = g_malloc0(sizeof(*v));
+
+    v->visitor.start_struct = ber_input_start_struct;
+    v->visitor.end_struct = ber_input_end_struct;
+    v->visitor.start_carray = ber_input_start_carray;
+    v->visitor.next_carray = ber_input_next_carray;
+    v->visitor.end_carray = ber_input_end_carray;
+    v->visitor.start_list = ber_input_start_list;
+    v->visitor.next_list = ber_input_next_list;
+    v->visitor.end_list = ber_input_end_list;
+    v->visitor.type_int = ber_input_type_int;
+    v->visitor.type_uint8 = ber_input_type_uint8;
+    v->visitor.type_uint16 = ber_input_type_uint16;
+    v->visitor.type_uint32 = ber_input_type_uint32;
+    v->visitor.type_uint64 = ber_input_type_uint64;
+    v->visitor.type_int8 = ber_input_type_int8;
+    v->visitor.type_int16 = ber_input_type_int16;
+    v->visitor.type_int32 = ber_input_type_int32;
+    v->visitor.type_int64 = ber_input_type_int64;
+    v->visitor.type_bool = ber_input_type_bool;
+    v->visitor.type_str = ber_input_type_str;
+    v->visitor.type_sized_buffer = ber_input_sized_buffer;
+    v->visitor.type_number = ber_input_type_number;
+
+    v->qfile = qfile;
+    v->cur_pos = 0;
+    v->max_allowed_buffer_size = max_allowed_buffer_size;
+    v->largest_needed_buffer = 0;
+
+    return v;
+}
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 8/9] asn1_test_visitor_serialization.diff
  2013-03-13 18:56 [Qemu-devel] [PATCH 0/9 v3] Implement and test asn1 ber visitors Joel Schopp
                   ` (6 preceding siblings ...)
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 7/9] asn1_input-visitor.diff Joel Schopp
@ 2013-03-13 18:56 ` Joel Schopp
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 9/9] update_maintainers.diff Joel Schopp
  8 siblings, 0 replies; 33+ messages in thread
From: Joel Schopp @ 2013-03-13 18:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joel Schopp, Stefan Berger, Michael Tsirkin

Add BER Visitor hooks to test-visitor-serialization

Cc: Michael Tsirkin <mst@redhat.com>
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 tests/Makefile                     |    2 +-
 tests/test-visitor-serialization.c |   72 ++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile b/tests/Makefile
index 567e36e..578d732 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -124,7 +124,7 @@ tests/test-qmp-output-visitor$(EXESUF): tests/test-qmp-output-visitor.o $(test-q
 tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
 tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
 tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y) qapi-types.o qapi-visit.o libqemuutil.a libqemustub.a
-tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
+tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) $(block-obj-y) libqemuutil.a libqemustub.a
 
 tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a
 
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 3c6b8df..aae7464 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -14,6 +14,7 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <float.h>
+#include <math.h>
 
 #include "qemu-common.h"
 #include "test-qapi-types.h"
@@ -23,6 +24,9 @@
 #include "qapi/qmp-output-visitor.h"
 #include "qapi/string-input-visitor.h"
 #include "qapi/string-output-visitor.h"
+#include "qapi/ber-input-visitor.h"
+#include "qapi/ber-output-visitor.h"
+#include "migration/qemu-file.h"
 
 typedef struct PrimitiveType {
     union {
@@ -701,6 +705,59 @@ static void string_cleanup(void *datap)
     string_input_visitor_cleanup(d->siv);
 }
 
+
+typedef struct BERSerializeData {
+    BEROutputVisitor *sov;
+    QEMUFile *qoutfile;
+    BERInputVisitor *siv;
+    QEMUFile *qinfile;
+} BERSerializeData;
+
+static void ber_serialize(void *native_in, void **datap,
+                          VisitorFunc visit, Error **errp, BERTypePC ber_type_pc)
+{
+    BERSerializeData *d = g_malloc0(sizeof(*d));
+
+    d->qoutfile = qemu_bufopen("w", NULL);
+    d->sov = ber_output_visitor_new(d->qoutfile, ber_type_pc);
+    visit(ber_output_get_visitor(d->sov), &native_in, errp);
+    *datap = d;
+}
+
+static void ber_primitive_serialize(void *native_in, void **datap,
+                          VisitorFunc visit, Error **errp)
+{
+    ber_serialize(native_in, datap, visit, errp, BER_TYPE_PRIMITIVE);
+}
+
+static void ber_constructed_serialize(void *native_in, void **datap,
+                                      VisitorFunc visit, Error **errp)
+{
+    ber_serialize(native_in, datap, visit, errp, BER_TYPE_CONSTRUCTED);
+}
+
+static void ber_deserialize(void **native_out, void *datap,
+                               VisitorFunc visit, Error **errp)
+{
+    BERSerializeData *d = datap;
+    const QEMUSizedBuffer *qsb = qemu_buf_get(d->qoutfile);
+    QEMUSizedBuffer *new_qsb = qsb_clone(qsb);
+    g_assert(new_qsb != NULL);
+
+    d->qinfile = qemu_bufopen("r", new_qsb);
+
+    d->siv = ber_input_visitor_new(d->qinfile, ~0);
+    visit(ber_input_get_visitor(d->siv), native_out, errp);
+}
+
+static void ber_cleanup(void *datap)
+{
+    BERSerializeData *d = datap;
+    ber_output_visitor_cleanup(d->sov);
+    ber_input_visitor_cleanup(d->siv);
+}
+
+
 /* visitor registration, test harness */
 
 /* note: to function interchangeably as a serialization mechanism your
@@ -722,6 +779,21 @@ static const SerializeOps visitors[] = {
         .cleanup = string_cleanup,
         .caps = VCAP_PRIMITIVES
     },
+    {
+        .type = "ASN.1 BER primitives",
+        .serialize = ber_primitive_serialize,
+        .deserialize = ber_deserialize,
+        .cleanup = ber_cleanup,
+        .caps = VCAP_PRIMITIVES
+    },
+    {
+        .type = "ASN.1 BER constructed",
+        .serialize = ber_constructed_serialize,
+        .deserialize = ber_deserialize,
+        .cleanup = ber_cleanup,
+        .caps = VCAP_PRIMITIVES | VCAP_LISTS
+    },
+
     { NULL }
 };
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH 9/9] update_maintainers.diff
  2013-03-13 18:56 [Qemu-devel] [PATCH 0/9 v3] Implement and test asn1 ber visitors Joel Schopp
                   ` (7 preceding siblings ...)
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 8/9] asn1_test_visitor_serialization.diff Joel Schopp
@ 2013-03-13 18:56 ` Joel Schopp
  8 siblings, 0 replies; 33+ messages in thread
From: Joel Schopp @ 2013-03-13 18:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joel Schopp, Stefan Berger, Michael Tsirkin

Since I'm throwing all this code out there I'm also signing up to maintain it.
Send me your bug reports.

Cc: Michael Tsirkin <mst@redhat.com>
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 MAINTAINERS |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0ca7e1d..8a2f444 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -790,3 +790,11 @@ Stable 0.10
 L: qemu-stable@nongnu.org
 T: git git://git.qemu.org/qemu-stable-0.10.git
 S: Orphan
+
+Visitors
+---------------
+ASN.1 BER
+M: Joel Schopp <jschopp@linux.vnet.ibm.com>
+S: Maintained
+F: /qapi/ber*
+F: include/qapi/ber*
\ No newline at end of file
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 2/9] qapi_c_arrays.diff
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 2/9] qapi_c_arrays.diff Joel Schopp
@ 2013-03-13 19:11   ` Anthony Liguori
  2013-03-13 22:54   ` Stefan Berger
  1 sibling, 0 replies; 33+ messages in thread
From: Anthony Liguori @ 2013-03-13 19:11 UTC (permalink / raw)
  To: Joel Schopp, qemu-devel; +Cc: Michael Tsirkin, Michael Roth, Paolo Bonzini


You need to adjust your patch script.  This will make the summary in git
be:

   qapi_c_arrays.gdiff

   Forward ported Mike's previously sent patch (see
   http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg05782.html )
   ...

Regards,

Anthony Liguori

Joel Schopp <jschopp@linux.vnet.ibm.com> writes:

> Forward ported Mike's previously sent patch
> (see http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg05782.html ) in my
> series since it implements a qapi array interface the ASN.1 BER visitor needs.
>
> Generally these will be serialized into lists, but the
> representation can be of any form so long as it can
> be deserialized into a single-dimension C array.
>
> Cc: Michael Tsirkin <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
> ---
>  include/qapi/visitor-impl.h |    4 ++++
>  include/qapi/visitor.h      |    4 ++++
>  qapi/qapi-visit-core.c      |   25 +++++++++++++++++++++++++
>  3 files changed, 33 insertions(+)
>
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index 5159964..9d87f2d 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -34,6 +34,10 @@ struct Visitor
>      void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
>      void (*type_number)(Visitor *v, double *obj, const char *name,
>                          Error **errp);
> +    void (*start_carray)(Visitor *v, void **obj, const char *name,
> +                         size_t elem_count, size_t elem_size, Error **errp);
> +    void (*next_carray)(Visitor *v, Error **errp);
> +    void (*end_carray)(Visitor *v, Error **errp);
>  
>      /* May be NULL */
>      void (*start_optional)(Visitor *v, bool *present, const char *name,
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index 1fef18c..74bddef 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -51,5 +51,9 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>  void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
>  void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
>  void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
> +void visit_start_carray(Visitor *v, void **obj, const char *name,
> +                        size_t elem_count, size_t elem_size, Error **errp);
> +void visit_next_carray(Visitor *v, Error **errp);
> +void visit_end_carray(Visitor *v, Error **errp);
>  
>  #endif
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 401ee6e..d9982f8 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -313,3 +313,28 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
>      g_free(enum_str);
>      *obj = value;
>  }
> +
> +void visit_start_carray(Visitor *v, void **obj, const char *name,
> +                        size_t elem_count, size_t elem_size, Error **errp)
> +{
> +    g_assert(v->start_carray);
> +    if (!error_is_set(errp)) {
> +        v->start_carray(v, obj, name, elem_count, elem_size, errp);
> +    }
> +}
> +
> +void visit_next_carray(Visitor *v, Error **errp)
> +{
> +    g_assert(v->next_carray);
> +    if (!error_is_set(errp)) {
> +        v->next_carray(v, errp);
> +    }
> +}
> +
> +void visit_end_carray(Visitor *v, Error **errp)
> +{
> +    g_assert(v->end_carray);
> +    if (!error_is_set(errp)) {
> +        v->end_carray(v, errp);
> +    }
> +}
> -- 
> 1.7.10.4

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 5/9] qapi_sized_buffer Joel Schopp
@ 2013-03-13 20:52   ` mdroth
  2013-03-13 22:00     ` Stefan Berger
  0 siblings, 1 reply; 33+ messages in thread
From: mdroth @ 2013-03-13 20:52 UTC (permalink / raw)
  To: Joel Schopp; +Cc: Michael Tsirkin, qemu-devel, Stefan Berger

On Wed, Mar 13, 2013 at 01:56:24PM -0500, Joel Schopp wrote:
> Add a sized buffer interface to qapi.

Isn't this just a special case of the visit_*_carray() interfaces? We
should avoid new interfaces if possible, since it adds to feature
disparities between visitor implementations.

> 
> Cc: Michael Tsirkin <mst@redhat.com>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
> ---
>  include/qapi/visitor-impl.h |    2 ++
>  include/qapi/visitor.h      |    2 ++
>  qapi/qapi-visit-core.c      |    8 ++++++++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index 9d87f2d..dc0e25c 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -38,6 +38,8 @@ struct Visitor
>                           size_t elem_count, size_t elem_size, Error **errp);
>      void (*next_carray)(Visitor *v, Error **errp);
>      void (*end_carray)(Visitor *v, Error **errp);
> +    void (*type_sized_buffer)(Visitor *v, uint8_t **obj, size_t size,
> +                              const char *name, Error **errp);
> 
>      /* May be NULL */
>      void (*start_optional)(Visitor *v, bool *present, const char *name,
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index 74bddef..7c7bb98 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -55,5 +55,7 @@ void visit_start_carray(Visitor *v, void **obj, const char *name,
>                          size_t elem_count, size_t elem_size, Error **errp);
>  void visit_next_carray(Visitor *v, Error **errp);
>  void visit_end_carray(Visitor *v, Error **errp);
> +void visit_type_sized_buffer(Visitor *v, uint8_t **obj, size_t len,
> +                             const char *name, Error **errp);
> 
>  #endif
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index d9982f8..4b36a54 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -338,3 +338,11 @@ void visit_end_carray(Visitor *v, Error **errp)
>          v->end_carray(v, errp);
>      }
>  }
> +
> +void visit_type_sized_buffer(Visitor *v, uint8_t **obj, size_t len,
> +                             const char *name, Error **errp)
> +{
> +    if (!error_is_set(errp)) {
> +        v->type_sized_buffer(v, obj, len, name, errp);
> +    }
> +}
> -- 
> 1.7.10.4
> 
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 3/9] two new file wrappers
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 3/9] two new file wrappers Joel Schopp
@ 2013-03-13 21:04   ` Eric Blake
  2013-03-14 10:49     ` Stefan Berger
  0 siblings, 1 reply; 33+ messages in thread
From: Eric Blake @ 2013-03-13 21:04 UTC (permalink / raw)
  To: Joel Schopp; +Cc: Michael Tsirkin, qemu-devel, Stefan Berger

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

On 03/13/2013 12:56 PM, Joel Schopp wrote:
> Add a 3 very short file wrapper functions to make code that follows more

s/a 3/3/

> readable.  Also export an existing function that is currently static.
> 
> Cc: Michael Tsirkin <mst@redhat.com>
> 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 |    3 +++
>  util/qemu-file.c              |   30 ++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 

> +
> +int qemu_peek_bytes(QEMUFile *f, uint8_t *buf, int size, size_t offset)

Should we be using off_t instead of size_t for offset; and size_t
instead of int for size (which implies ssize_t for return value)?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/9] qemu_qsb.diff
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 4/9] qemu_qsb.diff Joel Schopp
@ 2013-03-13 21:11   ` mdroth
  2013-03-13 21:28     ` Stefan Berger
  0 siblings, 1 reply; 33+ messages in thread
From: mdroth @ 2013-03-13 21:11 UTC (permalink / raw)
  To: Joel Schopp; +Cc: qemu-devel, Stefan Berger

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
> 
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/9] qemu_qsb.diff
  2013-03-13 21:11   ` mdroth
@ 2013-03-13 21:28     ` Stefan Berger
  2013-03-13 22:41       ` mdroth
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2013-03-13 21:28 UTC (permalink / raw)
  To: mdroth; +Cc: Joel Schopp, qemu-devel

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-13 20:52   ` mdroth
@ 2013-03-13 22:00     ` Stefan Berger
  2013-03-13 23:18       ` mdroth
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2013-03-13 22:00 UTC (permalink / raw)
  To: mdroth; +Cc: Joel Schopp, qemu-devel, Michael Tsirkin

On 03/13/2013 04:52 PM, mdroth wrote:
> On Wed, Mar 13, 2013 at 01:56:24PM -0500, Joel Schopp wrote:
>> Add a sized buffer interface to qapi.
> Isn't this just a special case of the visit_*_carray() interfaces? We
> should avoid new interfaces if possible, since it adds to feature
> disparities between visitor implementations.

Yes, it's a special case and carray seems more general.
However, I don't understand the interface of carray. It has a 
start_carray with all parameters given, then a next_carray and an 
end_carray. Why do we need multiple calls if one call (start_carray) 
could be used to serialize all the data already?

Regards,
     Stefan

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/9] qemu_qsb.diff
  2013-03-13 21:28     ` Stefan Berger
@ 2013-03-13 22:41       ` mdroth
  2013-03-13 22:47         ` mdroth
  0 siblings, 1 reply; 33+ messages in thread
From: mdroth @ 2013-03-13 22:41 UTC (permalink / raw)
  To: Stefan Berger; +Cc: Joel Schopp, qemu-devel

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
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/9] qemu_qsb.diff
  2013-03-13 22:41       ` mdroth
@ 2013-03-13 22:47         ` mdroth
  2013-03-13 23:11           ` Stefan Berger
  0 siblings, 1 reply; 33+ messages in thread
From: mdroth @ 2013-03-13 22:47 UTC (permalink / raw)
  To: Stefan Berger; +Cc: Joel Schopp, qemu-devel, quintela

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
> > 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 2/9] qapi_c_arrays.diff
  2013-03-13 18:56 ` [Qemu-devel] [PATCH 2/9] qapi_c_arrays.diff Joel Schopp
  2013-03-13 19:11   ` Anthony Liguori
@ 2013-03-13 22:54   ` Stefan Berger
  1 sibling, 0 replies; 33+ messages in thread
From: Stefan Berger @ 2013-03-13 22:54 UTC (permalink / raw)
  To: Joel Schopp, Michael Roth; +Cc: Paolo Bonzini, qemu-devel, Michael Tsirkin

On 03/13/2013 02:56 PM, Joel Schopp wrote:
> Forward ported Mike's previously sent patch
> (see http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg05782.html ) in my
> series since it implements a qapi array interface the ASN.1 BER visitor needs.
>
> Generally these will be serialized into lists, but the
> representation can be of any form so long as it can
> be deserialized into a single-dimension C array.
>
> Cc: Michael Tsirkin <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
> ---
>   include/qapi/visitor-impl.h |    4 ++++
>   include/qapi/visitor.h      |    4 ++++
>   qapi/qapi-visit-core.c      |   25 +++++++++++++++++++++++++
>   3 files changed, 33 insertions(+)
>
> +++ b/include/qapi/visitor.h
> @@ -51,5 +51,9 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>   void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
>   void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
>   void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
> +void visit_start_carray(Visitor *v, void **obj, const char *name,
> +                        size_t elem_count, size_t elem_size, Error **errp);
> +void visit_next_carray(Visitor *v, Error **errp);
> +void visit_end_carray(Visitor *v, Error **errp);
>
>   #endif
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 401ee6e..d9982f8 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -313,3 +313,28 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
>       g_free(enum_str);
>       *obj = value;
>   }
> +
> +void visit_start_carray(Visitor *v, void **obj, const char *name,
> +                        size_t elem_count, size_t elem_size, Error **errp)
> +{
> +    g_assert(v->start_carray);
> +    if (!error_is_set(errp)) {
> +        v->start_carray(v, obj, name, elem_count, elem_size, errp);
> +    }

So this is a c-like array with a given number of elements and each 
element having to have the same size? As long as all use cases for QEMU 
can live with this, I guess it's fine, though an array of sized buffers, 
each with different size, may be more 'general'.
How many other callers are there so that we can talk about disparities 
between visitors as mentioned in the comment in 5/9? At least we would 
have immediate callers for the sized buffer interface.

http://lists.gnu.org/archive/html/qemu-devel/2013-03/msg02254.html

Regards,
    Stefan

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 4/9] qemu_qsb.diff
  2013-03-13 22:47         ` mdroth
@ 2013-03-13 23:11           ` Stefan Berger
  0 siblings, 0 replies; 33+ messages in thread
From: Stefan Berger @ 2013-03-13 23:11 UTC (permalink / raw)
  To: mdroth; +Cc: Joel Schopp, qemu-devel, quintela

On 03/13/2013 06:47 PM, mdroth wrote:
> 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.

It almost sounds like there is a lock on QEMUFile forbidding it to be 
used for anything else.. strange.

>>> 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.
The vNVRAM would be used for writing persistent state of the software 
TPM into. The vNVRAM uses the block devices and with that we get the 
benefit of encryption (QCOW2) and migration.

The contents of the vNVRAM are layed out as one large ASN.1 stream. At 
the beginning is the header, then come the individual blobs, which may 
each be of different size and each is identified by a unique name. When 
reading a certain blob (given its name), we use the input visitor to 
scan through the BDRV to find the blob and then read it (using the sized 
buffer input visitor).
When writing we scan for the blob and switch from input visitor to 
output visitor to replace the blob. The requirement here is of course 
that the blob maintains its size so that the subsequent blobs are still 
part of that ASN.1 stream and don't destroy  the integrity of the ASN.1 
stream.

Code talks, so here's the hunk of the patch for BDRV support as it 
stands right now. The header may need some changes, but that's not the 
point.

Index: qemu-git.pt/qemu-bdrv-file.c
===================================================================
--- /dev/null
+++ qemu-git.pt/qemu-bdrv-file.c
@@ -0,0 +1,116 @@
+/*
+ * QEMU File : wrapper for bdrv
+ *
+ * Copyright (c) 2011, 2012, 2013 IBM Corporation
+ *
+ * Authors:
+ *   Stefan Berger <stefanb@linux.vnet.ibm.com>
+ *
+ * Permission is hereby granted, free of charge, to any person 
obtaining a cop
+ * of this software and associated documentation files (the 
"Software"), to de
+ * in the Software without restriction, including without limitation 
the right
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be 
included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
ARISING FRO
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/*
+ * Note: This QEMUFile wrapper introduces a dependency on the block layer.
+ *       We keep it in this separate file to avoid pulling block.o and its
+ *       dependencies into test programs.
+ */
+
+#include "qemu-common.h"
+#include "hw/hw.h"
+#include "block/block.h"
+
+typedef struct QEMUBdrvFile {
+    BlockDriverState *bds;
+    uint64_t offset;
+    QEMUFile *file;
+} QEMUBdrvFile;
+
+static int qf_bdrv_put_buffer(void *opaque, const uint8_t *buf,
+                              int64_t pos, int size)
+{
+    QEMUBdrvFile *s = opaque;
+
+    return bdrv_pwrite(s->bds, pos + s->offset, buf, size);
+}
+
+static int qf_bdrv_get_buffer(void *opaque, uint8_t *buf,
+                              int64_t pos, int size)
+{
+    QEMUBdrvFile *s = opaque;
+    ssize_t len = bdrv_getlength(s->bds) - (s->offset + pos);
+
+    if (len <= 0) {
+        return 0;
+    }
+
+    if (len > size) {
+        len = size;
+    }
+
+    len = bdrv_pread(s->bds, pos + s->offset, buf, len);
+
+    return len;
+}
+
+static int qf_bdrv_close(void *opaque)
+{
+    QEMUBdrvFile *s = opaque;
+
+    g_free(s);
+
+    return 0;
+}
+
+static const QEMUFileOps bdrv_read_ops = {
+    .get_buffer = qf_bdrv_get_buffer,
+    .close =      qf_bdrv_close
+};
+
+static const QEMUFileOps bdrv_write_ops = {
+    .put_buffer = qf_bdrv_put_buffer,
+    .close =      qf_bdrv_close
+};
+
+
+QEMUFile *qemu_bdrv_open(const char *mode, BlockDriverState *bds,
+                         uint64_t offset)
+{
+    QEMUBdrvFile *s;
+
+    if (mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] 
!= 0) {
+        fprintf(stderr, "qemu_bdrv_open: Argument validity check 
failed\n");
+        return NULL;
+    }
+
+    s = g_malloc0(sizeof(QEMUBdrvFile));
+    if (!s) {
+        return NULL;
+    }
+
+    s->bds = bds;
+    s->offset = offset;
+
+    if (mode[0] == 'r') {
+        s->file = qemu_fopen_ops(s, &bdrv_read_ops);
+    } else {
+        s->file = qemu_fopen_ops(s, &bdrv_write_ops);
+    }
+    return s->file;
+}
+


Turns out this QEMUFile abstraction can be used for more than just 
migration...

Regards,
    Stefan

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-13 22:00     ` Stefan Berger
@ 2013-03-13 23:18       ` mdroth
  2013-03-14  1:48         ` Stefan Berger
  0 siblings, 1 reply; 33+ messages in thread
From: mdroth @ 2013-03-13 23:18 UTC (permalink / raw)
  To: Stefan Berger; +Cc: Joel Schopp, qemu-devel, Michael Tsirkin

On Wed, Mar 13, 2013 at 06:00:24PM -0400, Stefan Berger wrote:
> On 03/13/2013 04:52 PM, mdroth wrote:
> >On Wed, Mar 13, 2013 at 01:56:24PM -0500, Joel Schopp wrote:
> >>Add a sized buffer interface to qapi.
> >Isn't this just a special case of the visit_*_carray() interfaces? We
> >should avoid new interfaces if possible, since it adds to feature
> >disparities between visitor implementations.
> 
> Yes, it's a special case and carray seems more general.
> However, I don't understand the interface of carray. It has a
> start_carray with all parameters given, then a next_carray and an
> end_carray. Why do we need multiple calls if one call (start_carray)
> could be used to serialize all the data already?

Visitors don't have any knowledge of the data structures they're visiting
outside of what we tell them via the visit_*() API.

For output, visit_start_carray() only provides enough information to
instruct a visitor on how to calculate the offsets of each element (and
perhaps allocate some memory for it's own internal buffers etc.)

For input, it provides enough to allocate storage, and calculate offsets
to each element it's deserializing into.

As far as what to do with each element, we need to make additional calls
to instruct it.

For example, a visitor for a 16-element array of:

typedef struct ComplexType {
    int32_t foo;
    char *bar;
} ComplexType;

would look something like:

visit_start_carray(v, ...); // instruct visitor how to calculate offsets
for (i = 0; i < 16; i++) {
    visit_type_ComplexType(v, ...) // instruct visitor how to handle elem
    visit_next_carray(v, ...); // instruct visitor to move to next offset
}
visit_end_carray(v, ...); // instruct visitor to finalize array

> 
> Regards,
>     Stefan
> 
> 
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-13 23:18       ` mdroth
@ 2013-03-14  1:48         ` Stefan Berger
  2013-03-14 12:18           ` mdroth
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2013-03-14  1:48 UTC (permalink / raw)
  To: mdroth; +Cc: Joel Schopp, qemu-devel, Michael Tsirkin

On 03/13/2013 07:18 PM, mdroth wrote:
> On Wed, Mar 13, 2013 at 06:00:24PM -0400, Stefan Berger wrote:
>> On 03/13/2013 04:52 PM, mdroth wrote:
>>
> Visitors don't have any knowledge of the data structures they're visiting
> outside of what we tell them via the visit_*() API.
>
> [...]
>
> For example, a visitor for a 16-element array of:
>
> typedef struct ComplexType {
>      int32_t foo;
>      char *bar;
> } ComplexType;
>
> would look something like:
>
> visit_start_carray(v, ...); // instruct visitor how to calculate offsets
> for (i = 0; i < 16; i++) {
>      visit_type_ComplexType(v, ...) // instruct visitor how to handle elem
>      visit_next_carray(v, ...); // instruct visitor to move to next offset
> }
> visit_end_carray(v, ...); // instruct visitor to finalize array

Given this example above, I think we will need the sized buffer. The 
sized buffer targets  binary arrays and their encoding. If I was to 
encode an 'unsigned char[n]' (e.g., n=200) using n, or n/2 or n/4 loops 
like above breaking it apart in u8, u16 or u32 respectively I think this 
would 'not bed good' also considering the 2 bytes for tag and length 
being added by ASN.1 for every such datatype (u8,u16,u32). The sized 
buffer allows you to for example take a memory page and write it out in 
one chunk adding a few bytes of ASN.1 'decoration' around the actual data.

    Stefan

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 3/9] two new file wrappers
  2013-03-13 21:04   ` Eric Blake
@ 2013-03-14 10:49     ` Stefan Berger
  0 siblings, 0 replies; 33+ messages in thread
From: Stefan Berger @ 2013-03-14 10:49 UTC (permalink / raw)
  To: Eric Blake; +Cc: Joel Schopp, qemu-devel, Michael Tsirkin

On 03/13/2013 05:04 PM, Eric Blake wrote:
> On 03/13/2013 12:56 PM, Joel Schopp wrote:
>> Add a 3 very short file wrapper functions to make code that follows more
> s/a 3/3/
>
>> readable.  Also export an existing function that is currently static.
>>
>> Cc: Michael Tsirkin <mst@redhat.com>
>> 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 |    3 +++
>>   util/qemu-file.c              |   30 ++++++++++++++++++++++++++++++
>>   2 files changed, 33 insertions(+)
>>
>> +
>> +int qemu_peek_bytes(QEMUFile *f, uint8_t *buf, int size, size_t offset)
> Should we be using off_t instead of size_t for offset; and size_t
> instead of int for size (which implies ssize_t for return value)?
>

Good question. Existing functions have signatures like this:

void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
static void qemu_file_skip(QEMUFile *f, int size)
static int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t 
offset)
static int qemu_peek_byte(QEMUFile *f, int offset)

So it's better if we line up behind them...

    Stefan

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-14  1:48         ` Stefan Berger
@ 2013-03-14 12:18           ` mdroth
  2013-03-14 13:39             ` Stefan Berger
  0 siblings, 1 reply; 33+ messages in thread
From: mdroth @ 2013-03-14 12:18 UTC (permalink / raw)
  To: Stefan Berger; +Cc: Joel Schopp, qemu-devel, Michael Tsirkin

On Wed, Mar 13, 2013 at 09:48:11PM -0400, Stefan Berger wrote:
> On 03/13/2013 07:18 PM, mdroth wrote:
> >On Wed, Mar 13, 2013 at 06:00:24PM -0400, Stefan Berger wrote:
> >>On 03/13/2013 04:52 PM, mdroth wrote:
> >>
> >Visitors don't have any knowledge of the data structures they're visiting
> >outside of what we tell them via the visit_*() API.
> >
> >[...]
> >
> >For example, a visitor for a 16-element array of:
> >
> >typedef struct ComplexType {
> >     int32_t foo;
> >     char *bar;
> >} ComplexType;
> >
> >would look something like:
> >
> >visit_start_carray(v, ...); // instruct visitor how to calculate offsets
> >for (i = 0; i < 16; i++) {
> >     visit_type_ComplexType(v, ...) // instruct visitor how to handle elem
> >     visit_next_carray(v, ...); // instruct visitor to move to next offset
> >}
> >visit_end_carray(v, ...); // instruct visitor to finalize array
> 
> Given this example above, I think we will need the sized buffer. The
> sized buffer targets  binary arrays and their encoding. If I was to
> encode an 'unsigned char[n]' (e.g., n=200) using n, or n/2 or n/4
> loops like above breaking it apart in u8, u16 or u32 respectively I
> think this would 'not bed good' also considering the 2 bytes for tag
> and length being added by ASN.1 for every such datatype
> (u8,u16,u32). The sized buffer allows you to for example take a
> memory page and write it out in one chunk adding a few bytes of
> ASN.1 'decoration' around the actual data.

You could do it with this interface as well actually. The Visitor will
need to maintain some internal state to differentiate what it does with
subsequent visit_type*/visit_next_carray/visit_end_carry. There's no
reason it couldn't also track the elem size so it could tag a buffer
"en masse" when visit_end_carray() gets called.

QMP*Visitor does something similar already for building up lists/arrays
before tacking them onto the parent structure.

> 
>    Stefan
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-14 12:18           ` mdroth
@ 2013-03-14 13:39             ` Stefan Berger
  2013-03-14 14:28               ` mdroth
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2013-03-14 13:39 UTC (permalink / raw)
  To: mdroth; +Cc: Joel Schopp, qemu-devel, Michael Tsirkin

On 03/14/2013 08:18 AM, mdroth wrote:
> On Wed, Mar 13, 2013 at 09:48:11PM -0400, Stefan Berger wrote:
>> On 03/13/2013 07:18 PM, mdroth wrote:
>>> On Wed, Mar 13, 2013 at 06:00:24PM -0400, Stefan Berger wrote:
>>>> On 03/13/2013 04:52 PM, mdroth wrote:
>>>>
>>> Visitors don't have any knowledge of the data structures they're visiting
>>> outside of what we tell them via the visit_*() API.
>>>
>>> [...]
>>>
>>> For example, a visitor for a 16-element array of:
>>>
>>> typedef struct ComplexType {
>>>      int32_t foo;
>>>      char *bar;
>>> } ComplexType;
>>>
>>> would look something like:
>>>
>>> visit_start_carray(v, ...); // instruct visitor how to calculate offsets
>>> for (i = 0; i < 16; i++) {
>>>      visit_type_ComplexType(v, ...) // instruct visitor how to handle elem
>>>      visit_next_carray(v, ...); // instruct visitor to move to next offset
>>> }
>>> visit_end_carray(v, ...); // instruct visitor to finalize array
>> Given this example above, I think we will need the sized buffer. The
>> sized buffer targets  binary arrays and their encoding. If I was to
>> encode an 'unsigned char[n]' (e.g., n=200) using n, or n/2 or n/4
>> loops like above breaking it apart in u8, u16 or u32 respectively I
>> think this would 'not bed good' also considering the 2 bytes for tag
>> and length being added by ASN.1 for every such datatype
>> (u8,u16,u32). The sized buffer allows you to for example take a
>> memory page and write it out in one chunk adding a few bytes of
>> ASN.1 'decoration' around the actual data.
> You could do it with this interface as well actually. The Visitor will
> need to maintain some internal state to differentiate what it does with
> subsequent visit_type*/visit_next_carray/visit_end_carry. There's no
> reason it couldn't also track the elem size so it could tag a buffer
> "en masse" when visit_end_carray() gets called.

It depends on what you pass into visit_start_carray. In your case if you 
pass in ComplexType you would pass in a sizeof(ComplexType) for the size 
of each element presumably. The problem is now you have char *foo, a 
string pointer, hanging off of this structure. How would you handle 
that? Serializing ComplexType's foo and pointer obviously won't do it. 
You need to follow the string pointer and serialize that as well. So we 
have different use cases here when wanting to serialize ComplexType 
versus a plain array with the carray calls somehow having to figure it 
out themselves -- how ??

    Stefan

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-14 13:39             ` Stefan Berger
@ 2013-03-14 14:28               ` mdroth
  2013-03-14 14:51                 ` Stefan Berger
  0 siblings, 1 reply; 33+ messages in thread
From: mdroth @ 2013-03-14 14:28 UTC (permalink / raw)
  To: Stefan Berger; +Cc: Joel Schopp, qemu-devel, Michael Tsirkin

On Thu, Mar 14, 2013 at 09:39:14AM -0400, Stefan Berger wrote:
> On 03/14/2013 08:18 AM, mdroth wrote:
> >On Wed, Mar 13, 2013 at 09:48:11PM -0400, Stefan Berger wrote:
> >>On 03/13/2013 07:18 PM, mdroth wrote:
> >>>On Wed, Mar 13, 2013 at 06:00:24PM -0400, Stefan Berger wrote:
> >>>>On 03/13/2013 04:52 PM, mdroth wrote:
> >>>>
> >>>Visitors don't have any knowledge of the data structures they're visiting
> >>>outside of what we tell them via the visit_*() API.
> >>>
> >>>[...]
> >>>
> >>>For example, a visitor for a 16-element array of:
> >>>
> >>>typedef struct ComplexType {
> >>>     int32_t foo;
> >>>     char *bar;
> >>>} ComplexType;
> >>>
> >>>would look something like:
> >>>
> >>>visit_start_carray(v, ...); // instruct visitor how to calculate offsets
> >>>for (i = 0; i < 16; i++) {
> >>>     visit_type_ComplexType(v, ...) // instruct visitor how to handle elem
> >>>     visit_next_carray(v, ...); // instruct visitor to move to next offset
> >>>}
> >>>visit_end_carray(v, ...); // instruct visitor to finalize array
> >>Given this example above, I think we will need the sized buffer. The
> >>sized buffer targets  binary arrays and their encoding. If I was to
> >>encode an 'unsigned char[n]' (e.g., n=200) using n, or n/2 or n/4
> >>loops like above breaking it apart in u8, u16 or u32 respectively I
> >>think this would 'not bed good' also considering the 2 bytes for tag
> >>and length being added by ASN.1 for every such datatype
> >>(u8,u16,u32). The sized buffer allows you to for example take a
> >>memory page and write it out in one chunk adding a few bytes of
> >>ASN.1 'decoration' around the actual data.
> >You could do it with this interface as well actually. The Visitor will
> >need to maintain some internal state to differentiate what it does with
> >subsequent visit_type*/visit_next_carray/visit_end_carry. There's no
> >reason it couldn't also track the elem size so it could tag a buffer
> >"en masse" when visit_end_carray() gets called.
> 
> It depends on what you pass into visit_start_carray. In your case if
> you pass in ComplexType you would pass in a sizeof(ComplexType) for
> the size of each element presumably. The problem is now you have
> char *foo, a string pointer, hanging off of this structure. How
> would you handle that? Serializing ComplexType's foo and pointer
> obviously won't do it.

Why not?  visit_type_ComplexType() knows how to deal with
the individual fields, including the string pointer. I'm not sure
what's at issue here.

In this case the handling for ComplexType would look something like:

visit_type_Complex:
    visit_start_struct
    visit_type_uin32 //foo
    visit_type_str //bar
    visit_end_struct

Granted, strings are easier to deal with. If char * was instead a plain
old uint8_t*, we'd need a nested call to start_carray for each element.
in this case it would look something like:

visit_type_Complex:
    visit_start_struct
    visit_type_uin32 //foo field
    visit_start_carray //bar field
    for (i = 0; i < len_of_bar; i++):
        visit_type_uint8
        visit_next_carray
    visit_end_carray
    visit_end_struct

The key is knowing the length. In open coded visitor routines we know
this, or where to get it, for routines generated from QAPI schemas
we'd a way to tell the code generators how to field the size, or state
the size in the schema directly. I had some patches to do this, but we
don't have a QAPI user that needs this yet. When we do,
visit_*_carray() should be able to handle it, so we should consolidate
around that interface since there are a lot of things to consider in
the scope of what a visitor implementation may be used for.

> would you handle that? Serializing ComplexType's foo and pointer
> obviously won't do it. You need to follow the string pointer and
> serialize that as well. So we have different use cases here when
> wanting to serialize ComplexType versus a plain array with the
> carray calls somehow having to figure it out themselves -- how ??

for a plain array we'd just replace visit_type_ComplexType() with
visit_type_uint{8,16,32,64} and change loop/elem_size params
accordingly.

> 
>    Stefan
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-14 14:28               ` mdroth
@ 2013-03-14 14:51                 ` Stefan Berger
  2013-03-14 15:11                   ` mdroth
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2013-03-14 14:51 UTC (permalink / raw)
  To: mdroth; +Cc: Joel Schopp, qemu-devel, Michael Tsirkin

On 03/14/2013 10:28 AM, mdroth wrote:
> On Thu, Mar 14, 2013 at 09:39:14AM -0400, Stefan Berger wrote:
>> On 03/14/2013 08:18 AM, mdroth wrote:
>>> On Wed, Mar 13, 2013 at 09:48:11PM -0400, Stefan Berger wrote:
>>>> On 03/13/2013 07:18 PM, mdroth wrote:
>>>>> On Wed, Mar 13, 2013 at 06:00:24PM -0400, Stefan Berger wrote:
>>>>>> On 03/13/2013 04:52 PM, mdroth wrote:
>>>>>>
>>>>> Visitors don't have any knowledge of the data structures they're visiting
>>>>> outside of what we tell them via the visit_*() API.
>>>>>
>>>>> [...]
>>>>>
>>>>> For example, a visitor for a 16-element array of:
>>>>>
>>>>> typedef struct ComplexType {
>>>>>      int32_t foo;
>>>>>      char *bar;
>>>>> } ComplexType;
>>>>>
>>>>> would look something like:
>>>>>
>>>>> visit_start_carray(v, ...); // instruct visitor how to calculate offsets
>>>>> for (i = 0; i < 16; i++) {
>>>>>      visit_type_ComplexType(v, ...) // instruct visitor how to handle elem
>>>>>      visit_next_carray(v, ...); // instruct visitor to move to next offset
>>>>> }
>>>>> visit_end_carray(v, ...); // instruct visitor to finalize array
>>>> Given this example above, I think we will need the sized buffer. The
>>>> sized buffer targets  binary arrays and their encoding. If I was to
>>>> encode an 'unsigned char[n]' (e.g., n=200) using n, or n/2 or n/4
>>>> loops like above breaking it apart in u8, u16 or u32 respectively I
>>>> think this would 'not bed good' also considering the 2 bytes for tag
>>>> and length being added by ASN.1 for every such datatype
>>>> (u8,u16,u32). The sized buffer allows you to for example take a
>>>> memory page and write it out in one chunk adding a few bytes of
>>>> ASN.1 'decoration' around the actual data.
>>> You could do it with this interface as well actually. The Visitor will
>>> need to maintain some internal state to differentiate what it does with
>>> subsequent visit_type*/visit_next_carray/visit_end_carry. There's no
>>> reason it couldn't also track the elem size so it could tag a buffer
>>> "en masse" when visit_end_carray() gets called.
>> It depends on what you pass into visit_start_carray. In your case if
>> you pass in ComplexType you would pass in a sizeof(ComplexType) for
>> the size of each element presumably. The problem is now you have
>> char *foo, a string pointer, hanging off of this structure. How
>> would you handle that? Serializing ComplexType's foo and pointer
>> obviously won't do it.
> Why not?  visit_type_ComplexType() knows how to deal with
> the individual fields, including the string pointer. I'm not sure
> what's at issue here.
>
> In this case the handling for ComplexType would look something like:
>
> visit_type_Complex:
>      visit_start_struct
>      visit_type_uin32 //foo
>      visit_type_str //bar
>      visit_end_struct
>
> Granted, strings are easier to deal with. If char * was instead a plain
> old uint8_t*, we'd need a nested call to start_carray for each element.
> in this case it would look something like:
>
> visit_type_Complex:
>      visit_start_struct
>      visit_type_uin32 //foo field
>      visit_start_carray //bar field
>      for (i = 0; i < len_of_bar; i++):
>          visit_type_uint8
>          visit_next_carray
>      visit_end_carray

You really want to create a separate element for each element in this 
potentially large binary array? I guess it depends on the underlying 
data, but this has the potential of generating a lot of control code 
around each such byte... As said, for ASN.1 encoding, each such byte 
would be decorated with a tag and a length value, consuming 2 more bytes 
per byte.

    Stefan

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-14 14:51                 ` Stefan Berger
@ 2013-03-14 15:11                   ` mdroth
  2013-03-14 15:24                     ` Stefan Berger
  0 siblings, 1 reply; 33+ messages in thread
From: mdroth @ 2013-03-14 15:11 UTC (permalink / raw)
  To: Stefan Berger; +Cc: Joel Schopp, qemu-devel, Michael Tsirkin

On Thu, Mar 14, 2013 at 10:51:49AM -0400, Stefan Berger wrote:
> On 03/14/2013 10:28 AM, mdroth wrote:
> >On Thu, Mar 14, 2013 at 09:39:14AM -0400, Stefan Berger wrote:
> >>On 03/14/2013 08:18 AM, mdroth wrote:
> >>>On Wed, Mar 13, 2013 at 09:48:11PM -0400, Stefan Berger wrote:
> >>>>On 03/13/2013 07:18 PM, mdroth wrote:
> >>>>>On Wed, Mar 13, 2013 at 06:00:24PM -0400, Stefan Berger wrote:
> >>>>>>On 03/13/2013 04:52 PM, mdroth wrote:
> >>>>>>
> >>>>>Visitors don't have any knowledge of the data structures they're visiting
> >>>>>outside of what we tell them via the visit_*() API.
> >>>>>
> >>>>>[...]
> >>>>>
> >>>>>For example, a visitor for a 16-element array of:
> >>>>>
> >>>>>typedef struct ComplexType {
> >>>>>     int32_t foo;
> >>>>>     char *bar;
> >>>>>} ComplexType;
> >>>>>
> >>>>>would look something like:
> >>>>>
> >>>>>visit_start_carray(v, ...); // instruct visitor how to calculate offsets
> >>>>>for (i = 0; i < 16; i++) {
> >>>>>     visit_type_ComplexType(v, ...) // instruct visitor how to handle elem
> >>>>>     visit_next_carray(v, ...); // instruct visitor to move to next offset
> >>>>>}
> >>>>>visit_end_carray(v, ...); // instruct visitor to finalize array
> >>>>Given this example above, I think we will need the sized buffer. The
> >>>>sized buffer targets  binary arrays and their encoding. If I was to
> >>>>encode an 'unsigned char[n]' (e.g., n=200) using n, or n/2 or n/4
> >>>>loops like above breaking it apart in u8, u16 or u32 respectively I
> >>>>think this would 'not bed good' also considering the 2 bytes for tag
> >>>>and length being added by ASN.1 for every such datatype
> >>>>(u8,u16,u32). The sized buffer allows you to for example take a
> >>>>memory page and write it out in one chunk adding a few bytes of
> >>>>ASN.1 'decoration' around the actual data.
> >>>You could do it with this interface as well actually. The Visitor will
> >>>need to maintain some internal state to differentiate what it does with
> >>>subsequent visit_type*/visit_next_carray/visit_end_carry. There's no
> >>>reason it couldn't also track the elem size so it could tag a buffer
> >>>"en masse" when visit_end_carray() gets called.
> >>It depends on what you pass into visit_start_carray. In your case if
> >>you pass in ComplexType you would pass in a sizeof(ComplexType) for
> >>the size of each element presumably. The problem is now you have
> >>char *foo, a string pointer, hanging off of this structure. How
> >>would you handle that? Serializing ComplexType's foo and pointer
> >>obviously won't do it.
> >Why not?  visit_type_ComplexType() knows how to deal with
> >the individual fields, including the string pointer. I'm not sure
> >what's at issue here.
> >
> >In this case the handling for ComplexType would look something like:
> >
> >visit_type_Complex:
> >     visit_start_struct
> >     visit_type_uin32 //foo
> >     visit_type_str //bar
> >     visit_end_struct
> >
> >Granted, strings are easier to deal with. If char * was instead a plain
> >old uint8_t*, we'd need a nested call to start_carray for each element.
> >in this case it would look something like:
> >
> >visit_type_Complex:
> >     visit_start_struct
> >     visit_type_uin32 //foo field
> >     visit_start_carray //bar field
> >     for (i = 0; i < len_of_bar; i++):
> >         visit_type_uint8
> >         visit_next_carray
> >     visit_end_carray
> 
> You really want to create a separate element for each element in
> this potentially large binary array? I guess it depends on the
> underlying data, but this has the potential of generating a lot of
> control code around each such byte... As said, for ASN.1 encoding,
> each such byte would be decorated with a tag and a length value,
> consuming 2 more bytes per byte.

I addressed this earlier. Your visitor doesn't have tag each
element: if it know it's handling an array (because we told it via
start_carray()), it can buffer them internally and tag the array en
masse when end_carray() is issued.

> 
>    Stefan
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-14 15:11                   ` mdroth
@ 2013-03-14 15:24                     ` Stefan Berger
  2013-03-14 21:06                       ` mdroth
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2013-03-14 15:24 UTC (permalink / raw)
  To: mdroth; +Cc: Joel Schopp, qemu-devel, Michael Tsirkin

On 03/14/2013 11:11 AM, mdroth wrote:
> On Thu, Mar 14, 2013 at 10:51:49AM -0400, Stefan Berger wrote:
>> On 03/14/2013 10:28 AM, mdroth wrote:
>>> On Thu, Mar 14, 2013 at 09:39:14AM -0400, Stefan Berger wrote:
>>>> On 03/14/2013 08:18 AM, mdroth wrote:
>>>>> On Wed, Mar 13, 2013 at 09:48:11PM -0400, Stefan Berger wrote:
>>>>>> On 03/13/2013 07:18 PM, mdroth wrote:
>>>>>>> On Wed, Mar 13, 2013 at 06:00:24PM -0400, Stefan Berger wrote:
>>>>>>>> On 03/13/2013 04:52 PM, mdroth wrote:
>>>>>>>>
>>>>>>> Visitors don't have any knowledge of the data structures they're visiting
>>>>>>> outside of what we tell them via the visit_*() API.
>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>> For example, a visitor for a 16-element array of:
>>>>>>>
>>>>>>> typedef struct ComplexType {
>>>>>>>      int32_t foo;
>>>>>>>      char *bar;
>>>>>>> } ComplexType;
>>>>>>>
>>>>>>> would look something like:
>>>>>>>
>>>>>>> visit_start_carray(v, ...); // instruct visitor how to calculate offsets
>>>>>>> for (i = 0; i < 16; i++) {
>>>>>>>      visit_type_ComplexType(v, ...) // instruct visitor how to handle elem
>>>>>>>      visit_next_carray(v, ...); // instruct visitor to move to next offset
>>>>>>> }
>>>>>>> visit_end_carray(v, ...); // instruct visitor to finalize array
>>>>>> Given this example above, I think we will need the sized buffer. The
>>>>>> sized buffer targets  binary arrays and their encoding. If I was to
>>>>>> encode an 'unsigned char[n]' (e.g., n=200) using n, or n/2 or n/4
>>>>>> loops like above breaking it apart in u8, u16 or u32 respectively I
>>>>>> think this would 'not bed good' also considering the 2 bytes for tag
>>>>>> and length being added by ASN.1 for every such datatype
>>>>>> (u8,u16,u32). The sized buffer allows you to for example take a
>>>>>> memory page and write it out in one chunk adding a few bytes of
>>>>>> ASN.1 'decoration' around the actual data.
>>>>> You could do it with this interface as well actually. The Visitor will
>>>>> need to maintain some internal state to differentiate what it does with
>>>>> subsequent visit_type*/visit_next_carray/visit_end_carry. There's no
>>>>> reason it couldn't also track the elem size so it could tag a buffer
>>>>> "en masse" when visit_end_carray() gets called.
>>>> It depends on what you pass into visit_start_carray. In your case if
>>>> you pass in ComplexType you would pass in a sizeof(ComplexType) for
>>>> the size of each element presumably. The problem is now you havechar *foo, a string pointer, hanging off of this structure. How
>>>> would you handle that? Serializing ComplexType's foo and pointer
>>>> obviously won't do it.
>>> Why not?  visit_type_ComplexType() knows how to deal with
>>> the individual fields, including the string pointer. I'm not sure
>>> what's at issue here.
>>>
>>> In this case the handling for ComplexType would look something like:
>>>
>>> visit_type_Complex:
>>>      visit_start_struct
>>>      visit_type_uin32 //foo
>>>      visit_type_str //bar
>>>      visit_end_struct
>>>
>>> Granted, strings are easier to deal with. If char * was instead a plain
>>> old uint8_t*, we'd need a nested call to start_carray for each element.
>>> in this case it would look something like:
>>>
>>> visit_type_Complex:
>>>      visit_start_struct
>>>      visit_type_uin32 //foo field
>>>      visit_start_carray //bar field
>>>      for (i = 0; i < len_of_bar; i++):
>>>          visit_type_uint8
>>>          visit_next_carray
>>>      visit_end_carray
>> You really want to create a separate element for each element in
>> this potentially large binary array? I guess it depends on the
>> underlying data, but this has the potential of generating a lot of
>> control code around each such byte... As said, for ASN.1 encoding,
>> each such byte would be decorated with a tag and a length value,
>> consuming 2 more bytes per byte.
> I addressed this earlier. Your visitor doesn't have tag each
> element: if it know it's handling an array (because we told it via
> start_carray()), it can buffer them internally and tag the array en
> masse when end_carray() is issued.

If we were to do this using carray on an array of structs of the 
following type

struct SimpleStruct {
     uint8_t a;
     uint8_t b;
     uint32_t c;
}

then the serialization of a and b would be buffered and flushed once the 
32bit output visitor (or any other than uint8_t output visitor) would be 
called? Now this does makes the implementation a lot more difficult.

   Stefan

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-14 15:24                     ` Stefan Berger
@ 2013-03-14 21:06                       ` mdroth
  2013-03-15  2:05                         ` Stefan Berger
  0 siblings, 1 reply; 33+ messages in thread
From: mdroth @ 2013-03-14 21:06 UTC (permalink / raw)
  To: Stefan Berger; +Cc: Joel Schopp, qemu-devel, Michael Tsirkin

On Thu, Mar 14, 2013 at 11:24:03AM -0400, Stefan Berger wrote:
> On 03/14/2013 11:11 AM, mdroth wrote:
> >On Thu, Mar 14, 2013 at 10:51:49AM -0400, Stefan Berger wrote:
> >>On 03/14/2013 10:28 AM, mdroth wrote:
> >>>On Thu, Mar 14, 2013 at 09:39:14AM -0400, Stefan Berger wrote:
> >>>>On 03/14/2013 08:18 AM, mdroth wrote:
> >>>>>On Wed, Mar 13, 2013 at 09:48:11PM -0400, Stefan Berger wrote:
> >>>>>>On 03/13/2013 07:18 PM, mdroth wrote:
> >>>>>>>On Wed, Mar 13, 2013 at 06:00:24PM -0400, Stefan Berger wrote:
> >>>>>>>>On 03/13/2013 04:52 PM, mdroth wrote:
> >>>>>>>>
> >>>>>>>Visitors don't have any knowledge of the data structures they're visiting
> >>>>>>>outside of what we tell them via the visit_*() API.
> >>>>>>>
> >>>>>>>[...]
> >>>>>>>
> >>>>>>>For example, a visitor for a 16-element array of:
> >>>>>>>
> >>>>>>>typedef struct ComplexType {
> >>>>>>>     int32_t foo;
> >>>>>>>     char *bar;
> >>>>>>>} ComplexType;
> >>>>>>>
> >>>>>>>would look something like:
> >>>>>>>
> >>>>>>>visit_start_carray(v, ...); // instruct visitor how to calculate offsets
> >>>>>>>for (i = 0; i < 16; i++) {
> >>>>>>>     visit_type_ComplexType(v, ...) // instruct visitor how to handle elem
> >>>>>>>     visit_next_carray(v, ...); // instruct visitor to move to next offset
> >>>>>>>}
> >>>>>>>visit_end_carray(v, ...); // instruct visitor to finalize array
> >>>>>>Given this example above, I think we will need the sized buffer. The
> >>>>>>sized buffer targets  binary arrays and their encoding. If I was to
> >>>>>>encode an 'unsigned char[n]' (e.g., n=200) using n, or n/2 or n/4
> >>>>>>loops like above breaking it apart in u8, u16 or u32 respectively I
> >>>>>>think this would 'not bed good' also considering the 2 bytes for tag
> >>>>>>and length being added by ASN.1 for every such datatype
> >>>>>>(u8,u16,u32). The sized buffer allows you to for example take a
> >>>>>>memory page and write it out in one chunk adding a few bytes of
> >>>>>>ASN.1 'decoration' around the actual data.
> >>>>>You could do it with this interface as well actually. The Visitor will
> >>>>>need to maintain some internal state to differentiate what it does with
> >>>>>subsequent visit_type*/visit_next_carray/visit_end_carry. There's no
> >>>>>reason it couldn't also track the elem size so it could tag a buffer
> >>>>>"en masse" when visit_end_carray() gets called.
> >>>>It depends on what you pass into visit_start_carray. In your case if
> >>>>you pass in ComplexType you would pass in a sizeof(ComplexType) for
> >>>>the size of each element presumably. The problem is now you havechar *foo, a string pointer, hanging off of this structure. How
> >>>>would you handle that? Serializing ComplexType's foo and pointer
> >>>>obviously won't do it.
> >>>Why not?  visit_type_ComplexType() knows how to deal with
> >>>the individual fields, including the string pointer. I'm not sure
> >>>what's at issue here.
> >>>
> >>>In this case the handling for ComplexType would look something like:
> >>>
> >>>visit_type_Complex:
> >>>     visit_start_struct
> >>>     visit_type_uin32 //foo
> >>>     visit_type_str //bar
> >>>     visit_end_struct
> >>>
> >>>Granted, strings are easier to deal with. If char * was instead a plain
> >>>old uint8_t*, we'd need a nested call to start_carray for each element.
> >>>in this case it would look something like:
> >>>
> >>>visit_type_Complex:
> >>>     visit_start_struct
> >>>     visit_type_uin32 //foo field
> >>>     visit_start_carray //bar field
> >>>     for (i = 0; i < len_of_bar; i++):
> >>>         visit_type_uint8
> >>>         visit_next_carray
> >>>     visit_end_carray
> >>You really want to create a separate element for each element in
> >>this potentially large binary array? I guess it depends on the
> >>underlying data, but this has the potential of generating a lot of
> >>control code around each such byte... As said, for ASN.1 encoding,
> >>each such byte would be decorated with a tag and a length value,
> >>consuming 2 more bytes per byte.
> >I addressed this earlier. Your visitor doesn't have tag each
> >element: if it know it's handling an array (because we told it via
> >start_carray()), it can buffer them internally and tag the array en
> >masse when end_carray() is issued.
> 
> If we were to do this using carray on an array of structs of the
> following type
> 
> struct SimpleStruct {
>     uint8_t a;
>     uint8_t b;
>     uint32_t c;
> }
> 
> then the serialization of a and b would be buffered and flushed once
> the 32bit output visitor (or any other than uint8_t output visitor)
> would be called?

I don't quite understand. For a struct, we'd tag each field
individually, right?

It's avoiding the need to tag each element in a list, each SimpleStruct,
that's at issue, right? We have a special case for u8 arrays that is
currently handled by qapi_sized_buffer(), and now we're trying to
generalize this optimized handling for more complex data types via
visit_carray_*?

I guess my first question is whether or not it's possible for more complex
data types. For u8 arrays we seem to use a special OCTET_STRING encoding.

If we're not sure we can do this more generally, we do have the option
of only special-casing u8 arrays to use the OCTET_STRING encoding, and
handle the others with "non-optimized" encodings.

If you have an idea for what a generalized, optimized encoding that's
applicable for non-u8 types would look like, we can work through that if
you have an example optimized encoding for, say, and array of
SimpleStruct.

> 
>   Stefan
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer
  2013-03-14 21:06                       ` mdroth
@ 2013-03-15  2:05                         ` Stefan Berger
  0 siblings, 0 replies; 33+ messages in thread
From: Stefan Berger @ 2013-03-15  2:05 UTC (permalink / raw)
  To: mdroth; +Cc: Joel Schopp, qemu-devel, Michael Tsirkin

On 03/14/2013 05:06 PM, mdroth wrote:
> On Thu, Mar 14, 2013 at 11:24:03AM -0400, Stefan Berger wrote:
>> On 03/14/2013 11:11 AM, mdroth wrote:
>>> On Thu, Mar 14, 2013 at 10:51:49AM -0400, Stefan Berger wrote:
>>>> On 03/14/2013 10:28 AM, mdroth wrote:
>>>>> On Thu, Mar 14, 2013 at 09:39:14AM -0400, Stefan Berger wrote:
>>>>>> On 03/14/2013 08:18 AM, mdroth wrote:
>>>>>>> On Wed, Mar 13, 2013 at 09:48:11PM -0400, Stefan Berger wrote:
>>>>>>>> On 03/13/2013 07:18 PM, mdroth wrote:
>>>>>>>>> On Wed, Mar 13, 2013 at 06:00:24PM -0400, Stefan Berger wrote:
>>>>>>>>>> On 03/13/2013 04:52 PM, mdroth wrote:
>>>>>>>>>>
>>>>>>>>> Visitors don't have any knowledge of the data structures they're visiting
>>>>>>>>> outside of what we tell them via the visit_*() API.
>>>>>>>>>
>>>>>>>>> [...]
>>>>>>>>>
>>>>>>>>> For example, a visitor for a 16-element array of:
>>>>>>>>>
>>>>>>>>> typedef struct ComplexType {
>>>>>>>>>      int32_t foo;
>>>>>>>>>      char *bar;
>>>>>>>>> } ComplexType;
>>>>>>>>>
>>>>>>>>> would look something like:
>>>>>>>>>
>>>>>>>>> visit_start_carray(v, ...); // instruct visitor how to calculate offsets
>>>>>>>>> for (i = 0; i < 16; i++) {
>>>>>>>>>      visit_type_ComplexType(v, ...) // instruct visitor how to handle elem
>>>>>>>>>      visit_next_carray(v, ...); // instruct visitor to move to next offset
>>>>>>>>> }
>>>>>>>>> visit_end_carray(v, ...); // instruct visitor to finalize array
>>>>>>>> Given this example above, I think we will need the sized buffer. The
>>>>>>>> sized buffer targets  binary arrays and their encoding. If I was to
>>>>>>>> encode an 'unsigned char[n]' (e.g., n=200) using n, or n/2 or n/4
>>>>>>>> loops like above breaking it apart in u8, u16 or u32 respectively I
>>>>>>>> think this would 'not bed good' also considering the 2 bytes for tag
>>>>>>>> and length being added by ASN.1 for every such datatype
>>>>>>>> (u8,u16,u32). The sized buffer allows you to for example take a
>>>>>>>> memory page and write it out in one chunk adding a few bytes of
>>>>>>>> ASN.1 'decoration' around the actual data.
>>>>>>> You could do it with this interface as well actually. The Visitor will
>>>>>>> need to maintain some internal state to differentiate what it does with
>>>>>>> subsequent visit_type*/visit_next_carray/visit_end_carry. There's no
>>>>>>> reason it couldn't also track the elem size so it could tag a buffer
>>>>>>> "en masse" when visit_end_carray() gets called.
>>>>>> It depends on what you pass into visit_start_carray. In your case if
>>>>>> you pass in ComplexType you would pass in a sizeof(ComplexType) for
>>>>>> the size of each element presumably. The problem is now you havechar *foo, a string pointer, hanging off of this structure. How
>>>>>> would you handle that? Serializing ComplexType's foo and pointer
>>>>>> obviously won't do it.
>>>>> Why not?  visit_type_ComplexType() knows how to deal with
>>>>> the individual fields, including the string pointer. I'm not sure
>>>>> what's at issue here.
>>>>>
>>>>> In this case the handling for ComplexType would look something like:
>>>>>
>>>>> visit_type_Complex:
>>>>>      visit_start_struct
>>>>>      visit_type_uin32 //foo
>>>>>      visit_type_str //bar
>>>>>      visit_end_struct
>>>>>
>>>>> Granted, strings are easier to deal with. If char * was instead a plain
>>>>> old uint8_t*, we'd need a nested call to start_carray for each element.
>>>>> in this case it would look something like:
>>>>>
>>>>> visit_type_Complex:
>>>>>      visit_start_struct
>>>>>      visit_type_uin32 //foo field
>>>>>      visit_start_carray //bar field
>>>>>      for (i = 0; i < len_of_bar; i++):
>>>>>          visit_type_uint8
>>>>>          visit_next_carray
>>>>>      visit_end_carray
>>>> You really want to create a separate element for each element in
>>>> this potentially large binary array? I guess it depends on the
>>>> underlying data, but this has the potential of generating a lot of
>>>> control code around each such byte... As said, for ASN.1 encoding,
>>>> each such byte would be decorated with a tag and a length value,
>>>> consuming 2 more bytes per byte.
>>> I addressed this earlier. Your visitor doesn't have tag each
>>> element: if it know it's handling an array (because we told it via
>>> start_carray()), it can buffer them internally and tag the array en
>>> masse when end_carray() is issued.
>> If we were to do this using carray on an array of structs of the
>> following type
>>
>> struct SimpleStruct {
>>      uint8_t a;
>>      uint8_t b;
>>      uint32_t c;
>> }
>>
>> then the serialization of a and b would be buffered and flushed once
>> the 32bit output visitor (or any other than uint8_t output visitor)
>> would be called?
> I don't quite understand. For a struct, we'd tag each field
> individually, right?
>
> It's avoiding the need to tag each element in a list, each SimpleStruct,
> that's at issue, right? We have a special case for u8 arrays that is
> currently handled by qapi_sized_buffer(), and now we're trying to
> generalize this optimized handling for more complex data types via
> visit_carray_*?

I have to admit there's one mistake in the sized buffer implementation 
and that is it should take the width of each element so each element can 
be encoded in network byte order, which I think is the key point here so 
the bytestream becomes portable Using the element width we can then walk 
the array of n elements and write them out in network byte order in one 
go. I also see the sized buffer more as a generalization of the string 
visitor that just happens to work on a null terminated array of bytes.

Using the carray it seems we would create special cases in the 
implementation for when a u8, u16, u32, or u64 follows versus a string, 
list or any more complex data type. The examples above seem to need to 
buffer the u8 and then write them out versus when it was to handle an 
array of structs. With the sized buffer we can do this all with a single 
call.

> I guess my first question is whether or not it's possible for more complex
> data types. For u8 arrays we seem to use a special OCTET_STRING encoding.
>
> If we're not sure we can do this more generally, we do have the option
> of only special-casing u8 arrays to use the OCTET_STRING encoding, and
> handle the others with "non-optimized" encodings.
>
> If you have an idea for what a generalized, optimized encoding that's
> applicable for non-u8 types would look like, we can work through that if
> you have an example optimized encoding for, say, and array of
> SimpleStruct.
For an array of SimpleStruct we use the carray. No endianess conversion 
is necessary here for the structure as a whole but each u16,u32, u64 
inside such a structure will be written out correctly with their 
respective visitor. A u16[] or u32[] inside that structure would then be 
handled with a sized buffer walking the array and normalizing each 
element's endianess.

Strictly speaking, for ASN.1 encoding we don't need a carray visitor 
(but maybe other visitor types need it). It can be simulated with the 
struct visitor, which in effect also causes one more level of nesting, 
just that the identifier would be different, which really is the only 
difference then.

    Stefan

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2013-03-15  2:06 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-13 18:56 [Qemu-devel] [PATCH 0/9 v3] Implement and test asn1 ber visitors Joel Schopp
2013-03-13 18:56 ` [Qemu-devel] [PATCH 1/9] qemu-file Joel Schopp
2013-03-13 18:56 ` [Qemu-devel] [PATCH 2/9] qapi_c_arrays.diff Joel Schopp
2013-03-13 19:11   ` Anthony Liguori
2013-03-13 22:54   ` Stefan Berger
2013-03-13 18:56 ` [Qemu-devel] [PATCH 3/9] two new file wrappers Joel Schopp
2013-03-13 21:04   ` Eric Blake
2013-03-14 10:49     ` Stefan Berger
2013-03-13 18:56 ` [Qemu-devel] [PATCH 4/9] qemu_qsb.diff Joel Schopp
2013-03-13 21:11   ` mdroth
2013-03-13 21:28     ` Stefan Berger
2013-03-13 22:41       ` mdroth
2013-03-13 22:47         ` mdroth
2013-03-13 23:11           ` Stefan Berger
2013-03-13 18:56 ` [Qemu-devel] [PATCH 5/9] qapi_sized_buffer Joel Schopp
2013-03-13 20:52   ` mdroth
2013-03-13 22:00     ` Stefan Berger
2013-03-13 23:18       ` mdroth
2013-03-14  1:48         ` Stefan Berger
2013-03-14 12:18           ` mdroth
2013-03-14 13:39             ` Stefan Berger
2013-03-14 14:28               ` mdroth
2013-03-14 14:51                 ` Stefan Berger
2013-03-14 15:11                   ` mdroth
2013-03-14 15:24                     ` Stefan Berger
2013-03-14 21:06                       ` mdroth
2013-03-15  2:05                         ` Stefan Berger
2013-03-13 18:56 ` [Qemu-devel] [PATCH 6/9] asn1_output-visitor.diff Joel Schopp
2013-03-13 18:56 ` [Qemu-devel] [PATCH 7/9] asn1_input-visitor.diff Joel Schopp
2013-03-13 18:56 ` [Qemu-devel] [PATCH 8/9] asn1_test_visitor_serialization.diff Joel Schopp
2013-03-13 18:56 ` [Qemu-devel] [PATCH 9/9] update_maintainers.diff Joel Schopp
  -- strict thread matches above, loose matches on Subject: below --
2013-03-13  3:09 [Qemu-devel] [PATCH 0/9 v2] Implement and test asn1 ber visitors Joel Schopp
2013-03-13  3:09 ` [Qemu-devel] [PATCH 2/9] qapi_c_arrays.diff Joel Schopp
2013-03-13 12:08   ` Eric Blake

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).