| Message ID | 1418728219-40595-3-git-send-email-tumanova@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> writes: > Put it in new probe_logical_blocksize(). > > Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> > --- > block/raw-posix.c | 41 ++++++++++++++++++++++++----------------- > 1 file changed, 24 insertions(+), 17 deletions(-) > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index e51293a..38172ca 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -217,11 +217,31 @@ static int raw_normalize_devicepath(const char **filename) > } > #endif > > +/* > + * Get logical block size via ioctl. On success return 0. Otherwise -errno. > + */ > +static int probe_logical_blocksize(int fd, unsigned int *sector_size) > +{ > +#if defined(BLKSSZGET) > +# define SECTOR_SIZE BLKSSZGET > +#elif defined(DKIOCGETBLOCKSIZE) > +# define SECTOR_SIZE DKIOCGETBLOCKSIZE > +#elif defined(DIOCGSECTORSIZE) > +# define SECTOR_SIZE DIOCGSECTORSIZE > +#else > + return -ENOTSUP > +#endif > + if (ioctl(fd, SECTOR_SIZE, sector_size) < 0) { > + return -errno; > + } > + return 0; > +#undef SECTOR_SIZE > +} > + > static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp) > { > BDRVRawState *s = bs->opaque; > char *buf; > - unsigned int sector_size; > > /* For /dev/sg devices the alignment is not really used. > With buffered I/O, we don't have any restrictions. */ > @@ -231,25 +251,12 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp) > return; > } > > - /* Try a few ioctls to get the right size */ > bs->request_alignment = 0; > s->buf_align = 0; > - > -#ifdef BLKSSZGET > - if (ioctl(fd, BLKSSZGET, §or_size) >= 0) { > - bs->request_alignment = sector_size; > - } > -#endif > -#ifdef DKIOCGETBLOCKSIZE > - if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) { > - bs->request_alignment = sector_size; > + /* Let's try to use the logical blocksize for the alignment. */ > + if (probe_logical_blocksize(fd, &bs->request_alignment) < 0) { > + bs->request_alignment = 0; > } > -#endif > -#ifdef DIOCGSECTORSIZE > - if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) { > - bs->request_alignment = sector_size; > - } > -#endif > #ifdef CONFIG_XFS > if (s->is_xfs) { > struct dioattr da; New in v4: this isn't just factoring out code, you also change it! Before: try BLKSSZGET (if defined), DKIOCGETBLOCKSIZE (if defined), DIOCGSECTORSIZE (if defined) in order until. The last one to succeed wins. If none succeeds, assume 0. After: use BLKSSZGET if defined, else try DKIOCGETBLOCKSIZE if defined, else try DIOCGSECTORSIZE if defined, else assume 0. Your change may well be an improvement. But your commit message should explain *why* it's an improvement. I guess I'd do a separate patch for that, but since the code patch seems obvious enough as it is, you may just improve its commit message and call it a day.
diff --git a/block/raw-posix.c b/block/raw-posix.c index e51293a..38172ca 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -217,11 +217,31 @@ static int raw_normalize_devicepath(const char **filename) } #endif +/* + * Get logical block size via ioctl. On success return 0. Otherwise -errno. + */ +static int probe_logical_blocksize(int fd, unsigned int *sector_size) +{ +#if defined(BLKSSZGET) +# define SECTOR_SIZE BLKSSZGET +#elif defined(DKIOCGETBLOCKSIZE) +# define SECTOR_SIZE DKIOCGETBLOCKSIZE +#elif defined(DIOCGSECTORSIZE) +# define SECTOR_SIZE DIOCGSECTORSIZE +#else + return -ENOTSUP +#endif + if (ioctl(fd, SECTOR_SIZE, sector_size) < 0) { + return -errno; + } + return 0; +#undef SECTOR_SIZE +} + static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp) { BDRVRawState *s = bs->opaque; char *buf; - unsigned int sector_size; /* For /dev/sg devices the alignment is not really used. With buffered I/O, we don't have any restrictions. */ @@ -231,25 +251,12 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp) return; } - /* Try a few ioctls to get the right size */ bs->request_alignment = 0; s->buf_align = 0; - -#ifdef BLKSSZGET - if (ioctl(fd, BLKSSZGET, §or_size) >= 0) { - bs->request_alignment = sector_size; - } -#endif -#ifdef DKIOCGETBLOCKSIZE - if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) { - bs->request_alignment = sector_size; + /* Let's try to use the logical blocksize for the alignment. */ + if (probe_logical_blocksize(fd, &bs->request_alignment) < 0) { + bs->request_alignment = 0; } -#endif -#ifdef DIOCGSECTORSIZE - if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) { - bs->request_alignment = sector_size; - } -#endif #ifdef CONFIG_XFS if (s->is_xfs) { struct dioattr da;
Put it in new probe_logical_blocksize(). Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> --- block/raw-posix.c | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-)