On 17.08.2017 12:57, Thomas Huth wrote:
> On 17.08.2017 11:53, David Hildenbrand wrote:
>> On 17.08.2017 08:25, Thomas Huth wrote:
>>> A lot of tests provide code for adding and removing a device via the
>>> device_add and device_del QMP commands. Maintaining this code in so
>>> many places is cumbersome and error-prone (some of the code parts
>>> check the responses in an incorrect way, for example), so let's
>>> provide some proper generic qtest functions for adding and removing a
>>> device instead.
>>>
>>> Signed-off-by: Thomas Huth <th...@redhat.com>
>>> ---
> [...]
>>> diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
>>> index 2dcdead..aada753 100644
>>> --- a/tests/libqos/pci.c
>>> +++ b/tests/libqos/pci.c
>>> @@ -394,21 +394,6 @@ QPCIBar qpci_legacy_iomap(QPCIDevice *dev, uint16_t
>>> addr)
>>> void qpci_plug_device_test(const char *driver, const char *id,
>>> uint8_t slot, const char *opts)
>>> {
>>> - QDict *response;
>>> - char *cmd;
>>> -
>>> - cmd = g_strdup_printf("{'execute': 'device_add',"
>>> - " 'arguments': {"
>>> - " 'driver': '%s',"
>>> - " 'addr': '%d',"
>>> - " %s%s"
>>> - " 'id': '%s'"
>>> - "}}", driver, slot,
>>> - opts ? opts : "", opts ? "," : "",
>>> - id);
>>> - response = qmp(cmd);
>>> - g_free(cmd);
>>> - g_assert(response);
>>> - g_assert(!qdict_haskey(response, "error"));
>>> - QDECREF(response);
>>> + qtest_hot_plug_device(driver, id, "'addr': '%d'%s%s", slot,
>>> + opts ? ", " : "", opts ? opts : "");
>
> While PCI uses 'addr' as additional parameter here...
so addr would be needed in addition.
>
> [...]
>>> + qtest_hot_plug_device("usb-tablet", id, "'port': '%d', 'bus': '%s.0'",
>>> + port, hcd_id);
>
> ... USB uses 'port' and 'bus' ...
>
> [...]
>>
>> Not sure if it would be better to avoid the fmt and provide more
>> parameters instead. Specifying a parameter as NULL will ifgnore it.
>>
>> void qtest_hot_plug_device(const char *driver, const char *id,
>> const char *drive, const char *port,
>> const char *bus)
>>
>> that should cover all cases and callers don't have to build strings in a
>> special format.
>
> ... so I'm afraid, but no, this does not work. The additional parameters
> are specific to the device type, so there is no way to get around the
> format string if we want to stay flexible here.
It will work, but yes it is not flexible. Just an idea because I don't
like having to specify json strings on a function.
>
> Thomas
>
--
Thanks,
David
