Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] iotests: Allow 147 to be run concurrently

[Max Reitz]

On 21.01.19 22:02, Eric Blake wrote:
> On 12/21/18 5:47 PM, Max Reitz wrote:
>> To do this, we need to allow creating the NBD server on various ports
>> instead of a single one (which may not even work if you run just one
>> instance, because something entirely else might be using that port).
> 
> Can you instead reuse the ideas from nbd_server_set_tcp_port() from
> qemu-iotests/common.nbd?
> 
>>
>> So we just pick a random port in [32768, 32768 + 1024) and try to create
>> a server there.  If that fails, we just retry until something sticks.
> 
> That has the advantage of checking whether a port is actually in use
> (using 'ss' - although it does limit the test to Linux-only; perhaps
> using socat instead of ss could make the test portable to non-Linux?)

But doesn't that give you race conditions?  That's the point of this
series, so you can run multiple instances of 147 concurrently.

>> For the IPv6 test, we need a different range, though (just above that
>> one).  This is because "localhost" resolves to both 127.0.0.1 and ::1.
>> This means that if you bind to it, it will bind to both, if possible, or
>> just one if the other is already in use.  Therefore, if the IPv6 test
>> has already taken [::1]:some_port and we then try to take
>> localhost:some_port, that will work -- only the second server will be
>> bound to 127.0.0.1:some_port alone and not [::1]:some_port in addition.
>> So we have two different servers on the same port, one for IPv4 and one
>> for IPv6.
>>
>> But when we then try to connect to the server through
>> localhost:some_port, we will always end up at the IPv6 one (as long as
>> it is up), and this may not be the one we want.
>>
>> Thus, we must make sure not to create an IPv6-only NBD server on the
>> same port as a normal "dual-stack" NBD server -- which is done by using
>> distinct port ranges, as explained above.
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>>  tests/qemu-iotests/147 | 98 +++++++++++++++++++++++++++++-------------
>>  1 file changed, 68 insertions(+), 30 deletions(-)
>>
> 
>> @@ -88,17 +92,29 @@ class QemuNBD(NBDBlockdevAddBase):
>>          except OSError:
>>              pass
>>  
>> +    def _try_server_up(self, *args):
>> +        status, msg = qemu_nbd_pipe('-f', imgfmt, test_img, *args)
>> +        if status == 0:
>> +            return True
>> +        if 'Address already in use' in msg:
>> +            return False
>> +        self.fail(msg)
> 
> Do you actually need to attempt a qemu-nbd process, if you take my
> suggestion of using ss to probe for an unused port?  And if not, do we
> still need qemu_nbd_pipe() added earlier in the series?
> 
> 
>> -        address = { 'type': 'inet',
>> -                    'data': {
>> -                        'host': 'localhost',
>> -                        'port': str(NBD_PORT)
>> -                    } }
>> -        self._server_up(address, export_name)
>> +        while True:
>> +            nbd_port = random.randrange(NBD_PORT_START, NBD_PORT_END)
> 
> common.nbd just iterates, instead of trying random ports.

I'm not sure which is better.  Iterating gives guaranteed termination,
trying random ports means the first one you try will usually work.

Max

signature.asc
Description: OpenPGP digital signature