qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Max Reitz <mreitz@redhat.com>, qemu-block@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	qemu-devel@nongnu.org, "Daniel P. Berrangé" <berrange@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 3/3] iotests: Allow 147 to be run concurrently
Date: Wed, 23 Jan 2019 08:33:41 -0600	[thread overview]
Message-ID: <c63ab4a5-7e28-2a21-c16e-a114a6ce7358@redhat.com> (raw)
In-Reply-To: <3bf30595-c8fc-9058-3860-c4640718b33d@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2693 bytes --]

On 1/23/19 7:12 AM, Max Reitz wrote:
> 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.

Hmm - that does imply that common.nbd's use of ss IS racy because it
checks in linear fashion and has a TOCTTOU window (affects at least
iotest 233). Your observation that random probes within a range are less
susceptible (although not immune) to the race is correct.

>> 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.

Is there any other way we can make the test more robust, perhaps by
using socket activation (that is, pre-open the port prior to starting
qemu_nbd, so that our code for finding a free socket is more easily
reusable), or by using Unix sockets for test 147 (that test seems to be
using TCP sockets only as a means to get to the real feature under test,
and not as the actual thing being tested)?

Hmm, and you made me realize that socket activation is NOT documented in
'man qemu-nbd'; I ought to fix that.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2019-01-23 14:34 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-21 23:47 [Qemu-devel] [PATCH 0/3] iotests: Allow 147 to be run concurrently Max Reitz
2018-12-21 23:47 ` [Qemu-devel] [PATCH 1/3] iotests.py: Add qemu_nbd_pipe() Max Reitz
2019-01-21 20:55   ` Eric Blake
2019-01-23 13:06     ` Max Reitz
2019-01-23 14:27       ` Eric Blake
2018-12-21 23:47 ` [Qemu-devel] [PATCH 2/3] iotests: Bind qemu-nbd to localhost in 147 Max Reitz
2019-01-21 20:56   ` Eric Blake
2018-12-21 23:47 ` [Qemu-devel] [PATCH 3/3] iotests: Allow 147 to be run concurrently Max Reitz
2019-01-21 20:50   ` [Qemu-devel] [Qemu-block] " John Snow
2019-01-23 13:05     ` Max Reitz
2019-01-23 17:34       ` Max Reitz
2019-01-23 17:47         ` John Snow
2019-01-25 15:01           ` Max Reitz
2019-01-21 21:02   ` [Qemu-devel] " Eric Blake
2019-01-23 13:12     ` Max Reitz
2019-01-23 14:33       ` Eric Blake [this message]
2019-01-23 14:37         ` Max Reitz
2019-01-23 14:43         ` Daniel P. Berrangé
2019-01-31  1:01 ` [Qemu-devel] [PATCH 0/3] " Max Reitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c63ab4a5-7e28-2a21-c16e-a114a6ce7358@redhat.com \
    --to=eblake@redhat.com \
    --cc=berrange@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).