On Wed, Jun 17, 2009 at 10:57 PM, Neil Brown<neilb@xxxxxxx> wrote: > On Tuesday June 16, dan.j.williams@xxxxxxxxx wrote: >> Hi Neil, >> >> The following changes since commit 6e92d480f765dc22a5abaa5658ad603353648c1c: >> NeilBrown (1): >> Release mdadm-3.0 >> >> are available in the git repository at: >> >> git://github.com/djbw/mdadm.git master >> > > Thanks. However..... > >> Dan Williams (6): >> fix RebuildMap() to retrieve 'subarray' info > > This hides two totally different changes in the one patch, which is > never a good idea. Yes. I removed the second fix. Also, validation found a bug in set_member_info so I needed to respin this patch. > I'm not at all convinced about the > > For imsm we need an additional fixup to force local naming otherwise > RebuildMap() will always choose foreign names. > > bit. Maybe if you could explain that. I took a second look and have decided to drop this hack in favor of requiring a configuration file in order to get local naming with Intel metadata. This patch would have degraded the protection afforded by explicitly naming arrays in mdadm.conf. The end result being that you will see the following result if the configuration file and the symlink created at Assemble() or Create() are removed: # cat /var/run/mdadm/map md127 imsm 53d6f8b1:7a783f24:f30483c5:705c48c7 /dev/md/imsm0 md126 /md127/0 aba8e22d:83f5033c:32ca5b1e:9579732d /dev/md/vol0 # rm /dev/md/vol0 # rm /etc/mdadm.conf # mdadm -Ir # cat /var/run/mdadm/map md127 imsm 53d6f8b1:7a783f24:f30483c5:705c48c7 /dev/md/imsm0 md126 /md127/0 aba8e22d:83f5033c:32ca5b1e:9579732d /dev/md/vol0_0 >> teach imsm and ddf what st->subarray means at load_super time >> fix examine_brief segfault >> imsm: fixup examine_brief to be more descriptive in the container only case > > Those seem OK. > >> fixup map file after Create > > This one bothers me too. If there is a need to update the mapfile, > then we should perform the required update, e.g. with map_update. > What exactly is needed here? I overlooked map_update. What is needed here is a refresh of the map_file after adding the first member array to an Intel metadata container. The uuid for an imsm container uses the ->family_num field of the metadata. This field is static, but is only set after the first member array has been created. Prior to this all devices are free floating spares and do not have any information that can identify specific container membership. At Create() time we take the uninitialized uuid from ->get_info_super() prior to updating the metadata. So the current result is: # mdadm --create /dev/md/imsm /dev/sd[b-e] -n 4 -e imsm # mdadm --create /dev/md/vol0 /dev/md/imsm -n 4 -l 0 # cat /var/run/mdadm/map md126 /md127/0 3e03aee2:78c3c593:1e8ecaf0:eefb53ed /dev/md/vol0 md127 imsm 53d6f8b1:7a783f24:f30483c5:705c48c7 /dev/md/imsm # mdadm -Ebs ARRAY metadata=imsm UUID=589d2d2c:4221a54d:acb63c06:c3907f52 ARRAY /dev/md/vol0 container=589d2d2c:4221a54d:acb63c06:c3907f52 member=0 UUID=57b89b63:5cd0eae1:17dd26b3:51cc78d4 RebuildMap() is a big hammer, but it has the nice side effect of updating the member array uuid and the container. I can use map_update(), but I will probably also need to call ->load_super() again so I can update the container uuid. > > >> imsm: fix activate_spare off-by-one > > I would have fixed this by simply changing the '<' to '<=', but your > fix is OK, and esize can certainly go. > > So for now I haven't pulled any of this. Ok, I will rework the map fixup patch then we can see if we are on the same page. Thanks, Dan -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
