Re: [PATCH 15/17] Monitor: more accurate size check when looking for spares

[Neil Brown <neilb@suse.de>]

On Tue, 9 Nov 2010 09:23:49 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> On Tue, Nov 9, 2010 at 7:43 AM, Czarnowska, Anna
> <anna.czarnowska@intel.com> wrote:
> >>
> >> Neil wondered if we can repurpose validate_geometry for this case?  It
> >> is already charged with checking if a disk is suitable to be added to
> >> an
> >> array.
> >
> > I had a look at validate_geometry and I don't think it makes sense to modify it for Monitor.
> > Validate_geometry is made for Create. When creating container validate_geometry doesn't check much for imsm. When adding to a volume all disks must be already in the same container and have some common free region. We want the check before we move a spare. And the whole spare is free (is not used by any arrays in that container). I think it is better to have smaller functions and use them as building blocks, than make a big function even more complicated for every special case.
> 
> Yes, and no :-). There is a line to be drawn between refactoring and
> adding complexity, and you are right that all things being equal
> validate_geometry() as it stands today is an awkward fit.  However,
> the architecture rework Neil is doing is coming precisely from the
> realization that we are generating lots of these special case routines
> that are doing not much more than representing the same data to
> slightly different contexts.  The conclusion to be drawn is that our
> interface from generic mdadm to the metadata handler is perhaps too
> fine grained.   Hence the refactoring and why in this case it is
> worthwhile to at least ask the question: can a routine tasked with
> validating disks are proper candidates to be added to an array /
> container be trivially re-purposed to handle the case of validating
> that a spare is suitable for an existing container?  I agree with your
> conclusion, but wanted to share why I think Neil asked the question,
> hopefully I am not putting words in his mouth.
> 
> --
> Dan


I had a proper look at the code and I think that when I said
"validate_geometry", I should have said "avail_size".

We can probably just use the one method for both 'avail_size' and
'min_acceptable_spare_size' though I'm not sure currently what it would look
like.

So I'm happy to take that patch with min_acceptable_spare_size, and I will
quite possibly revise it later.

NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html