On 18/12/2019 02.55, Juan Quintela wrote:
> We are repeating almost everything for each machine while creating the
> command line for migration. And once for source and another for
> destination. We start putting there opts_src and opts_dst.
>
> Signed-off-by: Juan Quintela <quint...@redhat.com>
> ---
> tests/migration-test.c | 44 ++++++++++++++++++++++++------------------
> 1 file changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index a5343fdc66..fbddcf2317 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -557,6 +557,7 @@ static int test_migrate_start(QTestState **from,
> QTestState **to,
> const char *opts_dst)
> {
> gchar *cmd_src, *cmd_dst;
> + gchar *cmd_source, *cmd_target;
The naming looks quite unfortunate to me ... "cmd_src" can easily be
mixed up with "cmd_source" ... but maybe you could do it without these
additional variables (see below) ...
[...]
> @@ -671,11 +671,17 @@ static int test_migrate_start(QTestState **from,
> QTestState **to,
> cmd_dst = tmp;
> }
>
> - *from = qtest_init(cmd_src);
> + cmd_source = g_strdup_printf("%s %s",
> + cmd_src, opts_src);
> g_free(cmd_src);
> + *from = qtest_init(cmd_source);
> + g_free(cmd_source);
>
> - *to = qtest_init(cmd_dst);
> + cmd_target = g_strdup_printf("%s %s",
> + cmd_dst, opts_dst);
> g_free(cmd_dst);
> + *to = qtest_init(cmd_target);
> + g_free(cmd_target);
May I suggest to qtest_initf() here instead:
*from = qtest_initf("%s %s", cmd_src, opts_src);
*to = qtest_initf("%s %s", cmd_dst, opts_dst);
And maybe you could even move the extra_opts here, too? e.g.:
*from = qtest_initf("%s %s %s", cmd_src, extra_opts ?: "", opts_src);
*to = qtest_initf("%s %s %s", cmd_dst, extra_opts ?: "", opts_dst);
Thomas
