Re: [Qemu-devel] [PATCH v5 2/5] raw-posix: Refactor logical block size detection.

[Ekaterina Tumanova]

On 01/02/2015 02:52 PM, Stefan Hajnoczi wrote:
> On Thu, Dec 18, 2014 at 12:18:01PM +0100, Ekaterina Tumanova wrote:
> > +#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
>
> Not a reason to respin, but I would have preferred simply moving the old
> code.
>
> I think the new code works because BLKSSZGET is Linux, DKIOCGETBLOCKSIZE
> is Mac OS, and DIOCGSECTORSIZE is FreeBSD.
>
> If there is a host OS where more than one ioctl is available and the
> first one fails then the new code is broken.  The old code didn't use
> #elif so each ioctl had a chance to run.

In this case, why should it have a chance to run, if we only use one
result at a time? (Old code overwrites first result with the second)

Plus as far as I understand, in this hypothetical case of 2 ioctls
defined, one will most probably will be a redefinition of another.

p.s. ok to all further comments to patches.

Thanks!
Kate.


> Also, the name SECTOR_SIZE is misleading.  It's not a sector size value
> but the "get sector size" ioctl request code.
>
> Stefan