All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches
@ 2015-06-23 13:32 Kővágó, Zoltán
  2015-06-23 13:32 ` [Qemu-devel] [PATCH 1/5] qapi: support implicit structs in OptsVisitor Kővágó, Zoltán
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Kővágó, Zoltán @ 2015-06-23 13:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Stefan Hajnoczi, Eduardo Habkost

I've cherry-picked the qapi related parts from my previous -audiodev
patch series, we can hopefully concentrate on one thing at a time.  The
most important changes in this patch series are the flattening of the
Netdev structures.  This way we can add proper nested structure support
into OptsVisitor, without requiring backward-compatibility hacks.

These patches are essentially same as the patches in v3 -audiodev
option, except that I updated it to master (and fixed the conflicts due
to error handling related changes).

Please review.

Kővágó, Zoltán (5):
  qapi: support implicit structs in OptsVisitor
  qapi: convert NumaOptions into a flat union
  qapi: change Netdev and NetLegacy into a flat union
  qapi: support nested structs in OptsVisitor
  opts: produce valid command line in qemu_opts_print

 block.c                                 |   2 +-
 hw/arm/musicpal.c                       |   2 +-
 hw/core/qdev-properties-system.c        |   2 +-
 hw/net/allwinner_emac.c                 |   2 +-
 hw/net/cadence_gem.c                    |   2 +-
 hw/net/dp8393x.c                        |   2 +-
 hw/net/e1000.c                          |   2 +-
 hw/net/eepro100.c                       |   2 +-
 hw/net/etraxfs_eth.c                    |   2 +-
 hw/net/fsl_etsec/etsec.c                |   2 +-
 hw/net/lan9118.c                        |   2 +-
 hw/net/lance.c                          |   2 +-
 hw/net/mcf_fec.c                        |   2 +-
 hw/net/milkymist-minimac2.c             |   2 +-
 hw/net/mipsnet.c                        |   2 +-
 hw/net/ne2000-isa.c                     |   2 +-
 hw/net/ne2000.c                         |   2 +-
 hw/net/opencores_eth.c                  |   2 +-
 hw/net/pcnet-pci.c                      |   2 +-
 hw/net/rocker/rocker_fp.c               |   2 +-
 hw/net/rtl8139.c                        |   2 +-
 hw/net/smc91c111.c                      |   2 +-
 hw/net/spapr_llan.c                     |   2 +-
 hw/net/stellaris_enet.c                 |   2 +-
 hw/net/vhost_net.c                      |  18 ++--
 hw/net/virtio-net.c                     |   6 +-
 hw/net/vmxnet3.c                        |   2 +-
 hw/net/xen_nic.c                        |   2 +-
 hw/net/xgmac.c                          |   2 +-
 hw/net/xilinx_axienet.c                 |   2 +-
 hw/net/xilinx_ethlite.c                 |   2 +-
 hw/usb/dev-network.c                    |   2 +-
 include/net/net.h                       |   4 +-
 monitor.c                               |  14 +--
 net/clients.h                           |  20 ++---
 net/dump.c                              |   9 +-
 net/hub.c                               |  24 +++--
 net/l2tpv3.c                            |   9 +-
 net/net.c                               | 110 +++++++++++------------
 net/netmap.c                            |   6 +-
 net/slirp.c                             |   9 +-
 net/socket.c                            |  11 +--
 net/tap-win32.c                         |   9 +-
 net/tap.c                               |  29 +++---
 net/vde.c                               |   9 +-
 net/vhost-user.c                        |  15 ++--
 numa.c                                  |   2 +-
 qapi-schema.json                        | 150 +++++++++++++++++++++++---------
 qapi/opts-visitor.c                     | 129 ++++++++++++++++++++++-----
 tests/qapi-schema/qapi-schema-test.json |   9 +-
 tests/test-opts-visitor.c               |  34 ++++++++
 util/qemu-option.c                      |  47 +++++++++-
 52 files changed, 471 insertions(+), 262 deletions(-)

-- 
2.4.3

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

* [Qemu-devel] [PATCH 1/5] qapi: support implicit structs in OptsVisitor
  2015-06-23 13:32 [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches Kővágó, Zoltán
@ 2015-06-23 13:32 ` Kővágó, Zoltán
  2015-06-23 13:32 ` [Qemu-devel] [PATCH 2/5] qapi: convert NumaOptions into a flat union Kővágó, Zoltán
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Kővágó, Zoltán @ 2015-06-23 13:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Stefan Hajnoczi, Michael Roth

They are required for flat unions (you still have to allocate the
structs).

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
 qapi/opts-visitor.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 7ae33b3..aa68814 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -149,6 +149,12 @@ opts_start_struct(Visitor *v, void **obj, const char *kind,
     }
 }
 
+static void
+opts_start_implicit_struct(Visitor *v, void **obj, size_t size, Error **errp)
+{
+    opts_start_struct(v, obj, NULL, NULL, size, errp);
+}
+
 
 static gboolean
 ghr_true(gpointer ign_key, gpointer ign_value, gpointer ign_user_data)
@@ -185,6 +191,12 @@ opts_end_struct(Visitor *v, Error **errp)
     ov->fake_id_opt = NULL;
 }
 
+static void
+opts_end_implicit_struct(Visitor *v, Error **errp)
+{
+    opts_end_struct(v, errp);
+}
+
 
 static GQueue *
 lookup_distinct(const OptsVisitor *ov, const char *name, Error **errp)
@@ -508,6 +520,9 @@ opts_visitor_new(const QemuOpts *opts)
     ov->visitor.start_struct = &opts_start_struct;
     ov->visitor.end_struct   = &opts_end_struct;
 
+    ov->visitor.start_implicit_struct = &opts_start_implicit_struct;
+    ov->visitor.end_implicit_struct = &opts_end_implicit_struct;
+
     ov->visitor.start_list = &opts_start_list;
     ov->visitor.next_list  = &opts_next_list;
     ov->visitor.end_list   = &opts_end_list;
-- 
2.4.3

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

* [Qemu-devel] [PATCH 2/5] qapi: convert NumaOptions into a flat union
  2015-06-23 13:32 [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches Kővágó, Zoltán
  2015-06-23 13:32 ` [Qemu-devel] [PATCH 1/5] qapi: support implicit structs in OptsVisitor Kővágó, Zoltán
@ 2015-06-23 13:32 ` Kővágó, Zoltán
  2015-07-02 17:25   ` Markus Armbruster
  2015-06-23 13:32 ` [Qemu-devel] [PATCH 3/5] qapi: change Netdev and NetLegacy " Kővágó, Zoltán
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Kővágó, Zoltán @ 2015-06-23 13:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Stefan Hajnoczi, Eduardo Habkost

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
 numa.c           |  2 +-
 qapi-schema.json | 47 ++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/numa.c b/numa.c
index 91fc6c1..8b0755d 100644
--- a/numa.c
+++ b/numa.c
@@ -140,7 +140,7 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
     }
 
     switch (object->kind) {
-    case NUMA_OPTIONS_KIND_NODE:
+    case NUMA_DRIVER_NODE:
         numa_node_parse(object->node, opts, &err);
         if (err) {
             goto error;
diff --git a/qapi-schema.json b/qapi-schema.json
index 106008c..7ebf99e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3537,17 +3537,6 @@
   'data': { '*console':'int', 'events': [ 'InputEvent' ] } }
 
 ##
-# @NumaOptions
-#
-# A discriminated record of NUMA options. (for OptsVisitor)
-#
-# Since 2.1
-##
-{ 'union': 'NumaOptions',
-  'data': {
-    'node': 'NumaNodeOptions' }}
-
-##
 # @NumaNodeOptions
 #
 # Create a guest NUMA node. (for OptsVisitor)
@@ -3574,6 +3563,42 @@
    '*memdev': 'str' }}
 
 ##
+# @NumaDriver
+#
+# List of possible numa drivers.
+#
+# Since: 2.4
+##
+{ 'enum': 'NumaDriver',
+  'data': [ 'node' ] }
+
+##
+# @NumaCommonOptions
+#
+# Common set of numa options.
+#
+# @type: the numa driver to use
+#
+# Since: 2.4
+##
+{ 'struct': 'NumaCommonOptions',
+  'data': {
+    'type': 'NumaDriver' } }
+
+##
+# @NumaOptions
+#
+# A discriminated record of NUMA options. (for OptsVisitor)
+#
+# Since 2.1
+##
+{ 'union': 'NumaOptions',
+  'base': 'NumaCommonOptions',
+  'discriminator': 'type',
+  'data': {
+    'node': 'NumaNodeOptions' }}
+
+##
 # @HostMemPolicy
 #
 # Host memory policy types
-- 
2.4.3

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

* [Qemu-devel] [PATCH 3/5] qapi: change Netdev and NetLegacy into a flat union
  2015-06-23 13:32 [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches Kővágó, Zoltán
  2015-06-23 13:32 ` [Qemu-devel] [PATCH 1/5] qapi: support implicit structs in OptsVisitor Kővágó, Zoltán
  2015-06-23 13:32 ` [Qemu-devel] [PATCH 2/5] qapi: convert NumaOptions into a flat union Kővágó, Zoltán
@ 2015-06-23 13:32 ` Kővágó, Zoltán
  2015-07-02 17:17   ` Markus Armbruster
  2015-06-23 13:33 ` [Qemu-devel] [PATCH 4/5] qapi: support nested structs in OptsVisitor Kővágó, Zoltán
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Kővágó, Zoltán @ 2015-06-23 13:32 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Vincenzo Maffione,
	Alexander Graf, Max Filippov, Gerd Hoffmann, Dmitry Fleytman,
	Edgar E. Iglesias, Rob Herring, Markus Armbruster, Scott Feldman,
	Jiri Pirko, Jan Kiszka, Stefan Hajnoczi, Giuseppe Lettieri,
	Luiz Capitulino, Luigi Rizzo, David Gibson, Peter Crosthwaite,
	Michael Walle, open list:sPAPR pseries

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
 hw/arm/musicpal.c                |   2 +-
 hw/core/qdev-properties-system.c |   2 +-
 hw/net/allwinner_emac.c          |   2 +-
 hw/net/cadence_gem.c             |   2 +-
 hw/net/dp8393x.c                 |   2 +-
 hw/net/e1000.c                   |   2 +-
 hw/net/eepro100.c                |   2 +-
 hw/net/etraxfs_eth.c             |   2 +-
 hw/net/fsl_etsec/etsec.c         |   2 +-
 hw/net/lan9118.c                 |   2 +-
 hw/net/lance.c                   |   2 +-
 hw/net/mcf_fec.c                 |   2 +-
 hw/net/milkymist-minimac2.c      |   2 +-
 hw/net/mipsnet.c                 |   2 +-
 hw/net/ne2000-isa.c              |   2 +-
 hw/net/ne2000.c                  |   2 +-
 hw/net/opencores_eth.c           |   2 +-
 hw/net/pcnet-pci.c               |   2 +-
 hw/net/rocker/rocker_fp.c        |   2 +-
 hw/net/rtl8139.c                 |   2 +-
 hw/net/smc91c111.c               |   2 +-
 hw/net/spapr_llan.c              |   2 +-
 hw/net/stellaris_enet.c          |   2 +-
 hw/net/vhost_net.c               |  18 +++----
 hw/net/virtio-net.c              |   6 +--
 hw/net/vmxnet3.c                 |   2 +-
 hw/net/xen_nic.c                 |   2 +-
 hw/net/xgmac.c                   |   2 +-
 hw/net/xilinx_axienet.c          |   2 +-
 hw/net/xilinx_ethlite.c          |   2 +-
 hw/usb/dev-network.c             |   2 +-
 include/net/net.h                |   4 +-
 monitor.c                        |  14 ++---
 net/clients.h                    |  20 +++----
 net/dump.c                       |   9 ++--
 net/hub.c                        |  24 ++++-----
 net/l2tpv3.c                     |   9 ++--
 net/net.c                        | 110 +++++++++++++++++++--------------------
 net/netmap.c                     |   6 +--
 net/slirp.c                      |   9 ++--
 net/socket.c                     |  11 ++--
 net/tap-win32.c                  |   9 ++--
 net/tap.c                        |  29 +++++------
 net/vde.c                        |   9 ++--
 net/vhost-user.c                 |  15 +++---
 qapi-schema.json                 | 103 +++++++++++++++++++++++++-----------
 46 files changed, 239 insertions(+), 224 deletions(-)

diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index a3b1314..72e2f8f 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -379,7 +379,7 @@ static void eth_cleanup(NetClientState *nc)
 }
 
 static NetClientInfo net_mv88w8618_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = eth_can_receive,
     .receive = eth_receive,
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index aa794ca..65ab5a9 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -213,7 +213,7 @@ static void set_netdev(Object *obj, Visitor *v, void *opaque,
     }
 
     queues = qemu_find_net_clients_except(str, peers,
-                                          NET_CLIENT_OPTIONS_KIND_NIC,
+                                          NET_CLIENT_DRIVER_NIC,
                                           MAX_QUEUE_NUM);
     if (queues == 0) {
         err = -ENOENT;
diff --git a/hw/net/allwinner_emac.c b/hw/net/allwinner_emac.c
index 0407dee..4fdf824 100644
--- a/hw/net/allwinner_emac.c
+++ b/hw/net/allwinner_emac.c
@@ -422,7 +422,7 @@ static const MemoryRegionOps aw_emac_mem_ops = {
 };
 
 static NetClientInfo net_aw_emac_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = aw_emac_can_receive,
     .receive = aw_emac_receive,
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 494a346..d74136a 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1175,7 +1175,7 @@ static void gem_set_link(NetClientState *nc)
 }
 
 static NetClientInfo net_gem_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = gem_can_receive,
     .receive = gem_receive,
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index cd889bc..504a4a1 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -807,7 +807,7 @@ static void dp8393x_reset(DeviceState *dev)
 }
 
 static NetClientInfo net_dp83932_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = dp8393x_can_receive,
     .receive = dp8393x_receive,
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index bab8e2a..fc8bf0d 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1512,7 +1512,7 @@ pci_e1000_uninit(PCIDevice *dev)
 }
 
 static NetClientInfo net_e1000_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = e1000_can_receive,
     .receive = e1000_receive,
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index c374c1a..cc4e0ae 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1842,7 +1842,7 @@ static void pci_nic_uninit(PCIDevice *pci_dev)
 }
 
 static NetClientInfo net_eepro100_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = nic_can_receive,
     .receive = nic_receive,
diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c
index 4773dea..5689fd8 100644
--- a/hw/net/etraxfs_eth.c
+++ b/hw/net/etraxfs_eth.c
@@ -582,7 +582,7 @@ static const MemoryRegionOps eth_ops = {
 };
 
 static NetClientInfo net_etraxfs_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = eth_can_receive,
     .receive = eth_receive,
diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
index c57365f..0bdc6f4 100644
--- a/hw/net/fsl_etsec/etsec.c
+++ b/hw/net/fsl_etsec/etsec.c
@@ -368,7 +368,7 @@ static void etsec_set_link_status(NetClientState *nc)
 }
 
 static NetClientInfo net_etsec_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = etsec_can_receive,
     .receive = etsec_receive,
diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index f169c38..08eecf0 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -1310,7 +1310,7 @@ static const MemoryRegionOps lan9118_16bit_mem_ops = {
 };
 
 static NetClientInfo net_lan9118_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = lan9118_can_receive,
     .receive = lan9118_receive,
diff --git a/hw/net/lance.c b/hw/net/lance.c
index 4baa016..08b36ff 100644
--- a/hw/net/lance.c
+++ b/hw/net/lance.c
@@ -92,7 +92,7 @@ static const MemoryRegionOps lance_mem_ops = {
 };
 
 static NetClientInfo net_lance_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = pcnet_can_receive,
     .receive = pcnet_receive,
diff --git a/hw/net/mcf_fec.c b/hw/net/mcf_fec.c
index 0255612..d13431e 100644
--- a/hw/net/mcf_fec.c
+++ b/hw/net/mcf_fec.c
@@ -440,7 +440,7 @@ static const MemoryRegionOps mcf_fec_ops = {
 };
 
 static NetClientInfo net_mcf_fec_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = mcf_fec_can_receive,
     .receive = mcf_fec_receive,
