On 16.09.2017 00:18, Eduardo Habkost wrote: > 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 <th...@redhat.com> writes: >>>>>> >>>>>>> On 05.09.2017 18:48, Dr. David Alan Gilbert wrote: >>>>>>>> * Markus Armbruster (arm...@redhat.com) wrote: >>>>>>>>> Thomas Huth <th...@redhat.com> 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).
Never mind, that was a unrealistic idea, since there are very likely devices in the list that prevent QEMU from starting if a certain property is missing... so we likely won't catch any crashes with such a test (unless we want to pedanticly maintain a blacklist of devices which can not be used in that test ... you certainly got a very good start for that in device-crash-test already, but I think the list will rather explode if we want to get it usable for this idea?) Thomas