[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH v4 09/20] parallels: Switch to .bdr

From: Eric Blake
Subject: Re: [Qemu-block] [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));
-    offset = block_status(s, sector_num, nb_sectors, pnum);
+    offset = block_status(s, offset >> BDRV_SECTOR_BITS,
+                          bytes >> BDRV_SECTOR_BITS, &count);

      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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]