Skip to contentSkip to conversation

scsi-disk.c: consider bl->max_transfer in INQUIRY emulation

PATCH In review Daniel Henrique Barboza 2 participants 2 messages Mar 6, 2018 – Mar 7, 2018

lore
Share & tools
lore
#0 lore raw

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

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/<dev>/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.

---
 hw/scsi/scsi-disk.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
hw/scsi/scsi-disk.c +15 -0
index 49d2559d93..c65c1ce56d 100644
@@ -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);
Signature · 1 line
2.14.3
Reviewed-by on v1 1/1
#1 lore raw
Quoted 55 lines · 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/<dev>/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.

---
 hw/scsi/scsi-disk.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
hw/scsi/scsi-disk.c +15 -0
index 49d2559d93..c65c1ce56d 100644
@@ -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);
Signature · 1 line
2.14.3