* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).