Re: [PATCH 2/2] mdadm: save previous journal_clean when reload super

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]

On Mon, Aug 28 2017, Song Liu wrote:

> In Incremental.c:count_active(), max_events is tracked to show to
> which devices are up to date. If a device has events==max_events+1,
> getinfo_super() is called to reload the superblock from this
> device. getinfo_super1() blindly set journal_clean to 0, which is
> wrong. This patch fixes this issue by saving previous
> journal_clean before calling getinfo_super().
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
>  Incremental.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Incremental.c b/Incremental.c
> index 6cf2174..b17b37f 100644
> --- a/Incremental.c
> +++ b/Incremental.c
> @@ -747,13 +747,16 @@ static int count_active(struct supertype *st, struct mdinfo *sra,
>  				;
>  			else if (info.events == max_events+1) {
>  				int i;
> +				int journal_clean;
>  				max_events = info.events;
>  				for (i = 0; i < raid_disks; i++)
>  					if (avail[i])
>  						avail[i]--;
>  				avail[info.disk.raid_disk] = 2;
>  				best[info.disk.raid_disk] = devnum;
> +				journal_clean = bestinfo->journal_clean;
>  				st->ss->getinfo_super(st, bestinfo, NULL);
> +				bestinfo->journal_clean = journal_clean;
>  			} else { /* info.events much bigger */
>  				memset(avail, 0, raid_disks);
>  				max_events = info.events;


This is a hack to work around a symptom.  It is not a real fix.

I'm not sure what the real fix is.
Maybe when you find "raid_disk == MD_DISK_ROLE_JOURNAL" you set
"journal_events = info.events", then after the loop, if
"journal_events >= max_events-1", you set "bestinfo->journal_clean = 1",
or something like that.
But you need to make it obvious that the code is correct, and the above
code looks like a hack.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]