|
From: | Eric Blake |
Subject: | Re: [Qemu-devel] [PATCH v4 09/20] parallels: Switch to .bdrv_co_block_status() |
Date: | Thu, 30 Nov 2017 16:18:02 -0600 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 11/30/2017 04:12 PM, Eric Blake wrote:
BDRVParallelsState *s = bs->opaque; - int64_t offset; + int count; + assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE)); qemu_co_mutex_lock(&s->lock); - offset = block_status(s, sector_num, nb_sectors, pnum); + offset = block_status(s, offset >> BDRV_SECTOR_BITS, + bytes >> BDRV_SECTOR_BITS, &count); qemu_co_mutex_unlock(&s->lock); if (offset < 0) {pnum=count*BDRV_SECTOR_SIZE and map=0 setting missed here.return 0; }Setting *map is only required when return value includes BDRV_BLOCK_OFFSET_VALID, so that one was not necessary. But you do raise an interesting point about a pre-existing bug with pnum not being set. Commit 298a1665a guarantees that *pnum is 0 (and not uninitialized garbage) - but that still violates the contract that we (want to) have that all drivers either make progress or return an error (setting pnum to 0 should only be done at EOF or on error).
Oh. The pre-existing code DID set pnum to a non-zero value, as a side effect of block_status(); the new code fails to do so. So it is not pre-existing; good catch!
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Prev in Thread] | Current Thread | [Next in Thread] |