diff mbox series

[v5,04/12] qapi: net: introduce a way to bypass qemu_opts_parse_noisily()

Message ID 20220627154749.871943-5-lvivier@redhat.com
State New
Headers show
Series qapi: net: add unix socket type support to netdev backend | expand

Commit Message

Laurent Vivier June 27, 2022, 3:47 p.m. UTC
As qemu_opts_parse_noisily() flattens the QAPI structures ("type" field
of Netdev structure can collides with "type" field of SocketAddress),
we introduce a way to bypass qemu_opts_parse_noisily() and use directly
visit_type_Netdev() to parse the backend parameters.

More details from Markus:

qemu_init() passes the argument of -netdev, -nic, and -net to
net_client_parse().

net_client_parse() parses with qemu_opts_parse_noisily(), passing
QemuOptsList qemu_netdev_opts for -netdev, qemu_nic_opts for -nic, and
qemu_net_opts for -net.  Their desc[] are all empty, which means any
keys are accepted.  The result of the parse (a QemuOpts) is stored in
the QemuOptsList.

Note that QemuOpts is flat by design.  In some places, we layer non-flat
on top using dotted keys convention, but not here.

net_init_clients() iterates over the stored QemuOpts, and passes them to
net_init_netdev(), net_param_nic(), or net_init_client(), respectively.

These functions pass the QemuOpts to net_client_init().  They also do
other things with the QemuOpts, which we can ignore here.

net_client_init() uses the opts visitor to convert the (flat) QemOpts to
a (non-flat) QAPI object Netdev.  Netdev is also the argument of QMP
command netdev_add.

The opts visitor was an early attempt to support QAPI in
(QemuOpts-based) CLI.  It restricts QAPI types to a certain shape; see
commit eb7ee2cbeb "qapi: introduce OptsVisitor".

A more modern way to support QAPI is qobject_input_visitor_new_str().
It uses keyval_parse() instead of QemuOpts for KEY=VALUE,... syntax, and
it also supports JSON syntax.  The former isn't quite as expressive as
JSON, but it's a lot closer than QemuOpts + opts visitor.

This commit paves the way to use of the modern way instead.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 include/net/net.h |  2 ++
 net/net.c         | 57 +++++++++++++++++++++++++++++++++++++++++++++++
 softmmu/vl.c      |  6 ++++-
 3 files changed, 64 insertions(+), 1 deletion(-)

Comments

Markus Armbruster June 29, 2022, 1:21 p.m. UTC | #1
Laurent Vivier <lvivier@redhat.com> writes:

> As qemu_opts_parse_noisily() flattens the QAPI structures ("type" field
> of Netdev structure can collides with "type" field of SocketAddress),
> we introduce a way to bypass qemu_opts_parse_noisily() and use directly
> visit_type_Netdev() to parse the backend parameters.
>
> More details from Markus:
>
> qemu_init() passes the argument of -netdev, -nic, and -net to
> net_client_parse().
>
> net_client_parse() parses with qemu_opts_parse_noisily(), passing
> QemuOptsList qemu_netdev_opts for -netdev, qemu_nic_opts for -nic, and
> qemu_net_opts for -net.  Their desc[] are all empty, which means any
> keys are accepted.  The result of the parse (a QemuOpts) is stored in
> the QemuOptsList.
>
> Note that QemuOpts is flat by design.  In some places, we layer non-flat
> on top using dotted keys convention, but not here.
>
> net_init_clients() iterates over the stored QemuOpts, and passes them to
> net_init_netdev(), net_param_nic(), or net_init_client(), respectively.
>
> These functions pass the QemuOpts to net_client_init().  They also do
> other things with the QemuOpts, which we can ignore here.
>
> net_client_init() uses the opts visitor to convert the (flat) QemOpts to
> a (non-flat) QAPI object Netdev.  Netdev is also the argument of QMP
> command netdev_add.
>
> The opts visitor was an early attempt to support QAPI in
> (QemuOpts-based) CLI.  It restricts QAPI types to a certain shape; see
> commit eb7ee2cbeb "qapi: introduce OptsVisitor".
>
> A more modern way to support QAPI is qobject_input_visitor_new_str().
> It uses keyval_parse() instead of QemuOpts for KEY=VALUE,... syntax, and
> it also supports JSON syntax.  The former isn't quite as expressive as
> JSON, but it's a lot closer than QemuOpts + opts visitor.
>
> This commit paves the way to use of the modern way instead.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  include/net/net.h |  2 ++
>  net/net.c         | 57 +++++++++++++++++++++++++++++++++++++++++++++++
>  softmmu/vl.c      |  6 ++++-
>  3 files changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index e755254443ea..826e14a78734 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -214,6 +214,8 @@ extern NICInfo nd_table[MAX_NICS];
>  extern const char *host_net_devices[];
>  
>  /* from net.c */
> +bool netdev_is_modern(const char *optarg);
> +void netdev_parse_modern(const char *optarg);
>  void net_client_parse(QemuOptsList *opts_list, const char *str);
>  void show_netdevs(void);
>  void net_init_clients(void);
> diff --git a/net/net.c b/net/net.c
> index f056e8aebfb2..83300ea1b816 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -54,6 +54,7 @@
>  #include "net/colo-compare.h"
>  #include "net/filter.h"
>  #include "qapi/string-output-visitor.h"
> +#include "qapi/qobject-input-visitor.h"
>  
>  /* Net bridge is currently not supported for W32. */
>  #if !defined(_WIN32)
> @@ -63,6 +64,16 @@
>  static VMChangeStateEntry *net_change_state_entry;
>  static QTAILQ_HEAD(, NetClientState) net_clients;
>  
> +typedef struct NetdevQueueEntry {
> +    Netdev *nd;
> +    Location loc;
> +    QSIMPLEQ_ENTRY(NetdevQueueEntry) entry;
> +} NetdevQueueEntry;
> +
> +typedef QSIMPLEQ_HEAD(, NetdevQueueEntry) NetdevQueue;
> +
> +static NetdevQueue nd_queue = QSIMPLEQ_HEAD_INITIALIZER(nd_queue);
> +
>  /***********************************************************/
>  /* network device redirectors */
>  
> @@ -1562,6 +1573,20 @@ out:
>      return ret;
>  }
>  
> +static void netdev_init_modern(void)
> +{
> +    while (!QSIMPLEQ_EMPTY(&nd_queue)) {
> +        NetdevQueueEntry *nd = QSIMPLEQ_FIRST(&nd_queue);
> +
> +        QSIMPLEQ_REMOVE_HEAD(&nd_queue, entry);
> +        loc_push_restore(&nd->loc);
> +        net_client_init1(nd->nd, true, &error_fatal);
> +        loc_pop(&nd->loc);
> +        qapi_free_Netdev(nd->nd);
> +        g_free(nd);
> +    }
> +}
> +
>  void net_init_clients(void)
>  {
>      net_change_state_entry =
> @@ -1569,6 +1594,8 @@ void net_init_clients(void)
>  
>      QTAILQ_INIT(&net_clients);
>  
> +    netdev_init_modern();
> +
>      qemu_opts_foreach(qemu_find_opts("netdev"), net_init_netdev, NULL,
>                        &error_fatal);
>  
> @@ -1579,6 +1606,36 @@ void net_init_clients(void)
>                        &error_fatal);
>  }
>  
> +/*
> + * Does this -netdev argument use modern rather than traditional syntax?
> + * Modern syntax is to be parsed with netdev_parse_modern().
> + * Traditional syntax is to be parsed with net_client_parse().
> + */
> +bool netdev_is_modern(const char *optarg)
> +{
> +    return false;
> +}
> +
> +/*
> + * netdev_parse_modern() uses modern, more expressive syntax than
> + * net_client_parse(), but supports only the netdev option.

Suggest "the -netdev option".

> + * netdev_parse_modern() appends to @nd_queue, whereas net_client_parse()
> + * appends to @qemu_netdev_opts.
> + */
> +void netdev_parse_modern(const char *optarg)
> +{
> +    Visitor *v;
> +    NetdevQueueEntry *nd;
> +
> +    v = qobject_input_visitor_new_str(optarg, "type", &error_fatal);
> +    nd = g_new(NetdevQueueEntry, 1);
> +    visit_type_Netdev(v, NULL, &nd->nd, &error_fatal);
> +    visit_free(v);
> +    loc_save(&nd->loc);
> +
> +    QSIMPLEQ_INSERT_TAIL(&nd_queue, nd, entry);
> +}
> +
>  void net_client_parse(QemuOptsList *opts_list, const char *optarg)
>  {
>      if (!qemu_opts_parse_noisily(opts_list, optarg, true)) {
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index f6461363d608..f557e2d8cb8a 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2794,7 +2794,11 @@ void qemu_init(int argc, char **argv, char **envp)
>                  break;
>              case QEMU_OPTION_netdev:
>                  default_net = 0;
> -                net_client_parse(qemu_find_opts("netdev"), optarg);
> +                if (netdev_is_modern(optarg)) {
> +                    netdev_parse_modern(optarg);
> +                } else {
> +                    net_client_parse(qemu_find_opts("netdev"), optarg);
> +                }
>                  break;
>              case QEMU_OPTION_nic:
>                  default_net = 0;

Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff mbox series

Patch

diff --git a/include/net/net.h b/include/net/net.h
index e755254443ea..826e14a78734 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -214,6 +214,8 @@  extern NICInfo nd_table[MAX_NICS];
 extern const char *host_net_devices[];
 
 /* from net.c */
+bool netdev_is_modern(const char *optarg);
+void netdev_parse_modern(const char *optarg);
 void net_client_parse(QemuOptsList *opts_list, const char *str);
 void show_netdevs(void);
 void net_init_clients(void);
diff --git a/net/net.c b/net/net.c
index f056e8aebfb2..83300ea1b816 100644
--- a/net/net.c
+++ b/net/net.c
@@ -54,6 +54,7 @@ 
 #include "net/colo-compare.h"
 #include "net/filter.h"
 #include "qapi/string-output-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 
 /* Net bridge is currently not supported for W32. */
 #if !defined(_WIN32)
@@ -63,6 +64,16 @@ 
 static VMChangeStateEntry *net_change_state_entry;
 static QTAILQ_HEAD(, NetClientState) net_clients;
 
+typedef struct NetdevQueueEntry {
+    Netdev *nd;
+    Location loc;
+    QSIMPLEQ_ENTRY(NetdevQueueEntry) entry;
+} NetdevQueueEntry;
+
+typedef QSIMPLEQ_HEAD(, NetdevQueueEntry) NetdevQueue;
+
+static NetdevQueue nd_queue = QSIMPLEQ_HEAD_INITIALIZER(nd_queue);
+
 /***********************************************************/
 /* network device redirectors */
 
@@ -1562,6 +1573,20 @@  out:
     return ret;
 }
 
+static void netdev_init_modern(void)
+{
+    while (!QSIMPLEQ_EMPTY(&nd_queue)) {
+        NetdevQueueEntry *nd = QSIMPLEQ_FIRST(&nd_queue);
+
+        QSIMPLEQ_REMOVE_HEAD(&nd_queue, entry);
+        loc_push_restore(&nd->loc);
+        net_client_init1(nd->nd, true, &error_fatal);
+        loc_pop(&nd->loc);
+        qapi_free_Netdev(nd->nd);
+        g_free(nd);
+    }
+}
+
 void net_init_clients(void)
 {
     net_change_state_entry =
@@ -1569,6 +1594,8 @@  void net_init_clients(void)
 
     QTAILQ_INIT(&net_clients);
 
+    netdev_init_modern();
+
     qemu_opts_foreach(qemu_find_opts("netdev"), net_init_netdev, NULL,
                       &error_fatal);
 
@@ -1579,6 +1606,36 @@  void net_init_clients(void)
                       &error_fatal);
 }
 
