[Qemu-devel] [PATCH v0 00/10]: More QObject conversions

[Qemu-devel] [PATCH v0 00/10]: More QObject conversions

[Luiz Capitulino]

 Hi there,

 Here goes another set of conversions, just to keep the ball rolling.

 The first patch introduces a function called qobject_from_fmt(), which can
be used to build QObjects from a specified printf-like format.

 Most of its code was based on a Python function with similiar functionality,
as far as I could understand the license is compatible and I've added a
copyright notice.

 Is this the right procedure?

 Most patches are simple (as we aren't converting errors yet), except the
last ones. Which use qobject_from_fmt() and are very cool, hopefully they're
right too. :)

PS: This series apply on top of my last 'Initial QObject conversion' series.

 Thanks.

[Qemu-devel] [PATCH 01/10] Introduce qmisc module

[Luiz Capitulino]

This module provides miscellania QObject functions.

Currently it exports qobject_from_fmt(), which is somewhat
based on Python's Py_BuildValue() function. It is capable of
creating QObjects from a specified string format.

For example, to create a QDict with mixed data-types one
could do:

QObject *obj = qobject_from_fmt("{ s: [ i, s ], s: i }", ... );

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 Makefile |    2 +-
 qmisc.c  |  222 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qmisc.h  |   19 +++++
 3 files changed, 242 insertions(+), 1 deletions(-)
 create mode 100644 qmisc.c
 create mode 100644 qmisc.h

diff --git a/Makefile b/Makefile
index d96fb4b..182f176 100644
--- a/Makefile
+++ b/Makefile
@@ -100,7 +100,7 @@ obj-y += buffered_file.o migration.o migration-tcp.o net.o qemu-sockets.o
 obj-y += qemu-char.o aio.o net-checksum.o savevm.o
 obj-y += msmouse.o ps2.o
 obj-y += qdev.o qdev-properties.o ssi.o
-obj-y += qint.o qstring.o qdict.o qlist.o qemu-config.o
+obj-y += qint.o qstring.o qdict.o qlist.o qmisc.o qemu-config.o
 
 obj-$(CONFIG_BRLAPI) += baum.o
 obj-$(CONFIG_WIN32) += tap-win32.o
diff --git a/qmisc.c b/qmisc.c
new file mode 100644
index 0000000..42b6f22
--- /dev/null
+++ b/qmisc.c
@@ -0,0 +1,222 @@
+/*
+ * Misc QObject functions.
+ *
+ * Copyright (C) 2009 Red Hat Inc.
+ *
+ * Authors:
+ *  Luiz Capitulino <lcapitulino@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+#include "qmisc.h"
+#include "qint.h"
+#include "qlist.h"
+#include "qdict.h"
+#include "qstring.h"
+#include "qobject.h"
+#include "qemu-common.h"
+
+/*
+ * qobject_from_fmt() and related functions are based on the Python's
+ * Py_BuildValue() and are subject to the Python Software Foundation
+ * License Version 2.
+ *
+ * Copyright (c) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009 Python
+ * Software Foundation.
+ */
+static QObject *build_qobject(const char **fmt, va_list *args);
+
+static QObject *do_mkdict(const char **fmt, va_list *args, int endchar,
+                          int nr_elems)
+{
+    int i;
+    QDict *qdict;
+
+    if (nr_elems < 0)
+        return NULL;
+
+    qdict = qdict_new();
+
+    for (i = 0; i < nr_elems; i += 2) {
+        QString *qs;
+        QObject *obj, *key;
+
+        key = build_qobject(fmt, args);
+        if (!key)
+            goto err;
+
+        obj = build_qobject(fmt, args);
+        if (!obj) {
+            qobject_decref(key);
+            goto err;
+        }
+
+        qs = qobject_to_qstring(key);
+        qdict_put_obj(qdict, qstring_get_str(qs), obj);
+        QDECREF(qs);
+    }
+
+    while (**fmt == ' ')
+        (*fmt)++;
+
+    if (**fmt != endchar)
+        goto err;
+
+    if (endchar)
+        ++*fmt;
+
+    return QOBJECT(qdict);
+
+err:
+    QDECREF(qdict);
+    return NULL;
+}
+
+static QObject *do_mklist(const char **fmt, va_list *args, int endchar,
+                          int nr_elems)
+{
+    int i;
+    QList *qlist;
+
+    if (nr_elems < 0)
+        return NULL;
+
+    qlist = qlist_new();
+
+    for (i = 0; i < nr_elems; i++) {
+        QObject *obj = build_qobject(fmt, args);
+        if (!obj)
+            goto err;
+        qlist_append_obj(qlist, obj);
+    }
+
+    while (**fmt == ' ')
+        (*fmt)++;
+
+    if (**fmt != endchar)
+        goto err;
+
+    if (endchar)
+        ++*fmt;
+
+    return QOBJECT(qlist);
+
+err:
+    QDECREF(qlist);
+    return NULL;
+}
+
+static int
+count_format(const char *format, int endchar)
+{
+    int count = 0;
+    int level = 0;
+
+    while (level > 0 || *format != endchar) {
+        switch (*format) {
+        case '\0':
+            /* Premature end */
+            return -1;
+        case '[':
+        case '{':
+            if (level == 0)
+                count++;
+            level++;
+            break;
+        case ']':
+        case '}':
+            level--;
+            break;
+        case ',':
+        case ':':
+        case ' ':
+        case '\t':
+            break;
+        default:
+            if (level == 0)
+                count++;
+        }
+        format++;
+    }
+    return count;
+}
+
+static QObject *build_qobject(const char **fmt, va_list *args)
+{
+    for (;;) {
+        switch (*(*fmt)++) {
+            case 'i':
+            {
+                QInt *qi = qint_from_int((int64_t) va_arg(*args, int64_t));
+                return QOBJECT(qi);
+            }
+            case 's':
+            {
+                QString *qs = qstring_from_str((char *) va_arg(*args, char *));
+                return QOBJECT(qs);
+            }
+            case '[':
+                return do_mklist(fmt, args, ']', count_format(*fmt, ']'));
+            case '{':
+                return do_mkdict(fmt, args, '}', count_format(*fmt, '}'));
+            case ',':
+            case ':':
+            case ' ':
+            case '\t':
+                break;
+            default:
+                return NULL;
+        }
+    }
+}
+
+/**
+ * qobject_from_fmt(): build QObjects from a specified format.
+ *
+ * Valid characters of the format:
+ *
+ * i   integer, map to QInt
+ * s   string, map to QString
+ * []  list, map to QList
+ * {}  dictionary, map to QDict
+ *
+ * Examples:
+ *
+ * - Create a QInt
+ *
+ *      qobject_from_fmt("i", 42);
+ *
+ * - Create a QList of QStrings
+ *
+ *      qobject_from_fmt("[ i, i, i ]", 0, 1 , 2);
+ *
+ * - Create a QDict with mixed data-types
+ *
+ *      qobject_from_fmt("{ s: [ i, s ], s: i }", ... );
+ *
+ * Return a strong reference to a QObject on success, NULL otherwise.
+ */
+QObject *qobject_from_fmt(const char *fmt, ...)
+{
+    va_list args;
+    QObject *obj;
+
+    va_start(args, fmt);
+    switch (*fmt) {
+        case '[':
+            fmt++;
+            obj = do_mklist(&fmt, &args, ']', count_format(fmt, ']'));
+            break;
+        case '{':
+            fmt++;
+            obj = do_mkdict(&fmt, &args, '}', count_format(fmt, '}'));
+            break;
+        default:
+            obj = build_qobject(&fmt, &args);
+            break;
+    }
+    va_end(args);
+
+    return obj;
+}
diff --git a/qmisc.h b/qmisc.h
new file mode 100644
index 0000000..ac481fe
--- /dev/null
+++ b/qmisc.h
@@ -0,0 +1,19 @@
+/*
+ * QMisc header file.
+ *
+ * Copyright (C) 2009 Red Hat Inc.
+ *
+ * Authors:
+ *  Luiz Capitulino <lcapitulino@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+#ifndef QMISC_H
+#define QMISC_H
+
+#include "qobject.h"
+
+QObject *qobject_from_fmt(const char *fmt, ...);
+
+#endif /* QMISC_H */
-- 
1.6.5.rc2.17.gdbc1b

Re: [Qemu-devel] [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Luiz Capitulino wrote:
> This module provides miscellania QObject functions.
>
> Currently it exports qobject_from_fmt(), which is somewhat
> based on Python's Py_BuildValue() function. It is capable of
> creating QObjects from a specified string format.
>
> For example, to create a QDict with mixed data-types one
> could do:
>
> QObject *obj = qobject_from_fmt("{ s: [ i, s ], s: i }", ... );
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  Makefile |    2 +-
>  qmisc.c  |  222 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qmisc.h  |   19 +++++
>  3 files changed, 242 insertions(+), 1 deletions(-)
>  create mode 100644 qmisc.c
>  create mode 100644 qmisc.h
>
> diff --git a/Makefile b/Makefile
> index d96fb4b..182f176 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -100,7 +100,7 @@ obj-y += buffered_file.o migration.o migration-tcp.o net.o qemu-sockets.o
>  obj-y += qemu-char.o aio.o net-checksum.o savevm.o
>  obj-y += msmouse.o ps2.o
>  obj-y += qdev.o qdev-properties.o ssi.o
> -obj-y += qint.o qstring.o qdict.o qlist.o qemu-config.o
> +obj-y += qint.o qstring.o qdict.o qlist.o qmisc.o qemu-config.o
>  
>  obj-$(CONFIG_BRLAPI) += baum.o
>  obj-$(CONFIG_WIN32) += tap-win32.o
> diff --git a/qmisc.c b/qmisc.c
> new file mode 100644
> index 0000000..42b6f22
> --- /dev/null
> +++ b/qmisc.c
> @@ -0,0 +1,222 @@
> +/*
> + * Misc QObject functions.
> + *
> + * Copyright (C) 2009 Red Hat Inc.
> + *
> + * Authors:
> + *  Luiz Capitulino <lcapitulino@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + */
> +#include "qmisc.h"
> +#include "qint.h"
> +#include "qlist.h"
> +#include "qdict.h"
> +#include "qstring.h"
> +#include "qobject.h"
> +#include "qemu-common.h"
> +
> +/*
> + * qobject_from_fmt() and related functions are based on the Python's
> + * Py_BuildValue() and are subject to the Python Software Foundation
> + * License Version 2.
> + *
> + * Copyright (c) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009 Python
> + * Software Foundation.
> + */
>   

If we're introducing third-party code under a new license, we need to 
update the top-level LICENSE file.  I took a brief look and it wasn't 
immediately clear that this license is GPL compatible.  According to the 
FSF, certain versions of this license are incompatible and some are 
compatible.  I think it would have been better to just write something 
from scratch...

> +            case '[':
> +                return do_mklist(fmt, args, ']', count_format(*fmt, ']'));
>   

Because this is bizarre.  It looks ahead to count the number of 
arguments which is a very strange way to parse something like this.

Why not a simple recursive decent parser?

> +/**
> + * qobject_from_fmt(): build QObjects from a specified format.
> + *
> + * Valid characters of the format:
> + *
> + * i   integer, map to QInt
> + * s   string, map to QString
> + * []  list, map to QList
> + * {}  dictionary, map to QDict
> + *
> + * Examples:
> + *
> + * - Create a QInt
> + *
> + *      qobject_from_fmt("i", 42);
> + *
> + * - Create a QList of QStrings
> + *
> + *      qobject_from_fmt("[ i, i, i ]", 0, 1 , 2);
> + *
> + * - Create a QDict with mixed data-types
> + *
> + *      qobject_from_fmt("{ s: [ i, s ], s: i }", ... );
> + *
> + * Return a strong reference to a QObject on success, NULL otherwise.
> + */
>   

But my real objection is that we should make this "{%s: [%d, %s], %s: 
%d}" so that we can mark it as a printf formatted function and get type 
checking.  You'll probably have to support both "%d" and "%" PRId64 for 
sanity sake.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 01/10] Introduce qmisc module

[Luiz Capitulino]

On Thu, 15 Oct 2009 09:02:48 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> Luiz Capitulino wrote:
> > This module provides miscellania QObject functions.
> >
> > Currently it exports qobject_from_fmt(), which is somewhat
> > based on Python's Py_BuildValue() function. It is capable of
> > creating QObjects from a specified string format.
> >
> > For example, to create a QDict with mixed data-types one
> > could do:
> >
> > QObject *obj = qobject_from_fmt("{ s: [ i, s ], s: i }", ... );
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  Makefile |    2 +-
> >  qmisc.c  |  222 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  qmisc.h  |   19 +++++
> >  3 files changed, 242 insertions(+), 1 deletions(-)
> >  create mode 100644 qmisc.c
> >  create mode 100644 qmisc.h
> >
> > diff --git a/Makefile b/Makefile
> > index d96fb4b..182f176 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -100,7 +100,7 @@ obj-y += buffered_file.o migration.o migration-tcp.o net.o qemu-sockets.o
> >  obj-y += qemu-char.o aio.o net-checksum.o savevm.o
> >  obj-y += msmouse.o ps2.o
> >  obj-y += qdev.o qdev-properties.o ssi.o
> > -obj-y += qint.o qstring.o qdict.o qlist.o qemu-config.o
> > +obj-y += qint.o qstring.o qdict.o qlist.o qmisc.o qemu-config.o
> >  
> >  obj-$(CONFIG_BRLAPI) += baum.o
> >  obj-$(CONFIG_WIN32) += tap-win32.o
> > diff --git a/qmisc.c b/qmisc.c
> > new file mode 100644
> > index 0000000..42b6f22
> > --- /dev/null
> > +++ b/qmisc.c
> > @@ -0,0 +1,222 @@
> > +/*
> > + * Misc QObject functions.
> > + *
> > + * Copyright (C) 2009 Red Hat Inc.
> > + *
> > + * Authors:
> > + *  Luiz Capitulino <lcapitulino@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.  See
> > + * the COPYING file in the top-level directory.
> > + */
> > +#include "qmisc.h"
> > +#include "qint.h"
> > +#include "qlist.h"
> > +#include "qdict.h"
> > +#include "qstring.h"
> > +#include "qobject.h"
> > +#include "qemu-common.h"
> > +
> > +/*
> > + * qobject_from_fmt() and related functions are based on the Python's
> > + * Py_BuildValue() and are subject to the Python Software Foundation
> > + * License Version 2.
> > + *
> > + * Copyright (c) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009 Python
> > + * Software Foundation.
> > + */
> >   
> 
> If we're introducing third-party code under a new license, we need to 
> update the top-level LICENSE file.  I took a brief look and it wasn't 
> immediately clear that this license is GPL compatible.  According to the 
> FSF, certain versions of this license are incompatible and some are 
> compatible.  I think it would have been better to just write something 
> from scratch...

 According to the Python's LICENSE file it's compatible since 2001
(2.0.1 release).

> > +            case '[':
> > +                return do_mklist(fmt, args, ']', count_format(*fmt, ']'));
> >   
> 
> Because this is bizarre.  It looks ahead to count the number of 
> arguments which is a very strange way to parse something like this.
> 
> Why not a simple recursive decent parser?

 I could try it, but I think this is going to take some time as
I would have to read more about it.

 I thought the Python's implementation was a good idea as we're short
in time and it was easy to adapt and is widely used in production.

> > +/**
> > + * qobject_from_fmt(): build QObjects from a specified format.
> > + *
> > + * Valid characters of the format:
> > + *
> > + * i   integer, map to QInt
> > + * s   string, map to QString
> > + * []  list, map to QList
> > + * {}  dictionary, map to QDict
> > + *
> > + * Examples:
> > + *
> > + * - Create a QInt
> > + *
> > + *      qobject_from_fmt("i", 42);
> > + *
> > + * - Create a QList of QStrings
> > + *
> > + *      qobject_from_fmt("[ i, i, i ]", 0, 1 , 2);
> > + *
> > + * - Create a QDict with mixed data-types
> > + *
> > + *      qobject_from_fmt("{ s: [ i, s ], s: i }", ... );
> > + *
> > + * Return a strong reference to a QObject on success, NULL otherwise.
> > + */
> >   
> 
> But my real objection is that we should make this "{%s: [%d, %s], %s: 
> %d}" so that we can mark it as a printf formatted function and get type 
> checking.  You'll probably have to support both "%d" and "%" PRId64 for 
> sanity sake.

 Trivial to do if we ignore the '%' characters. :))

Re: [Qemu-devel] [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Luiz Capitulino wrote:
>>> +/**
>>> + * qobject_from_fmt(): build QObjects from a specified format.
>>> + *
>>> + * Valid characters of the format:
>>> + *
>>> + * i   integer, map to QInt
>>> + * s   string, map to QString
>>> + * []  list, map to QList
>>> + * {}  dictionary, map to QDict
>>> + *
>>> + * Examples:
>>> + *
>>> + * - Create a QInt
>>> + *
>>> + *      qobject_from_fmt("i", 42);
>>> + *
>>> + * - Create a QList of QStrings
>>> + *
>>> + *      qobject_from_fmt("[ i, i, i ]", 0, 1 , 2);
>>> + *
>>> + * - Create a QDict with mixed data-types
>>> + *
>>> + *      qobject_from_fmt("{ s: [ i, s ], s: i }", ... );
>>> + *
>>> + * Return a strong reference to a QObject on success, NULL otherwise.
>>> + */
>>>   
>>>       
>> But my real objection is that we should make this "{%s: [%d, %s], %s: 
>> %d}" so that we can mark it as a printf formatted function and get type 
>> checking.  You'll probably have to support both "%d" and "%" PRId64 for 
>> sanity sake.
>>     
>
>  Trivial to do if we ignore the '%' characters. :))
>   
Okay, I'd like to see that and I'd like to see string parsing.  We can 
rewrite the parser later.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 01/10] Introduce qmisc module

[Luiz Capitulino]

On Thu, 15 Oct 2009 10:35:16 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> Luiz Capitulino wrote:
> >>> +/**
> >>> + * qobject_from_fmt(): build QObjects from a specified format.
> >>> + *
> >>> + * Valid characters of the format:
> >>> + *
> >>> + * i   integer, map to QInt
> >>> + * s   string, map to QString
> >>> + * []  list, map to QList
> >>> + * {}  dictionary, map to QDict
> >>> + *
> >>> + * Examples:
> >>> + *
> >>> + * - Create a QInt
> >>> + *
> >>> + *      qobject_from_fmt("i", 42);
> >>> + *
> >>> + * - Create a QList of QStrings
> >>> + *
> >>> + *      qobject_from_fmt("[ i, i, i ]", 0, 1 , 2);
> >>> + *
> >>> + * - Create a QDict with mixed data-types
> >>> + *
> >>> + *      qobject_from_fmt("{ s: [ i, s ], s: i }", ... );
> >>> + *
> >>> + * Return a strong reference to a QObject on success, NULL otherwise.
> >>> + */
> >>>   
> >>>       
> >> But my real objection is that we should make this "{%s: [%d, %s], %s: 
> >> %d}" so that we can mark it as a printf formatted function and get type 
> >> checking.  You'll probably have to support both "%d" and "%" PRId64 for 
> >> sanity sake.
> >>     
> >
> >  Trivial to do if we ignore the '%' characters. :))
> >   
> Okay, I'd like to see that and I'd like to see string parsing.  We can 
> rewrite the parser later.

 Great!

 I have some concerns about string parsing though, can we make it very
