Add several counterparts of qtest_qmp_assert_success() that can

 * Use va_list instead of ...
 * Accept a list of FDs to send
 * Return the response data

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qtest/libqtest.c |  99 +++++++++++++++++++++++++++++++++--
 tests/qtest/libqtest.h | 115 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 209 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index c3a0ef5bb4..603c26d955 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1229,14 +1229,23 @@ void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size)
     qtest_rsp(s);
 }
 
-void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
+void qtest_vqmp_assert_success(QTestState *qts,
+                               const char *fmt, va_list args)
 {
-    va_list ap;
     QDict *response;
 
-    va_start(ap, fmt);
-    response = qtest_vqmp(qts, fmt, ap);
-    va_end(ap);
+    response = qtest_vqmp_assert_success_ref(qts, fmt, args);
+
+    qobject_unref(response);
+}
+
+QDict *qtest_vqmp_assert_success_ref(QTestState *qts,
+                                     const char *fmt, va_list args)
+{
+    QDict *response;
+    QDict *ret;
+
+    response = qtest_vqmp(qts, fmt, args);
 
     g_assert(response);
     if (!qdict_haskey(response, "return")) {
@@ -1245,8 +1254,88 @@ void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
         g_string_free(s, true);
     }
     g_assert(qdict_haskey(response, "return"));
+    ret = qdict_get_qdict(response, "return");
+    qobject_ref(ret);
+    qobject_unref(response);
+
+    return ret;
+}
+
+#ifndef _WIN32
+QDict *qtest_vqmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
+                                         const char *fmt, va_list args)
+{
+    QDict *response;
+    QDict *ret;
+
+    response = qtest_vqmp_fds(qts, fds, nfds, fmt, args);
+
+    g_assert(response);
+    if (!qdict_haskey(response, "return")) {
+        GString *s = qobject_to_json_pretty(QOBJECT(response), true);
+        g_test_message("%s", s->str);
+        g_string_free(s, true);
+    }
+    g_assert(qdict_haskey(response, "return"));
+    ret = qdict_get_qdict(response, "return");
+    qobject_ref(ret);
+    qobject_unref(response);
+
+    return ret;
+}
+
+void qtest_vqmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
+                                   const char *fmt, va_list args)
+{
+    QDict *response;
+    response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt, args);
     qobject_unref(response);
 }
+#endif /* !_WIN32 */
+
+void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
+{
+    QDict *response;
+    va_list ap;
+    va_start(ap, fmt);
+    response = qtest_vqmp_assert_success_ref(qts, fmt, ap);
+    va_end(ap);
+    qobject_unref(response);
+}
+
+QDict *qtest_qmp_assert_success_ref(QTestState *qts, const char *fmt, ...)
+{
+    QDict *response;
+    va_list ap;
+    va_start(ap, fmt);
+    response = qtest_vqmp_assert_success_ref(qts, fmt, ap);
+    va_end(ap);
+    return response;
+}
+
+#ifndef _WIN32
+void qtest_qmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
+                                  const char *fmt, ...)
+{
+    QDict *response;
+    va_list ap;
+    va_start(ap, fmt);
+    response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt, ap);
+    va_end(ap);
+    qobject_unref(response);
+}
+
+QDict *qtest_qmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
+                                        const char *fmt, ...)
+{
+    QDict *response;
+    va_list ap;
+    va_start(ap, fmt);
+    response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt, ap);
+    va_end(ap);
+    return response;
+}
+#endif /* !_WIN32 */
 
 bool qtest_big_endian(QTestState *s)
 {
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 8d7d450963..41bc6633bd 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -693,6 +693,73 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data);
  */
 void qtest_remove_abrt_handler(void *data);
 
