Re: [Qemu-devel] [PATCH] scsi-disk: Don't enlarge min_io_size to max_io_size

[Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>]

Hi,

On 03/22/2018 04:38 AM, Fam Zheng wrote:
> Some backends report big max_io_sectors. Making min_io_size the same
> value in this case will make it impossible for guest to align memory,
> therefore the disk may not be usable at all.
>
> Change the default behavior (when min_io_size and opt_io_size are not
> specified in the command line), do not assume max_io_sectors is a good
> value for opt_io_size and min_io_size, use 512 instead.
>
> Reported-by: David Gibson <dgibson@redhat.com>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   hw/scsi/scsi-disk.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 5b7a48f5a5..76e3c9eaa4 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -714,10 +714,8 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
>
>                   /* 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);
> +                min_io_size = MIN(min_io_size ? : 512, max_io_sectors);
> +                opt_io_size = MIN(opt_io_size ? : 512, max_io_sectors);
>               }

This code you're changing was added in d082d16a5c ("consider 
bl->max_transfer ..").
I've borrowed this logic from scsi-generic.c, scsi_read_complete:

     if (s->type == TYPE_DISK &&
         r->req.cmd.buf[0] == INQUIRY &&
         r->req.cmd.buf[2] == 0xb0) {
         uint32_t max_transfer =
             blk_get_max_transfer(s->conf.blk) / s->blocksize;

         assert(max_transfer);
         stl_be_p(&r->buf[8], max_transfer);
         /* Also take care of the opt xfer len. */
         stl_be_p(&r->buf[12],
                  MIN_NON_ZERO(max_transfer, ldl_be_p(&r->buf[12])));
     }


Unless I've misunderstood the bug, you will want to change this code 
too. Otherwise
you'll fix it with emulated disks but it might appear when using SCSI 
passthrough.


Thanks,


Daniel


>               /* required VPD size with unmap support */
>               buflen = 0x40;