simple? Like, only valid as keys and not expect anything fancy inside
the string delimiters?

 Otherwise I think it would be easier to make it part of a grammar, 
which sounds like a good future improvement.

Re: [Qemu-devel] [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Luiz Capitulino wrote:
>>
>> Okay, I'd like to see that and I'd like to see string parsing.  We can 
>> rewrite the parser later.
>>     
>
>  Great!
>
>  I have some concerns about string parsing though, can we make it very
> simple? Like, only valid as keys and not expect anything fancy inside
> the string delimiters?
>   

Yup.  My main concern is that I don't want to convert all of the various 
commands only to have to revisit them to introduce a new formatting syntax.

>  Otherwise I think it would be easier to make it part of a grammar, 
> which sounds like a good future improvement.
>   

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Luiz Capitulino wrote:
>  Great!
>
>  I have some concerns about string parsing though, can we make it very
> simple? Like, only valid as keys and not expect anything fancy inside
> the string delimiters?
>
>  Otherwise I think it would be easier to make it part of a grammar, 
> which sounds like a good future improvement.
>   
You know, once we have a json parser, we'll probably just want to turn 
this into an asprintf() that feeds into the json parser.

So just make sure that the format is valid json.  In fact, it may be 
better to name this qobject_from_json().

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 01/10] Introduce qmisc module

[Daniel P. Berrange]

On Thu, Oct 15, 2009 at 12:26:22PM -0300, Luiz Capitulino wrote:
> On Thu, 15 Oct 2009 09:02:48 -0500
> Anthony Liguori <anthony@codemonkey.ws> wrote:
> 
> > 
> > If we're introducing third-party code under a new license, we need to 
> > update the top-level LICENSE file.  I took a brief look and it wasn't 
> > immediately clear that this license is GPL compatible.  According to the 
> > FSF, certain versions of this license are incompatible and some are 
> > compatible.  I think it would have been better to just write something 
> > from scratch...
> 
>  According to the Python's LICENSE file it's compatible since 2001
> (2.0.1 release).
> 
> > > +            case '[':
> > > +                return do_mklist(fmt, args, ']', count_format(*fmt, ']'));
> > >   
> > 
> > Because this is bizarre.  It looks ahead to count the number of 
> > arguments which is a very strange way to parse something like this.
> > 
> > Why not a simple recursive decent parser?
> 
>  I could try it, but I think this is going to take some time as
> I would have to read more about it.
> 
>  I thought the Python's implementation was a good idea as we're short
> in time and it was easy to adapt and is widely used in production.

There are at least 6  standalone, pure C [1] json parsers available 
already, some of which let you do json formatting too. So writing a
new parser, or untangling one from python seems like more trouble than
its worth to me. Likewise for generating formatted JSON output.

For QMP experimentation in libvirt I've started off by just grabbing
the json.h & json.c files from an existing impl

  http://mjson.svn.sourceforge.net/viewvc/mjson/trunk/src/

which are LGPLv2+  licensed, and copied them into libvirt source tree. 
They provide quite a nice simple API for both parsing & formatting
and integrate very easily with our codebase.

Regards,
Daniel

[1] See http://json.org/
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 01/10] Introduce qmisc module

[Daniel P. Berrange]

On Thu, Oct 15, 2009 at 05:39:36PM +0100, Daniel P. Berrange wrote:
> On Thu, Oct 15, 2009 at 12:26:22PM -0300, Luiz Capitulino wrote:
> > On Thu, 15 Oct 2009 09:02:48 -0500
> > Anthony Liguori <anthony@codemonkey.ws> wrote:
> > 
> > > 
> > > If we're introducing third-party code under a new license, we need to 
> > > update the top-level LICENSE file.  I took a brief look and it wasn't 
> > > immediately clear that this license is GPL compatible.  According to the 
> > > FSF, certain versions of this license are incompatible and some are 
> > > compatible.  I think it would have been better to just write something 
> > > from scratch...
> > 
> >  According to the Python's LICENSE file it's compatible since 2001
> > (2.0.1 release).
> > 
> > > > +            case '[':
> > > > +                return do_mklist(fmt, args, ']', count_format(*fmt, ']'));
> > > >   
> > > 
> > > Because this is bizarre.  It looks ahead to count the number of 
> > > arguments which is a very strange way to parse something like this.
> > > 
> > > Why not a simple recursive decent parser?
> > 
> >  I could try it, but I think this is going to take some time as
> > I would have to read more about it.
> > 
> >  I thought the Python's implementation was a good idea as we're short
> > in time and it was easy to adapt and is widely used in production.
> 
> There are at least 6  standalone, pure C [1] json parsers available 
> already, some of which let you do json formatting too. So writing a
> new parser, or untangling one from python seems like more trouble than
> its worth to me. Likewise for generating formatted JSON output.

Opps. Ignore me. I'm replying to totally the wrong email / context here :-)

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 01/10] Introduce qmisc module

[Luiz Capitulino]

On Thu, 15 Oct 2009 17:39:36 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Thu, Oct 15, 2009 at 12:26:22PM -0300, Luiz Capitulino wrote:
> > On Thu, 15 Oct 2009 09:02:48 -0500
> > Anthony Liguori <anthony@codemonkey.ws> wrote:
> > 
> > > 
> > > If we're introducing third-party code under a new license, we need to 
> > > update the top-level LICENSE file.  I took a brief look and it wasn't 
> > > immediately clear that this license is GPL compatible.  According to the 
> > > FSF, certain versions of this license are incompatible and some are 
> > > compatible.  I think it would have been better to just write something 
> > > from scratch...
> > 
> >  According to the Python's LICENSE file it's compatible since 2001
> > (2.0.1 release).
> > 
> > > > +            case '[':
> > > > +                return do_mklist(fmt, args, ']', count_format(*fmt, ']'));
> > > >   
> > > 
> > > Because this is bizarre.  It looks ahead to count the number of 
> > > arguments which is a very strange way to parse something like this.
> > > 
> > > Why not a simple recursive decent parser?
> > 
> >  I could try it, but I think this is going to take some time as
> > I would have to read more about it.
> > 
> >  I thought the Python's implementation was a good idea as we're short
> > in time and it was easy to adapt and is widely used in production.
> 
> There are at least 6  standalone, pure C [1] json parsers available 
> already, some of which let you do json formatting too. So writing a
> new parser, or untangling one from python seems like more trouble than
> its worth to me. Likewise for generating formatted JSON output.

 Not the right context but I was going to post about this soon, so
I think this is a good opportunity to talk about it.

 I didn't look at all available parsers from json.org, but this one:

http://fara.cs.uni-potsdam.de/~jsg/json_parser/

 Seems interesting.

 Anthony, are you ok in using external implementations like that
if they meet our requirements?

Re: [Qemu-devel] [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Luiz Capitulino wrote:
>  Not the right context but I was going to post about this soon, so
> I think this is a good opportunity to talk about it.
>
>  I didn't look at all available parsers from json.org, but this one:
>
> http://fara.cs.uni-potsdam.de/~jsg/json_parser/
>
>  Seems interesting.
>
>  Anthony, are you ok in using external implementations like that
> if they meet our requirements?
>   
If it's a library, it should be widely available (iow, packaged on all 
major distros) and there should be binaries available for Windows.

Otherwise, pulling the code into the tree isn't so bad provided that 
it's not huge.

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Paolo Bonzini]

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

On 10/15/2009 08:34 PM, Anthony Liguori wrote:
> Luiz Capitulino wrote:
>> Not the right context but I was going to post about this soon, so
>> I think this is a good opportunity to talk about it.
>>
>> I didn't look at all available parsers from json.org, but this one:
>>
>> http://fara.cs.uni-potsdam.de/~jsg/json_parser/
>>
>> Seems interesting.
>>
>> Anthony, are you ok in using external implementations like that
>> if they meet our requirements?
>
> Otherwise, pulling the code into the tree isn't so bad provided that
> it's not huge.

It's 36k, and pulling it in gives the opportunity to customize it.  For 
example, the attached patch allows to parse a "%BLAH" extension to JSON 
that is passed to the callback (since the parsing is done 
character-by-character, the callback can consume whatever it wants after 
the % sign).  Asprintf+parse JSON unfortunately isn't enough because 
you'd need to escape all strings.

Paolo

