| Message ID | 1464243305-10661-2-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 05/26 14:15, Fam Zheng wrote: > This is sometimes a useful value we should count in. Kevin, Max, could you review this please? Fam > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block/raw-posix.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index a4f5a1b..d3796ad 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -729,9 +729,56 @@ static void raw_reopen_abort(BDRVReopenState *state) > state->opaque = NULL; > } > > +static int hdev_get_max_transfer_length(dev_t dev) > +{ > + int ret; > + int fd; > + char *path; > + const char *end; > + char buf[32]; > + long len; > + > + path = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_sectors_kb", > + major(dev), minor(dev)); > + fd = open(path, O_RDONLY); > + if (fd < 0) { > + ret = -errno; > + goto out; > + } > + ret = read(fd, buf, sizeof(buf)); > + if (ret < 0) { > + ret = -errno; > + goto out; > + } else if (ret == 0) { > + ret = -EIO; > + goto out; > + } > + buf[ret] = 0; > + /* The file is ended with '\n', pass 'end' to accept that. */ > + ret = qemu_strtol(buf, &end, 10, &len); > + if (ret == 0 && end && *end == '\n') { > + ret = len * 1024 / BDRV_SECTOR_SIZE; > + } > + > + close(fd); > +out: > + g_free(path); > + return ret; > +} > + > static void raw_refresh_limits(BlockDriverState *bs, Error **errp) > { > BDRVRawState *s = bs->opaque; > + struct stat st; > + > + if (!fstat(s->fd, &st)) { > + if (S_ISBLK(st.st_mode)) { > + int ret = hdev_get_max_transfer_length(st.st_rdev); > + if (ret >= 0) { > + bs->bl.max_transfer_length = ret; > + } > + } > + } > > raw_probe_alignment(bs, s->fd, errp); > bs->bl.min_mem_alignment = s->buf_align; > -- > 2.8.2 > >
On 26.05.2016 08:15, Fam Zheng wrote: > This is sometimes a useful value we should count in. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block/raw-posix.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index a4f5a1b..d3796ad 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -729,9 +729,56 @@ static void raw_reopen_abort(BDRVReopenState *state) > state->opaque = NULL; > } > > +static int hdev_get_max_transfer_length(dev_t dev) > +{ > + int ret; > + int fd; > + char *path; > + const char *end; > + char buf[32]; > + long len; > + > + path = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_sectors_kb", > + major(dev), minor(dev)); I can't say I like this very much, but well, it won't do any harm on any systems that do not offer this path (i.e. any non-Linux system, I suppose). So I'm fine with it. > + fd = open(path, O_RDONLY); > + if (fd < 0) { > + ret = -errno; > + goto out; > + } > + ret = read(fd, buf, sizeof(buf)); > + if (ret < 0) { > + ret = -errno; > + goto out; > + } else if (ret == 0) { > + ret = -EIO; > + goto out; > + } > + buf[ret] = 0; Potential buffer overflow if ret == sizeof(buf). > + /* The file is ended with '\n', pass 'end' to accept that. */ > + ret = qemu_strtol(buf, &end, 10, &len); > + if (ret == 0 && end && *end == '\n') { > + ret = len * 1024 / BDRV_SECTOR_SIZE; Maybe there should be an overflow check here. > + } > + > + close(fd); This belongs in some error path, because otherwise the FD is leaked if the read() failed. Max > +out: > + g_free(path); > + return ret; > +} > + > static void raw_refresh_limits(BlockDriverState *bs, Error **errp) > { > BDRVRawState *s = bs->opaque; > + struct stat st; > + > + if (!fstat(s->fd, &st)) { > + if (S_ISBLK(st.st_mode)) { > + int ret = hdev_get_max_transfer_length(st.st_rdev); > + if (ret >= 0) { > + bs->bl.max_transfer_length = ret; > + } > + } > + } > > raw_probe_alignment(bs, s->fd, errp); > bs->bl.min_mem_alignment = s->buf_align; >
Am 02.06.2016 um 14:30 hat Max Reitz geschrieben: > On 26.05.2016 08:15, Fam Zheng wrote: > > This is sometimes a useful value we should count in. > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > block/raw-posix.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 47 insertions(+) > > > > diff --git a/block/raw-posix.c b/block/raw-posix.c > > index a4f5a1b..d3796ad 100644 > > --- a/block/raw-posix.c > > +++ b/block/raw-posix.c > > @@ -729,9 +729,56 @@ static void raw_reopen_abort(BDRVReopenState *state) > > state->opaque = NULL; > > } > > > > +static int hdev_get_max_transfer_length(dev_t dev) > > +{ > > + int ret; > > + int fd; > > + char *path; > > + const char *end; > > + char buf[32]; > > + long len; > > + > > + path = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_sectors_kb", > > + major(dev), minor(dev)); > > I can't say I like this very much, but well, it won't do any harm on any > systems that do not offer this path (i.e. any non-Linux system, I > suppose). So I'm fine with it. Haven't looked at the patch in detail yet, so I didn't want to send a comment yet, but I think this should be #ifdef-ed out for non-Linux. Also a quick search on the internet suggests that the BLKSECTGET ioctl is what we're looking for, so hopefully using sysfs is unnecessary anyway. Kevin
On Thu, 06/02 14:54, Kevin Wolf wrote: > Also a quick search on the internet suggests that the BLKSECTGET ioctl > is what we're looking for, so hopefully using sysfs is unnecessary > anyway. Oops! Looks like something went terribly wrong with my "quick search", will post v2. Thanks! Fam
diff --git a/block/raw-posix.c b/block/raw-posix.c index a4f5a1b..d3796ad 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -729,9 +729,56 @@ static void raw_reopen_abort(BDRVReopenState *state) state->opaque = NULL; } +static int hdev_get_max_transfer_length(dev_t dev) +{ + int ret; + int fd; + char *path; + const char *end; + char buf[32]; + long len; + + path = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_sectors_kb", + major(dev), minor(dev)); + fd = open(path, O_RDONLY); + if (fd < 0) { + ret = -errno; + goto out; + } + ret = read(fd, buf, sizeof(buf)); + if (ret < 0) { + ret = -errno; + goto out; + } else if (ret == 0) { + ret = -EIO; + goto out; + } + buf[ret] = 0; + /* The file is ended with '\n', pass 'end' to accept that. */ + ret = qemu_strtol(buf, &end, 10, &len); + if (ret == 0 && end && *end == '\n') { + ret = len * 1024 / BDRV_SECTOR_SIZE; + } + + close(fd); +out: + g_free(path); + return ret; +} + static void raw_refresh_limits(BlockDriverState *bs, Error **errp) { BDRVRawState *s = bs->opaque; + struct stat st; + + if (!fstat(s->fd, &st)) { + if (S_ISBLK(st.st_mode)) { + int ret = hdev_get_max_transfer_length(st.st_rdev); + if (ret >= 0) { + bs->bl.max_transfer_length = ret; + } + } + } raw_probe_alignment(bs, s->fd, errp); bs->bl.min_mem_alignment = s->buf_align;
This is sometimes a useful value we should count in. Signed-off-by: Fam Zheng <famz@redhat.com> --- block/raw-posix.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)