Re: [Qemu-devel] [PATCH v4 3/7] libqtest: New hmp() & friends

[Markus Armbruster]

Thomas Huth <address@hidden> writes:

> On 28/09/15 22:08, Markus Armbruster wrote:
>> New convenience function hmp() to facilitate use of
>> human-monitor-command in tests.  Use it to simplify its existing uses.
>> 
>> To blend into existing libqtest code, also add qtest_hmpv() and
>> qtest_hmp().  That, and the egregiously verbose GTK-Doc comment format
>> make this patch look bigger than it is.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>> ---
>>  tests/drive_del-test.c | 22 ++++++----------------
>>  tests/ide-test.c       |  8 ++------
>>  tests/libqtest.c       | 37 +++++++++++++++++++++++++++++++++++++
>>  tests/libqtest.h       | 33 +++++++++++++++++++++++++++++++++
>>  4 files changed, 78 insertions(+), 22 deletions(-)
>> 
>> diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c
>> index 8951f6f..3390946 100644
>> --- a/tests/drive_del-test.c
>> +++ b/tests/drive_del-test.c
>> @@ -16,28 +16,18 @@
>>  
>>  static void drive_add(void)
>>  {
>> -    QDict *response;
>> +    char *resp = hmp("drive_add 0 if=none,id=drive0");
>>  
>> -    response = qmp("{'execute': 'human-monitor-command',"
>> -                   " 'arguments': {"
>> -                   "   'command-line': 'drive_add 0 if=none,id=drive0'"
>> -                   "}}");
>> -    g_assert(response);
>> -    g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, "OK\r\n");
>> -    QDECREF(response);
>> +    g_assert_cmpstr(resp, ==, "OK\r\n");
>> +    g_free(resp);
>>  }
>>  
>>  static void drive_del(void)
>>  {
>> -    QDict *response;
>> +    char *resp = hmp("drive_del drive0");
>>  
>> -    response = qmp("{'execute': 'human-monitor-command',"
>> -                   " 'arguments': {"
>> -                   "   'command-line': 'drive_del drive0'"
>> -                   "}}");
>> -    g_assert(response);
>> -    g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, "");
>> -    QDECREF(response);
>> +    g_assert_cmpstr(resp, ==, "");
>> +    g_free(resp);
>>  }
>>  
>>  static void device_del(void)
>> diff --git a/tests/ide-test.c b/tests/ide-test.c
>> index 5594738..6173dfa 100644
>> --- a/tests/ide-test.c
>> +++ b/tests/ide-test.c
>> @@ -510,9 +510,7 @@ static void test_flush(void)
>>          tmp_path);
>>  
>>      /* Delay the completion of the flush request until we explicitly do it 
>> */
>> -    qmp_discard_response("{'execute':'human-monitor-command', 'arguments': 
>> {"
>> -                         " 'command-line':"
>> -                         " 'qemu-io ide0-hd0 \"break flush_to_os A\"'} }");
>> +    g_free(hmp("qemu-io ide0-hd0 \"break flush_to_os A\""));
>>  
>>      /* FLUSH CACHE command on device 0*/
>>      outb(IDE_BASE + reg_device, 0);
>> @@ -524,9 +522,7 @@ static void test_flush(void)
>>      assert_bit_clear(data, DF | ERR | DRQ);
>>  
>>      /* Complete the command */
>> -    qmp_discard_response("{'execute':'human-monitor-command', 'arguments': 
>> {"
>> -                         " 'command-line':"
>> -                         " 'qemu-io ide0-hd0 \"resume A\"'} }");
>> +    g_free(hmp("qemu-io ide0-hd0 \"resume A\""));
>>  
>>      /* Check registers */
>>      data = inb(IDE_BASE + reg_device);
>> diff --git a/tests/libqtest.c b/tests/libqtest.c
>> index 8dede56..2a396ba 100644
>> --- a/tests/libqtest.c
>> +++ b/tests/libqtest.c
>> @@ -483,6 +483,33 @@ void qtest_qmp_eventwait(QTestState *s, const char 
>> *event)
>>      }
>>  }
>>  
>> +char *qtest_hmpv(QTestState *s, const char *fmt, va_list ap)
>> +{
>> +    char *cmd;
>> +    QDict *resp;
>> +    char *ret;
>> +
>> +    cmd = g_strdup_vprintf(fmt, ap);
>> +    resp = qtest_qmp(s, "{'execute': 'human-monitor-command',"
>> +                     " 'arguments': {'command-line': %s}}",
>> +                     cmd);
>> +    ret = g_strdup(qdict_get_try_str(resp, "return"));
>> +    g_assert(ret);
>> +    QDECREF(resp);
>> +    g_free(cmd);
>> +    return ret;
>> +}
>> +
>> +char *qtest_hmp(QTestState *s, const char *fmt, ...)
>> +{
>> +    va_list ap;
>> +    char *ret;
>> +
>> +    va_start(ap, fmt);
>> +    ret = qtest_hmpv(s, fmt, ap);
>> +    va_end(ap);
>> +    return ret;
>> +}
>>  
>>  const char *qtest_get_arch(void)
>>  {
>> @@ -774,6 +801,16 @@ void qmp_discard_response(const char *fmt, ...)
>>      qtest_qmpv_discard_response(global_qtest, fmt, ap);
>>      va_end(ap);
>>  }
>> +char *hmp(const char *fmt, ...)
>> +{
>> +    va_list ap;
>> +    char *ret;
>> +
>> +    va_start(ap, fmt);
>> +    ret = qtest_hmpv(global_qtest, fmt, ap);
>> +    va_end(ap);
>> +    return ret;
>> +}
>>  
>>  bool qtest_big_endian(void)
>>  {
>> diff --git a/tests/libqtest.h b/tests/libqtest.h
>> index ec42031..b270f7b 100644
>> --- a/tests/libqtest.h
>> +++ b/tests/libqtest.h
>> @@ -120,6 +120,29 @@ QDict *qtest_qmp_receive(QTestState *s);
>>  void qtest_qmp_eventwait(QTestState *s, const char *event);
>>  
>>  /**
>> + * qtest_hmpv:
>> + * @s: #QTestState instance to operate on.
>> + * @fmt...: HMP command to send to QEMU
>> + *
>> + * Send HMP command to QEMU via QMP's human-monitor-command.
>> + *
>> + * Returns: the command's output.
>> + */
>> +char *qtest_hmp(QTestState *s, const char *fmt, ...);
>> +
>> +/**
>> + * qtest_hmpv:
>> + * @s: #QTestState instance to operate on.
>> + * @fmt: HMP command to send to QEMU
>> + * @ap: HMP command arguments
>> + *
>> + * Send HMP command to QEMU via QMP's human-monitor-command.
>> + *
>> + * Returns: the command's output.
>
> Maybe mention that the output string should be free'd with g_free() ?

Can't hurt.

>> + */
>> +char *qtest_hmpv(QTestState *s, const char *fmt, va_list ap);
>> +
>> +/**
>>   * qtest_get_irq:
>>   * @s: #QTestState instance to operate on.
>>   * @num: Interrupt to observe.
>> @@ -499,6 +522,16 @@ static inline void qmp_eventwait(const char *event)
>>  }
>>  
>>  /**
>> + * hmp:
>> + * @fmt...: HMP command to send to QEMU
>> + *
>> + * Send HMP command to QEMU via QMP's human-monitor-command.
>> + *
>> + * Returns: the command's output.
>
> dito
>
> Anyway, patch looks fine to me, so:
>
> Reviewed-by: Thomas Huth <address@hidden>

Thanks!