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