+/*
+ * Does this -netdev argument use modern rather than traditional syntax?
+ * Modern syntax is to be parsed with netdev_parse_modern().
+ * Traditional syntax is to be parsed with net_client_parse().
+ */
+bool netdev_is_modern(const char *optarg)
+{
+    return false;
+}
+
+/*
+ * netdev_parse_modern() uses modern, more expressive syntax than
+ * net_client_parse(), but supports only the netdev option.
+ * netdev_parse_modern() appends to @nd_queue, whereas net_client_parse()
+ * appends to @qemu_netdev_opts.
+ */
+void netdev_parse_modern(const char *optarg)
+{
+    Visitor *v;
+    NetdevQueueEntry *nd;
+
+    v = qobject_input_visitor_new_str(optarg, "type", &error_fatal);
+    nd = g_new(NetdevQueueEntry, 1);
+    visit_type_Netdev(v, NULL, &nd->nd, &error_fatal);
+    visit_free(v);
+    loc_save(&nd->loc);
+
+    QSIMPLEQ_INSERT_TAIL(&nd_queue, nd, entry);
+}
+
 void net_client_parse(QemuOptsList *opts_list, const char *optarg)
 {
     if (!qemu_opts_parse_noisily(opts_list, optarg, true)) {
diff --git a/softmmu/vl.c b/softmmu/vl.c
index f6461363d608..f557e2d8cb8a 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2794,7 +2794,11 @@  void qemu_init(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_netdev:
                 default_net = 0;
-                net_client_parse(qemu_find_opts("netdev"), optarg);
+                if (netdev_is_modern(optarg)) {
+                    netdev_parse_modern(optarg);
+                } else {
+                    net_client_parse(qemu_find_opts("netdev"), optarg);
+                }
                 break;
             case QEMU_OPTION_nic:
                 default_net = 0;