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

[Alexander Sosedkin]

On Tue, Dec 14, 2021 at 3:53 PM Michael S. Tsirkin <mst@redhat.com> 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 <armbru@redhat.com>
> 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 <mst@redhat.com>
>
>
> 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.

qemu-netdev-reproducer.py
Description: Text Data

qemu-netdev.qemu5.log
Description: Text Data

qemu-netdev.qemu6.log
Description: Text Data

qemu-netdev.qemu6-nocheck.log
Description: Text Data