[PATCH] Parallel make bug in Makefile of mdadm-3.3.1 (corrupted udev rules and/or systemd services)

[PATCH] Parallel make bug in Makefile of mdadm-3.3.1 (corrupted udev rules and/or systemd services)

[Samuli Suominen]

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

If you run with:

$ export MAKEFLAGS="-j9"
$ make install install-systemd

As in, combine "install" and "install-systemd" in the same command using
parallel make,
the content of .service files could end up in .rules, or otherway around
because GNU
make fill run them in parallel and the commands might ran at the same
time, and both
use same variables $file and same temporary files .install.tmp

This was reported at,
https://bugs.gentoo.org/show_bug.cgi?id=517218

The immediate workaround is,

$ export MAKEFLAGS="-j9"
$ make install
$ make install-systemd

And the attach patch makes it full proof so they can be ran at the same
line.

Thanks,
Samuli

[-- Attachment #2: parallel-make.patch --]
[-- Type: text/x-patch, Size: 2579 bytes --]

diff -ur mdadm-3.3.1.orig/Makefile mdadm-3.3.1/Makefile
--- mdadm-3.3.1.orig/Makefile	2014-06-05 09:45:56.000000000 +0300
+++ mdadm-3.3.1/Makefile	2014-07-17 09:52:51.670100989 +0300
@@ -281,26 +281,26 @@
 	$(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
 
 install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules
-	@for file in 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
-	do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp && \
-	   echo $(INSTALL) -D -m 644 udev-$${file#??-} $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
-	   $(INSTALL) -D -m 644 .install.tmp $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
-	   rm -f .install.tmp; \
+	@for file1 in 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
+	do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file1#??-} > .install.tmp.1 && \
+	   echo $(INSTALL) -D -m 644 udev-$${file1#??-} $(DESTDIR)$(UDEVDIR)/rules.d/$$file1 ; \
+	   $(INSTALL) -D -m 644 .install.tmp.1 $(DESTDIR)$(UDEVDIR)/rules.d/$$file1 ; \
+	   rm -f .install.tmp.1; \
 	done
 
 install-systemd: systemd/mdmon@.service
-	@for file in mdmon@.service mdmonitor.service mdadm-last-resort@.timer \
+	@for file2 in mdmon@.service mdmonitor.service mdadm-last-resort@.timer \
 		mdadm-last-resort@.service ; \
-	do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp && \
-	   echo $(INSTALL) -D -m 644 systemd/$$file $(DESTDIR)$(SYSTEMD_DIR)/$$file ; \
-	   $(INSTALL) -D -m 644 .install.tmp $(DESTDIR)$(SYSTEMD_DIR)/$$file ; \
-	   rm -f .install.tmp; \
+	do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file2 > .install.tmp.2 && \
+	   echo $(INSTALL) -D -m 644 systemd/$$file2 $(DESTDIR)$(SYSTEMD_DIR)/$$file2 ; \
+	   $(INSTALL) -D -m 644 .install.tmp.2 $(DESTDIR)$(SYSTEMD_DIR)/$$file2 ; \
+	   rm -f .install.tmp.2; \
 	done
-	@for file in mdadm.shutdown ; \
-	do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp && \
-	   echo $(INSTALL) -D -m 755  systemd/$$file $(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file ; \
-	   $(INSTALL) -D -m 755  .install.tmp $(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file ; \
-	   rm -f .install.tmp; \
+	@for file3 in mdadm.shutdown ; \
+	do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file3 > .install.tmp.3 && \
+	   echo $(INSTALL) -D -m 755  systemd/$$file3 $(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file3 ; \
+	   $(INSTALL) -D -m 755  .install.tmp.3 $(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file3 ; \
+	   rm -f .install.tmp.3; \
 	done
 	if [ -f /etc/SuSE-release -o -n "$(SUSE)" ] ;then $(INSTALL) -D -m 755 systemd/SUSE-mdadm_env.sh $(DESTDIR)$(SYSTEMD_DIR)/../scripts/mdadm_env.sh ;fi
 

Re: [PATCH] Parallel make bug in Makefile of mdadm-3.3.1 (corrupted udev rules and/or systemd services)

[Samuli Suominen]

On 17/07/14 10:30, Samuli Suominen wrote:
> If you run with:
>
> $ export MAKEFLAGS="-j9"
> $ make install install-systemd
>
> As in, combine "install" and "install-systemd" in the same command using
> parallel make,
> the content of .service files could end up in .rules, or otherway around
> because GNU
> make fill

s/fill/will/

>
> And the attach

s/attach/attached/

Sorry for the embarrasing typing errors in the first post.

Re: [PATCH] Parallel make bug in Makefile of mdadm-3.3.1 (corrupted udev rules and/or systemd services)

[NeilBrown]

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

On Thu, 17 Jul 2014 10:30:05 +0300 Samuli Suominen <ssuominen@gentoo.org>
wrote:

> If you run with:
> 
> $ export MAKEFLAGS="-j9"
> $ make install install-systemd
> 
> As in, combine "install" and "install-systemd" in the same command using
> parallel make,
> the content of .service files could end up in .rules, or otherway around
> because GNU
> make fill run them in parallel and the commands might ran at the same
> time, and both
> use same variables $file and same temporary files .install.tmp
> 
> This was reported at,
> https://bugs.gentoo.org/show_bug.cgi?id=517218
> 
> The immediate workaround is,
> 
> $ export MAKEFLAGS="-j9"
> $ make install
> $ make install-systemd
> 
> And the attach patch makes it full proof so they can be ran at the same
> line.
> 
> Thanks,
> Samuli

Hi,
 thanks for the bug report and the patch!

I don't think it is necessary to change the name for the variable (file ->
file1, file2 or file3).  Just leave it as 'file'.
However changing the name is important.  Could you send a patch which just
does that, and I'll apply it.

Also if the patch could be included inline in the mail, rather than as an
attachment, that would be nice.

(But if you don't want to bother, I'll make the fix anyway and credit you).

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] Parallel make bug in Makefile of mdadm-3.3.1 (corrupted udev rules and/or systemd services)

[Samuli Suominen]

On 17/07/14 11:25, NeilBrown wrote:
> On Thu, 17 Jul 2014 10:30:05 +0300 Samuli Suominen <ssuominen@gentoo.org>
> wrote:
>
>> If you run with:
>>
>> $ export MAKEFLAGS="-j9"
>> $ make install install-systemd
>>
>> As in, combine "install" and "install-systemd" in the same command using
>> parallel make,
>> the content of .service files could end up in .rules, or otherway around
>> because GNU
>> make fill run them in parallel and the commands might ran at the same
>> time, and both
>> use same variables $file and same temporary files .install.tmp
>>
>> This was reported at,
>> https://bugs.gentoo.org/show_bug.cgi?id=517218
>>
>> The immediate workaround is,
>>
>> $ export MAKEFLAGS="-j9"
>> $ make install
>> $ make install-systemd
>>
>> And the attach patch makes it full proof so they can be ran at the same
>> line.
>>
>> Thanks,
>> Samuli
> Hi,
>  thanks for the bug report and the patch!
>
> I don't think it is necessary to change the name for the variable (file ->
> file1, file2 or file3).  Just leave it as 'file'.
> However changing the name is important.  Could you send a patch which just
> does that, and I'll apply it.
>
> Also if the patch could be included inline in the mail, rather than as an
> attachment, that would be nice.
>
> (But if you don't want to bother, I'll make the fix anyway and credit you).
>
> Thanks,
> NeilBrown

Thanks for the review, I wasn't 100% sure which I should be changing,
the variable, or the filename,
so I changed both.
So, you are most likely correct.

Here is the patch that only changes the filename,

--- mdadm-3.3.1.orig/Makefile
+++ mdadm-3.3.1/Makefile
@@ -282,25 +282,25 @@
 
 install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules
     @for file in 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
-    do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp && \
+    do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp.1
&& \
        echo $(INSTALL) -D -m 644 udev-$${file#??-}
$(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
-       $(INSTALL) -D -m 644 .install.tmp
$(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
-       rm -f .install.tmp; \
+       $(INSTALL) -D -m 644 .install.tmp.1
$(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
+       rm -f .install.tmp.1; \
     done
 
 install-systemd: systemd/mdmon@.service
     @for file in mdmon@.service mdmonitor.service
mdadm-last-resort@.timer \
         mdadm-last-resort@.service ; \
-    do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp && \
+    do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp.2 && \
        echo $(INSTALL) -D -m 644 systemd/$$file
$(DESTDIR)$(SYSTEMD_DIR)/$$file ; \
-       $(INSTALL) -D -m 644 .install.tmp
$(DESTDIR)$(SYSTEMD_DIR)/$$file ; \
-       rm -f .install.tmp; \
+       $(INSTALL) -D -m 644 .install.tmp.2
$(DESTDIR)$(SYSTEMD_DIR)/$$file ; \
+       rm -f .install.tmp.2; \
     done
     @for file in mdadm.shutdown ; \
-    do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp && \
+    do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp.3 && \
        echo $(INSTALL) -D -m 755  systemd/$$file
$(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file ; \
-       $(INSTALL) -D -m 755  .install.tmp
$(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file ; \
-       rm -f .install.tmp; \
+       $(INSTALL) -D -m 755  .install.tmp.3
$(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file ; \
+       rm -f .install.tmp.3; \
     done
     if [ -f /etc/SuSE-release -o -n "$(SUSE)" ] ;then $(INSTALL) -D -m
755 systemd/SUSE-mdadm_env.sh
$(DESTDIR)$(SYSTEMD_DIR)/../scripts/mdadm_env.sh ;fi

Re: [PATCH] Parallel make bug in Makefile of mdadm-3.3.1 (corrupted udev rules and/or systemd services)

[Samuli Suominen]

This new patch, that only changes the filename, was tested and verified
to fix the problem in the downstream Gentoo bug,
https://bugs.gentoo.org/show_bug.cgi?id=517218#c18
So you were right :)

On 17/07/14 11:31, Samuli Suominen wrote:
> On 17/07/14 11:25, NeilBrown wrote:
>> On Thu, 17 Jul 2014 10:30:05 +0300 Samuli Suominen <ssuominen@gentoo.org>
>> wrote:
>>
>>> If you run with:
>>>
>>> $ export MAKEFLAGS="-j9"
>>> $ make install install-systemd
>>>
>>> As in, combine "install" and "install-systemd" in the same command using
>>> parallel make,
>>> the content of .service files could end up in .rules, or otherway around
>>> because GNU
>>> make fill run them in parallel and the commands might ran at the same
>>> time, and both
>>> use same variables $file and same temporary files .install.tmp
>>>
>>> This was reported at,
>>> https://bugs.gentoo.org/show_bug.cgi?id=517218
>>>
>>> The immediate workaround is,
>>>
>>> $ export MAKEFLAGS="-j9"
>>> $ make install
>>> $ make install-systemd
>>>
>>> And the attach patch makes it full proof so they can be ran at the same
>>> line.
>>>
>>> Thanks,
>>> Samuli
>> Hi,
>>  thanks for the bug report and the patch!
>>
>> I don't think it is necessary to change the name for the variable (file ->
>> file1, file2 or file3).  Just leave it as 'file'.
>> However changing the name is important.  Could you send a patch which just
>> does that, and I'll apply it.
>>
>> Also if the patch could be included inline in the mail, rather than as an
>> attachment, that would be nice.
>>
>> (But if you don't want to bother, I'll make the fix anyway and credit you).
>>
>> Thanks,
>> NeilBrown
> Thanks for the review, I wasn't 100% sure which I should be changing,
> the variable, or the filename,
> so I changed both.
> So, you are most likely correct.
>
> Here is the patch that only changes the filename,
>
> --- mdadm-3.3.1.orig/Makefile
> +++ mdadm-3.3.1/Makefile
> @@ -282,25 +282,25 @@
>  
>  install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules
>      @for file in 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
> -    do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp && \
> +    do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp.1
> && \
>         echo $(INSTALL) -D -m 644 udev-$${file#??-}
> $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
> -       $(INSTALL) -D -m 644 .install.tmp
> $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
> -       rm -f .install.tmp; \
> +       $(INSTALL) -D -m 644 .install.tmp.1
> $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
> +       rm -f .install.tmp.1; \
>      done
>  
>  install-systemd: systemd/mdmon@.service
>      @for file in mdmon@.service mdmonitor.service
> mdadm-last-resort@.timer \
>          mdadm-last-resort@.service ; \
> -    do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp && \
> +    do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp.2 && \
>         echo $(INSTALL) -D -m 644 systemd/$$file
> $(DESTDIR)$(SYSTEMD_DIR)/$$file ; \
> -       $(INSTALL) -D -m 644 .install.tmp
> $(DESTDIR)$(SYSTEMD_DIR)/$$file ; \
> -       rm -f .install.tmp; \
> +       $(INSTALL) -D -m 644 .install.tmp.2
> $(DESTDIR)$(SYSTEMD_DIR)/$$file ; \
> +       rm -f .install.tmp.2; \
>      done
>      @for file in mdadm.shutdown ; \
> -    do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp && \
> +    do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp.3 && \
>         echo $(INSTALL) -D -m 755  systemd/$$file
> $(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file ; \
> -       $(INSTALL) -D -m 755  .install.tmp
> $(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file ; \
> -       rm -f .install.tmp; \
> +       $(INSTALL) -D -m 755  .install.tmp.3
> $(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file ; \
> +       rm -f .install.tmp.3; \
>      done
>      if [ -f /etc/SuSE-release -o -n "$(SUSE)" ] ;then $(INSTALL) -D -m
> 755 systemd/SUSE-mdadm_env.sh
> $(DESTDIR)$(SYSTEMD_DIR)/../scripts/mdadm_env.sh ;fi
>
> --
> 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