On Wed, 8 Dec 2010 13:51:27 +0000 "Kwolek, Adam" <adam.kwolek@xxxxxxxxx>
wrote:
> I didn't describe those functions, because I've realized that it has to be changed after your email.
> I've planned to do this before I send patches, but unfortunately this "thread" was missed before sending ;).
> Here is current state.
>
> Functions:
> - path2devnum() (Translates path (or symbolic link) to md device given by user to device number.)
> - find_array_minor() (returns device minor for given array name, it uses maps to find names with additions (i.e. "_0") from assebbly)
> - find_array_minor2() (as find_array_minor() but works for frozen arrays also)
>
> Will be removed and replaced by:
>
> int find_array_minor_by_subdev(int subdev, int container, int *minor)
> {
> char text_version[PATH_MAX];
> char path[PATH_MAX];
> int i;
> struct stat s;
>
> sprintf(text_version, "md%i/%i", container, subdev);
> for (i = 127; i >= 0; i--) {
> char buf[PATH_MAX];
>
> snprintf(path, PATH_MAX, "/sys/block/md%d/md/", i);
> if (stat(path, &s) != -1) {
> char *version;
>
> strcat(path, "metadata_version");
> if (load_sys(path, buf))
> continue;
> version = strchr(buf, ':');
> /* compare without first letter
> * it could be marked as frozen with '-'
> */
> if (!version || strcmp(version + 2, text_version))
> continue;
> *minor = i;
> return 0;
> }
> }
>
> return -1;
> }
>
> This is a little part of find_array_minor() with changed input.
>
> This function searches given container for subdev and returns it minor id.
> i.e.: for input parameters 0 and 127 it creates search string i.e. "md127/0" and for such device returns minor (~126).
> For success 0 is returned, and -1 for failure (and passed minor variable remains untouched).
> It searches all devices from 127 to 0.
> I think it is simpler now (as you raised this issue), and mdadm doesn't make decisions based on array name/string/ now.
>
> Please let me know how do you find this change.
Yes much simpler and better. And I understand easily what you are trying to
do which is also good!
There are two problems:
1/ it assumes that subdevs have a number. I would prefer that generic code
assumed that a subdev has a name - if that happens to always be a numeric
name, that is up to the metadata handler.
2/ searching from 127 to 0 is wrong because md number can be much bigger than
127 if you have enough devices. It would be better to use readdir to get a
list of the /sys/block/mdXX that actually exist.
However I would rather not uses sysfs at all, but mdstat as that is easy to
read and search quickly.
Have a look at mdstat_by_component. Write something similar which is given a
container and subarray name and returns the corresponding mdstat_ent.
That would be much cleaner and even simpler.
>
> I've replaced open() with open_dev() in this patch (and in a few next one: 0003, 0007, 0012, 0019, 0021)
>
> If it is ok, please let me know if you want fresh series or you'll apply current one and I'll send patches for above changes then.
> In future patches, I'll address problem with lines longer than 80 characters also.
Please always send a series of patches against what you pull from my
devel-3.2 branch. I'll fix up any conflicts against anything I might have
committed since you pulled.
It is probably best to send smallish sets of patches - maybe about 10 at a
time. If they are accepted, you can send the next batch. If there are
issues, you can revise subsequent patches before you send them.
NeilBrown
>
> Please note also that implementation of searching spare devices for reshape is not final.
> It requires better integration with auto-rebuild searching spares mechanism.
> This task is in my nearest plans (I'm hoping to post fixes instead of whole series again ;))
>
>
> BR
> Adam
>
--
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
