* [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.