Philippe Mathieu-Daudé <phi...@redhat.com> writes:
> Now than we can probe if the TCG accelerator is available
> at runtime with a QMP command, do it once at the beginning
> and only register the tests we can run.
> We can then replace the #ifdef'ry by a runtime check.
>
> Suggested-by: Paolo Bonzini <pbonz...@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
Please read the last remark first. The other ones are detail; feel free
to skip them until we're done with the last one.
> ---
> tests/qtest/qmp-cmd-test.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
> index c98b78d0339..8902d2f169f 100644
> --- a/tests/qtest/qmp-cmd-test.c
> +++ b/tests/qtest/qmp-cmd-test.c
> @@ -21,19 +21,24 @@ const char common_args[] = "-nodefaults -machine none";
>
> /* Query smoke tests */
>
> +static bool tcg_accel_available;
> +
> static int query_error_class(const char *cmd)
> {
> - static struct {
> + static const struct {
> const char *cmd;
> int err_class;
> + /*
> + * Pointer to boolean.
Let's not paraphrase code in comments.
> + * If non-NULL and value is %true, the error class is skipped.
Suggest "When it points to true, the test case is skipped", and ...
> + */
> + bool *skip_err_class;
... name this just @skip. Or maybe @skip_ptr, because fails[i].skip
reads as if the test case was to be skipped.
> } fails[] = {
> /* Success depends on build configuration: */
Note the headline ^^^
> #ifndef CONFIG_SPICE
> { "query-spice", ERROR_CLASS_COMMAND_NOT_FOUND },
> #endif
> -#ifndef CONFIG_TCG
> - { "query-replay", ERROR_CLASS_COMMAND_NOT_FOUND },
> -#endif
> + { "query-replay", ERROR_CLASS_COMMAND_NOT_FOUND,
> &tcg_accel_available },
No longer fits under the headline. Move it under its own headline,
perhaps
/* Success depends on dynamic state as reported by other queries */
> #ifndef CONFIG_VNC
> { "query-vnc", ERROR_CLASS_GENERIC_ERROR },
> { "query-vnc-servers", ERROR_CLASS_GENERIC_ERROR },
> @@ -51,6 +56,9 @@ static int query_error_class(const char *cmd)
> int i;
>
> for (i = 0; fails[i].cmd; i++) {
> + if (fails[i].skip_err_class && *fails[i].skip_err_class) {
> + continue;
> + }
> if (!strcmp(cmd, fails[i].cmd)) {
> return fails[i].err_class;
> }
> @@ -334,6 +342,8 @@ int main(int argc, char *argv[])
> QmpSchema schema;
> int ret;
>
> + tcg_accel_available = qtest_has_accel("tcg");
> +
When does tcg_accel_available differ from defined(CONFIG_TCG)?
> g_test_init(&argc, &argv, NULL);
>
> qmp_schema_init(&schema);
