On 2/9/19 1:05 AM, Corey Hickey wrote:
> On 2019-01-07 21:37, bugfood-ml@xxxxxxxxxx wrote:
>> From: Corey Hickey <bugfood-c@xxxxxxxxxx>
>>
>> ...when not changing the number of disks.
>>
>> This patch needs context to explain. These are the relevant parts of
>> the original code (condensed and annotated):
>>
>> if (dir > 0) {
>> /* Increase data offset (reshape backwards) */
>> if (data_offset < sd->data_offset + min) {
>> pr_err("--data-offset too small on %s\n",
>> dn);
>> goto release;
>> }
>> } else {
>> /* Decrease data offset (reshape forwards) */
>> if (data_offset < sd->data_offset - min) {
>> pr_err("--data-offset too small on %s\n",
>> dn);
>> goto release;
>> }
>> }
>>
>> When this code is reached, mdadm has already decided on a reshape
>> direction. When increasing the data offset, the reshape runs backwards
>> (dir==1); when decreasing the data offset, the reshape runs forwards
>> (dir==-1).
>>
>> The conditional within the backwards reshape is correct: the requested
>> offset must be larger than the old offset plus a minimum delta; thus the
>> reshape has room to work.
>>
>> For the forwards reshape, the requested offset needs to be smaller than
>> the old offset minus a minimum delta; to do this correctly, the
>> comparison must be reversed.
>>
>> Also update the error message.
>>
>> Note: I have tested this change on a RAID 5 on Linux 4.18.0 and verified
>> that there were no errors from the kernel and that the device data
>> remained intact. I do not know if there are considerations for different
>> RAID levels.
>> ---
>> Grow.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Grow.c b/Grow.c
>> index 363b209..c2cae00 100644
>> --- a/Grow.c
>> +++ b/Grow.c
>> @@ -2613,8 +2613,8 @@ static int set_new_data_offset(struct mdinfo
>> *sra, struct supertype *st,
>> goto release;
>> }
>> if (data_offset != INVALID_SECTORS &&
>> - data_offset < sd->data_offset - min) {
>> - pr_err("--data-offset too small on %s\n",
>> + data_offset > sd->data_offset - min) {
>> + pr_err("--data-offset too large on %s\n",
>> dn);
>> goto release;
>> }
>
> Hi,
>
> Sorry to pester, but is this patch acceptable?
Patch looks fine, but you need to post it with a signed-off-by as by
standard Linux patch policy.
Cheers,
Jes
