[dm-devel] dm: Fix alignment stacking on partitioned devices

[Martin K. Petersen]

>>>>> "Mike" == Mike Snitzer <snitzer at redhat.com> writes:

Mike> One thing I noticed along the way is that: when stacking one or
Mike> more misaligned devices there is the potential for misleading
Mike> error messages, e.g.:

[...]

Mike> So sdb2 is taken to be aligned (even though it isn't, relative to
Mike> the queue's alignment_offset of 3584) and then when sdb1 is
Mike> stacked it is flagged as misaligned.  Doesn't seem right.

I know.  It's an unfortunate side-effect of having an iterative stacking
process.  I don't know the whole picture so I don't know which device
the real offender is.  At the time you add device #2 I have no
recollection of device #1.

When you create a new top-level (DM/MD) device and add the first disk to
it I have no option but to inherit that component's queue limits.  So
that's what initially defines the top-level.

In your case adding a misaligned device will cause the stacking to
report that alignment can be obtained by padding the top level device
with an offset. Then you add a device which happens to be naturally
aligned. And that means the stacking logic breaks down because the
0-alignment is incompatible with the top's nonzero alignment_offset.

Originally (before the Great Naming Debate) the `misaligned' flag was
called `inconsistent' (actually, it was called `consistent' and the
logic was reversed).  I think that's a better term for it.  Compatible
would also work, I guess.

So when blk_stack_limits() returns an error it is because no consistent
alignment could be calculated.  blk_stack_limits() returning 0 does not
imply "alignment" (i.e. an alignment_offset of 0).  A return value of 0
means that we have successfully added the bottom device and the
resulting top device queue_limits now apply.

Maybe you should print something along the lines of "adding device sdX
caused an alignment consistency error on DM device foo".  That's a bit
vague but doesn't single out sdX as much.

The alternative is to add a device list to the queue_limits so we can
record previously stacked devices.  But that opens up a whole other can
of worms...

-- 
Martin K. Petersen	Oracle Linux Engineering