1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH for 4.1? v1 0/7] testing/next (docker, win-cross)
  4. View patch

[Qemu-devel] [PATCH v1 5/7] tests/migration-test: don't spam the logs when we fail

Alex Bennée posted 7 patches 6 years, 6 months ago
[Qemu-devel] [PATCH v1 5/7] tests/migration-test: don't spam the logs when we fail
Posted by Alex Bennée 6 years, 6 months ago 
Quite often the information about which test failed is hidden by the
wall of repeated failures for each page. Stop outputting the error
after 10 bad pages and just summarise the total damage at the end.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/migration-test.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index b6434628e1c..ce041f80c2a 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -308,7 +308,7 @@ static void check_guests_ram(QTestState *who)
     uint8_t first_byte;
     uint8_t last_byte;
     bool hit_edge = false;
-    bool bad = false;
+    int bad = 0;
 
     qtest_memread(who, start_address, &first_byte, 1);
     last_byte = first_byte;
@@ -327,15 +327,20 @@ static void check_guests_ram(QTestState *who)
                 hit_edge = true;
                 last_byte = b;
             } else {
-                fprintf(stderr, "Memory content inconsistency at %x"
-                                " first_byte = %x last_byte = %x current = %x"
-                                " hit_edge = %x\n",
-                                address, first_byte, last_byte, b, hit_edge);
-                bad = true;
+                bad++;
+                if (bad <= 10) {
+                    fprintf(stderr, "Memory content inconsistency at %x"
+                            " first_byte = %x last_byte = %x current = %x"
+                            " hit_edge = %x\n",
+                            address, first_byte, last_byte, b, hit_edge);
+                }
             }
         }
     }
-    g_assert_false(bad);
+    if (bad >= 10) {
+        fprintf(stderr, "and in another %d pages", bad);
+    }
+    g_assert(bad == 0);
 }
 
 static void cleanup(const char *filename)
-- 
2.20.1
Re: [Qemu-devel] [PATCH v1 5/7] tests/migration-test: don't spam the logs when we fail
Posted by Dr. David Alan Gilbert 6 years, 6 months ago 
* Alex Bennée (alex.bennee@linaro.org) wrote:
> Quite often the information about which test failed is hidden by the
> wall of repeated failures for each page. Stop outputting the error
> after 10 bad pages and just summarise the total damage at the end.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Yep, occasionally you do find you do want the full set to see what's
going on, but this is true most of the time.

With 10 you should be able to see if it's a single page that's wrong or
the flip gone wrong.

> ---
>  tests/migration-test.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index b6434628e1c..ce041f80c2a 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -308,7 +308,7 @@ static void check_guests_ram(QTestState *who)
>      uint8_t first_byte;
>      uint8_t last_byte;
>      bool hit_edge = false;
> -    bool bad = false;
> +    int bad = 0;
>  
>      qtest_memread(who, start_address, &first_byte, 1);
>      last_byte = first_byte;
> @@ -327,15 +327,20 @@ static void check_guests_ram(QTestState *who)
>                  hit_edge = true;
>                  last_byte = b;
>              } else {
> -                fprintf(stderr, "Memory content inconsistency at %x"
> -                                " first_byte = %x last_byte = %x current = %x"
> -                                " hit_edge = %x\n",
> -                                address, first_byte, last_byte, b, hit_edge);
> -                bad = true;
> +                bad++;
> +                if (bad <= 10) {
> +                    fprintf(stderr, "Memory content inconsistency at %x"
> +                            " first_byte = %x last_byte = %x current = %x"
> +                            " hit_edge = %x\n",
> +                            address, first_byte, last_byte, b, hit_edge);
> +                }
>              }
>          }
>      }
> -    g_assert_false(bad);
> +    if (bad >= 10) {
> +        fprintf(stderr, "and in another %d pages", bad);
> +    }

as Laurent says, the 'another' would need -10 but other than that:


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

