Re: [Qemu-block] [VDSM] travis tests fail consistently since Apr 14

[Kevin Wolf]

Am 11.06.2018 um 12:43 hat Nir Soffer geschrieben:
> On Mon, Jun 11, 2018 at 1:03 PM Maor Lipchuk <address@hidden> wrote:
> 
> > On Tue, Jun 5, 2018 at 9:40 AM, Dan Kenigsberg <address@hidden> wrote:
> >
> >> On Mon, Jun 4, 2018 at 7:14 PM, Nir Soffer <address@hidden> wrote:
> >> > On Mon, Jun 4, 2018 at 6:56 PM Dan Kenigsberg <address@hidden>
> >> wrote:
> >> >>
> >> >> On Tue, May 8, 2018 at 11:59 AM, Nir Soffer <address@hidden>
> >> wrote:
> >> >> > There are several issues:
> >> >> >
> >> >> > 1. coverage fail after this patch:
> >> >> >
> >> >> >
> >> https://github.com/oVirt/vdsm/commit/6b905c2c134bcf344961d28eefbd05f2838d2ec8
> >> >> >
> >> >> > https://travis-ci.org/oVirt/vdsm/builds/366574414
> >> >> > ...
> >> >> > pwd
> >> >> > /vdsm/tests
> >> >> > ls .cov*
> >> >> > ls: cannot access .cov*: No such file or directory
> >> >> > make[1]: *** [check] Error 2
> >> >> > make[1]: Leaving directory `/vdsm/tests'
> >> >>
> >> >> That was me, sorry. This should solve it:
> >> >> https://gerrit.ovirt.org/#/c/91925/
> >> >
> >> > Thanks!
> >> >
> >> >> BTW, on my fc28 I see TestCountClusters.test_multiple_blocks failing
> >> with
> >> >>
> >> >> E           Error: Command ['/usr/bin/qemu-img', 'map', '--output',
> >> >> 'json', '/var/tmp/vdsm/test_multiple_blocks0/test'] failed with rc=-6
> >> >> out='' err="qemu-img:
> >> >> /builddir/build/BUILD/qemu-2.12.0-rc1/qemu-img.c:2680:
> >> >> get_block_status: Assertion `bytes' failed.\n"
> >> >>
> >> >> Any idea what's that?
> >> >
> >> >
> >> > Looks like qemu-img bug.
> >> >
> >> > Can you file a qemu-img bug?
> >>
> >> I hope Maor can translate the test to qemu-img speak.
> >>
> >
> > Opened the following bug:
> >   https://bugzilla.redhat.com/1589738
> >
> 
> Adding qemu-block

It's related to the unaligned image size. Correct image files should be
aligned to 512 byte sectors, so something is wrong with your image to
start with (hard disks don't have half sectors).

Anyway, git bisects points to this one:

a290f085901b528265787cd27ebda19c970be4ee is the first bad commit
commit a290f085901b528265787cd27ebda19c970be4ee
Author: Eric Blake <address@hidden>
Date:   Tue Feb 13 14:26:44 2018 -0600

    file-posix: Switch to .bdrv_co_block_status()
    
    We are gradually moving away from sector-based interfaces, towards
    byte-based.  Update the file protocol driver accordingly.
    
    In want_zero mode, we continue to report fine-grained hole
    information (the caller wants as much mapping detail as possible);
    but when not in that mode, the caller prefers larger *pnum and
    merely cares about what offsets are allocated at this layer, rather
    than where the holes live.  Since holes still read as zeroes at
    this layer (rather than deferring to a backing layer), we can take
    the shortcut of skipping lseek(), and merely state that all bytes
    are allocated.
    
    We can also drop redundant bounds checks that are already
    guaranteed by the block layer.
    
    Signed-off-by: Eric Blake <address@hidden>
    Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
    Reviewed-by: Fam Zheng <address@hidden>
    Signed-off-by: Kevin Wolf <address@hidden>

I think the problem is a bit higher up the call stack, but I'm not
completely sure yet. It manifests in img_map(), in this code:

        while (curr.start + curr.length < length) {
            ...
            n = QEMU_ALIGN_DOWN(MIN(1 << 30, length - offset), 
BDRV_SECTOR_SIZE);
            ret = get_block_status(bs, offset, n, &next);

The loop condition is still true because a single byte is left to be
processed, but n is aligned down to 0. I'm not sure why the
QEMU_ALIGN_DOWN() is even there.

Eric, would just removing the QEMU_ALIGN_DOWN() be correct?

Kevin