| Message ID | 1412930678.10650.20.camel@citrix.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Oct 10, 2014 at 09:44:38AM +0100, Ian Campbell wrote: [...] > Should subdirs-install depend on subdirs-all perhaps? > > commit 66bb8b04f0032ddf0aa007b0850be1ec15477d60 > Author: Ian Campbell <ian.campbell@citrix.com> > Date: Fri Oct 10 09:35:34 2014 +0100 > > Revert "tools/hotplug: fix race during xen.conf creation" > > This reverts commit eac3f5122fd4769b2885d8ad78bcbcf5df2472c1. > > The "all" target should never depend on "install", it is supposed to only build > not install. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > diff --git a/tools/hotplug/Linux/systemd/Makefile b/tools/hotplug/Linux/systemd/Makefile > index 6950d24..9c58b33 100644 > --- a/tools/hotplug/Linux/systemd/Makefile > +++ b/tools/hotplug/Linux/systemd/Makefile > @@ -21,11 +21,10 @@ ALL_XEN_SYSTEMD = $(XEN_SYSTEMD_MODULES) \ > $(XEN_SYSTEMD_SERVICE) > > .PHONY: all > -all: install > +all: $(ALL_XEN_SYSTEMD) > > .PHONY: clean > clean: > - rm -f $(ALL_XEN_SYSTEMD) > Now clean target just becomes empty... Wei.
On Fri, 2014-10-10 at 10:19 +0100, Wei Liu wrote: > On Fri, Oct 10, 2014 at 09:44:38AM +0100, Ian Campbell wrote: > [...] > > Should subdirs-install depend on subdirs-all perhaps? > > > > commit 66bb8b04f0032ddf0aa007b0850be1ec15477d60 > > Author: Ian Campbell <ian.campbell@citrix.com> > > Date: Fri Oct 10 09:35:34 2014 +0100 > > > > Revert "tools/hotplug: fix race during xen.conf creation" > > > > This reverts commit eac3f5122fd4769b2885d8ad78bcbcf5df2472c1. > > > > The "all" target should never depend on "install", it is supposed to only build > > not install. > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > > > diff --git a/tools/hotplug/Linux/systemd/Makefile b/tools/hotplug/Linux/systemd/Makefile > > index 6950d24..9c58b33 100644 > > --- a/tools/hotplug/Linux/systemd/Makefile > > +++ b/tools/hotplug/Linux/systemd/Makefile > > @@ -21,11 +21,10 @@ ALL_XEN_SYSTEMD = $(XEN_SYSTEMD_MODULES) \ > > $(XEN_SYSTEMD_SERVICE) > > > > .PHONY: all > > -all: install > > +all: $(ALL_XEN_SYSTEMD) > > > > .PHONY: clean > > clean: > > - rm -f $(ALL_XEN_SYSTEMD) > > > > Now clean target just becomes empty... ... the perils of lumping multiple changes into a single patch... Ian.
On Fri, Oct 10, Ian Campbell wrote: > I expect the actual problem is that the buildsystem is recusing into > this directory twice simultaneously for "all" and "install" at the same > time. That seems likely to be an issue with the Makefile in the parent > directory. > > Should subdirs-install depend on subdirs-all perhaps? Anthony, how do you invoke make? While I have seen it often right now I'm unable to reproduce. There are never 'all' and 'install' targets in the log. I think my script always called make rpmball. I think only the install rule should depend on ALL_XEN_SYSTEMD because in the end thats the only place that matters. There is nothing to build, and the single xen.conf file could also be generated on the fly during install. But still it would be nice to know what the root cause is. Olaf
On Fri, Oct 10, Ian Campbell wrote: > On Fri, 2014-10-10 at 10:19 +0100, Wei Liu wrote: > > On Fri, Oct 10, 2014 at 09:44:38AM +0100, Ian Campbell wrote: > > [...] > > > Should subdirs-install depend on subdirs-all perhaps? > > > > > > commit 66bb8b04f0032ddf0aa007b0850be1ec15477d60 > > > Author: Ian Campbell <ian.campbell@citrix.com> > > > Date: Fri Oct 10 09:35:34 2014 +0100 > > > > > > Revert "tools/hotplug: fix race during xen.conf creation" > > > > > > This reverts commit eac3f5122fd4769b2885d8ad78bcbcf5df2472c1. > > > > > > The "all" target should never depend on "install", it is supposed to only build > > > not install. > > > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > > > > > diff --git a/tools/hotplug/Linux/systemd/Makefile b/tools/hotplug/Linux/systemd/Makefile > > > index 6950d24..9c58b33 100644 > > > --- a/tools/hotplug/Linux/systemd/Makefile > > > +++ b/tools/hotplug/Linux/systemd/Makefile > > > @@ -21,11 +21,10 @@ ALL_XEN_SYSTEMD = $(XEN_SYSTEMD_MODULES) \ > > > $(XEN_SYSTEMD_SERVICE) > > > > > > .PHONY: all > > > -all: install > > > +all: $(ALL_XEN_SYSTEMD) > > > > > > .PHONY: clean > > > clean: > > > - rm -f $(ALL_XEN_SYSTEMD) > > > > > > > Now clean target just becomes empty... > > ... the perils of lumping multiple changes into a single patch... And it was wrong anyway because it will wipe everything in that dir. If at all, it should have been "rm -f $(XEN_SYSTEMD_MODULES)" to wipe the generated files. Olaf
On Fri, Oct 10, 2014 at 11:59:08AM +0200, Olaf Hering wrote: > On Fri, Oct 10, Ian Campbell wrote: > > > I expect the actual problem is that the buildsystem is recusing into > > this directory twice simultaneously for "all" and "install" at the same > > time. That seems likely to be an issue with the Makefile in the parent > > directory. > > > > Should subdirs-install depend on subdirs-all perhaps? > > Anthony, how do you invoke make? While I have seen it often right now > I'm unable to reproduce. There are never 'all' and 'install' targets in > the log. I think my script always called make rpmball. There are a lot of things in my build script, but it should be done to this: git clone ./configure --enable-systemd --other-stuff export MAKEFLAGS="-j24" edit .config to use local git tree make And later in the process, a `make install` is called but the faillure is before that. > I think only the install rule should depend on ALL_XEN_SYSTEMD because > in the end thats the only place that matters. There is nothing to build, > and the single xen.conf file could also be generated on the fly during > install. > > But still it would be nice to know what the root cause is. It look like hotplug stuff are done like that: make[2]: Entering directory '/build/xen-unstable/src/xen-unstable/tools' make -C hotplug install then calling make install on other subdir. And after entering tools/Linux, I can see both rules "all" and "install" been called on systemd directory (unfortunatly, at the same time on this machine). I think the problem is in tools/Linux/Makefile, where we have: all: subdirs-all install: all subdirs-install Maybe the "install" rules should depend only on the things it need to install, and not an "all".
On Fri, Oct 10, Anthony PERARD wrote: > I think the problem is in tools/Linux/Makefile, where we have: > all: subdirs-all > install: all subdirs-install > > Maybe the "install" rules should depend only on the things it need to > install, and not an "all". Does removing all fix the failure for you? Olaf
On Fri, Oct 10, Anthony PERARD wrote: > Yes, it does, no more weird two calls of make on the systemd/Makefile > (with all and install). There is only one call left: "make -C systemd > install". And make does not fail to make xen.conf. I ran into this as well just now, and removing all works for me as well. > From 41cfaf5318cb0346e31a8e97939b8805145eb182 Mon Sep 17 00:00:00 2001 > From: Anthony PERARD <anthony.perard@citrix.com> > Date: Fri, 10 Oct 2014 12:22:34 +0100 > Subject: [PATCH] tools/hotplug/Linux: Removing of "all" dependency from > "install" rule. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Tested-by: Olaf Hering <olaf@aepfle.de> > --- > tools/hotplug/Linux/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile > index 8cdaa9a..1706c05 100644 > --- a/tools/hotplug/Linux/Makefile > +++ b/tools/hotplug/Linux/Makefile > @@ -43,7 +43,7 @@ all: subdirs-all > build: > > .PHONY: install > -install: all install-initd install-scripts install-udev subdirs-install > +install: install-initd install-scripts install-udev subdirs-install > > # See docs/misc/distro_mapping.txt for INITD_DIR location > .PHONY: install-initd
On Fri, 2014-10-10 at 16:28 +0200, Olaf Hering wrote: > On Fri, Oct 10, Anthony PERARD wrote: > > > Yes, it does, no more weird two calls of make on the systemd/Makefile > > (with all and install). There is only one call left: "make -C systemd > > install". And make does not fail to make xen.conf. > > I ran into this as well just now, and removing all works for me as well. > > > From 41cfaf5318cb0346e31a8e97939b8805145eb182 Mon Sep 17 00:00:00 2001 > > From: Anthony PERARD <anthony.perard@citrix.com> > > Date: Fri, 10 Oct 2014 12:22:34 +0100 > > Subject: [PATCH] tools/hotplug/Linux: Removing of "all" dependency from > > "install" rule. > > > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > Tested-by: Olaf Hering <olaf@aepfle.de> Great. Could someone supply a bit of explanatory text for the commit message please. Do these indicate a similar problem: tools/hotplug/NetBSD/Makefile:install: all install-scripts install-rcd tools/hotplug/common/Makefile:install: all install-scripts ? I think not because no recursion is included. Ian.
On Fri, Oct 10, 2014 at 03:31:10PM +0100, Ian Campbell wrote: > On Fri, 2014-10-10 at 16:28 +0200, Olaf Hering wrote: > > On Fri, Oct 10, Anthony PERARD wrote: > > > > > Yes, it does, no more weird two calls of make on the systemd/Makefile > > > (with all and install). There is only one call left: "make -C systemd > > > install". And make does not fail to make xen.conf. > > > > I ran into this as well just now, and removing all works for me as well. > > > > > From 41cfaf5318cb0346e31a8e97939b8805145eb182 Mon Sep 17 00:00:00 2001 > > > From: Anthony PERARD <anthony.perard@citrix.com> > > > Date: Fri, 10 Oct 2014 12:22:34 +0100 > > > Subject: [PATCH] tools/hotplug/Linux: Removing of "all" dependency from > > > "install" rule. > > > > > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > > > Tested-by: Olaf Hering <olaf@aepfle.de> > > Great. Could someone supply a bit of explanatory text for the commit > message please. The "install" rules depends on both "all" and "subdirs-install" and "all" depends on "subdirs-all". This leads the "install" rules to call both "subdirs-all" and "subdirs-install" which create a race with two concurrent `make` within the same directory (systemd) trying to make the same things (xen.conf) and failing. Ultimatly, "install" should only depend on the things it needs to install, and not on "all". > Do these indicate a similar problem: > tools/hotplug/NetBSD/Makefile:install: all install-scripts install-rcd > tools/hotplug/common/Makefile:install: all install-scripts > ? > > I think not because no recursion is included. I think this could be an issue, install does not need all. But since there is no subdirs involves, there is no problem for now. Shall I update the patch to change all "install" rules of tools/hotplug/*/Makefile ?
Anthony PERARD writes ("[PATCH V2] tools/hotplug: Removing of "all" dependency from "install" rule."):
> The "install" rules depends on both "all" and "subdirs-install" and
> "all" depends on "subdirs-all". This leads the "install" rules to call
> both "subdirs-all" and "subdirs-install" which create a race with two
> concurrent `make` within the same directory (systemd) trying to make the
> same things (xen.conf) and failing.
Thanks for the investigation. I think this demonstrates that our
approach to subdirs is, at the very least, excessively fragile. But
this fix (or workaround, if we prefer) will do.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Thanks,
Ian.
On Mon, 2014-10-13 at 16:26 +0100, Ian Jackson wrote: > Anthony PERARD writes ("[PATCH V2] tools/hotplug: Removing of "all" dependency from "install" rule."): > > The "install" rules depends on both "all" and "subdirs-install" and > > "all" depends on "subdirs-all". This leads the "install" rules to call > > both "subdirs-all" and "subdirs-install" which create a race with two > > concurrent `make` within the same directory (systemd) trying to make the > > same things (xen.conf) and failing. > > Thanks for the investigation. I think this demonstrates that our > approach to subdirs is, at the very least, excessively fragile. But > this fix (or workaround, if we prefer) will do. > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Likewise, and applied. Ian.
On Mon, 2014-10-13 at 16:26 +0100, Ian Jackson wrote: > Anthony PERARD writes ("[PATCH V2] tools/hotplug: Removing of "all" dependency from "install" rule."): > > The "install" rules depends on both "all" and "subdirs-install" and > > "all" depends on "subdirs-all". This leads the "install" rules to call > > both "subdirs-all" and "subdirs-install" which create a race with two > > concurrent `make` within the same directory (systemd) trying to make the > > same things (xen.conf) and failing. > > Thanks for the investigation. I think this demonstrates that our > approach to subdirs is, at the very least, excessively fragile. But > this fix (or workaround, if we prefer) will do. > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Likewise, and applied. Ian.
diff --git a/tools/hotplug/Linux/systemd/Makefile b/tools/hotplug/Linux/systemd/Makefile index 6950d24..9c58b33 100644 --- a/tools/hotplug/Linux/systemd/Makefile +++ b/tools/hotplug/Linux/systemd/Makefile @@ -21,11 +21,10 @@ ALL_XEN_SYSTEMD = $(XEN_SYSTEMD_MODULES) \ $(XEN_SYSTEMD_SERVICE) .PHONY: all -all: install +all: $(ALL_XEN_SYSTEMD) .PHONY: clean clean: - rm -f $(ALL_XEN_SYSTEMD) .PHONY: install install: $(ALL_XEN_SYSTEMD)