From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Williams Subject: Re: [PATCH] imsm: assign UUID of spare device based on ARRAY *device* keyword in mdadm configuration file. Date: Wed, 14 Apr 2010 15:15:01 -0700 Message-ID: References: <66C59AD0932712458090B447266D638CD3DBE917@irsmsx504.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <66C59AD0932712458090B447266D638CD3DBE917@irsmsx504.ger.corp.intel.com> Sender: linux-raid-owner@vger.kernel.org To: "Hawrylewicz Czarnowski, Przemyslaw" Cc: Neil Brown , "linux-raid@vger.kernel.org" , "Ciechanowski, Ed" List-Id: linux-raid.ids On Thu, Apr 8, 2010 at 7:48 AM, Hawrylewicz Czarnowski, Przemyslaw wrote: > If config has list of devices associated with container and it compri= ses 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 > --- > =A0super-intel.c | =A0 48 +++++++++++++++++++++++++++++++++++++++++++= ----- > =A01 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) > =A0 =A0 =A0 =A0uuid_from_super_imsm(st, info->uuid); > =A0} > > +static int matchfd2devnum(int fd, int maj, int min) > +{ > + =A0 =A0 =A0 struct stat st; > + =A0 =A0 =A0 if (fstat(fd, &st) !=3D 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 1; > + =A0 =A0 =A0 return ((int)major(st.st_rdev) =3D=3D maj && (int)minor= (st.st_rdev) =3D=3D min); > +} > + > + > =A0/* check the config file to see if we can return a real uuid for t= his spare */ > -static void fixup_container_spare_uuid(struct mdinfo *inf) > +static void fixup_container_spare_uuid(struct mdinfo *inf, struct dl= *disk) > =A0{ > - =A0 =A0 =A0 struct mddev_ident_s *array_list; > + =A0 =A0 =A0 struct mddev_ident_s *array_list, *first_match =3D NULL= ; > > =A0 =A0 =A0 =A0if (inf->array.level !=3D LEVEL_CONTAINER || > =A0 =A0 =A0 =A0 =A0 =A0memcmp(inf->uuid, uuid_match_any, sizeof(int[4= ])) !=3D 0) > @@ -1520,12 +1529,41 @@ static void fixup_container_spare_uuid(struct= mdinfo *inf) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0_sst =3D= NULL; > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (_sst) { > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 memcpy(= inf->uuid, array_list->uuid, sizeof(int[4])); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 char *d= evices =3D array_list->devices; > + > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!fi= rst_match) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 first_match =3D array_list; > + > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 while (= disk && devices && *devices) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 char path[PATH_MAX]; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 char *p =3D devices; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 int fd; > + > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 devices =3D strchr(devices, ','); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 if (!devices) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 devices =3D p + strlen(p); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 if (devices-p < PATH_MAX) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 strncpy(path, p, devices - p); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 if ((fd =3D open(path, O_RDONLY)) >=3D 0) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (matchfd2devnum(fd, disk->m= ajor, disk->minor)) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 free(_sst); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 close(fd); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto match; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 close(fd); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 } > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 if (*devices =3D=3D ',') > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 devices++; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 } > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } This routine is already available: match_oneof() in config.c > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0free(_= sst); > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0} > +match:; Don't need a semicolon after a label. -- Dan -- To unsubscribe from this list: send the line "unsubscribe linux-raid" i= n the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html