[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] scsi-disk: Don't enlarge min_io_size to max_io_
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH] scsi-disk: Don't enlarge min_io_size to max_io_size |
Date: |
Mon, 26 Mar 2018 15:26:39 +0800 |
User-agent: |
Mutt/1.9.2 (2017-12-15) |
On Thu, 03/22 09:19, Daniel Henrique Barboza wrote:
> 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.
I am assuming (because I don't have a reproducer myself) what matters is
min_io_size here.
David, could you help test if you see the same problem with "-device
scsi-block"? If we I'll patch scsi-generic.c in v2 too.
Fam