On Fri, 26 Nov 2010 09:06:38 +0100 Adam Kwolek <adam.kwolek@xxxxxxxxx> wrote:
> Wrong number of disks during metadata update causes core dump.
> New disks number based on internal mdmon information has to used for calculation (not previously read from metadata).
>
> Signed-off-by: Adam Kwolek <adam.kwolek@xxxxxxxxx>
I've applied this, thanks.
One small comment ....
> ---
>
> super-intel.c | 27 ++++++++++++++++++---------
> 1 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/super-intel.c b/super-intel.c
> index 98e4c6d..1231fa8 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -3510,8 +3510,9 @@ static int write_super_imsm_spares(struct intel_super *super, int doclose)
> return 0;
> }
>
> -static int write_super_imsm(struct intel_super *super, int doclose)
> +static int write_super_imsm(struct supertype *st, int doclose)
> {
> + struct intel_super *super = st->sb;
> struct imsm_super *mpb = super->anchor;
> struct dl *d;
> __u32 generation;
> @@ -3519,6 +3520,7 @@ static int write_super_imsm(struct intel_super *super, int doclose)
> int spares = 0;
> int i;
> __u32 mpb_size = sizeof(struct imsm_super) - sizeof(struct imsm_disk);
> + int num_disks = 0;
>
> /* 'generation' is incremented everytime the metadata is written */
> generation = __le32_to_cpu(mpb->generation_num);
> @@ -3531,21 +3533,28 @@ static int write_super_imsm(struct intel_super *super, int doclose)
> if (mpb->orig_family_num == 0)
> mpb->orig_family_num = mpb->family_num;
>
> - mpb_size += sizeof(struct imsm_disk) * mpb->num_disks;
> for (d = super->disks; d; d = d->next) {
> if (d->index == -1)
> spares++;
> - else
> + else {
> mpb->disk[d->index] = d->disk;
> + num_disks++;
> + }
> }
> - for (d = super->missing; d; d = d->next)
> + for (d = super->missing; d; d = d->next) {
> mpb->disk[d->index] = d->disk;
> + num_disks++;
> + }
> + mpb->num_disks = num_disks;
> + mpb_size += sizeof(struct imsm_disk) * mpb->num_disks;
>
> for (i = 0; i < mpb->num_raid_devs; i++) {
> struct imsm_dev *dev = __get_imsm_dev(mpb, i);
> -
> - imsm_copy_dev(dev, get_imsm_dev(super, i));
> - mpb_size += sizeof_imsm_dev(dev, 0);
> + struct imsm_dev *dev2 = get_imsm_dev(super, i);
> + if ((dev) && (dev2)) {
I don't like these extra parentheses. They are just noise and don't improve
clarity. Please just:
if (dev && dev2) {
NeilBrown
> + imsm_copy_dev(dev, dev2);
> + mpb_size += sizeof_imsm_dev(dev, 0);
> + }
> }
> mpb_size += __le32_to_cpu(mpb->bbm_log_size);
> mpb->mpb_size = __cpu_to_le32(mpb_size);
> @@ -3665,7 +3674,7 @@ static int write_init_super_imsm(struct supertype *st)
> struct dl *d;
> for (d = super->disks; d; d = d->next)
> Kill(d->devname, NULL, 0, 1, 1);
> - return write_super_imsm(st->sb, 1);
> + return write_super_imsm(st, 1);
> }
> }
> #endif
> @@ -4938,7 +4947,7 @@ static void imsm_sync_metadata(struct supertype *container)
> if (!super->updates_pending)
> return;
>
> - write_super_imsm(super, 0);
> + write_super_imsm(container, 0);
>
> super->updates_pending = 0;
> }
--
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
