Re: [PATCH 2/3] scsi: make io_timeout configurable

[Philippe Mathieu-Daudé]

Hi Hannes,

On 11/16/20 19:31, Hannes Reinecke wrote:
> The current code sets an infinite timeout on SG_IO requests,
> causing the guest to stall if the host experiences a frame
> loss.
> This patch adds an 'io_timeout' parameter for SCSIDevice to
> make the SG_IO timeout configurable, and also shortens the
> default timeout to 30 seconds to avoid infinite stalls.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   hw/scsi/scsi-disk.c    |  6 ++++--
>   hw/scsi/scsi-generic.c | 17 +++++++++++------
>   include/hw/scsi/scsi.h |  4 +++-
>   3 files changed, 18 insertions(+), 9 deletions(-)

>   int scsi_SG_IO_FROM_DEV(BlockBackend *blk, uint8_t *cmd, uint8_t cmd_size,
> -                        uint8_t *buf, uint8_t buf_size)
> +                        uint8_t *buf, uint8_t buf_size, uint32_t timeout)
>   {
>       sg_io_hdr_t io_header;
>       uint8_t sensebuf[8];
> @@ -520,7 +522,7 @@ int scsi_SG_IO_FROM_DEV(BlockBackend *blk, uint8_t *cmd, 
> uint8_t cmd_size,
>       io_header.cmd_len = cmd_size;
>       io_header.mx_sb_len = sizeof(sensebuf);
>       io_header.sbp = sensebuf;
> -    io_header.timeout = 6000; /* XXX */
> +    io_header.timeout = timeout * 1000;

> @@ -637,7 +639,7 @@ static int get_stream_blocksize(BlockBackend *blk)
>       cmd[0] = MODE_SENSE;
>       cmd[4] = sizeof(buf);
>   
> -    ret = scsi_SG_IO_FROM_DEV(blk, cmd, sizeof(cmd), buf, sizeof(buf));
> +    ret = scsi_SG_IO_FROM_DEV(blk, cmd, sizeof(cmd), buf, sizeof(buf), 6);

Why is this timeout hardcoded? Due to the /* XXX */ comment?