Re: modify NetdevUserOptions through QMP in QEMU 6 - how?

2021-12-15 Thread Alexander Sosedkin
On Tue, Dec 14, 2021 at 3:53 PM Michael S. Tsirkin  wrote:
> On Mon, Dec 13, 2021 at 09:02:15AM +0100, Thomas Huth wrote:
> > On 10/12/2021 18.02, Alexander Sosedkin wrote:
> > > With QEMU 5 I could totally issue a QMP netdev_add
> > > with the same ID to adjust the NetdevUserOptions I want,
> > > such as restrict or hostfwd. No deleting needed,
> > > just a netdev_add with what I want changed as a param.
> >
> > I'm a little bit surprised that this worked, since AFAIK there is no code in
> > QEMU to *change* the parameters of a running netdev... likely the code added
> > a new netdev with the same ID, replacing the old one?
> >
> > > With QEMU 6 it started failing, claiming the ID is already used.
> > > And if I do netdev_del + netdev_add, I just lose connectivity.
> > > What's even stranger, I still see old netdev attached in info network:
> > >
> > > > netdev_del {'id': 'net0'}
> > > {}
> > > > human-monitor-command {'command-line': 'info network'}
> > > virtio-net-pci.0:
> > > index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
> > >   \ net0: index=0,type=user,net=10.0.2.0,restrict=off
> >
> > I think that's "normal" - there used to be problems in the past that the
> > devices (virtio-net-pci in this case) did not like the netdevs to be removed
> > on the fly. So the netdevs are kept around until you remove the device, too
> > (i.e. issue a device_del for the virtio-net-pci device).
> >
> > > > netdev_add {'type': 'user', 'id': 'net0', 'restrict': False, 'hostfwd': 
> > > > [{'str': 'tcp:127.0.0.1:58239-:22'}]}
> > > {}
> > > > human-monitor-command {'command-line': 'info network'}
> > > unseal: virtio-net-pci.0:
> > > index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
> > >   \ net0: index=0,type=user,net=10.0.2.0,restrict=off
> > > net0: index=0,type=user,net=10.0.2.0,restrict=off
> > >
> > > What's the correct QMP command sequence to modify NetdevUserOptions?
> >
> > AFAIK there is no way to modify running netdevs - you'd have to delete the
> > netdev and the device, and then add both again. But I might have missed
> > something here, so I CC:-ed some people who might be more familiar with the
> > details here.
> >
> >  Thomas
> >
> >
> > > Please CC me on replies.
>
>
> Wow this really goes to show how wide our feature matrix is.

For real. I consider myself an amateur QEMU abuser
and I can use the tiny corner I've worked with as a blanket.

