[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH v2 5/8] qcow: Switch to a byte-base
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH v2 5/8] qcow: Switch to a byte-based driver |
Date: |
Tue, 5 Jun 2018 09:36:04 -0400 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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 <address@hidden>
>
> >>-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 <address@hidden>