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

[Amos Kong <akong@redhat.com>]

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