[-- Attachment #2: fff --]
[-- Type: text/plain, Size: 13172 bytes --]

>From aa76cd652d740f535a7dfb47d2c4ecbd5d28d47a Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Fri, 16 Oct 2009 15:22:01 +0200
Subject: [PATCH] add a % escape that is passed verbatim to the callback

This could be used to parse something like { %s : %s } and
fetch the values for the placeholders from an external va_list.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 JSON_parser.c |  104 +++++++++++++++++++++++++++++++++++----------------------
 JSON_parser.h |    2 +
 comments.json |    6 +++-
 main.c        |   11 ++++++
 4 files changed, 82 insertions(+), 41 deletions(-)

diff --git a/JSON_parser.c b/JSON_parser.c
index 93e98c8..4c360be 100644
--- a/JSON_parser.c
+++ b/JSON_parser.c
@@ -151,6 +151,7 @@ enum classes {
     C_E,      /* E */
     C_ETC,    /* everything else */
     C_STAR,   /* * */   
+    C_PCT,    /* % - user escape */
     NR_CLASSES
 };
 
@@ -165,7 +166,7 @@ static int ascii_class[128] = {
     __,      __,      __,      __,      __,      __,      __,      __,
     __,      __,      __,      __,      __,      __,      __,      __,
 
-    C_SPACE, C_ETC,   C_QUOTE, C_ETC,   C_ETC,   C_ETC,   C_ETC,   C_ETC,
+    C_SPACE, C_ETC,   C_QUOTE, C_ETC,   C_ETC,   C_PCT,   C_ETC,   C_ETC,
     C_ETC,   C_ETC,   C_STAR,   C_PLUS,  C_COMMA, C_MINUS, C_POINT, C_SLASH,
     C_ZERO,  C_DIGIT, C_DIGIT, C_DIGIT, C_DIGIT, C_DIGIT, C_DIGIT, C_DIGIT,
     C_DIGIT, C_DIGIT, C_COLON, C_ETC,   C_ETC,   C_ETC,   C_ETC,   C_ETC,
@@ -239,7 +240,8 @@ enum actions
     ZX = -19, /* integer detected by zero */
     IX = -20, /* integer detected by 1-9 */
     EX = -21, /* next char is escaped */
-    UC = -22  /* Unicode character read */
+    UC = -22, /* Unicode character read */
+    XC = -23, /* Escape to callback */
 };
 
 
@@ -251,43 +253,43 @@ static int state_transition_table[NR_STATES][NR_CLASSES] = {
     state is OK and if the mode is MODE_DONE.
 
                  white                                      1-9                                   ABCDF  etc
-             space |  {  }  [  ]  :  ,  "  \  /  +  -  .  0  |  a  b  c  d  e  f  l  n  r  s  t  u  |  E  |  * */
-/*start  GO*/ {GO,GO,-6,__,-5,__,__,__,__,__,CB,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
-/*ok     OK*/ {OK,OK,__,-8,__,-7,__,-3,__,__,CB,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
-/*object OB*/ {OB,OB,__,-9,__,__,__,__,SB,__,CB,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
-/*key    KE*/ {KE,KE,__,__,__,__,__,__,SB,__,CB,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
-/*colon  CO*/ {CO,CO,__,__,__,__,-2,__,__,__,CB,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
-/*value  VA*/ {VA,VA,-6,__,-5,__,__,__,SB,__,CB,__,MX,__,ZX,IX,__,__,__,__,__,FA,__,NU,__,__,TR,__,__,__,__,__},
-/*array  AR*/ {AR,AR,-6,__,-5,-7,__,__,SB,__,CB,__,MX,__,ZX,IX,__,__,__,__,__,FA,__,NU,__,__,TR,__,__,__,__,__},
-/*string ST*/ {ST,__,ST,ST,ST,ST,ST,ST,-4,EX,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST},
-/*escape ES*/ {__,__,__,__,__,__,__,__,ST,ST,ST,__,__,__,__,__,__,ST,__,__,__,ST,__,ST,ST,__,ST,U1,__,__,__,__},
-/*u1     U1*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,U2,U2,U2,U2,U2,U2,U2,U2,__,__,__,__,__,__,U2,U2,__,__},
-/*u2     U2*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,U3,U3,U3,U3,U3,U3,U3,U3,__,__,__,__,__,__,U3,U3,__,__},
-/*u3     U3*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,U4,U4,U4,U4,U4,U4,U4,U4,__,__,__,__,__,__,U4,U4,__,__},
-/*u4     U4*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,UC,UC,UC,UC,UC,UC,UC,UC,__,__,__,__,__,__,UC,UC,__,__},
-/*minus  MI*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,ZE,IT,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
-/*zero   ZE*/ {OK,OK,__,-8,__,-7,__,-3,__,__,CB,__,__,DF,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
-/*int    IT*/ {OK,OK,__,-8,__,-7,__,-3,__,__,CB,__,__,DF,IT,IT,__,__,__,__,DE,__,__,__,__,__,__,__,__,DE,__,__},
-/*frac   FR*/ {OK,OK,__,-8,__,-7,__,-3,__,__,CB,__,__,__,FR,FR,__,__,__,__,E1,__,__,__,__,__,__,__,__,E1,__,__},
-/*e      E1*/ {__,__,__,__,__,__,__,__,__,__,__,E2,E2,__,E3,E3,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
-/*ex     E2*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,E3,E3,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
-/*exp    E3*/ {OK,OK,__,-8,__,-7,__,-3,__,__,__,__,__,__,E3,E3,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
-/*tr     T1*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,T2,__,__,__,__,__,__,__},
-/*tru    T2*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,T3,__,__,__,__},
-/*true   T3*/ {__,__,__,__,__,__,__,__,__,__,CB,__,__,__,__,__,__,__,__,__,OK,__,__,__,__,__,__,__,__,__,__,__},
-/*fa     F1*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,F2,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
-/*fal    F2*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,F3,__,__,__,__,__,__,__,__,__},
-/*fals   F3*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,F4,__,__,__,__,__,__},
-/*false  F4*/ {__,__,__,__,__,__,__,__,__,__,CB,__,__,__,__,__,__,__,__,__,OK,__,__,__,__,__,__,__,__,__,__,__},
-/*nu     N1*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,N2,__,__,__,__},
-/*nul    N2*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,N3,__,__,__,__,__,__,__,__,__},
-/*null   N3*/ {__,__,__,__,__,__,__,__,__,__,CB,__,__,__,__,__,__,__,__,__,__,__,OK,__,__,__,__,__,__,__,__,__},
-/*/      C1*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,C2},
-/*/*     C2*/ {C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C3},
-/**      C3*/ {C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,CE,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C3},
-/*_.     FX*/ {OK,OK,__,-8,__,-7,__,-3,__,__,__,__,__,__,FR,FR,__,__,__,__,E1,__,__,__,__,__,__,__,__,E1,__,__},
-/*\      D1*/ {__,__,__,__,__,__,__,__,__,D2,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
-/*\      D2*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,U1,__,__,__,__},
+             space |  {  }  [  ]  :  ,  "  \  /  +  -  .  0  |  a  b  c  d  e  f  l  n  r  s  t  u  |  E  |  *  % */
+/*start  GO*/ {GO,GO,-6,__,-5,__,__,__,__,__,CB,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,XC},
+/*ok     OK*/ {OK,OK,__,-8,__,-7,__,-3,__,__,CB,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
+/*object OB*/ {OB,OB,__,-9,__,__,__,__,SB,__,CB,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,XC},
+/*key    KE*/ {KE,KE,__,__,__,__,__,__,SB,__,CB,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,XC},
+/*colon  CO*/ {CO,CO,__,__,__,__,-2,__,__,__,CB,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
+/*value  VA*/ {VA,VA,-6,__,-5,__,__,__,SB,__,CB,__,MX,__,ZX,IX,__,__,__,__,__,FA,__,NU,__,__,TR,__,__,__,__,__,XC},
+/*array  AR*/ {AR,AR,-6,__,-5,-7,__,__,SB,__,CB,__,MX,__,ZX,IX,__,__,__,__,__,FA,__,NU,__,__,TR,__,__,__,__,__,XC},
+/*string ST*/ {ST,__,ST,ST,ST,ST,ST,ST,-4,EX,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST,ST},
+/*escape ES*/ {__,__,__,__,__,__,__,__,ST,ST,ST,__,__,__,__,__,__,ST,__,__,__,ST,__,ST,ST,__,ST,U1,__,__,__,__,__},
+/*u1     U1*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,U2,U2,U2,U2,U2,U2,U2,U2,__,__,__,__,__,__,U2,U2,__,__,__},
+/*u2     U2*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,U3,U3,U3,U3,U3,U3,U3,U3,__,__,__,__,__,__,U3,U3,__,__,__},
+/*u3     U3*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,U4,U4,U4,U4,U4,U4,U4,U4,__,__,__,__,__,__,U4,U4,__,__,__},
+/*u4     U4*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,UC,UC,UC,UC,UC,UC,UC,UC,__,__,__,__,__,__,UC,UC,__,__,__},
+/*minus  MI*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,ZE,IT,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
+/*zero   ZE*/ {OK,OK,__,-8,__,-7,__,-3,__,__,CB,__,__,DF,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
+/*int    IT*/ {OK,OK,__,-8,__,-7,__,-3,__,__,CB,__,__,DF,IT,IT,__,__,__,__,DE,__,__,__,__,__,__,__,__,DE,__,__,__},
+/*frac   FR*/ {OK,OK,__,-8,__,-7,__,-3,__,__,CB,__,__,__,FR,FR,__,__,__,__,E1,__,__,__,__,__,__,__,__,E1,__,__,__},
+/*e      E1*/ {__,__,__,__,__,__,__,__,__,__,__,E2,E2,__,E3,E3,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
+/*ex     E2*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,E3,E3,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
+/*exp    E3*/ {OK,OK,__,-8,__,-7,__,-3,__,__,__,__,__,__,E3,E3,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
+/*tr     T1*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,T2,__,__,__,__,__,__,__,__},
+/*tru    T2*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,T3,__,__,__,__,__},
+/*true   T3*/ {__,__,__,__,__,__,__,__,__,__,CB,__,__,__,__,__,__,__,__,__,OK,__,__,__,__,__,__,__,__,__,__,__,__},
+/*fa     F1*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,F2,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
+/*fal    F2*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,F3,__,__,__,__,__,__,__,__,__,__},
+/*fals   F3*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,F4,__,__,__,__,__,__,__},
+/*false  F4*/ {__,__,__,__,__,__,__,__,__,__,CB,__,__,__,__,__,__,__,__,__,OK,__,__,__,__,__,__,__,__,__,__,__,__},
+/*nu     N1*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,N2,__,__,__,__,__},
+/*nul    N2*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,N3,__,__,__,__,__,__,__,__,__,__},
+/*null   N3*/ {__,__,__,__,__,__,__,__,__,__,CB,__,__,__,__,__,__,__,__,__,__,__,OK,__,__,__,__,__,__,__,__,__,__},
+/*/      C1*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,C2,__},
+/*/*     C2*/ {C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C3,C2},
+/**      C3*/ {C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,CE,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C2,C3,C2},
+/*_.     FX*/ {OK,OK,__,-8,__,-7,__,-3,__,__,__,__,__,__,FR,FR,__,__,__,__,E1,__,__,__,__,__,__,__,__,E1,__,__,__},
+/*\      D1*/ {__,__,__,__,__,__,__,__,__,D2,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__},
+/*\      D2*/ {__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,__,U1,__,__,__,__,__},
 };
 
 
@@ -680,7 +682,6 @@ JSON_parser_char(JSON_parser jc, int next_char)
             return false;
         }
     }
-    
     add_char_to_parse_buffer(jc, next_char, next_class);
     
 /*
@@ -818,6 +819,29 @@ JSON_parser_char(JSON_parser jc, int next_char)
             jc->state = C1;
             jc->comment = 1;
             break;
+
+/* external callback */
+	case XC:
+            parse_buffer_pop_back_char(jc);
+	    switch (jc->stack[jc->top])
+	    {
+	    case MODE_KEY:
+                jc->type = JSON_T_NONE;
+		jc->state = CO;
+                if (!(*jc->callback)(jc->ctx, JSON_T_USER_KEY, NULL)) {
+                    return false;
+                }
+	        break;
+	    default:
+	        jc->state = OK;
+                jc->type = JSON_T_NONE;
+                if (!(*jc->callback)(jc->ctx, JSON_T_USER, NULL)) {
+                    return false;
+                }
+	        break;
+	    }
+	    break;
+
 /* empty } */
         case -9:        
             parse_buffer_clear(jc);
diff --git a/JSON_parser.h b/JSON_parser.h
index 3780aae..50cec2d 100644
--- a/JSON_parser.h
+++ b/JSON_parser.h
@@ -47,6 +47,8 @@ typedef enum
     JSON_T_FALSE,
     JSON_T_STRING,
     JSON_T_KEY,
+    JSON_T_USER,
+    JSON_T_USER_KEY,
     JSON_T_MAX
 } JSON_type;
 
diff --git a/comments.json b/comments.json
index 244f5e3..ad79ab0 100644
--- a/comments.json
+++ b/comments.json
@@ -113,4 +113,8 @@
 0.1e1,
 1e-1,
 1e00,2e+00,2e-00
-,"rosebud", "\u005C"]/**                       ******/
\ No newline at end of file
+,"rosebud", "\u005C",/**                  %%%     *%%%***%**/
+[%s],
+{%d:%s},
+{"name":%s},
+%s]/**                       ******/
diff --git a/main.c b/main.c
index 6651e12..226b125 100644
--- a/main.c
+++ b/main.c
@@ -29,6 +29,7 @@ int main(int argc, char* argv[]) {
     
     config.depth                  = 20;
     config.callback               = &print;
+    config.callback_ctx           = &input;
     config.allow_comments         = 1;
     config.handle_floats_manually = 0;
     
@@ -142,6 +143,16 @@ static int print(void* ctx, int type, const JSON_value* value)
         s_IsKey = 0;
         printf("string: '%s'\n", value->vu.str.value);
         break;
+    case JSON_T_USER_KEY:
+        s_IsKey = 1;
+        print_indention();
+        printf("user key = %%%c, value = ", fgetc(*(FILE**) ctx));
+        break;
+    case JSON_T_USER:
+        if (!s_IsKey) print_indention();
+        s_IsKey = 0;
+        printf("user: %%%c\n", fgetc(*(FILE**) ctx));
+        break;
     default:
         assert(0);
         break;
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Paolo Bonzini wrote:
> It's 36k, and pulling it in gives the opportunity to customize it.  
> For example, the attached patch allows to parse a "%BLAH" extension to 
> JSON that is passed to the callback (since the parsing is done 
> character-by-character, the callback can consume whatever it wants 
> after the % sign).  Asprintf+parse JSON unfortunately isn't enough 
> because you'd need to escape all strings.

What's the state of this library's upstream?  Should we be pushing these 
changes there and then attempting to package it?

I'd rather pull this in a submodule, try to get it packaged properly, 
and then eventually drop the submodule.  I don't want us to fork the 
library unless we have to.

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Paolo Bonzini]

On 10/16/2009 03:45 PM, Anthony Liguori wrote:
> Paolo Bonzini wrote:
>> It's 36k, and pulling it in gives the opportunity to customize it. For
>> example, the attached patch allows to parse a "%BLAH" extension to
>> JSON that is passed to the callback (since the parsing is done
>> character-by-character, the callback can consume whatever it wants
>> after the % sign). Asprintf+parse JSON unfortunately isn't enough
>> because you'd need to escape all strings.
>
> What's the state of this library's upstream? Should we be pushing these
> changes there and then attempting to package it?

There's no repository, there's no mention of it in the author's blog, it 
has seen six changes in two years according to the file's heading.  The 
only reference on da Internet is at 
http://tech.groups.yahoo.com/group/json/message/928.

On the other hand, it's down to the point (it has no object model of 
it's own), and it is fully asynchronous since it works 
character-by-character which makes it easier to extend as in my patch above.

Paolo

[Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Paolo Bonzini wrote:
> On 10/16/2009 03:45 PM, Anthony Liguori wrote:
>> Paolo Bonzini wrote:
>>> It's 36k, and pulling it in gives the opportunity to customize it. For
>>> example, the attached patch allows to parse a "%BLAH" extension to
>>> JSON that is passed to the callback (since the parsing is done
>>> character-by-character, the callback can consume whatever it wants
>>> after the % sign). Asprintf+parse JSON unfortunately isn't enough
>>> because you'd need to escape all strings.
>>
>> What's the state of this library's upstream? Should we be pushing these
>> changes there and then attempting to package it?
>
> There's no repository, there's no mention of it in the author's blog, 
> it has seen six changes in two years according to the file's heading.  
> The only reference on da Internet is at 
> http://tech.groups.yahoo.com/group/json/message/928.
>
> On the other hand, it's down to the point (it has no object model of 
> it's own), and it is fully asynchronous since it works 
> character-by-character which makes it easier to extend as in my patch 
> above.

Ugh!  I hate people trying to be clever.  The copyright is:

/*
Copyright (c) 2005 JSON.org

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 shall be used for Good, not Evil.

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.
*/

"The Software shall be used for Good, not Evil." is added as part of the 
licensing text.  That screws up the otherwise X11 license and is highly 
unlikely to be GPL compatible.

We can't pull this into the tree or even link against it as a library.  
Try contacting the others and see about getting that silliness removed.

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Paolo Bonzini]

On 10/16/2009 07:38 PM, Anthony Liguori wrote:
>
> Ugh!  I hate people trying to be clever.

Grrr, good catch.

I wrote this to the guy via Yahoo! but I cannot see his email address, 
so I've no clue if the email will actually reach him.

 > Hi, the QEMU project is discussing using your JSON parser.  However, 
 > the sentence "The Software shall be used for Good, not Evil" that
 > appears in the file is too clever and (even though the humorous
 > intent is obvious) it could be considered GPL-incompatible (or
 > any-other-license-incompatible for that matter).
 >
 > Would you consider removing that sentence from http://fara.cs.uni-
 > potsdam.de/~jsg/json_parser/JSON_parser.c?  If you cannot, you can
 > send it to me and CC anthony@codemonkey.ws.
 >
 > Thanks in advance.
 >
 > Paolo Bonzini

There can always be a plan B---Dan Berrange found a parser with a 
similar interface and if the weather doesn't improve I may even give a 
shot at writing one over the weekend.

Paolo

[Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Paolo Bonzini wrote:
>
> > Thanks in advance.
> >
> > Paolo Bonzini
>
> There can always be a plan B---Dan Berrange found a parser with a 
> similar interface and if the weather doesn't improve I may even give a 
> shot at writing one over the weekend.

I already am :-)  Stay tuned, I should have a patch later this afternoon.

I'd like to move all of the QObject/json code to a shared library too so 
that other tools like libvirt can just use that code.  Ideally, we would 
also provide a higher level monitor API too.

> Paolo

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Paolo Bonzini]

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

On 10/16/2009 11:37 PM, Anthony Liguori wrote:
>
> I already am :-)  Stay tuned, I should have a patch later this afternoon.

Was it a race?  (Seriously, sorry I didn't notice a couple of hours ago).

This one is ~5% slower than the "Evil" one, but half the size.  Tested 
against the comments.json file from the "Evil" parser and with valgrind 
too.  Does all the funky Unicode stuff too.

Paolo

[-- Attachment #2: json.c --]
[-- Type: text/plain, Size: 15699 bytes --]

/*
 * An event-based, asynchronous JSON parser.
 *
 * Copyright (C) 2009 Red Hat Inc.
 *
 * Authors:
 *  Paolo Bonzini <pbonzini@redhat.com>
 *
 * 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 "json.h"
#include <string.h>
#include <stdlib.h>

/* Common character classes.  */

#define CASE_XDIGIT \
        case 'a': case 'b': case 'c': case 'd': case 'e': case 'f': \
        case 'A': case 'B': case 'C': case 'D': case 'E': case 'F'

#define CASE_DIGIT \
        case '0': case '1': case '2': case '3': case '4': \
        case '5': case '6': case '7': case '8': case '9'

/* Helper function to go from \uXXXX-encoded UTF-16 to UTF-8.  */

static bool hex_to_utf8 (char *buf, char **dest, char *src)
{
    int i, n;
    uint8_t *p;

    for (i = n = 0; i < 4; i++) {
        n <<= 4;
        switch (src[i])
        {
        CASE_DIGIT: n |= src[i] - '0'; break;
        CASE_XDIGIT: n |= (src[i] & ~32) - 'A' + 10; break;
        default: return false;
        }
    }

    p = (uint8_t *)*dest;
    if (n < 128) {
        *p++ = n;
    } else if (n < 2048) {
        *p++ = 0xC0 | (n >> 6);
        *p++ = 0x80 | (n & 63);
    } else if (n < 0xDC00 || n > 0xDFFF) {
        *p++ = 0xE0 | (n >> 12);
        *p++ = 0x80 | ((n >> 6) & 63);
        *p++ = 0x80 | (n & 63);
    } else {
        /* Merge with preceding high surrogate.  */
        if (p - (uint8_t *)buf < 3
            || p[-3] != 0xED
            || p[-2] < 0xA0 || p[-2] > 0xAF) /* 0xD800..0xDBFF */
            return false;

        n += 0x10000 - 0xDC00;
        n |= ((p[-2] & 15) << 16) | ((p[-1] & 63) << 10);

        /* Overwrite high surrogate.  */
        p[-3] = 0xF0 | (n >> 18);
        p[-2] = 0x80 | ((n >> 12) & 63);
        p[-1] = 0x80 | ((n >> 6) & 63);
        *p++ = 0x80 | (n & 63);
    }
    *dest = (char *)p;
    return true;
}

struct json_parser {
    struct    json_parser_config c;
    size_t    n, alloc;
    char      *buf;
    size_t    sp;
    uint32_t  state, stack[128];
    char      start_buffer[4];
};

/* Managing the state stack.  */

static inline uint32_t *push_state (struct json_parser *p)
{
    p->stack[p->sp++] = p->state;
    return &p->state;
}

static inline void pop_state (struct json_parser *p)
{
    p->state = p->stack[--p->sp];
}


/* Managing the string/number buffer.  */

static inline void clear_buffer (struct json_parser *p)
{
    p->n = 0;
}

static inline void push_buffer (struct json_parser *p, char c)
{
    if (p->n == p->alloc) {
        size_t new_alloc = p->alloc * 2;
        if (p->buf == p->start_buffer) {
            p->buf = malloc (new_alloc);
            memcpy (p->buf, p->start_buffer, p->alloc);
        } else {
            p->buf = realloc (p->buf, new_alloc);
        }
        p->alloc = new_alloc;
    }
    p->buf[p->n++] = c;
}


/*
 * Parser states are organized like this:
 *   bit 0-7:   enum parser_state
 *   bit 8-15:  for IN_KEYWORD, index in keyword table
 *   bit 16-31: additional substate (enum parser_cookies)
 */

enum parser_state {
    START_PARSE,                /* at start of parsing */
    IN_KEYWORD,                 /* parsing keyword (match exactly) */
    START_KEY,                  /* expecting key */
    END_KEY,                    /* expecting colon */
    START_VALUE,                /* expecting value */
    END_VALUE,                  /* expecting comma or closing parenthesis */
    IN_NUMBER,                  /* parsing number (up to whitespace) */
    IN_STRING,                  /* parsing string */
    IN_STRING_BACKSLASH,        /* parsing string, copy one char verbatim */
    IN_COMMENT,                 /* comment mini-scanner */
};

enum parser_cookies {
    IN_UNUSED,

    IN_TRUE,                    /* for IN_KEYWORD */
    IN_FALSE,
    IN_NULL,

    IN_ARRAY,                   /* for {START,END}_{KEY,VALUE} */
    IN_DICT,

    IN_KEY,                     /* for IN_STRING */
    IN_VALUE,
};

#define STATE(state, cookie) \
    (((cookie) << 16) | (state))

#define STATE_KEYWORD(n, cookie) \
    (((cookie) << 16) | ((n) << 8) | IN_KEYWORD)

static const char keyword_table[] = "rue\0alse\0ull";
enum keyword_indices {
    KW_TRUE = 0,
    KW_FALSE = 4,
    KW_NULL = 9,
};



/* Parser actions.  These transfer to the appropriate state,
 * and invoke the callbacks.
 *
 * If there is a begin/end pair, begin pushes a state
 * and end pops it.
 */

static inline bool array_begin (struct json_parser *p)
{
    *push_state (p) = STATE (START_VALUE, IN_ARRAY); 
    return !p->c.array_begin || p->c.array_begin (p->c.data);
}

static inline bool array_end (struct json_parser *p)
{
    int state_cookie = (p->state >> 16);
    if (state_cookie != IN_ARRAY) return false;
    pop_state (p);
    return !p->c.array_end || p->c.array_end (p->c.data);
}


static inline bool object_begin (struct json_parser *p)
{
    *push_state (p) = STATE (START_KEY, IN_DICT);
    return !p->c.object_begin || p->c.object_begin (p->c.data);
}

static inline bool object_end (struct json_parser *p)
{
    int state_cookie = (p->state >> 16);
    if (state_cookie != IN_DICT) return false;
    pop_state (p);
    return !p->c.object_end || p->c.object_end (p->c.data);
}


static inline bool key_user (struct json_parser *p)
{
    return p->c.value_user && p->c.key (p->c.data, NULL, 0);
}


static inline bool number_begin (struct json_parser *p, char ch)
{
    *push_state (p) = IN_NUMBER;
    push_buffer (p, ch);
    return true;
}

static inline bool number_end (struct json_parser *p)
{
    char *end;
    bool result;
    long long ll;
    double d;

    pop_state (p);
    push_buffer (p, 0);
    ll = strtoll (p->buf, &end, 0);
    if (!*end)
        result = (!p->c.value_integer || p->c.value_integer (p->c.data, ll));
    else {
        d = strtod (p->buf, &end);
        result = (!*end &&
                  (!p->c.value_float || p->c.value_float (p->c.data, d)));
    }

    clear_buffer(p);
    return result;
}


static inline bool value_null (struct json_parser *p)
{
    return !p->c.value_null || p->c.value_null (p->c.data);
}


static inline bool value_boolean (struct json_parser *p, int n)
{
    return !p->c.value_boolean || p->c.value_boolean (p->c.data, n);
}


static inline bool string_begin (struct json_parser *p, int cookie)
{
    *push_state (p) = STATE (IN_STRING, cookie);
    return true;
}

static inline bool string_end (struct json_parser *p, int cookie)
{
    bool result;
    char *buf, *src, *dest;
    size_t n;

    pop_state (p); 
    push_buffer (p, 0);

    /* Unescape in place.  */
    for (n = p->n, buf = src = dest = p->buf; n > 0; n--) {
        if (*src != '\\') {
            *dest++ = *src++;
            continue;
        }
        if (n < 2)
            return false;

        src++;
        n--;
        switch (*src++) {
        case 'b': *dest++ = '\b'; continue;
        case 'f': *dest++ = '\f'; continue;
        case 'n': *dest++ = '\n'; continue;
        case 'r': *dest++ = '\r'; continue;
        case 't': *dest++ = '\t'; continue;

        case 'U': case 'u': 
            /* The [uU] has not been removed from n yet, hence subtract 5.  */
            if (n < 5 || !hex_to_utf8 (buf, &dest, src))
                return false;
            src += 4;
            n -= 4;
            continue;

        default: *dest++ = src[-1]; continue;
        }
    }

    buf = p->buf;
    n = dest - buf;
    if (cookie == IN_KEY)
        result = !p->c.key || p->c.key (p->c.data, buf, n);
    else
        result = !p->c.value_string || p->c.value_string (p->c.data, buf, n);
    clear_buffer(p);
    return result;
}


static inline bool value_user (struct json_parser *p)
{
    return p->c.value_user && p->c.value_user (p->c.data);
}


static inline bool comment (struct json_parser *p)
{
    return !p->c.comment || p->c.comment (p->c.data, p->buf, p->n);
}


bool json_parser_char(struct json_parser *p, char ch)
{
    for (;;) {
        int state = p->state & 255;
        int state_data = (p->state >> 8) & 255;
        int state_cookie = (p->state >> 16);
        // printf ("%d %d | %d %d\n", state, ch, state_cookie, p->sp);

        /* The big ugly parser.  Each case will always return or
         * continue, and we want to check this at link time if
         * possible.  */
#ifndef __OPTIMIZE__
#define link_error abort
#endif
        extern void link_error (void);

        switch (state)
        {
        /* First, however, a helpful definition...  */
#define SKIP_WHITE \
            switch (ch) { \
            case '/': goto do_start_comment; \
            case ' ': case '\t': case '\n': case '\r': case '\f': \
                return true; \
            default: \
                break; \
            }

        /* Unlike START_VALUE, this only accepts compound values.  */
        case START_PARSE:
            SKIP_WHITE;
            p->state = STATE (END_VALUE, state_cookie); 
            switch (ch)
            {
            case '[': return array_begin (p);
            case '{': return object_begin (p);
            default: return false;
            }
            link_error ();

        /* Only strings and user values are accepted here.  */
        case START_KEY:
            SKIP_WHITE;
            p->state = STATE (END_KEY, IN_DICT);
            switch (ch)
            {
            case '"': return string_begin (p, IN_KEY);
            case '%': return key_user (p);
            case '}': return object_end (p);
            default: return false;
            }
            link_error ();

        /* Accept any Javascript literal.  Checking p->sp ensures that
         * something like "[] []" is rejected (the first array is parsed
         * from START_PARSE.  */
        case START_VALUE:
            SKIP_WHITE;
            if (p->sp == 0)
                return false;
            p->state = STATE (END_VALUE, state_cookie); 
            switch (ch)
            {
            case 't': *push_state (p) = STATE_KEYWORD(KW_TRUE, IN_TRUE); return true;
            case 'f': *push_state (p) = STATE_KEYWORD(KW_FALSE, IN_FALSE); return true;
            case 'n': *push_state (p) = STATE_KEYWORD(KW_NULL, IN_NULL); return true;
            case '"': return string_begin (p, IN_VALUE);
            case '-':
            CASE_DIGIT: return number_begin (p, ch);
            case '[': return array_begin (p);
            case '{': return object_begin (p);
            case '%': return value_user (p);
            case ']': return array_end (p);
            default: return false;
            }
            link_error ();

        /* End of a key, look for a colon.  */
        case END_KEY:
            SKIP_WHITE;
            p->state = STATE (START_VALUE, IN_DICT);
            return (ch == ':');

        /* End of a value, look for a comma or closing parenthesis.  */
        case END_VALUE:
            SKIP_WHITE;
            p->state = STATE (state_cookie == IN_DICT ? START_KEY : START_VALUE,
                              state_cookie);
            switch (ch)
            {
            case ',': return true;
            case '}': return object_end (p);
            case ']': return array_end (p);
            default: return false;
            }
            link_error ();

        /* Table-driven keyword scanner.  Advance until mismatch or end
         * of keyword.  */
        case IN_KEYWORD:
            if (ch != keyword_table[state_data])
                return false;
            if (keyword_table[state_data + 1] != 0) {
                p->state = STATE_KEYWORD(state_data + 1, state_cookie);
                return true;
            }

            pop_state (p);
            switch (state_cookie) {
            case IN_TRUE: return value_boolean (p, 1);
            case IN_FALSE: return value_boolean (p, 0);
            case IN_NULL: return value_null (p);
            default: abort ();
            }
            link_error ();

        /* Eat until closing quote (special-casing \"). */
        case IN_STRING:
            switch (ch) {
            case '"': return string_end (p, state_cookie);
            case '\\': p->state = STATE (IN_STRING_BACKSLASH, state_cookie);
            default: push_buffer (p, ch); return true;
            }
            link_error ();

        /* Eat any character */
        case IN_STRING_BACKSLASH:
            push_buffer (p, ch); 
            p->state = STATE (IN_STRING, state_cookie);
            return true;

        /* Eat until a "bad" character is found, then we refine with
         * strtod/strtoll.  The character we end on is reprocessed in
         * the new state!  */
        case IN_NUMBER:
            switch (ch) {
            case '+':
            case '-':
            case '.':
            CASE_DIGIT:
            CASE_XDIGIT: push_buffer (p, ch); return true;
            default: if (!number_end (p)) return false; continue;
            }
            link_error ();

        /* Parse until '*' '/', then convert the whole comment to a
         * single blank and rescan. */
        do_start_comment:
            *push_state(p) = IN_COMMENT;
            if (p->c.comment) push_buffer(p, ch);
            return true;

        case IN_COMMENT:
            if (p->c.comment) push_buffer(p, ch);

            if      (state_cookie == 0 && ch != '*') return false;
            else if (state_cookie == 0             ) state_cookie = 1;
            else if (state_cookie == 1 && ch == '*') state_cookie = 2;
            else if (state_cookie == 2 && ch == '*') state_cookie = 2;
            else if (state_cookie == 2 && ch == '/') state_cookie = 3;
            else                                     state_cookie = 1;

            if (state_cookie < 3) {
                p->state = STATE(state, state_cookie);
                return true;
            } else {
                comment (p);
                pop_state (p);
                ch = ' ';
                continue;
            }
            link_error ();

        default:
            abort ();
        }

        link_error ();
    }
}

bool json_parser_string(struct json_parser *p, char *s, size_t n)
{
    while (n--)
        if (!json_parser_char(p, *s++))
            return false;
    return true;
}

struct json_parser *json_parser_new(struct json_parser_config *config)
{
    struct json_parser *p;
    p = malloc (sizeof *p);
    memcpy (&p->c, config, sizeof *config);
    p->n = 0;
    p->alloc = sizeof p->start_buffer;
    p->state = START_PARSE;
    p->buf = p->start_buffer;
    p->sp = 0;
    return p;
}

bool json_parser_destroy(struct json_parser *p)
{
    bool result = (p->state == END_VALUE) && (p->sp == 0);
    if (p->buf != p->start_buffer)
        free (p->buf);
    free (p);
    return result;
}

[-- Attachment #3: json.h --]
[-- Type: text/plain, Size: 2139 bytes --]

/*
 * An event-based, asynchronous JSON parser.
 *
 * Copyright (C) 2009 Red Hat Inc.
 *
 * Authors:
 *  Paolo Bonzini <pbonzini@redhat.com>
 *
 * 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.
 */


#ifndef JSON_H
#define JSON_H

#include <stddef.h>
#include <stdint.h>
#include <stdbool.h>

struct json_parser_config {
    bool (*array_begin) (void *);
    bool (*array_end) (void *);
    bool (*object_begin) (void *);
    bool (*object_end) (void *);
    bool (*key) (void *, const char *, size_t);
    bool (*value_integer) (void *, long long);
    bool (*value_float) (void *, double);
    bool (*value_null) (void *);
    bool (*value_boolean) (void *, int);
    bool (*value_string) (void *, const char *, size_t);
    bool (*value_user) (void *);
    bool (*comment) (void *, const char *, size_t);
    void *data;
};

struct json_parser;

struct json_parser *json_parser_new(struct json_parser_config *config);
bool json_parser_destroy(struct json_parser *p);
bool json_parser_char(struct json_parser *p, char ch);
bool json_parser_string(struct json_parser *p, char *buf, size_t n);

#endif /* JSON_H */


[-- Attachment #4: main.c --]
[-- Type: text/plain, Size: 3048 bytes --]

/* main.c */

/*
    This program demonstrates a simple application of JSON_parser. It reads
    a JSON text from STDIN, producing an error message if the text is rejected.

        % JSON_parser <test/pass1.json
*/

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <assert.h>
#include <locale.h>

#include "json.h"

#include <stddef.h>
#include <stdint.h>
#include <stdbool.h>

static int level = 0;
static int got_key = 0;

static void print_indent()
{
    printf ("%*s", 2 * level, "");
}
 
static bool array_begin (void *data)
{
    if (!got_key) print_indent(); else got_key = 0;
    printf ("[\n");
    ++level;
    return true;
}

static bool array_end (void *data)
{
    --level;
    print_indent ();
    printf ("]\n");
    return true;
}

static bool object_begin (void *data)
{
    if (!got_key) print_indent(); else got_key = 0;
    printf ("{\n");
    ++level;
    return true;
}

static bool object_end (void *data)
{
    --level;
    print_indent ();
    printf ("}\n");
    return true;
}

static bool key (void *data, const char *buf, size_t n)
{
    got_key = 1;
    print_indent ();
    if (buf)
	printf ("key = '%s', value = ", buf);
    else
	printf ("user key = %%%c, value = ", getchar());
    return true;
}

static bool value_integer (void *data, long long ll)
{
    if (!got_key) print_indent(); else got_key = 0;
    printf ("integer: %lld\n", ll);
    return true;
}

static bool value_float (void *data, double d)
{
    if (!got_key) print_indent(); else got_key = 0;
    printf ("float: %f\n", d);
    return true;
}

static bool value_null (void *data)
{
    if (!got_key) print_indent(); else got_key = 0;
    printf ("null\n");
    return true;
}

static bool value_boolean (void *data, int val)
{
    if (!got_key) print_indent(); else got_key = 0;
    printf ("%s\n", val ? "true" : "false");
    return true;
}

static bool value_string (void *data, const char *buf, size_t n)
{
    if (!got_key) print_indent(); else got_key = 0;
    printf ("string: '%s'\n", buf);
    return true;
}

static bool value_user (void *data)
{
    if (!got_key) print_indent(); else got_key = 0;
    printf ("user: %%%c\n", getchar());
    return true;
}



int main(int argc, char* argv[]) {
    static struct json_parser_config parser_config = {
        .array_begin = array_begin,
        .array_end = array_end,
        .object_begin = object_begin,
        .object_end = object_end,
        .key = key,
        .value_integer = value_integer,
        .value_float = value_float,
        .value_null = value_null,
        .value_boolean = value_boolean,
        .value_string = value_string,
        .value_user = value_user,
    };

    struct json_parser *p = json_parser_new(&parser_config);
    int count = 0;
    int ch;
    while ((ch = getchar ()) != EOF && json_parser_char (p, ch))
	count++;

    if (ch != EOF) {
	fprintf (stderr, "error at character %d\n", count);
	exit (1);
    }
    if (!json_parser_destroy (p)) {
	fprintf (stderr, "error at end of file\n");
	exit (1);
    }

    exit (0);
}

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[malc]

On Sat, 17 Oct 2009, Paolo Bonzini wrote:

> On 10/16/2009 11:37 PM, Anthony Liguori wrote:
> > 
> > I already am :-)  Stay tuned, I should have a patch later this afternoon.
> 
> Was it a race?  (Seriously, sorry I didn't notice a couple of hours ago).
> 
> This one is ~5% slower than the "Evil" one, but half the size.  Tested against
> the comments.json file from the "Evil" parser and with valgrind too.  Does all
> the funky Unicode stuff too.
> 

Just from cursory glance:

a. allocation can fail
b. strtod is locale dependent

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Paolo Bonzini]

On 10/17/2009 02:38 AM, malc wrote:
> a. allocation can fail

s/malloc/qemu_malloc/ etc. when it is time to merge.

> b. strtod is locale dependent

Right, but qemu probably would prefer to always do setlocale 
(LC_NUMERIC, "C"), or add a c_strtod function like

http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=blob_plain;f=lib/c-strtod.c

Thanks for the review, any additional pair of eyes can only help.

Paolo

[Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Paolo Bonzini wrote:
> On 10/16/2009 11:37 PM, Anthony Liguori wrote:
>>
>> I already am :-)  Stay tuned, I should have a patch later this 
>> afternoon.
>
> Was it a race?  (Seriously, sorry I didn't notice a couple of hours ago).
>
> This one is ~5% slower than the "Evil" one, but half the size.  Tested 
> against the comments.json file from the "Evil" parser and with 
> valgrind too.  Does all the funky Unicode stuff too.
>
> Paolo
> /*
>  * An event-based, asynchronous JSON parser.
>  *
>  * Copyright (C) 2009 Red Hat Inc.
>  *
>  * Authors:
>  *  Paolo Bonzini <pbonzini@redhat.com>
>  *
>  * 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 "json.h"
> #include <string.h>
> #include <stdlib.h>
>
> /* Common character classes.  */
>
> #define CASE_XDIGIT \
>         case 'a': case 'b': case 'c': case 'd': case 'e': case 'f': \
>         case 'A': case 'B': case 'C': case 'D': case 'E': case 'F'
>
> #define CASE_DIGIT \
>         case '0': case '1': case '2': case '3': case '4': \
>         case '5': case '6': case '7': case '8': case '9'
>
> /* Helper function to go from \uXXXX-encoded UTF-16 to UTF-8.  */
>
> static bool hex_to_utf8 (char *buf, char **dest, char *src)
> {
>     int i, n;
>     uint8_t *p;
>
>     for (i = n = 0; i < 4; i++) {
>         n <<= 4;
>         switch (src[i])
>         {
>         CASE_DIGIT: n |= src[i] - '0'; break;
>         CASE_XDIGIT: n |= (src[i] & ~32) - 'A' + 10; break;
>         default: return false;
>         }
>     }
>
>     p = (uint8_t *)*dest;
>     if (n < 128) {
>         *p++ = n;
>     } else if (n < 2048) {
>         *p++ = 0xC0 | (n >> 6);
>         *p++ = 0x80 | (n & 63);
>     } else if (n < 0xDC00 || n > 0xDFFF) {
>         *p++ = 0xE0 | (n >> 12);
>         *p++ = 0x80 | ((n >> 6) & 63);
>         *p++ = 0x80 | (n & 63);
>     } else {
>         /* Merge with preceding high surrogate.  */
>         if (p - (uint8_t *)buf < 3
>             || p[-3] != 0xED
>             || p[-2] < 0xA0 || p[-2] > 0xAF) /* 0xD800..0xDBFF */
>             return false;
>
>         n += 0x10000 - 0xDC00;
>         n |= ((p[-2] & 15) << 16) | ((p[-1] & 63) << 10);
>
>         /* Overwrite high surrogate.  */
>         p[-3] = 0xF0 | (n >> 18);
>         p[-2] = 0x80 | ((n >> 12) & 63);
>         p[-1] = 0x80 | ((n >> 6) & 63);
>         *p++ = 0x80 | (n & 63);
>     }
>     *dest = (char *)p;
>     return true;
> }
>
> struct json_parser {
>     struct    json_parser_config c;
>     size_t    n, alloc;
>     char      *buf;
>     size_t    sp;
>     uint32_t  state, stack[128];
>     char      start_buffer[4];
> };
>   

Having an explicit stack is unnecessary I think.  You can use a very 
simple scheme to detect the end of messages by simply counting {}, [], 
and being aware of the lexical rules.

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Paolo Bonzini wrote:
> On 10/16/2009 11:37 PM, Anthony Liguori wrote:
>>
>> I already am :-)  Stay tuned, I should have a patch later this 
>> afternoon.
>
> Was it a race?  (Seriously, sorry I didn't notice a couple of hours ago).
>
> This one is ~5% slower than the "Evil" one, but half the size.  Tested 
> against the comments.json file from the "Evil" parser and with 
> valgrind too.  Does all the funky Unicode stuff too.

I haven't benchmarked mine.  While yours came out an hour earlier, I 
included a full test suite, output QObjects, and support vararg parsing 
so I think I win :-)

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Paolo Bonzini]

On 10/17/2009 03:50 AM, Anthony Liguori wrote:
> Paolo Bonzini wrote:
>> On 10/16/2009 11:37 PM, Anthony Liguori wrote:
>>>
>>> I already am :-) Stay tuned, I should have a patch later this afternoon.
>>
>> Was it a race? (Seriously, sorry I didn't notice a couple of hours ago).
>>
>> This one is ~5% slower than the "Evil" one, but half the size. Tested
>> against the comments.json file from the "Evil" parser and with
>> valgrind too. Does all the funky Unicode stuff too.
>
> I haven't benchmarked mine. While yours came out an hour earlier, I
> included a full test suite, output QObjects, and support vararg parsing
> so I think I win :-)

Heh, Luiz and I had talked offlist and he'd take care of the rest 
(except the test suite) :-).

 > Having an explicit stack is unnecessary I think.

I'm curious to see yours now---the stack is used to detect things like 
[{"a":"b"},"c":"d"].  You could do that in the event handlers of course, 
but that kind of breaks the interface between the parser and event handlers.

Paolo

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Vincent Hanquez]

Anthony Liguori wrote:
> Paolo Bonzini wrote:
>> On 10/16/2009 11:37 PM, Anthony Liguori wrote:
>>>
>>> I already am :-)  Stay tuned, I should have a patch later this 
>>> afternoon.
>>
>> Was it a race?  (Seriously, sorry I didn't notice a couple of hours 
>> ago).
>>
>> This one is ~5% slower than the "Evil" one, but half the size.  
>> Tested against the comments.json file from the "Evil" parser and with 
>> valgrind too.  Does all the funky Unicode stuff too.
>
> I haven't benchmarked mine.  While yours came out an hour earlier, I 
> included a full test suite, output QObjects, and support vararg 
> parsing so I think I win :-)
ar.. got mine too, i've been doing for the last 3 weeks slowly;

it got a raw/pretty printer, an interruptible parser (on the same idea 
as JSON_parser.c), it's faster than JSON_parser.c [1],
it's completely generic (more like a library than an embedded thing), 
fully JSON compliant (got a test suite too), support
user supplied alloc functions, and callback for integer/float doesn't 
have their data converted automatically which means
that the user of the library can use whatever it want to support the 
non-limited size JSON number (or just return errors for user that want 
the limit).

the library by itself is 39K with -g last time i've looked.

also the library comes with a jsonlint binary that's equivalent to 
xmllint (well formatting and verification).

I'll package thing up and post a link to it on monday.

-- 
Vincent

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Luiz Capitulino]

On Sat, 17 Oct 2009 11:01:33 +0100
Vincent Hanquez <vincent@snarc.org> wrote:

> Anthony Liguori wrote:
> > Paolo Bonzini wrote:
> >> On 10/16/2009 11:37 PM, Anthony Liguori wrote:
> >>>
> >>> I already am :-)  Stay tuned, I should have a patch later this 
> >>> afternoon.
> >>
> >> Was it a race?  (Seriously, sorry I didn't notice a couple of hours 
> >> ago).
> >>
> >> This one is ~5% slower than the "Evil" one, but half the size.  
> >> Tested against the comments.json file from the "Evil" parser and with 
> >> valgrind too.  Does all the funky Unicode stuff too.
> >
> > I haven't benchmarked mine.  While yours came out an hour earlier, I 
> > included a full test suite, output QObjects, and support vararg 
> > parsing so I think I win :-)
> ar.. got mine too, i've been doing for the last 3 weeks slowly;

 Very nice to see all these contributions.

> it got a raw/pretty printer, an interruptible parser (on the same idea 
> as JSON_parser.c), it's faster than JSON_parser.c [1],
> it's completely generic (more like a library than an embedded thing), 
> fully JSON compliant (got a test suite too), support
> user supplied alloc functions, and callback for integer/float doesn't 
> have their data converted automatically which means
> that the user of the library can use whatever it want to support the 
> non-limited size JSON number (or just return errors for user that want 
> the limit).
> 
> the library by itself is 39K with -g last time i've looked.

 Integration with QObjects is a killer feature, I think it's the
stronger argument against grabbing one from the internet.

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Paolo Bonzini]

On 10/18/2009 04:06 PM, Luiz Capitulino wrote:
>   Integration with QObjects is a killer feature, I think it's the
> stronger argument against grabbing one from the internet.

Yeah, I'd say let's go with Anthony's stuff.  I'll rebase the encoder on 
top of it soonish (I still think it's best if JSON encoding lies in 
QObject like a kind of toString).  If we'll need the asynchronous 
parsing later, we can easily replace it with mine or Vincent's.

Paolo

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Paolo Bonzini wrote:
> On 10/18/2009 04:06 PM, Luiz Capitulino wrote:
>>   Integration with QObjects is a killer feature, I think it's the
>> stronger argument against grabbing one from the internet.
>
> Yeah, I'd say let's go with Anthony's stuff.  I'll rebase the encoder 
> on top of it soonish (I still think it's best if JSON encoding lies in 
> QObject like a kind of toString).  If we'll need the asynchronous 
> parsing later, we can easily replace it with mine or Vincent's.

One thing I want to add as a feature to the 0.12 release is a nice 
client API.  To have this, we'll need message boundary identification 
and a JSON encoder.  I'll focus on the message boundary identification 
today.

I'd strongly suggest making the JSON encoder live outside of QObject.  
There are many possible ways to represent a QObject.  Think of JSON as a 
view of the QObject model.  The human monitor mode representation is a 
different view.

Regards,

Anthony Liguori

> Paolo

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Luiz Capitulino]

