diff mbox series

[v4,04/11] tests: Use consistent names and sizes for migration

Message ID 20180105215246.908-5-quintela@redhat.com
State New
Headers show
Series Add make check tests for Migration | expand

Commit Message

Juan Quintela Jan. 5, 2018, 9:52 p.m. UTC
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/migration-test.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Peter Xu Jan. 10, 2018, 6:30 a.m. UTC | #1
On Fri, Jan 05, 2018 at 10:52:39PM +0100, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/migration-test.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index d81f22118b..f469235d0b 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>  
>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>          init_bootfile_x86(bootpath);
> -        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
> -                                  " -name pcsource,debug-threads=on"
> +        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> +                                  " -name source,debug-threads=on"

A pure question: when will the name matter?

>                                    " -serial file:%s/src_serial"
>                                    " -drive file=%s,format=raw",
>                                    accel, tmpfs, bootpath);
> -        cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
> -                                  " -name pcdest,debug-threads=on"
> +        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
> +                                  " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -drive file=%s,format=raw"
>                                    " -incoming %s",
> @@ -459,12 +459,12 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>          }
>          init_bootfile_ppc(bootpath);
>          cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> -                                  " -name pcsource,debug-threads=on"
> +                                  " -name source,debug-threads=on"
>                                    " -serial file:%s/src_serial"
>                                    " -drive file=%s,if=pflash,format=raw",
>                                    accel, tmpfs, bootpath);
>          cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
> -                                  " -name pcdest,debug-threads=on"
> +                                  " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -drive file=%s,if=pflash,format=raw"
>                                    " -incoming %s",
> -- 
> 2.14.3
>
Juan Quintela Jan. 10, 2018, 8:43 a.m. UTC | #2
Peter Xu <peterx@redhat.com> wrote:
> On Fri, Jan 05, 2018 at 10:52:39PM +0100, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  tests/migration-test.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>> 
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index d81f22118b..f469235d0b 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>>  
>>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>          init_bootfile_x86(bootpath);
>> -        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
>> -                                  " -name pcsource,debug-threads=on"
>> +        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
>> +                                  " -name source,debug-threads=on"
>
> A pure question: when will the name matter?

It don't matter, but ARM didn't want to use the pcname, and decided for
yet a different command line.  I would like to have the same command
line for all architectures.  At least the less gratuitous differences.

As you can guess, name don't matter at all., but telling ARNM people to
be consistent with things that are not consistent .... O:-)


Later, Juan.


>
>>                                    " -serial file:%s/src_serial"
>>                                    " -drive file=%s,format=raw",
>>                                    accel, tmpfs, bootpath);
>> -        cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
>> -                                  " -name pcdest,debug-threads=on"
>> +        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
>> +                                  " -name target,debug-threads=on"
>>                                    " -serial file:%s/dest_serial"
>>                                    " -drive file=%s,format=raw"
>>                                    " -incoming %s",
>> @@ -459,12 +459,12 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>>          }
>>          init_bootfile_ppc(bootpath);
>>          cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
>> -                                  " -name pcsource,debug-threads=on"
>> +                                  " -name source,debug-threads=on"
>>                                    " -serial file:%s/src_serial"
>>                                    " -drive file=%s,if=pflash,format=raw",
>>                                    accel, tmpfs, bootpath);
>>          cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
>> -                                  " -name pcdest,debug-threads=on"
>> +                                  " -name target,debug-threads=on"
>>                                    " -serial file:%s/dest_serial"
>>                                    " -drive file=%s,if=pflash,format=raw"
>>                                    " -incoming %s",
>> -- 
>> 2.14.3
>>
Peter Xu Jan. 10, 2018, 8:54 a.m. UTC | #3
On Wed, Jan 10, 2018 at 09:43:48AM +0100, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > On Fri, Jan 05, 2018 at 10:52:39PM +0100, Juan Quintela wrote:
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >>  tests/migration-test.c | 12 ++++++------
> >>  1 file changed, 6 insertions(+), 6 deletions(-)
> >> 
> >> diff --git a/tests/migration-test.c b/tests/migration-test.c
> >> index d81f22118b..f469235d0b 100644
> >> --- a/tests/migration-test.c
> >> +++ b/tests/migration-test.c
> >> @@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
> >>  
> >>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> >>          init_bootfile_x86(bootpath);
> >> -        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
> >> -                                  " -name pcsource,debug-threads=on"
> >> +        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> >> +                                  " -name source,debug-threads=on"
> >
> > A pure question: when will the name matter?
> 
> It don't matter, but ARM didn't want to use the pcname, and decided for
> yet a different command line.  I would like to have the same command
> line for all architectures.  At least the less gratuitous differences.
> 
> As you can guess, name don't matter at all., but telling ARNM people to
> be consistent with things that are not consistent .... O:-)

Ah, fine. :)

This test only support x86 and ppc for now, right?

(Btw, AFAIK debug-threads=on is only useful if any of us wants to
 glance at thread names.  In other words, would it be even simpler to
 just remove that "-name" line? :-)
Dr. David Alan Gilbert Jan. 10, 2018, 11:49 a.m. UTC | #4
* Juan Quintela (quintela@redhat.com) wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/migration-test.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index d81f22118b..f469235d0b 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>  
>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>          init_bootfile_x86(bootpath);
> -        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
> -                                  " -name pcsource,debug-threads=on"
> +        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> +                                  " -name source,debug-threads=on"
>                                    " -serial file:%s/src_serial"
>                                    " -drive file=%s,format=raw",
>                                    accel, tmpfs, bootpath);
> -        cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
> -                                  " -name pcdest,debug-threads=on"
> +        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
> +                                  " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -drive file=%s,format=raw"
>                                    " -incoming %s",
> @@ -459,12 +459,12 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>          }
>          init_bootfile_ppc(bootpath);
>          cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> -                                  " -name pcsource,debug-threads=on"
> +                                  " -name source,debug-threads=on"
>                                    " -serial file:%s/src_serial"
>                                    " -drive file=%s,if=pflash,format=raw",
>                                    accel, tmpfs, bootpath);
>          cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
> -                                  " -name pcdest,debug-threads=on"
> +                                  " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -drive file=%s,if=pflash,format=raw"
>                                    " -incoming %s",

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

