[Qemu-devel] [qapi] Cannot use list of strings

[Qemu-devel] [qapi] Cannot use list of strings

[Lluís Vilanova]

Tried using a list of strings as an argument to a command, but the generated
code references the 'strList' type, which does not exist.

Is a specialized version for "['str']" missing, or should I define my own type
with a single field of 'str' type?


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [qapi] Cannot use list of strings

[Lluís Vilanova]

Lluís Vilanova writes:

> Tried using a list of strings as an argument to a command, but the generated
> code references the 'strList' type, which does not exist.

> Is a specialized version for "['str']" missing, or should I define my own type
> with a single field of 'str' type?

I could also not find some handy code in QEMU to parse the arguments for the
instrumentation library. For example:

    qemu -instr file=/blah.so,arg=a1,arg=a2

Or just:

    qemu -instr file=/blah.so,args=a1,a2


As it seems, QEMU's option parsing has no routine that I could find to perform
this (it also looks like it's not possible to escape commas).


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [qapi] Cannot use list of strings

[Stefan Hajnoczi]

On Mon, Apr 15, 2013 at 10:04:24PM +0200, Lluís Vilanova wrote:
> Tried using a list of strings as an argument to a command, but the generated
> code references the 'strList' type, which does not exist.
> 
> Is a specialized version for "['str']" missing, or should I define my own type
> with a single field of 'str' type?

akong just hit this too.

I think it's a question for aliguori, luiz, or mdroth.

Stefan

Re: [Qemu-devel] [qapi] Cannot use list of strings

[Paolo Bonzini]

Il 16/04/2013 10:49, Stefan Hajnoczi ha scritto:
>> > Tried using a list of strings as an argument to a command, but the generated
>> > code references the 'strList' type, which does not exist.
>> > 
>> > Is a specialized version for "['str']" missing, or should I define my own type
>> > with a single field of 'str' type?
> akong just hit this too.
> 
> I think it's a question for aliguori, luiz, or mdroth.

Laszlo defined and used String for this purpose:

##
# @String
#
# A fat type wrapping 'str', to be embedded in lists.
#
# Since 1.2
##
{ 'type': 'String',
  'data': {
    'str': 'str' } }


Paolo

Re: [Qemu-devel] [qapi] Cannot use list of strings

[Amos Kong]

On Tue, Apr 16, 2013 at 10:54:56AM +0200, Paolo Bonzini wrote:
> Il 16/04/2013 10:49, Stefan Hajnoczi ha scritto:
> >> > Tried using a list of strings as an argument to a command, but the generated
> >> > code references the 'strList' type, which does not exist.
> >> > 
> >> > Is a specialized version for "['str']" missing, or should I define my own type
> >> > with a single field of 'str' type?
> > akong just hit this too.

That thread: http://marc.info/?l=qemu-devel&m=136572276530702&w=3

> > I think it's a question for aliguori, luiz, or mdroth.
> 
> Laszlo defined and used String for this purpose:

Eric said String list contains additional JSON structure.
At least, it works.

===================== using StringList
    '*unicast':        ['String'],
    '*multicast':      ['String']

{
    "return": [
        {
            "name": "virtio-net-pci.0", 
            "multicast": [
                {
                    "str": "01:80:c2:00:00:21"
                }, 
                {
                    "str": "00:00:00:00:00:00"
                }
            ]
        }, 
        ....
    ]
}
======================== using strList
    '*unicast':        ['str'],
    '*multicast':      ['str']

Eric, is it expected format?

{
    "return": [
        {
            "name": "virtio-net-pci.0", 
            "multicast": [
                    "str": "01:80:c2:00:00:21",
                    "str": "00:00:00:00:00:00"
            ]
        },
        ....
    ]
}
========================

I changed qapi scripts to define struct, struct list,
visit/free functions for 'str'. But it conflicts with
existed str functions. I hadn't find a solution.

Do we need to defind strList & visit functions in qapi-core files?
not generated by scripts.


diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9e19920..5167d85 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -282,6 +282,10 @@ fdecl.write(mcgen('''
 exprs = parse_schema(sys.stdin)
 exprs = filter(lambda expr: not expr.has_key('gen'), exprs)

+ret = "\n"
+ret += generate_fwd_struct('str', {'str': 'str'})
+fdecl.write(ret)
+
 for expr in exprs:
     ret = "\n"
     if expr.has_key('type'):
@@ -319,6 +323,15 @@ for expr in exprs:
         continue
     fdecl.write(ret)

+ret = "\n"
+ret += generate_struct('str', "", {'str': 'str'}) + "\n"
+ret += generate_type_cleanup_decl('str' + "List")
+fdef.write(generate_type_cleanup('str' + "List") + "\n")
+ret += generate_type_cleanup_decl('str')
+fdef.write(generate_type_cleanup('str') + "\n")
+
+fdecl.write(ret)
+
 fdecl.write('''
 #endif
 ''')
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index a276540..089dda7 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -354,6 +354,13 @@ for expr in exprs:
         ret += generate_enum_declaration(expr['enum'], expr['data'])
         fdecl.write(ret)

+ret = generate_visit_struct('str', {'str': 'str'})
+ret += generate_visit_list('str', {'str': 'str'})
+fdef.write(ret)
+
+ret = generate_declaration('str', {'str': 'str'})
+fdecl.write(ret)
+
 fdecl.write('''
 #endif
 ''')

 
> ##
> # @String
> #
> # A fat type wrapping 'str', to be embedded in lists.
> #
> # Since 1.2
> ##
> { 'type': 'String',
>   'data': {
>     'str': 'str' } }

-- 
			Amos.

Re: [Qemu-devel] [qapi] Cannot use list of strings

[Eric Blake]

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

On 04/16/2013 04:13 AM, Amos Kong wrote:
> Eric said String list contains additional JSON structure.
> At least, it works.

Using the fat 'String' wrapper works, but requires more effort to decode.

> 
> ===================== using StringList
>     '*unicast':        ['String'],
>     '*multicast':      ['String']
> 
> {
>     "return": [
>         {
>             "name": "virtio-net-pci.0", 
>             "multicast": [
>                 {
>                     "str": "01:80:c2:00:00:21"
>                 }, 
>                 {
>                     "str": "00:00:00:00:00:00"
>                 }
>             ]

Libvirt can live with this, even though it is more work, if you can't
figure out how to make 'str' work.  I don't know enough about the JSON
generator to quickly dive in and figure out why you can't make an array
of native types, though.

>         }, 
>         ....
>     ]
> }
> ======================== using strList
>     '*unicast':        ['str'],
>     '*multicast':      ['str']
> 
> Eric, is it expected format?
> 
> {
>     "return": [
>         {
>             "name": "virtio-net-pci.0", 
>             "multicast": [
>                     "str": "01:80:c2:00:00:21",
>                     "str": "00:00:00:00:00:00"
>             ]

No.  This is not valid JSON.  ':' is only allowed inside {}.  The point
of an array is to either have an array of objects (as in [
{'name':value}, {'name':value} ]) or an array of native types (as in [
value, value ]).  The ideal solution is for the output QMP to look like:

    "multicast": [
        "01:80:c2:00:00:21",
        "00:00:00:00:00:00"
    ]

which seems like it should work as '*multicast':['str'], if you can
figure out how to make the generator play along.

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


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

Re: [Qemu-devel] [qapi] Cannot use list of strings

[Lluís Vilanova]

Paolo Bonzini writes:

> Il 16/04/2013 10:49, Stefan Hajnoczi ha scritto:
>>> > Tried using a list of strings as an argument to a command, but the generated
>>> > code references the 'strList' type, which does not exist.
>>> > 
>>> > Is a specialized version for "['str']" missing, or should I define my own type
>>> > with a single field of 'str' type?
>> akong just hit this too.
>> 
>> I think it's a question for aliguori, luiz, or mdroth.

> Laszlo defined and used String for this purpose:

Ok, thanks for the info. I suppose I'll go for 'StringList' in the meantime.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [qapi] Cannot use list of strings

[mdroth]

On Tue, Apr 16, 2013 at 10:49:19AM +0200, Stefan Hajnoczi wrote:
> On Mon, Apr 15, 2013 at 10:04:24PM +0200, Lluís Vilanova wrote:
> > Tried using a list of strings as an argument to a command, but the generated
> > code references the 'strList' type, which does not exist.
> > 
> > Is a specialized version for "['str']" missing, or should I define my own type
> > with a single field of 'str' type?

I would say just give that a shot. Stick:

typedef struct strList
{
    char *value;
    struct strList *next;
} strList;

in include/qapi/visitor.h and see what happens.

It's not immediately obvious to me why that wouldn't work, except maybe
that sometimes the code generators will special case on CamelCase types,
but that shouldn't be too hard to work around if it's an issue.

If that works, the same approach can probably be taken for all "native" qapi
types: str, bool, [u]int[(8|16|32|64)]

> 
> akong just hit this too.
> 
> I think it's a question for aliguori, luiz, or mdroth.
> 
> Stefan
> 

Re: [Qemu-devel] [qapi] Cannot use list of strings

[Amos Kong]

On Tue, Apr 16, 2013 at 09:50:17AM -0500, mdroth wrote:
> On Tue, Apr 16, 2013 at 10:49:19AM +0200, Stefan Hajnoczi wrote:
> > On Mon, Apr 15, 2013 at 10:04:24PM +0200, Lluís Vilanova wrote:
> > > Tried using a list of strings as an argument to a command, but the generated
> > > code references the 'strList' type, which does not exist.
> > > 
> > > Is a specialized version for "['str']" missing, or should I define my own type
> > > with a single field of 'str' type?
> 
> I would say just give that a shot. Stick:
> 
> typedef struct strList
> {
>     char *value;
>     struct strList *next;
> } strList;
> 
> in include/qapi/visitor.h and see what happens.

Hi Michael,

| In file included from /home/devel/qemu/include/qapi/error.h:16,
|                  from /home/devel/qemu/include/qapi/visitor.h:16,
|                  from /home/devel/qemu/include/qapi/dealloc-visitor.h:17,
|                  from qapi-types.c:17:
| ./qapi-types.h:2158: error: expected specifier-qualifier-list before ‘strList’
| make: *** [qapi-types.o] Error 1


("->" means "is include to")
qapi-types.h -> include/qapi/error.h -> include/qapi/visitor.h

'strList' is used in qapi-types.h, we should not declare strList in
include/qapi/visitor.h. I changed scripts/qapi-types.py to declare
strList in qapi-types.h, but got a redefinition error.

| In file included from qga/qapi-generated/qga-qmp-commands.h:19,
|                  from qga/commands.c:15:
| qga/qapi-generated/qga-qapi-types.h:23: error: redefinition of ‘struct strList’
| qga/qapi-generated/qga-qapi-types.h:26: error: conflicting types for ‘strList’
| ./qapi-types.h:26: note: previous declaration of ‘strList’ was here
| make: *** [qga/commands.o] Error 1


I used an _ugly_ "#ifndef" to limit it only be defined in qapi-types.h.
Michael, others  do you have some suggestion?


Except strList, I also added a function: visit_type_strList(). Then it works.
New output:

{'execute': 'query-mac-table'}
{
    "return": [
        {
            "name": "virtio-net-pci.0", 
            "uni_overflow": false, 
            "nobcast": false, 
            "promisc": false, 
            "multicast": [
                "01:00:5e:00:00:01", 
                "33:33:00:00:00:01", 
                "33:33:ff:12:34:57"
            ], 
            "nouni": false, 
            "nomulti": false, 
            "allmulti": false, 
            "multi_overflow": false, 
            "alluni": false
        }, 
        {
            "name": "virtio-net-pci.1", 
             ....
             ....
        }
    ]
}

# cat draft.patch
diff --git a/hmp.c b/hmp.c
index 8820224..1cf524e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -656,7 +656,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
 void hmp_info_mac_table(Monitor *mon, const QDict *qdict)
 {
     MacTableInfoList *table_list, *table_entry;
-    StringList *str_entry;
+    strList *str_entry;
 
     table_list = qmp_query_mac_table(NULL);
     table_entry = table_list;
@@ -698,7 +698,7 @@ void hmp_info_mac_table(Monitor *mon, const QDict *qdict)
             str_entry = table_entry->value->unicast;
             monitor_printf(mon, " \\ unicast:\n");
             while (str_entry) {
-                monitor_printf(mon, "    %s\n", str_entry->value->str);
+                monitor_printf(mon, "    %s\n", str_entry->value);
                 str_entry = str_entry->next;
             }
         }
@@ -707,7 +707,7 @@ void hmp_info_mac_table(Monitor *mon, const QDict *qdict)
             str_entry = table_entry->value->multicast;
             monitor_printf(mon, " \\ multicast:\n");
             while (str_entry) {
-                monitor_printf(mon, "    %s\n", str_entry->value->str);
+                monitor_printf(mon, "    %s\n", str_entry->value);
                 str_entry = str_entry->next;
             }
         }
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 8b15759..b72f163 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -197,8 +197,8 @@ static MacTableInfo *virtio_net_query_mactable(NetClientState *nc)
 {
     VirtIONet *n = qemu_get_nic_opaque(nc);
     MacTableInfo *info;
-    StringList *str_list = NULL;
-    StringList *entry;
+    strList *str_list = NULL;
+    strList *entry;
     char str[12];
     int i;
 
@@ -234,7 +234,7 @@ static MacTableInfo *virtio_net_query_mactable(NetClientState *nc)
                 n->mac_table.macs[i * ETH_ALEN + 4],
                 n->mac_table.macs[i * ETH_ALEN + 5]);
         entry->value = g_malloc0(sizeof(String *));
-        entry->value->str = g_strdup(str);
+        entry->value = g_strdup(str);
         entry->next = str_list;
         str_list = entry;
     }
@@ -253,7 +253,7 @@ static MacTableInfo *virtio_net_query_mactable(NetClientState *nc)
                 n->mac_table.macs[i * ETH_ALEN + 4],
                 n->mac_table.macs[i * ETH_ALEN + 5]);
         entry->value = g_malloc0(sizeof(String *));
-        entry->value->str = g_strdup(str);
+        entry->value = g_strdup(str);
         entry->next = str_list;
         str_list = entry;
     }
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 1fef18c..2e36edb 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -50,6 +50,7 @@ void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
 void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
+void visit_type_strList(Visitor *m, strList ** obj, const char *name, Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index febf62a..5c8e77e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3537,8 +3537,8 @@
     '*nobcast':        'bool',
     '*multi_overflow': 'bool',
     '*uni_overflow':   'bool',
-    '*unicast':        ['String'],
-    '*multicast':      ['String'] }}
+    '*unicast':        ['str'],
+    '*multicast':      ['str'] }}
 
 ##
 # @query-mac-table:
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 401ee6e..aefe227 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -257,6 +257,28 @@ void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp)
     }
 }
 
+void visit_type_strList(Visitor *m, strList ** obj, const char *name, Error **errp)
+{
+    GenericList *i, **prev = (GenericList **)obj;
+    Error *err = NULL;
+
+    if (!error_is_set(errp)) {
+        visit_start_list(m, name, &err);
+        if (!err) {
+            for (; (i = visit_next_list(m, prev, &err)) != NULL; prev = &i) {
+                strList *native_i = (strList *)i;
+                visit_type_str(m, &native_i->value, NULL, &err);
+            }
+            error_propagate(errp, err);
+            err = NULL;
+
+            /* Always call end_list if start_list succeeded.  */
+            visit_end_list(m, &err);
+        }
+        error_propagate(errp, err);
+    }
+}
+
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
 {
     if (!error_is_set(errp)) {
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9e19920..f2ca373 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -276,6 +276,14 @@ fdecl.write(mcgen('''
 #include <stdbool.h>
 #include <stdint.h>
 
+#ifndef QGA_QAPI_TYPES_H
+typedef struct strList
+{
+  char *value;
+  struct strList *next;
+} strList;
+#endif
+
 ''',
                   guard=guardname(h_file)))
 

[Qemu-devel] [RFC PATCH] qapi: introduce strList and visit_type_strList() (was Re: [qapi] Cannot use list of strings)

[Amos Kong]

On Wed, Apr 17, 2013 at 04:33:03PM +0800, Amos Kong wrote:
> On Tue, Apr 16, 2013 at 09:50:17AM -0500, mdroth wrote:
> > On Tue, Apr 16, 2013 at 10:49:19AM +0200, Stefan Hajnoczi wrote:
> > > On Mon, Apr 15, 2013 at 10:04:24PM +0200, Lluís Vilanova wrote:
> > > > Tried using a list of strings as an argument to a command, but the generated
> > > > code references the 'strList' type, which does not exist.
> > > > 
> > > > Is a specialized version for "['str']" missing, or should I define my own type
> > > > with a single field of 'str' type?
> > 
> > I would say just give that a shot. Stick:
> > 
> > typedef struct strList
> > {
> >     char *value;
> >     struct strList *next;
> > } strList;
> > 
> > in include/qapi/visitor.h and see what happens.
> 
> Hi Michael,
> 
> | In file included from /home/devel/qemu/include/qapi/error.h:16,
> |                  from /home/devel/qemu/include/qapi/visitor.h:16,
> |                  from /home/devel/qemu/include/qapi/dealloc-visitor.h:17,
> |                  from qapi-types.c:17:
> | ./qapi-types.h:2158: error: expected specifier-qualifier-list before ‘strList’
> | make: *** [qapi-types.o] Error 1
> 
> 
> ("->" means "is include to")
> qapi-types.h -> include/qapi/error.h -> include/qapi/visitor.h
> 
> 'strList' is used in qapi-types.h, we should not declare strList in
> include/qapi/visitor.h. I changed scripts/qapi-types.py to declare
> strList in qapi-types.h, but got a redefinition error.
> 
> | In file included from qga/qapi-generated/qga-qmp-commands.h:19,
> |                  from qga/commands.c:15:
> | qga/qapi-generated/qga-qapi-types.h:23: error: redefinition of ‘struct strList’
> | qga/qapi-generated/qga-qapi-types.h:26: error: conflicting types for ‘strList’
> | ./qapi-types.h:26: note: previous declaration of ‘strList’ was here
> | make: *** [qga/commands.o] Error 1
> 
> 
> I used an _ugly_ "#ifndef" to limit it only be defined in qapi-types.h.
> Michael, others  do you have some suggestion?
 

>From ecbd69a8ad4b5e600ac56b1cda4c4c04e0175742 Mon Sep 17 00:00:00 2001
From: Amos Kong <akong@redhat.com>
Date: Thu, 18 Apr 2013 10:56:24 +0800
Subject: [RFC PATCH] qapi: introduce strList and visit_type_strList()

Currently we can only use ['String'] to add string to a list,
it contains some additional JSON structure.
    "multicast": [
        {
            "str": "01:80:c2:00:00:21"
        },
        {
            "str": "00:00:00:00:00:00"
        }
    ]

This patch introdued strList, we can use ['str']

    "multicast": [
        "01:00:5e:00:00:01",
        "33:33:ff:12:34:57"
    ]

Signed-off-by: Amos Kong <akong@redhat.com>
---
I used an _ugly_ "#ifndef" to limit it only be defined in qapi-types.h.
Michael, others  do you have some suggestion?
---
 include/qapi/visitor.h |    1 +
 qapi/qapi-visit-core.c |   22 ++++++++++++++++++++++
 scripts/qapi-types.py  |    8 ++++++++
 3 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 1fef18c..2e36edb 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -50,6 +50,7 @@ void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
 void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
+void visit_type_strList(Visitor *m, strList ** obj, const char *name, Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
 
 #endif
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 401ee6e..aefe227 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -257,6 +257,28 @@ void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp)
     }
 }
 
+void visit_type_strList(Visitor *m, strList ** obj, const char *name, Error **errp)
+{
+    GenericList *i, **prev = (GenericList **)obj;
+    Error *err = NULL;
+
+    if (!error_is_set(errp)) {
+        visit_start_list(m, name, &err);
+        if (!err) {
+            for (; (i = visit_next_list(m, prev, &err)) != NULL; prev = &i) {
+                strList *native_i = (strList *)i;
+                visit_type_str(m, &native_i->value, NULL, &err);
+            }
+            error_propagate(errp, err);
+            err = NULL;
+
+            /* Always call end_list if start_list succeeded.  */
+            visit_end_list(m, &err);
+        }
+        error_propagate(errp, err);
+    }
+}
+
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
 {
     if (!error_is_set(errp)) {
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9e19920..f2ca373 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -276,6 +276,14 @@ fdecl.write(mcgen('''
 #include <stdbool.h>
 #include <stdint.h>
 
+#ifndef QGA_QAPI_TYPES_H
+typedef struct strList
+{
+  char *value;
+  struct strList *next;
+} strList;
+#endif
+
 ''',
                   guard=guardname(h_file)))
 
-- 
1.7.1

[Qemu-devel] [PATCH v2] qapi: add struct strList and visit_type_strList()

[Amos Kong]

Currently we can only use ['String'] to add string to a list,
it contains some additional JSON structure.
    "multicast": [
        {
            "str": "01:80:c2:00:00:21"
        },
        {
            "str": "00:00:00:00:00:00"
        }
    ]

This patch adds struct strList and visit function, we can use ['str'] in
schema file.

    "multicast": [
        "01:00:5e:00:00:01",
        "33:33:ff:12:34:57"
    ]

V2: remove ugly #ifndef, add forward declaration in qapi-types.h,
    and define the struct in include/qapi/visitor.h (Paolo)

Signed-off-by: Amos Kong <akong@redhat.com>
---
 include/qapi/visitor.h |    7 +++++++
 qapi/qapi-visit-core.c |   23 +++++++++++++++++++++++
 scripts/qapi-types.py  |    1 +
 scripts/qapi.py        |    4 ++--
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 1fef18c..743ff92 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -22,6 +22,11 @@ typedef struct GenericList
     struct GenericList *next;
 } GenericList;
 
+typedef struct strList {
+    char *value;
+    struct strList *next;
+} strList;
+
 typedef struct Visitor Visitor;
 
 void visit_start_handle(Visitor *v, void **obj, const char *kind,
@@ -50,6 +55,8 @@ void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
 void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
+void visit_type_strList(Visitor *m, strList ** obj, const char *name,
+                        Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
 
 #endif
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 401ee6e..dc54cc8 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -257,6 +257,29 @@ void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp)
     }
 }
 
+void visit_type_strList(Visitor *m, strList ** obj, const char *name,
+                        Error **errp)
+{
+    GenericList *i, **prev = (GenericList **)obj;
+    Error *err = NULL;
+
+    if (!error_is_set(errp)) {
+        visit_start_list(m, name, &err);
+        if (!err) {
+            for (; (i = visit_next_list(m, prev, &err)) != NULL; prev = &i) {
+                strList *native_i = (strList *)i;
+                visit_type_str(m, &native_i->value, NULL, &err);
+            }
+            error_propagate(errp, err);
+            err = NULL;
+
+            /* Always call end_list if start_list succeeded.  */
+            visit_end_list(m, &err);
+        }
+        error_propagate(errp, err);
+    }
+}
+
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
 {
     if (!error_is_set(errp)) {
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9e19920..e449a14 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -276,6 +276,7 @@ fdecl.write(mcgen('''
 #include <stdbool.h>
 #include <stdint.h>
 
+struct strList;
 ''',
                   guard=guardname(h_file)))
 
diff --git a/scripts/qapi.py b/scripts/qapi.py
index afc5f32..14f9f4d 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -166,11 +166,11 @@ def c_fun(name, protect=True):
     return c_var(name, protect).replace('.', '_')
 
 def c_list_type(name):
-    return '%sList' % name
+    return 'struct %sList' % name
 
 def type_name(name):
     if type(name) == list:
-        return c_list_type(name[0])
+        return '%sList' % name[0]
     return name
 
 enum_types = []
-- 
1.7.1

Re: [Qemu-devel] [PATCH v2] qapi: add struct strList and visit_type_strList()

[Paolo Bonzini]

Il 24/04/2013 18:08, Amos Kong ha scritto:
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index 9e19920..e449a14 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -276,6 +276,7 @@ fdecl.write(mcgen('''
>  #include <stdbool.h>
>  #include <stdint.h>
>  
> +struct strList;
>  ''',
>                    guard=guardname(h_file)))
>  
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index afc5f32..14f9f4d 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -166,11 +166,11 @@ def c_fun(name, protect=True):
>      return c_var(name, protect).replace('.', '_')
>  
>  def c_list_type(name):
> -    return '%sList' % name
> +    return 'struct %sList' % name
>  
>  def type_name(name):
>      if type(name) == list:
> -        return c_list_type(name[0])
> +        return '%sList' % name[0]
>      return name

This second change is needed to generate the correct function names,
right?  If so,

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [Qemu-devel] [PATCH v2] qapi: add struct strList and visit_type_strList()

[Amos Kong]

On Wed, Apr 24, 2013 at 06:11:55PM +0200, Paolo Bonzini wrote:
> Il 24/04/2013 18:08, Amos Kong ha scritto:
> > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> > index 9e19920..e449a14 100644
> > --- a/scripts/qapi-types.py
> > +++ b/scripts/qapi-types.py
> > @@ -276,6 +276,7 @@ fdecl.write(mcgen('''
> >  #include <stdbool.h>
> >  #include <stdint.h>
> >  
> > +struct strList;
> >  ''',
> >                    guard=guardname(h_file)))
> >  
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index afc5f32..14f9f4d 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -166,11 +166,11 @@ def c_fun(name, protect=True):
> >      return c_var(name, protect).replace('.', '_')
> >  
> >  def c_list_type(name):
> > -    return '%sList' % name
> > +    return 'struct %sList' % name
> >  
> >  def type_name(name):
> >      if type(name) == list:
> > -        return c_list_type(name[0])
> > +        return '%sList' % name[0]
> >      return name
> 
> This second change is needed to generate the correct function names,
> right?  If so,
 
Yes, otherwise, the visit function name will be "visit_type_struct nameList".

> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Paolo

Thanks 

-- 
			Amos.

Re: [Qemu-devel] [PATCH v2] qapi: add struct strList and visit_type_strList()

[mdroth]

On Thu, Apr 25, 2013 at 12:08:05AM +0800, Amos Kong wrote:
> Currently we can only use ['String'] to add string to a list,
> it contains some additional JSON structure.
>     "multicast": [
>         {
>             "str": "01:80:c2:00:00:21"
>         },
>         {
>             "str": "00:00:00:00:00:00"
>         }
>     ]
> 
> This patch adds struct strList and visit function, we can use ['str'] in
> schema file.
> 
>     "multicast": [
>         "01:00:5e:00:00:01",
>         "33:33:ff:12:34:57"
>     ]
> 
> V2: remove ugly #ifndef, add forward declaration in qapi-types.h,
>     and define the struct in include/qapi/visitor.h (Paolo)
> 
> Signed-off-by: Amos Kong <akong@redhat.com>

Sorry for the delay in getting back to you, I started to respond last
week and ended up hacking something up that take a different approach.

Namely: we don't hardcode visit_type_strList(), but instead teach the
qapi code generators about native types we currently allow in schemas
('str', 'int', 'number', 'bool') and have them handle code generation
for these like they would for any user-defined type.

This approach works too, but I think this solution is more complete.
I'm still working out some bits with the unit tests but I've pushed my
WIP here for reference:

https://github.com/mdroth/qemu/commits/qapi-native-lists

Please give that a shot.

> ---
>  include/qapi/visitor.h |    7 +++++++
>  qapi/qapi-visit-core.c |   23 +++++++++++++++++++++++
>  scripts/qapi-types.py  |    1 +
>  scripts/qapi.py        |    4 ++--
>  4 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index 1fef18c..743ff92 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -22,6 +22,11 @@ typedef struct GenericList
>      struct GenericList *next;
>  } GenericList;
> 
> +typedef struct strList {
> +    char *value;
> +    struct strList *next;
> +} strList;
> +
>  typedef struct Visitor Visitor;
> 
>  void visit_start_handle(Visitor *v, void **obj, const char *kind,
> @@ -50,6 +55,8 @@ void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
>  void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>  void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
>  void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
> +void visit_type_strList(Visitor *m, strList ** obj, const char *name,
> +                        Error **errp);
>  void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
> 
>  #endif
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 401ee6e..dc54cc8 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -257,6 +257,29 @@ void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp)
>      }
>  }
> 
> +void visit_type_strList(Visitor *m, strList ** obj, const char *name,
> +                        Error **errp)
> +{
> +    GenericList *i, **prev = (GenericList **)obj;
> +    Error *err = NULL;
> +
> +    if (!error_is_set(errp)) {
> +        visit_start_list(m, name, &err);
> +        if (!err) {
> +            for (; (i = visit_next_list(m, prev, &err)) != NULL; prev = &i) {
> +                strList *native_i = (strList *)i;
> +                visit_type_str(m, &native_i->value, NULL, &err);
> +            }
> +            error_propagate(errp, err);
> +            err = NULL;
> +
> +            /* Always call end_list if start_list succeeded.  */
> +            visit_end_list(m, &err);
> +        }
> +        error_propagate(errp, err);
> +    }
> +}
> +
>  void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
>  {
>      if (!error_is_set(errp)) {
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index 9e19920..e449a14 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -276,6 +276,7 @@ fdecl.write(mcgen('''
>  #include <stdbool.h>
>  #include <stdint.h>
> 
> +struct strList;
>  ''',
>                    guard=guardname(h_file)))
> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index afc5f32..14f9f4d 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -166,11 +166,11 @@ def c_fun(name, protect=True):
>      return c_var(name, protect).replace('.', '_')
> 
>  def c_list_type(name):
> -    return '%sList' % name
> +    return 'struct %sList' % name
> 
>  def type_name(name):
>      if type(name) == list:
> -        return c_list_type(name[0])
> +        return '%sList' % name[0]
>      return name
> 
>  enum_types = []
> -- 
> 1.7.1
> 

Re: [Qemu-devel] [PATCH v2] qapi: add struct strList and visit_type_strList()

[Amos Kong]

Hi Michael,

On Wed, Apr 24, 2013 at 11:46:25AM -0500, mdroth wrote:
> On Thu, Apr 25, 2013 at 12:08:05AM +0800, Amos Kong wrote:
> > Currently we can only use ['String'] to add string to a list,
> > it contains some additional JSON structure.
> >     "multicast": [
> >         {
> >             "str": "01:80:c2:00:00:21"
> >         },
> >         {
> >             "str": "00:00:00:00:00:00"
> >         }
> >     ]
> > 
> > This patch adds struct strList and visit function, we can use ['str'] in
> > schema file.
> > 
> >     "multicast": [
> >         "01:00:5e:00:00:01",
> >         "33:33:ff:12:34:57"
> >     ]
> > 
> > V2: remove ugly #ifndef, add forward declaration in qapi-types.h,
> >     and define the struct in include/qapi/visitor.h (Paolo)
> > 
> > Signed-off-by: Amos Kong <akong@redhat.com>
> 
> Sorry for the delay in getting back to you, I started to respond last
> week and ended up hacking something up that take a different approach.
> 
> Namely: we don't hardcode visit_type_strList(), but instead teach the
> qapi code generators about native types we currently allow in schemas
> ('str', 'int', 'number', 'bool') and have them handle code generation
> for these like they would for any user-defined type.

It's better.
 
> This approach works too, but I think this solution is more complete.
> I'm still working out some bits with the unit tests but I've pushed my
> WIP here for reference:
> 
> https://github.com/mdroth/qemu/commits/qapi-native-lists
> 
> Please give that a shot.

Your patchset looks good to me, I only added some trivial comments on
github.

-- 
			Amos.