Re: Subject:[PATCH 001:013]: md: Raid0 reshape

[Andre Noll <maan@systemlinux.org>]

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

On 11:23, Raz wrote:

> >> +     if (mddev->level == 0) {
> >> +             max_sectors = mddev->array_sectors;
> >> +             j = mddev->recovery_cp;
> >> +     }
> >>       printk(KERN_INFO "md: %s of RAID array %s\n", desc, mdname(mddev));
> >>       printk(KERN_INFO "md: minimum _guaranteed_  speed:"
> >>               " %d KB/sec/disk.\n", speed_min(mddev));
> >
> > Hm, we want to get rid of personality-dependent code in md.c, so new
> > code should never check mddev->level. In the first hunk I think it
> > would be possible to check if pers->sync_request is NULL.
> No. sync_request does exists. this is what Neil wanted me to do.
> implement raid0_sync
> so I will be using md for this purpose. I knew that md patch is a
> problem, this is why I posted
> it first.

OK, so pers->sync_request can't be used, but that was only one
possible option to replace the check mddev->level == 0 by something
more generic. BTW: The log message should describe in which case the
resync status shows an incorrect value and why one can not determine
which value to use for max_sectors by looking at mddev->recovery.

> > Is the second hunk really necessary? AFAICS md_do_sync() won't be
> > called for raid0 anyway.
> second hunk is necessary for resume reshape. md_do_sync is called
> indirecrly by raid0d -->md_check_recovery.

Yes, I didn't notice because raid0d() is introduced later in patch
10/13 of your series. When introducing new and apparently dead code
it's always nice to let people know that the new code is going to be
used by a function that will be added in a subsequent patch :)

Regards
Andre
-- 
The only person who always got his work done by Friday was Robinson Crusoe

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]