Re: RAIDFrame changes

[Greg Oster <oster%cs.usask.ca@localhost>]

On Sun, 27 Dec 2015 11:49:36 +0000 (UTC)
mlelstv%serpens.de@localhost (Michael van Elst) wrote:

> Hi,
> 
> now that RAIDFrame is usuable at a module I have prepared
> a patch to refactor the driver to use the common dksubr code.
> 
> The patch can be found at
> 
>  http://ftp.netbsd.org/pub/NetBSD/misc/mlelstv/raidframe.diff
> 
> and it includes a few other fixes too:
> 
> - finding components now does proper kernel locking by using
>   bdev_strategy, previously it would trigger an assertion in the sd
>   driver (and maybe others).
> 
> - finding components now runs in two passes to prefer wedges over
>   raw partitions when the wedge starts at offset 0.
> 
> - defer RAIDFRAME_SHUTDOWN (raidctl -u operation) to raidclose.
>   - side effect is that 'raidctl -u' succeeds even for units that
>         haven't been configured. The previous behaviour was to keep
>         the embryonal unit and fail ioctl and close operations which
>         prevents unloading of the module.
> 
> - moved raidput again to raid_detach() because raid_detach_unlocked()
>   is only called for initialized units.
> 
> - use common dksubr code
>   - the fake device softc now stores a pointer back to the real softc
>   - no longer uses a private bufq
>   - private disklabel and disk_busy code is gone.
> 
> - some extra messages
> 
> 
> 
> I will commit this in the next days if there is no objection.

Thanks for working on this, but "next days" doesn't provide any real
time during the "Holiday Season".  :(

Have these changes, especially those to raiddump(), been extensively
tested?  RF_PROTECTED_SECTORS used to be required in raiddump() so as
not to eat the component label.  raid_dumpblocks() now contains what
raiddump() used to, but it raiddump() has to be able to
select which of the underlying components is still alive, and not to
attempt to dump to anything other than a RAID 1 device.  I'm not seeing
the mechanisms by which those requirements are still met........

Methinks these changes should have been explicitly reviewed by the
RAIDframe maintainer *before* they were committed. :(

Later...

Greg Oster