As mentioned in the previous patch, our use of QemuOpt group "netdev"
has two purposes: collect the CLI arguments, and serve as a witness
for monitor hotplug actions.  As the latter didn't use anything but an
id, it felt rather unclean to have to touch QemuOpts at all when going
through QMP, so let's instead track things with a bool field in
NetClientState.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/net/net.h |  1 +
 monitor/misc.c    |  4 +---
 net/net.c         | 37 +++++++++++--------------------------
 3 files changed, 13 insertions(+), 29 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 96e6eae8176e..094e966af9ec 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -98,6 +98,7 @@ struct NetClientState {
     unsigned rxfilter_notify_enabled:1;
     int vring_enable;
     int vnet_hdr_len;
+    bool is_netdev;
     QTAILQ_HEAD(, NetFilterState) filters;
 };

diff --git a/monitor/misc.c b/monitor/misc.c
index 41a86e7012a1..6c45fa490ff5 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -2035,13 +2035,11 @@ void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str)
     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;
         const char *name = ncs[i]->name;
         if (strncmp(str, name, len)) {
             continue;
         }
-        opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
-        if (opts) {
+        if (ncs[i]->is_netdev) {
             readline_add_completion(rs, name);
         }
     }
diff --git a/net/net.c b/net/net.c
index a2065aabede2..38778e831da2 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1060,6 +1060,15 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp)
         }
         return -1;
     }
+
+    if (is_netdev) {
+        NetClientState *nc;
+
+        nc = qemu_find_netdev(netdev->id);
+        assert(nc);
+        nc->is_netdev = true;
+    }
+
     return 0;
 }

@@ -1172,34 +1181,12 @@ void netdev_add(QemuOpts *opts, Error **errp)

 void qmp_netdev_add(Netdev *netdev, Error **errp)
 {
-    Error *local_err = NULL;
-    QemuOptsList *opts_list;
-    QemuOpts *opts;
-
-    opts_list = qemu_find_opts_err("netdev", &local_err);
-    if (local_err) {
-        goto out;
-    }
-
-    opts = qemu_opts_create(opts_list, netdev->id, 1, &local_err);
-    if (local_err) {
-        goto out;
-    }
-
-    net_client_init1(netdev, true, &local_err);
-    if (local_err) {
-        qemu_opts_del(opts);
-        goto out;
-    }
-
-out:
-    error_propagate(errp, local_err);
+    net_client_init1(netdev, true, errp);
 }

 void qmp_netdev_del(const char *id, Error **errp)
 {
     NetClientState *nc;
-    QemuOpts *opts;

     nc = qemu_find_netdev(id);
     if (!nc) {
@@ -1208,14 +1195,12 @@ void qmp_netdev_del(const char *id, Error **errp)
         return;
     }

-    opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), id);
-    if (!opts) {
+    if (!nc->is_netdev) {
         error_setg(errp, "Device '%s' is not a netdev", id);
         return;
     }

     qemu_del_net_client(nc);
-    qemu_opts_del(opts);
 }

 static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
-- 
2.25.1

Eric Blake <eblake@redhat.com> writes:

> As mentioned in the previous patch, our use of QemuOpt group "netdev"
> has two purposes: collect the CLI arguments, and serve as a witness
> for monitor hotplug actions.  As the latter didn't use anything but an
> id, it felt rather unclean to have to touch QemuOpts at all when going
> through QMP, so let's instead track things with a bool field in
> NetClientState.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  include/net/net.h |  1 +
>  monitor/misc.c    |  4 +---
>  net/net.c         | 37 +++++++++++--------------------------
>  3 files changed, 13 insertions(+), 29 deletions(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 96e6eae8176e..094e966af9ec 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -98,6 +98,7 @@ struct NetClientState {
>      unsigned rxfilter_notify_enabled:1;
>      int vring_enable;
>      int vnet_hdr_len;
> +    bool is_netdev;
>      QTAILQ_HEAD(, NetFilterState) filters;
>  };
>
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 41a86e7012a1..6c45fa490ff5 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -2035,13 +2035,11 @@ void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str)
>      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;
>          const char *name = ncs[i]->name;
>          if (strncmp(str, name, len)) {
>              continue;
>          }
> -        opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
> -        if (opts) {
> +        if (ncs[i]->is_netdev) {
>              readline_add_completion(rs, name);
>          }
>      }
> diff --git a/net/net.c b/net/net.c
> index a2065aabede2..38778e831da2 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1060,6 +1060,15 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp)
>          }
>          return -1;
>      }
> +
> +    if (is_netdev) {
> +        NetClientState *nc;
> +
> +        nc = qemu_find_netdev(netdev->id);
> +        assert(nc);
> +        nc->is_netdev = true;
> +    }
> +
>      return 0;
>  }
>

Would be simpler if net_client_init_fun[]() returned the NetClientState.
But that's clearly more than we can get done today.

> @@ -1172,34 +1181,12 @@ void netdev_add(QemuOpts *opts, Error **errp)
>
>  void qmp_netdev_add(Netdev *netdev, Error **errp)
>  {
> -    Error *local_err = NULL;
> -    QemuOptsList *opts_list;
> -    QemuOpts *opts;
> -
> -    opts_list = qemu_find_opts_err("netdev", &local_err);
> -    if (local_err) {
> -        goto out;
> -    }
> -
> -    opts = qemu_opts_create(opts_list, netdev->id, 1, &local_err);
> -    if (local_err) {
> -        goto out;
> -    }
> -
> -    net_client_init1(netdev, true, &local_err);
> -    if (local_err) {
> -        qemu_opts_del(opts);
> -        goto out;
> -    }
> -
> -out:
> -    error_propagate(errp, local_err);
> +    net_client_init1(netdev, true, errp);
>  }

Nice!

>
>  void qmp_netdev_del(const char *id, Error **errp)
>  {
>      NetClientState *nc;
> -    QemuOpts *opts;
>
>      nc = qemu_find_netdev(id);
>      if (!nc) {
> @@ -1208,14 +1195,12 @@ void qmp_netdev_del(const char *id, Error **errp)
>          return;
>      }
>
> -    opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), id);
> -    if (!opts) {
> +    if (!nc->is_netdev) {
>          error_setg(errp, "Device '%s' is not a netdev", id);
>          return;
>      }
>
>      qemu_del_net_client(nc);
> -    qemu_opts_del(opts);
>  }
>
>  static void netfilter_print_info(Monitor *mon, NetFilterState *nf)

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