On Mon, 7 Jul 2014 14:18:40 +0000 "Baldysiak, Pawel"
[off-list ref] wrote:

quoted

On Monday, July 07, 2014 3:06 AM Neil Brown wrote:
To: Baldysiak, Pawel
Cc: linux-raid@vger.kernel.org; Paszkiewicz, Artur
Subject: Re: [PATCH] Grow: Do not use grow-continue unit file if reshape is
starting

On Fri, 4 Jul 2014 08:59:00 +0000 "Baldysiak, Pawel"
[off-list ref] wrote:

quoted

Mdadm should use mdadm-grow-continue unit file only if reshape is
going to be continued. Otherwise, array specific reshape with IMSM
metadata will fail to start, due to missing information about ongoing
migration - grow-continue will try to start again the reshape process.

I don't think I agree.  I think mdadm should always use mdadm-grow-
continue unit file if it is available.

Please explain in more detail what problem you are seeing.

NeilBrown

Hi Neil

If we try to do array-specific reshape (e.g. change from level 0 to 5) 
mdadm will impose all changes to metadata and start reshape - before running grow-continue.
After that  grow-continue will see that reshape progress is '0', so will overwrite "restart" by setting it to =0.

I assume you are referring to this code:
	if (st->ss->external && restart && (info->reshape_progress == 0)) {
          .....
		if ((verify_reshape_position(info, reshape.level) >= 0) &&
		    (info->reshape_progress == 0))
			restart = 0;
	}

in reshape_array().

Then it will try to impose all changes again and fail - with error:
"Cannot set device shape for /dev/md/vol" - It will be unable to write changes to sysfs,
due to already started reshape (which freeze at '0' progress).

We can try to fix it by moving part of code that runs grow-continue in reshape_array earlier,
 so "start_reshape() " will be executed by grow-continue (I am testing that kind of patch now), 
or forbid running grow-continue when reshape is about to start (patch is tested, attach in my prev mail).

What do you think about that?
Do you have any other idea how to fix this issue?

I suspect that the real problem is in the code above.
Maybe it should read the "sync_action" attribute and if it is "reshape", then
this is obviously a restart and it shouldn't set "restart" to 0.

Can you try something like that?

NeilBrown

Thanks
Pawel Baldysiak

quoted

quoted

Signed-off-by: Pawel Baldysiak <redacted>
---
 Grow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Grow.c b/Grow.c
index a2f4f14..0cd9442 100644
--- a/Grow.c
+++ b/Grow.c

@@ -3272,7 +3272,7 @@ started:
 		return 1;
 	}

-	if (!forked && !check_env("MDADM_NO_SYSTEMCTL"))
+	if (restart && !forked && !check_env("MDADM_NO_SYSTEMCTL"))
 		if (continue_via_systemd(container ?: sra->sys_name)) {
 			free(fdlist);
 			free(offsets);

-- 

--