* [PATCH] mdadm/grow: reshape would be stuck from raid1 to raid5
@ 2017-03-20 5:20 Zhilong Liu
2017-03-27 22:10 ` jes.sorensen
0 siblings, 1 reply; 10+ messages in thread
From: Zhilong Liu @ 2017-03-20 5:20 UTC (permalink / raw)
To: jes.sorensen; +Cc: linux-raid, Zhilong Liu
it would be stuck at the beginning of reshape progress
when grows array from raid1 to raid5, correct the name
of mdadm-grow-continue@.service in continue_via_systemd.
reproduce steps:
./mdadm -CR /dev/md0 -l1 -b internal -n2 /dev/loop[0-1]
./mdadm --grow /dev/md0 -l5 -n3 -a /dev/loop2
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
Grow.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/Grow.c b/Grow.c
index 455c5f9..10c02a1 100755
--- a/Grow.c
+++ b/Grow.c
@@ -2808,13 +2808,11 @@ static int continue_via_systemd(char *devnm)
*/
close(2);
open("/dev/null", O_WRONLY);
- snprintf(pathbuf, sizeof(pathbuf), "mdadm-grow-continue@%s.service",
- devnm);
+ snprintf(pathbuf, sizeof(pathbuf), "mdadm-grow-continue@.service");
status = execl("/usr/bin/systemctl", "systemctl",
"start",
pathbuf, NULL);
- status = execl("/bin/systemctl", "systemctl", "start",
- pathbuf, NULL);
+ pr_err("/usr/bin/systemctl %s got failure\n", pathbuf);
exit(1);
case -1: /* Just do it ourselves. */
break;
--
2.6.6
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] mdadm/grow: reshape would be stuck from raid1 to raid5
2017-03-20 5:20 [PATCH] mdadm/grow: reshape would be stuck from raid1 to raid5 Zhilong Liu
@ 2017-03-27 22:10 ` jes.sorensen
2017-03-28 14:03 ` Liu Zhilong
2017-03-30 7:38 ` [PATCH v1] " Zhilong Liu
0 siblings, 2 replies; 10+ messages in thread
From: jes.sorensen @ 2017-03-27 22:10 UTC (permalink / raw)
To: Zhilong Liu; +Cc: linux-raid, Harald Hoyer
Zhilong Liu <zlliu@suse.com> writes:
> it would be stuck at the beginning of reshape progress
> when grows array from raid1 to raid5, correct the name
> of mdadm-grow-continue@.service in continue_via_systemd.
>
> reproduce steps:
> ./mdadm -CR /dev/md0 -l1 -b internal -n2 /dev/loop[0-1]
> ./mdadm --grow /dev/md0 -l5 -n3 -a /dev/loop2
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
> Grow.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index 455c5f9..10c02a1 100755
> --- a/Grow.c
> +++ b/Grow.c
> @@ -2808,13 +2808,11 @@ static int continue_via_systemd(char *devnm)
> */
> close(2);
> open("/dev/null", O_WRONLY);
> - snprintf(pathbuf, sizeof(pathbuf), "mdadm-grow-continue@%s.service",
> - devnm);
> + snprintf(pathbuf, sizeof(pathbuf), "mdadm-grow-continue@.service");
My memory is rusty here, isn't systemctl interpreting the device name in
mdadm-grow-continue@<device>.service as an argument?
> status = execl("/usr/bin/systemctl", "systemctl",
> "start",
> pathbuf, NULL);
> - status = execl("/bin/systemctl", "systemctl", "start",
> - pathbuf, NULL);
> + pr_err("/usr/bin/systemctl %s got failure\n", pathbuf);
> exit(1);
This assumes systemctl is location in /usr/bin only - you removed the
fallback case for it being location in /bin.
In addition, instead of saying 'got failure' lets do something with the
errno value so the user gets a more descriptive error message.
Cheers,
Jes
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mdadm/grow: reshape would be stuck from raid1 to raid5
2017-03-27 22:10 ` jes.sorensen
@ 2017-03-28 14:03 ` Liu Zhilong
2017-03-30 7:38 ` [PATCH v1] " Zhilong Liu
1 sibling, 0 replies; 10+ messages in thread
From: Liu Zhilong @ 2017-03-28 14:03 UTC (permalink / raw)
To: jes.sorensen; +Cc: linux-raid, Harald Hoyer, shli
On 03/28/2017 06:10 AM, jes.sorensen@gmail.com wrote:
> Zhilong Liu <zlliu@suse.com> writes:
>> it would be stuck at the beginning of reshape progress
>> when grows array from raid1 to raid5, correct the name
>> of mdadm-grow-continue@.service in continue_via_systemd.
>>
>> reproduce steps:
>> ./mdadm -CR /dev/md0 -l1 -b internal -n2 /dev/loop[0-1]
>> ./mdadm --grow /dev/md0 -l5 -n3 -a /dev/loop2
>>
>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>> ---
>> Grow.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/Grow.c b/Grow.c
>> index 455c5f9..10c02a1 100755
>> --- a/Grow.c
>> +++ b/Grow.c
>> @@ -2808,13 +2808,11 @@ static int continue_via_systemd(char *devnm)
>> */
>> close(2);
>> open("/dev/null", O_WRONLY);
>> - snprintf(pathbuf, sizeof(pathbuf), "mdadm-grow-continue@%s.service",
>> - devnm);
>> + snprintf(pathbuf, sizeof(pathbuf), "mdadm-grow-continue@.service");
> My memory is rusty here, isn't systemctl interpreting the device name in
> mdadm-grow-continue@<device>.service as an argument?
actually, the service started failed. paste the journalctl log here when
reshape from mirror to raid5.
command: ./mdadm --grow /dev/md0 -l5 -n3 -a /dev/loop2
Mar 28 21:43:47 linux-g0sr kernel: --- level:5 rd:3 wd:3
Mar 28 21:43:47 linux-g0sr kernel: disk 0, o:1, dev:loop0
Mar 28 21:43:47 linux-g0sr kernel: disk 1, o:1, dev:loop1
Mar 28 21:43:47 linux-g0sr kernel: disk 2, o:1, dev:loop2
Mar 28 21:43:47 linux-g0sr kernel: md: reshape of RAID array md0
Mar 28 21:43:47 linux-g0sr kernel: md: minimum _guaranteed_ speed: 1000
KB/sec/disk.
Mar 28 21:43:47 linux-g0sr kernel: md: using maximum available idle IO
bandwidth (but not more than 2000 KB/sec) for reshape.
Mar 28 21:43:47 linux-g0sr kernel: md: using 128k window, over a total
of 19968k.
Mar 28 21:43:47 linux-g0sr systemd[1]: Started Manage MD Reshape on
/dev/md0.
Mar 28 21:43:47 linux-g0sr systemd[1]: mdadm-grow-continue@md0.service:
Main process exited, code=exited, status=2/INVALIDARGUMENT
Mar 28 21:43:47 linux-g0sr systemd[1]: mdadm-grow-continue@md0.service:
Unit entered failed state.
Mar 28 21:43:47 linux-g0sr systemd[1]: mdadm-grow-continue@md0.service:
Failed with result 'exit-code'.
Mar 28 21:44:03 linux-g0sr kernel: md: md0: reshape interrupted.
>
>> status = execl("/usr/bin/systemctl", "systemctl",
>> "start",
>> pathbuf, NULL);
>> - status = execl("/bin/systemctl", "systemctl", "start",
>> - pathbuf, NULL);
>> + pr_err("/usr/bin/systemctl %s got failure\n", pathbuf);
>> exit(1);
> This assumes systemctl is location in /usr/bin only - you removed the
> fallback case for it being location in /bin.
>
> In addition, instead of saying 'got failure' lets do something with the
> errno value so the user gets a more descriptive error message.
I would continue to look at this issue, thanks for your time.
Thanks,
-Zhilong
> Cheers,
> Jes
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1] mdadm/grow: reshape would be stuck from raid1 to raid5
2017-03-27 22:10 ` jes.sorensen
2017-03-28 14:03 ` Liu Zhilong
@ 2017-03-30 7:38 ` Zhilong Liu
2017-03-30 15:50 ` Jes Sorensen
2017-04-03 4:36 ` NeilBrown
1 sibling, 2 replies; 10+ messages in thread
From: Zhilong Liu @ 2017-03-30 7:38 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
systemctl doesn't interpret mdadm-grow-continue@.service
correctly due to the wrong argument provided in [service],
it should be corrected %I as %i. Otherwise, if the service
cannot start by systemctl and the reshap progress would be
stuck all time when grows array from raid1 to raid5.
reproduce steps:
./mdadm -CR /dev/md0 -l1 -b internal -n2 /dev/loop[0-1]
./mdadm --grow /dev/md0 -l5 -n3 -a /dev/loop2
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
systemd/mdadm-grow-continue@.service | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/systemd/mdadm-grow-continue@.service b/systemd/mdadm-grow-continue@.service
index 5c667d2..882bc0b 100644
--- a/systemd/mdadm-grow-continue@.service
+++ b/systemd/mdadm-grow-continue@.service
@@ -10,7 +10,7 @@ Description=Manage MD Reshape on /dev/%I
DefaultDependencies=no
[Service]
-ExecStart=BINDIR/mdadm --grow --continue /dev/%I
+ExecStart=BINDIR/mdadm --grow --continue /dev/%i
StandardInput=null
StandardOutput=null
StandardError=null
--
2.10.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v1] mdadm/grow: reshape would be stuck from raid1 to raid5
2017-03-30 7:38 ` [PATCH v1] " Zhilong Liu
@ 2017-03-30 15:50 ` Jes Sorensen
2017-04-03 4:36 ` NeilBrown
1 sibling, 0 replies; 10+ messages in thread
From: Jes Sorensen @ 2017-03-30 15:50 UTC (permalink / raw)
To: Zhilong Liu; +Cc: linux-raid
On 03/30/2017 03:38 AM, Zhilong Liu wrote:
> systemctl doesn't interpret mdadm-grow-continue@.service
> correctly due to the wrong argument provided in [service],
> it should be corrected %I as %i. Otherwise, if the service
> cannot start by systemctl and the reshap progress would be
> stuck all time when grows array from raid1 to raid5.
>
> reproduce steps:
> ./mdadm -CR /dev/md0 -l1 -b internal -n2 /dev/loop[0-1]
> ./mdadm --grow /dev/md0 -l5 -n3 -a /dev/loop2
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
> systemd/mdadm-grow-continue@.service | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
This solution looks more correct to me :)
Applied!
Thanks,
Jes
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] mdadm/grow: reshape would be stuck from raid1 to raid5
2017-03-30 7:38 ` [PATCH v1] " Zhilong Liu
2017-03-30 15:50 ` Jes Sorensen
@ 2017-04-03 4:36 ` NeilBrown
2017-04-04 5:07 ` Zhilong
1 sibling, 1 reply; 10+ messages in thread
From: NeilBrown @ 2017-04-03 4:36 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]
On Thu, Mar 30 2017, Zhilong Liu wrote:
> systemctl doesn't interpret mdadm-grow-continue@.service
> correctly due to the wrong argument provided in [service],
> it should be corrected %I as %i. Otherwise, if the service
> cannot start by systemctl and the reshap progress would be
> stuck all time when grows array from raid1 to raid5.
>
> reproduce steps:
> ./mdadm -CR /dev/md0 -l1 -b internal -n2 /dev/loop[0-1]
> ./mdadm --grow /dev/md0 -l5 -n3 -a /dev/loop2
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
> systemd/mdadm-grow-continue@.service | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/systemd/mdadm-grow-continue@.service b/systemd/mdadm-grow-continue@.service
> index 5c667d2..882bc0b 100644
> --- a/systemd/mdadm-grow-continue@.service
> +++ b/systemd/mdadm-grow-continue@.service
> @@ -10,7 +10,7 @@ Description=Manage MD Reshape on /dev/%I
> DefaultDependencies=no
>
> [Service]
> -ExecStart=BINDIR/mdadm --grow --continue /dev/%I
> +ExecStart=BINDIR/mdadm --grow --continue /dev/%i
Do you know why this makes a difference? I don't think it should.
man systemd.unit says that "%i" is the "Instance name" while "%I" is the
"Unescaped instance name".
The Instance name here is something like "md0" so there is nothing to
escape.
I would rather not change it unless we know exactly why it is broken,
and I don't find your explanation to be convincing.
NeilBrown
> StandardInput=null
> StandardOutput=null
> StandardError=null
> --
> 2.10.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] mdadm/grow: reshape would be stuck from raid1 to raid5
2017-04-03 4:36 ` NeilBrown
@ 2017-04-04 5:07 ` Zhilong
2017-04-04 5:40 ` Zhilong
0 siblings, 1 reply; 10+ messages in thread
From: Zhilong @ 2017-04-04 5:07 UTC (permalink / raw)
To: NeilBrown; +Cc: Jes.Sorensen, linux-raid
Send from iPhone
> 在 2017年4月3日,12:36,NeilBrown <neilb@suse.com> 写道:
>
>> On Thu, Mar 30 2017, Zhilong Liu wrote:
>>
>> systemctl doesn't interpret mdadm-grow-continue@.service
>> correctly due to the wrong argument provided in [service],
>> it should be corrected %I as %i. Otherwise, if the service
>> cannot start by systemctl and the reshap progress would be
>> stuck all time when grows array from raid1 to raid5.
>>
>> reproduce steps:
>> ./mdadm -CR /dev/md0 -l1 -b internal -n2 /dev/loop[0-1]
>> ./mdadm --grow /dev/md0 -l5 -n3 -a /dev/loop2
>>
>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>> ---
>> systemd/mdadm-grow-continue@.service | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/systemd/mdadm-grow-continue@.service b/systemd/mdadm-grow-continue@.service
>> index 5c667d2..882bc0b 100644
>> --- a/systemd/mdadm-grow-continue@.service
>> +++ b/systemd/mdadm-grow-continue@.service
>> @@ -10,7 +10,7 @@ Description=Manage MD Reshape on /dev/%I
>> DefaultDependencies=no
>>
>> [Service]
>> -ExecStart=BINDIR/mdadm --grow --continue /dev/%I
>> +ExecStart=BINDIR/mdadm --grow --continue /dev/%i
>
> Do you know why this makes a difference? I don't think it should.
> man systemd.unit says that "%i" is the "Instance name" while "%I" is the
> "Unescaped instance name".
>
> The Instance name here is something like "md0" so there is nothing to
> escape.
>
> I would rather not change it unless we know exactly why it is broken,
> and I don't find your explanation to be convincing.
>
Exactly, you're correct, in this case, %i and %I are the same. The root cause is the ExecStart part, all the path name should be verified by systemd-escape,such as:
/sbin/mdadm should be corrected as sbin-mdadm, and /dev/%I should be -dev-%I. Thus I'm sorry for this patch, I do agree with you not to change it. And say sorry to Jes.
Thanks
-zhilong
> NeilBrown
>
>
>> StandardInput=null
>> StandardOutput=null
>> StandardError=null
>> --
>> 2.10.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] mdadm/grow: reshape would be stuck from raid1 to raid5
2017-04-04 5:07 ` Zhilong
@ 2017-04-04 5:40 ` Zhilong
2017-04-04 6:30 ` NeilBrown
0 siblings, 1 reply; 10+ messages in thread
From: Zhilong @ 2017-04-04 5:40 UTC (permalink / raw)
To: NeilBrown; +Cc: Jes.Sorensen, linux-raid
Send from iPhone
> 在 2017年4月4日,13:07,Zhilong <zlliu@suse.com> 写道:
>
>
>
> Send from iPhone
>
>>> 在 2017年4月3日,12:36,NeilBrown <neilb@suse.com> 写道:
>>>
>>> On Thu, Mar 30 2017, Zhilong Liu wrote:
>>>
>>> systemctl doesn't interpret mdadm-grow-continue@.service
>>> correctly due to the wrong argument provided in [service],
>>> it should be corrected %I as %i. Otherwise, if the service
>>> cannot start by systemctl and the reshap progress would be
>>> stuck all time when grows array from raid1 to raid5.
>>>
>>> reproduce steps:
>>> ./mdadm -CR /dev/md0 -l1 -b internal -n2 /dev/loop[0-1]
>>> ./mdadm --grow /dev/md0 -l5 -n3 -a /dev/loop2
>>>
>>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>>> ---
>>> systemd/mdadm-grow-continue@.service | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/systemd/mdadm-grow-continue@.service b/systemd/mdadm-grow-continue@.service
>>> index 5c667d2..882bc0b 100644
>>> --- a/systemd/mdadm-grow-continue@.service
>>> +++ b/systemd/mdadm-grow-continue@.service
>>> @@ -10,7 +10,7 @@ Description=Manage MD Reshape on /dev/%I
>>> DefaultDependencies=no
>>>
>>> [Service]
>>> -ExecStart=BINDIR/mdadm --grow --continue /dev/%I
>>> +ExecStart=BINDIR/mdadm --grow --continue /dev/%i
>>
>> Do you know why this makes a difference? I don't think it should.
>> man systemd.unit says that "%i" is the "Instance name" while "%I" is the
>> "Unescaped instance name".
>>
>> The Instance name here is something like "md0" so there is nothing to
>> escape.
>>
>> I would rather not change it unless we know exactly why it is broken,
>> and I don't find your explanation to be convincing.
>>
>
> Exactly, you're correct, in this case, %i and %I are the same. The root cause is the ExecStart part, all the path name should be verified by systemd-escape,such as:
> /sbin/mdadm should be corrected as sbin-mdadm, and /dev/%I should be -dev-%I. Thus I'm sorry for this patch, I do agree with you not to change it. And say sorry to Jes.
>
How about modifying this patch as:
ExecStart=-sbin-mdadm --grow --continue -dev-%I
Thanks,
Zhilong
> Thanks
> -zhilong
>
>> NeilBrown
>>
>>
>>> StandardInput=null
>>> StandardOutput=null
>>> StandardError=null
>>> --
>>> 2.10.2
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] mdadm/grow: reshape would be stuck from raid1 to raid5
2017-04-04 5:40 ` Zhilong
@ 2017-04-04 6:30 ` NeilBrown
2017-04-05 15:37 ` jes.sorensen
0 siblings, 1 reply; 10+ messages in thread
From: NeilBrown @ 2017-04-04 6:30 UTC (permalink / raw)
To: Zhilong; +Cc: Jes.Sorensen, linux-raid
[-- Attachment #1: Type: text/plain, Size: 2882 bytes --]
On Tue, Apr 04 2017, Zhilong wrote:
> Send from iPhone
>
>> 在 2017年4月4日,13:07,Zhilong <zlliu@suse.com> 写道:
>>
>>
>>
>> Send from iPhone
>>
>>>> 在 2017年4月3日,12:36,NeilBrown <neilb@suse.com> 写道:
>>>>
>>>> On Thu, Mar 30 2017, Zhilong Liu wrote:
>>>>
>>>> systemctl doesn't interpret mdadm-grow-continue@.service
>>>> correctly due to the wrong argument provided in [service],
>>>> it should be corrected %I as %i. Otherwise, if the service
>>>> cannot start by systemctl and the reshap progress would be
>>>> stuck all time when grows array from raid1 to raid5.
>>>>
>>>> reproduce steps:
>>>> ./mdadm -CR /dev/md0 -l1 -b internal -n2 /dev/loop[0-1]
>>>> ./mdadm --grow /dev/md0 -l5 -n3 -a /dev/loop2
>>>>
>>>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>>>> ---
>>>> systemd/mdadm-grow-continue@.service | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/systemd/mdadm-grow-continue@.service b/systemd/mdadm-grow-continue@.service
>>>> index 5c667d2..882bc0b 100644
>>>> --- a/systemd/mdadm-grow-continue@.service
>>>> +++ b/systemd/mdadm-grow-continue@.service
>>>> @@ -10,7 +10,7 @@ Description=Manage MD Reshape on /dev/%I
>>>> DefaultDependencies=no
>>>>
>>>> [Service]
>>>> -ExecStart=BINDIR/mdadm --grow --continue /dev/%I
>>>> +ExecStart=BINDIR/mdadm --grow --continue /dev/%i
>>>
>>> Do you know why this makes a difference? I don't think it should.
>>> man systemd.unit says that "%i" is the "Instance name" while "%I" is the
>>> "Unescaped instance name".
>>>
>>> The Instance name here is something like "md0" so there is nothing to
>>> escape.
>>>
>>> I would rather not change it unless we know exactly why it is broken,
>>> and I don't find your explanation to be convincing.
>>>
>>
>> Exactly, you're correct, in this case, %i and %I are the same. The root cause is the ExecStart part, all the path name should be verified by systemd-escape,such as:
>> /sbin/mdadm should be corrected as sbin-mdadm, and /dev/%I should be -dev-%I. Thus I'm sorry for this patch, I do agree with you not to change it. And say sorry to Jes.
>>
>
> How about modifying this patch as:
>
> ExecStart=-sbin-mdadm --grow --continue -dev-%I
>
Why do you think anything needs changing here?
I have a tumbleweed install with the standard
mdadm-grow-continue@.server
file. i.e.
ExecStart=/sbin/mdadm --grow --continue /dev/%I
I run
strace -f -s 1000 -p 1 -o /tmp/strace
in one window, then
systemctl start mdadm-grow-continue@md0.service
in another.
Then
grep execve /tmp/strace
shows:
18680 execve("/sbin/mdadm", ["/sbin/mdadm", "--grow", "--continue", "/dev/md0"], [/* 3 vars */] <unfinished ...>
which shows that mdadm is being correctly.
There is nothing to fix here that I can see.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] mdadm/grow: reshape would be stuck from raid1 to raid5
2017-04-04 6:30 ` NeilBrown
@ 2017-04-05 15:37 ` jes.sorensen
0 siblings, 0 replies; 10+ messages in thread
From: jes.sorensen @ 2017-04-05 15:37 UTC (permalink / raw)
To: NeilBrown; +Cc: Zhilong, linux-raid
NeilBrown <neilb@suse.com> writes:
> On Tue, Apr 04 2017, Zhilong wrote:
>
>> Send from iPhone
>>
>>> 在 2017年4月4日,13:07,Zhilong <zlliu@suse.com> 写道:
>>>
>>>
>>>
>>> Send from iPhone
>>>
>>>>> 在 2017年4月3日,12:36,NeilBrown <neilb@suse.com> 写道:
>>>>>
>>>>> On Thu, Mar 30 2017, Zhilong Liu wrote:
>>>>>
>>>>> systemctl doesn't interpret mdadm-grow-continue@.service
>>>>> correctly due to the wrong argument provided in [service],
>>>>> it should be corrected %I as %i. Otherwise, if the service
>>>>> cannot start by systemctl and the reshap progress would be
>>>>> stuck all time when grows array from raid1 to raid5.
>>>>>
>>>>> reproduce steps:
>>>>> ./mdadm -CR /dev/md0 -l1 -b internal -n2 /dev/loop[0-1]
>>>>> ./mdadm --grow /dev/md0 -l5 -n3 -a /dev/loop2
>>>>>
>>>>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>>>>> ---
>>>>> systemd/mdadm-grow-continue@.service | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/systemd/mdadm-grow-continue@.service
>>>>> b/systemd/mdadm-grow-continue@.service
>>>>> index 5c667d2..882bc0b 100644
>>>>> --- a/systemd/mdadm-grow-continue@.service
>>>>> +++ b/systemd/mdadm-grow-continue@.service
>>>>> @@ -10,7 +10,7 @@ Description=Manage MD Reshape on /dev/%I
>>>>> DefaultDependencies=no
>>>>>
>>>>> [Service]
>>>>> -ExecStart=BINDIR/mdadm --grow --continue /dev/%I
>>>>> +ExecStart=BINDIR/mdadm --grow --continue /dev/%i
>>>>
>>>> Do you know why this makes a difference? I don't think it should.
>>>> man systemd.unit says that "%i" is the "Instance name" while "%I" is the
>>>> "Unescaped instance name".
>>>>
>>>> The Instance name here is something like "md0" so there is nothing to
>>>> escape.
>>>>
>>>> I would rather not change it unless we know exactly why it is broken,
>>>> and I don't find your explanation to be convincing.
>>>>
>>>
>>> Exactly, you're correct, in this case, %i and %I are the same. The
>>> root cause is the ExecStart part, all the path name should be
>>> verified by systemd-escape,such as:
>>> /sbin/mdadm should be corrected as sbin-mdadm, and /dev/%I should
>>> be -dev-%I. Thus I'm sorry for this patch, I do agree with you not
>>> to change it. And say sorry to Jes.
>>>
>>
>> How about modifying this patch as:
>>
>> ExecStart=-sbin-mdadm --grow --continue -dev-%I
>>
>
> Why do you think anything needs changing here?
>
> I have a tumbleweed install with the standard
> mdadm-grow-continue@.server
> file. i.e.
>
> ExecStart=/sbin/mdadm --grow --continue /dev/%I
>
> I run
> strace -f -s 1000 -p 1 -o /tmp/strace
>
> in one window, then
>
> systemctl start mdadm-grow-continue@md0.service
>
> in another.
>
> Then
>
> grep execve /tmp/strace
>
> shows:
>
> 18680 execve("/sbin/mdadm", ["/sbin/mdadm", "--grow", "--continue", "/dev/md0"], [/* 3 vars */] <unfinished ...>
>
> which shows that mdadm is being correctly.
>
> There is nothing to fix here that I can see.
Unless I see some convincing arguments, I am going to revert this
patch.
Jes
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-04-05 15:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-20 5:20 [PATCH] mdadm/grow: reshape would be stuck from raid1 to raid5 Zhilong Liu
2017-03-27 22:10 ` jes.sorensen
2017-03-28 14:03 ` Liu Zhilong
2017-03-30 7:38 ` [PATCH v1] " Zhilong Liu
2017-03-30 15:50 ` Jes Sorensen
2017-04-03 4:36 ` NeilBrown
2017-04-04 5:07 ` Zhilong
2017-04-04 5:40 ` Zhilong
2017-04-04 6:30 ` NeilBrown
2017-04-05 15:37 ` jes.sorensen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.