From: "Daniel P. Berrange" <berrange@redhat.com>
To: Knut Omang <knut.omang@oracle.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>,
qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v6 1/4] tests: Add test-listen - a stress test for QEMU socket listen
Date: Mon, 7 Aug 2017 09:45:24 +0100 [thread overview]
Message-ID: <20170807084524.GF18259@redhat.com> (raw)
In-Reply-To: <7c4ddf95a57f7dfa546c11f8a08a9091b8bd1ceb.1501362828.git-series.knut.omang@oracle.com>
On Sat, Jul 29, 2017 at 11:18:15PM +0200, Knut Omang wrote:
> There's a potential race condition between multiple bind()'s
> attempting to bind to the same port, which occasionally
> allows more than one bind to succeed against the same port.
>
> When a subsequent listen() call is made with the same socket
> only one will succeed.
>
> The current QEMU code does however not take this situation into account
> and the listen will cause the code to break out and fail even
> when there are actually available ports to use.
>
> This test exposes two subtests:
>
> /socket/listen-serial
> /socket/listen-compete
>
> The "compete" subtest creates a number of threads and have them all trying to bind
> to the same port with a large enough offset input to
> allow all threads to get it's own port.
> The "serial" subtest just does the same, except in series in a
> single thread.
>
> The serial version passes, probably in most versions of QEMU.
>
> The parallel version exposes the problem in a relatively reliable way,
> eg. it fails a majority of times, but not with a 100% rate, occasional
> passes can be seen. Nevertheless this is quite good given that
> the bug was tricky to reproduce and has been left undetected for
> a while.
>
> The problem seems to be present in all versions of QEMU.
>
> The original failure scenario occurred with VNC port allocation
> in a traditional Xen based build, in different code
> but with similar functionality.
>
> Reported-by: Bhavesh Davda <bhavesh.davda@oracle.com>
> Signed-off-by: Knut Omang <knut.omang@oracle.com>
> Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>
> Reviewed-by: Bhavesh Davda <bhavesh.davda@oracle.com>
> Reviewed-by: Girish Moodalbail <girish.moodalbail@oracle.com>
> ---
> tests/Makefile.include | 2 +-
> tests/test-listen.c | 253 ++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 255 insertions(+)
> create mode 100644 tests/test-listen.c
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 7af278d..b37c0c8 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -128,6 +128,7 @@ check-unit-y += tests/test-bufferiszero$(EXESUF)
> gcov-files-check-bufferiszero-y = util/bufferiszero.c
> check-unit-y += tests/test-uuid$(EXESUF)
> check-unit-y += tests/ptimer-test$(EXESUF)
> +#check-unit-y += tests/test-listen$(EXESUF)
> gcov-files-ptimer-test-y = hw/core/ptimer.c
> check-unit-y += tests/test-qapi-util$(EXESUF)
> gcov-files-test-qapi-util-y = qapi/qapi-util.c
> @@ -769,6 +770,7 @@ tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
> tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o $(test-util-obj-y)
> tests/numa-test$(EXESUF): tests/numa-test.o
> tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o tests/acpi-utils.o
> +tests/test-listen$(EXESUF): tests/test-listen.o $(test-util-obj-y)
>
> tests/migration/stress$(EXESUF): tests/migration/stress.o
> $(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
> diff --git a/tests/test-listen.c b/tests/test-listen.c
> new file mode 100644
> index 0000000..5c07537
> --- /dev/null
> +++ b/tests/test-listen.c
> @@ -0,0 +1,253 @@
> +/*
> + * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
> + * Author: Knut Omang <knut.omang@oracle.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
Can you change that to "version 2 or later" - per the LICENSE file, we don't
accept contributions under "version 2 only" except for 4 specific subdirs:
"As of July 2013, contributions under version 2 of the GNU General Public
License (and no later version) are only accepted for the following files
or directories: bsd-user/, linux-user/, hw/vfio/, hw/xen/xen_pt*."
> + *
> + * Test parallel port listen configuration with
> + * dynamic port allocation
> + */
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 :|
next prev parent reply other threads:[~2017-08-07 8:45 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-29 21:18 [Qemu-devel] [PATCH v6 0/4] Unit test+fix for problem with QEMU handling of multiple bind()s to the same port Knut Omang
2017-07-29 21:18 ` [Qemu-devel] [PATCH v6 1/4] tests: Add test-listen - a stress test for QEMU socket listen Knut Omang
2017-08-07 8:45 ` Daniel P. Berrange [this message]
2017-08-07 9:21 ` Knut Omang
2017-08-07 10:27 ` Daniel P. Berrange
2017-08-07 10:30 ` Knut Omang
2017-07-29 21:18 ` [Qemu-devel] [PATCH v6 2/4] sockets: factor out a new try_bind() function Knut Omang
2017-07-29 21:18 ` [Qemu-devel] [PATCH v6 3/4] sockets: factor out create_fast_reuse_socket Knut Omang
2017-08-07 8:38 ` Daniel P. Berrange
2017-07-29 21:18 ` [Qemu-devel] [PATCH v6 4/4] sockets: Handle race condition between binds to the same port Knut Omang
2017-08-07 8:40 ` Daniel P. Berrange
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=20170807084524.GF18259@redhat.com \
--to=berrange@redhat.com \
--cc=knut.omang@oracle.com \
--cc=kraxel@redhat.com \
--cc=pbonzini@redhat.com \
--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).