On Thu, 18 Mar 2010 22:13:06 -0400 Doug Ledford <dledford@xxxxxxxxxx> wrote: > I have several minor fixes against the 3.1.2 mdadm. Thanks! > > First attachment: mdadm-3.1.1-endian.patch > Problem: on ppc arches mdadm would fail to assemble arrays if you used > mdadm -Db /dev/<array> to populate the ARRAY line in mdadm.conf. This > is because the code in Detail.c calls fname_from_uuid() which honors the > swapuuid setting in the superblock switch struct. This is great for ddf > and imsm superblocks which call fname_from_uuid() all over the place and > are therefore consistent. However, super1.c does *not* call > fname_from_uuid()...ever. And super1.c does *not* perform the byte > swapping as per swapuuid in the switch struct. At least not on > *display*, but it *does* honor swapuuid on actual analysis of the uuid > in terms of figuring out matches. So, if you attempt to change the > value of swapuuid in the super1 switch struct, then uuid's never match > what's printed out (aka, the output of -D and -E both fail to match > against the detected uuid when assembling, although at least -D and -E > are now consistent). Instead, I did a simple (and gross) hack to ignore > swapuuid in fname_from_uuid() only for version 1 superblocks. The other > alternative might be to make super1.c call fname_from_uuid() all over > the place instead of printing things out itself, but that would involve > struct changes as the way it stores/accesses uuids in super1 is not per > char like fname_from_uuid and that's precisely part of the problem. Ohh, that is horrible isn't it. I have applied your patch but I very much hope to "fix" the problem in a more comprehensive way so that your fix will disappear. I suspect that I will change the common code to use an array of bytes and do any conversion from u32[4] to u8[1] in the per-metadata code. > > Second: mdadm-3.1.2-mapfile.patch > Problem: Neil's support for putting the mdadm map file wherever you need > it is nice, but one place in particular needs a special case. > Specifically, mdadm already creates /dev/md if needed to store symlinks, > or in the case of mdmon needing to create pid/sock files (if ALT_RUN is > set to /dev/md). This creates an expectation in udev and friends that > mdadm will create /dev/md when needed, period. We need it if we place > the mapfile in /dev/md prior to any symlinks or mdmon files being > created, so special case that one location in order to make us > consistent with both mdmon and mdopen. If we don't, we will fail to > create our mapfile. I thought udev always created the directory if it was needed, and always removed it if it became empty after deleting an entry. But I have no strong basis for believing that. However I do think that mdadm should create the directory in this case - not any parents, but if they exist and are writeable, then create the directory. So I modified you patch to do that when creating the mapfile (not when reading it). > > Third: mdadm-3.1.2-rebuild.patch > Problem: The above issue pointed out a different issue. Specifically, > if we can't open our mapfile, then we call RebuildMap, and at the end of > the rebuild, we trigger a new change event on the raid device. Well, if > the write of the new map file failed because the directory we wanted to > write into didn't exist, this creates an infinite loop where udev calls > mdadm, mdadm fails to open file, mdadm rebuils map, mdadm fails to write > new map, but mdadm triggers a change event to cause udev to rerun mdadm > on the belief that the map file *was* updated. So, if we fail to write > the new mapfile, don't signal a change event, let the situation drop. > This avoids the infinite loop. Very appropriate - thanks. > > Fourth: mdadm-3.1.2-mapname.patch > Feature: If we are able to easily select the location of the mapfile via > the use of ALT_RUN at compile time, it makes sense to also be able to > set the filename. This way you can make it something obvious such as > /dev/md + md-device-map which makes the file name both unlikely to > conflict with a device name (where as I could see map conflicting in > certain specialized scenarios, like the array holding map data at the > local governmental office) and obvious in purpose. > > Seems reasonable. I've included that patch too. Thanks a lot, NeilBrown -- 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
