[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [dm-devel] [PATCH] DM RAID: add RAID1 support



On May 25, 2011, at 3:05 AM, Christoph Hellwig wrote:

>> +#ifdef md_raid1_congested
>> +	if (rs->raid_type->level == 1)
>> +		return md_raid1_congested(&rs->md, bits);
>> +#endif
>> +
>> 	return md_raid5_congested(&rs->md, bits);
> 
> How well is this tested?  md_raid1_congested is a not a cpp macro
> in your md patch, so with that patch it never would be called.
> 
> Also I think the congested call really should go through function
> pointer indirections instead of opencoding all possible variants like
> this.

ok.

Indeed, these #if[n]def's mean that RAID1 is not allowed until they are removed.  Thus, the patch doesn't mean much until 1) the necessary changes are made in 'raid1.c' and 2) these macros are removed.  I thought I could be clever and introduce the functionality and have it available when the changes to 'raid1.c' were finished - not quite right.

Anyway, please look over the rest of the patch.  We can delay it until the 'raid1.c' changes are made.

 brassow


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]