[dm-devel] [PATCH 00/24] dm-raid456 support using md/raid5.c, now with dirty-log

[Heinz Mauelshagen]

On Thu, 2010-06-17 at 15:41 +1000, Neil Brown wrote:
> On Wed, 16 Jun 2010 13:26:27 +0200
> Heinz Mauelshagen <heinzm at redhat.com> wrote:
> 
> > On Wed, 2010-06-16 at 09:45 +1000, Neil Brown wrote:
> > > Hi Heinz,
> > > 
> > > On Tue, 15 Jun 2010 15:23:26 +0200
> > > Heinz Mauelshagen <heinzm at redhat.com> wrote:
> > > 
> > > > 
> > > > Neil,
> > > > 
> > > > for missing devices we (re)load the table with error mapped device(s) in
> > > > their place rather than identifying them with a special char/string.
> > > > Did you go for the later in order to avoid some MD hassle with error
> > > > targets being accessed by the MD personalities? If not so, we can drop
> > > > the special '-' char support to identify missing devices.
> > > 
> > > When raid5 determines that a device has failed and so it will not write to it
> > > again, it must be sure that the failure is record in the metadata before it
> > > proceeds with that decision not to even try writing to the failed device.
> > 
> > Yes, that's the mandatory order to avoid data loss.
> > 
> > > Otherwise a crash/restart before the metadata was safely updated would result
> > > in corruption.
> > > 
> > > This means that it must be possible for user-space to explicitly tell the
> > > raid5 that a device is known to be faulty.
> > > Doing that through the constructor seems the natural way to do it.
> > 
> > If /dev/mapper/error with an error target mapping would be used instead,
> > would that cause consistency troubles to the MD personality if accessing
> > those rather than the NULL device solution with the '-' argument you
> > have now?
> > If not so, we could drop the '-' support, which I'm aiming at.
> > 
> 
> I would need to differentiate between /dev/mapper/error (where errors are
> expected) and any other device (where errors are not expected).  How do you
> propose I do that?

Is that differentiation mandatory for the MD personality at all?

You'll get immediate and durable errors from the error target and hence
you'd instantly drop the leg.

> 
> > > 
> > > > 
> > > > I'm thinking about adding a ctr wrapper to allow us to keep the "raid45"
> > > > ctr interfaces semantics (ie. the arguments) and the new interface to
> > > > "raid456" if you don't have objections. That would be implemented by 2
> > > > targets being registered implemented in one module.
> > > 
> > > I have no strong objections, though having two targets that do almost the
> > > same things seems rather ugly.
> > 
> > Keep in mind it'll only be another target_type structs, another
> > raid_ctr() function plus another (de)registration of the respective
> > target. The ctr functions for raid45 and raid456 can share moset of the
> > code anyway.
> 
> Sure.
> 
> > 
> > > 
> > > Is there existing published code that uses the extra arguments to raid45?
> > 
> > Yes, dmraid with a list of RAID5 supporting metadata format handlers.
> 
> I just had a look at the latest dmraid from CVS and the only place that I can
> find where a raid45 target is created is in lib/activate/activate.c in
> _dm_raid45_bol.
> This only uses arguments that my code understands.
> What am I missing?

I was arguing against breaking API compatibility.
dm-raid45.c is being shipped by distros.

Thanks,
Heinz

> Thanks,
> NeilBrown