On Sun, 18 Oct 2009 09:49:55 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> Paolo Bonzini wrote:
> > On 10/18/2009 04:06 PM, Luiz Capitulino wrote:
> >>   Integration with QObjects is a killer feature, I think it's the
> >> stronger argument against grabbing one from the internet.
> >
> > Yeah, I'd say let's go with Anthony's stuff.  I'll rebase the encoder 
> > on top of it soonish (I still think it's best if JSON encoding lies in 
> > QObject like a kind of toString).  If we'll need the asynchronous 
> > parsing later, we can easily replace it with mine or Vincent's.
> 
> One thing I want to add as a feature to the 0.12 release is a nice 
> client API.  To have this, we'll need message boundary identification 
> and a JSON encoder.  I'll focus on the message boundary identification 
> today.
> 
> I'd strongly suggest making the JSON encoder live outside of QObject.  
> There are many possible ways to represent a QObject.  Think of JSON as a 
> view of the QObject model.  The human monitor mode representation is a 
> different view.

 I agree.

 QObject's methods should only be used/needed by the object layer itself,
if the problem at hand handles high level data types (QInt, QDict, etc)
then we need a new type.

 The right way to have what Paolo is suggesting, would be to have a
toString() method in the object layer and allow it to be overridden.

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Paolo Bonzini]

