Re: [PATCH] Revert "tests/migration: Reduce autoconverge initial bandwidth"

[Thomas Huth]

On 23/06/2020 19.35, Philippe Mathieu-Daudé wrote:
> On 6/23/20 7:07 PM, Thomas Huth wrote:
>> On 23/06/2020 17.39, Philippe Mathieu-Daudé wrote:
>>> On 6/23/20 4:56 PM, Michael S. Tsirkin wrote:
>>>> This reverts commit 6d1da867e65f ("tests/migration: Reduce autoconverge 
>>>> initial bandwidth")
>>>> since that change makes unit tests much slower for all developers, while 
>>>> it's not
>>>> a robust way to fix migration tests. Migration tests need to find
>>>> a more robust way to discover a reasonable bandwidth without slowing
>>>> things down for everyone.
>>>
>>> Please also mention we can do this since 1de8e4c4dcf which allow
>>> marked the s390x job as "unstable" and allow it to fail.
>>>
>>> But if nobody is going to look at it, instead lets disable
>>> it until someone figure out the issue:
>>>
>>> -- >8 --
>>> diff --git a/.travis.yml b/.travis.yml
>>> index 74158f741b..364e67b14b 100644
>>> --- a/.travis.yml
>>> +++ b/.travis.yml
>>> @@ -507,6 +507,7 @@ jobs:
>>>
>>>      - name: "[s390x] Clang (disable-tcg)"
>>>        arch: s390x
>>> +      if: false # Temporarily disabled due to issue testing migration
>>> (see commit 6d1da867e65).
>>>        dist: bionic
>>>        compiler: clang
>>>        addons:
>>
>> Sorry, but that looks wrong. First, the disable-tcg test does not run
>> the qtests at all. So this is certainly the wrong location here.
> 
> Indeed, this is the previous job:
> 
> -- >8 --
> diff --git a/.travis.yml b/.travis.yml
> index 74158f741b..b399e20078 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -464,6 +464,7 @@ jobs:
>          - CONFIG="--disable-containers
> --target-list=ppc64-softmmu,ppc64le-linux-user"
> 
>      - name: "[s390x] GCC check-tcg"
> +      if: false # Temporarily disabled due to issue testing migration
> (see commit 6d1da867e65).
>        arch: s390x
>        dist: bionic
>        addons:
> ---
> 
>> Second,
>> if just one of the qtests is failing, please only disable that single
>> failing qtest and not the whole test pipeline.
> 
> Last time we talked about this Dave was against that option:
> 
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg690085.html
> 

Was he? Citing his reply to the mail from your URL:

 "Before we take the hammer to it, could you try reducing it's initial
bandwidth"

So all I can see is that he first wanted to try something different than
disabling the test. And now,  instead of using a small hammer to disable
just this test, you now even use a very *big* hammer to disable *all*
tests. That's just a very bad idea. Please don't.

 Thomas