Re: [Qemu-devel] Failing iotests in v2.3.0-rc2 / master

[Kevin Wolf <kwolf@redhat.com>]

Am 15.04.2015 um 06:53 hat Jeff Cody geschrieben:
> On Tue, Apr 14, 2015 at 11:57:35AM +0200, Kevin Wolf wrote:
> > Am 11.04.2015 um 05:41 hat Andreas Färber geschrieben:
> > > Hi,
> > > 
> > > 001 seems to hang for -qcow (or is not reasonably "quick": >5 min).
> > > 
> > > 033 is failing for -vhdx.
> > > 
> > > (Note that `make check-block` only tests -qcow2, so didn't uncover
> > > either of them.)
> > > 
> > > Given a failing test, am I seeing correctly that there is no command
> > > line option to skip this one failing test? -x seems to be for groups only.
> > > 
> > > Regards,
> > > Andreas
> > > 
> > > $ ./check -v -T -qcow -g quick
> > > [...]
> > > 001 6s ...        [05:12:39]
> > 
> > qcow1 is just really slow. 001 passes for me, after 202 seconds (that's
> > on my SSD, YMMV).
> > 
> > > $ ./check -v -T -vhdx -g quick
> > > [...]
> > > 033 1s ...        [04:06:09] [04:06:11] - output mismatch (see 033.out.bad)
> > 
> > This seems to be because blkdebug doesn't implement .bdrv_truncate.
> > Currently the test case isn't suitable for VHDX, which uses explicit
> > bdrv_truncate() calls to grow the image file. I'll send a patch for
> > blkdebug to allow this.
> > 
> > However, it seems that there is another problem which causes assertion
> > failures when using VHDX over blkdebug. Jeff, does the following fix
> > make sense to you? (I think it does, but I don't understand yet why the
> > assertion failure is only triggered with blkdebug - or in other words:
> > "how could this ever work?")
> > 
> > Kevin
> 
> Kevin,
> 
> Yes, looking at that fix it makes sense - we are wanting to pad the
> back part of the block after the actual data with zeros. That back
> length should be (block size - (bytes avail + block offset)), which is
> iov2.iov_len.
> 
> There are two reasons I think we haven't seen this issue (it has been
> hidden):
> 
> 1.) If bs->file supports zero init, we don't do any of this

I see. file does and blkdebug doesn't, so that's the crucial difference.

> 2.) This is done for the case when the existing BAT state is
> PAYLOAD_BLOCK_ZERO.  Until recently (commit 30af51c), we didn't
> create VHDX files with blocks in the PAYLOAD_BLOCK_ZERO state.

Right, I wasn't aware of this either any more.

> So it has been a latent bug in a hitherto rarely (if ever) exercised
> path.

Thanks for your explanation, it's clear to me now what's going on. I'll
send out the patches (for both blkdebug and vhdx) right away. You can
either pick up the vhdx one, or just give your Acked-by and then I'll
merge it through my block tree.

Kevin