Re: [PATCH] tests/qemu_iotests: Minimize usage of used ports

[Eric Blake]

On 2/6/20 10:37 AM, Max Reitz wrote:

> > thank you and I am sorry for not digging deep enough. This week my CI failed 
> > with:
> >
> > 01:24:06 DEBUG| [stdout] +ERROR: test_inet (__main__.QemuNBD)
> > 01:24:06 DEBUG| [stdout] 
> > +----------------------------------------------------------------------
> > 01:24:06 DEBUG| [stdout] +Traceback (most recent call last):
> > 01:24:06 DEBUG| [stdout] +  File "147", line 85, in setUp
> > 01:24:06 DEBUG| [stdout] +    self.vm.launch()
> > 01:24:06 DEBUG| [stdout] +  File 
> > "/home/jenkins/ppc64le/qemu-master/tests/qemu-iotests/../../python/qemu/machine.py",
> >  line 302, in launch
> > 01:24:06 DEBUG| [stdout] +    self._launch()
> > 01:24:06 DEBUG| [stdout] +  File 
> > "/home/jenkins/ppc64le/qemu-master/tests/qemu-iotests/../../python/qemu/machine.py",
> >  line 319, in _launch
> > 01:24:06 DEBUG| [stdout] +    self._pre_launch()
> > 01:24:06 DEBUG| [stdout] +  File 
> > "/home/jenkins/ppc64le/qemu-master/tests/qemu-iotests/../../python/qemu/qtest.py",
> >  line 106, in _pre_launch
> > 01:24:06 DEBUG| [stdout] +    super(QEMUQtestMachine, self)._pre_launch()
> > 01:24:06 DEBUG| [stdout] +  File 
> > "/home/jenkins/ppc64le/qemu-master/tests/qemu-iotests/../../python/qemu/machine.py",
> >  line 270, in _pre_launch
> > 01:24:06 DEBUG| [stdout] +    self._qmp = 
> > qmp.QEMUMonitorProtocol(self._vm_monitor, server=True)
> > 01:24:06 DEBUG| [stdout] +  File 
> > "/home/jenkins/ppc64le/qemu-master/tests/qemu-iotests/../../python/qemu/qmp.py",
> >  line 60, in __init__
> > 01:24:06 DEBUG| [stdout] +    self.__sock.bind(self.__address)
> > 01:24:06 DEBUG| [stdout] +OSError: [Errno 98] Address already in use

Was this test 147?  If so, see:
https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg01469.html

because that failure matches what I was seeing.

> > I made the mistake of reproducing this on my home system using the qemu 
> > revision that I had and assuming it's caused by a used port. So I limited the 
> > port range and used nc to occupy the port. It sort-of reproduced but instead of 
> > Address already in use it hanged until I kill the nc process. Then it failed 
> > with:
> >
> > +Traceback (most recent call last):
> > +  File "147", line 124, in test_inet
> > +    flatten_sock_addr(address))
> > +  File "147", line 59, in client_test
> > +    self.assert_qmp(result, 'return', {})
> > +  File "/home/medic/Work/Projekty/qemu/tests/qemu-iotests/iotests.py", line 
> > 821, in assert_qmp
> > +    result = self.dictpath(d, path)
> > +  File "/home/medic/Work/Projekty/qemu/tests/qemu-iotests/iotests.py", line 
> > 797, in dictpath
> > +    self.fail('failed path traversal for "%s" in "%s"' % (path, str(d)))
> > +AssertionError: failed path traversal for "return" in "{'error': {'class': 
> > 'GenericError', 'desc': 'Failed to read initial magic: Unexpected end-of-file before all bytes were 
> > read'}}"

That's a secondary failure, I assume if the initial bug is fixed we are 
less likely to hit the secondary one; but the secondary one may still be 
worth fixing.

