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

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

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

Please confirm that this patch is intended to solve only the problem with
hmp (and disallow duplicated ids)
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?

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

> Markus Armbruster <armbru@redhat.com> writes:
>
> > This means commit 08712fcb85 "net: Track netdevs in NetClientState
> > rather than QemuOpt" didn't actually replace QemuOpts completely.
> >
> > This affects QMP:
> >
> >     $ socat "READLINE,history=$HOME/.qmp_history,prompt=QMP>"
> UNIX-CONNECT:$HOME/work/images/test-qmp
> >     {"QMP": {"version": {"qemu": {"micro": 92, "minor": 1, "major": 5},
> "package": "v5.2.0-rc2-19-gff85db769f-dirty"}, "capabilities": ["oob"]}}
> >     QMP>{ "execute": "qmp_capabilities", "arguments": { "enable":
> ["oob"] } }
> >     {"return": {}}
> >     QMP>{"execute": "netdev_add", "arguments": {"type": "user",
> "id":"net0"}}
> >     {"return": {}}
> >     QMP>{"execute": "netdev_add", "arguments": {"type": "user",
> "id":"net0"}}
> >     {"return": {}}
> >
> > Results in two netdevs called "net0".  Needs fixing.
>
> Here's my attempt.  If it looks good to you, I'll post it as a proper
> patch.
>
>
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index a6a6684df1..8bc6b7bcc6 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1638,9 +1638,7 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict)
>      }
>
>      netdev_add(opts, &err);
> -    if (err) {
> -        qemu_opts_del(opts);
> -    }
> +    qemu_opts_del(opts);
>
>  out:
>      hmp_handle_error(mon, err);
> diff --git a/net/net.c b/net/net.c
> index 794c652282..eb743aca23 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;
> --
> 2.26.2
>
>

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