diff --git a/hw/net/milkymist-minimac2.c b/hw/net/milkymist-minimac2.c
index f06afaa..d3f2feb 100644
--- a/hw/net/milkymist-minimac2.c
+++ b/hw/net/milkymist-minimac2.c
@@ -443,7 +443,7 @@ static void milkymist_minimac2_reset(DeviceState *d)
 }
 
 static NetClientInfo net_milkymist_minimac2_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = minimac2_can_rx,
     .receive = minimac2_rx,
diff --git a/hw/net/mipsnet.c b/hw/net/mipsnet.c
index c813e0c..89d11ec 100644
--- a/hw/net/mipsnet.c
+++ b/hw/net/mipsnet.c
@@ -212,7 +212,7 @@ static const VMStateDescription vmstate_mipsnet = {
 };
 
 static NetClientInfo net_mipsnet_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = mipsnet_can_receive,
     .receive = mipsnet_receive,
diff --git a/hw/net/ne2000-isa.c b/hw/net/ne2000-isa.c
index 17e7199..7da26c9 100644
--- a/hw/net/ne2000-isa.c
+++ b/hw/net/ne2000-isa.c
@@ -42,7 +42,7 @@ typedef struct ISANE2000State {
 } ISANE2000State;
 
 static NetClientInfo net_ne2000_isa_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = ne2000_can_receive,
     .receive = ne2000_receive,
diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index 3492db3..09fa0e3 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -703,7 +703,7 @@ void ne2000_setup_io(NE2000State *s, DeviceState *dev, unsigned size)
 }
 
 static NetClientInfo net_ne2000_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = ne2000_can_receive,
     .receive = ne2000_receive,
diff --git a/hw/net/opencores_eth.c b/hw/net/opencores_eth.c
index 3642046..4b6407b 100644
--- a/hw/net/opencores_eth.c
+++ b/hw/net/opencores_eth.c
@@ -473,7 +473,7 @@ static ssize_t open_eth_receive(NetClientState *nc,
 }
 
 static NetClientInfo net_open_eth_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = open_eth_can_receive,
     .receive = open_eth_receive,
diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
index 8305d1b..cb81de9 100644
--- a/hw/net/pcnet-pci.c
+++ b/hw/net/pcnet-pci.c
@@ -271,7 +271,7 @@ static void pci_pcnet_uninit(PCIDevice *dev)
 }
 
 static NetClientInfo net_pci_pcnet_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = pcnet_can_receive,
     .receive = pcnet_receive,
diff --git a/hw/net/rocker/rocker_fp.c b/hw/net/rocker/rocker_fp.c
index d8d934c..36d8e6c 100644
--- a/hw/net/rocker/rocker_fp.c
+++ b/hw/net/rocker/rocker_fp.c
@@ -163,7 +163,7 @@ static void fp_port_set_link_status(NetClientState *nc)
 }
 
 static NetClientInfo fp_port_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = fp_port_can_receive,
     .receive = fp_port_receive,
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index e0db472..391d1fb 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -3445,7 +3445,7 @@ static void rtl8139_set_link_status(NetClientState *nc)
 }
 
 static NetClientInfo net_rtl8139_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = rtl8139_can_receive,
     .receive = rtl8139_receive,
diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
index 74e06e6..4c85eb5 100644
--- a/hw/net/smc91c111.c
+++ b/hw/net/smc91c111.c
@@ -737,7 +737,7 @@ static const MemoryRegionOps smc91c111_mem_ops = {
 };
 
 static NetClientInfo net_smc91c111_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = smc91c111_can_receive,
     .receive = smc91c111_receive,
diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
index 2dd5ec1..45c61f5 100644
--- a/hw/net/spapr_llan.c
+++ b/hw/net/spapr_llan.c
@@ -188,7 +188,7 @@ static ssize_t spapr_vlan_receive(NetClientState *nc, const uint8_t *buf,
 }
 
 static NetClientInfo net_spapr_vlan_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = spapr_vlan_can_receive,
     .receive = spapr_vlan_receive,
diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
index 278a654..9baf1c1 100644
--- a/hw/net/stellaris_enet.c
+++ b/hw/net/stellaris_enet.c
@@ -452,7 +452,7 @@ static void stellaris_enet_reset(stellaris_enet_state *s)
 }
 
 static NetClientInfo net_stellaris_enet_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = stellaris_enet_can_receive,
     .receive = stellaris_enet_receive,
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 9bd360b..7fdb788 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -95,10 +95,10 @@ static const int *vhost_net_get_feature_bits(struct vhost_net *net)
     const int *feature_bits = 0;
 
     switch (net->nc->info->type) {
-    case NET_CLIENT_OPTIONS_KIND_TAP:
+    case NET_CLIENT_DRIVER_TAP:
         feature_bits = kernel_feature_bits;
         break;
-    case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
+    case NET_CLIENT_DRIVER_VHOST_USER:
         feature_bits = user_feature_bits;
         break;
     default:
@@ -125,7 +125,7 @@ void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
 static int vhost_net_get_fd(NetClientState *backend)
 {
     switch (backend->info->type) {
-    case NET_CLIENT_OPTIONS_KIND_TAP:
+    case NET_CLIENT_DRIVER_TAP:
         return tap_get_fd(backend);
     default:
         fprintf(stderr, "vhost-net requires tap backend\n");
@@ -237,7 +237,7 @@ static int vhost_net_start_one(struct vhost_net *net,
         net->nc->info->poll(net->nc, false);
     }
 
-    if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP) {
+    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
         qemu_set_fd_handler(net->backend, NULL, NULL, NULL);
         file.fd = net->backend;
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
@@ -253,7 +253,7 @@ static int vhost_net_start_one(struct vhost_net *net,
     return 0;
 fail:
     file.fd = -1;
-    if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP) {
+    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
         while (file.index-- > 0) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
             int r = vhost_ops->vhost_call(&net->dev, VHOST_NET_SET_BACKEND,
@@ -276,14 +276,14 @@ static void vhost_net_stop_one(struct vhost_net *net,
 {
     struct vhost_vring_file file = { .fd = -1 };
 
-    if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP) {
+    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
             int r = vhost_ops->vhost_call(&net->dev, VHOST_NET_SET_BACKEND,
                                           &file);
             assert(r >= 0);
         }
-    } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
+    } else if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
             int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
@@ -400,10 +400,10 @@ VHostNetState *get_vhost_net(NetClientState *nc)
     }
 
     switch (nc->info->type) {
-    case NET_CLIENT_OPTIONS_KIND_TAP:
+    case NET_CLIENT_DRIVER_TAP:
         vhost_net = tap_get_vhost_net(nc);
         break;
-    case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
+    case NET_CLIENT_DRIVER_VHOST_USER:
         vhost_net = vhost_user_get_vhost_net(nc);
         break;
     default:
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index d728233..9c53d7b 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -398,7 +398,7 @@ static int peer_attach(VirtIONet *n, int index)
         return 0;
     }
 
-    if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) {
+    if (nc->peer->info->type != NET_CLIENT_DRIVER_TAP) {
         return 0;
     }
 
@@ -413,7 +413,7 @@ static int peer_detach(VirtIONet *n, int index)
         return 0;
     }
 
-    if (nc->peer->info->type !=  NET_CLIENT_OPTIONS_KIND_TAP) {
+    if (nc->peer->info->type !=  NET_CLIENT_DRIVER_TAP) {
         return 0;
     }
 
@@ -1501,7 +1501,7 @@ static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f,
 }
 
 static NetClientInfo net_virtio_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = virtio_net_can_receive,
     .receive = virtio_net_receive,
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 104a0f5..38ee1c6 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1927,7 +1927,7 @@ static void vmxnet3_set_link_status(NetClientState *nc)
 }
 
 static NetClientInfo net_vmxnet3_info = {
-        .type = NET_CLIENT_OPTIONS_KIND_NIC,
+        .type = NET_CLIENT_DRIVER_NIC,
         .size = sizeof(NICState),
         .can_receive = vmxnet3_can_receive,
         .receive = vmxnet3_receive,
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index 19ecfc4..b6fc62b 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -302,7 +302,7 @@ static ssize_t net_rx_packet(NetClientState *nc, const uint8_t *buf, size_t size
 /* ------------------------------------------------------------- */
 
 static NetClientInfo net_xen_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = net_rx_ok,
     .receive = net_rx_packet,
diff --git a/hw/net/xgmac.c b/hw/net/xgmac.c
index b068f3a..316f4a8 100644
--- a/hw/net/xgmac.c
+++ b/hw/net/xgmac.c
@@ -369,7 +369,7 @@ out:
 }
 
 static NetClientInfo net_xgmac_enet_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = eth_can_rx,
     .receive = eth_rx,
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 9205770..70b9566 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -923,7 +923,7 @@ xilinx_axienet_data_stream_push(StreamSlave *obj, uint8_t *buf, size_t size)
 }
 
 static NetClientInfo net_xilinx_enet_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = eth_can_rx,
     .receive = eth_rx,
diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index ad6b553..ab555f6 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -214,7 +214,7 @@ static void xilinx_ethlite_reset(DeviceState *dev)
 }
 
 static NetClientInfo net_xilinx_ethlite_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = eth_can_rx,
     .receive = eth_rx,
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 5eeb4c6..c6bcf88 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1341,7 +1341,7 @@ static void usb_net_handle_destroy(USBDevice *dev)
 }
 
 static NetClientInfo net_usbnet_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
+    .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
     .can_receive = usbnet_can_receive,
     .receive = usbnet_receive,
diff --git a/include/net/net.h b/include/net/net.h
index 6a6cbef..c0e00ef 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -59,7 +59,7 @@ typedef int (SetVnetLE)(NetClientState *, bool);
 typedef int (SetVnetBE)(NetClientState *, bool);
 
 typedef struct NetClientInfo {
-    NetClientOptionsKind type;
+    NetClientDriver type;
     size_t size;
     NetReceive *receive;
     NetReceive *receive_raw;
@@ -104,7 +104,7 @@ typedef struct NICState {
 char *qemu_mac_strdup_printf(const uint8_t *macaddr);
 NetClientState *qemu_find_netdev(const char *id);
 int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
-                                 NetClientOptionsKind type, int max);
+                                 NetClientDriver type, int max);
 NetClientState *qemu_new_net_client(NetClientInfo *info,
                                     NetClientState *peer,
                                     const char *model,
diff --git a/monitor.c b/monitor.c
index aeea2b5..a40138b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4188,8 +4188,8 @@ void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str)
     }
     len = strlen(str);
     readline_set_completion_index(rs, len);
-    for (i = 0; NetClientOptionsKind_lookup[i]; i++) {
-        add_completion_option(rs, str, NetClientOptionsKind_lookup[i]);
+    for (i = 0; NetClientDriver_lookup[i]; i++) {
+        add_completion_option(rs, str, NetClientDriver_lookup[i]);
     }
 }
 
@@ -4389,7 +4389,7 @@ void set_link_completion(ReadLineState *rs, int nb_args, const char *str)
         NetClientState *ncs[MAX_QUEUE_NUM];
         int count, i;
         count = qemu_find_net_clients_except(NULL, ncs,
-                                             NET_CLIENT_OPTIONS_KIND_NONE,
+                                             NET_CLIENT_DRIVER_NONE,
                                              MAX_QUEUE_NUM);
         for (i = 0; i < MIN(count, MAX_QUEUE_NUM); i++) {
             const char *name = ncs[i]->name;
@@ -4414,7 +4414,7 @@ void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str)
 
     len = strlen(str);
     readline_set_completion_index(rs, len);