> Yes it's probably an unintended side effect but yes it
> did work it seems, so we really should not just break it
> without warning.
>
>
> Probably this one:
>
> commit 831734cce6494032e9233caff4d8442b3a1e7fef
> Author: Markus Armbruster 
> Date:   Wed Nov 25 11:02:20 2020 +0100
>
> net: Fix handling of id in netdev_add and netdev_del
>
>
>
> Jason, what is your take here?
>
>
> Alexander, what happens if we just drop the duplicate ID check? Do
> things work for you again?
> Warning: completely untested.
>
> Signed-off-by: Michael S. Tsirkin 
>
>
> diff --git a/net/net.c b/net/net.c
> index f0d14dbfc1..01f5a187b6 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1055,12 +1055,6 @@ 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) {
> --
> MST
>

> (in other email)
> Got a reproducer for me so I can double-check?

Yeah, I've sketched one out in Python, see attachments.

qemu-netdev-reproducer.py sets up a user netdev with restrict=off,
then netdev_adds a weird thing, unrestricting and adding a hostfwd entry.

qemu-netdev.qemu5.log is the output of it reconfiguring great on Fedora 34,
even though it adds a duplicate netdev judging by `info network`:
\ net0: index=0,type=user,net=10.0.2.0,restrict=on
\ net0: index=0,type=user,net=10.0.2.0,restrict=off

qemu-netdev.qemu6.log is how it fails with duplicate ID now
(vendor=nixpkgs, but I've seen same error reports from Fedora 35):
{'error': {'class': 'GenericError', 'desc': "Duplicate ID 'net0'"}}

qemu-netdev.qemu6-nocheck.log is the behaviour with your patch applied,
behaving the same way QEMU 5 does.

Hope that answers all the questions from the thread;
if not, please re-raise your questions and I'll do my best to answer
them timely.

Just to stress it,
I'm not barging in with "you broke the old behaviour, revert things".
I recognize that I've been doing weird things and
merely ask what's the way forward to get the features I used to have access to.
Not gonna lie, I would like to see it reverted,
but having a proper way to reconfigure would be even better.
Thank you.
# fluff

import atexit
import json
import socket
import subprocess
import time

QMP_PORT = 12345
TEST_PORT = 12346
QEMU = 

Re: modify NetdevUserOptions through QMP in QEMU 6 - how?

2021-12-14 Thread Michael S. Tsirkin
On Wed, Dec 15, 2021 at 08:03:50AM +0100, Thomas Huth wrote:
> So if changing netdev parameters on the fly is something that we want, we
> should implement this properly instead indeed, and not via such an
> accidental bug.

How to do it is a separate thing, users don't really care at all.

-- 
MST




Re: modify NetdevUserOptions through QMP in QEMU 6 - how?

2021-12-14 Thread Michael S. Tsirkin
On Wed, Dec 15, 2021 at 08:03:50AM +0100, Thomas Huth wrote:
> On 15/12/2021 04.31, Jason Wang wrote:
> > On Tue, Dec 14, 2021 at 10:53 PM Michael S. Tsirkin  wrote:
> > > 
> > > On Mon, Dec 13, 2021 at 09:02:15AM +0100, Thomas Huth wrote:
> > > >   Hi!
> > > > 
> > > > On 10/12/2021 18.02, Alexander Sosedkin wrote:
> > > > > With QEMU 5 I could totally issue a QMP netdev_add
> > > > > with the same ID to adjust the NetdevUserOptions I want,
> > > > > such as restrict or hostfwd. No deleting needed,
> > > > > just a netdev_add with what I want changed as a param.
> > > > 
> > > > I'm a little bit surprised that this worked, since AFAIK there is no 
> > > > code in
> > > > QEMU to *change* the parameters of a running netdev... likely the code 
> > > > added
> > > > a new netdev with the same ID, replacing the old one?
> > > > 
> > > > > With QEMU 6 it started failing, claiming the ID is already used.
> > > > > And if I do netdev_del + netdev_add, I just lose connectivity.
> > > > > What's even stranger, I still see old netdev attached in info network:
> > > > > 
> > > > > > netdev_del {'id': 'net0'}
> > > > > {}
> > > > > > human-monitor-command {'command-line': 'info network'}
> > > > > virtio-net-pci.0:
> > > > > index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
> > > > >\ net0: index=0,type=user,net=10.0.2.0,restrict=off
> > > > 
> > > > I think that's "normal" - there used to be problems in the past that the
> > > > devices (virtio-net-pci in this case) did not like the netdevs to be 
> > > > removed
> > > > on the fly. So the netdevs are kept around until you remove the device, 
> > > > too
> > > > (i.e. issue a device_del for the virtio-net-pci device).
> > > > 
> > > > > > netdev_add {'type': 'user', 'id': 'net0', 'restrict': False, 
> > > > > > 'hostfwd': [{'str': 'tcp:127.0.0.1:58239-:22'}]}
> > > > > {}
> > > > > > human-monitor-command {'command-line': 'info network'}
> > > > > unseal: virtio-net-pci.0:
> > > > > index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
> > > > >\ net0: index=0,type=user,net=10.0.2.0,restrict=off
> > > > > net0: index=0,type=user,net=10.0.2.0,restrict=off
> > > > > 
> > > > > What's the correct QMP command sequence to modify NetdevUserOptions?
> > > > 
> > > > AFAIK there is no way to modify running netdevs - you'd have to delete 
> > > > the
> > > > netdev and the device, and then add both again. But I might have missed
> > > > something here, so I CC:-ed some people who might be more familiar with 
> > > > the
> > > > details here.
> > > > 
> > > >   Thomas
> > > > 
> > > > 
> > > > > Please CC me on replies.
> > > 
> > > 
> > > Wow this really goes to show how wide our feature matrix is.
> > > 
> > > Yes it's probably an unintended side effect but yes it
> > > did work it seems, so we really should not just break it
> > > without warning.
> > > 
> > > 
> > > Probably this one:
> > > 
> > > commit 831734cce6494032e9233caff4d8442b3a1e7fef
> > > Author: Markus Armbruster 
> > > Date:   Wed Nov 25 11:02:20 2020 +0100
> > > 
> > >  net: Fix handling of id in netdev_add and netdev_del
> > > 
> > > 
> > > 
> > > Jason, what is your take here?
> > 
> > I might be wrong, but I agree with Thomas. Adding a netdev with the
> > same ID looks wrong, if it works, it looks like a bug.
> 
> It certainly calls for trouble as soon as you try to delete the netdev again
> - does it delete the first (inactive) instance? Does it delete the second
> active one? Does it delete both? (Otherwise it will leave a dangling
> instance behind) ...
> So if changing netdev parameters on the fly is something that we want, we
> should implement this properly instead indeed, and not via such an
> accidental bug.
> 
>  Thomas


Alexander, could you supply a reporoducer so we can check in which
QEMU versions it worked?
If it worked for a long time, then even if it was a result of a bug
it's an accidental ABI and we should not just break it.

-- 
MST




Re: modify NetdevUserOptions through QMP in QEMU 6 - how?

2021-12-14 Thread Michael S. Tsirkin
On Wed, Dec 15, 2021 at 07:48:06AM +0100, Markus Armbruster wrote:
> Jason Wang  writes:
> 
> > On Tue, Dec 14, 2021 at 10:53 PM Michael S. Tsirkin  wrote:
> >>
> >> On Mon, Dec 13, 2021 at 09:02:15AM +0100, Thomas Huth wrote:
> >> >  Hi!
> >> >
> >> > On 10/12/2021 18.02, Alexander Sosedkin wrote:
> >> > > With QEMU 5 I could totally issue a QMP netdev_add
> >> > > with the same ID to adjust the NetdevUserOptions I want,
> >> > > such as restrict or hostfwd. No deleting needed,
> >> > > just a netdev_add with what I want changed as a param.
> >> >
> >> > I'm a little bit surprised that this worked, since AFAIK there is no 
> >> > code in
> >> > QEMU to *change* the parameters of a running netdev... likely the code 
> >> > added
> >> > a new netdev with the same ID, replacing the old one?
> >> >
> >> > > With QEMU 6 it started failing, claiming the ID is already used.
> >> > > And if I do netdev_del + netdev_add, I just lose connectivity.
> >> > > What's even stranger, I still see old netdev attached in info network:
> >> > >
> >> > > > netdev_del {'id': 'net0'}
> >> > > {}
> >> > > > human-monitor-command {'command-line': 'info network'}
> >> > > virtio-net-pci.0:
> >> > > index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
> >> > >   \ net0: index=0,type=user,net=10.0.2.0,restrict=off
> >> >
> >> > I think that's "normal" - there used to be problems in the past that the
> >> > devices (virtio-net-pci in this case) did not like the netdevs to be 
> >> > removed
> >> > on the fly. So the netdevs are kept around until you remove the device, 
> >> > too
> >> > (i.e. issue a device_del for the virtio-net-pci device).
> >> >
> >> > > > netdev_add {'type': 'user', 'id': 'net0', 'restrict': False, 
> >> > > > 'hostfwd': [{'str': 'tcp:127.0.0.1:58239-:22'}]}
> >> > > {}
> >> > > > human-monitor-command {'command-line': 'info network'}
> >> > > unseal: virtio-net-pci.0:
> >> > > index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
> >> > >   \ net0: index=0,type=user,net=10.0.2.0,restrict=off
> >> > > net0: index=0,type=user,net=10.0.2.0,restrict=off
> >> > >
> >> > > What's the correct QMP command sequence to modify NetdevUserOptions?
> >> >
> >> > AFAIK there is no way to modify running netdevs - you'd have to delete 
> >> > the
> >> > netdev and the device, and then add both again. But I might have missed
> >> > something here, so I CC:-ed some people who might be more familiar with 
> >> > the
> >> > details here.
> >> >
> >> >  Thomas
> >> >
> >> >
> >> > > Please CC me on replies.
> >>
> >>
> >> Wow this really goes to show how wide our feature matrix is.
> >>
> >> Yes it's probably an unintended side effect but yes it
> >> did work it seems, so we really should not just break it
> >> without warning.
> 
> Depends.  See below.
> 
> >> Probably this one:
> >>
> >> commit 831734cce6494032e9233caff4d8442b3a1e7fef
> >> Author: Markus Armbruster 
> >> Date:   Wed Nov 25 11:02:20 2020 +0100
> >>
> >> net: Fix handling of id in netdev_add and netdev_del
> 
>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 

Re: modify NetdevUserOptions through QMP in QEMU 6 - how?

2021-12-14 Thread Thomas Huth

On 15/12/2021 04.31, Jason Wang wrote:

On Tue, Dec 14, 2021 at 10:53 PM Michael S. Tsirkin  wrote:


On Mon, Dec 13, 2021 at 09:02:15AM +0100, Thomas Huth wrote:

  Hi!

On 10/12/2021 18.02, Alexander Sosedkin wrote:

With QEMU 5 I could totally issue a QMP netdev_add
with the same ID to adjust the NetdevUserOptions I want,
such as restrict or hostfwd. No deleting needed,
just a netdev_add with what I want changed as a param.


I'm a little bit surprised that this worked, since AFAIK there is no code in
QEMU to *change* the parameters of a running netdev... likely the code added
a new netdev with the same ID, replacing the old one?


With QEMU 6 it started failing, claiming the ID is already used.
And if I do netdev_del + netdev_add, I just lose connectivity.
What's even stranger, I still see old netdev attached in info network:


netdev_del {'id': 'net0'}

{}

human-monitor-command {'command-line': 'info network'}

virtio-net-pci.0:
index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
   \ net0: index=0,type=user,net=10.0.2.0,restrict=off


I think that's "normal" - there used to be problems in the past that the
devices (virtio-net-pci in this case) did not like the netdevs to be removed
on the fly. So the netdevs are kept around until you remove the device, too
(i.e. issue a device_del for the virtio-net-pci device).


netdev_add {'type': 'user', 'id': 'net0', 'restrict': False, 'hostfwd': 
[{'str': 'tcp:127.0.0.1:58239-:22'}]}

{}

human-monitor-command {'command-line': 'info network'}

unseal: virtio-net-pci.0:
index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
   \ net0: index=0,type=user,net=10.0.2.0,restrict=off
net0: index=0,type=user,net=10.0.2.0,restrict=off

What's the correct QMP command sequence to modify NetdevUserOptions?


AFAIK there is no way to modify running netdevs - you'd have to delete the
netdev and the device, and then add both again. But I might have missed
something here, so I CC:-ed some people who might be more familiar with the
details here.

  Thomas



Please CC me on replies.



Wow this really goes to show how wide our feature matrix is.

Yes it's probably an unintended side effect but yes it
did work it seems, so we really should not just break it
without warning.


Probably this one:

commit 831734cce6494032e9233caff4d8442b3a1e7fef
Author: Markus Armbruster 
Date:   Wed Nov 25 11:02:20 2020 +0100

 net: Fix handling of id in netdev_add and netdev_del



Jason, what is your take here?


I might be wrong, but I agree with Thomas. Adding a netdev with the
same ID looks wrong, if it works, it looks like a bug. 


It certainly calls for trouble as soon as you try to delete the netdev again 
- does it delete the first (inactive) instance? Does it delete the second 
active one? Does it delete both? (Otherwise it will leave a dangling 
instance behind) ...
So if changing netdev parameters on the fly is something that we want, we 
should implement this properly instead indeed, and not via such an 
accidental bug.


 Thomas




Re: modify NetdevUserOptions through QMP in QEMU 6 - how?

2021-12-14 Thread Markus Armbruster
Jason Wang  writes:

> On Tue, Dec 14, 2021 at 10:53 PM Michael S. Tsirkin  wrote:
>>
>> On Mon, Dec 13, 2021 at 09:02:15AM +0100, Thomas Huth wrote:
>> >  Hi!
>> >
>> > On 10/12/2021 18.02, Alexander Sosedkin wrote:
>> > > With QEMU 5 I could totally issue a QMP netdev_add
>> > > with the same ID to adjust the NetdevUserOptions I want,
>> > > such as restrict or hostfwd. No deleting needed,
>> > > just a netdev_add with what I want changed as a param.
>> >
>> > I'm a little bit surprised that this worked, since AFAIK there is no code 
>> > in
>> > QEMU to *change* the parameters of a running netdev... likely the code 
>> > added
>> > a new netdev with the same ID, replacing the old one?
>> >
>> > > With QEMU 6 it started failing, claiming the ID is already used.
>> > > And if I do netdev_del + netdev_add, I just lose connectivity.
>> > > What's even stranger, I still see old netdev attached in info network:
>> > >
>> > > > netdev_del {'id': 'net0'}
>> > > {}
>> > > > human-monitor-command {'command-line': 'info network'}
>> > > virtio-net-pci.0:
>> > > index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
>> > >   \ net0: index=0,type=user,net=10.0.2.0,restrict=off
>> >
>> > I think that's "normal" - there used to be problems in the past that the
>> > devices (virtio-net-pci in this case) did not like the netdevs to be 
>> > removed
>> > on the fly. So the netdevs are kept around until you remove the device, too
>> > (i.e. issue a device_del for the virtio-net-pci device).
>> >
>> > > > netdev_add {'type': 'user', 'id': 'net0', 'restrict': False, 
>> > > > 'hostfwd': [{'str': 'tcp:127.0.0.1:58239-:22'}]}
>> > > {}
>> > > > human-monitor-command {'command-line': 'info network'}
>> > > unseal: virtio-net-pci.0:
>> > > index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
>> > >   \ net0: index=0,type=user,net=10.0.2.0,restrict=off
>> > > net0: index=0,type=user,net=10.0.2.0,restrict=off
>> > >
>> > > What's the correct QMP command sequence to modify NetdevUserOptions?
>> >
>> > AFAIK there is no way to modify running netdevs - you'd have to delete the
>> > netdev and the device, and then add both again. But I might have missed
>> > something here, so I CC:-ed some people who might be more familiar with the
>> > details here.
>> >
>> >  Thomas
>> >
>> >
>> > > Please CC me on replies.
>>
>>
>> Wow this really goes to show how wide our feature matrix is.
>>
>> Yes it's probably an unintended side effect but yes it
>> did work it seems, so we really should not just break it
>> without warning.

Depends.  See below.

>> Probably this one:
>>
>> commit 831734cce6494032e9233caff4d8442b3a1e7fef
>> Author: Markus Armbruster 
>> Date:   Wed Nov 25 11:02:20 2020 +0100
>>
>> net: Fix handling of id in netdev_add and netdev_del

   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 
   Fixes: 08712fcb851034228b61f75bd922863a984a4f60
   Cc: qemu-sta...@nongnu.org
   Signed-off-by: Markus Armbruster 
   Reviewed-by: 

Re: modify NetdevUserOptions through QMP in QEMU 6 - how?

2021-12-14 Thread Jason Wang
On Tue, Dec 14, 2021 at 10:53 PM Michael S. Tsirkin  wrote:
>
> On Mon, Dec 13, 2021 at 09:02:15AM +0100, Thomas Huth wrote:
> >  Hi!
> >
> > On 10/12/2021 18.02, Alexander Sosedkin wrote:
> > > With QEMU 5 I could totally issue a QMP netdev_add
> > > with the same ID to adjust the NetdevUserOptions I want,
> > > such as restrict or hostfwd. No deleting needed,
> > > just a netdev_add with what I want changed as a param.
> >
> > I'm a little bit surprised that this worked, since AFAIK there is no code in
> > QEMU to *change* the parameters of a running netdev... likely the code added
> > a new netdev with the same ID, replacing the old one?
> >
> > > With QEMU 6 it started failing, claiming the ID is already used.
> > > And if I do netdev_del + netdev_add, I just lose connectivity.
> > > What's even stranger, I still see old netdev attached in info network:
> > >
> > > > netdev_del {'id': 'net0'}
> > > {}
> > > > human-monitor-command {'command-line': 'info network'}
> > > virtio-net-pci.0:
> > > index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
> > >   \ net0: index=0,type=user,net=10.0.2.0,restrict=off
> >
> > I think that's "normal" - there used to be problems in the past that the
> > devices (virtio-net-pci in this case) did not like the netdevs to be removed
> > on the fly. So the netdevs are kept around until you remove the device, too
> > (i.e. issue a device_del for the virtio-net-pci device).
> >
> > > > netdev_add {'type': 'user', 'id': 'net0', 'restrict': False, 'hostfwd': 
> > > > [{'str': 'tcp:127.0.0.1:58239-:22'}]}
> > > {}
> > > > human-monitor-command {'command-line': 'info network'}
> > > unseal: virtio-net-pci.0:
> > > index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
> > >   \ net0: index=0,type=user,net=10.0.2.0,restrict=off
> > > net0: index=0,type=user,net=10.0.2.0,restrict=off
> > >
> > > What's the correct QMP command sequence to modify NetdevUserOptions?
> >
> > AFAIK there is no way to modify running netdevs - you'd have to delete the
> > netdev and the device, and then add both again. But I might have missed
> > something here, so I CC:-ed some people who might be more familiar with the
> > details here.
> >
> >  Thomas
> >
> >
> > > Please CC me on replies.
>
>
> Wow this really goes to show how wide our feature matrix is.
>
> Yes it's probably an unintended side effect but yes it
> did work it seems, so we really should not just break it
> without warning.
>
>
> Probably this one:
>
> commit 831734cce6494032e9233caff4d8442b3a1e7fef
> Author: Markus Armbruster 
> Date:   Wed Nov 25 11:02:20 2020 +0100
>
> net: Fix handling of id in netdev_add and netdev_del
>
>
>
> Jason, what is your take here?

I might be wrong, but I agree with Thomas. Adding a netdev with the
same ID looks wrong, if it works, it looks like a bug. And I don't
think we support changing netdev properties.

Thanks

>
>
> Alexander, what happens if we just drop the duplicate ID check? Do
> things work for you again?
> Warning: completely untested.
>
> Signed-off-by: Michael S. Tsirkin 
>
>
> diff --git a/net/net.c b/net/net.c
> index f0d14dbfc1..01f5a187b6 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1055,12 +1055,6 @@ 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) {
> --
> MST
>




Re: modify NetdevUserOptions through QMP in QEMU 6 - how?

2021-12-14 Thread Michael S. Tsirkin
On Mon, Dec 13, 2021 at 09:02:15AM +0100, Thomas Huth wrote:
>  Hi!
> 
> On 10/12/2021 18.02, Alexander Sosedkin wrote:
> > With QEMU 5 I could totally issue a QMP netdev_add
> > with the same ID to adjust the NetdevUserOptions I want,
> > such as restrict or hostfwd. No deleting needed,
> > just a netdev_add with what I want changed as a param.
> 
> I'm a little bit surprised that this worked, since AFAIK there is no code in
> QEMU to *change* the parameters of a running netdev... likely the code added
> a new netdev with the same ID, replacing the old one?
> 
> > With QEMU 6 it started failing, claiming the ID is already used.
> > And if I do netdev_del + netdev_add, I just lose connectivity.
> > What's even stranger, I still see old netdev attached in info network:
> > 
> > > netdev_del {'id': 'net0'}
> > {}
> > > human-monitor-command {'command-line': 'info network'}
> > virtio-net-pci.0:
> > index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
> >   \ net0: index=0,type=user,net=10.0.2.0,restrict=off
> 
> I think that's "normal" - there used to be problems in the past that the
> devices (virtio-net-pci in this case) did not like the netdevs to be removed
> on the fly. So the netdevs are kept around until you remove the device, too
> (i.e. issue a device_del for the virtio-net-pci device).
> 
> > > netdev_add {'type': 'user', 'id': 'net0', 'restrict': False, 'hostfwd': 
> > > [{'str': 'tcp:127.0.0.1:58239-:22'}]}
> > {}
> > > human-monitor-command {'command-line': 'info network'}
> > unseal: virtio-net-pci.0:
> > index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
> >   \ net0: index=0,type=user,net=10.0.2.0,restrict=off
> > net0: index=0,type=user,net=10.0.2.0,restrict=off
> > 
> > What's the correct QMP command sequence to modify NetdevUserOptions?
> 
> AFAIK there is no way to modify running netdevs - you'd have to delete the
> netdev and the device, and then add both again. But I might have missed
> something here, so I CC:-ed some people who might be more familiar with the
> details here.
> 
>  Thomas
> 
> 
> > Please CC me on replies.


Wow this really goes to show how wide our feature matrix is.

Yes it's probably an unintended side effect but yes it
did work it seems, so we really should not just break it
without warning.


Probably this one:

commit 831734cce6494032e9233caff4d8442b3a1e7fef
Author: Markus Armbruster 
Date:   Wed Nov 25 11:02:20 2020 +0100

net: Fix handling of id in netdev_add and netdev_del



Jason, what is your take here?


Alexander, what happens if we just drop the duplicate ID check? Do
things work for you again?
Warning: completely untested.

Signed-off-by: Michael S. Tsirkin 


diff --git a/net/net.c b/net/net.c
index f0d14dbfc1..01f5a187b6 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1055,12 +1055,6 @@ 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) {
-- 
MST




Re: modify NetdevUserOptions through QMP in QEMU 6 - how?

2021-12-13 Thread Thomas Huth

 Hi!

On 10/12/2021 18.02, Alexander Sosedkin wrote:

With QEMU 5 I could totally issue a QMP netdev_add
with the same ID to adjust the NetdevUserOptions I want,
such as restrict or hostfwd. No deleting needed,
just a netdev_add with what I want changed as a param.


I'm a little bit surprised that this worked, since AFAIK there is no code in 
QEMU to *change* the parameters of a running netdev... likely the code added 
a new netdev with the same ID, replacing the old one?



With QEMU 6 it started failing, claiming the ID is already used.
And if I do netdev_del + netdev_add, I just lose connectivity.
What's even stranger, I still see old netdev attached in info network:


netdev_del {'id': 'net0'}

{}

human-monitor-command {'command-line': 'info network'}

virtio-net-pci.0:
index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
  \ net0: index=0,type=user,net=10.0.2.0,restrict=off


I think that's "normal" - there used to be problems in the past that the 
devices (virtio-net-pci in this case) did not like the netdevs to be removed 
on the fly. So the netdevs are kept around until you remove the device, too 
(i.e. issue a device_del for the virtio-net-pci device).



netdev_add {'type': 'user', 'id': 'net0', 'restrict': False, 'hostfwd': 
[{'str': 'tcp:127.0.0.1:58239-:22'}]}

{}

human-monitor-command {'command-line': 'info network'}

unseal: virtio-net-pci.0:
index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
  \ net0: index=0,type=user,net=10.0.2.0,restrict=off
net0: index=0,type=user,net=10.0.2.0,restrict=off

What's the correct QMP command sequence to modify NetdevUserOptions?


AFAIK there is no way to modify running netdevs - you'd have to delete the 
netdev and the device, and then add both again. But I might have missed 
something here, so I CC:-ed some people who might be more familiar with the 
details here.


 Thomas



Please CC me on replies.





modify NetdevUserOptions through QMP in QEMU 6 - how?

2021-12-10 Thread Alexander Sosedkin
With QEMU 5 I could totally issue a QMP netdev_add
with the same ID to adjust the NetdevUserOptions I want,
such as restrict or hostfwd. No deleting needed,
just a netdev_add with what I want changed as a param.

With QEMU 6 it started failing, claiming the ID is already used.
And if I do netdev_del + netdev_add, I just lose connectivity.
What's even stranger, I still see old netdev attached in info network:

> netdev_del {'id': 'net0'}
{}
> human-monitor-command {'command-line': 'info network'}
virtio-net-pci.0:
index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
 \ net0: index=0,type=user,net=10.0.2.0,restrict=off

> netdev_add {'type': 'user', 'id': 'net0', 'restrict': False, 'hostfwd': 
> [{'str': 'tcp:127.0.0.1:58239-:22'}]}
{}
> human-monitor-command {'command-line': 'info network'}
unseal: virtio-net-pci.0:
index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
 \ net0: index=0,type=user,net=10.0.2.0,restrict=off
net0: index=0,type=user,net=10.0.2.0,restrict=off

What's the correct QMP command sequence to modify NetdevUserOptions?

Please CC me on replies.