Re: [PATCH] hmp: Changed hmp_netdev_add() using qmp_marshal_netdev_add()

[Yuri Benditovich <yuri.benditovich@daynix.com>]

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

On Tue, Nov 24, 2020 at 3:36 PM Markus Armbruster <armbru@redhat.com> wrote:

> Markus Armbruster <armbru@redhat.com> writes:
>
> > Yuri Benditovich <yuri.benditovich@daynix.com> writes:
> >
> >> Please confirm that this patch is intended to solve only the problem
> with
> >> hmp (and disallow duplicated ids)
> >
> > The intent is to reject duplicate ID and to accept non-duplicate ID, no
> > matter how the device is created (CLI, HMP, QMP) or a prior instance was
> > deleted (HMP, QMP).
> >
> >> With it the netdev that was added from qemu's command line and was
> deleted
> >> (for example by hmp) still can't be created, correct?
> >
> > Yet another case; back to the drawing board...
>
> Next try.  Hope this is one holds water :)
>
>
> diff --git a/net/net.c b/net/net.c
> index 794c652282..c1dc75fc37 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -978,6 +978,7 @@ static int (* const
> net_client_init_fun[NET_CLIENT_DRIVER__MAX])(
>  static int net_client_init1(const Netdev *netdev, bool is_netdev, Error
> **errp)
>  {
>      NetClientState *peer = NULL;
> +    NetClientState *nc;
>
>      if (is_netdev) {
>          if (netdev->type == NET_CLIENT_DRIVER_NIC ||
> @@ -1005,6 +1006,12 @@ static int net_client_init1(const Netdev *netdev,
> bool is_netdev, Error **errp)
>          }
>      }
>
> +    nc = qemu_find_netdev(netdev->id);
> +    if (nc) {
> +        error_setg(errp, "Duplicate ID '%s'", netdev->id);
> +        return -1;
> +    }
> +
>      if (net_client_init_fun[netdev->type](netdev, netdev->id, peer, errp)
> < 0) {
>          /* FIXME drop when all init functions store an Error */
>          if (errp && !*errp) {
> @@ -1015,8 +1022,6 @@ static int net_client_init1(const Netdev *netdev,
> bool is_netdev, Error **errp)
>      }
>
>      if (is_netdev) {
> -        NetClientState *nc;
> -
>          nc = qemu_find_netdev(netdev->id);
>          assert(nc);
>          nc->is_netdev = true;
> @@ -1137,6 +1142,7 @@ void qmp_netdev_add(Netdev *netdev, Error **errp)
>  void qmp_netdev_del(const char *id, Error **errp)
>  {
>      NetClientState *nc;
> +    QemuOpts *opts;
>
>      nc = qemu_find_netdev(id);
>      if (!nc) {
> @@ -1151,6 +1157,16 @@ void qmp_netdev_del(const char *id, Error **errp)
>      }
>
>      qemu_del_net_client(nc);
> +
> +    /*
> +     * Wart: we need to delete the QemuOpts associated with netdevs
> +     * created via CLI or HMP, to avoid bogus "Duplicate ID" errors in
> +     * HMP netdev_add.
> +     */
> +    opts = qemu_opts_find(qemu_find_opts("netdev"), id);
> +    if (opts) {
> +        qemu_opts_del(opts);
> +    }
>  }
>
>
With this part there is no need to unconditionally delete the options
in hmp_netdev_add,
correct?


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

[-- Attachment #2: Type: text/html, Size: 3952 bytes --]