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

[Andrew Melnichenko]

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

On Sat, Nov 21, 2020 at 5:31 PM Yuri Benditovich <yuri.benditovich@daynix.com> wrote:

On Sat, Nov 21, 2020 at 5:24 PM Yuri Benditovich <yuri.benditovich@daynix.com> wrote:

On Fri, Nov 20, 2020 at 2:58 PM Markus Armbruster <armbru@redhat.com> wrote:

Andrew Melnichenko <andrew@daynix.com> writes:

> Ping
>
> On Thu, Jul 16, 2020 at 6:26 AM <andrew@daynix.com> wrote:
>
>> From: Andrew Melnychenko <andrew@daynix.com>
>>
>> There is an issue, that netdev can't be removed if it was added using hmp.
>> The bug appears after 08712fcb851034228b61f75bd922863a984a4f60 commit.
>> It happens because of unclear QemuOpts that was created during
>> hmp_netdev_add(), now it uses qmp analog function -
>> qmp_marshal_netdev_add().
>>
>> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
>> ---
>>  monitor/hmp-cmds.c | 15 +++------------
>>  1 file changed, 3 insertions(+), 12 deletions(-)
>>
>> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
>> index 2b0b58a336..b747935687 100644
>> --- a/monitor/hmp-cmds.c
>> +++ b/monitor/hmp-cmds.c
>> @@ -1597,19 +1597,10 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
>>  void hmp_netdev_add(Monitor *mon, const QDict *qdict)
>>  {
>>      Error *err = NULL;
>> -    QemuOpts *opts;
>> -
>> -    opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict, &err);
>> -    if (err) {
>> -        goto out;
>> -    }
>> +    QDict *non_constant_dict = qdict_clone_shallow(qdict);
>>
>> -    netdev_add(opts, &err);
>> -    if (err) {
>> -        qemu_opts_del(opts);
>> -    }
>> -
>> -out:
>> +    qmp_marshal_netdev_add(non_constant_dict, NULL, &err);
>> +    qobject_unref(non_constant_dict);
>>      hmp_handle_error(mon, err);
>>  }

qmp_marshal_netdev_add() uses the QObject input visitor, which feels
wrong for HMP input.

What exactly is the problem you're trying to solve?  Can you show us a
reproducer?

The problem was found during work on hotplug/unplug problems with q35

run q35 VM with netdev and hotpluggable nic (virtio or e1000e)

unplug the nic (device_del)

delete the netdev ()

add netdev with the same id as before - fail (Duplicated ID)

Q35 is not mandatory for reproduction, the same with '-machine pc'