>> I'd strongly suggest making the JSON encoder live outside of QObject.
>> There are many possible ways to represent a QObject.  Think of JSON as a
>> view of the QObject model.  The human monitor mode representation is a
>> different view.

My rationale was that since QObject is tailored over JSON, we might as 
well declare JSON to be "the" preferred view of the QObject model.

The human monitor representation would be provided by qstring_format in 
my patches (and a QError method would call qstring_format in the 
appropriate way, returning a C string with the result).

I think the different opinions is also due to different background; mine 
is in Smalltalk where class extensions---aka monkeypatching---are done 
in a different style than for example in Python.  Adding a "write as 
escaped JSON" method to QString would be akin to monkeypatching.

>   I agree.
>
>   QObject's methods should only be used/needed by the object layer itself,
> if the problem at hand handles high level data types (QInt, QDict, etc)
> then we need a new type.
>
>   The right way to have what Paolo is suggesting, would be to have a
> toString() method in the object layer and allow it to be overridden.

That's exactly what I did in my patches, except I called it encode_json 
rather than toString.

Paolo

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Luiz Capitulino]

On Sun, 18 Oct 2009 17:25:47 +0200
Paolo Bonzini <bonzini@gnu.org> wrote:

> 
> >> I'd strongly suggest making the JSON encoder live outside of QObject.
> >> There are many possible ways to represent a QObject.  Think of JSON as a
> >> view of the QObject model.  The human monitor mode representation is a
> >> different view.
> 
> My rationale was that since QObject is tailored over JSON, we might as 
> well declare JSON to be "the" preferred view of the QObject model.

 Maybe this makes sense today as the Monitor is the only heavy user
of QObjects, but I don't think we should count on that.

 As things evolve, I believe more subsystems will start using QObjects
and any "particular" view of it will make little sense.

 To be honest I don't know if this is good, I fear we will end up
enhancing QObjects to the extreme to do OOP in QEMU...

> The human monitor representation would be provided by qstring_format in 
> my patches (and a QError method would call qstring_format in the 
> appropriate way, returning a C string with the result).
> 
> I think the different opinions is also due to different background; mine 
> is in Smalltalk where class extensions---aka monkeypatching---are done 
> in a different style than for example in Python.  Adding a "write as 
> escaped JSON" method to QString would be akin to monkeypatching.

 True.

> >   I agree.
> >
> >   QObject's methods should only be used/needed by the object layer itself,
> > if the problem at hand handles high level data types (QInt, QDict, etc)
> > then we need a new type.
> >
> >   The right way to have what Paolo is suggesting, would be to have a
> > toString() method in the object layer and allow it to be overridden.
> 
> That's exactly what I did in my patches, except I called it encode_json 
> rather than toString.

 Okay, I just took a quick look at them and am looking at Anthony's
right now.

 Anyway, my brainstorm on this would be to have to_string() and have
default methods on all types to return a simple string representation.
The QJson type could override to_string() if needed, this way specific
json bits stays inside the json module.

 But I see that Anthony has added a qjson type already..

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Luiz Capitulino wrote:
>  Okay, I just took a quick look at them and am looking at Anthony's
> right now.
>
>  Anyway, my brainstorm on this would be to have to_string() and have
> default methods on all types to return a simple string representation.
>   

What's the value of integrating into the objects verses having a 
separate function that can apply it to the objects?

Prototype languages are very different and it's not typically a good 
idea to mix styles like this.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Paolo Bonzini]

On 10/18/2009 06:32 PM, Anthony Liguori wrote:
>
> What's the value of integrating into the objects verses having a
> separate function that can apply it to the objects?

That's just different style.  Of course you could do a 
switch(qobject_type(qobject)) instead of using polymorphism.  It would 
be nicer in some ways, and uglier in other ways.  toString however seems 
pervasive enough that it could deserve a place as a QObject virtual method.

Anyway, I probably won't have much code in QEMU in the end, so there's 
no value in arguing when anyway a very nice design is emerging.  It 
looks like Anthony has most of the JSON plumbing in his brain, so it's 
better if he keeps the flow going.  Feel free to steal my code.

Once your stuff is settled I'll see what's missing and rebase/resend.

Paolo, at one point tempted to s/encode_json/to_string/ and resubmit :-)

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Luiz Capitulino]

On Sun, 18 Oct 2009 11:32:11 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> Luiz Capitulino wrote:
> >  Okay, I just took a quick look at them and am looking at Anthony's
> > right now.
> >
> >  Anyway, my brainstorm on this would be to have to_string() and have
> > default methods on all types to return a simple string representation.
> >   
> 
> What's the value of integrating into the objects verses having a 
> separate function that can apply it to the objects?

 Right now it doesn't have any real value, besides being a different
style which seems to fit well with the QObject design.

 In the future it might be needed though, common code might want to
change certain methods' behavior before passing QObjects down a call
stack.

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

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

Anthony Liguori wrote:
> Paolo Bonzini wrote:
>> On 10/18/2009 04:06 PM, Luiz Capitulino wrote:
>>>   Integration with QObjects is a killer feature, I think it's the
>>> stronger argument against grabbing one from the internet.
>>
>> Yeah, I'd say let's go with Anthony's stuff.  I'll rebase the encoder 
>> on top of it soonish (I still think it's best if JSON encoding lies 
>> in QObject like a kind of toString).  If we'll need the asynchronous 
>> parsing later, we can easily replace it with mine or Vincent's.
>
> One thing I want to add as a feature to the 0.12 release is a nice 
> client API.  To have this, we'll need message boundary identification 
> and a JSON encoder.  I'll focus on the message boundary identification 
> today.

Here's a first pass.  I'll clean up this afternoon and post a proper 
patch.  It turned out to work pretty well.

Regards,

Anthony Liguori

[-- Attachment #2: json-streamer.c --]
[-- Type: text/x-csrc, Size: 8034 bytes --]

#include <stdint.h>
#include <stdio.h>
#include <errno.h>
#include <string.h>

#define offset_of(type, member) ((unsigned long)&(((type *)0)->member))
#define container_of(obj, type, member) (type *)((void *)(obj) - offset_of(type, member))

/*
 * \"([^\\\"]|(\\\"\\'\\\\\\/\\b\\f\\n\\r\\t\\u[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]))*\"
 * '([^\\']|(\\\"\\'\\\\\\/\\b\\f\\n\\r\\t\\u[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]))*'
 * 0|([1-9][0-9]*(.[0-9]+)?([eE]([-+])?[0-9]+))
 * [{}\[\],:]
 * [a-z]+
 *
 */

enum json_lexer_state {
    ERROR = 0,
    IN_DONE,
    IN_DQ_UCODE3,
    IN_DQ_UCODE2,
    IN_DQ_UCODE1,
    IN_DQ_UCODE0,
    IN_DQ_STRING_ESCAPE,
    IN_DQ_STRING,
    IN_SQ_UCODE3,
    IN_SQ_UCODE2,
    IN_SQ_UCODE1,
    IN_SQ_UCODE0,
    IN_SQ_STRING_ESCAPE,
    IN_SQ_STRING,
    IN_ZERO,
    IN_DIGITS,
    IN_DIGIT,
    IN_EXP_E,
    IN_MANTISSA,
    IN_MANTISSA_DIGITS,
    IN_NONZERO_NUMBER,
    IN_NEG_NONZERO_NUMBER,
    IN_KEYWORD,
    IN_WHITESPACE,
    IN_START,
    DONE,
    SKIP,
};

static const uint8_t json_lexer[][256] =  {
    [IN_DONE] = {
        [1 ... 0x7F] = DONE,
    },

    /* double quote string */
    [IN_DQ_UCODE3] = {
        ['0' ... '9'] = IN_DQ_STRING,
        ['a' ... 'f'] = IN_DQ_STRING,
        ['A' ... 'F'] = IN_DQ_STRING,
    },
    [IN_DQ_UCODE2] = {
        ['0' ... '9'] = IN_DQ_UCODE3,
        ['a' ... 'f'] = IN_DQ_UCODE3,
        ['A' ... 'F'] = IN_DQ_UCODE3,
    },
    [IN_DQ_UCODE1] = {
        ['0' ... '9'] = IN_DQ_UCODE2,
        ['a' ... 'f'] = IN_DQ_UCODE2,
        ['A' ... 'F'] = IN_DQ_UCODE2,
    },
    [IN_DQ_UCODE0] = {
        ['0' ... '9'] = IN_DQ_UCODE1,
        ['a' ... 'f'] = IN_DQ_UCODE1,
        ['A' ... 'F'] = IN_DQ_UCODE1,
    },
    [IN_DQ_STRING_ESCAPE] = {
        ['b'] = IN_DQ_STRING,
        ['f'] =  IN_DQ_STRING,
        ['n'] =  IN_DQ_STRING,
        ['r'] =  IN_DQ_STRING,
        ['t'] =  IN_DQ_STRING,
        ['\''] = IN_DQ_STRING,
        ['\"'] = IN_DQ_STRING,
        ['u'] = IN_DQ_UCODE0,
    },
    [IN_DQ_STRING] = {
        [1 ... 0xFF] = IN_DQ_STRING,
        ['\\'] = IN_DQ_STRING_ESCAPE,
        ['"'] = IN_DONE,
    },

    /* single quote string */
    [IN_SQ_UCODE3] = {
        ['0' ... '9'] = IN_SQ_STRING,
        ['a' ... 'f'] = IN_SQ_STRING,
        ['A' ... 'F'] = IN_SQ_STRING,
    },
    [IN_SQ_UCODE2] = {
        ['0' ... '9'] = IN_SQ_UCODE3,
        ['a' ... 'f'] = IN_SQ_UCODE3,
        ['A' ... 'F'] = IN_SQ_UCODE3,
    },
    [IN_SQ_UCODE1] = {
        ['0' ... '9'] = IN_SQ_UCODE2,
        ['a' ... 'f'] = IN_SQ_UCODE2,
        ['A' ... 'F'] = IN_SQ_UCODE2,
    },
    [IN_SQ_UCODE0] = {
        ['0' ... '9'] = IN_SQ_UCODE1,
        ['a' ... 'f'] = IN_SQ_UCODE1,
        ['A' ... 'F'] = IN_SQ_UCODE1,
    },
    [IN_SQ_STRING_ESCAPE] = {
        ['b'] = IN_SQ_STRING,
        ['f'] =  IN_SQ_STRING,
        ['n'] =  IN_SQ_STRING,
        ['r'] =  IN_SQ_STRING,
        ['t'] =  IN_SQ_STRING,
        ['\''] = IN_SQ_STRING,
        ['\"'] = IN_SQ_STRING,
        ['u'] = IN_SQ_UCODE0,
    },
    [IN_SQ_STRING] = {
        [1 ... 0xFF] = IN_SQ_STRING,
        ['\\'] = IN_SQ_STRING_ESCAPE,
        ['\''] = IN_DONE,
    },

    /* Zero */
    [IN_ZERO] = {
        [1 ... 0x7F] = DONE,
        ['0' ... '9'] = ERROR,
    },

    /* Non-zero numbers */
    [IN_DIGITS] = {
        [1 ... 0x7F] = DONE,
        ['0' ... '9'] = IN_DIGITS,
    },

    [IN_DIGIT] = {
        ['0' ... '9'] = IN_DIGITS,
    },

    [IN_EXP_E] = {
        ['-'] = IN_DIGIT,
        ['+'] = IN_DIGIT,
        ['0' ... '9'] = IN_DIGITS,
    },

    [IN_MANTISSA_DIGITS] = {
        [1 ... 0x7F] = DONE,
        ['0' ... '9'] = IN_MANTISSA_DIGITS,
        ['e'] = IN_EXP_E,
        ['E'] = IN_EXP_E,
    },

    [IN_MANTISSA] = {
        ['0' ... '9'] = IN_MANTISSA_DIGITS,
    },

    [IN_NONZERO_NUMBER] = {
        [1 ... 0x7F] = DONE,
        ['0' ... '9'] = IN_NONZERO_NUMBER,
        ['e'] = IN_EXP_E,
        ['E'] = IN_EXP_E,
        ['.'] = IN_MANTISSA,
    },

    [IN_NEG_NONZERO_NUMBER] = {
        ['1' ... '9'] = IN_NONZERO_NUMBER,
    },

    /* keywords */
    [IN_KEYWORD] = {
        [1 ... 0x7F] = DONE,
        ['a' ... 'z'] = IN_KEYWORD,
    },

    /* whitespace */
    [IN_WHITESPACE] = {
        [1 ... 0x7F] = SKIP,
        [' '] = IN_WHITESPACE,
        ['\t'] = IN_WHITESPACE,
        ['\r'] = IN_WHITESPACE,
        ['\n'] = IN_WHITESPACE,
    },        

    /* top level rule */
    [IN_START] = {
        ['"'] = IN_DQ_STRING,
        ['\''] = IN_SQ_STRING,
        ['0'] = IN_ZERO,
        ['1' ... '9'] = IN_NONZERO_NUMBER,
        ['-'] = IN_NEG_NONZERO_NUMBER,
        ['{'] = IN_DONE,
        ['}'] = IN_DONE,
        ['['] = IN_DONE,
        [']'] = IN_DONE,
        [','] = IN_DONE,
        [':'] = IN_DONE,
        ['a' ... 'z'] = IN_KEYWORD,
        [' '] = IN_WHITESPACE,
        ['\t'] = IN_WHITESPACE,
        ['\r'] = IN_WHITESPACE,
        ['\n'] = IN_WHITESPACE,
    },
};

typedef struct JSONLexer
{
    void (*emit)(struct JSONLexer *lexer, const char *token);
    enum json_lexer_state state;
    char token[1024];
    size_t len;
} JSONLexer;

void json_lexer_init(JSONLexer *lexer,
                     void (*func)(JSONLexer *, const char *))
{
    lexer->emit = func;
    lexer->state = IN_START;
    lexer->len = 0;
    lexer->token[lexer->len] = 0;
}

int json_lexer_feed(JSONLexer *lexer, char ch)
{
    lexer->state = json_lexer[lexer->state][(uint8_t)ch];

    if (lexer->state == DONE || lexer->state == SKIP) {
        if (lexer->state == DONE) {
            lexer->emit(lexer, lexer->token);
        }
        lexer->state = json_lexer[IN_START][(uint8_t)ch];
        lexer->len = 0;
    }

    if (lexer->state == ERROR) {
        return -EINVAL;
    }

    if (lexer->len < (sizeof(lexer->token) - 1)) {
        lexer->token[lexer->len++] = ch;
    }
    lexer->token[lexer->len] = 0;

    return 0;
}

typedef struct JSONMessageParser
{
    void (*emit)(struct JSONMessageParser *parser, const char *message);
    JSONLexer lexer;
    int brace_count;
    int bracket_count;
    char buffer[1024];
    size_t len;
} JSONMessageParser;

static void json_message_process_token(JSONLexer *lexer, const char *token)
{
    JSONMessageParser *parser = container_of(lexer, JSONMessageParser, lexer);

    if (strcmp(token, "{") == 0) {
        parser->brace_count++;
    } else if (strcmp(token, "}") == 0) {
        parser->brace_count--;
    } else if (strcmp(token, "[") == 0) {
        parser->bracket_count++;
    } else if (strcmp(token, "]") == 0) {
        parser->bracket_count--;
    }

    if (parser->brace_count == 0 &&
        parser->bracket_count == 0) {
        parser->emit(parser, parser->buffer);
        parser->len = 0;
    }
}

void json_message_parser_init(JSONMessageParser *parser,
                              void (*func)(JSONMessageParser *, const char *))
{
    parser->emit = func;
    parser->brace_count = 0;
    parser->bracket_count = 0;
    parser->len = 0;
    parser->buffer[parser->len] = 0;

    json_lexer_init(&parser->lexer, json_message_process_token);
}

int json_message_parser_feed(JSONMessageParser *parser,
                             const char *buffer, size_t size)
{
    size_t i;

    for (i = 0; i < size; i++) {
        int ret;

        parser->buffer[parser->len++] = buffer[i];
        parser->buffer[parser->len] = 0; /* FIXME overflow */

        ret = json_lexer_feed(&parser->lexer, buffer[i]);
        if (ret < 0) {
            return ret;
        }
    }

    return 0;
}

static void got_message(JSONMessageParser *parser, const char *message)
{
    printf("got message `%s'\n", message);
}

int main(int argc, char **argv)
{
    JSONMessageParser parser = {};
    char buf[2];
    int ch;

    json_message_parser_init(&parser, got_message);

    while ((ch = getchar()) != EOF) {
        buf[0] = ch;
        buf[1] = 0;

        if (json_message_parser_feed(&parser, buf, 1) < 0) {
            fprintf(stderr, "Invalid character `%c'\n", ch);
            return 1;
        }
    }

    return 0;
}

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Vincent Hanquez]

