Re: [PATCH v2 10/18] tests/qtest/migration: Add a test for HMP completion

["Dr. David Alan Gilbert" <dave@treblig.org>]

* Fabiano Rosas (farosas@suse.de) wrote:
> The following patches will touch HMP readline completion for migration
> parameters, add a test case.
> 
> The test puts the monitor on a chardev via socket and bypasses qtest
> facilities because it needs to emit raw codes to readline. It
> therefore requires a couple of new helpers to read/write to the
> monitor socket.
> 
> Usage:
> QTEST_QEMU_BINARY=./qemu-system-x86_64 \
> ./tests/qtest/migration-test --full -p /x86_64/migration/hmp/completion
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---

> +static void hmp_sock_read(int fd, char *buf, size_t buf_sz)
> +{
> +    char *p = buf;
> +    size_t sz = buf_sz - 1;
> +
> +    assert(fd >= 0);
> +    memset(buf, 0, buf_sz);
> +
> +    while (sz > 0) {
> +        ssize_t r = read(fd, p, sz);
> +        char *prompt;
> +
> +        if (!r) {
> +            break;
> +        } else if (r < 0) {
> +            if (errno == EINTR) {
> +                continue;
> +            }
> +            g_assert_not_reached();
> +        }
> +
> +        p += r;
> +        sz -= r;
> +
> +        prompt = strstr(buf, "(qemu) ");
> +        if (prompt) {
> +            *prompt = '\0';

I'm a bit confused by what you want the output to look like after this,
are you assuming this is the end of the string you're reading so truncation
is OK?

> +            break;
> +        }
> +    }
> +}
> +
> +static int comp(const void *a, const void *b)
> +{
> +    return strcmp(*(const char **) a, *(const char **) b);
> +}

Can you use qemu_pstrcmp0 there? (Maybe with g_ptr_array and g_ptr_array_sort is
what adds to some of the other questions below?)

> +static void get_migration_opts_sorted(GString *exp, const char * const *lookup, int n)
> +{
> +    g_autofree char **opts_array = g_new0(char *, n);
> +    uint8_t i;
> +
> +    for (i = 0; i < n; i++) {
> +        opts_array[i] = g_strdup(lookup[i]);
> +    }

If you made opts_array one element larger, and added a NULL at the end could you...

> +    qsort(opts_array, n, sizeof(char *), comp);
> +
> +    for (i = 0; i < n; i++) {
> +        g_string_append(exp, opts_array[i]);
> +        if (i + 1 != n) {
> +            g_string_append(exp, " ");
> +        }
> +    }

Use g_strjoinv here?

> +    for (i = 0; i < n; i++) {
> +        g_free(opts_array[i]);
> +    }
> +}

Dave

> +static void hmp_completion_single(int fd, const struct HMPTestData *t)
> +{
> +    g_autoptr(GString) exp = g_string_new("");
> +    char buf[8192];
> +    char *output;
> +
> +    test_case_line = t->line;
> +
> +    if (g_str_equal(t->output1, "@caps@")) {
> +        g_string_append(exp, "migrate_set_capability ");
> +        get_migration_opts_sorted(exp, MigrationCapability_lookup.array,
> +                                  MIGRATION_CAPABILITY__MAX);
> +    } else if (g_str_equal(t->output1, "@params@")) {
> +        g_string_append(exp, "migrate_set_parameter ");
> +        get_migration_opts_sorted(exp, MigrationParameter_lookup.array,
> +                                  MIGRATION_PARAMETER__MAX);
> +    } else {
> +        g_string_append(exp, t->output1);
> +    }
> +
> +    hmp_sock_write(fd, t->input1);
> +    hmp_sock_read(fd, buf, sizeof(buf));
> +
> +    /*
> +     * readline first rewrites the input to the common root of the
> +     * completions, then outputs the completion suggestions:
> +     *
> +     * (qemu) info migr<TAB>
> +     * (qemu) migrate migrate_parameters
> +     * migrate_capabilities ...
> +     */
> +    output = strstr(buf, t->input2);
> +    assert_hmp_match_text(exp->str, output);
> +
> +    /* ^U backward kill line */
> +    hmp_sock_write(fd, "\x15");
> +}
> +
> +static void test_hmp_completion(char *name, MigrateCommon *args)
> +{
> +    g_autofree char *cmdline;
> +    char buf[1024];
> +    QTestState *qts;
> +    int sockfds[2];
> +
> +    assert(!qemu_socketpair(AF_UNIX, SOCK_STREAM, 0, sockfds));
> +    qemu_clear_cloexec(sockfds[1]);
> +
> +    cmdline = g_strdup_printf("-chardev socket,id=mon0,fd=%d "
> +                              "-mon chardev=mon0,mode=readline -S",
> +                              sockfds[1]);
> +    qts = qtest_init(cmdline);
> +    close(sockfds[1]);
> +
> +    /* read HMP banner */
> +    hmp_sock_read(sockfds[0], buf, sizeof(buf));
> +
> +    for (int i = 0; i < G_N_ELEMENTS(completion_cases); i++) {
> +        hmp_completion_single(sockfds[0], &completion_cases[i]);
> +    }
> +
> +    close(sockfds[0]);
> +    qtest_quit(qts);
> +}
>  #endif /* CONFIG_HMP */
>  
>  static void test_baddest(char *name, MigrateCommon *args)
> @@ -460,5 +650,7 @@ void migration_test_add_misc(MigrationTestEnv *env)
>  #ifdef CONFIG_HMP
>      migration_test_add("/migration/hmp/parameters",
>                         test_hmp_migration_parameters);
> +    migration_test_add("/migration/hmp/completion",
> +                       test_hmp_completion);
>  #endif
>  }
> -- 
> 2.53.0
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/