All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>,
	qemu-block@nongnu.org, Kevin Wolf <kwolf@redhat.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 3/3] iotests: Allow 147 to be run concurrently
Date: Wed, 23 Jan 2019 14:43:20 +0000	[thread overview]
Message-ID: <20190123144320.GJ27270@redhat.com> (raw)
In-Reply-To: <c63ab4a5-7e28-2a21-c16e-a114a6ce7358@redhat.com>

On Wed, Jan 23, 2019 at 08:33:41AM -0600, Eric Blake wrote:
> 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)?

The problem with using socket activation is that you then are not getting
test coverage of the non-activation code paths which are quite significant
things we really want to be testing.

I do wonder if there's a case to be made for having iotests run inside a
container with private network namespace such that they then have a
predictable environment.  You could then simply declare that if a test
needs a TCP port, it should use  "port 9000 + $TEST_NUM". So every
test can safely run in parallel.

If the entire test harness needs to be run multiple in parallel each
run woudl be a separate container, and so again avoid clashing.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

  parent reply	other threads:[~2019-01-23 14:43 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
2019-01-23 14:37         ` Max Reitz
2019-01-23 14:43         ` Daniel P. Berrangé [this message]
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=20190123144320.GJ27270@redhat.com \
    --to=berrange@redhat.com \
    --cc=eblake@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.