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

[Yuri Benditovich]

The patch below solves both issues: with netdev created by hmp and with netdev created from command-line:

diff --git a/net/net.c b/net/net.c
index bcd5da4aa0..98294f24ed 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1155,6 +1155,11 @@ void qmp_netdev_del(const char *id, Error **errp)
     }

     qemu_del_net_client(nc);
+
+    QemuOpts *opts = qemu_opts_find(qemu_find_opts("netdev"), id);
+    if (opts) {
+        qemu_opts_del(opts);
+    }
 }

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

Please let me know if you have any objections.

If not, I'll send it as a patch.

On Mon, Nov 23, 2020 at 5:35 PM Yuri Benditovich <yuri.benditovich@daynix.com> wrote:

On Mon, Nov 23, 2020 at 11:25 AM Markus Armbruster <armbru@redhat.com> wrote:

Andrew Melnichenko <andrew@daynix.com> writes:

> --000000000000f73b2205b4aef0c5
> Content-Type: text/plain; charset="UTF-8"
>
> Hi, the bug can be reproduced like that:
>
>> QEMU 5.1.50 monitor - type 'help' for more information
>> (qemu) netdev_add
>> type=tap,id=net0,script=/home/and/SRCS/qemu/ifup.sh,downscript=no
>> (qemu) info network
>> hub 0
>>  \ hub0port1: __org.qemu.net1: index=0,type=user,net=10.0.2.0,restrict=off
>>  \ hub0port0: e1000e.0:
>> index=0,type=nic,model=e1000e,macaddr=52:54:00:12:34:56
>> dnet0: index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:57
>> net0:
>> index=0,type=tap,ifname=tap0,script=/home/and/SRCS/qemu/ifup.sh,downscript=no
>> (qemu) netdev_del net0
>> (qemu) info network
>> hub 0
>>  \ hub0port1: __org.qemu.net1: index=0,type=user,net=10.0.2.0,restrict=off
>>  \ hub0port0: e1000e.0:
>> index=0,type=nic,model=e1000e,macaddr=52:54:00:12:34:56
>> dnet0: index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:57
>> (qemu) netdev_add
>> type=tap,id=net0,script=/home/and/SRCS/qemu/ifup.sh,downscript=no
>> Try "help netdev_add" for more information
>> (qemu) info network
>> hub 0
>>  \ hub0port1: __org.qemu.net1: index=0,type=user,net=10.0.2.0,restrict=off
>>  \ hub0port0: e1000e.0:
>> index=0,type=nic,model=e1000e,macaddr=52:54:00:12:34:56
>> dnet0: index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:57
>> (qemu)
>>
>>
> 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?


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

Unfortunately, if I'm not mistaken, with this fix qemu will be able to create from hmp several devices with the same id

(which is not expected).

For example:

netdev_add user,id=net0

netdev_add user,id=net0

info network lists 2 devices net0

 

 out:
     hmp_handle_error(mon, err);