+/**
+ * qtest_vqmp_assert_success:
+ * @qts: QTestState instance to operate on
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
+ * supported after '%'.
+ * @args: variable arguments for @fmt
+ *
+ * Sends a QMP message to QEMU and asserts that a 'return' key is present in
+ * the response.
+ */
+void qtest_vqmp_assert_success(QTestState *qts,
+                               const char *fmt, va_list args)
+    G_GNUC_PRINTF(2, 0);
+
+/**
+ * qtest_vqmp_assert_success_ref:
+ * @qts: QTestState instance to operate on
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
+ * supported after '%'.
+ * @args: variable arguments for @fmt
+ *
+ * Sends a QMP message to QEMU, asserts that a 'return' key is present in
+ * the response, and returns the response.
+ */
+QDict *qtest_vqmp_assert_success_ref(QTestState *qts,
+                                     const char *fmt, va_list args)
+    G_GNUC_PRINTF(2, 0);
+
+#ifndef _WIN32
+/**
+ * qtest_vqmp_fds_assert_success:
+ * @qts: QTestState instance to operate on
+ * @fds: the file descriptors to send
+ * @nfds: number of @fds to send
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
+ * supported after '%'.
+ * @args: variable arguments for @fmt
+ *
+ * Sends a QMP message with file descriptors to QEMU and
+ * asserts that a 'return' key is present in the response.
+ */
+void qtest_vqmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
+                                   const char *fmt, va_list args)
+    G_GNUC_PRINTF(4, 0);
+
+/**
+ * qtest_vqmp_fds_assert_success_ref:
+ * @qts: QTestState instance to operate on
+ * @fds: the file descriptors to send
+ * @nfds: number of @fds to send
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
+ * supported after '%'.
+ * @args: variable arguments for @fmt
+ *
+ * Sends a QMP message with file descriptors to QEMU,
+ * asserts that a 'return' key is present in the response,
+ * and returns the response.
+ */
+QDict *qtest_vqmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
+                                         const char *fmt, va_list args)
+    G_GNUC_PRINTF(4, 0);
+#endif /* !_WIN32 */
+
 /**
  * qtest_qmp_assert_success:
  * @qts: QTestState instance to operate on
@@ -706,6 +773,54 @@ void qtest_remove_abrt_handler(void *data);
 void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
     G_GNUC_PRINTF(2, 3);
 
+/**
+ * qtest_qmp_assert_success_ref:
+ * @qts: QTestState instance to operate on
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
+ * supported after '%'.
+ *
+ * Sends a QMP message to QEMU, asserts that a 'return' key is present in
+ * the response, and returns the response.
+ */
+QDict *qtest_qmp_assert_success_ref(QTestState *qts, const char *fmt, ...)
+    G_GNUC_PRINTF(2, 3);
+
+#ifndef _WIN32
+/**
+ * qtest_qmp_fd_assert_success:
+ * @qts: QTestState instance to operate on
+ * @fds: the file descriptors to send
+ * @nfds: number of @fds to send
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
+ * supported after '%'.
+ *
+ * Sends a QMP message with file descriptors to QEMU and
+ * asserts that a 'return' key is present in the response.
+ */
+void qtest_qmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
+                                  const char *fmt, ...)
+    G_GNUC_PRINTF(4, 5);
+
+/**
+ * qtest_qmp_fd_assert_success_ref:
+ * @qts: QTestState instance to operate on
+ * @fds: the file descriptors to send
+ * @nfds: number of @fds to send
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
+ * supported after '%'.
+ *
+ * Sends a QMP message with file descriptors to QEMU,
+ * asserts that a 'return' key is present in the response,
+ * and returns the response.
+ */
+QDict *qtest_qmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
+                                        const char *fmt, ...)
+    G_GNUC_PRINTF(4, 5);
+#endif /* !_WIN32 */
+
 /**
  * qtest_cb_for_every_machine:
  * @cb: Pointer to the callback function
-- 
2.40.1

Daniel P. Berrangé <berrange@redhat.com> wrote:
> Add several counterparts of qtest_qmp_assert_success() that can
>
>  * Use va_list instead of ...
>  * Accept a list of FDs to send
>  * Return the response data
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qtest/libqtest.c |  99 +++++++++++++++++++++++++++++++++--
>  tests/qtest/libqtest.h | 115 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 209 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index c3a0ef5bb4..603c26d955 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1229,14 +1229,23 @@ void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size)
>      qtest_rsp(s);
>  }
>  
> -void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
> +void qtest_vqmp_assert_success(QTestState *qts,
> +                               const char *fmt, va_list args)
>  {
> -    va_list ap;
>      QDict *response;
>  
> -    va_start(ap, fmt);
> -    response = qtest_vqmp(qts, fmt, ap);
> -    va_end(ap);
> +    response = qtest_vqmp_assert_success_ref(qts, fmt, args);
> +
> +    qobject_unref(response);
> +}
> +
> +QDict *qtest_vqmp_assert_success_ref(QTestState *qts,
> +                                     const char *fmt, va_list args)
> +{
> +    QDict *response;
> +    QDict *ret;
> +
> +    response = qtest_vqmp(qts, fmt, args);
>  
>      g_assert(response);
>      if (!qdict_haskey(response, "return")) {
> @@ -1245,8 +1254,88 @@ void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
>          g_string_free(s, true);
>      }
>      g_assert(qdict_haskey(response, "return"));
> +    ret = qdict_get_qdict(response, "return");
> +    qobject_ref(ret);
> +    qobject_unref(response);
> +
> +    return ret;
> +}
> +
> +#ifndef _WIN32
> +QDict *qtest_vqmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
> +                                         const char *fmt, va_list args)
> +{
> +    QDict *response;
> +    QDict *ret;
> +
> +    response = qtest_vqmp_fds(qts, fds, nfds, fmt, args);
> +
> +    g_assert(response);
> +    if (!qdict_haskey(response, "return")) {
> +        GString *s = qobject_to_json_pretty(QOBJECT(response), true);
> +        g_test_message("%s", s->str);
> +        g_string_free(s, true);

I know we are not consistent ot this file, but what about using autoptr here?

        g_autoptr(GString) *s = qobject_to_json_pretty(QOBJECT(response), true);
        g_test_message("%s", s->str);

??

> +    }
> +    g_assert(qdict_haskey(response, "return"));
> +    ret = qdict_get_qdict(response, "return");
> +    qobject_ref(ret);
> +    qobject_unref(response);
> +
> +    return ret;
> +}
> +
> +void qtest_vqmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
> +                                   const char *fmt, va_list args)
> +{
> +    QDict *response;
> +    response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt, args);
>      qobject_unref(response);
>  }
> +#endif /* !_WIN32 */
> +
> +void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
> +{
> +    QDict *response;
> +    va_list ap;
> +    va_start(ap, fmt);
> +    response = qtest_vqmp_assert_success_ref(qts, fmt, ap);
> +    va_end(ap);
> +    qobject_unref(response);
> +}
> +
> +QDict *qtest_qmp_assert_success_ref(QTestState *qts, const char *fmt, ...)
> +{
> +    QDict *response;
> +    va_list ap;
> +    va_start(ap, fmt);
> +    response = qtest_vqmp_assert_success_ref(qts, fmt, ap);
> +    va_end(ap);
> +    return response;
> +}
> +
> +#ifndef _WIN32
> +void qtest_qmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
> +                                  const char *fmt, ...)
> +{
> +    QDict *response;
> +    va_list ap;
> +    va_start(ap, fmt);
> +    response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt, ap);
> +    va_end(ap);
> +    qobject_unref(response);
> +}
> +
> +QDict *qtest_qmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
> +                                        const char *fmt, ...)
> +{
> +    QDict *response;
> +    va_list ap;
> +    va_start(ap, fmt);
> +    response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt, ap);
> +    va_end(ap);
> +    return response;
> +}
> +#endif /* !_WIN32 */
>  
>  bool qtest_big_endian(QTestState *s)
>  {
> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
> index 8d7d450963..41bc6633bd 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -693,6 +693,73 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data);
>   */
>  void qtest_remove_abrt_handler(void *data);
>  
> +/**
> + * qtest_vqmp_assert_success:
> + * @qts: QTestState instance to operate on
> + * @fmt: QMP message to send to qemu, formatted like
> + * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
> + * supported after '%'.
> + * @args: variable arguments for @fmt
> + *
> + * Sends a QMP message to QEMU and asserts that a 'return' key is present in
> + * the response.
> + */
> +void qtest_vqmp_assert_success(QTestState *qts,
> +                               const char *fmt, va_list args)
> +    G_GNUC_PRINTF(2, 0);
> +
> +/**
> + * qtest_vqmp_assert_success_ref:
> + * @qts: QTestState instance to operate on
> + * @fmt: QMP message to send to qemu, formatted like
> + * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
> + * supported after '%'.
> + * @args: variable arguments for @fmt
> + *
> + * Sends a QMP message to QEMU, asserts that a 'return' key is present in
> + * the response, and returns the response.
> + */
> +QDict *qtest_vqmp_assert_success_ref(QTestState *qts,
> +                                     const char *fmt, va_list args)
> +    G_GNUC_PRINTF(2, 0);
> +
> +#ifndef _WIN32
> +/**
> + * qtest_vqmp_fds_assert_success:
> + * @qts: QTestState instance to operate on
> + * @fds: the file descriptors to send
> + * @nfds: number of @fds to send
> + * @fmt: QMP message to send to qemu, formatted like
> + * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
> + * supported after '%'.
> + * @args: variable arguments for @fmt
> + *
> + * Sends a QMP message with file descriptors to QEMU and
> + * asserts that a 'return' key is present in the response.
> + */
> +void qtest_vqmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
> +                                   const char *fmt, va_list args)
> +    G_GNUC_PRINTF(4, 0);
> +
> +/**
> + * qtest_vqmp_fds_assert_success_ref:
> + * @qts: QTestState instance to operate on
> + * @fds: the file descriptors to send
> + * @nfds: number of @fds to send
> + * @fmt: QMP message to send to qemu, formatted like
> + * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
> + * supported after '%'.
> + * @args: variable arguments for @fmt
> + *
> + * Sends a QMP message with file descriptors to QEMU,
> + * asserts that a 'return' key is present in the response,
> + * and returns the response.
> + */
> +QDict *qtest_vqmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
> +                                         const char *fmt, va_list args)
> +    G_GNUC_PRINTF(4, 0);
> +#endif /* !_WIN32 */
> +
>  /**
>   * qtest_qmp_assert_success:
>   * @qts: QTestState instance to operate on
> @@ -706,6 +773,54 @@ void qtest_remove_abrt_handler(void *data);
>  void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
>      G_GNUC_PRINTF(2, 3);
>  
> +/**
> + * qtest_qmp_assert_success_ref:
> + * @qts: QTestState instance to operate on
> + * @fmt: QMP message to send to qemu, formatted like
> + * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
> + * supported after '%'.
> + *
> + * Sends a QMP message to QEMU, asserts that a 'return' key is present in
> + * the response, and returns the response.
> + */
> +QDict *qtest_qmp_assert_success_ref(QTestState *qts, const char *fmt, ...)
> +    G_GNUC_PRINTF(2, 3);
> +
> +#ifndef _WIN32
> +/**
> + * qtest_qmp_fd_assert_success:
> + * @qts: QTestState instance to operate on
> + * @fds: the file descriptors to send
> + * @nfds: number of @fds to send
> + * @fmt: QMP message to send to qemu, formatted like
> + * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
> + * supported after '%'.
> + *
> + * Sends a QMP message with file descriptors to QEMU and
> + * asserts that a 'return' key is present in the response.
> + */
> +void qtest_qmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
> +                                  const char *fmt, ...)
> +    G_GNUC_PRINTF(4, 5);
> +
> +/**
> + * qtest_qmp_fd_assert_success_ref:
> + * @qts: QTestState instance to operate on
> + * @fds: the file descriptors to send
> + * @nfds: number of @fds to send
> + * @fmt: QMP message to send to qemu, formatted like
> + * qobject_from_jsonf_nofail().  See parse_interpolation() for what's
> + * supported after '%'.
> + *
> + * Sends a QMP message with file descriptors to QEMU,
> + * asserts that a 'return' key is present in the response,
> + * and returns the response.
> + */
> +QDict *qtest_qmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
> +                                        const char *fmt, ...)
> +    G_GNUC_PRINTF(4, 5);
> +#endif /* !_WIN32 */
> +
>  /**
>   * qtest_cb_for_every_machine:
>   * @cb: Pointer to the callback function

On Thu, Jun 01, 2023 at 02:20:34PM +0200, Juan Quintela wrote:
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> > Add several counterparts of qtest_qmp_assert_success() that can
> >
> >  * Use va_list instead of ...
> >  * Accept a list of FDs to send
> >  * Return the response data
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  tests/qtest/libqtest.c |  99 +++++++++++++++++++++++++++++++++--
> >  tests/qtest/libqtest.h | 115 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 209 insertions(+), 5 deletions(-)
> >
> > diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> > index c3a0ef5bb4..603c26d955 100644
> > --- a/tests/qtest/libqtest.c
> > +++ b/tests/qtest/libqtest.c


> > +#ifndef _WIN32
> > +QDict *qtest_vqmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
> > +                                         const char *fmt, va_list args)
> > +{
> > +    QDict *response;
> > +    QDict *ret;
> > +
> > +    response = qtest_vqmp_fds(qts, fds, nfds, fmt, args);
> > +
> > +    g_assert(response);
> > +    if (!qdict_haskey(response, "return")) {
> > +        GString *s = qobject_to_json_pretty(QOBJECT(response), true);
> > +        g_test_message("%s", s->str);
> > +        g_string_free(s, true);
> 
> I know we are not consistent ot this file, but what about using autoptr here?
> 
>         g_autoptr(GString) *s = qobject_to_json_pretty(QOBJECT(response), true);
>         g_test_message("%s", s->str);
> 
> ??

Sure.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Daniel P. Berrangé <berrange@redhat.com> wrote:
> Add several counterparts of qtest_qmp_assert_success() that can
>
>  * Use va_list instead of ...
>  * Accept a list of FDs to send
>  * Return the response data
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qtest/libqtest.c |  99 +++++++++++++++++++++++++++++++++--
>  tests/qtest/libqtest.h | 115 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 209 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index c3a0ef5bb4..603c26d955 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1229,14 +1229,23 @@ void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size)
>      qtest_rsp(s);
>  }
>  
> -void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
> +void qtest_vqmp_assert_success(QTestState *qts,
> +                               const char *fmt, va_list args)
>  {
> -    va_list ap;
>      QDict *response;
>  
> -    va_start(ap, fmt);
> -    response = qtest_vqmp(qts, fmt, ap);
> -    va_end(ap);
> +    response = qtest_vqmp_assert_success_ref(qts, fmt, args);
> +
> +    qobject_unref(response);
> +}
> +
> +QDict *qtest_vqmp_assert_success_ref(QTestState *qts,
> +                                     const char *fmt, va_list args)
> +{

I was about to chime in that this two functions should be declared in
reverse and see that Thomas had the same ide.

Later, Juan

On 31/05/2023 15.23, Daniel P. Berrangé wrote:
> Add several counterparts of qtest_qmp_assert_success() that can
> 
>   * Use va_list instead of ...
>   * Accept a list of FDs to send
>   * Return the response data
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/qtest/libqtest.c |  99 +++++++++++++++++++++++++++++++++--
>   tests/qtest/libqtest.h | 115 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 209 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index c3a0ef5bb4..603c26d955 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1229,14 +1229,23 @@ void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size)
>       qtest_rsp(s);
>   }
>   
> -void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
> +void qtest_vqmp_assert_success(QTestState *qts,
> +                               const char *fmt, va_list args)
>   {
> -    va_list ap;
>       QDict *response;
>   
> -    va_start(ap, fmt);
> -    response = qtest_vqmp(qts, fmt, ap);
> -    va_end(ap);
> +    response = qtest_vqmp_assert_success_ref(qts, fmt, args);
> +
> +    qobject_unref(response);
> +}
> +
> +QDict *qtest_vqmp_assert_success_ref(QTestState *qts,
> +                                     const char *fmt, va_list args)
> +{
> +    QDict *response;
> +    QDict *ret;
> +
> +    response = qtest_vqmp(qts, fmt, args);
>   
>       g_assert(response);
>       if (!qdict_haskey(response, "return")) {
> @@ -1245,8 +1254,88 @@ void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
>           g_string_free(s, true);
>       }
>       g_assert(qdict_haskey(response, "return"));
> +    ret = qdict_get_qdict(response, "return");
> +    qobject_ref(ret);
> +    qobject_unref(response);
> +
> +    return ret;
> +}
> +
> +#ifndef _WIN32
> +QDict *qtest_vqmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
> +                                         const char *fmt, va_list args)
> +{
> +    QDict *response;
> +    QDict *ret;
> +
> +    response = qtest_vqmp_fds(qts, fds, nfds, fmt, args);
> +
> +    g_assert(response);
> +    if (!qdict_haskey(response, "return")) {
> +        GString *s = qobject_to_json_pretty(QOBJECT(response), true);
> +        g_test_message("%s", s->str);
> +        g_string_free(s, true);
> +    }
> +    g_assert(qdict_haskey(response, "return"));
> +    ret = qdict_get_qdict(response, "return");
> +    qobject_ref(ret);
> +    qobject_unref(response);
> +
> +    return ret;
> +}
> +
> +void qtest_vqmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
> +                                   const char *fmt, va_list args)
> +{
> +    QDict *response;
> +    response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt, args);
>       qobject_unref(response);
>   }

<bikeshedding>The ordering is a little bit inconsistent ... for some pairs, 
you do the _success() function first, and for some others you do the 
_success_ref() function first. IMHO it would be nicer to have the same 
ordering everywhere, preferably with the _success_ref() function first 
(since that's the one that is called from the other).</bikeshedding>

> +#endif /* !_WIN32 */
> +
> +void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
> +{
> +    QDict *response;
> +    va_list ap;
> +    va_start(ap, fmt);
> +    response = qtest_vqmp_assert_success_ref(qts, fmt, ap);

You could use qtest_vqmp_assert_success() instead, I think, and dro the 
qobject_unref() below.

> +    va_end(ap);
> +    qobject_unref(response);
> +}
> +
> +QDict *qtest_qmp_assert_success_ref(QTestState *qts, const char *fmt, ...)
> +{
> +    QDict *response;
> +    va_list ap;
> +    va_start(ap, fmt);
> +    response = qtest_vqmp_assert_success_ref(qts, fmt, ap);
> +    va_end(ap);
> +    return response;
> +}
> +
> +#ifndef _WIN32
> +void qtest_qmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
> +                                  const char *fmt, ...)
> +{
> +    QDict *response;
> +    va_list ap;
> +    va_start(ap, fmt);
> +    response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt, ap);

