On Thu, Apr 8, 2010 at 7:48 AM, Hawrylewicz Czarnowski, Przemyslaw
<przemyslaw.hawrylewicz.czarnowski@xxxxxxxxx> wrote:
> If config has list of devices associated with container and it comprises given spare drive, UUID is set apporopriately to this container.
> It allows to assign spare with proper container according
> to content in devices parameter in configuration file.
>
I guess it would be nice if a user could direct spares via the
configuration file, but what gets honored in the future when this
value conflicts with a container group id in the metadata? Especially
when the actual device associated with a given name /dev/sda changes
from one boot to the next, potentially even across controllers, or the
admin moves the drive and forgets to update the configuration file.
As much as possible I would like everything to be determined from the
platform / metadata and discourage using the 'devices' line in the
configuration file.
So, I'm on the fence with this one.
A couple technical comments below:
> Signed-off-by: Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@xxxxxxxxx>
> ---
> super-intel.c | 48 +++++++++++++++++++++++++++++++++++++++++++-----
> 1 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/super-intel.c b/super-intel.c
> index a196ca3..41a4cc6 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -1497,10 +1497,19 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info)
> uuid_from_super_imsm(st, info->uuid);
> }
>
> +static int matchfd2devnum(int fd, int maj, int min)
> +{
> + struct stat st;
> + if (fstat(fd, &st) != 0)
> + return 1;
> + return ((int)major(st.st_rdev) == maj && (int)minor(st.st_rdev) == min);
> +}
> +
> +
> /* check the config file to see if we can return a real uuid for this spare */
> -static void fixup_container_spare_uuid(struct mdinfo *inf)
> +static void fixup_container_spare_uuid(struct mdinfo *inf, struct dl *disk)
> {
> - struct mddev_ident_s *array_list;
> + struct mddev_ident_s *array_list, *first_match = NULL;
>
> if (inf->array.level != LEVEL_CONTAINER ||
> memcmp(inf->uuid, uuid_match_any, sizeof(int[4])) != 0)
> @@ -1520,12 +1529,41 @@ static void fixup_container_spare_uuid(struct mdinfo *inf)
> _sst = NULL;
>
> if (_sst) {
> - memcpy(inf->uuid, array_list->uuid, sizeof(int[4]));
> + char *devices = array_list->devices;
> +
> + if (!first_match)
> + first_match = array_list;
> +
> + while (disk && devices && *devices) {
> + char path[PATH_MAX];
> + char *p = devices;
> + int fd;
> +
> + devices = strchr(devices, ',');
> + if (!devices)
> + devices = p + strlen(p);
> + if (devices-p < PATH_MAX) {
> + strncpy(path, p, devices - p);
> + if ((fd = open(path, O_RDONLY)) >= 0) {
> + if (matchfd2devnum(fd, disk->major, disk->minor)) {
> + free(_sst);
> + close(fd);
> + goto match;
> + }
> + close(fd);
> + }
> + if (*devices == ',')
> + devices++;
> + }
> + }
This routine is already available: match_oneof() in config.c
> free(_sst);
> - break;
> }
> }
> }
> +match:;
Don't need a semicolon after a label.
--
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
