From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41368) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XFJxb-0003Tu-7G for qemu-devel@nongnu.org; Thu, 07 Aug 2014 05:26:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XFJxV-0008Sa-1y for qemu-devel@nongnu.org; Thu, 07 Aug 2014 05:26:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:61161) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XFJxU-0008OP-Ri for qemu-devel@nongnu.org; Thu, 07 Aug 2014 05:26:05 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s779Q38Z009546 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Thu, 7 Aug 2014 05:26:03 -0400 Date: Thu, 7 Aug 2014 10:26:01 +0100 From: Stefan Hajnoczi Message-ID: <20140807092601.GA32038@stefanha-thinkpad.redhat.com> References: <1407186691-16103-1-git-send-email-jsnow@redhat.com> <20140806094327.GB17426@stefanha-thinkpad.redhat.com> <87egwtsvmr.fsf@blackfin.pond.sub.org> <53E25CCF.9090000@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="UlVJffcvxoiEqYs2" Content-Disposition: inline In-Reply-To: <53E25CCF.9090000@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 00/30] AHCI test suite framework List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: pbonzini@redhat.com, mst@redhat.com, Markus Armbruster , qemu-devel@nongnu.org --UlVJffcvxoiEqYs2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 06, 2014 at 12:50:23PM -0400, John Snow wrote: >=20 >=20 > On 08/06/2014 07:30 AM, Markus Armbruster wrote: > >Stefan Hajnoczi writes: > > > >>On Mon, Aug 04, 2014 at 05:11:01PM -0400, John Snow wrote: > >>>This patch series introduces a number of small fixes and tweaks to > >>>help support an AHCI test suite that in the future I hope to expand > >>>to a fuller regression suite to help guide the development of the > >>>AHCI device support under, in particular, the Q35 machine type in QEMU. > >>> > >>>Paolo Bonzini has contributed a number of cleanup and refactoring patc= hes > >>>that support changes to the PIO setup FIS packet construction code, wh= ich > >>>is necessary for testing ths specification adherence of the IDENTIFY c= ommand, > >>>which issues its data exclusively via PIO mechanisms. > >>> > >>>The ahci-test code being checked in represents a minimum of functional= ity > >>>needed in order to issue and receive commands from the AHCI HBA under = the > >>>libqos / qtest environment. > >>> > >>>In V2, as detailed below, these tests are not currently expected to pa= ss. > >>>I will post a complementary patch outside of this set that highlights > >>>the exact set of tests that will not pass, which can help verify at le= ast > >>>the portions of these tests that do work correctly. > >>> > >>>Assertions that currently fail: > >>> - Ordering of PCI capabilities as defined by either AHCI or Intel= ICH9 > >>> - Boot-time values of the PxTFD register, which should not have v= alid > >>> data until after a D2H FIS is received, but does in Qemu 2.1 > >>> - Boot-time values of the PxSIG register, which should have a spe= cific > >>> placeholder signature until the first D2H FIS is received, but = is > >>> currently blank. > >>> - The "Descriptor Processed" interrupt is expected after the IDEN= TIFY > >>> command exhausts the given PRDT, but is not seen. > >> > >>I guess these are the assertion failures: > >>ERROR:tests/ahci-test.c:777:ahci_test_pci_spec: assertion failed > >>((data & 0xFF) =3D=3D PCI_CAP_ID_MSI): (0x00000012 =3D=3D 0x00000005) > >>GTester: last random seed: R02Sd92815a5d013e8433808b903b2b13fb0 > >>** > >>ERROR:tests/ahci-test.c:1165:ahci_test_port_spec: assertion failed > >>((reg) & ((0x01)) =3D=3D ((0x01))): (0x00000000 =3D=3D 0x00000001) > >>GTester: last random seed: R02S4d6c05e864dc777e64141cdc6d2a18cf > >>** > >>ERROR:tests/ahci-test.c:1360:ahci_test_identify: assertion failed > >>((reg) & ((0x20)) =3D=3D ((0x20))): (0x00000000 =3D=3D 0x00000020) > >>GTester: last random seed: R02S2b3b330b83a66badb24da80b48120b1d > >> > >>Why publish this patch series if the test fails? We can't merge failing > >>tests. > > > >Correct. > > > >What I do when I want to start some bug fixing work with tests is to > >write the tests to expect the actual, incorrect behavior, with a > >greppable comment documenting the correct behavior. Then clean that up > >as the bugs get fixed. > > >=20 > I thought it was valid to submit a failing test if... well, the behavior = was > wrong. Stefan said no warnings, so I took that to mean "This should fail.= " I > didn't think it was too strange to have a failing test for something that > was not feature complete. >=20 > So, if it's not appropriate to have a failing test at any stage (Regressi= ons > only?) now's a good time to let me know how you would like me to accompli= sh > no warnings but have the tests pass. In my V1 I did just print a "WARN" > string which was reasonable greppable to find the failure cases. >=20 > My next guess at something workable would be to stick the assertions behi= nd > a bool that could be toggled on/off via a flag that could be toggled with > --all or similar to hit the expected failure cases. No warnings inside of > the test harness, no failures, and cases could be found by grepping the n= ame > of the boolean and/or some accompanying comment. Note for the mailing list: John and I had an IRC chat where I mentioned that the git history must always be bisectable, which is the fundamental reason why failing tests cannot be merged. My suggestion is to post-pone tests that are not fixed by this patch series, and I like Markus idea to assert the incorrect behavior (with a comment) in the meantime, too. Stefan --UlVJffcvxoiEqYs2 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJT40YpAAoJEJykq7OBq3PIXcgIALFGNDHCyb9jUo+zjMPW+8cN CMo7l0vSMGsKTuHWDDLtjXeOgmBzqvv41INosnlrn5dVkMJ+shTXe7MjnqO6ErEq G08UWhjoa2rntoBRC9htx/OrchvIjAqQue4nGZ/gwgKkgy/1d6fbG3ZNbgfHhH2y brGo2uSsusCtP6eu3Y4kBWM7pfldzaL6+/I+eD7e+lGUWW/2UL5cT8opqDw6QMaf GSeaaZ2Xm62YKhILRvuWgdk6u8OrfWsDcDbNBAOJKEnAcFjUOakmWmZNN9iLxEtA I/D/paaRm0Y3Ntg0aGKttrVftxRcMUDBRx6RuXUhya9aLqZiPT985aUzOAmivoc= =EOIy -----END PGP SIGNATURE----- --UlVJffcvxoiEqYs2--