Re: [Qemu-devel] [Qemu-block] [PATCH v2 5/8] qcow: Switch to a byte-based driver

[Jeff Cody <jcody@redhat.com>]

On Tue, Jun 05, 2018 at 06:12:33AM -0500, Eric Blake wrote:
> On 06/04/2018 04:33 PM, Jeff Cody wrote:
> >On Thu, May 31, 2018 at 03:50:43PM -0500, Eric Blake wrote:
> >>We are gradually moving away from sector-based interfaces, towards
> >>byte-based.  The qcow driver is now ready to fully utilize the
> >>byte-based callback interface, as long as we override the default
> >>alignment to still be 512 (needed at least for asserts present
> >>because of encryption, but easier to do everywhere than to audit
> >>which sub-sector requests are handled correctly, especially since
> >>we no longer recommend qcow for new disk images).
> >>
> >>Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> >>-static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
> >>-                         int nb_sectors, QEMUIOVector *qiov)
> >>+static void qcow_refresh_limits(BlockDriverState *bs, Error **errp)
> >>+{
> >>+    /* At least encrypted images require 512-byte alignment. Apply the
> >>+     * limit universally, rather than just on encrypted images, as
> >>+     * it's easier to let the block layer handle rounding than to
> >>+     * audit this code further. */
> >>+    bs->bl.request_alignment = BDRV_SECTOR_SIZE;
> >>+}
> >>+
> >>+static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, uint64_t offset,
> >>+                                       uint64_t bytes, QEMUIOVector *qiov,
> >>+                                       int flags)
> >>  {
> >>      BDRVQcowState *s = bs->opaque;
> >>      int offset_in_cluster;
> >>@@ -624,9 +632,8 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
> >>      QEMUIOVector hd_qiov;
> >>      uint8_t *buf;
> >>      void *orig_buf;
> >>-    int64_t offset = sector_num * BDRV_SECTOR_SIZE;
> >>-    int64_t bytes = nb_sectors * BDRV_SECTOR_SIZE;
> >>
> >>+    assert(!flags);
> >
> >Why this assert here and in the _pwritev()?
> 
> We're changing from an interface that didn't have flags to one that does,
> but we are not prepared to handle any flags, so the assert proves the block
> layer doesn't hand us any flags we aren't expecting (there are no block
> layer flags for pread at the moment; and no flags for pwrite because we
> didn't set bs->supported_write_flags).


Thanks.

Reviewed-by: Jeff Cody <jcody@redhat.com>