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

[Yuri Benditovich]

I'm sorry for the confusion. Andrew's description of steps to reproduce the problem is correct.

I've described another problem present in the master but not related to this patch.

On Sun, Nov 22, 2020 at 11:45 AM Andrew Melnichenko <andrew@daynix.com> wrote:

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'