On Thu, Aug 31 2017, Song Liu wrote: >> On Aug 31, 2017, at 4:36 PM, NeilBrown <neilb@xxxxxxxx> wrote: >> >> On Thu, Aug 31 2017, Song Liu wrote: >> >>> Summary: >>> >>> Handling super == NULL case in avail_size1() was removed a while >>> back. However, it is still useful in the following stack: >>> >>> avail_size1() with st->sb == NULL >>> array_try_spare() with st == NULL >>> try_spare() with st == NULL >>> Incremental() with st == NULL >> >> I assume you mean "st->sb == NULL" in each case here? What you have >> written doesn't make sense. >> > > I did mean st == NULL. In array_try_spare(), if st is NULL, st2 is > allocated with match_metadata_desc(), and avail_size() is called with > st2. Ahhh, OK. But if st isn't NULL I think the problem still exists as when an st is passed to try_spare() it doesn't have an st->sb. > >>> >>> This patch adds the handling of super == NULL back to avail_size1(). >> >> I doubt this is the best thing to do. >> If we don't have st->sb, calling avail_size doesn't make any sense. >> Maybe we should be calling validate_geometry. >> > > This was the same logic that got removed a few years ago. So far it > works for my use cases. And I think it makes (some) sense. > Is validate_geometry() really better than this approach? validate_geometry is better because it doesn't expect 'sb' to be set. The documentation for avail_size says: * 'mdadm' only calls this for existing arrays where a possible * spare is being added. In the try_spare() case there is no existing array. Rather, try_spare() is checking if a particular geometry is valid. It is rather sad that this auto-sparing functionality has been broken for 4 years and no-one noticed until now. I really should have created some self-tests.... Thanks, NeilBrown
Attachment:
signature.asc
Description: PGP signature
