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

[Eric Blake <eblake@redhat.com>]

On 11/23/20 3:25 AM, Markus Armbruster wrote:

>> Its still actual bug - I've checked it with the
>> master(2c6605389c1f76973d92b69b85d40d94b8f1092c).
> 
> I can see this with an even simpler reproducer:
> 
>     $ qemu-system-x86_64 -S -display none -nodefaults -monitor stdio
>     QEMU 5.1.92 monitor - type 'help' for more information
>     (qemu) netdev_add user,id=net0
>     (qemu) info network
>     net0: index=0,type=user,net=10.0.2.0,restrict=off
>     (qemu) netdev_del net0
>     (qemu) info network
>     (qemu) netdev_add user,id=net0
>     upstream-qemu: Duplicate ID 'net0' for netdev
>     Try "help netdev_add" for more information
> 
> The appended patch fixes it for me.  It relies on nothing using the
> "netdev" QemuOpts anymore.  Eric, what do you think?

Makes sense to me.  My quick audit for qemu_find_opts("netdev") finds only:

monitor/hmp-cmds.c:    opts =
qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict, &err);

net/net.c:    if (qemu_opts_foreach(qemu_find_opts("netdev"),

softmmu/vl.c:                if
(net_client_parse(qemu_find_opts("netdev"), optarg) == -1) {

where the latter two are related (we gather the command line opts, and
initialize net devs based on them, but never refer to those opts again),
and the first is the one you are proposing to change.


> 
> 
> 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);
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org