-    count = qemu_find_net_clients_except(NULL, ncs, NET_CLIENT_OPTIONS_KIND_NIC,
+    count = qemu_find_net_clients_except(NULL, ncs, NET_CLIENT_DRIVER_NIC,
                                          MAX_QUEUE_NUM);
     for (i = 0; i < MIN(count, MAX_QUEUE_NUM); i++) {
         QemuOpts *opts;
@@ -4506,7 +4506,7 @@ void host_net_remove_completion(ReadLineState *rs, int nb_args, const char *str)
     readline_set_completion_index(rs, len);
     if (nb_args == 2) {
         count = qemu_find_net_clients_except(NULL, ncs,
-                                             NET_CLIENT_OPTIONS_KIND_NONE,
+                                             NET_CLIENT_DRIVER_NONE,
                                              MAX_QUEUE_NUM);
         for (i = 0; i < MIN(count, MAX_QUEUE_NUM); i++) {
             int id;
@@ -4523,13 +4523,13 @@ void host_net_remove_completion(ReadLineState *rs, int nb_args, const char *str)
         return;
     } else if (nb_args == 3) {
         count = qemu_find_net_clients_except(NULL, ncs,
-                                             NET_CLIENT_OPTIONS_KIND_NIC,
+                                             NET_CLIENT_DRIVER_NIC,
                                              MAX_QUEUE_NUM);
         for (i = 0; i < MIN(count, MAX_QUEUE_NUM); i++) {
             int id;
             const char *name;
 
-            if (ncs[i]->info->type == NET_CLIENT_OPTIONS_KIND_HUBPORT ||
+            if (ncs[i]->info->type == NET_CLIENT_DRIVER_HUBPORT ||
                 net_hub_id_for_client(ncs[i], &id)) {
                 continue;
             }
diff --git a/net/clients.h b/net/clients.h
index d47530e..2aac0ee 100644
--- a/net/clients.h
+++ b/net/clients.h
@@ -27,39 +27,39 @@
 #include "net/net.h"
 #include "qapi-types.h"
 
-int net_init_dump(const NetClientOptions *opts, const char *name,
+int net_init_dump(const void *opts, const char *name,
                   NetClientState *peer, Error **errp);
 
 #ifdef CONFIG_SLIRP
-int net_init_slirp(const NetClientOptions *opts, const char *name,
+int net_init_slirp(const void *opts, const char *name,
                    NetClientState *peer, Error **errp);
 #endif
 
-int net_init_hubport(const NetClientOptions *opts, const char *name,
+int net_init_hubport(const void *opts, const char *name,
                      NetClientState *peer, Error **errp);
 
-int net_init_socket(const NetClientOptions *opts, const char *name,
+int net_init_socket(const void *opts, const char *name,
                     NetClientState *peer, Error **errp);
 
-int net_init_tap(const NetClientOptions *opts, const char *name,
+int net_init_tap(const void *opts, const char *name,
                  NetClientState *peer, Error **errp);
 
-int net_init_bridge(const NetClientOptions *opts, const char *name,
+int net_init_bridge(const void *opts, const char *name,
                     NetClientState *peer, Error **errp);
 
-int net_init_l2tpv3(const NetClientOptions *opts, const char *name,
+int net_init_l2tpv3(const void *opts, const char *name,
                     NetClientState *peer, Error **errp);
 #ifdef CONFIG_VDE
-int net_init_vde(const NetClientOptions *opts, const char *name,
+int net_init_vde(const void *opts, const char *name,
                  NetClientState *peer, Error **errp);
 #endif
 
 #ifdef CONFIG_NETMAP
-int net_init_netmap(const NetClientOptions *opts, const char *name,
+int net_init_netmap(const void *opts, const char *name,
                     NetClientState *peer, Error **errp);
 #endif
 
-int net_init_vhost_user(const NetClientOptions *opts, const char *name,
+int net_init_vhost_user(const void *opts, const char *name,
                         NetClientState *peer, Error **errp);
 
 #endif /* QEMU_NET_CLIENTS_H */
diff --git a/net/dump.c b/net/dump.c
index 02c8064..6aca19d 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -94,7 +94,7 @@ static void dump_cleanup(NetClientState *nc)
 }
 
 static NetClientInfo net_dump_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_DUMP,
+    .type = NET_CLIENT_DRIVER_DUMP,
     .size = sizeof(DumpState),
     .receive = dump_receive,
     .cleanup = dump_cleanup,
@@ -146,16 +146,13 @@ static int net_dump_init(NetClientState *peer, const char *device,
     return 0;
 }
 
-int net_init_dump(const NetClientOptions *opts, const char *name,
+int net_init_dump(const void *opts, const char *name,
                   NetClientState *peer, Error **errp)
 {
     int len;
     const char *file;
     char def_file[128];
-    const NetdevDumpOptions *dump;
-
-    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP);
-    dump = opts->dump;
+    const NetdevDumpOptions *dump = opts;
 
     assert(peer);
 
diff --git a/net/hub.c b/net/hub.c
index 3047f12..b47ee5d 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -130,7 +130,7 @@ static void net_hub_port_cleanup(NetClientState *nc)
 }
 
 static NetClientInfo net_hub_port_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_HUBPORT,
+    .type = NET_CLIENT_DRIVER_HUBPORT,
     .size = sizeof(NetHubPort),
     .can_receive = net_hub_port_can_receive,
     .receive = net_hub_port_receive,
@@ -265,10 +265,10 @@ int net_hub_id_for_client(NetClientState *nc, int *id)
 {
     NetHubPort *port;
 
-    if (nc->info->type == NET_CLIENT_OPTIONS_KIND_HUBPORT) {
+    if (nc->info->type == NET_CLIENT_DRIVER_HUBPORT) {
         port = DO_UPCAST(NetHubPort, nc, nc);
     } else if (nc->peer != NULL && nc->peer->info->type ==
-            NET_CLIENT_OPTIONS_KIND_HUBPORT) {
+            NET_CLIENT_DRIVER_HUBPORT) {
         port = DO_UPCAST(NetHubPort, nc, nc->peer);
     } else {
         return -ENOENT;
@@ -280,14 +280,12 @@ int net_hub_id_for_client(NetClientState *nc, int *id)
     return 0;
 }
 
-int net_init_hubport(const NetClientOptions *opts, const char *name,
+int net_init_hubport(const void *opts, const char *name,
                      NetClientState *peer, Error **errp)
 {
-    const NetdevHubPortOptions *hubport;
+    const NetdevHubPortOptions *hubport = opts;
 
-    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_HUBPORT);
     assert(!peer);
-    hubport = opts->hubport;
 
     net_hub_add_port(hubport->hubid, name);
     return 0;
@@ -314,14 +312,14 @@ void net_hub_check_clients(void)
             }
 
             switch (peer->info->type) {
-            case NET_CLIENT_OPTIONS_KIND_NIC:
+            case NET_CLIENT_DRIVER_NIC:
                 has_nic = 1;
                 break;
-            case NET_CLIENT_OPTIONS_KIND_USER:
-            case NET_CLIENT_OPTIONS_KIND_TAP:
-            case NET_CLIENT_OPTIONS_KIND_SOCKET:
-            case NET_CLIENT_OPTIONS_KIND_VDE:
-            case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
+            case NET_CLIENT_DRIVER_USER:
+            case NET_CLIENT_DRIVER_TAP:
+            case NET_CLIENT_DRIVER_SOCKET:
+            case NET_CLIENT_DRIVER_VDE:
+            case NET_CLIENT_DRIVER_VHOST_USER:
                 has_host_dev = 1;
                 break;
             default:
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 4f9bcee..f33e291 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -516,7 +516,7 @@ static void net_l2tpv3_cleanup(NetClientState *nc)
 }
 
 static NetClientInfo net_l2tpv3_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_L2TPV3,
+    .type = NET_CLIENT_DRIVER_L2TPV3,
     .size = sizeof(NetL2TPV3State),
     .receive = net_l2tpv3_receive_dgram,
     .receive_iov = net_l2tpv3_receive_dgram_iov,
@@ -524,12 +524,12 @@ static NetClientInfo net_l2tpv3_info = {
     .cleanup = net_l2tpv3_cleanup,
 };
 
-int net_init_l2tpv3(const NetClientOptions *opts,
+int net_init_l2tpv3(const void *opts,
                     const char *name,
                     NetClientState *peer, Error **errp)
 {
     /* FIXME error_setg(errp, ...) on failure */
-    const NetdevL2TPv3Options *l2tpv3;
+    const NetdevL2TPv3Options *l2tpv3 = opts;
     NetL2TPV3State *s;
     NetClientState *nc;
     int fd = -1, gairet;
@@ -545,9 +545,6 @@ int net_init_l2tpv3(const NetClientOptions *opts,
     s->queue_tail = 0;
     s->header_mismatch = false;
 
-    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_L2TPV3);
-    l2tpv3 = opts->l2tpv3;
-
     if (l2tpv3->has_ipv6 && l2tpv3->ipv6) {
         s->ipv6 = l2tpv3->ipv6;
     } else {
diff --git a/net/net.c b/net/net.c
index cc36c7b..5f230a5 100644
--- a/net/net.c
+++ b/net/net.c
@@ -312,7 +312,7 @@ NICState *qemu_new_nic(NetClientInfo *info,
     NICState *nic;
     int i, queues = MAX(1, conf->peers.queues);
 
-    assert(info->type == NET_CLIENT_OPTIONS_KIND_NIC);
+    assert(info->type == NET_CLIENT_DRIVER_NIC);
     assert(info->size >= sizeof(NICState));
 
     nic = g_malloc0(info->size + sizeof(NetClientState) * queues);
@@ -382,18 +382,18 @@ void qemu_del_net_client(NetClientState *nc)
     NetClientState *ncs[MAX_QUEUE_NUM];
     int queues, i;
 
-    assert(nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC);
+    assert(nc->info->type != NET_CLIENT_DRIVER_NIC);
 
     /* If the NetClientState belongs to a multiqueue backend, we will change all
      * other NetClientStates also.
      */
     queues = qemu_find_net_clients_except(nc->name, ncs,
-                                          NET_CLIENT_OPTIONS_KIND_NIC,
+                                          NET_CLIENT_DRIVER_NIC,
                                           MAX_QUEUE_NUM);
     assert(queues != 0);
 
     /* If there is a peer NIC, delete and cleanup client, but do not free. */
-    if (nc->peer && nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
+    if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_NIC) {
         NICState *nic = qemu_get_nic(nc->peer);
         if (nic->peer_deleted) {
             return;
@@ -449,7 +449,7 @@ void qemu_foreach_nic(qemu_nic_foreach func, void *opaque)
     NetClientState *nc;
 
     QTAILQ_FOREACH(nc, &net_clients, next) {
-        if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
+        if (nc->info->type == NET_CLIENT_DRIVER_NIC) {
             if (nc->queue_index == 0) {
                 func(qemu_get_nic(nc), opaque);
             }
@@ -595,7 +595,7 @@ void qemu_flush_or_purge_queued_packets(NetClientState *nc, bool purge)
 {
     nc->receive_disabled = 0;
 
-    if (nc->peer && nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_HUBPORT) {
+    if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_HUBPORT) {
         if (net_hub_flush(nc->peer)) {
             qemu_notify_event();
         }
@@ -725,7 +725,7 @@ NetClientState *qemu_find_netdev(const char *id)
     NetClientState *nc;
 
     QTAILQ_FOREACH(nc, &net_clients, next) {
-        if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC)
+        if (nc->info->type == NET_CLIENT_DRIVER_NIC)
             continue;
         if (!strcmp(nc->name, id)) {
             return nc;
@@ -736,7 +736,7 @@ NetClientState *qemu_find_netdev(const char *id)
 }
 
 int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
-                                 NetClientOptionsKind type, int max)
+                                 NetClientDriver type, int max)
 {
     NetClientState *nc;
     int ret = 0;
@@ -810,15 +810,12 @@ int qemu_find_nic_model(NICInfo *nd, const char * const *models,
     return -1;
 }
 
-static int net_init_nic(const NetClientOptions *opts, const char *name,
+static int net_init_nic(const void *opts, const char *name,
                         NetClientState *peer, Error **errp)
 {
     int idx;
     NICInfo *nd;
-    const NetLegacyNicOptions *nic;
-
-    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_NIC);
-    nic = opts->nic;
+    const NetLegacyNicOptions *nic = opts;
 
     idx = nic_get_free_idx();
     if (idx == -1 || nb_nics >= MAX_NICS) {
@@ -878,32 +875,32 @@ static int net_init_nic(const NetClientOptions *opts, const char *name,
 }
 
 
-static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
-    const NetClientOptions *opts,
+static int (* const net_client_init_fun[NET_CLIENT_DRIVER_MAX])(
+    const void *opts,
     const char *name,
     NetClientState *peer, Error **errp) = {
-        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
+        [NET_CLIENT_DRIVER_NIC]       = net_init_nic,
 #ifdef CONFIG_SLIRP
-        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
+        [NET_CLIENT_DRIVER_USER]      = net_init_slirp,
 #endif
-        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
-        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
+        [NET_CLIENT_DRIVER_TAP]       = net_init_tap,
+        [NET_CLIENT_DRIVER_SOCKET]    = net_init_socket,
 #ifdef CONFIG_VDE
-        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
+        [NET_CLIENT_DRIVER_VDE]       = net_init_vde,
 #endif
 #ifdef CONFIG_NETMAP
-        [NET_CLIENT_OPTIONS_KIND_NETMAP]    = net_init_netmap,
+        [NET_CLIENT_DRIVER_NETMAP]    = net_init_netmap,
 #endif
-        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
+        [NET_CLIENT_DRIVER_DUMP]      = net_init_dump,
 #ifdef CONFIG_NET_BRIDGE
-        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
+        [NET_CLIENT_DRIVER_BRIDGE]    = net_init_bridge,
 #endif
-        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
+        [NET_CLIENT_DRIVER_HUBPORT]   = net_init_hubport,
 #ifdef CONFIG_VHOST_NET_USED
-        [NET_CLIENT_OPTIONS_KIND_VHOST_USER] = net_init_vhost_user,
+        [NET_CLIENT_DRIVER_VHOST_USER] = net_init_vhost_user,
 #endif
 #ifdef CONFIG_L2TPV3
-        [NET_CLIENT_OPTIONS_KIND_L2TPV3]    = net_init_l2tpv3,
+        [NET_CLIENT_DRIVER_L2TPV3]    = net_init_l2tpv3,
 #endif
 };
 
@@ -914,35 +911,37 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
         const Netdev    *netdev;
         const NetLegacy *net;
     } u;
-    const NetClientOptions *opts;
+    NetClientDriver kind;
+    const void *opts;
     const char *name;
 
     if (is_netdev) {
         u.netdev = object;
-        opts = u.netdev->opts;
+        kind = u.netdev->kind;
+        opts = u.netdev->data;
         name = u.netdev->id;
 
-        switch (opts->kind) {
+        switch (kind) {
 #ifdef CONFIG_SLIRP
-        case NET_CLIENT_OPTIONS_KIND_USER:
+        case NET_CLIENT_DRIVER_USER:
 #endif
-        case NET_CLIENT_OPTIONS_KIND_TAP:
-        case NET_CLIENT_OPTIONS_KIND_SOCKET:
+        case NET_CLIENT_DRIVER_TAP:
+        case NET_CLIENT_DRIVER_SOCKET:
 #ifdef CONFIG_VDE
-        case NET_CLIENT_OPTIONS_KIND_VDE:
+        case NET_CLIENT_DRIVER_VDE:
 #endif
 #ifdef CONFIG_NETMAP
-        case NET_CLIENT_OPTIONS_KIND_NETMAP:
+        case NET_CLIENT_DRIVER_NETMAP:
 #endif
 #ifdef CONFIG_NET_BRIDGE
-        case NET_CLIENT_OPTIONS_KIND_BRIDGE:
+        case NET_CLIENT_DRIVER_BRIDGE:
 #endif
-        case NET_CLIENT_OPTIONS_KIND_HUBPORT:
+        case NET_CLIENT_DRIVER_HUBPORT:
 #ifdef CONFIG_VHOST_NET_USED
-        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
+        case NET_CLIENT_DRIVER_VHOST_USER:
 #endif
 #ifdef CONFIG_L2TPV3
-        case NET_CLIENT_OPTIONS_KIND_L2TPV3:
+        case NET_CLIENT_DRIVER_L2TPV3:
 #endif
             break;
 
@@ -953,8 +952,9 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
         }
     } else {
         u.net = object;
-        opts = u.net->opts;
-        if (opts->kind == NET_CLIENT_OPTIONS_KIND_HUBPORT) {
+        kind = u.net->kind;
+        opts = u.net->data;
+        if (kind == NET_CLIENT_DRIVER_HUBPORT) {
             error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
                        "a net type");
             return -1;
@@ -963,22 +963,22 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
         name = u.net->has_id ? u.net->id : u.net->name;
     }
 
-    if (net_client_init_fun[opts->kind]) {
+    if (net_client_init_fun[kind]) {
         NetClientState *peer = NULL;
 
         /* Do not add to a vlan if it's a -netdev or a nic with a netdev=
          * parameter. */
         if (!is_netdev &&
-            (opts->kind != NET_CLIENT_OPTIONS_KIND_NIC ||
-             !opts->nic->has_netdev)) {
+            (kind != NET_CLIENT_DRIVER_NIC ||
+             !((NetLegacyNicOptions *) opts)->has_netdev)) {
             peer = net_hub_add_port(u.net->has_vlan ? u.net->vlan : 0, NULL);
         }
 
-        if (net_client_init_fun[opts->kind](opts, name, peer, errp) < 0) {
+        if (net_client_init_fun[kind](opts, name, peer, errp) < 0) {
             /* FIXME drop when all init functions store an Error */
             if (errp && !*errp) {
                 error_setg(errp, QERR_DEVICE_INIT_FAILED,
-                           NetClientOptionsKind_lookup[opts->kind]);
+                           NetClientDriver_lookup[kind]);
             }
             return -1;
         }
@@ -1078,7 +1078,7 @@ void hmp_host_net_remove(Monitor *mon, const QDict *qdict)
                      device, vlan_id);
         return;
     }
-    if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
+    if (nc->info->type == NET_CLIENT_DRIVER_NIC) {
         error_report("invalid host network device '%s'", device);
         return;
     }
@@ -1144,7 +1144,7 @@ void print_net_client(Monitor *mon, NetClientState *nc)
 {
     monitor_printf(mon, "%s: index=%d,type=%s,%s\n", nc->name,
                    nc->queue_index,
-                   NetClientOptionsKind_lookup[nc->info->type],
+                   NetClientDriver_lookup[nc->info->type],
                    nc->info_str);
 }
 
@@ -1163,7 +1163,7 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
         }
 
         /* only query rx-filter information of NIC */
-        if (nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC) {
+        if (nc->info->type != NET_CLIENT_DRIVER_NIC) {
             if (has_name) {
                 error_setg(errp, "net client(%s) isn't a NIC", name);
                 return NULL;
@@ -1203,7 +1203,7 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
 void hmp_info_network(Monitor *mon, const QDict *qdict)
 {
     NetClientState *nc, *peer;
-    NetClientOptionsKind type;
+    NetClientDriver type;
 
     net_hub_info(mon);
 
@@ -1216,10 +1216,10 @@ void hmp_info_network(Monitor *mon, const QDict *qdict)
             continue;
         }
 
-        if (!peer || type == NET_CLIENT_OPTIONS_KIND_NIC) {
+        if (!peer || type == NET_CLIENT_DRIVER_NIC) {
             print_net_client(mon, nc);
         } /* else it's a netdev connected to a NIC, printed with the NIC */
-        if (peer && type == NET_CLIENT_OPTIONS_KIND_NIC) {
+        if (peer && type == NET_CLIENT_DRIVER_NIC) {
             monitor_printf(mon, " \\ ");
             print_net_client(mon, peer);
         }
@@ -1233,7 +1233,7 @@ void qmp_set_link(const char *name, bool up, Error **errp)
     int queues, i;
 
     queues = qemu_find_net_clients_except(name, ncs,
-                                          NET_CLIENT_OPTIONS_KIND_MAX,
+                                          NET_CLIENT_DRIVER_MAX,
                                           MAX_QUEUE_NUM);
 
     if (queues == 0) {
@@ -1260,7 +1260,7 @@ void qmp_set_link(const char *name, bool up, Error **errp)
          * multiple clients that can still communicate with each other in
          * disconnected mode. For now maintain this compatibility.
          */
-        if (nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
+        if (nc->peer->info->type == NET_CLIENT_DRIVER_NIC) {
             for (i = 0; i < queues; i++) {
                 ncs[i]->peer->link_down = !up;
             }
@@ -1296,7 +1296,7 @@ void net_cleanup(void)
      */
     while (!QTAILQ_EMPTY(&net_clients)) {
         nc = QTAILQ_FIRST(&net_clients);
-        if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
+        if (nc->info->type == NET_CLIENT_DRIVER_NIC) {
             qemu_del_nic(qemu_get_nic(nc));
         } else {
             qemu_del_net_client(nc);
@@ -1328,7 +1328,7 @@ void net_check_clients(void)
     QTAILQ_FOREACH(nc, &net_clients, next) {
         if (!nc->peer) {
             fprintf(stderr, "Warning: %s %s has no peer\n",
-                    nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC ?
+                    nc->info->type == NET_CLIENT_DRIVER_NIC ?
                     "nic" : "netdev", nc->name);
         }
     }
diff --git a/net/netmap.c b/net/netmap.c
index 508b829..537de09 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -417,7 +417,7 @@ static void netmap_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
 
 /* NetClientInfo methods */
 static NetClientInfo net_netmap_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NETMAP,
+    .type = NET_CLIENT_DRIVER_NETMAP,
     .size = sizeof(NetmapState),
     .receive = netmap_receive,
     .receive_iov = netmap_receive_iov,
@@ -435,11 +435,11 @@ static NetClientInfo net_netmap_info = {
  *
  * ... -net netmap,ifname="..."
  */
-int net_init_netmap(const NetClientOptions *opts,
+int net_init_netmap(const void *opts,
                     const char *name, NetClientState *peer, Error **errp)
 {
     /* FIXME error_setg(errp, ...) on failure */
-    const NetdevNetmapOptions *netmap_opts = opts->netmap;
+    const NetdevNetmapOptions *netmap_opts = opts;
     NetClientState *nc;
     NetmapPriv me;
     NetmapState *s;
diff --git a/net/slirp.c b/net/slirp.c
index 7657b38..b856ab9 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -126,7 +126,7 @@ static void net_slirp_cleanup(NetClientState *nc)
 }
 
 static NetClientInfo net_slirp_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_USER,
+    .type = NET_CLIENT_DRIVER_USER,
     .size = sizeof(SlirpState),
     .receive = net_slirp_receive,
     .cleanup = net_slirp_cleanup,
@@ -736,19 +736,16 @@ static const char **slirp_dnssearch(const StringList *dnsname)
     return ret;
 }
 
-int net_init_slirp(const NetClientOptions *opts, const char *name,
+int net_init_slirp(const void *opts, const char *name,
                    NetClientState *peer, Error **errp)
 {
     /* FIXME error_setg(errp, ...) on failure */
     struct slirp_config_str *config;
     char *vnet;
     int ret;
-    const NetdevUserOptions *user;
+    const NetdevUserOptions *user = opts;
     const char **dnssearch;
 
-    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_USER);
-    user = opts->user;
-
     vnet = user->has_net ? g_strdup(user->net) :
            user->has_ip  ? g_strdup_printf("%s/24", user->ip) :
            NULL;
diff --git a/net/socket.c b/net/socket.c
index c752696..c5b4f8d 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -346,7 +346,7 @@ static void net_socket_cleanup(NetClientState *nc)
 }
 
 static NetClientInfo net_dgram_socket_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_SOCKET,
+    .type = NET_CLIENT_DRIVER_SOCKET,
     .size = sizeof(NetSocketState),
     .receive = net_socket_receive_dgram,
     .cleanup = net_socket_cleanup,
@@ -429,7 +429,7 @@ static void net_socket_connect(void *opaque)
 }
 
 static NetClientInfo net_socket_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_SOCKET,
+    .type = NET_CLIENT_DRIVER_SOCKET,
     .size = sizeof(NetSocketState),
     .receive = net_socket_receive,
     .cleanup = net_socket_cleanup,
@@ -699,15 +699,12 @@ static int net_socket_udp_init(NetClientState *peer,
     return 0;
 }
 
-int net_init_socket(const NetClientOptions *opts, const char *name,
+int net_init_socket(const void *opts, const char *name,
                     NetClientState *peer, Error **errp)
 {
     /* FIXME error_setg(errp, ...) on failure */
     Error *err = NULL;
-    const NetdevSocketOptions *sock;
-
-    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_SOCKET);
-    sock = opts->socket;
+    const NetdevSocketOptions *sock = opts;
 
     if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast +
         sock->has_udp != 1) {
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 625d53c..7f20809 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -723,7 +723,7 @@ static void tap_set_vnet_hdr_len(NetClientState *nc, int len)
 }
 
 static NetClientInfo net_tap_win32_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_TAP,
+    .type = NET_CLIENT_DRIVER_TAP,
     .size = sizeof(TAPState),
     .receive = tap_receive,
     .cleanup = tap_cleanup,
@@ -761,14 +761,11 @@ static int tap_win32_init(NetClientState *peer, const char *model,
     return 0;
 }
 
-int net_init_tap(const NetClientOptions *opts, const char *name,
+int net_init_tap(const void *opts, const char *name,
                  NetClientState *peer, Error **errp)
 {
     /* FIXME error_setg(errp, ...) on failure */
-    const NetdevTapOptions *tap;
-
-    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_TAP);
-    tap = opts->tap;
+    const NetdevTapOptions *tap = opts;
 
     if (!tap->has_ifname) {
         error_report("tap: no interface name");
diff --git a/net/tap.c b/net/tap.c
index bd01590..1d31c51 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -221,7 +221,7 @@ static bool tap_has_ufo(NetClientState *nc)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
 
-    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
+    assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
 
     return s->has_ufo;
 }
@@ -230,7 +230,7 @@ static bool tap_has_vnet_hdr(NetClientState *nc)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
 
-    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
+    assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
 
     return !!s->host_vnet_hdr_len;
 }
@@ -239,7 +239,7 @@ static bool tap_has_vnet_hdr_len(NetClientState *nc, int len)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
 
-    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
+    assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
 
     return !!tap_probe_vnet_hdr_len(s->fd, len);
 }
@@ -248,7 +248,7 @@ static void tap_set_vnet_hdr_len(NetClientState *nc, int len)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
 
-    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
+    assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
     assert(len == sizeof(struct virtio_net_hdr_mrg_rxbuf) ||
            len == sizeof(struct virtio_net_hdr));
 
@@ -260,7 +260,7 @@ static void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
 
-    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
+    assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
     assert(!!s->host_vnet_hdr_len == using_vnet_hdr);
 
     s->using_vnet_hdr = using_vnet_hdr;
@@ -326,14 +326,14 @@ static void tap_poll(NetClientState *nc, bool enable)
 int tap_get_fd(NetClientState *nc)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
-    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
+    assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
     return s->fd;
 }
 
 /* fd support */
 
 static NetClientInfo net_tap_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_TAP,
+    .type = NET_CLIENT_DRIVER_TAP,
     .size = sizeof(TAPState),
     .receive = tap_receive,
     .receive_raw = tap_receive_raw,
@@ -557,17 +557,14 @@ static int net_bridge_run_helper(const char *helper, const char *bridge,
     }
 }
 
-int net_init_bridge(const NetClientOptions *opts, const char *name,
+int net_init_bridge(const void *opts, const char *name,
                     NetClientState *peer, Error **errp)
 {
-    const NetdevBridgeOptions *bridge;
+    const NetdevBridgeOptions *bridge = opts;
     const char *helper, *br;
     TAPState *s;
     int fd, vnet_hdr;
 
-    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_BRIDGE);
-    bridge = opts->bridge;
-
     helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER;
     br     = bridge->has_br     ? bridge->br     : DEFAULT_BRIDGE_INTERFACE;
 
@@ -716,10 +713,10 @@ static int get_fds(char *str, char *fds[], int max)
     return i;
 }
 
-int net_init_tap(const NetClientOptions *opts, const char *name,
+int net_init_tap(const void *opts, const char *name,
                  NetClientState *peer, Error **errp)
 {
-    const NetdevTapOptions *tap;
+    const NetdevTapOptions *tap = opts;
     int fd, vnet_hdr = 0, i = 0, queues;
     /* for the no-fd, no-helper case */
     const char *script = NULL; /* suppress wrong "uninit'd use" gcc warning */
@@ -728,8 +725,6 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
     const char *vhostfdname;
     char ifname[128];
 
-    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_TAP);
-    tap = opts->tap;
     queues = tap->has_queues ? tap->queues : 1;
     vhostfdname = tap->has_vhostfd ? tap->vhostfd : NULL;
 
@@ -890,7 +885,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
 VHostNetState *tap_get_vhost_net(NetClientState *nc)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
-    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
+    assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
     return s->vhost_net;
 }
 
diff --git a/net/vde.c b/net/vde.c
index dacaa64..2aaf6d5 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -68,7 +68,7 @@ static void vde_cleanup(NetClientState *nc)
 }
 
 static NetClientInfo net_vde_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_VDE,
+    .type = NET_CLIENT_DRIVER_VDE,
     .size = sizeof(VDEState),
     .receive = vde_receive,
     .cleanup = vde_cleanup,
@@ -109,14 +109,11 @@ static int net_vde_init(NetClientState *peer, const char *model,
     return 0;
 }
 
-int net_init_vde(const NetClientOptions *opts, const char *name,
+int net_init_vde(const void *opts, const char *name,
                  NetClientState *peer, Error **errp)
 {
     /* FIXME error_setg(errp, ...) on failure */
-    const NetdevVdeOptions *vde;
-
-    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VDE);
-    vde = opts->vde;
+    const NetdevVdeOptions *vde = opts;
 
     /* missing optional values have been initialized to "all bits zero" */
     if (net_vde_init(peer, "vde", name, vde->sock, vde->port, vde->group,
diff --git a/net/vhost-user.c b/net/vhost-user.c
index b51bc04..69b7daa 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -30,7 +30,7 @@ typedef struct VhostUserChardevProps {
 VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
 {
     VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
-    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
+    assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_USER);
     return s->vhost_net;
 }
 
@@ -75,20 +75,20 @@ static void vhost_user_cleanup(NetClientState *nc)
 
 static bool vhost_user_has_vnet_hdr(NetClientState *nc)
 {
-    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
+    assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_USER);
 
     return true;
 }
 
 static bool vhost_user_has_ufo(NetClientState *nc)
 {
-    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
+    assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_USER);
 
     return true;
 }
 
 static NetClientInfo net_vhost_user_info = {
-        .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
+        .type = NET_CLIENT_DRIVER_VHOST_USER,
         .size = sizeof(VhostUserState),
         .cleanup = vhost_user_cleanup,
         .has_vnet_hdr = vhost_user_has_vnet_hdr,
@@ -227,16 +227,13 @@ static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
     return 0;
 }
 
-int net_init_vhost_user(const NetClientOptions *opts, const char *name,
+int net_init_vhost_user(const void *opts, const char *name,
                         NetClientState *peer, Error **errp)
 {
     uint32_t queues;
-    const NetdevVhostUserOptions *vhost_user_opts;
+    const NetdevVhostUserOptions *vhost_user_opts = opts;
     CharDriverState *chr;
 
-    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
-    vhost_user_opts = opts->vhost_user;
-
     chr = net_vhost_parse_chardev(vhost_user_opts, errp);
     if (!chr) {
         return -1;
diff --git a/qapi-schema.json b/qapi-schema.json
index 7ebf99e..2a3cfe3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2475,34 +2475,20 @@
     '*queues':        'uint32' } }
 
 ##
-# @NetClientOptions
+# @NetClientDriver
 #
-# A discriminated record of network device traits.
-#
-# Since 1.2
-#
-# 'l2tpv3' - since 2.1
+# Possible netdev drivers.
 #
+# Since 2.4
 ##
-{ 'union': 'NetClientOptions',
-  'data': {
-    'none':     'NetdevNoneOptions',
-    'nic':      'NetLegacyNicOptions',
-    'user':     'NetdevUserOptions',
-    'tap':      'NetdevTapOptions',
-    'l2tpv3':   'NetdevL2TPv3Options',
-    'socket':   'NetdevSocketOptions',
-    'vde':      'NetdevVdeOptions',
-    'dump':     'NetdevDumpOptions',
-    'bridge':   'NetdevBridgeOptions',
-    'hubport':  'NetdevHubPortOptions',
-    'netmap':   'NetdevNetmapOptions',
-    'vhost-user': 'NetdevVhostUserOptions' } }
+{ 'enum': 'NetClientDriver',
+  'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde', 'dump',
+            'bridge', 'hubport', 'netmap', 'vhost-user' ] }
 
 ##
-# @NetLegacy
+# @NetLegacyBase
 #
-# Captures the configuration of a network device; legacy.
+# Captures the common configuration of a network device; legacy.
 #
 # @vlan: #optional vlan number
 #
@@ -2512,30 +2498,87 @@
 #
 # @opts: device type specific properties (legacy)
 #
-# Since 1.2
+# @type: the netdev driver to use
+#
+# Since 2.4
 ##
-{ 'struct': 'NetLegacy',
+{ 'struct': 'NetLegacyBase',
   'data': {
     '*vlan': 'int32',
     '*id':   'str',
     '*name': 'str',
-    'opts':  'NetClientOptions' } }
+    'type': 'NetClientDriver' } }
 
 ##
-# @Netdev
+# @NetLegacy
 #
-# Captures the configuration of a network device.
+# Captures the configuration of a network device; legacy.
+#
+# Since 1.2
+#
+# 'l2tpv3' - since 2.1
+##
+{ 'union': 'NetLegacy',
+  'base': 'NetLegacyBase',
+  'discriminator': 'type',
+  'data': {
+    'none':     'NetdevNoneOptions',
+    'nic':      'NetLegacyNicOptions',
+    'user':     'NetdevUserOptions',
+    'tap':      'NetdevTapOptions',
+    'l2tpv3':   'NetdevL2TPv3Options',
+    'socket':   'NetdevSocketOptions',
+    'vde':      'NetdevVdeOptions',
+    'dump':     'NetdevDumpOptions',
+    'bridge':   'NetdevBridgeOptions',
+    'hubport':  'NetdevHubPortOptions',
+    'netmap':   'NetdevNetmapOptions',
+    'vhost-user': 'NetdevVhostUserOptions' } }
+
+
+##
+# @NetdevBase
+#
+# Captures the common configuration of a network device.
 #
 # @id: identifier for monitor commands.
 #
 # @opts: device type specific properties
 #
-# Since 1.2
+# @type: the netdev driver to use
+#
+# Since 2.4
 ##
-{ 'struct': 'Netdev',
+{ 'struct': 'NetdevBase',
   'data': {
     'id':   'str',
-    'opts': 'NetClientOptions' } }
+    'type': 'NetClientDriver' } }
+
+##
+# @Netdev
+#
+# Captures the configuration of a network device.
+#
+# Since 1.2
+#
+# 'l2tpv3' - since 2.1
+##
+{ 'union': 'Netdev',
+  'base': 'NetdevBase',
+  'discriminator': 'type',
+  'data': {
+    'none':     'NetdevNoneOptions',
+    'nic':      'NetLegacyNicOptions',
+    'user':     'NetdevUserOptions',
+    'tap':      'NetdevTapOptions',
+    'l2tpv3':   'NetdevL2TPv3Options',
+    'socket':   'NetdevSocketOptions',
+    'vde':      'NetdevVdeOptions',
+    'dump':     'NetdevDumpOptions',
+    'bridge':   'NetdevBridgeOptions',
+    'hubport':  'NetdevHubPortOptions',
+    'netmap':   'NetdevNetmapOptions',
+    'vhost-user': 'NetdevVhostUserOptions' } }
 
 ##
 # @InetSocketAddress
-- 
2.4.3

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

* [Qemu-devel] [PATCH 4/5] qapi: support nested structs in OptsVisitor
  2015-06-23 13:32 [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches Kővágó, Zoltán
                   ` (2 preceding siblings ...)
  2015-06-23 13:32 ` [Qemu-devel] [PATCH 3/5] qapi: change Netdev and NetLegacy " Kővágó, Zoltán
@ 2015-06-23 13:33 ` Kővágó, Zoltán
  2015-07-02 17:34   ` Markus Armbruster
  2015-06-23 13:33 ` [Qemu-devel] [PATCH 5/5] opts: produce valid command line in qemu_opts_print Kővágó, Zoltán
  2015-06-25  6:16 ` [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches Gerd Hoffmann
  5 siblings, 1 reply; 19+ messages in thread
From: Kővágó, Zoltán @ 2015-06-23 13:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Stefan Hajnoczi, Michael Roth

The current OptsVisitor flattens the whole structure, if there are same
named fields under different paths the current visitor can't cope with
them (it'll just set the first field, leaving the others unspecified (if
they're optional) or erroring out (if they're required).

This patch add support for it, by always requiring a complete path in
case of nested structs.  Fields in the path are separated by dots,
similar to C structs (without pointers), like `foo.bar'.

You must provide a full path even in non-ambigous cases.  The previous
two commits hopefully ensures that this change doesn't create backward
compatibility problems.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>

---

Rationale for this commit: see these threads
http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg04189.html
http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg04186.html

Change from v2 -audiodev patch:
* only fully qualified paths are allowed


 qapi/opts-visitor.c                     | 114 ++++++++++++++++++++++++++------
 tests/qapi-schema/qapi-schema-test.json |   9 ++-
 tests/test-opts-visitor.c               |  34 ++++++++++
 3 files changed, 135 insertions(+), 22 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index aa68814..ff61d42 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -71,6 +71,7 @@ struct OptsVisitor
      * schema, with a single mandatory scalar member. */
     ListMode list_mode;
     GQueue *repeated_opts;
+    char *repeated_name;
 
     /* When parsing a list of repeating options as integers, values of the form
      * "a-b", representing a closed interval, are allowed. Elements in the
@@ -86,6 +87,9 @@ struct OptsVisitor
      * not survive or escape the OptsVisitor object.
      */
     QemuOpt *fake_id_opt;
+
+    /* List of field names leading to the current structure. */
+    GQueue *nested_names;
 };
 
 
@@ -100,6 +104,7 @@ static void
 opts_visitor_insert(GHashTable *unprocessed_opts, const QemuOpt *opt)
 {
     GQueue *list;
+    assert(opt);
 
     list = g_hash_table_lookup(unprocessed_opts, opt->name);
     if (list == NULL) {
@@ -127,6 +132,9 @@ opts_start_struct(Visitor *v, void **obj, const char *kind,
     if (obj) {
         *obj = g_malloc0(size > 0 ? size : 1);
     }
+
+    g_queue_push_tail(ov->nested_names, (gpointer) name);
+
     if (ov->depth++ > 0) {
         return;
     }
@@ -169,6 +177,8 @@ opts_end_struct(Visitor *v, Error **errp)
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
     GQueue *any;
 
+    g_queue_pop_tail(ov->nested_names);
+
     if (--ov->depth > 0) {
         return;
     }
@@ -198,15 +208,54 @@ opts_end_implicit_struct(Visitor *v, Error **errp)
 }
 
 
+static void
+sum_strlen(gpointer data, gpointer user_data)
+{
+    const char *str = data;
+    size_t *sum_len = user_data;
+
+    if (str) { /* skip NULLs */
+        *sum_len += strlen(str) + 1;
+    }
+}
+
+static void
+append_str(gpointer data, gpointer user_data)
+{
+    const char *str = data;
+    char *concat_str = user_data;
+
+    if (str) {
+        strcat(concat_str, str);
+        strcat(concat_str, ".");
+    }
+}
+
+/* lookup a name, using a fully qualified version */
 static GQueue *
-lookup_distinct(const OptsVisitor *ov, const char *name, Error **errp)
+lookup_distinct(const OptsVisitor *ov, const char *name, char **out_key,
+                Error **errp)
 {
-    GQueue *list;
+    GQueue *list = NULL;
+    char *key;
+    size_t sum_len = strlen(name);
+
+    g_queue_foreach(ov->nested_names, sum_strlen, &sum_len);
+    key = g_malloc(sum_len+1);
+    key[0] = 0;
+    g_queue_foreach(ov->nested_names, append_str, key);
+    strcat(key, name);
+
+    list = g_hash_table_lookup(ov->unprocessed_opts, key);
+    if (list && out_key) {
+        *out_key = g_strdup(key);
+    }
 
-    list = g_hash_table_lookup(ov->unprocessed_opts, name);
     if (!list) {
         error_setg(errp, QERR_MISSING_PARAMETER, name);
     }
+
+    g_free(key);
     return list;
 }
 
@@ -218,7 +267,7 @@ opts_start_list(Visitor *v, const char *name, Error **errp)
 
     /* we can't traverse a list in a list */
     assert(ov->list_mode == LM_NONE);
-    ov->repeated_opts = lookup_distinct(ov, name, errp);
+    ov->repeated_opts = lookup_distinct(ov, name, &ov->repeated_name, errp);
     if (ov->repeated_opts != NULL) {
         ov->list_mode = LM_STARTED;
     }
@@ -254,11 +303,9 @@ opts_next_list(Visitor *v, GenericList **list, Error **errp)
         /* range has been completed, fall through in order to pop option */
 
     case LM_IN_PROGRESS: {
-        const QemuOpt *opt;
-
-        opt = g_queue_pop_head(ov->repeated_opts);
+        g_queue_pop_head(ov->repeated_opts);
         if (g_queue_is_empty(ov->repeated_opts)) {
-            g_hash_table_remove(ov->unprocessed_opts, opt->name);
+            g_hash_table_remove(ov->unprocessed_opts, ov->repeated_name);
             return NULL;
         }
         link = &(*list)->next;
@@ -284,22 +331,28 @@ opts_end_list(Visitor *v, Error **errp)
            ov->list_mode == LM_SIGNED_INTERVAL ||
            ov->list_mode == LM_UNSIGNED_INTERVAL);
     ov->repeated_opts = NULL;
+
+    g_free(ov->repeated_name);
+    ov->repeated_name = NULL;
+
     ov->list_mode = LM_NONE;
 }
 
 
 static const QemuOpt *
-lookup_scalar(const OptsVisitor *ov, const char *name, Error **errp)
+lookup_scalar(const OptsVisitor *ov, const char *name, char** out_key,
+              Error **errp)
 {
     if (ov->list_mode == LM_NONE) {
         GQueue *list;
 
         /* the last occurrence of any QemuOpt takes effect when queried by name
          */
-        list = lookup_distinct(ov, name, errp);
+        list = lookup_distinct(ov, name, out_key, errp);
         return list ? g_queue_peek_tail(list) : NULL;
     }
     assert(ov->list_mode == LM_IN_PROGRESS);
+    assert(out_key == NULL || *out_key == NULL);
     return g_queue_peek_head(ov->repeated_opts);
 }
 
@@ -321,13 +374,15 @@ opts_type_str(Visitor *v, char **obj, const char *name, Error **errp)
 {
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
     const QemuOpt *opt;
+    char *key = NULL;
 
-    opt = lookup_scalar(ov, name, errp);
+    opt = lookup_scalar(ov, name, &key, errp);
     if (!opt) {
         return;
     }
     *obj = g_strdup(opt->str ? opt->str : "");
-    processed(ov, name);
+    processed(ov, key);
+    g_free(key);
 }
 
 
@@ -337,8 +392,9 @@ opts_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
 {
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
     const QemuOpt *opt;
+    char *key = NULL;
 
-    opt = lookup_scalar(ov, name, errp);
+    opt = lookup_scalar(ov, name, &key, errp);
     if (!opt) {
         return;
     }
@@ -355,13 +411,15 @@ opts_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
         } else {
             error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
                        "on|yes|y|off|no|n");
+            g_free(key);
             return;
         }
     } else {
         *obj = true;
     }
 
-    processed(ov, name);
+    processed(ov, key);
+    g_free(key);
 }
 
 
@@ -373,13 +431,14 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
     const char *str;
     long long val;
     char *endptr;
+    char *key = NULL;
 
     if (ov->list_mode == LM_SIGNED_INTERVAL) {
         *obj = ov->range_next.s;
         return;
     }
 
-    opt = lookup_scalar(ov, name, errp);
+    opt = lookup_scalar(ov, name, &key, errp);
     if (!opt) {
         return;
     }
@@ -393,11 +452,13 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
     if (errno == 0 && endptr > str && INT64_MIN <= val && val <= INT64_MAX) {
         if (*endptr == '\0') {
             *obj = val;
-            processed(ov, name);
+            processed(ov, key);
+            g_free(key);
             return;
         }
         if (*endptr == '-' && ov->list_mode == LM_IN_PROGRESS) {
             long long val2;
+            assert(key == NULL);
 
             str = endptr + 1;
             val2 = strtoll(str, &endptr, 0);
@@ -418,6 +479,7 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
     error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
                (ov->list_mode == LM_NONE) ? "an int64 value" :
                                             "an int64 value or range");
+    g_free(key);
 }
 
 
@@ -429,13 +491,14 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     const char *str;
     unsigned long long val;
     char *endptr;
+    char *key = NULL;
 
     if (ov->list_mode == LM_UNSIGNED_INTERVAL) {
         *obj = ov->range_next.u;
         return;
     }
 
-    opt = lookup_scalar(ov, name, errp);
+    opt = lookup_scalar(ov, name, &key, errp);
     if (!opt) {
         return;
     }
@@ -447,11 +510,13 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     if (parse_uint(str, &val, &endptr, 0) == 0 && val <= UINT64_MAX) {
         if (*endptr == '\0') {
             *obj = val;
-            processed(ov, name);
+            processed(ov, key);
+            g_free(key);
             return;
         }
         if (*endptr == '-' && ov->list_mode == LM_IN_PROGRESS) {
             unsigned long long val2;
+            assert(key == NULL);
 
             str = endptr + 1;
             if (parse_uint_full(str, &val2, 0) == 0 &&
@@ -470,6 +535,7 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
                (ov->list_mode == LM_NONE) ? "a uint64 value" :
                                             "a uint64 value or range");
+    g_free(key);
 }
 
 
@@ -480,8 +546,9 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     const QemuOpt *opt;
     int64_t val;
     char *endptr;
+    char *key = NULL;
 
-    opt = lookup_scalar(ov, name, errp);
+    opt = lookup_scalar(ov, name, &key, errp);
     if (!opt) {
         return;
     }
@@ -491,11 +558,13 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     if (val < 0 || *endptr) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
                    "a size value representible as a non-negative int64");
+        g_free(key);
         return;
     }
 
     *obj = val;
-    processed(ov, name);
+    processed(ov, key);
+    g_free(key);
 }
 
 
@@ -506,7 +575,7 @@ opts_optional(Visitor *v, bool *present, const char *name, Error **errp)
 
     /* we only support a single mandatory scalar field in a list node */
     assert(ov->list_mode == LM_NONE);
-    *present = (lookup_distinct(ov, name, NULL) != NULL);
+    *present = (lookup_distinct(ov, name, NULL, NULL) != NULL);
 }
 
 
@@ -517,6 +586,8 @@ opts_visitor_new(const QemuOpts *opts)
 
     ov = g_malloc0(sizeof *ov);
 
+    ov->nested_names = g_queue_new();
+
     ov->visitor.start_struct = &opts_start_struct;
     ov->visitor.end_struct   = &opts_end_struct;
 
@@ -560,6 +631,7 @@ opts_visitor_cleanup(OptsVisitor *ov)
     if (ov->unprocessed_opts != NULL) {
         g_hash_table_destroy(ov->unprocessed_opts);
     }
+    g_queue_free(ov->nested_names);
     g_free(ov->fake_id_opt);
     g_free(ov);
 }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index c7eaa86..a818eff 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -81,6 +81,11 @@
 { 'command': 'user_def_cmd3', 'data': {'a': 'int', '*b': 'int' },
   'returns': 'int' }
 
+# For testing hierarchy support in opts-visitor
+{ 'struct': 'UserDefOptionsSub',
+  'data': {
+    '*nint': 'int' } }
+
 # For testing integer range flattening in opts-visitor. The following schema
 # corresponds to the option format:
 #
@@ -94,7 +99,9 @@
     '*u64' : [ 'uint64' ],
     '*u16' : [ 'uint16' ],
     '*i64x':   'int'     ,
-    '*u64x':   'uint64'  } }
+    '*u64x':   'uint64'  ,
+    'sub0':    'UserDefOptionsSub',
+    'sub1':    'UserDefOptionsSub' } }
 
 # testing event
 { 'struct': 'EventStructOne',
diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index 1c753d9..4393266 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -178,6 +178,34 @@ expect_u64_max(OptsVisitorFixture *f, gconstpointer test_data)
     g_assert(f->userdef->u64->value == UINT64_MAX);
 }
 
+static void
+expect_both(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    expect_ok(f, test_data);
+    g_assert(f->userdef->sub0->has_nint);
+    g_assert(f->userdef->sub0->nint == 13);
+    g_assert(f->userdef->sub1->has_nint);
+    g_assert(f->userdef->sub1->nint == 17);
+}
+
+static void
+expect_sub0(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    expect_ok(f, test_data);
+    g_assert(f->userdef->sub0->has_nint);
+    g_assert(f->userdef->sub0->nint == 13);
+    g_assert(!f->userdef->sub1->has_nint);
+}
+
+static void
+expect_sub1(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    expect_ok(f, test_data);
+    g_assert(!f->userdef->sub0->has_nint);
+    g_assert(f->userdef->sub1->has_nint);
+    g_assert(f->userdef->sub1->nint == 13);
+}
+
 /* test cases */
 
 int
@@ -271,6 +299,12 @@ main(int argc, char **argv)
     add_test("/visitor/opts/i64/range/2big/full", &expect_fail,
              "i64=-0x8000000000000000-0x7fffffffffffffff");
 
+    /* Test nested structs support */
+    add_test("/visitor/opts/nested/unqualified", &expect_fail, "nint=13");
+    add_test("/visitor/opts/nested/both",        &expect_both,
+             "sub0.nint=13,sub1.nint=17");
+    add_test("/visitor/opts/nested/sub0",        &expect_sub0, "sub0.nint=13");
+    add_test("/visitor/opts/nested/sub1",        &expect_sub1, "sub1.nint=13");
     g_test_run();
     return 0;
 }
-- 
2.4.3

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

* [Qemu-devel] [PATCH 5/5] opts: produce valid command line in qemu_opts_print
  2015-06-23 13:32 [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches Kővágó, Zoltán
                   ` (3 preceding siblings ...)
  2015-06-23 13:33 ` [Qemu-devel] [PATCH 4/5] qapi: support nested structs in OptsVisitor Kővágó, Zoltán
@ 2015-06-23 13:33 ` Kővágó, Zoltán
  2015-07-02 17:58   ` Markus Armbruster
  2015-06-25  6:16 ` [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches Gerd Hoffmann
  5 siblings, 1 reply; 19+ messages in thread
From: Kővágó, Zoltán @ 2015-06-23 13:33 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, open list:Block layer core, Markus Armbruster,
	Stefan Hajnoczi

This will let us print options in a format that the user would actually
write it on the command line (foo=bar,baz=asd,etc=def), without
prepending a spurious comma at the beginning of the list, or quoting
values unnecessarily.  This patch provides the following changes:
* write and id=, if the option has an id
* do not print separator before the first element
* do not quote string arguments which only contains letters or numbers
* properly escape commas (,) for QEMU, apostrophe (') for shell

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>

---

Chages from v1 -audiodev patch:
* print id=
* proper value escaping (apostrophe and comma)
* renamed d_sep -> separator


 block.c            |  2 +-
 util/qemu-option.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index e2e33fd..a18af00 100644
--- a/block.c
+++ b/block.c
@@ -3825,7 +3825,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
     }
 
     if (!quiet) {
-        printf("Formatting '%s', fmt=%s", filename, fmt);
+        printf("Formatting '%s', fmt=%s ", filename, fmt);
         qemu_opts_print(opts, " ");
         puts("");
     }
diff --git a/util/qemu-option.c b/util/qemu-option.c
index efe9d27..db60ac9 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -730,14 +730,53 @@ void qemu_opts_del(QemuOpts *opts)
     g_free(opts);
 }
 
-void qemu_opts_print(QemuOpts *opts, const char *sep)
+/* print value properly escaping it for the shell (at least for bash) */
+static void escaped_print(const char *value)
+{
+    const char *ptr;
+    bool need_quote = false;
+
+    for (ptr = value; *ptr; ++ptr) {
+        if (!qemu_isalnum(*ptr)) {
+            need_quote = true;
+            break;
+        }
+    }
+
+    if (need_quote) {
+        putchar('\'');
+        for (ptr = value; *ptr; ++ptr) {
+            if (*ptr == '\'') {
+                printf("'\\''");
+            } else if (*ptr == ',') {
+                printf(",,");
+            } else {
+                putchar(*ptr);
+            }
+        }
+        putchar('\'');
+    } else {
+        printf("%s", value);
+    }
+}
+
+void qemu_opts_print(QemuOpts *opts, const char *separator)
 {
     QemuOpt *opt;
     QemuOptDesc *desc = opts->list->desc;
+    const char *sep = "";
+
+    if (opts->id) {
+        printf("id=");
+        escaped_print(opts->id);
+        sep = separator;
+    }
 
     if (desc[0].name == NULL) {
         QTAILQ_FOREACH(opt, &opts->head, next) {
-            printf("%s%s=\"%s\"", sep, opt->name, opt->str);
+            printf("%s%s=", sep, opt->name);
+            escaped_print(opt->str);
+            sep = separator;
         }
         return;
     }
@@ -750,13 +789,15 @@ void qemu_opts_print(QemuOpts *opts, const char *sep)
             continue;
         }
         if (desc->type == QEMU_OPT_STRING) {
-            printf("%s%s='%s'", sep, desc->name, value);
+            printf("%s%s=", sep, desc->name);
+            escaped_print(value);
         } else if ((desc->type == QEMU_OPT_SIZE ||
                     desc->type == QEMU_OPT_NUMBER) && opt) {
             printf("%s%s=%" PRId64, sep, desc->name, opt->value.uint);
         } else {
             printf("%s%s=%s", sep, desc->name, value);
         }
+        sep = separator;
     }
 }
 
-- 
2.4.3

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

* Re: [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches
  2015-06-23 13:32 [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches Kővágó, Zoltán
                   ` (4 preceding siblings ...)
  2015-06-23 13:33 ` [Qemu-devel] [PATCH 5/5] opts: produce valid command line in qemu_opts_print Kővágó, Zoltán
@ 2015-06-25  6:16 ` Gerd Hoffmann
  2015-06-25  7:57   ` Markus Armbruster
  2015-07-02 18:00   ` Markus Armbruster
  5 siblings, 2 replies; 19+ messages in thread
From: Gerd Hoffmann @ 2015-06-25  6:16 UTC (permalink / raw)
  To: Kővágó, Zoltán
  Cc: Eduardo Habkost, qemu-devel, Stefan Hajnoczi, Markus Armbruster

On Di, 2015-06-23 at 15:32 +0200, Kővágó, Zoltán wrote:
> I've cherry-picked the qapi related parts from my previous -audiodev
> patch series, we can hopefully concentrate on one thing at a time.  The
> most important changes in this patch series are the flattening of the
> Netdev structures.  This way we can add proper nested structure support
> into OptsVisitor, without requiring backward-compatibility hacks.

Applies and builds fine, no obvious regressions in testing.

Tested-by: Gerd Hoffmann <kraxel@redhat.com>

Getting this merged before hard freeze would be great ...

Any takers?  Markus?

thanks,
  Gerd

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

* Re: [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches
  2015-06-25  6:16 ` [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches Gerd Hoffmann
@ 2015-06-25  7:57   ` Markus Armbruster
  2015-06-25  8:53     ` Thomas Huth
  2015-06-25 14:01     ` Gerd Hoffmann
  2015-07-02 18:00   ` Markus Armbruster
  1 sibling, 2 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-06-25  7:57 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel, Eduardo Habkost, Stefan Hajnoczi, dirty.ice.hu

Gerd Hoffmann <kraxel@redhat.com> writes:

> On Di, 2015-06-23 at 15:32 +0200, Kővágó, Zoltán wrote:
>> I've cherry-picked the qapi related parts from my previous -audiodev
>> patch series, we can hopefully concentrate on one thing at a time.  The
>> most important changes in this patch series are the flattening of the
>> Netdev structures.  This way we can add proper nested structure support
>> into OptsVisitor, without requiring backward-compatibility hacks.
>
> Applies and builds fine, no obvious regressions in testing.
>
> Tested-by: Gerd Hoffmann <kraxel@redhat.com>
>
> Getting this merged before hard freeze would be great ...
>
> Any takers?  Markus?

I'll *try* to make time for it.  When's the hard freeze again?

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

* Re: [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches
  2015-06-25  7:57   ` Markus Armbruster
@ 2015-06-25  8:53     ` Thomas Huth
  2015-06-25 14:01     ` Gerd Hoffmann
  1 sibling, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2015-06-25  8:53 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel

On Thu, 25 Jun 2015 09:57:03 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
> > On Di, 2015-06-23 at 15:32 +0200, Kővágó, Zoltán wrote:
> >> I've cherry-picked the qapi related parts from my previous -audiodev
> >> patch series, we can hopefully concentrate on one thing at a time.  The
> >> most important changes in this patch series are the flattening of the
> >> Netdev structures.  This way we can add proper nested structure support
> >> into OptsVisitor, without requiring backward-compatibility hacks.
> >
> > Applies and builds fine, no obvious regressions in testing.
> >
> > Tested-by: Gerd Hoffmann <kraxel@redhat.com>
> >
> > Getting this merged before hard freeze would be great ...
> >
> > Any takers?  Markus?
> 
> I'll *try* to make time for it.  When's the hard freeze again?

7th of July according to: http://qemu-project.org/Planning/2.4

 Thomas

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

* Re: [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches
  2015-06-25  7:57   ` Markus Armbruster
  2015-06-25  8:53     ` Thomas Huth
@ 2015-06-25 14:01     ` Gerd Hoffmann
  1 sibling, 0 replies; 19+ messages in thread
From: Gerd Hoffmann @ 2015-06-25 14:01 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, Eduardo Habkost, Stefan Hajnoczi, dirty.ice.hu

On Do, 2015-06-25 at 09:57 +0200, Markus Armbruster wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
> > On Di, 2015-06-23 at 15:32 +0200, Kővágó, Zoltán wrote:
> >> I've cherry-picked the qapi related parts from my previous -audiodev
> >> patch series, we can hopefully concentrate on one thing at a time.  The
> >> most important changes in this patch series are the flattening of the
> >> Netdev structures.  This way we can add proper nested structure support
> >> into OptsVisitor, without requiring backward-compatibility hacks.
> >
> > Applies and builds fine, no obvious regressions in testing.
> >
> > Tested-by: Gerd Hoffmann <kraxel@redhat.com>
> >
> > Getting this merged before hard freeze would be great ...
> >
> > Any takers?  Markus?
> 
> I'll *try* to make time for it.  When's the hard freeze again?

IIRC almost two weeks from now, let me check ...

Hmm, wiki.qemu.org seems to be down ATM.

cheers,
  Gerd

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

* Re: [Qemu-devel] [PATCH 3/5] qapi: change Netdev and NetLegacy into a flat union
  2015-06-23 13:32 ` [Qemu-devel] [PATCH 3/5] qapi: change Netdev and NetLegacy " Kővágó, Zoltán
@ 2015-07-02 17:17   ` Markus Armbruster
  0 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-07-02 17:17 UTC (permalink / raw)
  To: Kővágó, Zoltán
  Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, qemu-devel,
	Vincenzo Maffione, Luiz Capitulino, Max Filippov, Gerd Hoffmann,
	Dmitry Fleytman, Edgar E. Iglesias, Rob Herring, Alexander Graf,
	Scott Feldman, Jiri Pirko, Jan Kiszka, Stefan Hajnoczi,
	Giuseppe Lettieri, Luigi Rizzo, David Gibson, Peter Crosthwaite,
	Michael Walle, open list:sPAPR pseries

"Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:

> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
[...]
> diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
> index a3b1314..72e2f8f 100644
> --- a/hw/arm/musicpal.c
> +++ b/hw/arm/musicpal.c
> @@ -379,7 +379,7 @@ static void eth_cleanup(NetClientState *nc)
>  }
>  
>  static NetClientInfo net_mv88w8618_info = {
> -    .type = NET_CLIENT_OPTIONS_KIND_NIC,
> +    .type = NET_CLIENT_DRIVER_NIC,
>      .size = sizeof(NICState),
>      .can_receive = eth_can_receive,
>      .receive = eth_receive,
[Many more renames snipped, mind-numbing, hope it's nothing but renames...]
> diff --git a/net/clients.h b/net/clients.h
> index d47530e..2aac0ee 100644
> --- a/net/clients.h
> +++ b/net/clients.h
> @@ -27,39 +27,39 @@
>  #include "net/net.h"
>  #include "qapi-types.h"
>  
> -int net_init_dump(const NetClientOptions *opts, const char *name,
> +int net_init_dump(const void *opts, const char *name,
>                    NetClientState *peer, Error **errp);

Why do we need to bypass the type system now?

Hmm, the reason is the conversion to flat unions loses type
NetClientOptions, member of both Netdev and NetLegacy, because the type
gets inlined into both containing types.  More below.

>  
>  #ifdef CONFIG_SLIRP
> -int net_init_slirp(const NetClientOptions *opts, const char *name,
> +int net_init_slirp(const void *opts, const char *name,
>                     NetClientState *peer, Error **errp);
>  #endif
>  
> -int net_init_hubport(const NetClientOptions *opts, const char *name,
> +int net_init_hubport(const void *opts, const char *name,
>                       NetClientState *peer, Error **errp);
>  
> -int net_init_socket(const NetClientOptions *opts, const char *name,
> +int net_init_socket(const void *opts, const char *name,
>                      NetClientState *peer, Error **errp);
>  
> -int net_init_tap(const NetClientOptions *opts, const char *name,
> +int net_init_tap(const void *opts, const char *name,
>                   NetClientState *peer, Error **errp);
>  
> -int net_init_bridge(const NetClientOptions *opts, const char *name,
> +int net_init_bridge(const void *opts, const char *name,
>                      NetClientState *peer, Error **errp);
>  
> -int net_init_l2tpv3(const NetClientOptions *opts, const char *name,
> +int net_init_l2tpv3(const void *opts, const char *name,
>                      NetClientState *peer, Error **errp);
>  #ifdef CONFIG_VDE
> -int net_init_vde(const NetClientOptions *opts, const char *name,
> +int net_init_vde(const void *opts, const char *name,
>                   NetClientState *peer, Error **errp);
>  #endif
>  
>  #ifdef CONFIG_NETMAP
> -int net_init_netmap(const NetClientOptions *opts, const char *name,
> +int net_init_netmap(const void *opts, const char *name,
>                      NetClientState *peer, Error **errp);
>  #endif
>  
> -int net_init_vhost_user(const NetClientOptions *opts, const char *name,
> +int net_init_vhost_user(const void *opts, const char *name,
>                          NetClientState *peer, Error **errp);
>  
>  #endif /* QEMU_NET_CLIENTS_H */
> diff --git a/net/dump.c b/net/dump.c
> index 02c8064..6aca19d 100644
> --- a/net/dump.c
> +++ b/net/dump.c
> @@ -94,7 +94,7 @@ static void dump_cleanup(NetClientState *nc)
>  }
>  
>  static NetClientInfo net_dump_info = {
> -    .type = NET_CLIENT_OPTIONS_KIND_DUMP,
> +    .type = NET_CLIENT_DRIVER_DUMP,
>      .size = sizeof(DumpState),
>      .receive = dump_receive,
>      .cleanup = dump_cleanup,
> @@ -146,16 +146,13 @@ static int net_dump_init(NetClientState *peer, const char *device,
>      return 0;
>  }
>  
> -int net_init_dump(const NetClientOptions *opts, const char *name,
> +int net_init_dump(const void *opts, const char *name,
>                    NetClientState *peer, Error **errp)
>  {
>      int len;
>      const char *file;
>      char def_file[128];
> -    const NetdevDumpOptions *dump;
> -
> -    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP);
> -    dump = opts->dump;
> +    const NetdevDumpOptions *dump = opts;
>  
>      assert(peer);
>  

Before the patch: we pass tagged union NetClientOptions to the tag's
callback, and fetch the tag's union member.

After: we pass the union member directly.  Because all callbacks have to
have the same type, we need one that can hold all the union members.
Since they're all pointers, void * works.

A union of all members would do as well.  The QAPI code generator
actually generates it, but gives it no name.  Fixable.

Further down, I challenge having separate types Netdev and Netlegacy.
If we used the same one for both, we could pass it to the callbacks, I
think.

[More of the same...]
> diff --git a/net/net.c b/net/net.c
> index cc36c7b..5f230a5 100644
> --- a/net/net.c
> +++ b/net/net.c
[...]
> @@ -878,32 +875,32 @@ static int net_init_nic(const NetClientOptions *opts, const char *name,
>  }
>  
>  
> -static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
> -    const NetClientOptions *opts,
> +static int (* const net_client_init_fun[NET_CLIENT_DRIVER_MAX])(
> +    const void *opts,
>      const char *name,
>      NetClientState *peer, Error **errp) = {
> -        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
> +        [NET_CLIENT_DRIVER_NIC]       = net_init_nic,
>  #ifdef CONFIG_SLIRP
> -        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
> +        [NET_CLIENT_DRIVER_USER]      = net_init_slirp,
>  #endif
> -        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
> -        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
> +        [NET_CLIENT_DRIVER_TAP]       = net_init_tap,
> +        [NET_CLIENT_DRIVER_SOCKET]    = net_init_socket,
>  #ifdef CONFIG_VDE
> -        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
> +        [NET_CLIENT_DRIVER_VDE]       = net_init_vde,
>  #endif
>  #ifdef CONFIG_NETMAP
> -        [NET_CLIENT_OPTIONS_KIND_NETMAP]    = net_init_netmap,
> +        [NET_CLIENT_DRIVER_NETMAP]    = net_init_netmap,
>  #endif
> -        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
> +        [NET_CLIENT_DRIVER_DUMP]      = net_init_dump,
>  #ifdef CONFIG_NET_BRIDGE
> -        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
> +        [NET_CLIENT_DRIVER_BRIDGE]    = net_init_bridge,
>  #endif
> -        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
> +        [NET_CLIENT_DRIVER_HUBPORT]   = net_init_hubport,
>  #ifdef CONFIG_VHOST_NET_USED
> -        [NET_CLIENT_OPTIONS_KIND_VHOST_USER] = net_init_vhost_user,
> +        [NET_CLIENT_DRIVER_VHOST_USER] = net_init_vhost_user,
>  #endif
>  #ifdef CONFIG_L2TPV3
> -        [NET_CLIENT_OPTIONS_KIND_L2TPV3]    = net_init_l2tpv3,
> +        [NET_CLIENT_DRIVER_L2TPV3]    = net_init_l2tpv3,
>  #endif
>  };
>  
> @@ -914,35 +911,37 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>          const Netdev    *netdev;
>          const NetLegacy *net;
>      } u;
> -    const NetClientOptions *opts;
> +    NetClientDriver kind;
> +    const void *opts;
>      const char *name;
>  
>      if (is_netdev) {
>          u.netdev = object;
> -        opts = u.netdev->opts;
> +        kind = u.netdev->kind;
> +        opts = u.netdev->data;
>          name = u.netdev->id;
>  
> -        switch (opts->kind) {
> +        switch (kind) {
>  #ifdef CONFIG_SLIRP
> -        case NET_CLIENT_OPTIONS_KIND_USER:
> +        case NET_CLIENT_DRIVER_USER:
>  #endif
> -        case NET_CLIENT_OPTIONS_KIND_TAP:
> -        case NET_CLIENT_OPTIONS_KIND_SOCKET:
> +        case NET_CLIENT_DRIVER_TAP:
> +        case NET_CLIENT_DRIVER_SOCKET:
>  #ifdef CONFIG_VDE
> -        case NET_CLIENT_OPTIONS_KIND_VDE:
> +        case NET_CLIENT_DRIVER_VDE:
>  #endif
>  #ifdef CONFIG_NETMAP
> -        case NET_CLIENT_OPTIONS_KIND_NETMAP:
> +        case NET_CLIENT_DRIVER_NETMAP:
>  #endif
>  #ifdef CONFIG_NET_BRIDGE
> -        case NET_CLIENT_OPTIONS_KIND_BRIDGE:
> +        case NET_CLIENT_DRIVER_BRIDGE:
>  #endif
> -        case NET_CLIENT_OPTIONS_KIND_HUBPORT:
> +        case NET_CLIENT_DRIVER_HUBPORT:
>  #ifdef CONFIG_VHOST_NET_USED
> -        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> +        case NET_CLIENT_DRIVER_VHOST_USER:
>  #endif
>  #ifdef CONFIG_L2TPV3
> -        case NET_CLIENT_OPTIONS_KIND_L2TPV3:
> +        case NET_CLIENT_DRIVER_L2TPV3:
>  #endif
>              break;
>  
> @@ -953,8 +952,9 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>          }
>      } else {
>          u.net = object;
> -        opts = u.net->opts;
> -        if (opts->kind == NET_CLIENT_OPTIONS_KIND_HUBPORT) {
> +        kind = u.net->kind;
> +        opts = u.net->data;
> +        if (kind == NET_CLIENT_DRIVER_HUBPORT) {
>              error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
>                         "a net type");
>              return -1;
> @@ -963,22 +963,22 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>          name = u.net->has_id ? u.net->id : u.net->name;
>      }
>  
> -    if (net_client_init_fun[opts->kind]) {
> +    if (net_client_init_fun[kind]) {
>          NetClientState *peer = NULL;
>  
>          /* Do not add to a vlan if it's a -netdev or a nic with a netdev=
>           * parameter. */
>          if (!is_netdev &&
> -            (opts->kind != NET_CLIENT_OPTIONS_KIND_NIC ||
> -             !opts->nic->has_netdev)) {
> +            (kind != NET_CLIENT_DRIVER_NIC ||
> +             !((NetLegacyNicOptions *) opts)->has_netdev)) {
>              peer = net_hub_add_port(u.net->has_vlan ? u.net->vlan : 0, NULL);
>          }
>  
> -        if (net_client_init_fun[opts->kind](opts, name, peer, errp) < 0) {
> +        if (net_client_init_fun[kind](opts, name, peer, errp) < 0) {
>              /* FIXME drop when all init functions store an Error */
>              if (errp && !*errp) {
>                  error_setg(errp, QERR_DEVICE_INIT_FAILED,
> -                           NetClientOptionsKind_lookup[opts->kind]);
> +                           NetClientDriver_lookup[kind]);
>              }
>              return -1;
>          }
[...]
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 7ebf99e..2a3cfe3 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2475,34 +2475,20 @@
>      '*queues':        'uint32' } }
>  
>  ##
> -# @NetClientOptions
> +# @NetClientDriver
>  #
> -# A discriminated record of network device traits.
> -#
> -# Since 1.2
> -#
> -# 'l2tpv3' - since 2.1
> +# Possible netdev drivers.
>  #
> +# Since 2.4
>  ##
> -{ 'union': 'NetClientOptions',
> -  'data': {
> -    'none':     'NetdevNoneOptions',
> -    'nic':      'NetLegacyNicOptions',
> -    'user':     'NetdevUserOptions',
> -    'tap':      'NetdevTapOptions',
> -    'l2tpv3':   'NetdevL2TPv3Options',
> -    'socket':   'NetdevSocketOptions',
> -    'vde':      'NetdevVdeOptions',
> -    'dump':     'NetdevDumpOptions',
> -    'bridge':   'NetdevBridgeOptions',
> -    'hubport':  'NetdevHubPortOptions',
> -    'netmap':   'NetdevNetmapOptions',
> -    'vhost-user': 'NetdevVhostUserOptions' } }
> +{ 'enum': 'NetClientDriver',
> +  'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde', 'dump',
> +            'bridge', 'hubport', 'netmap', 'vhost-user' ] }

Diff is confusing around here.  You could add your new stuff after the
changed stuff, where it won't upset diff, and move it to where you want
it in a separate patch.

If we could name the enum NetClientOptionsKind first, then rename it,
this patch would split into an interesting and a mechanical part.
Unfortunately, the qapi code generators don't let us.  Meh.

We could turn that error into a warning, though.

Anyway, this hunk results in:

   ##
   # @NetClientDriver
   #
   # Possible netdev drivers.
   #
   # Since 2.4
   ##
   { 'enum': 'NetClientDriver',
     'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde', 'dump',
               'bridge', 'hubport', 'netmap', 'vhost-user' ] }

I'd prefer "Available netdev drivers."

>  
>  ##
> -# @NetLegacy
> +# @NetLegacyBase
>  #
> -# Captures the configuration of a network device; legacy.
> +# Captures the common configuration of a network device; legacy.
>  #
>  # @vlan: #optional vlan number
>  #
> @@ -2512,30 +2498,87 @@
>  #
>  # @opts: device type specific properties (legacy)
>  #
> -# Since 1.2
> +# @type: the netdev driver to use
> +#
> +# Since 2.4
>  ##
> -{ 'struct': 'NetLegacy',
> +{ 'struct': 'NetLegacyBase',
>    'data': {
>      '*vlan': 'int32',
>      '*id':   'str',
>      '*name': 'str',
> -    'opts':  'NetClientOptions' } }
> +    'type': 'NetClientDriver' } }

Results in:

   ##
   # @NetLegacyBase
   #
   # Captures the common configuration of a network device; legacy.
   #
   # @vlan: #optional vlan number
   #
   # @id: #optional identifier for monitor commands
   #
   # @name: #optional identifier for monitor commands, ignored if @id is present
   #
   # @opts: device type specific properties (legacy)
   #
   # @type: the netdev driver to use
   #
   # Since 2.4
   ##
   { 'struct': 'NetLegacy',
     'data': {
       '*vlan': 'int32',
       '*id':   'str',
       '*name': 'str',
       'type': 'NetClientDriver' } }

Documents nonexistent member @opts, oops.

>  
>  ##
> -# @Netdev
> +# @NetLegacy
>  #
> -# Captures the configuration of a network device.
> +# Captures the configuration of a network device; legacy.
> +#
> +# Since 1.2
> +#
> +# 'l2tpv3' - since 2.1
> +##
> +{ 'union': 'NetLegacy',
> +  'base': 'NetLegacyBase',
> +  'discriminator': 'type',
> +  'data': {
> +    'none':     'NetdevNoneOptions',
> +    'nic':      'NetLegacyNicOptions',
> +    'user':     'NetdevUserOptions',
> +    'tap':      'NetdevTapOptions',
> +    'l2tpv3':   'NetdevL2TPv3Options',
> +    'socket':   'NetdevSocketOptions',
> +    'vde':      'NetdevVdeOptions',
> +    'dump':     'NetdevDumpOptions',
> +    'bridge':   'NetdevBridgeOptions',
> +    'hubport':  'NetdevHubPortOptions',
> +    'netmap':   'NetdevNetmapOptions',
> +    'vhost-user': 'NetdevVhostUserOptions' } }

Results in:

   ##
   # @NetLegacy
   #
   # Captures the configuration of a network device; legacy.
   #
   # Since 1.2
   #
   # 'l2tpv3' - since 2.1
   ##
   { 'union': 'NetLegacy',
     'base': 'NetLegacyBase',
     'discriminator': 'type',
     'data': {
       'none':     'NetdevNoneOptions',
       'nic':      'NetLegacyNicOptions',
       'user':     'NetdevUserOptions',
       'tap':      'NetdevTapOptions',
       'l2tpv3':   'NetdevL2TPv3Options',
       'socket':   'NetdevSocketOptions',
       'vde':      'NetdevVdeOptions',
       'dump':     'NetdevDumpOptions',
       'bridge':   'NetdevBridgeOptions',
       'hubport':  'NetdevHubPortOptions',
       'netmap':   'NetdevNetmapOptions',
       'vhost-user': 'NetdevVhostUserOptions' } }

Changed from:

   ##
   # @NetLegacy
   #
   # Captures the configuration of a network device; legacy.
   #
   # @vlan: #optional vlan number
   #
   # @id: #optional identifier for monitor commands
   #
   # @name: #optional identifier for monitor commands, ignored if @id is present
   #
   # @opts: device type specific properties (legacy)
   #
   # Since 1.2
   ##
   { 'struct': 'NetLegacy',
     'data': {
       '*vlan': 'int32',
       '*id':   'str',
       '*name': 'str',
       'opts':  'NetClientOptions' } }

where NetClientOptions is

   ##
   # @NetClientOptions
   #
   # A discriminated record of network device traits.
   #
   # Since 1.2
   #
   # 'l2tpv3' - since 2.1
   #
   ##
   { 'union': 'NetClientOptions',
     'data': {
       'none':     'NetdevNoneOptions',
       'nic':      'NetLegacyNicOptions',
       'user':     'NetdevUserOptions',
       'tap':      'NetdevTapOptions',
       'l2tpv3':   'NetdevL2TPv3Options',
       'socket':   'NetdevSocketOptions',
       'vde':      'NetdevVdeOptions',
       'dump':     'NetdevDumpOptions',
       'bridge':   'NetdevBridgeOptions',
       'hubport':  'NetdevHubPortOptions',
       'netmap':   'NetdevNetmapOptions',
       'vhost-user': 'NetdevVhostUserOptions' } }

Faithful conversion.

Since the changed types aren't used by any command or event (yet), we
don't have to worry about backward compatibility.

> +
> +
> +##
> +# @NetdevBase
> +#
> +# Captures the common configuration of a network device.
>  #
>  # @id: identifier for monitor commands.
>  #
>  # @opts: device type specific properties

@opts doesn't exist.

>  #
> -# Since 1.2
> +# @type: the netdev driver to use
> +#
> +# Since 2.4
>  ##
> -{ 'struct': 'Netdev',
> +{ 'struct': 'NetdevBase',
>    'data': {
>      'id':   'str',
> -    'opts': 'NetClientOptions' } }
> +    'type': 'NetClientDriver' } }
> +
> +##
> +# @Netdev
> +#
> +# Captures the configuration of a network device.
> +#
> +# Since 1.2
> +#
> +# 'l2tpv3' - since 2.1
> +##
> +{ 'union': 'Netdev',
> +  'base': 'NetdevBase',
> +  'discriminator': 'type',
> +  'data': {
> +    'none':     'NetdevNoneOptions',
> +    'nic':      'NetLegacyNicOptions',
> +    'user':     'NetdevUserOptions',
> +    'tap':      'NetdevTapOptions',
> +    'l2tpv3':   'NetdevL2TPv3Options',
> +    'socket':   'NetdevSocketOptions',
> +    'vde':      'NetdevVdeOptions',
> +    'dump':     'NetdevDumpOptions',
> +    'bridge':   'NetdevBridgeOptions',
> +    'hubport':  'NetdevHubPortOptions',
> +    'netmap':   'NetdevNetmapOptions',
> +    'vhost-user': 'NetdevVhostUserOptions' } }
>  
>  ##
>  # @InetSocketAddress

Again, faithful conversion.

Netdev is identical to NetLegacy, except base is NetdevBase instead of
NetLegacyBase.

NetLegacyBase is NetdevBase plus optional members vlan and name.

Therefore, NetLegacy is Netdev plus optional members vlan and name.

Making the two separate types enables some compiler type checking.  Not
sure it's worth the trouble, but that's not your fault, you're only
converting stuff.

Flat unions carry a bit of notational overhead, but I hope to ease that
later on.

Overall, just a few nits, and a type system cheat I'd prefer to avoid.

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

* Re: [Qemu-devel] [PATCH 2/5] qapi: convert NumaOptions into a flat union
  2015-06-23 13:32 ` [Qemu-devel] [PATCH 2/5] qapi: convert NumaOptions into a flat union Kővágó, Zoltán
@ 2015-07-02 17:25   ` Markus Armbruster
  0 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-07-02 17:25 UTC (permalink / raw)
  To: Kővágó, Zoltán
  Cc: qemu-devel, Stefan Hajnoczi, Eduardo Habkost

"Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:

> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> ---
>  numa.c           |  2 +-
>  qapi-schema.json | 47 ++++++++++++++++++++++++++++++++++++-----------
>  2 files changed, 37 insertions(+), 12 deletions(-)
>
> diff --git a/numa.c b/numa.c
> index 91fc6c1..8b0755d 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -140,7 +140,7 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
>      }
>  
>      switch (object->kind) {
> -    case NUMA_OPTIONS_KIND_NODE:
> +    case NUMA_DRIVER_NODE:
>          numa_node_parse(object->node, opts, &err);
>          if (err) {
>              goto error;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 106008c..7ebf99e 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3537,17 +3537,6 @@
>    'data': { '*console':'int', 'events': [ 'InputEvent' ] } }
>  
>  ##
> -# @NumaOptions
> -#
> -# A discriminated record of NUMA options. (for OptsVisitor)
> -#
> -# Since 2.1
> -##
> -{ 'union': 'NumaOptions',
> -  'data': {
> -    'node': 'NumaNodeOptions' }}
> -
> -##
>  # @NumaNodeOptions
>  #
>  # Create a guest NUMA node. (for OptsVisitor)
> @@ -3574,6 +3563,42 @@
>     '*memdev': 'str' }}
>  
>  ##
> +# @NumaDriver
> +#
> +# List of possible numa drivers.
> +#
> +# Since: 2.4
> +##
> +{ 'enum': 'NumaDriver',
> +  'data': [ 'node' ] }
> +
> +##
> +# @NumaCommonOptions
> +#
> +# Common set of numa options.
> +#
> +# @type: the numa driver to use
> +#
> +# Since: 2.4
> +##
> +{ 'struct': 'NumaCommonOptions',
> +  'data': {
> +    'type': 'NumaDriver' } }
> +
> +##
> +# @NumaOptions
> +#
> +# A discriminated record of NUMA options. (for OptsVisitor)
> +#
> +# Since 2.1
> +##
> +{ 'union': 'NumaOptions',
> +  'base': 'NumaCommonOptions',
> +  'discriminator': 'type',
> +  'data': {
> +    'node': 'NumaNodeOptions' }}
> +
> +##
>  # @HostMemPolicy
>  #
>  # Host memory policy types

Here, the notational overhead of flat unions borders on the ridiculous.

Anyway, faithful conversion.

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

* Re: [Qemu-devel] [PATCH 4/5] qapi: support nested structs in OptsVisitor
  2015-06-23 13:33 ` [Qemu-devel] [PATCH 4/5] qapi: support nested structs in OptsVisitor Kővágó, Zoltán
@ 2015-07-02 17:34   ` Markus Armbruster
  2015-07-03 11:10     ` Kővágó Zoltán
  0 siblings, 1 reply; 19+ messages in thread
From: Markus Armbruster @ 2015-07-02 17:34 UTC (permalink / raw)
  To: Kővágó, Zoltán
  Cc: qemu-devel, Stefan Hajnoczi, Michael Roth

"Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:

> The current OptsVisitor flattens the whole structure, if there are same
> named fields under different paths the current visitor can't cope with
> them (it'll just set the first field, leaving the others unspecified (if
> they're optional) or erroring out (if they're required).
>
> This patch add support for it, by always requiring a complete path in
> case of nested structs.  Fields in the path are separated by dots,
> similar to C structs (without pointers), like `foo.bar'.
>
> You must provide a full path even in non-ambigous cases.  The previous
> two commits hopefully ensures that this change doesn't create backward
> compatibility problems.
>
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
>
> ---
>
> Rationale for this commit: see these threads
> http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg04189.html
> http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg04186.html

So, despite your flattening work, we still need to parse option strings
into nested QAPI types?

Can you give examples of the need for such nested QAPI types in
configuration?  I may have seen them already in your audio patches, but
I think having some right here would be useful.

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

* Re: [Qemu-devel] [PATCH 5/5] opts: produce valid command line in qemu_opts_print
  2015-06-23 13:33 ` [Qemu-devel] [PATCH 5/5] opts: produce valid command line in qemu_opts_print Kővágó, Zoltán
@ 2015-07-02 17:58   ` Markus Armbruster
  0 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-07-02 17:58 UTC (permalink / raw)
  To: Kővágó, Zoltán
  Cc: Kevin Wolf, Stefan Hajnoczi, qemu-devel, open list:Block layer core

"Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:

> This will let us print options in a format that the user would actually
> write it on the command line (foo=bar,baz=asd,etc=def), without
> prepending a spurious comma at the beginning of the list, or quoting
> values unnecessarily.  This patch provides the following changes:
> * write and id=, if the option has an id
> * do not print separator before the first element
> * do not quote string arguments which only contains letters or numbers
> * properly escape commas (,) for QEMU, apostrophe (') for shell
>
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
>
> ---
>
> Chages from v1 -audiodev patch:
> * print id=
> * proper value escaping (apostrophe and comma)
> * renamed d_sep -> separator
>
>
>  block.c            |  2 +-
>  util/qemu-option.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/block.c b/block.c
> index e2e33fd..a18af00 100644
> --- a/block.c
> +++ b/block.c
> @@ -3825,7 +3825,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
>      }
>  
>      if (!quiet) {
> -        printf("Formatting '%s', fmt=%s", filename, fmt);
> +        printf("Formatting '%s', fmt=%s ", filename, fmt);
>          qemu_opts_print(opts, " ");
>          puts("");
>      }
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index efe9d27..db60ac9 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -730,14 +730,53 @@ void qemu_opts_del(QemuOpts *opts)
>      g_free(opts);
>  }
>  
> -void qemu_opts_print(QemuOpts *opts, const char *sep)
> +/* print value properly escaping it for the shell (at least for bash) */
> +static void escaped_print(const char *value)
> +{
> +    const char *ptr;
> +    bool need_quote = false;
> +
> +    for (ptr = value; *ptr; ++ptr) {
> +        if (!qemu_isalnum(*ptr)) {
> +            need_quote = true;
> +            break;
> +        }
> +    }

Rather eager to quote.  Shell wants you to quote

    |  &  ;  <  >  (  )  $ `  \  "  '  <space>  <tab>  <newline>

always, and

    *   ?   [   #   ˜   =   %

in some contexts.

In particular, there is no need to quote the common characters '.', '-'
and '_'.

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02

> +
> +    if (need_quote) {
> +        putchar('\'');
> +        for (ptr = value; *ptr; ++ptr) {
> +            if (*ptr == '\'') {
> +                printf("'\\''");

Won't achieve the stated purpose "for shell", because backslash escapes
do not work within single quotes.

You could try double quote, but things become even more complicated
then.

> +            } else if (*ptr == ',') {
> +                printf(",,");
> +            } else {
> +                putchar(*ptr);
> +            }

If we do backslash escapes, should we escape non-printing characters?

> +        }
> +        putchar('\'');
> +    } else {
> +        printf("%s", value);
> +    }
> +}
> +
> +void qemu_opts_print(QemuOpts *opts, const char *separator)
>  {
>      QemuOpt *opt;
>      QemuOptDesc *desc = opts->list->desc;
> +    const char *sep = "";
> +
> +    if (opts->id) {
> +        printf("id=");
> +        escaped_print(opts->id);

Since opts->id has passed id_wellformed(), it won't need quoting.
Your escaped_print() may quote it anyway, though.

> +        sep = separator;
> +    }
>  
>      if (desc[0].name == NULL) {
>          QTAILQ_FOREACH(opt, &opts->head, next) {
> -            printf("%s%s=\"%s\"", sep, opt->name, opt->str);
> +            printf("%s%s=", sep, opt->name);
> +            escaped_print(opt->str);
> +            sep = separator;
>          }
>          return;
>      }
> @@ -750,13 +789,15 @@ void qemu_opts_print(QemuOpts *opts, const char *sep)
>              continue;
>          }
>          if (desc->type == QEMU_OPT_STRING) {
> -            printf("%s%s='%s'", sep, desc->name, value);
> +            printf("%s%s=", sep, desc->name);
> +            escaped_print(value);
>          } else if ((desc->type == QEMU_OPT_SIZE ||
>                      desc->type == QEMU_OPT_NUMBER) && opt) {
>              printf("%s%s=%" PRId64, sep, desc->name, opt->value.uint);
>          } else {
>              printf("%s%s=%s", sep, desc->name, value);
>          }
> +        sep = separator;
>      }
>  }

Things I like about the patch:

* Making parameter sep a true separator

* Printing opts->id

* Escaping comma

I'm not sure the "quote for shell" feature is worth the bother.  If we
want it, then

    "foo=<bar>,msg=\"hello\",gnu=on"

would perhaps be nicer than

    foo="<bar>",msg="\"hello\"",gnu=on

but could be even more complicated to do.

If you want to pursue quoting for shell, I recommend to split this patch
into a part I like and a part we want to discuss :)

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

* Re: [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches
  2015-06-25  6:16 ` [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches Gerd Hoffmann
  2015-06-25  7:57   ` Markus Armbruster
@ 2015-07-02 18:00   ` Markus Armbruster
  2015-07-03  7:00     ` Gerd Hoffmann
  1 sibling, 1 reply; 19+ messages in thread
From: Markus Armbruster @ 2015-07-02 18:00 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel, Eduardo Habkost, Stefan Hajnoczi, dirty.ice.hu

Gerd Hoffmann <kraxel@redhat.com> writes:

> On Di, 2015-06-23 at 15:32 +0200, Kővágó, Zoltán wrote:
>> I've cherry-picked the qapi related parts from my previous -audiodev
>> patch series, we can hopefully concentrate on one thing at a time.  The
>> most important changes in this patch series are the flattening of the
>> Netdev structures.  This way we can add proper nested structure support
>> into OptsVisitor, without requiring backward-compatibility hacks.
>
> Applies and builds fine, no obvious regressions in testing.
>
> Tested-by: Gerd Hoffmann <kraxel@redhat.com>
>
> Getting this merged before hard freeze would be great ...
>
> Any takers?  Markus?

A first round of review is my best offer, I'm afraid: I'll be away the
next two weeks.

Any particular reason why we want this in 2.4?

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

* Re: [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches
  2015-07-02 18:00   ` Markus Armbruster
@ 2015-07-03  7:00     ` Gerd Hoffmann
  2015-07-03  9:09       ` Markus Armbruster
  0 siblings, 1 reply; 19+ messages in thread
From: Gerd Hoffmann @ 2015-07-03  7:00 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: dirty.ice.hu, qemu-devel, Stefan Hajnoczi, Eduardo Habkost

On Do, 2015-07-02 at 20:00 +0200, Markus Armbruster wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
> > On Di, 2015-06-23 at 15:32 +0200, Kővágó, Zoltán wrote:
> >> I've cherry-picked the qapi related parts from my previous -audiodev
> >> patch series, we can hopefully concentrate on one thing at a time.  The
> >> most important changes in this patch series are the flattening of the
> >> Netdev structures.  This way we can add proper nested structure support
> >> into OptsVisitor, without requiring backward-compatibility hacks.
> >
> > Applies and builds fine, no obvious regressions in testing.
> >
> > Tested-by: Gerd Hoffmann <kraxel@redhat.com>
> >
> > Getting this merged before hard freeze would be great ...
> >
> > Any takers?  Markus?
> 
> A first round of review is my best offer, I'm afraid: I'll be away the
> next two weeks.
> 
> Any particular reason why we want this in 2.4?

Looked easy as everybody agreed that flattening the qemu structs is a
good thing, and merging stuff helps keeping the out-of-tree patch queues
smaller ...

cheers,
  Gerd

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

* Re: [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches
  2015-07-03  7:00     ` Gerd Hoffmann
@ 2015-07-03  9:09       ` Markus Armbruster
  2015-07-03 10:04         ` Markus Armbruster
  0 siblings, 1 reply; 19+ messages in thread
From: Markus Armbruster @ 2015-07-03  9:09 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: Eduardo Habkost, qemu-devel, Stefan Hajnoczi, dirty.ice.hu

Gerd Hoffmann <kraxel@redhat.com> writes:

> On Do, 2015-07-02 at 20:00 +0200, Markus Armbruster wrote:
>> Gerd Hoffmann <kraxel@redhat.com> writes:
>> 
>> > On Di, 2015-06-23 at 15:32 +0200, Kővágó, Zoltán wrote:
>> >> I've cherry-picked the qapi related parts from my previous -audiodev
>> >> patch series, we can hopefully concentrate on one thing at a time.  The
>> >> most important changes in this patch series are the flattening of the
>> >> Netdev structures.  This way we can add proper nested structure support
>> >> into OptsVisitor, without requiring backward-compatibility hacks.
>> >
>> > Applies and builds fine, no obvious regressions in testing.
>> >
>> > Tested-by: Gerd Hoffmann <kraxel@redhat.com>
>> >
>> > Getting this merged before hard freeze would be great ...
>> >
>> > Any takers?  Markus?
>> 
>> A first round of review is my best offer, I'm afraid: I'll be away the
>> next two weeks.
>> 
>> Any particular reason why we want this in 2.4?
>
> Looked easy as everybody agreed that flattening the qemu structs is a
> good thing, and merging stuff helps keeping the out-of-tree patch queues
> smaller ...

Fair enough.

The series does three things, I think:

1. QAPI struct flattening [PATCH 2+3]

2. OptsVisitor nesting support [PATCH 1+4]

3. Nicer qemu_opts_print() [PATCH 5]

Regarding 1., I'm fine with the general approach.  In fact, proper
netdev_add qapification will need the flattening anyway (if it's
possible at all, but I'd like to try).  However, PATCH 3 needs a few
comment edits (could be done on commit), and I'd prefer to avoid the
type system cheat.  See my review for details.

Regarding 2., I have questions, and don't think we should rush it.

Regarding 3., I'm again fine with the general idea, but the "quoting for
shell" part doesn't look ready.  Take out that part if you want
something committed *now*.

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

* Re: [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches
  2015-07-03  9:09       ` Markus Armbruster
@ 2015-07-03 10:04         ` Markus Armbruster
  0 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-07-03 10:04 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Eduardo Habkost, Michael Tokarev, qemu-devel, Stefan Hajnoczi,
	dirty.ice.hu

Markus Armbruster <armbru@redhat.com> writes:

> Gerd Hoffmann <kraxel@redhat.com> writes:
>
>> On Do, 2015-07-02 at 20:00 +0200, Markus Armbruster wrote:
>>> Gerd Hoffmann <kraxel@redhat.com> writes:
>>> 
>>> > On Di, 2015-06-23 at 15:32 +0200, Kővágó, Zoltán wrote:
>>> >> I've cherry-picked the qapi related parts from my previous -audiodev
>>> >> patch series, we can hopefully concentrate on one thing at a time.  The
>>> >> most important changes in this patch series are the flattening of the
>>> >> Netdev structures.  This way we can add proper nested structure support
>>> >> into OptsVisitor, without requiring backward-compatibility hacks.
>>> >
>>> > Applies and builds fine, no obvious regressions in testing.
>>> >
>>> > Tested-by: Gerd Hoffmann <kraxel@redhat.com>
>>> >
>>> > Getting this merged before hard freeze would be great ...
>>> >
>>> > Any takers?  Markus?
>>> 
>>> A first round of review is my best offer, I'm afraid: I'll be away the
>>> next two weeks.
>>> 
>>> Any particular reason why we want this in 2.4?
>>
>> Looked easy as everybody agreed that flattening the qemu structs is a
>> good thing, and merging stuff helps keeping the out-of-tree patch queues
>> smaller ...
>
> Fair enough.
>
> The series does three things, I think:
>
> 1. QAPI struct flattening [PATCH 2+3]
>
> 2. OptsVisitor nesting support [PATCH 1+4]
>
> 3. Nicer qemu_opts_print() [PATCH 5]
>
> Regarding 1., I'm fine with the general approach.  In fact, proper
> netdev_add qapification will need the flattening anyway (if it's
> possible at all, but I'd like to try).  However, PATCH 3 needs a few
> comment edits (could be done on commit), and I'd prefer to avoid the
> type system cheat.  See my review for details.

I'll go offline tonight, back on 22nd.  I think PATCH 2 could go through
Eduardo (NUMA maintainer), and PATCH 3 through Stefan (net maintainer).

> Regarding 2., I have questions, and don't think we should rush it.
>
> Regarding 3., I'm again fine with the general idea, but the "quoting for
> shell" part doesn't look ready.  Take out that part if you want
> something committed *now*.

You could try -trivial for this one.

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

* Re: [Qemu-devel] [PATCH 4/5] qapi: support nested structs in OptsVisitor
  2015-07-02 17:34   ` Markus Armbruster
@ 2015-07-03 11:10     ` Kővágó Zoltán
  0 siblings, 0 replies; 19+ messages in thread
From: Kővágó Zoltán @ 2015-07-03 11:10 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, Stefan Hajnoczi, Michael Roth

2015-07-02 19:34 keltezéssel, Markus Armbruster írta:
> "Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:
>
>> The current OptsVisitor flattens the whole structure, if there are same
>> named fields under different paths the current visitor can't cope with
>> them (it'll just set the first field, leaving the others unspecified (if
>> they're optional) or erroring out (if they're required).
>>
>> This patch add support for it, by always requiring a complete path in
>> case of nested structs.  Fields in the path are separated by dots,
>> similar to C structs (without pointers), like `foo.bar'.
>>
>> You must provide a full path even in non-ambigous cases.  The previous
>> two commits hopefully ensures that this change doesn't create backward
>> compatibility problems.
>>
>> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
>>
>> ---
>>
>> Rationale for this commit: see these threads
>> http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg04189.html
>> http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg04186.html
>
> So, despite your flattening work, we still need to parse option strings
> into nested QAPI types?

The reason why this whole flattening stuff is done it's because 
otherwise this nested parsing patch breaks backward compatibility.

>
> Can you give examples of the need for such nested QAPI types in
> configuration?  I may have seen them already in your audio patches, but
> I think having some right here would be useful.

I'm afraid that only the audio patches contains code that actually use 
this new feature.  But that has a lot of configuration options that 
apply to both input and output, and having a shared struct that is 
available via `in.' and `out.' is better than having to manually 
duplicate all these settings under unique names in a single struct (like 
in_frequency, out_frequency, in_channels, out_channels, ...).

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

end of thread, other threads:[~2015-07-03 11:10 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-23 13:32 [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches Kővágó, Zoltán
2015-06-23 13:32 ` [Qemu-devel] [PATCH 1/5] qapi: support implicit structs in OptsVisitor Kővágó, Zoltán
2015-06-23 13:32 ` [Qemu-devel] [PATCH 2/5] qapi: convert NumaOptions into a flat union Kővágó, Zoltán
2015-07-02 17:25   ` Markus Armbruster
2015-06-23 13:32 ` [Qemu-devel] [PATCH 3/5] qapi: change Netdev and NetLegacy " Kővágó, Zoltán
2015-07-02 17:17   ` Markus Armbruster
2015-06-23 13:33 ` [Qemu-devel] [PATCH 4/5] qapi: support nested structs in OptsVisitor Kővágó, Zoltán
2015-07-02 17:34   ` Markus Armbruster
2015-07-03 11:10     ` Kővágó Zoltán
2015-06-23 13:33 ` [Qemu-devel] [PATCH 5/5] opts: produce valid command line in qemu_opts_print Kővágó, Zoltán
2015-07-02 17:58   ` Markus Armbruster
2015-06-25  6:16 ` [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches Gerd Hoffmann
2015-06-25  7:57   ` Markus Armbruster
2015-06-25  8:53     ` Thomas Huth
2015-06-25 14:01     ` Gerd Hoffmann
2015-07-02 18:00   ` Markus Armbruster
2015-07-03  7:00     ` Gerd Hoffmann
2015-07-03  9:09       ` Markus Armbruster
2015-07-03 10:04         ` Markus Armbruster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.