On Tue, 2021-06-08 at 22:14 +0300, Vladimir Sementsov-Ogievskiy wrote:
> 08.06.2021 16:16, Paolo Bonzini wrote:
> > Even though it was only called for devices that have bs->sg set (which
> > must be character devices), sg_get_max_segments looked at /sys/dev/block
> > which only works for block devices.
> >
> > On Linux the sg driver has its own way to provide the maximum number of
> > iovecs in a scatter/gather list, so add support for it. The block device
> > path is kept because it will be reinstated in the next patches.
> >
> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> > ---
> > block/file-posix.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/block/file-posix.c b/block/file-posix.c
> > index f37dfc10b3..536998a1d6 100644
> > --- a/block/file-posix.c
> > +++ b/block/file-posix.c
> > @@ -1180,6 +1180,17 @@ static int sg_get_max_segments(int fd)
> > goto out;
> > }
> >
> > + if (S_ISCHR(st.st_mode)) {
>
> Why not check "if (bs->sg) {" instead? It seems to be more consistent with
> issuing SG_ ioctl. Or what I miss?
I also think so. Actually the 'hdev_is_sg' has a check for character device as
well,
in addition to a few more checks that make sure that we are really
dealing with the quirky /dev/sg character device.
>
> > + if (ioctl(fd, SG_GET_SG_TABLESIZE, &ret) == 0) {
> > + return ret;
> > + }
> > + return -ENOTSUP;
> > + }
> > +
> > + if (!S_ISBLK(st.st_mode)) {
> > + return -ENOTSUP;
> > + }
> > +
> > sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_segments",
> > major(st.st_rdev), minor(st.st_rdev));
> > sysfd = open(sysfspath, O_RDONLY);
> >
>
>
Other than that, this is the same as the patch from Tom Yan:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg768262.html
In this version he does check if the SG_GET_SG_TABLESIZE is defined, so
you might want to do this as well.
Best regards,
Maxim Levitsky