> +    g_assert(bad == 0);
>  }
>  
>  static void cleanup(const char *filename)
> -- 
> 2.20.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v1 5/7] tests/migration-test: don't spam the logs when we fail
Posted by Thomas Huth 6 years, 6 months ago 
On 12/07/2019 13.18, Alex Bennée wrote:
> Quite often the information about which test failed is hidden by the
> wall of repeated failures for each page. Stop outputting the error
> after 10 bad pages and just summarise the total damage at the end.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/migration-test.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index b6434628e1c..ce041f80c2a 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -308,7 +308,7 @@ static void check_guests_ram(QTestState *who)
>      uint8_t first_byte;
>      uint8_t last_byte;
>      bool hit_edge = false;
> -    bool bad = false;
> +    int bad = 0;
>  
>      qtest_memread(who, start_address, &first_byte, 1);
>      last_byte = first_byte;
> @@ -327,15 +327,20 @@ static void check_guests_ram(QTestState *who)
>                  hit_edge = true;
>                  last_byte = b;
>              } else {
> -                fprintf(stderr, "Memory content inconsistency at %x"
> -                                " first_byte = %x last_byte = %x current = %x"
> -                                " hit_edge = %x\n",
> -                                address, first_byte, last_byte, b, hit_edge);
> -                bad = true;
> +                bad++;
> +                if (bad <= 10) {
> +                    fprintf(stderr, "Memory content inconsistency at %x"
> +                            " first_byte = %x last_byte = %x current = %x"
> +                            " hit_edge = %x\n",
> +                            address, first_byte, last_byte, b, hit_edge);
> +                }
>              }
>          }
>      }
> -    g_assert_false(bad);
> +    if (bad >= 10) {
> +        fprintf(stderr, "and in another %d pages", bad);
> +    }
> +    g_assert(bad == 0);
>  }
>  
>  static void cleanup(const char *filename)

