On Thu, 09 Jun 2011 18:29:21 +0200 Adam Kwolek <adam.kwolek@xxxxxxxxx> wrote:
> getinfo_super() can clear entire 'inf' structure before filling with new
> information. Disk number required later is lost.
>
> Restore disk number information after getinfo_super() call.
>
> Signed-off-by: Adam Kwolek <adam.kwolek@xxxxxxxxx>
> ---
>
> Create.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Create.c b/Create.c
> index 7b4d0fe..d01dea7 100644
> --- a/Create.c
> +++ b/Create.c
> @@ -805,7 +805,6 @@ int Create(struct supertype *st, char *mddev,
> switch(pass) {
> case 1:
> *inf = info;
> -
> inf->disk.number = dnum;
> inf->disk.raid_disk = dnum;
> if (inf->disk.raid_disk < raiddisks)
> @@ -856,12 +855,13 @@ int Create(struct supertype *st, char *mddev,
> /* getinfo_super might have lost these ... */
> inf->disk.major = major(stb.st_rdev);
> inf->disk.minor = minor(stb.st_rdev);
> + inf->disk.number = dnum;
> + inf->disk.raid_disk = dnum;
> }
> break;
> case 2:
> inf->errors = 0;
> rv = 0;
> -
> rv = add_disk(mdfd, st, &info, inf);
>
> if (rv) {
Unfortunately this is wrong too.
There should be no need to set disk.number and disk.raid_disk as the
->getinfo_super is supposed to have set them.
As the comment in mdadm.h says:
/* Extract generic details from metadata. This could be details about
* the container, or about an individual array within the container.
* The determination is made either by:
* load_super being given a 'component' string.
* validate_geometry determining what to create.
* The info includes both array information and device information.
* The particular device should be:
* The last device added by add_to_super
* The device the metadata was loaded from by load_super
* If 'map' is present, then it is an array raid_disks long
* (raid_disk must already be set and correct) and it is filled
* with 1 for slots that are thought to be active and 0 for slots which
* appear to be failed/missing.
* *info is zeroed out before data is added.
*/
In this case, ->add_to_super was recently called so it should record state so
that getinfo_super can return the correct information. That is what super0
and super1 does.
It seems that this has been wrong for a long time and my recent change to
zero the info structure showed it up.
So I'll let the patch through so as not to hold things up unnecessarily, but
I would really like you to see if you can fix add_to_super and getinfo_super
so that they follow the documented behaviour.
Thanks,
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
