Re: [PATCH 1/1] tests/qtest: add scenario for -readconfig handling

[Daniel P . Berrangé]

On Mon, Aug 08, 2022 at 07:55:50AM +0200, Thomas Huth wrote:
> On 05/08/2022 13.55, Daniel P. Berrangé wrote:
> > This test of -readconfig validates the last three regressions we
> > have fixed with -readconfig:
> > 
> >   * Interpretation of memory size units as MiB not bytes
> >   * Allow use of [spice]
> >   * Allow use of [object]
> 
> Good idea!
> 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   tests/qtest/meson.build       |   1 +
> >   tests/qtest/readconfig-test.c | 195 ++++++++++++++++++++++++++++++++++
> >   2 files changed, 196 insertions(+)
> >   create mode 100644 tests/qtest/readconfig-test.c
> > 
> > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> > index 3a474010e4..be4b30dea2 100644
> > --- a/tests/qtest/meson.build
> > +++ b/tests/qtest/meson.build
> > @@ -26,6 +26,7 @@ qtests_generic = [
> >     'qom-test',
> >     'test-hmp',
> >     'qos-test',
> > +  'readconfig-test',
> >   ]
> >   if config_host.has_key('CONFIG_MODULES')
> >     qtests_generic += [ 'modules-test' ]
> > diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c
> > new file mode 100644
> > index 0000000000..2e604d7c2d
> > --- /dev/null
> > +++ b/tests/qtest/readconfig-test.c
> > @@ -0,0 +1,195 @@
> > +/*
> > + * Validate -readconfig
> > + *
> > + * Copyright (c) 2022 Red Hat, Inc.
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or 
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "libqtest.h"
> > +#include "qapi/error.h"
> > +#include "qapi/qapi-visit-machine.h"
> > +#include "qapi/qapi-visit-qom.h"
> > +#include "qapi/qapi-visit-ui.h"
> > +#include "qapi/qmp/qdict.h"
> > +#include "qapi/qmp/qlist.h"
> > +#include "qapi/qobject-input-visitor.h"
> > +#include "qapi/qmp/qstring.h"
> > +#include "qemu/units.h"
> > +
> > +static QTestState *qtest_init_with_config(const char *cfgdata)
> > +{
> > +    GError *error = NULL;
> > +    g_autofree char *args = NULL;
> > +    int cfgfd = -1;
> > +    g_autofree char *cfgpath = NULL;
> > +    QTestState *qts;
> > +    ssize_t ret;
> > +
> > +    cfgfd = g_file_open_tmp("readconfig-test-XXXXXX", &cfgpath, &error);
> > +    g_assert_no_error(error);
> > +    g_assert_cmpint(cfgfd, >=, 0);
> > +
> > +    ret = qemu_write_full(cfgfd, cfgdata, strlen(cfgdata));
> > +    if (ret < 0) {
> > +        unlink(cfgpath);
> > +    }
> > +    g_assert_cmpint(ret, ==, strlen(cfgdata));
> > +
> > +    close(cfgfd);
> 
> Maybe move the close() before the "if (ret < 0)"
> 
> > +    args = g_strdup_printf("-nodefaults -machine none -readconfig %s", 
> > cfgpath);
> > +
> > +    qts = qtest_init(args);
> > +
> > +    unlink(cfgpath);
> > +
> > +    return qts;
> > +}
> > +
> > +static void test_x86_memdev_resp(QObject *res)
> > +{
> > +    Visitor *v;
> > +    g_autoptr(MemdevList) memdevs = NULL;
> > +    Memdev *memdev;
> > +
> > +    g_assert(res);
> > +    v = qobject_input_visitor_new(res);
> > +    visit_type_MemdevList(v, NULL, &memdevs, &error_abort);
> > +
> > +    g_assert(memdevs);
> > +    g_assert(memdevs->value);
> > +    g_assert(!memdevs->next);
> > +
> > +    memdev = memdevs->value;
> > +    g_assert_cmpstr(memdev->id, ==, "ram");
> > +    g_assert_cmpint(memdev->size, ==, 200 * MiB);
> > +
> > +    visit_free(v);
> > +}
> > +
> > +static void test_x86_memdev(void)
> > +{
> > +    QDict *resp;
> > +    QTestState *qts;
> > +    const char *cfgdata =
> > +        "[memory]\n"
> > +        "size = \"200\"";
> > +
> > +    qts = qtest_init_with_config(cfgdata);
> > +   /* Test valid command */
> 
> Comment has bad indentation.
> 
> > +    resp = qtest_qmp(qts, "{ 'execute': 'query-memdev' }");
> > +    test_x86_memdev_resp(qdict_get(resp, "return"));
> > +    qobject_unref(resp);
> > +
> > +    qtest_quit(qts);
> > +}
> > +
> > +
> > +#ifdef CONFIG_SPICE
> > +static void test_spice_resp(QObject *res)
> > +{
> > +    Visitor *v;
> > +    g_autoptr(SpiceInfo) spice = NULL;
> > +
> > +    g_assert(res);
> > +    v = qobject_input_visitor_new(res);
> > +    visit_type_SpiceInfo(v, "spcie", &spice, &error_abort);
> 
> That "spcie" looks like a typo?

Yes, but I believe this argument is ignored by the visitor in this
scenario - it didn't fail for me at least.

Regardless, sHould be fixed of course


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 :|