From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37584) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dsyw9-0004bX-85 for qemu-devel@nongnu.org; Fri, 15 Sep 2017 18:18:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dsyw6-0004sO-34 for qemu-devel@nongnu.org; Fri, 15 Sep 2017 18:18:13 -0400 Date: Fri, 15 Sep 2017 19:18:02 -0300 From: Eduardo Habkost Message-ID: <20170915221802.GA10621@localhost.localdomain> References: <1504605227-5124-1-git-send-email-thuth@redhat.com> <871snlwev0.fsf@dusky.pond.sub.org> <20170905164835.GF2112@work-vm> <6e03aa82-3663-c251-c8e1-ac2e7eb0e7e7@redhat.com> <87mv68e2i3.fsf@dusky.pond.sub.org> <20170909204133.GH7570@localhost.localdomain> <825b29f3-7857-558e-9092-855c9f75463e@redhat.com> <20170912173723.GA7570@localhost.localdomain> <808c097f-a422-3aba-2e0d-d8360425f75b@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <808c097f-a422-3aba-2e0d-d8360425f75b@redhat.com> Subject: Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH] tests: Add a device_add/del HMP test List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: Markus Armbruster , qemu-devel@nongnu.org, "Dr. David Alan Gilbert" , qemu-ppc@nongnu.org, Igor Mammedov , John Snow , Andreas =?iso-8859-1?Q?F=E4rber?= , Philippe =?iso-8859-1?Q?Mathieu-Daud=E9?= On Wed, Sep 13, 2017 at 07:45:11AM +0200, Thomas Huth wrote: > On 12.09.2017 19:37, Eduardo Habkost wrote: > > On Mon, Sep 11, 2017 at 08:13:21AM +0200, Thomas Huth wrote: > >> On 09.09.2017 22:41, Eduardo Habkost wrote: > >>> On Wed, Sep 06, 2017 at 08:59:32AM +0200, Markus Armbruster wrote: > >>>> Thomas Huth writes: > >>>> > >>>>> On 05.09.2017 18:48, Dr. David Alan Gilbert wrote: > >>>>>> * Markus Armbruster (armbru@redhat.com) wrote: > >>>>>>> Thomas Huth writes: > >>>>>>> > >>>>>>>> People tend to forget to mark internal devices with "user_creatable = false > >>>>>>>> or hotpluggable = false, and these devices can crash QEMU if added via the > >>>>>>>> HMP monitor. So let's add a test to run through all devices and that tries > >>>>>>>> to add them blindly (without arguments) to see whether this could crash the > >>>>>>>> QEMU instance. > >> [...] > >>>>>>> * The device supports only cold plug with -device, not hot plug with > >>>>>>> device_add. > >>>>> > >>>>> We've got Eduardo's scripts/device-crash-test script for that already, > >>>>> so no need to cover that here. > >>>> > >>>> Point taken. So this test is really about hot plug / unplug. Suggest > >>>> to clarify the commit message: s/add them blindly/hotplug and unplug > >>>> them blindly/. > >>> > >>> We could extend device-crash-test to test device_add too, as it > >>> already has extra code to deal with known crashes and testing > >>> multiple machine-types. Also, any additional code we write to > >>> ensure we add mandatory arguments or plug only to valid buses > >>> would apply to both -device and device_add. I also think Python > >>> test code is easier to maintain and extend, but that's just my > >>> personal preference. > >> > >> Adding device_add/del support to device-crash-test is certainly an > >> option. The problem is that nobody runs it by default, so this won't > >> help to avoid that new problems are being committed to the repository. > >> > >> I think we really should have a test for "make check", too. So would my > >> test be acceptable if I'd rewrite it to use QMP instead (I don't think I > >> could do the full list that Markus mentioned, but at least a basic test > >> via QMP as a start)? > > > > We can run device-crash-test on "make check", we just need to > > choose what's the subset of tests we want to run (because testing > > all machine+device+target combinations would take too long). > > Maybe we should just run it one time for every machine - and try to add > all available devices at once? Yes, it makes sense. I will keep that in mind when trying to implement device_add support on device-crash-test (but if anybody wants to volunteer to implement it, be my guest). -- Eduardo