(I'm OK with increasing the RAM size but we don't need to, some may find
it a bit wasteful in tests, but it's only 106M extra)

> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Dr. David Alan Gilbert Jan. 10, 2018, 11:49 a.m. UTC | #5
* Peter Xu (peterx@redhat.com) wrote:
> On Wed, Jan 10, 2018 at 09:43:48AM +0100, Juan Quintela wrote:
> > Peter Xu <peterx@redhat.com> wrote:
> > > On Fri, Jan 05, 2018 at 10:52:39PM +0100, Juan Quintela wrote:
> > >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> > >> ---
> > >>  tests/migration-test.c | 12 ++++++------
> > >>  1 file changed, 6 insertions(+), 6 deletions(-)
> > >> 
> > >> diff --git a/tests/migration-test.c b/tests/migration-test.c
> > >> index d81f22118b..f469235d0b 100644
> > >> --- a/tests/migration-test.c
> > >> +++ b/tests/migration-test.c
> > >> @@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
> > >>  
> > >>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> > >>          init_bootfile_x86(bootpath);
> > >> -        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
> > >> -                                  " -name pcsource,debug-threads=on"
> > >> +        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> > >> +                                  " -name source,debug-threads=on"
> > >
> > > A pure question: when will the name matter?
> > 
> > It don't matter, but ARM didn't want to use the pcname, and decided for
> > yet a different command line.  I would like to have the same command
> > line for all architectures.  At least the less gratuitous differences.
> > 
> > As you can guess, name don't matter at all., but telling ARNM people to
> > be consistent with things that are not consistent .... O:-)
> 
> Ah, fine. :)
> 
> This test only support x86 and ppc for now, right?
> 
> (Btw, AFAIK debug-threads=on is only useful if any of us wants to
>  glance at thread names.  In other words, would it be even simpler to
>  just remove that "-name" line? :-)

It makes debugging hangs/crashes easier sometimes, so keep it.

Dave

> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Juan Quintela Jan. 10, 2018, 2:58 p.m. UTC | #6
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Peter Xu (peterx@redhat.com) wrote:
>> On Wed, Jan 10, 2018 at 09:43:48AM +0100, Juan Quintela wrote:
>> > Peter Xu <peterx@redhat.com> wrote:
>> > > On Fri, Jan 05, 2018 at 10:52:39PM +0100, Juan Quintela wrote:
>> > >> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> > >> ---
>> > >>  tests/migration-test.c | 12 ++++++------
>> > >>  1 file changed, 6 insertions(+), 6 deletions(-)
>> > >> 
>> > >> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> > >> index d81f22118b..f469235d0b 100644
>> > >> --- a/tests/migration-test.c
>> > >> +++ b/tests/migration-test.c
>> > >> @@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>> > >>  
>> > >>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> > >>          init_bootfile_x86(bootpath);
>> > >> -        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
>> > >> -                                  " -name pcsource,debug-threads=on"
>> > >> +        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
>> > >> +                                  " -name source,debug-threads=on"
>> > >
>> > > A pure question: when will the name matter?
>> > 
>> > It don't matter, but ARM didn't want to use the pcname, and decided for
>> > yet a different command line.  I would like to have the same command
>> > line for all architectures.  At least the less gratuitous differences.
>> > 
>> > As you can guess, name don't matter at all., but telling ARNM people to
>> > be consistent with things that are not consistent .... O:-)
>> 
>> Ah, fine. :)
>> 
>> This test only support x86 and ppc for now, right?
>> 
>> (Btw, AFAIK debug-threads=on is only useful if any of us wants to
>>  glance at thread names.  In other words, would it be even simpler to
>>  just remove that "-name" line? :-)
>
> It makes debugging hangs/crashes easier sometimes, so keep it.

Agreed.  I think that this should be default on qemu O:-)

Later, Juan.
diff mbox series

Patch

diff --git a/tests/migration-test.c b/tests/migration-test.c
index d81f22118b..f469235d0b 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -440,13 +440,13 @@  static void test_migrate_start(QTestState **from, QTestState **to,
 
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         init_bootfile_x86(bootpath);
-        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
-                                  " -name pcsource,debug-threads=on"
+        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
+                                  " -name source,debug-threads=on"
                                   " -serial file:%s/src_serial"
                                   " -drive file=%s,format=raw",
                                   accel, tmpfs, bootpath);
-        cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
-                                  " -name pcdest,debug-threads=on"
+        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
+                                  " -name target,debug-threads=on"
                                   " -serial file:%s/dest_serial"
                                   " -drive file=%s,format=raw"
                                   " -incoming %s",
@@ -459,12 +459,12 @@  static void test_migrate_start(QTestState **from, QTestState **to,
         }
         init_bootfile_ppc(bootpath);
         cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
-                                  " -name pcsource,debug-threads=on"
+                                  " -name source,debug-threads=on"
                                   " -serial file:%s/src_serial"
                                   " -drive file=%s,if=pflash,format=raw",
                                   accel, tmpfs, bootpath);
         cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
-                                  " -name pcdest,debug-threads=on"
+                                  " -name target,debug-threads=on"
                                   " -serial file:%s/dest_serial"
                                   " -drive file=%s,if=pflash,format=raw"
                                   " -incoming %s",