Good idea.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Re: [Qemu-devel] [PATCH v1 5/7] tests/migration-test: don't spam the logs when we fail
Posted by Laurent Vivier 6 years, 6 months ago 
On 12/07/2019 13:18, Alex Bennée wrote:
> Quite often the information about which test failed is hidden by the
> wall of repeated failures for each page. Stop outputting the error
> after 10 bad pages and just summarise the total damage at the end.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/migration-test.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index b6434628e1c..ce041f80c2a 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -308,7 +308,7 @@ static void check_guests_ram(QTestState *who)
>      uint8_t first_byte;
>      uint8_t last_byte;
>      bool hit_edge = false;
> -    bool bad = false;
> +    int bad = 0;
>  
>      qtest_memread(who, start_address, &first_byte, 1);
>      last_byte = first_byte;
> @@ -327,15 +327,20 @@ static void check_guests_ram(QTestState *who)
>                  hit_edge = true;
>                  last_byte = b;
>              } else {
> -                fprintf(stderr, "Memory content inconsistency at %x"
> -                                " first_byte = %x last_byte = %x current = %x"
> -                                " hit_edge = %x\n",
> -                                address, first_byte, last_byte, b, hit_edge);
> -                bad = true;
> +                bad++;
> +                if (bad <= 10) {
> +                    fprintf(stderr, "Memory content inconsistency at %x"
> +                            " first_byte = %x last_byte = %x current = %x"
> +                            " hit_edge = %x\n",
> +                            address, first_byte, last_byte, b, hit_edge);
> +                }
>              }
>          }
>      }
> -    g_assert_false(bad);
> +    if (bad >= 10) {
> +        fprintf(stderr, "and in another %d pages", bad);

"bad - 10" as you have already displayed 10 errors.

Thanks,
Laurent
Re: [Qemu-devel] [PATCH v1 5/7] tests/migration-test: don't spam the logs when we fail
Posted by Alex Bennée 6 years, 6 months ago 
Laurent Vivier <lvivier@redhat.com> writes:

> On 12/07/2019 13:18, Alex Bennée wrote:
>> Quite often the information about which test failed is hidden by the
>> wall of repeated failures for each page. Stop outputting the error
>> after 10 bad pages and just summarise the total damage at the end.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  tests/migration-test.c | 19 ++++++++++++-------
>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index b6434628e1c..ce041f80c2a 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -308,7 +308,7 @@ static void check_guests_ram(QTestState *who)
>>      uint8_t first_byte;
>>      uint8_t last_byte;
>>      bool hit_edge = false;
>> -    bool bad = false;
>> +    int bad = 0;
>>
>>      qtest_memread(who, start_address, &first_byte, 1);
>>      last_byte = first_byte;
>> @@ -327,15 +327,20 @@ static void check_guests_ram(QTestState *who)
>>                  hit_edge = true;
>>                  last_byte = b;
>>              } else {
>> -                fprintf(stderr, "Memory content inconsistency at %x"
>> -                                " first_byte = %x last_byte = %x current = %x"
>> -                                " hit_edge = %x\n",
>> -                                address, first_byte, last_byte, b, hit_edge);
>> -                bad = true;
>> +                bad++;
>> +                if (bad <= 10) {
>> +                    fprintf(stderr, "Memory content inconsistency at %x"
>> +                            " first_byte = %x last_byte = %x current = %x"
>> +                            " hit_edge = %x\n",
>> +                            address, first_byte, last_byte, b, hit_edge);
>> +                }
>>              }
>>          }
>>      }
>> -    g_assert_false(bad);
>> +    if (bad >= 10) {
>> +        fprintf(stderr, "and in another %d pages", bad);
>
> "bad - 10" as you have already displayed 10 errors.

Will do.

>
> Thanks,
> Laurent


--
Alex Bennée
Re: [Qemu-devel] [PATCH v1 5/7] tests/migration-test: don't spam the logs when we fail
Posted by Laurent Vivier 6 years, 6 months ago 
On 12/07/2019 15:35, Alex Bennée wrote:
> 
> Laurent Vivier <lvivier@redhat.com> writes:
> 
>> On 12/07/2019 13:18, Alex Bennée wrote:
>>> Quite often the information about which test failed is hidden by the
>>> wall of repeated failures for each page. Stop outputting the error
>>> after 10 bad pages and just summarise the total damage at the end.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>  tests/migration-test.c | 19 ++++++++++++-------
>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>>> index b6434628e1c..ce041f80c2a 100644
>>> --- a/tests/migration-test.c
>>> +++ b/tests/migration-test.c
>>> @@ -308,7 +308,7 @@ static void check_guests_ram(QTestState *who)
>>>      uint8_t first_byte;
>>>      uint8_t last_byte;
>>>      bool hit_edge = false;
>>> -    bool bad = false;
>>> +    int bad = 0;
>>>
>>>      qtest_memread(who, start_address, &first_byte, 1);
>>>      last_byte = first_byte;
>>> @@ -327,15 +327,20 @@ static void check_guests_ram(QTestState *who)
>>>                  hit_edge = true;
>>>                  last_byte = b;
>>>              } else {
>>> -                fprintf(stderr, "Memory content inconsistency at %x"
>>> -                                " first_byte = %x last_byte = %x current = %x"
>>> -                                " hit_edge = %x\n",
>>> -                                address, first_byte, last_byte, b, hit_edge);
>>> -                bad = true;
>>> +                bad++;
>>> +                if (bad <= 10) {
>>> +                    fprintf(stderr, "Memory content inconsistency at %x"
>>> +                            " first_byte = %x last_byte = %x current = %x"
>>> +                            " hit_edge = %x\n",
>>> +                            address, first_byte, last_byte, b, hit_edge);
>>> +                }
>>>              }
>>>          }
>>>      }
>>> -    g_assert_false(bad);
>>> +    if (bad >= 10) {
>>> +        fprintf(stderr, "and in another %d pages", bad);
>>
>> "bad - 10" as you have already displayed 10 errors.
> 
> Will do.

You can add my:

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Thanks,
Laurent

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.