Re: [PATCH v5 1/5] iotests: remove 'linux' from default supported platforms

[Max Reitz]

On 02.10.19 18:44, Kevin Wolf wrote:
> Not sure where in this thread to reply, but since my name is mentioned
> in this mail, it might at least be not the worst one.
> 
> Am 02.10.2019 um 13:57 hat Max Reitz geschrieben:
>> On 02.10.19 06:46, Thomas Huth wrote:
>>> On 01/10/2019 20.44, Max Reitz wrote:
>>> [...]
>>>> As I have said, the conceptual problem is that the iotests now run as
>>>> part of make check.  As such, allowing auto tests to run on non-Linux
>>>> platforms may introduce build failures that I cannot do anything about.
>>>
>>> Well, simply run "make vm-build-openbsd", "make vm-build-freebsd", ...
>>> if something fails there, it likely should not be in the auto group.
>>
>> Then we come to Windows and macOS.
>>
>> What I’ve proposed to John on IRC was to simply skip the iotests in make
>> check for non-Linux systems.
> 
> I think this really makes sense. Or at least have a blacklist of OSes
> that we can't support iotests on, which would contain at least Windows
> and OS X. Windows because I'm pretty sure that it doesn't work anyway,
> and OS X because if something fails there, we have no way to reproduce.
> The occasional build failures on OS X that must be fixed by blindly
> guessing what could work without any way to test the fix are already bad
> enough.
> 
>>> Max, I can understand that you are a little bit annoyed that this "make
>>> check with iotests" caused some extra hurdles for you. But honestly,
>>> removing that again would be quite egoistic by you. Try to put yourself
>>> into the position of a "normal", non-blocklayer-maintainer QEMU
>>> developer. For me, iotests were a *constant* source of frustration.
>>> Often, when I ran them on top of my latest and greatest patches, to
>>> check whether I caused a regression, the iotests simply failed. Then I
>>> had to start debugging - did my patches cause the break, or is "master"
>>> broken, too? In almost all cases, there was an issue in the master
>>> branch already, either because they were failing on s390x, or because I
>>> was using ext4 instead of xfs, or because I was using an older distro
>>> than you, etc... . So while the iotests likely worked fine for the
>>> limited set of systems that you, Kevin and the other hard-core block
>>> layer developers are using, it's constantly broken for everybody else
>>> who is not using the very same setup as you. The only way of *not*
>>> getting upset about the iotests was to simply completely *ignore* them.
>>> Is it that what you want?
>>
>> It usually worked fine for me because it’s rather rare that non-block
>> patches broke the iotests.
> 
> I disagree. It happened all the time that someone else broke the iotests
> in master and we needed to fix it up.

OK.

(In my experience, it’s still mostly block patches, only that they tend
to go through non-block trees.)

>> Maybe my main problem is that I feel like now I have to deal with all of
>> the fallout, even though adding the iotests to make check wasn’t my idea
>> and neither me nor Kevin ever consented.  (I don’t know whether Kevin
>> consented implicitly, but I don’t see his name in bdd95e47844f2d8b.)
>>
>> You can’t just give me more responsibility without my consent and then
>> call me egoistic for not liking it.
> 
> I think I may have implicitly consented by helping Alex with the changes
> to 'check' that made its output fit better in 'make check'.
> 
> Basically, my stance is that I share your dislike for the effects on me
> personally (also, I can't run 'make check' any more before a pull
> request without testing half of the iotests twice because I still need a
> full iotests run), but I also think that having iotests in 'make check'
> is really the right thing for the project despite the inconvenience it
> means for me.

Then I expect you to NACK Thomas’s patch, and I take it to mean that I
can rely on you to handle basically all make check iotests failures that
are not caused by my own pull requests.

Works for me.

>> I know you’ll say that we just need to ensure we can add more tests,
>> then.  But for one thing, the most important tests are the ones that
>> take the longest, like 041.  And the other of course is that adding any
>> more tests to make check just brings me more pain, so I won’t do it.
> 
> That's something that can be fixed by tweaking the auto group.
> 
> Can we run tests in 'auto' that require the system emulator? If so,
> let's add 030 040 041 to the default set. They take long, but they are
> among the most valuable tests we have. If this makes the tests take too
> much time, let's remove some less important ones instead.
> 
>> [1] There is the recent case of Kevin’s pull request having been
>> rejected because his test failed on anything but x64.  I’m torn here,
>> because I would have seen that failure on my -m32 build.  So it isn’t
>> like it would have evaded our view for long.
> 
> I messed up, so this pull request was rightly stopped. I consider this
> one a feature, not a bug.

Sure, that was a good thing.  I just wonder whether that instance is
enough to justify the pain.

Max

signature.asc
Description: OpenPGP digital signature