Re: [PATCH 0/4] IMSM: Add support for 4Kn sector size drives

Pawel Baldysiak [off-list ref] writes:

This patch set adds support for IMSM with 4Kn sector size drives
First patch adds the generic function for receiving sector size,
rest are IMSM specific.
Internal calculation are still based on 512-bytes sector,
variables are converted during read/write from/to member drive.
Mixing of devices with different sector size is not allowed.

Pawel Baldysiak (4):
  Add function for getting member drive sector size
  IMSM: Read and store device sector size
  IMSM: Add support for 4Kn sector size drives
  IMSM: 4Kn drives support - adapt general migration record

 mdadm.h       |   1 +
 super-intel.c | 315 +++++++++++++++++++++++++++++++++++++++++++++-------------
 super1.c      |   3 +-
 util.c        |  16 +++
 4 files changed, 265 insertions(+), 70 deletions(-)

Hi Pawel,

This set mostly looks good - a couple of comments:

+int get_dev_sector_size(int fd, char *dname, unsigned int *sectsizep)

This introduces a *dname but nowhere in your code is it actually used. I
am not necessarily against this, and it looks like we do it in some
places, but not others. However do you anticipate using it in future
changes you have lined up?

I noticed you changed hard coded 512 byte limits to hard coded 4096
when rounding up sizes for posix_memalign() etc. Wouldn't it be cleaner
to introduce a MAX_SECTOR_SIZE or similar?

Cheers,
Jes