> > After a brief study I thought qemu is not doing the job well enough and wanted to add a 
> > protection. Anyway after a more thorough overview I came to a different conclusion and 
> > that is that we are facing the same issue as with incoming migration about a year ago. 
> > What happened is that I started "nc -l localhost 32789" which results in:
> >
> > COMMAND   PID  USER   FD   TYPE  DEVICE SIZE/OFF NODE NAME
> > nc      26758 medic    3u  IPv6 9579487      0t0  TCP localhost:32789 (LISTEN)
> >
> > Then we start the server by "_try_server_up" where qemu-nbd detects the port is 
> > occupied on IPv6 but available on IPv4, so it claims it:
> > COMMAND   PID  USER   FD   TYPE  DEVICE SIZE/OFF NODE NAME
> > nc        26758 medic    3u  IPv6 9579487      0t0  TCP localhost:32789 (LISTEN)
> > qemu-nbd  26927 medic    4u  IPv4 9591857      0t0  TCP localhost:32789 (LISTEN)
> >
> > and reports success. Then we try to connect but the hotplugged VM first 
> > attempts to connect on the IPv6 address and hangs for infinity.
> >
> > Now is this an expected behavior? If so then we need the find_free_address (but 
> > preferably directly in _try_server_up just before starting the qemu-nbd) to 
> > leave as little time-frame for collision as possible. Otherwise the test is 
> > alright and qemu-nbd needs a fix to bail out in case some address is already 
> > used (IIRC this is what incoming migration does).
>
> Ah, OK.
>
> Well, expected behavior...  It’s a shame, that’s what it is.

In libnbd, we recently improved the testsuite by switching over to 
systemd-style fd passing: instead of asking qemu-nbd to open a random 
port (and hoping it is available), we instead pre-open the port (where 
failure is under our control) and then pass in that fd with environment 
variables to qemu-nbd, which in turn guarantees that qemu-nbd won't hit 
failures in trying to use the port.  Maybe we should utilize that more 
in qemu's own testsuite.

Also, I need to revisit my proposed patches for letting qemu-nbd support 
TLS over Unix sockets, as that's another way to avoid TCP contention 
(right now, qemu has an anachronistic prohibition against the 
combination of TLS and Unix sockets).

> > My second mistake was testing this on the old code-base and rebasing it only 
> > before sending the patch (without testing after the rebase). If I were to test 
> > it first, I would have found out that the real reproducer is simply running the 
> > test as the commit 8dff69b9415b4287e900358744b732195e1ab2e2 broke it.
> >
> >
> > So basically there are 2 actions:
> >
> > 1. fix the test as on my system it fails in 100% of cases, bisect says the 
> > first bad commit is 8dff69b9415b4287e900358744b732195e1ab2e2. Would anyone have 
> > time in digging into this? I already spent way too much time on this and don't 
> > really know what that commit is trying to do.
>
> Yep, I’ve sent a patch:
>
> https://lists.nongnu.org/archive/html/qemu-block/2020-02/msg00294.html

Ah, so we did notice the same problem.

> > 2. decide on the behavior when IPv4/6 is already in use (bail-out or start).
> > 2a. In case it should bail-out than the test is correct and there is no need 
> > for my patch. On the other hand qemu-nbd has to be fixed
>
> I don’t think it makes much sense to let qemu’s NBD server ensure that
> it claims both IPv4 and IPv6 in case the user specifies a
> non-descriptive hostname.
>
> > 2b. Otherwise I can send a v2 that will check the port in the _try_server_up 
> > just before starting qemu-nbd to minimize the risk of using a utilized port (or 
> > should you decide it's not worth checking, I can simply forget about this)
>
> Hm.  It wouldn’t be fully reliable, but, well...  The risk would be minimal.
>
> OTOH, would it work if we just did a %s/localhost/127.0.0.1/ in the
> test?  We have specific cases for IPv6, so I think it makes sense to
> force IPv4 in all other cases.

Except then it will fail on machines configured for IPv6-only.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org