Anthony Liguori wrote:
> Here's a first pass.  I'll clean up this afternoon and post a proper 
> patch.  It turned out to work pretty well.
It doesn't seems to validate anything ?? or is it just a lexer ?
you're also including ' as a string escape value (is that the single 
quote thing you were talking about ?) which strikes me as invalid JSON.

-- 
Vincent

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

On Sun, Oct 18, 2009 at 12:32 PM, Vincent Hanquez <vincent@snarc.org> wrote:
> Anthony Liguori wrote:
>>
>> Here's a first pass.  I'll clean up this afternoon and post a proper
>> patch.  It turned out to work pretty well.
>
> It doesn't seems to validate anything ?? or is it just a lexer ?

That's just a lexer.  I posted a parser earlier.  However, now I'm
thinking I should update the parser to use the lexer.

> you're also including ' as a string escape value (is that the single quote
> thing you were talking about ?) which strikes me as invalid JSON.

It's a compatible extension.  We accept strings with those escapes but
our encoder won't generate them.

> --
> Vincent
>

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Vincent Hanquez]

Luiz Capitulino wrote:
>> it got a raw/pretty printer, an interruptible parser (on the same idea 
>> as JSON_parser.c), it's faster than JSON_parser.c [1],
>> it's completely generic (more like a library than an embedded thing), 
>> fully JSON compliant (got a test suite too), support
>> user supplied alloc functions, and callback for integer/float doesn't 
>> have their data converted automatically which means
>> that the user of the library can use whatever it want to support the 
>> non-limited size JSON number (or just return errors for user that want 
>> the limit).
>>
>> the library by itself is 39K with -g last time i've looked.
>>     
>
>  Integration with QObjects is a killer feature, I think it's the
> stronger argument against grabbing one from the internet.
>   
I can't think of any reason why integration with qobject would take more 
than 50 lines of C on the user side of the library.
since the API is completely SAX like (i call it SAJ for obvious reason), 
you get callback entering/leaving object/array
and callback for every values (string, int, float, null, true, false) as 
a char * + length. for exactly the same reason, integration with glib 
would take the same 50 lines "effort".

note that FTR, obviously i'ld like to have my library used, but i'm 
happy that any library that is *fully* JSON compliant is used (no 
extensions however since you're obviously loosing the benefit of using 
JSON if you create extensions).

-- 
Vincent

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Luiz Capitulino]

On Sun, 18 Oct 2009 16:06:29 +0100
Vincent Hanquez <vincent@snarc.org> wrote:

> Luiz Capitulino wrote:
> >> it got a raw/pretty printer, an interruptible parser (on the same idea 
> >> as JSON_parser.c), it's faster than JSON_parser.c [1],
> >> it's completely generic (more like a library than an embedded thing), 
> >> fully JSON compliant (got a test suite too), support
> >> user supplied alloc functions, and callback for integer/float doesn't 
> >> have their data converted automatically which means
> >> that the user of the library can use whatever it want to support the 
> >> non-limited size JSON number (or just return errors for user that want 
> >> the limit).
> >>
> >> the library by itself is 39K with -g last time i've looked.
> >>     
> >
> >  Integration with QObjects is a killer feature, I think it's the
> > stronger argument against grabbing one from the internet.
> >   
> I can't think of any reason why integration with qobject would take more 
> than 50 lines of C on the user side of the library.
> since the API is completely SAX like (i call it SAJ for obvious reason), 
> you get callback entering/leaving object/array
> and callback for every values (string, int, float, null, true, false) as 
> a char * + length. for exactly the same reason, integration with glib 
> would take the same 50 lines "effort".

 No lines is a lot better than 50. :)

 The real problem though is that the parsers I looked at had their own
"object model", some of them are quite simple others are more sophisticated
than QObject. Making no use of any kind of intermediate representation like
this is a feature, as things get simpler.

 Also, don't get me wrong, but if we would consider your parser we
would have to consider the others two or three that are listed in
json.org and have a compatible license.

> note that FTR, obviously i'ld like to have my library used, but i'm 
> happy that any library that is *fully* JSON compliant is used (no 
> extensions however since you're obviously loosing the benefit of using 
> JSON if you create extensions).

 This is already settled, I hope.

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Paolo Bonzini]

On 10/18/2009 05:35 PM, Luiz Capitulino wrote:
>> (no
>>  extensions however since you're obviously loosing the benefit of using
>>  JSON if you create extensions).
>   This is already settled, I hope.

I think he's referring to things such as putting things such as 
single-quoted strings, or % escapes for formatting.  I have no qualms 
with that as long as what goes on the wire is 100% JSON.

Paolo

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Vincent Hanquez]

Luiz Capitulino wrote:
>> I can't think of any reason why integration with qobject would take more 
>> than 50 lines of C on the user side of the library.
>> since the API is completely SAX like (i call it SAJ for obvious reason), 
>> you get callback entering/leaving object/array
>> and callback for every values (string, int, float, null, true, false) as 
>> a char * + length. for exactly the same reason, integration with glib 
>> would take the same 50 lines "effort".
>>     
>
>  No lines is a lot better than 50. :)
>   
well it all depends on how you see thing; whilst i'm happy to help all 
sort of integration (qemu in this case), my library has been made for 
integrating with absolutely any object model. so 50 lines seems like a 
win to me, because I could do the same thing on a project that use glib, 
or some QT model using exactly the same engine. Hence the reason why i'm 
packaging it as a .a/.so library. (not that I particularly object to an 
embedded use case too).

I think that's a win in the end when people can just reuse wheels 
instead of designing new one for catering for special needs.
>  The real problem though is that the parsers I looked at had their own
> "object model", some of them are quite simple others are more sophisticated
> than QObject. Making no use of any kind of intermediate representation like
> this is a feature, as things get simpler.
>
>  Also, don't get me wrong, but if we would consider your parser we
> would have to consider the others two or three that are listed in
> json.org and have a compatible license.
>   
most of the parser there are either, weirdly licensed, have an object 
model integrated with it, are not interruptible,
or are quite complex for no apparent reason; I carefully read all of 
them, before choosing to reimplement one from scratch.

-- 
Vincent

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Anthony Liguori]

Vincent Hanquez wrote: 
> I can't think of any reason why integration with qobject would take 
> more than 50 lines of C on the user side of the library.
> since the API is completely SAX like (i call it SAJ for obvious 
> reason), you get callback entering/leaving object/array
> and callback for every values (string, int, float, null, true, false) 
> as a char * + length. for exactly the same reason, integration with 
> glib would take the same 50 lines "effort".
>
> note that FTR, obviously i'ld like to have my library used, but i'm 
> happy that any library that is *fully* JSON compliant is used (no 
> extensions however since you're obviously loosing the benefit of using 
> JSON if you create extensions).

We need two sets of extensions for use within qemu.  Single quoted 
strings and varargs support.  While single quoted strings would be easy 
to add to any library, vararg support is a bit more tricky as you need 
to carefully consider where you pop from the varargs list.  A simple 
sprintf() isn't sufficient for embedding QObjects.

When generating on-the-wire response traffic, we shouldn't use any of 
the extensions so it will be 100% json.org compliant.

I'm pretty sure if you tried to duplicate the functionality of my 
patches, it would be much more than 50 lines.  That's not saying it's a 
better json parser, just that we're looking for very particular features 
from it.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Vincent Hanquez]

Anthony Liguori wrote:
> Vincent Hanquez wrote:
>> I can't think of any reason why integration with qobject would take 
>> more than 50 lines of C on the user side of the library.
>> since the API is completely SAX like (i call it SAJ for obvious 
>> reason), you get callback entering/leaving object/array
>> and callback for every values (string, int, float, null, true, false) 
>> as a char * + length. for exactly the same reason, integration with 
>> glib would take the same 50 lines "effort".
>>
>> note that FTR, obviously i'ld like to have my library used, but i'm 
>> happy that any library that is *fully* JSON compliant is used (no 
>> extensions however since you're obviously loosing the benefit of 
>> using JSON if you create extensions).
>
> We need two sets of extensions for use within qemu.  Single quoted 
> strings and varargs support.  While single quoted strings would be 
> easy to add to any library, vararg support is a bit more tricky as you 
> need to carefully consider where you pop from the varargs list.  A 
> simple sprintf() isn't sufficient for embedding QObjects.
care to explain what's a single quoted string and varargs support means 
in your context ? (just a simple example you do maybe ?)
> When generating on-the-wire response traffic, we shouldn't use any of 
> the extensions so it will be 100% json.org compliant.
great.
> I'm pretty sure if you tried to duplicate the functionality of my 
> patches, it would be much more than 50 lines.  That's not saying it's 
> a better json parser, just that we're looking for very particular 
> features from it.
Since it doesn't appears to be linked to json particularly, I don't 
understand why it's a feature of the parser though.. and then any parser 
could grow the support you need on top of the parser couldn't they ?

-- 
Vincent

Re: [Qemu-devel] Re: [PATCH 01/10] Introduce qmisc module

[Paolo Bonzini]

On 10/18/2009 06:46 PM, Vincent Hanquez wrote:
> care to explain what's a single quoted string and varargs support means
> in your context ? (just a simple example you do maybe ?)

single-quoted string: Being able to parse 'name' in addition to "name", 
which is convenient because in C the latter would be \"name\".

varargs: Being able to call some external function when a %+letter 
sequence is found, which would fetch the key or value for an external 
source (for example a varargs list so that you can do a printf-style 
QObject factory function, where the template is itself written in 
JSON-like syntax).

The important thing anyway is that the encoder is conservative (i.e. 
100% valid JSON) in what it emits.  This is something everybody totally 
agrees on.

Paolo

[Qemu-devel] [PATCH 02/10] monitor: Convert do_memory_save() to QObject

[Luiz Capitulino]

Note that errors are not being converted yet.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c       |    2 +-
 qemu-monitor.hx |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/monitor.c b/monitor.c
index 3424e60..44ff994 100644
--- a/monitor.c
+++ b/monitor.c
@@ -977,7 +977,7 @@ static void do_print(Monitor *mon, const QDict *qdict)
     monitor_printf(mon, "\n");
 }
 
-static void do_memory_save(Monitor *mon, const QDict *qdict)
+static void do_memory_save(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     FILE *f;
     uint32_t size = qdict_get_int(qdict, "size");
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 29999c6..70b9125 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -666,7 +666,8 @@ ETEXI
         .args_type  = "val:l,size:i,filename:s",
         .params     = "addr size file",
         .help       = "save to disk virtual memory dump starting at 'addr' of size 'size'",
-        .mhandler.cmd = do_memory_save,
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_memory_save,
     },
 
 STEXI
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] [PATCH 03/10] monitor: Convert do_physical_memory_save() to QObject

[Luiz Capitulino]

Note that errors are not being converted yet.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c       |    3 ++-
 qemu-monitor.hx |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/monitor.c b/monitor.c
index 44ff994..a095971 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1008,7 +1008,8 @@ static void do_memory_save(Monitor *mon, const QDict *qdict, QObject **ret_data)
     fclose(f);
 }
 
-static void do_physical_memory_save(Monitor *mon, const QDict *qdict)
+static void do_physical_memory_save(Monitor *mon, const QDict *qdict,
+                                    QObject **ret_data)
 {
     FILE *f;
     uint32_t l;
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 70b9125..1c605bd 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -680,7 +680,8 @@ ETEXI
         .args_type  = "val:l,size:i,filename:s",
         .params     = "addr size file",
         .help       = "save to disk physical memory dump starting at 'addr' of size 'size'",
-        .mhandler.cmd = do_physical_memory_save,
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_physical_memory_save,
     },
 
 STEXI
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] [PATCH 04/10] monitor: Convert do_migrate() to QObject

[Luiz Capitulino]

Error is still directly printed, as we are only converting
regular output.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 migration.c     |    5 ++++-
 migration.h     |    2 +-
 qemu-monitor.hx |    3 ++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/migration.c b/migration.c
index 7f93e3f..4212f71 100644
--- a/migration.c
+++ b/migration.c
@@ -52,7 +52,10 @@ void qemu_start_incoming_migration(const char *uri)
         fprintf(stderr, "unknown migration protocol: %s\n", uri);
 }
 
-void do_migrate(Monitor *mon, const QDict *qdict)
+/**
+ * do_migrate(): Migrate the VM.
+ */
+void do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     MigrationState *s = NULL;
     const char *p;
diff --git a/migration.h b/migration.h
index 53b923d..17ca9ff 100644
--- a/migration.h
+++ b/migration.h
@@ -50,7 +50,7 @@ struct FdMigrationState
 
 void qemu_start_incoming_migration(const char *uri);
 
-void do_migrate(Monitor *mon, const QDict *qdict);
+void do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 void do_migrate_cancel(Monitor *mon, const QDict *qdict);
 
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 1c605bd..8e7bfd4 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -726,7 +726,8 @@ ETEXI
         .args_type  = "detach:-d,uri:s",
         .params     = "[-d] uri",
         .help       = "migrate to URI (using -d to not wait for completion)",
-        .mhandler.cmd = do_migrate,
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_migrate,
     },
 
 STEXI
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] [PATCH 05/10] monitor: Convert do_migrate_set_speed() to QObject

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 migration.c     |    5 ++++-
 migration.h     |    2 +-
 qemu-monitor.hx |    3 ++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/migration.c b/migration.c
index 4212f71..c2baeea 100644
--- a/migration.c
+++ b/migration.c
@@ -93,7 +93,10 @@ void do_migrate_cancel(Monitor *mon, const QDict *qdict)
         s->cancel(s);
 }
 
-void do_migrate_set_speed(Monitor *mon, const QDict *qdict)
+/**
+ * do_migrate_set_speed(): Set maximum migration speed.
+ */
+void do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     double d;
     char *ptr;
diff --git a/migration.h b/migration.h
index 17ca9ff..40bd610 100644
--- a/migration.h
+++ b/migration.h
@@ -54,7 +54,7 @@ void do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 void do_migrate_cancel(Monitor *mon, const QDict *qdict);
 
-void do_migrate_set_speed(Monitor *mon, const QDict *qdict);
+void do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 uint64_t migrate_max_downtime(void);
 
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 8e7bfd4..af1bae5 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -753,7 +753,8 @@ ETEXI
         .args_type  = "value:s",
         .params     = "value",
         .help       = "set maximum speed (in bytes) for migrations",
-        .mhandler.cmd = do_migrate_set_speed,
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_migrate_set_speed,
     },
 
 STEXI
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] [PATCH 06/10] monitor: Convert do_migrate_cancel() to QObject

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 migration.c     |    5 ++++-
 migration.h     |    2 +-
 qemu-monitor.hx |    3 ++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/migration.c b/migration.c
index c2baeea..112a5b6 100644
--- a/migration.c
+++ b/migration.c
@@ -85,7 +85,10 @@ void do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
     }
 }
 
-void do_migrate_cancel(Monitor *mon, const QDict *qdict)
+/**
+ * do_migrate_cancel(): Cancel the current VM migration.
+ */
+void do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     MigrationState *s = current_migration;
 
diff --git a/migration.h b/migration.h
index 40bd610..2d28b8f 100644
--- a/migration.h
+++ b/migration.h
@@ -52,7 +52,7 @@ void qemu_start_incoming_migration(const char *uri);
 
 void do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
-void do_migrate_cancel(Monitor *mon, const QDict *qdict);
+void do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 void do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index af1bae5..41fbfd3 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -740,7 +740,8 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "cancel the current VM migration",
-        .mhandler.cmd = do_migrate_cancel,
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_migrate_cancel,
     },
 
 STEXI
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] [PATCH 07/10] monitor: Convert do_info_migrate() to QObject

[Luiz Capitulino]

Return a QDict, which may contain another QDict if the migration
process is active.

The main QDict contains the following:

- "status": migration status
- "ram": only present if "status" is "active", it is a QDict with the
   following information (in bytes):
        - "transferred": amount of RAM transferred
        - "remaining": amount of RAM remaining
        - "total": total RAM

IMPORTANT: as a QInt stores a int64_t integer, those RAM values
are going to stored as int64_t and not as uint64_t as they are
today. If this is a problem QInt will have to be changed.

This commit should not change user output, the following is an
example of the returned QDict:

{ "status": "active", "ram":
   { "transferred": 885030912, "remaining": 198529024, "total": 1082392576 } }

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 migration.c |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
 migration.h |    3 +-
 monitor.c   |    3 +-
 3 files changed, 64 insertions(+), 11 deletions(-)

diff --git a/migration.c b/migration.c
index 112a5b6..b3bf00f 100644
--- a/migration.c
+++ b/migration.c
@@ -18,6 +18,7 @@
 #include "sysemu.h"
 #include "block.h"
 #include "qemu_socket.h"
+#include "qmisc.h"
 
 //#define DEBUG_MIGRATION
 
@@ -158,29 +159,79 @@ void do_migrate_set_downtime(Monitor *mon, const QDict *qdict)
     max_downtime = (uint64_t)d;
 }
 
-void do_info_migrate(Monitor *mon)
+void migration_user_print(Monitor *mon, const QObject *data)
+{
+    QDict *qdict;
+
+    assert(qobject_type(data) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(data);
+
+    monitor_printf(mon, "Migration status: %s\n",
+                   qdict_get_str(qdict, "status"));
+
+    if (qdict_haskey(qdict, "ram")) {
+        QDict *qdict_ram = qobject_to_qdict(qdict_get(qdict, "ram"));
+        monitor_printf(mon, "transferred ram: %" PRIu64 " kbytes\n",
+                       qdict_get_int(qdict_ram, "transferred") >> 10);
+        monitor_printf(mon, "remaining ram: %" PRIu64 " kbytes\n",
+                       qdict_get_int(qdict_ram, "remaining") >> 10);
+        monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n",
+                       qdict_get_int(qdict_ram, "total") >> 10);
+    }
+}
+
+/**
+ * do_info_migrate(): Show migration status
+ *
+ * Return a QDict. If migration is active there will be another
+ * QDict with RAM information.
+ *
+ * The main QDict contains the following:
+ *
+ * - "status": migration status
+ * - "ram": only present if "status" is "active", it is a QDict with the
+ *   following information (in bytes):
+ *          - "transferred": amount of RAM transferred
+ *          - "remaining": amount of RAM remaining
+ *          - "total": total RAM
+ *
+ * Examples:
+ *
+ * 1. If migration is "completed", "error" or "cancelled":
+ *
+ * { "status": "completed" }
+ *
+ * 2. If migration is "active":
+ *
+ * { "status": "active", "ram":
+ *             { "transferred": 123, "remaining": 123, "total": 246 } }
+ */
+void do_info_migrate(Monitor *mon, QObject **ret_data)
 {
     MigrationState *s = current_migration;
 
     if (s) {
-        monitor_printf(mon, "Migration status: ");
         switch (s->get_status(s)) {
         case MIG_STATE_ACTIVE:
-            monitor_printf(mon, "active\n");
-            monitor_printf(mon, "transferred ram: %" PRIu64 " kbytes\n", ram_bytes_transferred() >> 10);
-            monitor_printf(mon, "remaining ram: %" PRIu64 " kbytes\n", ram_bytes_remaining() >> 10);
-            monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n", ram_bytes_total() >> 10);
+            *ret_data = qobject_from_fmt("{ s: s, s: { s: i, s: i, s: i } }",
+                                         "status", "active",
+                                         "ram",
+                                         "transferred", ram_bytes_transferred(),
+                                         "remaining", ram_bytes_remaining(),
+                                         "total", ram_bytes_total());
             break;
         case MIG_STATE_COMPLETED:
-            monitor_printf(mon, "completed\n");
+            *ret_data = qobject_from_fmt("{ s: s }", "status", "completed");
             break;
         case MIG_STATE_ERROR:
-            monitor_printf(mon, "failed\n");
+            *ret_data = qobject_from_fmt("{ s: s }", "status", "failed");
             break;
         case MIG_STATE_CANCELLED:
-            monitor_printf(mon, "cancelled\n");
+            *ret_data = qobject_from_fmt("{ s: s }", "status", "cancelled");
             break;
         }
+        if (*ret_data == NULL)
+            monitor_printf(mon, "Migration: could not build QObject\n");
     }
 }
 
