Re: [PATCH 5/5] imsm: verify maximum supported active volumes in assembly

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

 



On Mon, 30 Jan 2012 10:24:25 +0000 "Labun, Marcin" <Marcin.Labun@xxxxxxxxx>
wrote:

> Hi,
> > 
> > hi,
> >  I've applied the first four in this series, but I don't like this one.
> > 
> > 1/ I don't think there is any need to impose this restriction when
> >    assembling an array - only when creating a new volume in an array.
> We want to support only as many volumes as OROM (platform) supports. Disks with IMSM metadata may be carried from other systems.

I don't think that makes sense.

There are two important issues here.

1/ we don't want to make a change to an array which will make that array
  inaccessible to the OROM.
2/ we don't want to make it difficult for users to get their data.


So any create or grow attempt which could lead to the OROM being unable to
access the array should be stopped (unless explicitly over-ridden).

But any attempt to access data on any array (i.e. assemble an array) should
succeed no matter whether it is compatible with the OROM or not.

So --create and --grow should check for platform compatibility.
--assemble should not.

--examine and --detail should probably report if the array is incompatible
with the OROM.


> 
> > 
> > 2/ The check_volumes_number approach feels clumsy ... there must be a
> >    a better way (Assuming it is needed at all).
> container_content handler is to check OROM capabilities and mark unsupported volumes with 
> disable bits (array.state:MD_SB_BLOCK_CONTAINER_RESHAPE and MD_SB_BLOCK_VOLUME). The caller function shall interpret the bits and act
> accordingly to the context (for instance block activation but display the volume information). I think that this restriction
> falls into that area. However, there are only a few situations when this info is really needed ie. When volumes are activated in various ways.
> Therefore I thought it makes sense to pass the info if counting of activate volumes is really needed. 
> More straightforward option is to add another input parameters to container_content and change it in numerous places.
> Do you prefer this way? Or add a specialized handler to count the volumes when it is needed?

Hmmm.. so the issue is that a single controller can have multiple separate
containers (families?) with their own metadata.
So a controller with 4 ports could have a 2-device container with a RAID1 and
a RAID0, and a separate 2-device container with just a RAID10.

And maybe the controller has a limit of 3 arrays, so adding another array to
the second pair would be an error.

Is that the right sort of scenario?

So when assembling the second container we don't need to look at the first
one, but when adding an array to the second, we do need to look at the first
one.
And looking at the first one is a little expensive so we don't want to do it
if we don't have to.  Still correct?

It sounds to me like validate_geometry is the place to check for other
containers on the controller.  That should be called at exactly the times you
need.... for create anyway.  Might also need it in reshape_super as well.

Would that work?

Alternately, we probably do want it for --examine so we can report possible
problems... maybe it isn't that expensive and we can always do the check??
How expensive would it be to always do the check on number of volumes??


NeilBrown

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux