Re: [Qemu-devel] [PATCH 1/1] scsi-disk.c: consider bl->max_transfer in INQUIRY emulation

[Fam Zheng]

On Tue, 03/06 12:44, Daniel Henrique Barboza wrote:
> The calculation of the max_transfer atribute of BlockDriverState
> makes considerations such as max_segments and transfer_length via
> the BLKSECTGET ioctl (if available).
> 
> However, bl->max_transfer isn't considered when emulating the INQUIRY
> 'Block Limit' response to the scsi-hd devices. This leads to situations
> where the declared max_sectors from the INQUIRY response is inconsistent
> with the block limits, which isn't ideal. It can also be misleading to the
> user that sets /sys/block//queue/max_sectors_kb to a certain
> value, then finds a different value in the guest OS for the same disk.
> 
> Following the same logic scsi_read_complete from scsi-generic.c does
> when patching the response of the Block Limits VPD back to the guest,
> change the max_io_sectors value of the emulated Block Limits VPD
> response by considering the blk_get_max_transfer of the related
> BlockDriverState. Use MIN_NOT_ZERO to be sure that the minimal
> value is chosen.
> 
> Given that we're changing max_io_sectors, consider that min_io_sectors
> and opt_io_sectors can't be greater than the new calculated value.
> 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  hw/scsi/scsi-disk.c | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 49d2559d93..c65c1ce56d 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -704,6 +704,21 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, 
> uint8_t *outbuf)
>  page_code);
>  return -1;
>  }
> +if (s->qdev.type == TYPE_DISK) {
> +int max_transfer_blk = 
> blk_get_max_transfer(s->qdev.conf.blk);
> +int max_io_sectors_blk =
> +max_transfer_blk / s->qdev.blocksize;
> +
> +max_io_sectors =
> +MIN_NON_ZERO(max_io_sectors_blk, max_io_sectors);
> +
> +/* min_io_size and opt_io_size can't be greater than
> + * max_io_sectors */
> +min_io_size =
> +MIN_NON_ZERO(min_io_size, max_io_sectors);
> +opt_io_size =
> +MIN_NON_ZERO(opt_io_size, max_io_sectors);
> +}
>  /* required VPD size with unmap support */
>  buflen = 0x40;
>  memset(outbuf + 4, 0, buflen - 4);
> -- 
> 2.14.3
> 

Reviewed-by: Fam Zheng 

[Qemu-devel] [PATCH 1/1] scsi-disk.c: consider bl->max_transfer in INQUIRY emulation

[Daniel Henrique Barboza]

The calculation of the max_transfer atribute of BlockDriverState
makes considerations such as max_segments and transfer_length via
the BLKSECTGET ioctl (if available).

However, bl->max_transfer isn't considered when emulating the INQUIRY
'Block Limit' response to the scsi-hd devices. This leads to situations
where the declared max_sectors from the INQUIRY response is inconsistent
with the block limits, which isn't ideal. It can also be misleading to the
user that sets /sys/block//queue/max_sectors_kb to a certain
value, then finds a different value in the guest OS for the same disk.

Following the same logic scsi_read_complete from scsi-generic.c does
when patching the response of the Block Limits VPD back to the guest,
change the max_io_sectors value of the emulated Block Limits VPD
response by considering the blk_get_max_transfer of the related
BlockDriverState. Use MIN_NOT_ZERO to be sure that the minimal
value is chosen.

Given that we're changing max_io_sectors, consider that min_io_sectors
and opt_io_sectors can't be greater than the new calculated value.

Signed-off-by: Daniel Henrique Barboza 
---
 hw/scsi/scsi-disk.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 49d2559d93..c65c1ce56d 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -704,6 +704,21 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, 
uint8_t *outbuf)
 page_code);
 return -1;
 }
+if (s->qdev.type == TYPE_DISK) {
+int max_transfer_blk = blk_get_max_transfer(s->qdev.conf.blk);
+int max_io_sectors_blk =
+max_transfer_blk / s->qdev.blocksize;
+
+max_io_sectors =
+MIN_NON_ZERO(max_io_sectors_blk, max_io_sectors);
+
+/* min_io_size and opt_io_size can't be greater than
+ * max_io_sectors */
+min_io_size =
+MIN_NON_ZERO(min_io_size, max_io_sectors);
+opt_io_size =
+MIN_NON_ZERO(opt_io_size, max_io_sectors);
+}
 /* required VPD size with unmap support */
 buflen = 0x40;
 memset(outbuf + 4, 0, buflen - 4);
-- 
2.14.3