On Tue, Dec 14, 2021 at 3:53 PM Michael S. Tsirkin <m...@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 <arm...@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 <m...@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.
# fluff
import atexit
import json
import socket
import subprocess
import time
QMP_PORT = 12345
TEST_PORT = 12346
QEMU = 'qemu-system-x86_64'
ARGS = ['-nographic',
'-qmp', f'tcp:127.0.0.1:{QMP_PORT},server,nowait,nodelay',
'-device', 'virtio-net-pci,netdev=net0',
'-netdev', 'user,id=net0,restrict=on']
p = subprocess.Popen([QEMU] + ARGS, stdout=subprocess.DEVNULL)
atexit.register(p.kill)
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)
time.sleep(.2)
try:
print('QMP connecting, attempt 1/2')
sock.connect(('127.0.0.1', QMP_PORT))
except ConnectionRefusedError:
print('QMP connecting, attempt 2/2')
time.sleep(2)
sock.connect(('127.0.0.1', QMP_PORT))
print('QMP connected...')
def execute(cmd, **kwargs):
sock.send(json.dumps(
{'execute': cmd, 'arguments': kwargs} if kwargs else {'execute': cmd}
).encode())
print('executed:', cmd, kwargs)
def expect(validator_func=None):
r = b''
while True:
r += sock.recv(1)
if r[-1] == ord('\n'):
reply = json.loads(r.decode())
print(f'received: {reply}')
if validator_func is not None:
assert validator_func(reply)
return reply
execute('qmp_capabilities')
expect(lambda r: 'QMP' in r)
# inspect as started up
execute('human-monitor-command', **{'command-line': 'info network'})
expect(lambda r: r == {'return': {}})
reply = expect(lambda r: set(r.keys()) == {'return'})
# Should be:
# virtio-net-pci.0: index=0,type=nic,model=virtio-net-pci,macaddr=...
# \ net0: index=0,type=user,net=10.0.2.0,restrict=on
assert('restrict=on' in reply['return'])
print()
# modify
execute('netdev_add', type='user', id='net0',
restrict=False, # unrestrict
hostfwd=[{'str': f'tcp:127.0.0.1:{TEST_PORT}-:1'}])
# ^ *not* a list of strings as the documentation claims
# inspect modified
execute('human-monitor-command', **{'command-line': 'info network'})
expect(lambda r: r == {'return': {}})
reply = expect(lambda r: set(r.keys()) == {'return'})
# And now we have two net0s:
# virtio-net-pci.0: index=0,type=nic,model=virtio-net-pci,macaddr=...
# \ net0: index=0,type=user,net=10.0.2.0,restrict=on
# \ net0: index=0,type=user,net=10.0.2.0,restrict=off
assert('restrict=on' in reply['return'])
assert('restrict=off' in reply['return'])
# connect to modified
sock_test = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
time.sleep(.2)
sock_test.connect(('127.0.0.1', TEST_PORT))
sock_test.close()
print(f'port {TEST_PORT} accepting connections')
QMP connecting, attempt 1/2
QMP connected...
executed: qmp_capabilities {}
received: {'QMP': {'version': {'qemu': {'micro': 0, 'minor': 2, 'major': 5}, 'package': 'qemu-5.2.0-8.fc34'}, 'capabilities': ['oob']}}
executed: human-monitor-command {'command-line': 'info network'}
received: {'return': {}}
received: {'return': 'virtio-net-pci.0: index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56\r\n \\ net0: index=0,type=user,net=10.0.2.0,restrict=on\r\n'}
executed: netdev_add {'type': 'user', 'id': 'net0', 'restrict': False, 'hostfwd': [{'str': 'tcp:127.0.0.1:12346-:1'}]}
executed: human-monitor-command {'command-line': 'info network'}
received: {'return': {}}
received: {'return': 'virtio-net-pci.0: index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56\r\n \\ net0: index=0,type=user,net=10.0.2.0,restrict=on\r\nnet0: index=0,type=user,net=10.0.2.0,restrict=off\r\n'}
port 12346 accepting connections
QMP connecting, attempt 1/2
QMP connected...
executed: qmp_capabilities {}
received: {'QMP': {'version': {'qemu': {'micro': 0, 'minor': 1, 'major': 6}, 'package': ''}, 'capabilities': ['oob']}}
executed: human-monitor-command {'command-line': 'info network'}
received: {'return': {}}
received: {'return': 'virtio-net-pci.0: index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56\r\n \\ net0: index=0,type=user,net=10.0.2.0,restrict=on\r\n'}
executed: netdev_add {'type': 'user', 'id': 'net0', 'restrict': False, 'hostfwd': [{'str': 'tcp:127.0.0.1:12346-:1'}]}
executed: human-monitor-command {'command-line': 'info network'}
received: {'error': {'class': 'GenericError', 'desc': "Duplicate ID 'net0'"}}
Traceback (most recent call last):
File "/home/asosedki/reproducers/qemu-netdev-reproducer.py", line 79, in <module>
expect(lambda r: r == {'return': {}})
File "/home/asosedki/reproducers/qemu-netdev-reproducer.py", line 48, in expect
assert validator_func(reply)
AssertionError
QMP connecting, attempt 1/2
QMP connected...
executed: qmp_capabilities {}
received: {'QMP': {'version': {'qemu': {'micro': 0, 'minor': 1, 'major': 6}, 'package': ''}, 'capabilities': ['oob']}}
executed: human-monitor-command {'command-line': 'info network'}
received: {'return': {}}
received: {'return': 'virtio-net-pci.0: index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56\r\n \\ net0: index=0,type=user,net=10.0.2.0,restrict=on\r\n'}
executed: netdev_add {'type': 'user', 'id': 'net0', 'restrict': False, 'hostfwd': [{'str': 'tcp:127.0.0.1:12346-:1'}]}
executed: human-monitor-command {'command-line': 'info network'}
received: {'return': {}}
received: {'return': 'virtio-net-pci.0: index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56\r\n \\ net0: index=0,type=user,net=10.0.2.0,restrict=on\r\nnet0: index=0,type=user,net=10.0.2.0,restrict=off\r\n'}
port 12346 accepting connections
