Am 25.05.2016 um 00:25 hat Eric Blake geschrieben:
> Another step on our continuing quest to switch to byte-based
> interfaces.
>
> As this is the first byte-based iscsi interface, convert
> is_request_lun_aligned() into two versions, one for sectors
> and one for bytes.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> block/iscsi.c | 53 +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 31 insertions(+), 22 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 0acc3dc..3dbfd57 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -401,18 +401,25 @@ static int64_t sector_qemu2lun(int64_t sector, IscsiLun
> *iscsilun)
> return sector * BDRV_SECTOR_SIZE / iscsilun->block_size;
> }
>
> -static bool is_request_lun_aligned(int64_t sector_num, int nb_sectors,
> - IscsiLun *iscsilun)
> +static bool is_byte_request_lun_aligned(int64_t offset, int count,
> + IscsiLun *iscsilun)
> {
> - if ((sector_num * BDRV_SECTOR_SIZE) % iscsilun->block_size ||
> - (nb_sectors * BDRV_SECTOR_SIZE) % iscsilun->block_size) {
> - error_report("iSCSI misaligned request: "
> - "iscsilun->block_size %u, sector_num %" PRIi64
> - ", nb_sectors %d",
> - iscsilun->block_size, sector_num, nb_sectors);
> - return 0;
> + if (offset % iscsilun->block_size || count % iscsilun->block_size) {
> + error_report("iSCSI misaligned request: "
> + "iscsilun->block_size %u, offset %" PRIi64
> + ", count %d",
> + iscsilun->block_size, offset, count);
> + return false;
> }
> - return 1;
> + return true;
> +}
> +
> +static bool is_sector_request_lun_aligned(int64_t sector_num, int nb_sectors,
> + IscsiLun *iscsilun)
> +{
> + return is_byte_request_lun_aligned(sector_num << BDRV_SECTOR_BITS,
> + nb_sectors << BDRV_SECTOR_BITS,
> + iscsilun);
> }
You're switching from (nb_sectors * BDRV_SECTOR_SIZE) to (nb_sectors <<
BDRV_SECTOR_BITS). The difference is that the former is a 64 bit
calculation because BDRV_SECTOR_BITS is unsigned long long, whereas the
latter is a 32 bit calculation.
Fortunately, it seems to me that all input values come directly from the
block layer which already limits requests to BDRV_REQUEST_MAX_SECTORS.
So we should be safe from overflows here.
> static int
> -coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num,
> - int nb_sectors, BdrvRequestFlags flags)
> +coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
> + int count, BdrvRequestFlags flags)
> {
> IscsiLun *iscsilun = bs->opaque;
> struct IscsiTask iTask;
> @@ -978,7 +985,7 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs,
> int64_t sector_num,
> uint32_t nb_blocks;
> bool use_16_for_ws = iscsilun->use_16_for_rw;
>
> - if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
> + if (!is_byte_request_lun_aligned(offset, count, iscsilun)) {
> return -EINVAL;
> }
Should this become -ENOTSUP so that emulation can take over rather than
failing the request?
We should probably also always set bs->bl.pwrite_zeroes_alignment, with
a fallback to iscsilun->block_size if we don't have iscsilun->lbp.lbpws.
But that's a separate patch.
Kevin
