The io_timeout property, introduced in c9b6609 (part of 6.0) is
silently overwritten by the hardcoded default value of 30 seconds
(DEFAULT_IO_TIMEOUT) in scsi_generic_realize because that function is
being called after the properties have already been applied.

The property definition already has a default value which is applied
correctly when no value is explicitly set, so we can just remove the
code which overrides the io_timeout completely.

This has been tested by stracing SG_IO operations with the io_timeout
property set and unset and now sets the timeout field in the ioctl
request to the proper value.

Fixes: c9b6609b69facad ("scsi: make io_timeout configurable")
Signed-off-by: Lorenz Brun <lorenz@brun.one>
---
 hw/scsi/scsi-generic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index b7b04e1d63..ee945f87e3 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -752,7 +752,6 @@ static void scsi_generic_realize(SCSIDevice *s, Error **errp)
 
     /* Only used by scsi-block, but initialize it nevertheless to be clean.  */
     s->default_scsi_version = -1;
-    s->io_timeout = DEFAULT_IO_TIMEOUT;
     scsi_generic_read_device_inquiry(s);
 }
 
-- 
2.42.0

On 15/3/24 15:58, Lorenz Brun wrote:
> The io_timeout property, introduced in c9b6609 (part of 6.0) is
> silently overwritten by the hardcoded default value of 30 seconds
> (DEFAULT_IO_TIMEOUT) in scsi_generic_realize because that function is
> being called after the properties have already been applied.
> 
> The property definition already has a default value which is applied
> correctly when no value is explicitly set, so we can just remove the
> code which overrides the io_timeout completely.
> 
> This has been tested by stracing SG_IO operations with the io_timeout
> property set and unset and now sets the timeout field in the ioctl
> request to the proper value.
> 
> Fixes: c9b6609b69facad ("scsi: make io_timeout configurable")
> Signed-off-by: Lorenz Brun <lorenz@brun.one>
> ---
>   hw/scsi/scsi-generic.c | 1 -
>   1 file changed, 1 deletion(-)

Thanks, patch queued.

Lorenz Brun <lorenz@brun.one> writes:

> The io_timeout property, introduced in c9b6609 (part of 6.0) is
> silently overwritten by the hardcoded default value of 30 seconds
> (DEFAULT_IO_TIMEOUT) in scsi_generic_realize because that function is
> being called after the properties have already been applied.
>
> The property definition already has a default value which is applied
> correctly when no value is explicitly set, so we can just remove the
> code which overrides the io_timeout completely.
>
> This has been tested by stracing SG_IO operations with the io_timeout
> property set and unset and now sets the timeout field in the ioctl
> request to the proper value.
>
> Fixes: c9b6609b69facad ("scsi: make io_timeout configurable")
> Signed-off-by: Lorenz Brun <lorenz@brun.one>
> ---
>  hw/scsi/scsi-generic.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
> index b7b04e1d63..ee945f87e3 100644
> --- a/hw/scsi/scsi-generic.c
> +++ b/hw/scsi/scsi-generic.c
> @@ -752,7 +752,6 @@ static void scsi_generic_realize(SCSIDevice *s, Error **errp)
>  
>      /* Only used by scsi-block, but initialize it nevertheless to be clean.  */
>      s->default_scsi_version = -1;
> -    s->io_timeout = DEFAULT_IO_TIMEOUT;
>      scsi_generic_read_device_inquiry(s);
>  }

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro