* [PATCH 0/1] net: Fix handling of id in netdev_add and netdev_del @ 2020-11-25 10:02 Markus Armbruster 2020-11-25 10:02 ` [PATCH 1/1] " Markus Armbruster 2020-11-27 5:25 ` [PATCH 0/1] " Jason Wang 0 siblings, 2 replies; 7+ messages in thread From: Markus Armbruster @ 2020-11-25 10:02 UTC (permalink / raw) To: qemu-devel; +Cc: yuri.benditovich, jasowang, andrew, peter.maydell This is a regression fix, but the regression is already in 5.0. I think it's too late for 5.2. If I'm right, then the issue should be documented in the release notes (I can do that). Markus Armbruster (1): net: Fix handling of id in netdev_add and netdev_del net/net.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/1] net: Fix handling of id in netdev_add and netdev_del 2020-11-25 10:02 [PATCH 0/1] net: Fix handling of id in netdev_add and netdev_del Markus Armbruster @ 2020-11-25 10:02 ` Markus Armbruster 2020-11-25 13:18 ` Eric Blake 2020-11-27 5:25 ` [PATCH 0/1] " Jason Wang 1 sibling, 1 reply; 7+ messages in thread From: Markus Armbruster @ 2020-11-25 10:02 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell, andrew, jasowang, qemu-stable, yuri.benditovich CLI -netdev accumulates in option group "netdev". Before commit 08712fcb85 "net: Track netdevs in NetClientState rather than QemuOpt", netdev_add added to the option group, and netdev_del removed from it, both HMP and QMP. Thus, every netdev had a corresponding QemuOpts in this option group. Commit 08712fcb85 dropped this for QMP netdev_add and both netdev_del. Now a netdev has a corresponding QemuOpts only when it was created with CLI or HMP. Two issues: * QMP and HMP netdev_del can leave QemuOpts behind, breaking HMP netdev_add. 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 Fix by restoring the QemuOpts deletion in qmp_netdev_del(), but with a guard, because the QemuOpts need not exist. * QMP netdev_add loses its "no duplicate ID" check. Reproducer: $ qemu-system-x86_64 -S -display none -qmp stdio {"QMP": {"version": {"qemu": {"micro": 92, "minor": 1, "major": 5}, "package": "v5.2.0-rc2-1-g02c1f0142c"}, "capabilities": ["oob"]}} {"execute": "qmp_capabilities"} {"return": {}} {"execute": "netdev_add", "arguments": {"type": "user", "id":"net0"}} {"return": {}} {"execute": "netdev_add", "arguments": {"type": "user", "id":"net0"}} {"return": {}} Fix by adding a duplicate ID check to net_client_init1() to replace the lost one. The check is redundant for callers where QemuOpts still checks, i.e. for CLI and HMP. Reported-by: Andrew Melnichenko <andrew@daynix.com> Fixes: 08712fcb851034228b61f75bd922863a984a4f60 Cc: qemu-stable@nongnu.org Signed-off-by: Markus Armbruster <armbru@redhat.com> --- net/net.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/net/net.c b/net/net.c index 794c652282..c1dc75fc37 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; @@ -1137,6 +1142,7 @@ void qmp_netdev_add(Netdev *netdev, Error **errp) void qmp_netdev_del(const char *id, Error **errp) { NetClientState *nc; + QemuOpts *opts; nc = qemu_find_netdev(id); if (!nc) { @@ -1151,6 +1157,16 @@ void qmp_netdev_del(const char *id, Error **errp) } qemu_del_net_client(nc); + + /* + * Wart: we need to delete the QemuOpts associated with netdevs + * created via CLI or HMP, to avoid bogus "Duplicate ID" errors in + * HMP netdev_add. + */ + opts = qemu_opts_find(qemu_find_opts("netdev"), id); + if (opts) { + qemu_opts_del(opts); + } } static void netfilter_print_info(Monitor *mon, NetFilterState *nf) -- 2.26.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] net: Fix handling of id in netdev_add and netdev_del 2020-11-25 10:02 ` [PATCH 1/1] " Markus Armbruster @ 2020-11-25 13:18 ` Eric Blake 0 siblings, 0 replies; 7+ messages in thread From: Eric Blake @ 2020-11-25 13:18 UTC (permalink / raw) To: Markus Armbruster, qemu-devel Cc: yuri.benditovich, jasowang, qemu-stable, andrew, peter.maydell On 11/25/20 4:02 AM, Markus Armbruster wrote: > CLI -netdev accumulates in option group "netdev". > > Before commit 08712fcb85 "net: Track netdevs in NetClientState rather > than QemuOpt", netdev_add added to the option group, and netdev_del > removed from it, both HMP and QMP. Thus, every netdev had a > corresponding QemuOpts in this option group. > > Commit 08712fcb85 dropped this for QMP netdev_add and both netdev_del. > Now a netdev has a corresponding QemuOpts only when it was created > with CLI or HMP. Two issues: > > * QMP and HMP netdev_del can leave QemuOpts behind, breaking HMP > netdev_add. 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 > > Fix by restoring the QemuOpts deletion in qmp_netdev_del(), but with > a guard, because the QemuOpts need not exist. > > * QMP netdev_add loses its "no duplicate ID" check. Reproducer: > > $ qemu-system-x86_64 -S -display none -qmp stdio > {"QMP": {"version": {"qemu": {"micro": 92, "minor": 1, "major": 5}, "package": "v5.2.0-rc2-1-g02c1f0142c"}, "capabilities": ["oob"]}} > {"execute": "qmp_capabilities"} > {"return": {}} > {"execute": "netdev_add", "arguments": {"type": "user", "id":"net0"}} > {"return": {}} > {"execute": "netdev_add", "arguments": {"type": "user", "id":"net0"}} > {"return": {}} > > Fix by adding a duplicate ID check to net_client_init1() to replace > the lost one. The check is redundant for callers where QemuOpts > still checks, i.e. for CLI and HMP. > > Reported-by: Andrew Melnichenko <andrew@daynix.com> > Fixes: 08712fcb851034228b61f75bd922863a984a4f60 > Cc: qemu-stable@nongnu.org > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > net/net.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) The adventure in QAPIfication of netdev_add has been long. Thanks for working on this. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/1] net: Fix handling of id in netdev_add and netdev_del 2020-11-25 10:02 [PATCH 0/1] net: Fix handling of id in netdev_add and netdev_del Markus Armbruster 2020-11-25 10:02 ` [PATCH 1/1] " Markus Armbruster @ 2020-11-27 5:25 ` Jason Wang 2020-11-27 9:12 ` Markus Armbruster 2021-01-15 13:56 ` Markus Armbruster 1 sibling, 2 replies; 7+ messages in thread From: Jason Wang @ 2020-11-27 5:25 UTC (permalink / raw) To: Markus Armbruster, qemu-devel; +Cc: yuri.benditovich, andrew, peter.maydell On 2020/11/25 下午6:02, Markus Armbruster wrote: > This is a regression fix, but the regression is already in 5.0. I > think it's too late for 5.2. If I'm right, then the issue should be > documented in the release notes (I can do that). Please do that and I've queued this for the next release. Thanks > > Markus Armbruster (1): > net: Fix handling of id in netdev_add and netdev_del > > net/net.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/1] net: Fix handling of id in netdev_add and netdev_del 2020-11-27 5:25 ` [PATCH 0/1] " Jason Wang @ 2020-11-27 9:12 ` Markus Armbruster 2021-01-15 13:56 ` Markus Armbruster 1 sibling, 0 replies; 7+ messages in thread From: Markus Armbruster @ 2020-11-27 9:12 UTC (permalink / raw) To: Jason Wang; +Cc: yuri.benditovich, andrew, qemu-devel, peter.maydell Jason Wang <jasowang@redhat.com> writes: > On 2020/11/25 下午6:02, Markus Armbruster wrote: >> This is a regression fix, but the regression is already in 5.0. I >> think it's too late for 5.2. If I'm right, then the issue should be >> documented in the release notes (I can do that). > > > Please do that and I've queued this for the next release. Done. Thanks! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/1] net: Fix handling of id in netdev_add and netdev_del 2020-11-27 5:25 ` [PATCH 0/1] " Jason Wang 2020-11-27 9:12 ` Markus Armbruster @ 2021-01-15 13:56 ` Markus Armbruster 2021-01-18 2:49 ` Jason Wang 1 sibling, 1 reply; 7+ messages in thread From: Markus Armbruster @ 2021-01-15 13:56 UTC (permalink / raw) To: Jason Wang; +Cc: yuri.benditovich, andrew, qemu-devel, peter.maydell Jason Wang <jasowang@redhat.com> writes: > On 2020/11/25 下午6:02, Markus Armbruster wrote: >> This is a regression fix, but the regression is already in 5.0. I >> think it's too late for 5.2. If I'm right, then the issue should be >> documented in the release notes (I can do that). > > > Please do that and I've queued this for the next release. Ping? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/1] net: Fix handling of id in netdev_add and netdev_del 2021-01-15 13:56 ` Markus Armbruster @ 2021-01-18 2:49 ` Jason Wang 0 siblings, 0 replies; 7+ messages in thread From: Jason Wang @ 2021-01-18 2:49 UTC (permalink / raw) To: Markus Armbruster; +Cc: yuri.benditovich, andrew, qemu-devel, peter.maydell On 2021/1/15 下午9:56, Markus Armbruster wrote: > Jason Wang <jasowang@redhat.com> writes: > >> On 2020/11/25 下午6:02, Markus Armbruster wrote: >>> This is a regression fix, but the regression is already in 5.0. I >>> think it's too late for 5.2. If I'm right, then the issue should be >>> documented in the release notes (I can do that). >> >> Please do that and I've queued this for the next release. > Ping? > > Hi Markus: I plan to send a pull request by the end of this week which should contain this series. Thanks ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-01-18 2:50 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-25 10:02 [PATCH 0/1] net: Fix handling of id in netdev_add and netdev_del Markus Armbruster 2020-11-25 10:02 ` [PATCH 1/1] " Markus Armbruster 2020-11-25 13:18 ` Eric Blake 2020-11-27 5:25 ` [PATCH 0/1] " Jason Wang 2020-11-27 9:12 ` Markus Armbruster 2021-01-15 13:56 ` Markus Armbruster 2021-01-18 2:49 ` Jason Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.