Extend stripeoffset and stripesize of GEOMs from u_int to off_t

[eugen_grosbein.net]

Summary

GEOM's stripeoffset overflows at 4 gigabyte margin (2^32) because of its u_int type. This leads to incorrect data in the output generated by "sysctl kern.geom.confxml" command, "graid list" etc. when GEOM array has volumes larger than 4G.

Test Plan

Create an array with first RAID1 volume /dev/raid/r0 spanning first 4097 megabytes of ada0 and ada1 disks:

$ graid label -S 4097M Promise r0 RAID1 ada0 ada1

Then, add second SINGLE volume /dev/raid/r1 to the same array utilizing the rest of ada0:

$ graid label Promise r1 SINGLE ada0
$ graid list | grep -A4 raid/r1
2. Name: raid/r1
   Mediasize: 6374293504 (5.9G)
   Sectorsize: 512
   Stripesize: 0
   Stripeoffset: 1048576

It shows stripeoffset as 1M instead of 4097M due to overflow in the GEOM despite of right 64 bit offsets within GEOM_RAID itself.

[mav]

As I have written in private: I see no big problem here, while applying this breaks an ABIs (BTW there should be bumped disk ABI version). But other thoughts are welcome.

[imp]

sys/geom/geom_disk.h

100 ↗

(On Diff #36356)

this breaks binary compat, so we need
#define DISK_VERSION_06 0x5856105f
#define DISK_VERSION DISK_VERSION_06
to make sure the breakage is enforced.

sys/geom/raid/g_raid.c

2426 ↗

(On Diff #36356)

surprised this didn't whine before.

[kib]

Does this change break ABI or KBI ? Or both ?

Some providers take stripesize/offset in create, e.g. nop. Do they need a fix included into the change ?

sys/geom/geom.h

204 ↗

(On Diff #36356)

I do not like the gap between sectorsize and stripesize.

[eugen_grosbein.net]

DISK_VERSION bumped.

[eugen_grosbein.net]

sys/geom/geom_disk.h

100 ↗

(On Diff #36356)

Thanks! Done.

[eugen_grosbein.net]

In D13426#280534, @kib wrote:

Does this change break ABI or KBI ? Or both ?

Some providers take stripesize/offset in create, e.g. nop. Do they need a fix included into the change ?

nop and others not mentioned in the diff use "off_t" internaly already.

[eugen_grosbein.net]

sys/geom/geom.h

204 ↗

(On Diff #36356)

Do you mean that compiler will use 8-byte alignment ffo "stripesize", so "sectorsize" should be moved nead the end of the struct?

[eugen_grosbein.net]

In D13426#280534, @kib wrote:

Does this change break ABI or KBI ? Or both ?

It seems, yes. No MFC planned.

[kib]

In D13426#280601, @eugen_grosbein.net wrote:

In D13426#280534, @kib wrote:

Does this change break ABI or KBI ? Or both ?

It seems, yes. No MFC planned.

The 'yes' answer to tri-state question is not informative.

If there is ABI breakage, its impact must be evaluated. It could be that this change is not acceptable if affecting symver-ed interfaces.

[kib]

In D13426#280595, @eugen_grosbein.net wrote:

nop and others not mentioned in the diff use "off_t" internaly already.

Look at the argument types for g_nop_create() as an example.

[imp]

In D13426#280610, @kib wrote:

In D13426#280601, @eugen_grosbein.net wrote:

In D13426#280534, @kib wrote:

Does this change break ABI or KBI ? Or both ?

It seems, yes. No MFC planned.

The 'yes' answer to tri-state question is not informative.

If there is ABI breakage, its impact must be evaluated. It could be that this change is not acceptable if affecting symver-ed interfaces.

I believe this is a KBI breakage. I don't think it's an ABI breakage. But I don't see any changes to libgeom.

[eugen_grosbein.net]

In D13426#280612, @imp wrote:

I believe this is a KBI breakage. I don't think it's an ABI breakage. But I don't see any changes to libgeom.

libgeom already uses off_t/ssize_t/strtoumax for stripesize/stripeoffset.

[eugen_grosbein.net]

More changes: geom_nop, geom_stripe and geom_redboot.

[eugen_grosbein.net]

In D13426#280611, @kib wrote:

In D13426#280595, @eugen_grosbein.net wrote:

nop and others not mentioned in the diff use "off_t" internaly already.

Look at the argument types for g_nop_create() as an example.

Yes, I've missed that part, thanks. More changes added for geom_nop and geom_stripe (but not its on-disk metadata), and for geom_redboot (not sure if it's connected to build, though).

[eugen_grosbein.net]

In D13426#280534, @kib wrote:

Does this change break ABI or KBI ? Or both ?

All changes are within kernel land only and ioctls DIOCGSTRIPE{SIZE|OFFSET} already use off_t.

[eugen_grosbein.net]

I'm going to commit this soon unless an objection is raised.