this patch changes the driver to uses 16 Byte CDBs for READ/WRITE only if the target requires 64bit lba addressing.
On one hand this saves 6 bytes in each PDU on the other hand it seems that 10 Byte CDBs seems to be much better supported and tested as a recent issue I had with a major storage supplier lined out. For WRITESAME the logic is a bit more tricky as WRITESAME10 with UNMAP was added really late. Thus a fallback to WRITESAME16 is possible if it supports UNMAP and WRITESAME10 not. Signed-off-by: Peter Lieven <p...@kamp.de> --- block/iscsi.c | 58 +++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 18 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index d241e83..019b324 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -65,6 +65,7 @@ typedef struct IscsiLun { unsigned char *zeroblock; unsigned long *allocationmap; int cluster_sectors; + bool use_16_for_rw; } IscsiLun; typedef struct IscsiTask { @@ -368,10 +369,17 @@ static int coroutine_fn iscsi_co_writev(BlockDriverState *bs, num_sectors = sector_qemu2lun(nb_sectors, iscsilun); iscsi_co_init_iscsitask(iscsilun, &iTask); retry: - iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba, - data, num_sectors * iscsilun->block_size, - iscsilun->block_size, 0, 0, 0, 0, 0, - iscsi_co_generic_cb, &iTask); + if (iscsilun->use_16_for_rw) { + iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba, + data, num_sectors * iscsilun->block_size, + iscsilun->block_size, 0, 0, 0, 0, 0, + iscsi_co_generic_cb, &iTask); + } else { + iTask.task = iscsi_write10_task(iscsilun->iscsi, iscsilun->lun, lba, + data, num_sectors * iscsilun->block_size, + iscsilun->block_size, 0, 0, 0, 0, 0, + iscsi_co_generic_cb, &iTask); + } if (iTask.task == NULL) { g_free(buf); return -ENOMEM; @@ -545,20 +553,17 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, iscsi_co_init_iscsitask(iscsilun, &iTask); retry: - switch (iscsilun->type) { - case TYPE_DISK: + if (iscsilun->use_16_for_rw) { iTask.task = iscsi_read16_task(iscsilun->iscsi, iscsilun->lun, lba, num_sectors * iscsilun->block_size, iscsilun->block_size, 0, 0, 0, 0, 0, iscsi_co_generic_cb, &iTask); - break; - default: + } else { iTask.task = iscsi_read10_task(iscsilun->iscsi, iscsilun->lun, lba, num_sectors * iscsilun->block_size, iscsilun->block_size, 0, 0, 0, 0, 0, iscsi_co_generic_cb, &iTask); - break; } if (iTask.task == NULL) { return -ENOMEM; @@ -864,19 +869,27 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, struct IscsiTask iTask; uint64_t lba; uint32_t nb_blocks; + bool use_16_for_ws = iscsilun->use_16_for_rw; if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { return -EINVAL; } - if ((flags & BDRV_REQ_MAY_UNMAP) && !iscsilun->lbp.lbpws) { - /* WRITE SAME with UNMAP is not supported by the target, - * fall back and try WRITE SAME without UNMAP */ - flags &= ~BDRV_REQ_MAY_UNMAP; + if (flags & BDRV_REQ_MAY_UNMAP) { + if (!use_16_for_ws && !iscsilun->lbp.lbpws10) { + /* WRITESAME10 with UNMAP is unsupported try WRITESAME16 */ + use_16_for_ws = true; + } + if (use_16_for_ws && !iscsilun->lbp.lbpws) { + /* WRITESAME16 with UNMAP is not supported by the target, + * fall back and try WRITESAME10/16 without UNMAP */ + flags &= ~BDRV_REQ_MAY_UNMAP; + use_16_for_ws = iscsilun->use_16_for_rw; + } } if (!(flags & BDRV_REQ_MAY_UNMAP) && !iscsilun->has_write_same) { - /* WRITE SAME without UNMAP is not supported by the target */ + /* WRITESAME without UNMAP is not supported by the target */ return -ENOTSUP; } @@ -889,10 +902,18 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, iscsi_co_init_iscsitask(iscsilun, &iTask); retry: - if (iscsi_writesame16_task(iscsilun->iscsi, iscsilun->lun, lba, - iscsilun->zeroblock, iscsilun->block_size, - nb_blocks, 0, !!(flags & BDRV_REQ_MAY_UNMAP), - 0, 0, iscsi_co_generic_cb, &iTask) == NULL) { + if (use_16_for_ws) { + iTask.task = iscsi_writesame16_task(iscsilun->iscsi, iscsilun->lun, lba, + iscsilun->zeroblock, iscsilun->block_size, + nb_blocks, 0, !!(flags & BDRV_REQ_MAY_UNMAP), + 0, 0, iscsi_co_generic_cb, &iTask); + } else { + iTask.task = iscsi_writesame10_task(iscsilun->iscsi, iscsilun->lun, lba, + iscsilun->zeroblock, iscsilun->block_size, + nb_blocks, 0, !!(flags & BDRV_REQ_MAY_UNMAP), + 0, 0, iscsi_co_generic_cb, &iTask); + } + if (iTask.task == NULL) { return -ENOMEM; } @@ -1087,6 +1108,7 @@ static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp) iscsilun->num_blocks = rc16->returned_lba + 1; iscsilun->lbpme = rc16->lbpme; iscsilun->lbprz = rc16->lbprz; + iscsilun->use_16_for_rw = (rc16->returned_lba > 0xffffffff); } } break; -- 1.7.9.5