On 05/02/2018 02:13 PM, Max Reitz wrote: > On 2018-05-02 20:03, John Snow wrote: >> >> >> On 04/21/2018 12:54 PM, Max Reitz wrote: >>> This test case has been broken since 398e6ad014df261d (roughly half a >>> year). qemu-img amend requires its output image to be R/W, so it opens >>> it as such; the node is then turned into an read-only node automatically >>> which is now accompanied by a warning, however. This warning has not >>> been part of the reference output. >>> >>> For one thing, this warning shows that we cannot keep the test case as >>> it is. We would need a format that has no create_opts but that does >>> have write support -- we do not have such a format, though. >>> >>> Another thing is that qemu now actually checks whether an image format >>> supports amendment instead of whether it has create_opts (since the >>> former always implies the latter). So we can now use any format that >>> does not support amendment (even if it supports creation) and thus test >>> the same code path. >>> >>> The reason nobody has noticed the breakage until now of course is the >>> fact that nobody runs the iotests for nbd+bochs. There actually was >>> never any reason to set the protocol to "nbd" but because that was >>> technically correct; functionally it made no difference. So that is the >>> first thing we are going to change: Make the protocol "file" instead so >>> that people might actually notice breakage here. >>> >>> Secondly, now that bochs no longer works for the amend test case, we >>> have to change the format there anyway. Set let us just bend the truth > > I suppose s/Set/So/ > >>> a bit, declare this test a raw test. In fact, that does not even >>> concern the bochs test cases, other than the output now reading 'bochs' >>> instead of 'IMGFMT'. >>> >>> So with this test now being a raw test, we can rework the amend test >>> case to use raw instead. >>> >>> Signed-off-by: Max Reitz <mre...@redhat.com> >> >> Well, it passes... Not sure if I'm wild about the format change, it >> sounds like a failure of our CI more than something that needed to >> change in the test, but... shrug. > > I think it's not really an issue in the CI. Testing all possible > combinations of protocols and formats seems too much to me. > > I think it really is an issue in our test suite. There are many tests > that work only with a single combination (numerous file+qcow2 tests, for > instance), so having our great test matrix capability is completely > useless for them. > > Maybe tests should be able to offer a preferred format+protocol > (+options) combination? Then, when you run check without any such > arguments, it would just run each test in its preferred mode. > > Max >
"You're not wrong" -- iotests presents an impossible matrix. You're not going to fix our infrastructural problems with this one patch, which looks like a hack in contrast to how the rest of iotests is presently structured. I don't oppose it, though. Just voicing my full-throated ambivalence. There's definitely a discussion or two to be had about what instrumenting iotests looks like and how we can ensure we're getting proper coverage. The current solution *is* bad. --js
