Re: [Qemu-devel] [PATCH 4/7] tests/qgraph: arm/raspi2 machine node

[Philippe Mathieu-Daudé]

On 07/11/2018 12:30 PM, Paolo Bonzini wrote:
> On 11/07/2018 16:59, Stefan Hajnoczi wrote:
>>> +    machine->obj.get_device = raspi2_get_device;
>>> +    machine->obj.destructor = raspi2_destroy;
>>> +    qos_create_sdhci_mm(&machine->sdhci, 0x3f300000, &(QSDHCIProperties) {
>>> +        .version = 3,
>>> +        .baseclock = 52,
>>> +        .capab.sdma = false,
>>> +        .capab.reg = 0x052134b4
>>> +    });
>>> +    return &machine->obj;
>>> +}
>>> +
>>> +static void raspi2(void)
>>> +{
>>> +    qos_node_create_machine("arm/raspi2", qos_create_machine_arm_raspi2);
>>> +    qos_node_contains("arm/raspi2", "generic-sdhci");
>>> +}
>>> +
>>> +libqos_init(raspi2);
>> Hmm...I didn't realize it would be necessary to manually define each
>> machine type.  I thought qgraph would use qemu-system-x86_64
>> -machine/-device/info qtree to introspect QEMU and instantiate the
>> appropriate drivers.
> 
> That doesn't provide enough information yet.  However, PCI devices are
> already discovered automatically and the next step (next year :)) could
> be to use device tree or ACPI to discover embedded devices.
> 
> Using QOM properties instead of duplicating things like QSDHCIProperties
> is a great idea.  Of course the duplication was already there ("old
> sdhci_t structure" in patch 7), so I don't think it should be a blocker,
> but indeed there's a better way to do it.
> 
>> My concern is that this manual approach of defining machines complicates
>> qtests.  This file will need to be modified by multiple people - each of
>> them will be interested in a specific driver and not interested in the
>> driver that other people have added.
> 
> This is exactly the opposite problem that we have now: when you write a
> test you are interested typically only in one machine, and the
> "if(x86)elseif(arm)" gets in the way.  (Also: it's also difficult to
> split the ifs across files, i.e. it scales worse; and there's lots of
> duplicated code across tests to do "for" loops of g_test_add, because
> the ifs don't lend itself to "automatic" generation of test cases via
> path walk).
> 
> The advantage is that (just like for OS vs. QEMU) the structure of this
> file matches closely the board files in hw/arm/, which is something you
> should already be familiar with if you're adding a new board to QEMU.

Some projects organize their tests alongside the source to be tested.
This is often messy, but might be clearer to see source testing coverage.
That said, I'd rather keep it out of 'source' tree (as of now). Using a
simple script we can notice the same (which device models matches a test).