dito, could use qtest_vqmp_fds_assert_success() instead.

> +    va_end(ap);
> +    qobject_unref(response);
> +}
> +
> +QDict *qtest_qmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
> +                                        const char *fmt, ...)
> +{
> +    QDict *response;
> +    va_list ap;
> +    va_start(ap, fmt);
> +    response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt, ap);
> +    va_end(ap);
> +    return response;
> +}
> +#endif /* !_WIN32 */

Just nits, so anyway:
Reviewed-by: Thomas Huth <thuth@redhat.com>

On Thu, Jun 01, 2023 at 11:23:01AM +0200, Thomas Huth wrote:
> On 31/05/2023 15.23, Daniel P. Berrangé wrote:
> > Add several counterparts of qtest_qmp_assert_success() that can
> > 
> >   * Use va_list instead of ...
> >   * Accept a list of FDs to send
> >   * Return the response data
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   tests/qtest/libqtest.c |  99 +++++++++++++++++++++++++++++++++--
> >   tests/qtest/libqtest.h | 115 +++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 209 insertions(+), 5 deletions(-)


> > +#ifndef _WIN32
> > +QDict *qtest_vqmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
> > +                                         const char *fmt, va_list args)
> > +{
> > +    QDict *response;
> > +    QDict *ret;
> > +
> > +    response = qtest_vqmp_fds(qts, fds, nfds, fmt, args);
> > +
> > +    g_assert(response);
> > +    if (!qdict_haskey(response, "return")) {
> > +        GString *s = qobject_to_json_pretty(QOBJECT(response), true);
> > +        g_test_message("%s", s->str);
> > +        g_string_free(s, true);
> > +    }
> > +    g_assert(qdict_haskey(response, "return"));
> > +    ret = qdict_get_qdict(response, "return");
> > +    qobject_ref(ret);
> > +    qobject_unref(response);
> > +
> > +    return ret;
> > +}
> > +
> > +void qtest_vqmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
> > +                                   const char *fmt, va_list args)
> > +{
> > +    QDict *response;
> > +    response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt, args);
> >       qobject_unref(response);
> >   }
> 
> <bikeshedding>The ordering is a little bit inconsistent ... for some pairs,
> you do the _success() function first, and for some others you do the
> _success_ref() function first. IMHO it would be nicer to have the same
> ordering everywhere, preferably with the _success_ref() function first
> (since that's the one that is called from the other).</bikeshedding>

Sure, will do.

> 
> > +#endif /* !_WIN32 */
> > +
> > +void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
> > +{
> > +    QDict *response;
> > +    va_list ap;
> > +    va_start(ap, fmt);
> > +    response = qtest_vqmp_assert_success_ref(qts, fmt, ap);
> 
> You could use qtest_vqmp_assert_success() instead, I think, and dro the
> qobject_unref() below.

Agreed


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|