On 08/31/2017 06:05 AM, Daniel P. Berrange wrote: > Make the crypto driver implement the bdrv_co_preadv|pwritev > callbacks, and also use bdrv_co_preadv|pwritev for I/O > with the protocol driver beneath. > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > block/crypto.c | 103 > +++++++++++++++++++++++++++++---------------------------- > 1 file changed, 53 insertions(+), 50 deletions(-) >
> static coroutine_fn int > -block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num, > - int remaining_sectors, QEMUIOVector *qiov) > +block_crypto_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes, > + QEMUIOVector *qiov, int flags) > { > BlockCrypto *crypto = bs->opaque; > - int cur_nr_sectors; /* number of sectors in current iteration */ > + uint64_t cur_bytes; /* number of bytes in current iteration */ > uint64_t bytes_done = 0; > uint8_t *cipher_data = NULL; > QEMUIOVector hd_qiov; > int ret = 0; > - size_t payload_offset = > - qcrypto_block_get_payload_offset(crypto->block) / BDRV_SECTOR_SIZE; > + size_t payload_offset = qcrypto_block_get_payload_offset(crypto->block); Pre-existing: is size_t the right type, or can we overflow a 64-bit offset on a 32-bit host? > + uint64_t sector_num = offset / BDRV_SECTOR_SIZE; > + > + assert((offset % BDRV_SECTOR_SIZE) == 0); > + assert((bytes % BDRV_SECTOR_SIZE) == 0); The osdep.h macros might be nicer than open-coding; furthermore, if desired, you could shorten to: assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE)); > > static coroutine_fn int > -block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num, > - int remaining_sectors, QEMUIOVector *qiov) > +block_crypto_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t > bytes, > + QEMUIOVector *qiov, int flags) > { Hmm - you don't set supported_write_flags. But presumably, if the underlying BDS supports BDRV_REQUEST_FUA, then crypto can likewise support that flag by passing it through to the underlying device after encryption. > BlockCrypto *crypto = bs->opaque; > - int cur_nr_sectors; /* number of sectors in current iteration */ > + uint64_t cur_bytes; /* number of bytes in current iteration */ > uint64_t bytes_done = 0; > uint8_t *cipher_data = NULL; > QEMUIOVector hd_qiov; > int ret = 0; > - size_t payload_offset = > - qcrypto_block_get_payload_offset(crypto->block) / BDRV_SECTOR_SIZE; > + size_t payload_offset = qcrypto_block_get_payload_offset(crypto->block); > + uint64_t sector_num = offset / BDRV_SECTOR_SIZE; > + > + assert((offset % BDRV_SECTOR_SIZE) == 0); > + assert((bytes % BDRV_SECTOR_SIZE) == 0); Same comment as for read. > @@ -611,8 +613,9 @@ BlockDriver bdrv_crypto_luks = { > .bdrv_truncate = block_crypto_truncate, > .create_opts = &block_crypto_create_opts_luks, > > - .bdrv_co_readv = block_crypto_co_readv, > - .bdrv_co_writev = block_crypto_co_writev, > + .bdrv_refresh_limits = block_crypto_refresh_limits, > + .bdrv_co_preadv = block_crypto_co_preadv, > + .bdrv_co_pwritev = block_crypto_co_pwritev, > .bdrv_getlength = block_crypto_getlength, > .bdrv_get_info = block_crypto_get_info_luks, > .bdrv_get_specific_info = block_crypto_get_specific_info_luks, Looks weird when = isn't consistently aligned, but we use more than one space. My preference is to just use one space everywhere, as adding a longer name to the list doesn't require a mass re-format of all other entries; but I'm not opposed when people like the aligned = for legibility. So up to you if you do anything in response to my nit. Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature