From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58398) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zm3Bz-00040T-4W for qemu-devel@nongnu.org; Tue, 13 Oct 2015 13:16:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zm3Bw-0002n1-Cz for qemu-devel@nongnu.org; Tue, 13 Oct 2015 13:16:51 -0400 Received: from mx6-phx2.redhat.com ([209.132.183.39]:56078) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zm3Bw-0002mr-5E for qemu-devel@nongnu.org; Tue, 13 Oct 2015 13:16:48 -0400 Date: Tue, 13 Oct 2015 13:16:40 -0400 (EDT) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Message-ID: <1301277322.30257806.1444756600732.JavaMail.zimbra@redhat.com> In-Reply-To: <561D39B6.4070405@redhat.com> References: <1444746378-12338-1-git-send-email-marcandre.lureau@redhat.com> <2086792899.30234228.1444752646175.JavaMail.zimbra@redhat.com> <561D39B6.4070405@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PULL v3 00/51] Ivshmem patches List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: Peter Maydell , Andrew Jones , Claudio Fontana , QEMU Developers , Stefan Hajnoczi , =?utf-8?Q?Marc-Andr=C3=A9?= Lureau , Paolo Bonzini , Cam Macdonell ----- Original Message ----- >=20 >=20 > On 10/13/2015 12:10 PM, Marc-Andr=C3=A9 Lureau wrote: > > Hi > >=20 > > ----- Original Message ----- > >> On 13 October 2015 at 15:25, wrote: > >>> From: Marc-Andr=C3=A9 Lureau > >>> > >>> The following changes since commit > >>> c49d3411faae8ffaab8f7e5db47405a008411c10: > >>> > >>> Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10= -12' > >>> into staging (2015-10-13 10:42:06 +0100) > >>> > >>> are available in the git repository at: > >>> > >>> git@github.com:elmarco/qemu.git tags/ivshmem-pull-request > >>> > >>> for you to fetch changes up to feb3f96c4ff1613dd4d0bebda09fe349f8c3e2= dd: > >>> > >>> doc: document ivshmem & hugepages (2015-10-13 15:29:53 +0200) > >>> > >>> ---------------------------------------------------------------- > >>> v3 with build fixes on osx & x86 > >>> ---------------------------------------------------------------- > >> > >> This asserts in the tests on OSX: > >> > >> GTESTER check-qtest-i386 > >> blkdebug: Suspended request 'A' > >> blkdebug: Resuming request 'A' > >> Using POSIX shared memory: /qtest-68262-3644687833 > >> ftruncate(/qtest-68262-3644687833) failed: Invalid argument > >> ** > >=20 > > I'll try to reproduce on freebsd. It's weird that this ftruncate() woul= d > > fail on osx but not on linux, perhaps a osx security? > >=20 > >> ERROR:/Users/pm215/src/qemu-for-merges/tests/ivshmem-test.c:299:void > >> test_ivshmem_server(): assertion failed (ret =3D=3D 0): (-1 =3D=3D 0) > >> GTester: last random seed: R02S141a4c6774f852248b61ebcd666b7ad5 > >> > >> (I'm afraid I didn't notice this in earlier testing because > >> for some reason I'm not clear on an assertion failure doesn't > >> always cause the test harness to fail.) > >> > >> Some asides, which you should look into but which don't need > >> to be fixed for this pull request: > >> * having the test use 'is QTEST_LOG set' as its "should we be verbose > >> in the server failure path" is not terribly helpful because QTEST_LOG > >> enables vast volumes of libqtest tracing of communications between > >> qemu and the test harness, and anything else is lost in the noise > >=20 > > What do you suggest instead? > >=20 >=20 > https://developer.gnome.org/glib/stable/glib-Testing.html#g-test-verbose Yes, but the question is why we have QTEST_LOG then :) I don't mind much us= ing g_test_verbose(), except it is a different way to enable logging, also = it could spew a lot of debug too :) >=20 > >> * ivshmem_server_init() has uses of IVSHMEM_SERVER_DEBUG before the > >> verbose flag is copied into server->verbose, which means they won't > >> print things out when they should > >=20 > > True, I moved it up. > >=20 > >> * ivshmem_server_start() is inconsistent about whether it wants > >> to report "something failed messages to stderr or via the debug macro > >=20 > > stderr is for fatal errors. Only gethugepagesize() that I added uses st= derr > > too because it's a generic function. I'll make it take IvshmemServer > > argument and use IVSHMEM_SERVER_DEBUG. > >=20 > >> * ivshmem_server_ftruncate() is using some bizarre code to > >> align up to a power of 2. We have pow2ceil() for this > >=20 > > ok > >=20 > >> * Printing "Using POSIX shared memory" in the test output for a > >> normal non-verbose test run isn't great: generally our tests should > >> be silent except regarding failures > >=20 > > This was added by earlier reviewer request. I guess during make check, > > there could be a way to make it silent instead. > >=20 > > thanks > >=20 >=20 >=20