Re: [Qemu-devel] [PATCH 6/9] tests/libqos/pci: Clean up string interpolation into QMP input

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 07/21/2017 08:53 AM, Markus Armbruster wrote:
>> Leaving interpolation into JSON to qmp() is more robust than building
>> QMP input manually, as explained in the commit before previous.
>> 
>> The case in qpci_plug_device_test() is a bit complicated: it
>> interpolates several JSON object members, not just a value.  Clean it
>> up by passing them in as QDict rather than string, so we can leave
>> interpolation to qmp() here and to qobject_from_jsonf() in callers.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
>> 
>> +    response = qmp("{'execute': 'device_add', 'arguments': %p }", args);
>> +
>
> I like this; in fact, in my earlier series attempting to remove
> qobject_from_jsonf(), I heavily relied on %p being useful, to the point
> that I even added a helper function to make it easier (off-hand, it was
> something like qmp_execute(const char *command, QDict *arguments))
>
>> @@ -674,6 +676,7 @@ static void pci_hotplug(void)
>>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>          qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
>>      }
>> +
>>      qtest_shutdown(qs);
>
> Spurious whitespace addition?

Is it spurious if the result looks better?  ;)

There's a blank line after pci_test_start(), and that makes me want one
before the matching qtest_shutdown().

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!