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)? >>>> If I'm reading the code right it's creating the device with the same >>>> name as the device; I wonder if that always works? >>> >>> Why not? The id is just an arbitrary string, isn't it? >> >> Since you're using HMP, you get to quote ',', which occurs in some >> device names[*]. Enjoy! ;-P >> >> Picking IDs that aren't anti-social may be easier. I'm considering to fail the test if it detects a device with a ',' in its name. Such devices should really not be there in QEMU... Thomas
