[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/4] block: Avoid copy-on-read assertions
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 0/4] block: Avoid copy-on-read assertions |
Date: |
Mon, 2 Oct 2017 16:50:30 +0200 |
User-agent: |
Mutt/1.9.0 (2017-09-02) |
Am 01.10.2017 um 00:05 hat Eric Blake geschrieben:
> On 09/30/2017 04:19 PM, address@hidden wrote:
> > Hi,
> >
> > This series failed build test on s390x host. Please find the details below.
> >
>
> > /var/tmp/patchew-tester-tmp-a2p2tpcc/src/block/io.c: In function
> > ‘bdrv_aligned_preadv’:
> > /var/tmp/patchew-tester-tmp-a2p2tpcc/src/block/io.c:955:9: error: ‘ret’ may
> > be used uninitialized in this function [-Werror=maybe-uninitialized]
> > int ret;
> > ^~~
>
> Blah - I compiled with -g instead of -O2, which masks this warning in my
> setup.
>
> The warning is a false negative (the error message is actually pointing
> to a line in bdrv_co_do_copy_on_readv - but the compiler must have
> inlined it into bdrv_aligned_preadv) - the function is only ever called
> with non-zero bytes, and therefore the 'while (cluster_bytes)' loop will
> execute at least once, and ret always gets assigned. But the compiler
> can't see that, so I'll squash this in:
Well, you could help the compiler with this:
assert(cluster_bytes > 0);
Then it compiles. Unfortunately, the compiler was right and you weren't:
$ ./qemu-io -C -c 'read 0 0' /tmp/test.qcow2
qemu-io: block/io.c:988: bdrv_co_do_copy_on_readv: Assertion `cluster_bytes
> 0' failed.
Abgebrochen (Speicherabzug geschrieben)
Maybe a case to add to the test?
> commit a201636c3133827bd632d5fdd9eb1f5df81d0e0e
> Author: Eric Blake <address@hidden>
> Date: Sat Sep 30 14:27:51 2017 -0500
>
> fixup! block: Perform copy-on-read in loop
>
> Signed-off-by: Eric Blake <address@hidden>
>
> diff --git a/block/io.c b/block/io.c
> index 5ef5adc7a7..e7519464bb 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -952,7 +952,7 @@ static int coroutine_fn
> bdrv_co_do_copy_on_readv(BdrvChild *child,
> int64_t cluster_offset;
> unsigned int cluster_bytes;
> size_t skip_bytes;
> - int ret;
> + int ret = 0;
> int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
> BDRV_REQUEST_MAX_BYTES);
> unsigned int progress = 0;
I would prefer a ret = 0 immediately before err: so that we'll still get
warning if we forget assigning ret in any future error path.
Kevin
signature.asc
Description: PGP signature
- Re: [Qemu-devel] [PATCH 0/4] block: Avoid copy-on-read assertions,
Kevin Wolf <=