Re: [PATCH 4/8] tests/qtest/migration-test: Build command line using GString API

[Philippe Mathieu-Daudé]

On 19/1/23 11:59, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (philmd@linaro.org) wrote:
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   tests/qtest/migration-test.c | 85 ++++++++++++++++++------------------
> >   1 file changed, 42 insertions(+), 43 deletions(-)
> >
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index dbde726adf..36e6074653 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -582,13 +582,13 @@ typedef struct {
> >   static int test_migrate_start(QTestState **from, QTestState **to,
> >                                 const char *uri, MigrateStart *args)
> >   {
>
> bit of a big change with lots of things moving around, I think it's
> mostly OK but...

I'll see how to split.

> > +    g_autoptr(GString) cmd_common = NULL;
> >       g_autofree gchar *arch_source = NULL;
> > +    g_autoptr(GString) cmd_source = NULL;
> >       g_autofree gchar *arch_target = NULL;
> > -    g_autofree gchar *cmd_source = NULL;
> > -    g_autofree gchar *cmd_target = NULL;
> > -    const gchar *ignore_stderr;
> > +    g_autoptr(GString) cmd_target = NULL;
> > +    const gchar *ignore_stderr = NULL;
> >       g_autofree char *bootpath = NULL;
> > -    g_autofree char *shmem_opts = NULL;
> >       g_autofree char *shmem_path = NULL;
> >       const char *arch = qtest_get_arch();
> >       const char *machine_opts = NULL;
> > @@ -602,6 +602,12 @@ static int test_migrate_start(QTestState **from, 
> > QTestState **to,
> >       }
> >   
> >       got_stop = false;
> > +
> > +    cmd_common = g_string_new("");
> > +    g_string_append(cmd_common, "-accel tcg ");
> > +    g_string_append_printf(cmd_common, "-accel kvm%s ",
> > +                           args->use_dirty_ring ? ",dirty-ring-size=4096" : 
> > "");
> > +
>
> Isn't that swapping the order of -accel tcg and -accel kvm ?
> In the original it's
>                      g_strdup_printf("-accel kvm%s -accel tcg%s%s "
>
> I think you're ending up with tcg first?

Oops good catch, thanks!