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

[Daniel Henrique Barboza]

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 <address@hidden>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>   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;