diff --git a/migration.h b/migration.h
index 2d28b8f..7666c4b 100644
--- a/migration.h
+++ b/migration.h
@@ -60,7 +60,8 @@ uint64_t migrate_max_downtime(void);
 
 void do_migrate_set_downtime(Monitor *mon, const QDict *qdict);
 
-void do_info_migrate(Monitor *mon);
+void migration_user_print(Monitor *mon, const QObject *data);
+void do_info_migrate(Monitor *mon, QObject **ret_data);
 
 int exec_start_incoming_migration(const char *host_port);
 
diff --git a/monitor.c b/monitor.c
index a095971..7e5c07d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2177,7 +2177,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show migration status",
-        .mhandler.info = do_info_migrate,
+        .user_print = migration_user_print,
+        .mhandler.info_new = do_info_migrate,
     },
     {
         .name       = "balloon",
-- 
1.6.5.rc2.17.gdbc1b

Re: [Qemu-devel] [PATCH 07/10] monitor: Convert do_info_migrate() to QObject

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> Return a QDict, which may contain another QDict if the migration
> process is active.
>
> The main QDict contains the following:
>
> - "status": migration status
> - "ram": only present if "status" is "active", it is a QDict with the
>    following information (in bytes):
>         - "transferred": amount of RAM transferred
>         - "remaining": amount of RAM remaining
>         - "total": total RAM
>
> IMPORTANT: as a QInt stores a int64_t integer, those RAM values
> are going to stored as int64_t and not as uint64_t as they are
> today. If this is a problem QInt will have to be changed.
>
> This commit should not change user output, the following is an
> example of the returned QDict:
>
> { "status": "active", "ram":
>    { "transferred": 885030912, "remaining": 198529024, "total": 1082392576 } }
[...]
> diff --git a/migration.c b/migration.c
> index 112a5b6..b3bf00f 100644
> --- a/migration.c
> +++ b/migration.c
[...]
> @@ -158,29 +159,79 @@ void do_migrate_set_downtime(Monitor *mon, const QDict *qdict)
[...]
> +/**
> + * do_info_migrate(): Show migration status
> + *
> + * Return a QDict. If migration is active there will be another
> + * QDict with RAM information.
> + *
> + * The main QDict contains the following:
> + *
> + * - "status": migration status
> + * - "ram": only present if "status" is "active", it is a QDict with the
> + *   following information (in bytes):
> + *          - "transferred": amount of RAM transferred
> + *          - "remaining": amount of RAM remaining
> + *          - "total": total RAM
> + *
> + * Examples:
> + *
> + * 1. If migration is "completed", "error" or "cancelled":
> + *
> + * { "status": "completed" }

This suggests that a failed or cancelled migration is reported with
"completed", which is wrong.  Since it's just an example, you could
leave out the ', "error" or "cancelled"' part.

> + *
> + * 2. If migration is "active":
> + *
> + * { "status": "active", "ram":
> + *             { "transferred": 123, "remaining": 123, "total": 246 } }
> + */
> +void do_info_migrate(Monitor *mon, QObject **ret_data)
>  {
>      MigrationState *s = current_migration;
>  
>      if (s) {
> -        monitor_printf(mon, "Migration status: ");
>          switch (s->get_status(s)) {
>          case MIG_STATE_ACTIVE:
> -            monitor_printf(mon, "active\n");
> -            monitor_printf(mon, "transferred ram: %" PRIu64 " kbytes\n", ram_bytes_transferred() >> 10);
> -            monitor_printf(mon, "remaining ram: %" PRIu64 " kbytes\n", ram_bytes_remaining() >> 10);
> -            monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n", ram_bytes_total() >> 10);
> +            *ret_data = qobject_from_fmt("{ s: s, s: { s: i, s: i, s: i } }",
> +                                         "status", "active",
> +                                         "ram",
> +                                         "transferred", ram_bytes_transferred(),
> +                                         "remaining", ram_bytes_remaining(),
> +                                         "total", ram_bytes_total());
>              break;
>          case MIG_STATE_COMPLETED:
> -            monitor_printf(mon, "completed\n");
> +            *ret_data = qobject_from_fmt("{ s: s }", "status", "completed");
>              break;
>          case MIG_STATE_ERROR:
> -            monitor_printf(mon, "failed\n");
> +            *ret_data = qobject_from_fmt("{ s: s }", "status", "failed");
>              break;
>          case MIG_STATE_CANCELLED:
> -            monitor_printf(mon, "cancelled\n");
> +            *ret_data = qobject_from_fmt("{ s: s }", "status", "cancelled");
>              break;
>          }
> +        if (*ret_data == NULL)
> +            monitor_printf(mon, "Migration: could not build QObject\n");
>      }
>  }
>  
[...]

Re: [Qemu-devel] [PATCH 07/10] monitor: Convert do_info_migrate() to QObject

[Anthony Liguori]

Luiz Capitulino wrote:
> Return a QDict, which may contain another QDict if the migration
> process is active.
>
> The main QDict contains the following:
>
> - "status": migration status
> - "ram": only present if "status" is "active", it is a QDict with the
>    following information (in bytes):
>         - "transferred": amount of RAM transferred
>         - "remaining": amount of RAM remaining
>         - "total": total RAM
>
> IMPORTANT: as a QInt stores a int64_t integer, those RAM values
> are going to stored as int64_t and not as uint64_t as they are
> today. If this is a problem QInt will have to be changed.
>
> This commit should not change user output, the following is an
> example of the returned QDict:
>
> { "status": "active", "ram":
>    { "transferred": 885030912, "remaining": 198529024, "total": 1082392576 } }
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  migration.c |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
>  migration.h |    3 +-
>  monitor.c   |    3 +-
>  3 files changed, 64 insertions(+), 11 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 112a5b6..b3bf00f 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -18,6 +18,7 @@
>  #include "sysemu.h"
>  #include "block.h"
>  #include "qemu_socket.h"
> +#include "qmisc.h"
>  
>  //#define DEBUG_MIGRATION
>  
> @@ -158,29 +159,79 @@ void do_migrate_set_downtime(Monitor *mon, const QDict *qdict)
>      max_downtime = (uint64_t)d;
>  }
>  
> -void do_info_migrate(Monitor *mon)
> +void migration_user_print(Monitor *mon, const QObject *data)
> +{
> +    QDict *qdict;
> +
> +    assert(qobject_type(data) == QTYPE_QDICT);
> +    qdict = qobject_to_qdict(data);
> +
> +    monitor_printf(mon, "Migration status: %s\n",
> +                   qdict_get_str(qdict, "status"));
> +
> +    if (qdict_haskey(qdict, "ram")) {
> +        QDict *qdict_ram = qobject_to_qdict(qdict_get(qdict, "ram"));
> +        monitor_printf(mon, "transferred ram: %" PRIu64 " kbytes\n",
> +                       qdict_get_int(qdict_ram, "transferred") >> 10);
> +        monitor_printf(mon, "remaining ram: %" PRIu64 " kbytes\n",
> +                       qdict_get_int(qdict_ram, "remaining") >> 10);
> +        monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n",
> +                       qdict_get_int(qdict_ram, "total") >> 10);
> +    }
> +}
> +
> +/**
> + * do_info_migrate(): Show migration status
> + *
> + * Return a QDict. If migration is active there will be another
> + * QDict with RAM information.
> + *
> + * The main QDict contains the following:
> + *
> + * - "status": migration status
> + * - "ram": only present if "status" is "active", it is a QDict with the
> + *   following information (in bytes):
> + *          - "transferred": amount of RAM transferred
> + *          - "remaining": amount of RAM remaining
> + *          - "total": total RAM
> + *
> + * Examples:
> + *
> + * 1. If migration is "completed", "error" or "cancelled":
> + *
> + * { "status": "completed" }
> + *
> + * 2. If migration is "active":
> + *
> + * { "status": "active", "ram":
> + *             { "transferred": 123, "remaining": 123, "total": 246 } }
> + */
> +void do_info_migrate(Monitor *mon, QObject **ret_data)
>  {
>      MigrationState *s = current_migration;
>  
>      if (s) {
> -        monitor_printf(mon, "Migration status: ");
>          switch (s->get_status(s)) {
>          case MIG_STATE_ACTIVE:
> -            monitor_printf(mon, "active\n");
> -            monitor_printf(mon, "transferred ram: %" PRIu64 " kbytes\n", ram_bytes_transferred() >> 10);
> -            monitor_printf(mon, "remaining ram: %" PRIu64 " kbytes\n", ram_bytes_remaining() >> 10);
> -            monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n", ram_bytes_total() >> 10);
> +            *ret_data = qobject_from_fmt("{ s: s, s: { s: i, s: i, s: i } }",
> +                                         "status", "active",
> +                                         "ram",
> +                                         "transferred", ram_bytes_transferred(),
> +                                         "remaining", ram_bytes_remaining(),
> +                                         "total", ram_bytes_total());
>   

While we're at it, let's add string parsing support.  So this becomes

*ret_data = qobject_from_fmt("{'status': %s, 'ram': {'transferred': %d, 
'remaining': %d, 'total': %d}}",
                                                 
ram_bytes_transferred(), ram_bytes_remaining(), ram_bytes_total());

Regards,

Anthony Liguori

[Qemu-devel] [PATCH 08/10] monitor: Convert bdrv_info() to QObject

[Luiz Capitulino]

Each block device information is stored in a QDict and the
returned QObject is a QList of all devices.

The QDict contains the following:

- "device": device name
- "type": device type
- "removable": 1 if the device is removable 0 otherwise
- "locked": 1 if the device is locked 0 otherwise
- "backing_file": backing file name if one is used
- "inserted": only present if the device is inserted, it is a QDict
  containing the following:
        - "file": device file name
        - "ro": 1 if read-only 0 otherwise
        - "drv": driver format name
        - "encrypted":  1 if encrypted 0 otherwise

The current implemention uses integers as booleans, to make
things simple those integers are stored in the QDict. Ideally,
we would have a QBool type and this is probably going to be
a requirement for the protocol.

But the integers will do the job for now.

This commit should not change user output, the following is an
example of the returned QList:

[ { "device": "ide0-hd0", "type": "hd", "removable": 0,
    "file": "/tmp/foobar", "ro": 0, "drv": "qcow2", "encrypted": 0 }
  { "device": "floppy0", "type": "floppy", "removable": 1,
  "locked": 0 } ]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 Makefile  |    1 +
 block.c   |  132 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
 block.h   |    4 +-
 monitor.c |    3 +-
 4 files changed, 118 insertions(+), 22 deletions(-)

diff --git a/Makefile b/Makefile
index 182f176..d29d871 100644
--- a/Makefile
+++ b/Makefile
@@ -62,6 +62,7 @@ recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES)
 # block-obj-y is code used by both qemu system emulation and qemu-img
 
 block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
+block-obj-y += qint.o qstring.o qlist.o qdict.o qmisc.o
 block-obj-y += nbd.o block.o aio.o aes.o osdep.o
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
diff --git a/block.c b/block.c
index 33f3d65..31a58e4 100644
--- a/block.c
+++ b/block.c
@@ -26,6 +26,10 @@
 #include "monitor.h"
 #include "block_int.h"
 #include "module.h"
+#include "qlist.h"
+#include "qdict.h"
+#include "qstring.h"
+#include "qmisc.h"
 
 #ifdef CONFIG_BSD
 #include <sys/types.h>
@@ -1075,43 +1079,131 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
     return bs->drv->bdrv_is_allocated(bs, sector_num, nb_sectors, pnum);
 }
 
-void bdrv_info(Monitor *mon)
+static void bdrv_print_dict(QObject *obj, void *opaque)
 {
+    QDict *bs_dict;
+    Monitor *mon = opaque;
+
+    assert(qobject_type(obj) == QTYPE_QDICT);
+    bs_dict = qobject_to_qdict(obj);
+
+    monitor_printf(mon, "%s: type=%s removable=%d",
+                        qdict_get_str(bs_dict, "device"),
+                        qdict_get_str(bs_dict, "type"),
+                        (int) qdict_get_int(bs_dict, "removable"));
+
+    if (qdict_get_int(bs_dict, "removable")) {
+        monitor_printf(mon, " locked=%d",(int)qdict_get_int(bs_dict, "locked"));
+    }
+
+    if (qdict_haskey(bs_dict, "inserted")) {
+        QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted"));
+        monitor_printf(mon, " file=%s", qdict_get_str(qdict, "file"));
+        if (qdict_haskey(qdict, "backing_file")) {
+            monitor_printf(mon, " backing_file=%s",
+                           qdict_get_str(qdict, "backing_file"));
+        }
+        monitor_printf(mon, " ro=%d drv=%s encrypted=%d",
+                            (int) qdict_get_int(qdict, "ro"),
+                            qdict_get_str(qdict, "drv"),
+                            (int) qdict_get_int(qdict, "encrypted"));
+    } else {
+        monitor_printf(mon, " [not inserted]");
+    }
+
+    monitor_printf(mon, "\n");
+}
+
+void bdrv_user_print(Monitor *mon, const QObject *data)
+{
+    QList *bs_list;
+
+    assert(qobject_type(data) == QTYPE_QLIST);
+    bs_list = qobject_to_qlist(data);
+
+    qlist_iter(bs_list, bdrv_print_dict, mon);
+}
+
+/**
+ * bdrv_info(): Show block devices
+ *
+ * Each block device information is stored in a QDict and the
+ * returned QObject is a QList of all devices.
+ *
+ * The QDict contains the following:
+ *
+ * - "device": device name
+ * - "type": device type
+ * - "removable": 1 if the device is removable 0 otherwise
+ * - "locked": 1 if the device is locked 0 otherwise
+ * - "backing_file": backing file name if one is used
+ * - "inserted": only present if the device is inserted, it is a QDict
+ *    containing the following:
+ *          - "file": device file name
+ *          - "ro": 1 if read-only 0 otherwise
+ *          - "drv": driver format name
+ *          - "encrypted":  1 if encrypted 0 otherwise
+ *
+ * Example:
+ *
+ * [ { "device": "ide0-hd0", "type": "hd", "removable": 0,
+ *     "file": "/tmp/foobar", "ro": 0, "drv": "qcow2", "encrypted": 0 }
+ *   { "device": "floppy0", "type": "floppy", "removable": 1,
+ *     "locked": 0 } ]
+ */
+void bdrv_info(Monitor *mon, QObject **ret_data)
+{
+    QList *bs_list;
     BlockDriverState *bs;
 
+    bs_list = qlist_new();
+
     for (bs = bdrv_first; bs != NULL; bs = bs->next) {
-        monitor_printf(mon, "%s:", bs->device_name);
-        monitor_printf(mon, " type=");
+        QObject *bs_obj;
+        const char *type = "unknown";
+
         switch(bs->type) {
         case BDRV_TYPE_HD:
-            monitor_printf(mon, "hd");
+            type = "hd";
             break;
         case BDRV_TYPE_CDROM:
-            monitor_printf(mon, "cdrom");
+            type = "cdrom";
             break;
         case BDRV_TYPE_FLOPPY:
-            monitor_printf(mon, "floppy");
+            type = "floppy";
             break;
         }
-        monitor_printf(mon, " removable=%d", bs->removable);
-        if (bs->removable) {
-            monitor_printf(mon, " locked=%d", bs->locked);
-        }
+
+        bs_obj = qobject_from_fmt("{ s: s, s: s, s: i, s: i }",
+                                  "device", bs->device_name,
+                                  "type", type,
+                                  "removable", bs->removable,
+                                  "locked", bs->locked);
+        assert(bs_obj != NULL);
+
         if (bs->drv) {
-            monitor_printf(mon, " file=");
-            monitor_print_filename(mon, bs->filename);
+            QObject *obj;
+            QDict *bs_dict = qobject_to_qdict(bs_obj);
+
+            obj = qobject_from_fmt("{ s: s, s: i, s: s, s: i }",
+                                   "file", bs->filename,
+                                   "ro", bs->read_only,
+                                   "drv", bs->drv->format_name,
+                                   "encrypted", bdrv_is_encrypted(bs));
+            assert(obj != NULL);
+
             if (bs->backing_file[0] != '\0') {
-                monitor_printf(mon, " backing_file=");
-                monitor_print_filename(mon, bs->backing_file);
+                QDict *qdict = qobject_to_qdict(obj);
+                qdict_put(qdict, "backing_file",
+                          qstring_from_str(bs->backing_file));
             }
-            monitor_printf(mon, " ro=%d", bs->read_only);
-            monitor_printf(mon, " drv=%s", bs->drv->format_name);
-            monitor_printf(mon, " encrypted=%d", bdrv_is_encrypted(bs));
-        } else {
-            monitor_printf(mon, " [not inserted]");
+            qdict_put_obj(bs_dict, "inserted", obj);
         }
-        monitor_printf(mon, "\n");
+
+        qlist_append_obj(bs_list, bs_obj);
     }
+
+    *ret_data = QOBJECT(bs_list);
 }
 
 /* The "info blockstats" command. */
diff --git a/block.h b/block.h
index a966afb..99dc360 100644
--- a/block.h
+++ b/block.h
@@ -4,6 +4,7 @@
 #include "qemu-aio.h"
 #include "qemu-common.h"
 #include "qemu-option.h"
+#include "qobject.h"
 
 /* block.c */
 typedef struct BlockDriver BlockDriver;
@@ -41,7 +42,8 @@ typedef struct QEMUSnapshotInfo {
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB)
 
-void bdrv_info(Monitor *mon);
+void bdrv_user_print(Monitor *mon, const QObject *data);
+void bdrv_info(Monitor *mon, QObject **ret_data);
 void bdrv_info_stats(Monitor *mon);
 
 void bdrv_init(void);
diff --git a/monitor.c b/monitor.c
index 7e5c07d..39d3201 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1979,7 +1979,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the block devices",
-        .mhandler.info = bdrv_info,
+        .user_print = bdrv_user_print,
+        .mhandler.info_new = bdrv_info,
     },
     {
         .name       = "blockstats",
-- 
1.6.5.rc2.17.gdbc1b

Re: [Qemu-devel] [PATCH 08/10] monitor: Convert bdrv_info() to QObject

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> Each block device information is stored in a QDict and the
> returned QObject is a QList of all devices.
>
> The QDict contains the following:
>
> - "device": device name
> - "type": device type
> - "removable": 1 if the device is removable 0 otherwise
> - "locked": 1 if the device is locked 0 otherwise
> - "backing_file": backing file name if one is used

Isn't this in "interted" rather than here?

If yes, the comment in the code needs fixing, too.

> - "inserted": only present if the device is inserted, it is a QDict

I'd call this "media".

>   containing the following:
>         - "file": device file name
>         - "ro": 1 if read-only 0 otherwise
>         - "drv": driver format name
>         - "encrypted":  1 if encrypted 0 otherwise
>
> The current implemention uses integers as booleans, to make
> things simple those integers are stored in the QDict. Ideally,
> we would have a QBool type and this is probably going to be
> a requirement for the protocol.
>
> But the integers will do the job for now.
>
> This commit should not change user output, the following is an
> example of the returned QList:
>
> [ { "device": "ide0-hd0", "type": "hd", "removable": 0,
>     "file": "/tmp/foobar", "ro": 0, "drv": "qcow2", "encrypted": 0 }
>   { "device": "floppy0", "type": "floppy", "removable": 1,
>   "locked": 0 } ]
[...]

Re: [Qemu-devel] [PATCH 08/10] monitor: Convert bdrv_info() to QObject

[Luiz Capitulino]

On Sat, 10 Oct 2009 14:18:03 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > Each block device information is stored in a QDict and the
> > returned QObject is a QList of all devices.
> >
> > The QDict contains the following:
> >
> > - "device": device name
> > - "type": device type
> > - "removable": 1 if the device is removable 0 otherwise
> > - "locked": 1 if the device is locked 0 otherwise
> > - "backing_file": backing file name if one is used
> 
> Isn't this in "interted" rather than here?
> 
> If yes, the comment in the code needs fixing, too.

 Yes, although I don't think I should resend just because of that.

Re: [Qemu-devel] [PATCH 08/10] monitor: Convert bdrv_info() to QObject

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Sat, 10 Oct 2009 14:18:03 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>> 
>> > Each block device information is stored in a QDict and the
>> > returned QObject is a QList of all devices.
>> >
>> > The QDict contains the following:
>> >
>> > - "device": device name
>> > - "type": device type
>> > - "removable": 1 if the device is removable 0 otherwise
>> > - "locked": 1 if the device is locked 0 otherwise
>> > - "backing_file": backing file name if one is used
>> 
>> Isn't this in "interted" rather than here?
>> 
>> If yes, the comment in the code needs fixing, too.
>
>  Yes, although I don't think I should resend just because of that.

Any other way to reliably avoid commit of a misleading comment with a
misleading commit message?

If it goes in that way, we need to follow up with a commit fixing the
comment.

Re: [Qemu-devel] [PATCH 08/10] monitor: Convert bdrv_info() to QObject

[Anthony Liguori]

Luiz Capitulino wrote:
> Each block device information is stored in a QDict and the
> returned QObject is a QList of all devices.
>
> The QDict contains the following:
>
> - "device": device name
> - "type": device type
> - "removable": 1 if the device is removable 0 otherwise
> - "locked": 1 if the device is locked 0 otherwise
> - "backing_file": backing file name if one is used
> - "inserted": only present if the device is inserted, it is a QDict
>   containing the following:
>         - "file": device file name
>         - "ro": 1 if read-only 0 otherwise
>         - "drv": driver format name
>         - "encrypted":  1 if encrypted 0 otherwise
>
> The current implemention uses integers as booleans, to make
> things simple those integers are stored in the QDict. Ideally,
> we would have a QBool type and this is probably going to be
> a requirement for the protocol.
>
> But the integers will do the job for now.
>
> This commit should not change user output, the following is an
> example of the returned QList:
>
> [ { "device": "ide0-hd0", "type": "hd", "removable": 0,
>     "file": "/tmp/foobar", "ro": 0, "drv": "qcow2", "encrypted": 0 }
>   { "device": "floppy0", "type": "floppy", "removable": 1,
>   "locked": 0 } ]
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  Makefile  |    1 +
>  block.c   |  132 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
>  block.h   |    4 +-
>  monitor.c |    3 +-
>  4 files changed, 118 insertions(+), 22 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 182f176..d29d871 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -62,6 +62,7 @@ recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES)
>  # block-obj-y is code used by both qemu system emulation and qemu-img
>  
>  block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
> +block-obj-y += qint.o qstring.o qlist.o qdict.o qmisc.o
>  block-obj-y += nbd.o block.o aio.o aes.o osdep.o
>  block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
>  block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
> diff --git a/block.c b/block.c
> index 33f3d65..31a58e4 100644
> --- a/block.c
> +++ b/block.c
> @@ -26,6 +26,10 @@
>  #include "monitor.h"
>  #include "block_int.h"
>  #include "module.h"
> +#include "qlist.h"
> +#include "qdict.h"
> +#include "qstring.h"
> +#include "qmisc.h"
>  
>  #ifdef CONFIG_BSD
>  #include <sys/types.h>
> @@ -1075,43 +1079,131 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
>      return bs->drv->bdrv_is_allocated(bs, sector_num, nb_sectors, pnum);
>  }
>  
> -void bdrv_info(Monitor *mon)
> +static void bdrv_print_dict(QObject *obj, void *opaque)
>  {
> +    QDict *bs_dict;
> +    Monitor *mon = opaque;
> +
> +    assert(qobject_type(obj) == QTYPE_QDICT);
> +    bs_dict = qobject_to_qdict(obj);
> +
> +    monitor_printf(mon, "%s: type=%s removable=%d",
> +                        qdict_get_str(bs_dict, "device"),
> +                        qdict_get_str(bs_dict, "type"),
> +                        (int) qdict_get_int(bs_dict, "removable"));
>   

This is a very common format.  So much so, it probably makes sense to have:

monitor_print_dict(mon, "device", bs_dict);

> +
> +    if (qdict_get_int(bs_dict, "removable")) {
> +        monitor_printf(mon, " locked=%d",(int)qdict_get_int(bs_dict, "locked"));
> +    }
>   

Which suggests that the dict entry should be locked, not removable.

> +    if (qdict_haskey(bs_dict, "inserted")) {
>   

I guess you could flatten nested dicts.

> +        QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted"));
> +        monitor_printf(mon, " file=%s", qdict_get_str(qdict, "file"));
> +        if (qdict_haskey(qdict, "backing_file")) {
> +            monitor_printf(mon, " backing_file=%s",
> +                           qdict_get_str(qdict, "backing_file"));
> +        }
> +        monitor_printf(mon, " ro=%d drv=%s encrypted=%d",
> +                            (int) qdict_get_int(qdict, "ro"),
> +                            qdict_get_str(qdict, "drv"),
> +                            (int) qdict_get_int(qdict, "encrypted"));
> +    } else {
> +        monitor_printf(mon, " [not inserted]");
>   

This bit you would probably have to handle manually.

Alternatively, you could build a compat dict from the new dict format 
and then print that with the monitor_print_dict function.
   
Regards,

Anthony Liguori

[Qemu-devel] [PATCH 09/10] monitor: Convert pci_device_hot_add() to QObject

[Luiz Capitulino]

Return a QDict with the following device information:

- "domain": domain number
- "bus": bus number
- "slot": slot number
- "function": function number

This commit should not change user output, the following is an
example of the returned QDict:

{ "domain": 0, "bus": 0, "slot": 5, "function": 0 }

Please, note that this is only in case of success. In error
conditions the handler still calls monitor_printf() directly,
as errors are not being converted yet.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hw/pci-hotplug.c |   40 ++++++++++++++++++++++++++++++++++++----
 qemu-monitor.hx  |    3 ++-
 sysemu.h         |    3 ++-
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index ccd2cf3..3234265 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -33,6 +33,7 @@
 #include "scsi-disk.h"
 #include "virtio-blk.h"
 #include "qemu-config.h"
+#include "qmisc.h"
 
 #if defined(TARGET_I386) || defined(TARGET_X86_64)
 static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
@@ -177,7 +178,36 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
     return dev;
 }
 
-void pci_device_hot_add(Monitor *mon, const QDict *qdict)
+void pci_add_user_print(Monitor *mon, const QObject *data)
+{
+    QDict *qdict;
+
+    assert(qobject_type(data) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(data);
+
+    monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
+                   (int) qdict_get_int(qdict, "domain"),
+                   (int) qdict_get_int(qdict, "bus"),
+                   (int) qdict_get_int(qdict, "slot"),
+                   (int) qdict_get_int(qdict, "function"));
+
+}
+
+/**
+ * pci_device_hot_add(): Hot add PCI device
+ *
+ * Return a QDict with the following device information:
+ *
+ * - "domain": domain number
+ * - "bus": bus number
+ * - "slot": slot number
+ * - "function": function number
+ *
+ * Example:
+ *
+ * { "domain": 0, "bus": 0, "slot": 5, "function": 0 }
+ */
+void pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     PCIDevice *dev = NULL;
     const char *pci_addr = qdict_get_str(qdict, "pci_addr");
@@ -204,9 +234,11 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, "invalid type: %s\n", type);
 
     if (dev) {
-        monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
-                       0, pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
-                       PCI_FUNC(dev->devfn));
+        *ret_data = qobject_from_fmt(" { s:i, s:i, s:i, s:i }",
+                                     "domain", 0, "bus", pci_bus_num(dev->bus),
+                                     "slot", PCI_SLOT(dev->devfn),
+                                     "function", PCI_FUNC(dev->devfn));
+        assert(*ret_data != NULL);
     } else
         monitor_printf(mon, "failed to add %s\n", opts);
 }
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 41fbfd3..37dc863 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -801,7 +801,8 @@ ETEXI
         .args_type  = "pci_addr:s,type:s,opts:s?",
         .params     = "auto|[[<domain>:]<bus>:]<slot> nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]...",
         .help       = "hot-add PCI device",
-        .mhandler.cmd = pci_device_hot_add,
+        .user_print = pci_add_user_print,
+        .mhandler.cmd_new = pci_device_hot_add,
     },
 #endif
 
diff --git a/sysemu.h b/sysemu.h
index 763861d..17b91c4 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -208,7 +208,8 @@ DriveInfo *add_init_drive(const char *opts);
 void destroy_nic(dev_match_fn *match_fn, void *arg);
 
 /* pci-hotplug */
-void pci_device_hot_add(Monitor *mon, const QDict *qdict);
+void pci_add_user_print(Monitor *mon, const QObject *data);
+void pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
 void drive_hot_add(Monitor *mon, const QDict *qdict);
 void pci_device_hot_remove(Monitor *mon, const char *pci_addr);
 void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] [PATCH 10/10] monitor: Convert do_pci_device_hot_remove() to QObject

[Luiz Capitulino]

Errors are still directly printed, as we are only converting
regular output.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hw/pci-hotplug.c |    3 ++-
 qemu-monitor.hx  |    3 ++-
 sysemu.h         |    3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 3234265..ce06573 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -262,7 +262,8 @@ void pci_device_hot_remove(Monitor *mon, const char *pci_addr)
     qdev_unplug(&d->qdev);
 }
 
-void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict)
+void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict,
+                              QObject **ret_data)
 {
     pci_device_hot_remove(mon, qdict_get_str(qdict, "pci_addr"));
 }
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 37dc863..1c5879a 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -817,7 +817,8 @@ ETEXI
         .args_type  = "pci_addr:s",
         .params     = "[[<domain>:]<bus>:]<slot>",
         .help       = "hot remove PCI device",
-        .mhandler.cmd = do_pci_device_hot_remove,
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_pci_device_hot_remove,
     },
 #endif
 
diff --git a/sysemu.h b/sysemu.h
index 17b91c4..896916f 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -212,7 +212,8 @@ void pci_add_user_print(Monitor *mon, const QObject *data);
 void pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
 void drive_hot_add(Monitor *mon, const QDict *qdict);
 void pci_device_hot_remove(Monitor *mon, const char *pci_addr);
-void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
+void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict,
+                              QObject **ret_data);
 void pci_device_hot_remove_success(PCIDevice *dev);
 
 /* serial ports */
-- 
1.6.5.rc2.17.gdbc1b

Re: [Qemu-devel] [PATCH v0 00/10]: More QObject conversions

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

>  Hi there,
>
>  Here goes another set of conversions, just to keep the ball rolling.
>
>  The first patch introduces a function called qobject_from_fmt(), which can
> be used to build QObjects from a specified printf-like format.
>
>  Most of its code was based on a Python function with similiar functionality,
> as far as I could understand the license is compatible and I've added a
> copyright notice.
>
>  Is this the right procedure?
>
>  Most patches are simple (as we aren't converting errors yet), except the
> last ones. Which use qobject_from_fmt() and are very cool, hopefully they're
> right too. :)
>
> PS: This series apply on top of my last 'Initial QObject conversion' series.
>
>  Thanks.

The separation of presentation from logic seems to work reasonably well
so far.  The separation involves building explicit data to be sent by a
separate presentation function instead of printing right away.  It's key
that this data building code is about as legible and easy to maintain as
the old, straightforward printing.  qobject_from_fmt() helps with that,
in my opinion.

Of course, much "interesting" stuff remains, chiefly errors.  Also any
output that doesn't fit into the "run command to completion, report its
result" mold (not sure we need that).  We'll see how it goes.


PS: Fun to see Greenspun's Tenth Rule in action once more.

Re: [Qemu-devel] [PATCH v0 00/10]: More QObject conversions

[Luiz Capitulino]

On Sat, 10 Oct 2009 14:31:12 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> >  Hi there,
> >
> >  Here goes another set of conversions, just to keep the ball rolling.
> >
> >  The first patch introduces a function called qobject_from_fmt(), which can
> > be used to build QObjects from a specified printf-like format.
> >
> >  Most of its code was based on a Python function with similiar functionality,
> > as far as I could understand the license is compatible and I've added a
> > copyright notice.
> >
> >  Is this the right procedure?
> >
> >  Most patches are simple (as we aren't converting errors yet), except the
> > last ones. Which use qobject_from_fmt() and are very cool, hopefully they're
> > right too. :)
> >
> > PS: This series apply on top of my last 'Initial QObject conversion' series.
> >
> >  Thanks.
> 
> The separation of presentation from logic seems to work reasonably well
> so far.  The separation involves building explicit data to be sent by a
> separate presentation function instead of printing right away.  It's key
> that this data building code is about as legible and easy to maintain as
> the old, straightforward printing.  qobject_from_fmt() helps with that,
> in my opinion.

 Yes, it helps, although adding more abstraction layers usually makes
things more complex.. That is, this is never going to be as simple as
directly calling monitor_printf().

> Of course, much "interesting" stuff remains, chiefly errors.

 Yes, and errors are more complicated than regular output because
sometimes we have something like:

command_handler()
 func1()
  func2()
   func3() <--- calls monitor_printf() on error

 Also, an error condition has to be differently handled from regular
output, so that the protocol can emit it as an error.

 I've implemented QError, which (IMHO) is a good enough solution
for the problem. Will submit it as soon as I get more feedback
on this last series.

> Also any
> output that doesn't fit into the "run command to completion, report its
> result" mold (not sure we need that).  We'll see how it goes.

 Do you have an existing example?

> PS: Fun to see Greenspun's Tenth Rule in action once more.

 Let's rewrite qemu in lisp then. :-)

Re: [Qemu-devel] [PATCH v0 00/10]: More QObject conversions

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Sat, 10 Oct 2009 14:31:12 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>> 
>> >  Hi there,
>> >
>> >  Here goes another set of conversions, just to keep the ball rolling.
>> >
>> >  The first patch introduces a function called qobject_from_fmt(), which can
>> > be used to build QObjects from a specified printf-like format.
>> >
>> >  Most of its code was based on a Python function with similiar functionality,
>> > as far as I could understand the license is compatible and I've added a
>> > copyright notice.
>> >
>> >  Is this the right procedure?
>> >
>> >  Most patches are simple (as we aren't converting errors yet), except the
>> > last ones. Which use qobject_from_fmt() and are very cool, hopefully they're
>> > right too. :)
>> >
>> > PS: This series apply on top of my last 'Initial QObject conversion' series.
>> >
>> >  Thanks.
>> 
>> The separation of presentation from logic seems to work reasonably well
>> so far.  The separation involves building explicit data to be sent by a
>> separate presentation function instead of printing right away.  It's key
>> that this data building code is about as legible and easy to maintain as
>> the old, straightforward printing.  qobject_from_fmt() helps with that,
>> in my opinion.
>
>  Yes, it helps, although adding more abstraction layers usually makes
> things more complex.. That is, this is never going to be as simple as
> directly calling monitor_printf().
>
>> Of course, much "interesting" stuff remains, chiefly errors.
>
>  Yes, and errors are more complicated than regular output because
> sometimes we have something like:
>
> command_handler()
>  func1()
>   func2()
>    func3() <--- calls monitor_printf() on error

Additionally, the fact that we failed is propagated up the call chain
some.  Ideally all the way up to command_handler().  Tedious, but
typically unavoidable, because upper functions need to be stopped from
continuing after failure.

>  Also, an error condition has to be differently handled from regular
> output, so that the protocol can emit it as an error.

It's a different kind of response, but it is built in similar ways.

A command sends either a normal response or an error response.  Both are
objects.

command_handler() builds a normal response object, possibly
incrementally, possibly delegating part of the job to other functions.
You use parameter passing for that.

When it runs into an error, the job needs to be aborted in an orderly
manner, and an error object needs to be built.  Any partially built
normal response needs to be discarded.

Both normal and error response object are built pretty much the same
way.  Maybe the same mechanism to pass them around could work for both.

>  I've implemented QError, which (IMHO) is a good enough solution
> for the problem. Will submit it as soon as I get more feedback
> on this last series.

Happy to review that when it's ready.

>> Also any
>> output that doesn't fit into the "run command to completion, report its
>> result" mold (not sure we need that).  We'll see how it goes.
>
>  Do you have an existing example?

It occured to me, i.e. it's purely theoretical until proven to exist.
Thus, "we'll see how it goes".

>> PS: Fun to see Greenspun's Tenth Rule in action once more.
>
>  Let's rewrite qemu in lisp then. :-)

Hey, it would settle the indentation arguments as well!