On Thu, 2020-12-10 at 18:36 +0800, Tom Yan wrote:
> On Wed, 9 Dec 2020 at 21:54, Maxim Levitsky <mlevi...@redhat.com> wrote:
> > From: Tom Yan <tom.t...@gmail.com>
> >
> > We can and should get max transfer length and max segments for all host
> > devices / cdroms (on Linux).
> >
> > Also use MIN_NON_ZERO instead when we clamp max transfer length against
> > max segments.
> >
> > Signed-off-by: Tom Yan <tom.t...@gmail.com>
> > Signed-off-by: Maxim Levitsky <mlevi...@redhat.com>
> > ---
> > block/file-posix.c | 59 +++++++++++++++++++++++++++++++++-------------
> > 1 file changed, 43 insertions(+), 16 deletions(-)
> >
> > diff --git a/block/file-posix.c b/block/file-posix.c
> > index d5fd1dbcd2..226ddbbdad 100644
> > --- a/block/file-posix.c
> > +++ b/block/file-posix.c
> > @@ -1162,6 +1162,12 @@ static void raw_reopen_abort(BDRVReopenState *state)
> >
> > static int sg_get_max_transfer_length(int fd)
> > {
> > + /*
> > + * BLKSECTGET for /dev/sg* character devices incorrectly returns
> > + * the max transfer size in bytes (rather than in blocks).
> > + * Also note that /dev/sg* doesn't support BLKSSZGET ioctl.
> > + */
> The second statement should be removed. Also maybe it's better to have
> the first one right above the line `return max_bytes;`.
Done, thanks.
Best regards,
Maxim Levitsky
> > +
> > #ifdef BLKSECTGET
> > int max_bytes = 0;
> >
> > @@ -1175,7 +1181,22 @@ static int sg_get_max_transfer_length(int fd)
> > #endif
> > }
> >
> > -static int sg_get_max_segments(int fd)
> > +static int get_max_transfer_length(int fd)
> > +{
> > +#if defined(BLKSECTGET)
> > + int sect = 0;
> > +
> > + if (ioctl(fd, BLKSECTGET, §) == 0) {
> > + return sect << 9;
> > + } else {
> > + return -errno;
> > + }
> > +#else
> > + return -ENOSYS;
> > +#endif
> > +}
> > +
> > +static int get_max_segments(int fd)
> > {
> > #ifdef CONFIG_LINUX
> > char buf[32];
> > @@ -1230,23 +1251,29 @@ static void raw_refresh_limits(BlockDriverState
> > *bs, Error **errp)
> > {
> > BDRVRawState *s = bs->opaque;
> >
> > - if (bs->sg) {
> > - int ret = sg_get_max_transfer_length(s->fd);
> > + raw_probe_alignment(bs, s->fd, errp);
> > + bs->bl.min_mem_alignment = s->buf_align;
> > + bs->bl.opt_mem_alignment = MAX(s->buf_align, qemu_real_host_page_size);
> > +}
> >
> > - if (ret > 0 && ret <= BDRV_REQUEST_MAX_BYTES) {
> > - bs->bl.max_transfer = pow2floor(ret);
> > - }
> > +static void hdev_refresh_limits(BlockDriverState *bs, Error **errp)
> > +{
> > + BDRVRawState *s = bs->opaque;
> >
> > - ret = sg_get_max_segments(s->fd);
> > - if (ret > 0) {
> > - bs->bl.max_transfer = MIN(bs->bl.max_transfer,
> > - ret * qemu_real_host_page_size);
> > - }
> > + int ret = bs->sg ? sg_get_max_transfer_length(s->fd) :
> > + get_max_transfer_length(s->fd);
> > +
> > + if (ret > 0 && ret <= BDRV_REQUEST_MAX_BYTES) {
> > + bs->bl.max_transfer = pow2floor(ret);
> > }
> >
> > - raw_probe_alignment(bs, s->fd, errp);
> > - bs->bl.min_mem_alignment = s->buf_align;
> > - bs->bl.opt_mem_alignment = MAX(s->buf_align, qemu_real_host_page_size);
> > + ret = get_max_segments(s->fd);
> > + if (ret > 0) {
> > + bs->bl.max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
> > + ret * qemu_real_host_page_size);
> > + }
> > +
> > + raw_refresh_limits(bs, errp);
> > }
> >
> > static int check_for_dasd(int fd)
> > @@ -3601,7 +3628,7 @@ static BlockDriver bdrv_host_device = {
> > .bdrv_co_pdiscard = hdev_co_pdiscard,
> > .bdrv_co_copy_range_from = raw_co_copy_range_from,
> > .bdrv_co_copy_range_to = raw_co_copy_range_to,
> > - .bdrv_refresh_limits = raw_refresh_limits,
> > + .bdrv_refresh_limits = hdev_refresh_limits,
> > .bdrv_io_plug = raw_aio_plug,
> > .bdrv_io_unplug = raw_aio_unplug,
> > .bdrv_attach_aio_context = raw_aio_attach_aio_context,
> > @@ -3725,7 +3752,7 @@ static BlockDriver bdrv_host_cdrom = {
> > .bdrv_co_preadv = raw_co_preadv,
> > .bdrv_co_pwritev = raw_co_pwritev,
> > .bdrv_co_flush_to_disk = raw_co_flush_to_disk,
> > - .bdrv_refresh_limits = raw_refresh_limits,
> > + .bdrv_refresh_limits = hdev_refresh_limits,
> > .bdrv_io_plug = raw_aio_plug,
> > .bdrv_io_unplug = raw_aio_unplug,
> > .bdrv_attach_aio_context = raw_aio_attach_aio_context,
> > --
> > 2.26.2
> >
