[libvirt] [PATCH 0/4] build: use subdir-objects

Automake 1.14 is annoyingly loud about warning that the future automake 2.0 will turn on subdir-objects by default. Since automake 1.9 also supports subdir-objects, the best course of action is to enable the feature. But we have to fix some problems before doing so. Also, I noticed some issues with .x files while searching for places that needed fixing. Eric Blake (4): build: avoid $(srcdir) in *_SOURCES build: use library rather than cross-directory compilation tests: check remaining .x files build: use automake subdir-objects .gitignore | 1 + cfg.mk | 3 +- configure.ac | 2 +- daemon/Makefile.am | 55 +++++++++++----- src/Makefile.am | 139 +++++++++++++++++++++++---------------- src/lock_protocol-structs | 55 ++++++++++++++++ src/lxc_monitor_protocol-structs | 16 +++++ tests/Makefile.am | 4 +- 8 files changed, 198 insertions(+), 77 deletions(-) create mode 100644 src/lock_protocol-structs create mode 100644 src/lxc_monitor_protocol-structs -- 1.8.3.1

Trying to enable automake's subdir-objects option resulted in the creation of literal directories such as src/$(srcdir)/remote/. I traced this to the fact that we had used a literal $(srcdir) in a location that later fed an automake *_SOURCES variable. This has also been reported as an automake bug: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13928 but it's better to fix our code than to wait for an automake fix. Some things to remember that affect VPATH builds, and where an in-tree build is blissfully unaware of the issues: if a VPATH build fails to find a file that was used as a prereq of any other target, then the rule for that file will expand $@ to prefer the current build dir (bad because a VPATH build on a fresh checkout will then stick $@ in the current directory instead of the desired srcdir); conversely, if a VPATH build finds the file in srcdir but decides it needs to be rebuilt, then the rule for that file will expand $@ to include the directory where it was found out-of-date (bad for an explicit listing of $(srcdir)/$@ because an incremental VPATH build will then expand srcdir twice). As we want these files to go into srcdir unconditionally, we have to massage or avoid $@ for any recipe that involves one of these files. Therefore, this patch removes all uses of $(srcdir) from any generated file name that later feeds a *_SOURCES variable, and then rewrites all the recipes to generate those files to hard-code their creation into srcdir without the use of $@. * src/Makefile.am (REMOTE_DRIVER_GENERATED): Drop $(srcdir); VPATH builds know how to find the files, and automake subdir-objects fails with it in place. (LXC_MONITOR_PROTOCOL_GENERATED, (LXC_MONITOR_GENERATED) (ACCESS_DRIVER_GENERATED, LOCK_PROTOCOL_GENERATED): Likewise. (*_client_bodies.h): Hard-code rules to write into srcdir, as VPATH tries to build $@ locally if missing. (util/virkeymaps.h): Likewise. (lxc/lxc_monitor_dispatch.h): Likewise. (access/viraccessapi*): Likewise. (locking/lock_daemon_dispatch_stubs.h): Likewise. * daemon/Makeflie.am (DAEMON_GENERATED, remote_dispatch.h): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> fixup DAEMON_GENERATED --- daemon/Makefile.am | 23 ++++++----- src/Makefile.am | 109 ++++++++++++++++++++++++++++++----------------------- 2 files changed, 74 insertions(+), 58 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 90689f8..e0b8744 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -29,10 +29,10 @@ INCLUDES = \ CLEANFILES = -DAEMON_GENERATED = \ - $(srcdir)/remote_dispatch.h \ - $(srcdir)/lxc_dispatch.h \ - $(srcdir)/qemu_dispatch.h \ +DAEMON_GENERATED = \ + remote_dispatch.h \ + lxc_dispatch.h \ + qemu_dispatch.h \ $(NULL) DAEMON_SOURCES = \ @@ -75,20 +75,23 @@ REMOTE_PROTOCOL = $(top_srcdir)/src/remote/remote_protocol.x LXC_PROTOCOL = $(top_srcdir)/src/remote/lxc_protocol.x QEMU_PROTOCOL = $(top_srcdir)/src/remote/qemu_protocol.x -$(srcdir)/remote_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ +remote_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) $(AM_V_GEN)$(PERL) -w $(srcdir)/../src/rpc/gendispatch.pl \ - --mode=server remote REMOTE $(REMOTE_PROTOCOL) > $@ + --mode=server remote REMOTE $(REMOTE_PROTOCOL) \ + > $(srcdir)/remote_dispatch.h -$(srcdir)/lxc_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ +lxc_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ $(LXC_PROTOCOL) $(AM_V_GEN)$(PERL) -w $(srcdir)/../src/rpc/gendispatch.pl \ - --mode=server lxc LXC $(LXC_PROTOCOL) > $@ + --mode=server lxc LXC $(LXC_PROTOCOL) \ + > $(srcdir)/lxc_dispatch.h -$(srcdir)/qemu_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ +qemu_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ $(QEMU_PROTOCOL) $(AM_V_GEN)$(PERL) -w $(srcdir)/../src/rpc/gendispatch.pl \ - --mode=server qemu QEMU $(QEMU_PROTOCOL) > $@ + --mode=server qemu QEMU $(QEMU_PROTOCOL) \ + > $(srcdir)/qemu_dispatch.h if WITH_LIBVIRTD diff --git a/src/Makefile.am b/src/Makefile.am index ca35e04..7f76af7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -154,12 +154,12 @@ UTIL_SOURCES = \ EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeycode-mapgen.py -BUILT_SOURCES += $(srcdir)/util/virkeymaps.h +BUILT_SOURCES += util/virkeymaps.h -$(srcdir)/util/virkeymaps.h: $(srcdir)/util/keymaps.csv \ +util/virkeymaps.h: $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeycode-mapgen.py $(AM_V_GEN)$(PYTHON) $(srcdir)/util/virkeycode-mapgen.py \ - <$(srcdir)/util/keymaps.csv >$@ + <$(srcdir)/util/keymaps.csv >$(srcdir)/util/virkeymaps.h EXTRA_DIST += util/virthreadpthread.c util/virthreadwin32.c @@ -185,8 +185,8 @@ LOCK_DRIVER_SANLOCK_HELPER_SOURCES = \ locking/sanlock_helper.c LOCK_PROTOCOL_GENERATED = \ - $(srcdir)/locking/lock_protocol.h \ - $(srcdir)/locking/lock_protocol.c \ + locking/lock_protocol.h \ + locking/lock_protocol.c \ $(NULL) LOCK_PROTOCOL = $(srcdir)/locking/lock_protocol.x @@ -216,11 +216,11 @@ LOCK_DAEMON_SOURCES = \ locking/lock_daemon_dispatch.h \ $(NULL) -$(srcdir)/locking/lock_daemon_dispatch_stubs.h: $(LOCK_PROTOCOL) \ +locking/lock_daemon_dispatch_stubs.h: $(LOCK_PROTOCOL) \ $(srcdir)/rpc/gendispatch.pl Makefile.am $(AM_V_GEN)perl -w $(srcdir)/rpc/gendispatch.pl --mode=server \ virLockSpaceProtocol VIR_LOCK_SPACE_PROTOCOL \ - $(LOCK_PROTOCOL) > $@ + $(LOCK_PROTOCOL) > $(srcdir)/locking/lock_daemon_dispatch_stubs.h NETDEV_CONF_SOURCES = \ @@ -302,35 +302,39 @@ CONF_SOURCES = \ # The remote RPC driver, covering domains, storage, networks, etc REMOTE_DRIVER_GENERATED = \ - $(srcdir)/remote/remote_protocol.c \ - $(srcdir)/remote/remote_protocol.h \ - $(srcdir)/remote/remote_client_bodies.h \ - $(srcdir)/remote/lxc_protocol.c \ - $(srcdir)/remote/lxc_protocol.h \ - $(srcdir)/remote/lxc_client_bodies.h \ - $(srcdir)/remote/qemu_protocol.c \ - $(srcdir)/remote/qemu_protocol.h \ - $(srcdir)/remote/qemu_client_bodies.h + remote/remote_protocol.c \ + remote/remote_protocol.h \ + remote/remote_client_bodies.h \ + remote/lxc_protocol.c \ + remote/lxc_protocol.h \ + remote/lxc_client_bodies.h \ + remote/qemu_protocol.c \ + remote/qemu_protocol.h \ + remote/qemu_client_bodies.h \ + $(NULL) REMOTE_PROTOCOL = $(srcdir)/remote/remote_protocol.x LXC_PROTOCOL = $(srcdir)/remote/lxc_protocol.x QEMU_PROTOCOL = $(srcdir)/remote/qemu_protocol.x REMOTE_DRIVER_PROTOCOL = $(REMOTE_PROTOCOL) $(QEMU_PROTOCOL) $(LXC_PROTOCOL) -$(srcdir)/remote/remote_client_bodies.h: $(srcdir)/rpc/gendispatch.pl \ +remote/remote_client_bodies.h: $(srcdir)/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) Makefile.am $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=client \ - remote REMOTE $(REMOTE_PROTOCOL) > $@ + remote REMOTE $(REMOTE_PROTOCOL) \ + > $(srcdir)/remote/remote_client_bodies.h -$(srcdir)/remote/lxc_client_bodies.h: $(srcdir)/rpc/gendispatch.pl \ +remote/lxc_client_bodies.h: $(srcdir)/rpc/gendispatch.pl \ $(LXC_PROTOCOL) Makefile.am $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=client \ - lxc LXC $(LXC_PROTOCOL) > $@ + lxc LXC $(LXC_PROTOCOL) \ + > $(srcdir)/remote/lxc_client_bodies.h -$(srcdir)/remote/qemu_client_bodies.h: $(srcdir)/rpc/gendispatch.pl \ +remote/qemu_client_bodies.h: $(srcdir)/rpc/gendispatch.pl \ $(QEMU_PROTOCOL) Makefile.am $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=client \ - qemu QEMU $(QEMU_PROTOCOL) > $@ + qemu QEMU $(QEMU_PROTOCOL) \ + > $(srcdir)/remote/qemu_client_bodies.h REMOTE_DRIVER_SOURCES = \ gnutls_1_0_compat.h \ @@ -534,16 +538,16 @@ XEN_DRIVER_SOURCES += xen/xen_inotify.c xen/xen_inotify.h endif WITH_XEN_INOTIFY LXC_MONITOR_PROTOCOL_GENERATED = \ - $(srcdir)/lxc/lxc_monitor_protocol.h \ - $(srcdir)/lxc/lxc_monitor_protocol.c \ + lxc/lxc_monitor_protocol.h \ + lxc/lxc_monitor_protocol.c \ $(NULL) LXC_MONITOR_GENERATED = \ - $(srcdir)/lxc/lxc_monitor_dispatch.h \ + lxc/lxc_monitor_dispatch.h \ $(NULL) LXC_CONTROLLER_GENERATED = \ - $(srcdir)/lxc/lxc_controller_dispatch.h \ + lxc/lxc_controller_dispatch.h \ $(NULL) LXC_GENERATED = \ @@ -554,15 +558,17 @@ LXC_GENERATED = \ LXC_MONITOR_PROTOCOL = $(srcdir)/lxc/lxc_monitor_protocol.x -$(srcdir)/lxc/lxc_monitor_dispatch.h: $(srcdir)/rpc/gendispatch.pl \ +lxc/lxc_monitor_dispatch.h: $(srcdir)/rpc/gendispatch.pl \ $(LXC_MONITOR_PROTOCOL) Makefile.am $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=client \ - virLXCMonitor VIR_LXC_MONITOR $(LXC_MONITOR_PROTOCOL) > $@ + virLXCMonitor VIR_LXC_MONITOR $(LXC_MONITOR_PROTOCOL) > \ + $(srcdir)/lxc/lxc_monitor_dispatch.h -$(srcdir)/lxc/lxc_controller_dispatch.h: $(srcdir)/rpc/gendispatch.pl \ +lxc/lxc_controller_dispatch.h: $(srcdir)/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) Makefile.am $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=server \ - virLXCMonitor VIR_LXC_MONITOR $(LXC_MONITOR_PROTOCOL) > $@ + virLXCMonitor VIR_LXC_MONITOR $(LXC_MONITOR_PROTOCOL) > \ + $(srcdir)/lxc/lxc_controller_dispatch.h EXTRA_DIST += \ $(LXC_MONITOR_PROTOCOL) \ @@ -824,12 +830,13 @@ SECURITY_DRIVER_APPARMOR_SOURCES = \ security/security_apparmor.h security/security_apparmor.c ACCESS_DRIVER_GENERATED = \ - $(srcdir)/access/viraccessapicheck.h \ - $(srcdir)/access/viraccessapicheck.c \ - $(srcdir)/access/viraccessapicheckqemu.h \ - $(srcdir)/access/viraccessapicheckqemu.c \ - $(srcdir)/access/viraccessapichecklxc.h \ - $(srcdir)/access/viraccessapichecklxc.c + access/viraccessapicheck.h \ + access/viraccessapicheck.c \ + access/viraccessapicheckqemu.h \ + access/viraccessapicheckqemu.c \ + access/viraccessapichecklxc.h \ + access/viraccessapichecklxc.c \ + $(NULL) ACCESS_DRIVER_SYM_FILES = \ libvirt_access.syms \ @@ -1541,32 +1548,38 @@ libvirt_access_lxc.xml: $(srcdir)/rpc/gendispatch.pl \ $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=aclapi \ lxc LXC $(LXC_PROTOCOL) > $@ -$(srcdir)/access/viraccessapicheck.h: $(srcdir)/rpc/gendispatch.pl \ +access/viraccessapicheck.h: $(srcdir)/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) Makefile.am $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=aclheader \ - remote REMOTE $(REMOTE_PROTOCOL) > $@ -$(srcdir)/access/viraccessapicheck.c: $(srcdir)/rpc/gendispatch.pl \ + remote REMOTE $(REMOTE_PROTOCOL) \ + > $(srcdir)/access/viraccessapicheck.h +access/viraccessapicheck.c: $(srcdir)/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) Makefile.am $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=aclbody \ - remote REMOTE $(REMOTE_PROTOCOL) access/viraccessapicheck.h > $@ + remote REMOTE $(REMOTE_PROTOCOL) access/viraccessapicheck.h \ + > $(srcdir)/access/viraccessapicheck.c -$(srcdir)/access/viraccessapicheckqemu.h: $(srcdir)/rpc/gendispatch.pl \ +access/viraccessapicheckqemu.h: $(srcdir)/rpc/gendispatch.pl \ $(QEMU_PROTOCOL) Makefile.am $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=aclheader \ - qemu QEMU $(QEMU_PROTOCOL) > $@ -$(srcdir)/access/viraccessapicheckqemu.c: $(srcdir)/rpc/gendispatch.pl \ + qemu QEMU $(QEMU_PROTOCOL) \ + > $(srcdir)/access/viraccessapicheckqemu.h +access/viraccessapicheckqemu.c: $(srcdir)/rpc/gendispatch.pl \ $(QEMU_PROTOCOL) Makefile.am $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=aclbody \ - qemu QEMU $(QEMU_PROTOCOL) access/viraccessapicheckqemu.h > $@ + qemu QEMU $(QEMU_PROTOCOL) access/viraccessapicheckqemu.h \ + > $(srcdir)/access/viraccessapicheckqemu.c -$(srcdir)/access/viraccessapichecklxc.h: $(srcdir)/rpc/gendispatch.pl \ +access/viraccessapichecklxc.h: $(srcdir)/rpc/gendispatch.pl \ $(LXC_PROTOCOL) Makefile.am $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=aclheader \ - lxc LXC $(LXC_PROTOCOL) > $@ -$(srcdir)/access/viraccessapichecklxc.c: $(srcdir)/rpc/gendispatch.pl \ + lxc LXC $(LXC_PROTOCOL) \ + > $(srcdir)/access/viraccessapichecklxc.h +access/viraccessapichecklxc.c: $(srcdir)/rpc/gendispatch.pl \ $(LXC_PROTOCOL) Makefile.am $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=aclbody \ - lxc LXC $(LXC_PROTOCOL) access/viraccessapichecklxc.h > $@ + lxc LXC $(LXC_PROTOCOL) access/viraccessapichecklxc.h \ + > $(srcdir)/access/viraccessapichecklxc.c # Add all conditional sources just in case... EXTRA_DIST += \ -- 1.8.3.1

On 09.09.2013 17:51, Eric Blake wrote:
Trying to enable automake's subdir-objects option resulted in the creation of literal directories such as src/$(srcdir)/remote/. I traced this to the fact that we had used a literal $(srcdir) in a location that later fed an automake *_SOURCES variable. This has also been reported as an automake bug: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13928 but it's better to fix our code than to wait for an automake fix.
Some things to remember that affect VPATH builds, and where an in-tree build is blissfully unaware of the issues: if a VPATH build fails to find a file that was used as a prereq of any other target, then the rule for that file will expand $@ to prefer the current build dir (bad because a VPATH build on a fresh checkout will then stick $@ in the current directory instead of the desired srcdir); conversely, if a VPATH build finds the file in srcdir but decides it needs to be rebuilt, then the rule for that file will expand $@ to include the directory where it was found out-of-date (bad for an explicit listing of $(srcdir)/$@ because an incremental VPATH build will then expand srcdir twice). As we want these files to go into srcdir unconditionally, we have to massage or avoid $@ for any recipe that involves one of these files.
Therefore, this patch removes all uses of $(srcdir) from any generated file name that later feeds a *_SOURCES variable, and then rewrites all the recipes to generate those files to hard-code their creation into srcdir without the use of $@.
* src/Makefile.am (REMOTE_DRIVER_GENERATED): Drop $(srcdir); VPATH builds know how to find the files, and automake subdir-objects fails with it in place. (LXC_MONITOR_PROTOCOL_GENERATED, (LXC_MONITOR_GENERATED) (ACCESS_DRIVER_GENERATED, LOCK_PROTOCOL_GENERATED): Likewise. (*_client_bodies.h): Hard-code rules to write into srcdir, as VPATH tries to build $@ locally if missing. (util/virkeymaps.h): Likewise. (lxc/lxc_monitor_dispatch.h): Likewise. (access/viraccessapi*): Likewise. (locking/lock_daemon_dispatch_stubs.h): Likewise. * daemon/Makeflie.am (DAEMON_GENERATED, remote_dispatch.h): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
fixup DAEMON_GENERATED --- daemon/Makefile.am | 23 ++++++----- src/Makefile.am | 109 ++++++++++++++++++++++++++++++----------------------- 2 files changed, 74 insertions(+), 58 deletions(-)
diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 90689f8..e0b8744 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -29,10 +29,10 @@ INCLUDES = \
CLEANFILES =
-DAEMON_GENERATED = \ - $(srcdir)/remote_dispatch.h \ - $(srcdir)/lxc_dispatch.h \ - $(srcdir)/qemu_dispatch.h \ +DAEMON_GENERATED = \ + remote_dispatch.h \ + lxc_dispatch.h \ + qemu_dispatch.h \ $(NULL)
DAEMON_SOURCES = \ @@ -75,20 +75,23 @@ REMOTE_PROTOCOL = $(top_srcdir)/src/remote/remote_protocol.x LXC_PROTOCOL = $(top_srcdir)/src/remote/lxc_protocol.x QEMU_PROTOCOL = $(top_srcdir)/src/remote/qemu_protocol.x
-$(srcdir)/remote_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ +remote_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) $(AM_V_GEN)$(PERL) -w $(srcdir)/../src/rpc/gendispatch.pl \ - --mode=server remote REMOTE $(REMOTE_PROTOCOL) > $@ + --mode=server remote REMOTE $(REMOTE_PROTOCOL) \ + > $(srcdir)/remote_dispatch.h
The other option is to have this line as:
$(srcdir)/$@
But this version is okay as is. ACK. Michal

On 09/10/2013 03:58 AM, Michal Privoznik wrote:
Some things to remember that affect VPATH builds, and where an in-tree build is blissfully unaware of the issues: if a VPATH build fails to find a file that was used as a prereq of any other target, then the rule for that file will expand $@ to prefer the current build dir (bad because a VPATH build on a fresh checkout will then stick $@ in the current directory instead of the desired srcdir); conversely, if a VPATH build finds the file in srcdir but decides it needs to be rebuilt, then the rule for that file will expand $@ to include the directory where it was found out-of-date (bad for an explicit listing of $(srcdir)/$@ because an incremental VPATH build will then expand srcdir twice). As we want these files to go into srcdir unconditionally, we have to massage or avoid $@ for any recipe that involves one of these files.
-$(srcdir)/remote_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ +remote_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) $(AM_V_GEN)$(PERL) -w $(srcdir)/../src/rpc/gendispatch.pl \ - --mode=server remote REMOTE $(REMOTE_PROTOCOL) > $@ + --mode=server remote REMOTE $(REMOTE_PROTOCOL) \ + > $(srcdir)/remote_dispatch.h
The other option is to have this line as:
$(srcdir)/$@
No, that explicitly does not work in VPATH setups, as mentioned in my commit message. On the first run (after a fresh checkout), $@ sees that remote_dispatch.h does not exist, so $@ expands to 'remote_dispatch.h', and the recipe creates $(srcdir)/remote_dispatch.h. But in an incremental rebuild, after changing a prerequisite file (such as the .x file embedded in $(REMOTE_PROTOCOL)), $@ discovers that the target file already exists in $(srcdir) via the VPATH lookup, so it expands $@ to '$(srcdir)/remote_dispatch.h', and the recipe attempts to build '$(srcdir)/$(srcdir)/remote_dispatch.h'. If $(srcdir) is .., then you have a mess on your hands (creating the wrong file, and the right file didn't get updated). Hence, the patch HAS to use absolute naming, rather than $@ magic, for anything that we _always_ want placed into srcdir. (For an in-tree build, $(srcdir) is '.', and since you can't tell between './remote_dispatch.h' vs. '././remote_dispatch.h', the problem only affects VPATH builds.)
But this version is okay as is. ACK.
Thanks for reviewing the series; I'll push shortly. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

If we use subdir-objects with automake, any reference to a cross-directory .c file will result in automake creating rules that track dependency in the cross directory. But this presents a problem during 'make distclean' - if the cross directory is cleaned up first, then the daemon directory will be left with dangling references to .Po dependency files that no longer exist. Meanwhile, referring to the cross-directory .c file means that we are compiling the file twice - once in src, and once in daemon. Better is to compile just once in src into a convenience library, and then use that library from daemon. The tests directory had a similar situation of a cross-directory .c file; to solve that, we actually need a convenience library. * daemon/Makefile.am (DAEMON_SOURCES): Drop .c files... (libvirtd_LDADD): ...and instead use library. (libvirtd_conf_la_SOURCES): Declare a new convenience library. (libvirtd_LDFLAGS): Drop duplicate flag. * tests/Makefile.am (libvirtdconftest_SOURCES): Drop .c file... (libvirtdconftest_LDADD): ..and instead use library. Signed-off-by: Eric Blake <eblake@redhat.com> --- daemon/Makefile.am | 32 +++++++++++++++++++++++++------- tests/Makefile.am | 4 ++-- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index e0b8744..eb58de9 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -37,14 +37,12 @@ DAEMON_GENERATED = \ DAEMON_SOURCES = \ libvirtd.c libvirtd.h \ - libvirtd-config.c libvirtd-config.h \ remote.c remote.h \ stream.c stream.h \ - ../src/remote/remote_protocol.c \ - ../src/remote/lxc_protocol.c \ - ../src/remote/qemu_protocol.c \ $(DAEMON_GENERATED) +LIBVIRTD_CONF_SOURCES = libvirtd-config.c libvirtd-config.h + DISTCLEANFILES = EXTRA_DIST = \ remote_dispatch.h \ @@ -67,7 +65,9 @@ EXTRA_DIST = \ THREADS.txt \ libvirtd.pod.in \ libvirtd.8.in \ - $(DAEMON_SOURCES) + $(DAEMON_SOURCES) \ + $(LIBVIRTD_CONF_SOURCES) \ + $(NULL) BUILT_SOURCES = @@ -95,6 +95,22 @@ qemu_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ if WITH_LIBVIRTD +# Build a convenience library, for reuse in tests/libvirtdconftest +noinst_LTLIBRARIES = libvirtd_conf.la +libvirtd_conf_la_SOURCES = $(LIBVIRTD_CONF_SOURCES) +libvirtd_conf_la_CFLAGS = \ + $(LIBXML_CFLAGS) \ + $(WARN_CFLAGS) $(PIE_CFLAGS) \ + $(COVERAGE_CFLAGS) \ + $(NULL) +libvirtd_conf_la_LDFLAGS = \ + $(RELRO_LDFLAGS) \ + $(PIE_LDFLAGS) \ + $(COVERAGE_LDFLAGS) \ + $(NO_INDIRECT_LDFLAGS) \ + $(NULL) +libvirtd_conf_la_LIBADD = $(LIBXML_LIBS) + man8_MANS = libvirtd.8 sbin_PROGRAMS = libvirtd @@ -130,7 +146,6 @@ libvirtd_CFLAGS = \ libvirtd_LDFLAGS = \ $(RELRO_LDFLAGS) \ $(PIE_LDFLAGS) \ - $(RELRO_LDFLAGS) \ $(COVERAGE_LDFLAGS) \ $(NO_INDIRECT_LDFLAGS) \ $(NULL) @@ -148,8 +163,11 @@ libvirtd_LDADD += ../src/libvirt_probes.lo endif WITH_DTRACE_PROBES libvirtd_LDADD += \ + libvirtd_conf.la \ ../src/libvirt-lxc.la \ - ../src/libvirt-qemu.la + ../src/libvirt-qemu.la \ + ../src/libvirt_driver_remote.la \ + $(NULL) if ! WITH_DRIVER_MODULES if WITH_QEMU diff --git a/tests/Makefile.am b/tests/Makefile.am index 1a24367..e5cf740 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -599,8 +599,8 @@ commandhelper_LDFLAGS = -static if WITH_LIBVIRTD libvirtdconftest_SOURCES = \ libvirtdconftest.c testutils.h testutils.c \ - ../daemon/libvirtd-config.c -libvirtdconftest_LDADD = $(LDADDS) + $(NULL) +libvirtdconftest_LDADD = ../daemon/libvirtd_conf.la $(LDADDS) else ! WITH_LIBVIRTD EXTRA_DIST += libvirtdconftest.c endif ! WITH_LIBVIRTD -- 1.8.3.1

We have been adding new .x files without keeping the list of *-structs files up-to-date. This adds the support for the recent additions. In the process of testing this, I also noticed that Fedora 19's use of dwarves-1.10 (providing pdwtags version 1.9) was producing a single line on stderr but still giving enough useful info on stdout that we could check structs; the real goal of checking stderr separately from stdout was to avoid the bug in dwarves-1.9 where stdout was empty (see bug http://bugzilla.redhat.com/772358). * src/Makefile.am (struct_prefix, PROTOCOL_STRUCTS): Add missing struct tests. (PDWTAGS): Work with Fedora 19 pdwtags. (lxc_monitor_protocol-struct, lock_protocol-struct): New rules. * src/lxc_monitor_protocol-structs: New file. * src/lock_protocol-structs): Likewise. * cfg.mk (generated_files): Enlarge list. Signed-off-by: Eric Blake <eblake@redhat.com> --- cfg.mk | 3 ++- src/Makefile.am | 21 ++++++++++++--- src/lock_protocol-structs | 55 ++++++++++++++++++++++++++++++++++++++++ src/lxc_monitor_protocol-structs | 16 ++++++++++++ 4 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 src/lock_protocol-structs create mode 100644 src/lxc_monitor_protocol-structs diff --git a/cfg.mk b/cfg.mk index b839667..e6584e8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -33,8 +33,9 @@ gnulib_dir = $(srcdir)/.gnulib # This is all gnulib files, as well as generated files for RPC code. generated_files = \ $(srcdir)/daemon/*_dispatch.h \ + $(srcdir)/src/*/*_dispatch.h \ $(srcdir)/src/remote/*_client_bodies.h \ - $(srcdir)/src/remote/*_protocol.[ch] \ + $(srcdir)/src/*/*_protocol.[ch] \ $(srcdir)/gnulib/lib/*.[ch] # We haven't converted all scripts to using gnulib's init.sh yet. diff --git a/src/Makefile.am b/src/Makefile.am index 7f76af7..90b7554 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -364,10 +364,10 @@ EXTRA_DIST += $(REMOTE_DRIVER_PROTOCOL) \ # The alternation of the following regexps matches both cases. r1 = /\* \d+ \*/ r2 = /\* <[[:xdigit:]]+> \S+:\d+ \*/ -struct_prefix = (remote_|qemu_|lxc_|virNet|keepalive_) +struct_prefix = (remote_|qemu_|lxc_|keepalive|vir(Net|LockSpace|LXCMonitor)) # Depending on configure options, libtool creates one or both of -# {,.libs/}libvirt_remote_driver_la-remote_protocol.o. We want the +# {,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want the # newest of the two, in case configure options changed and a stale # file is left around from an earlier build. PDWTAGS = \ @@ -376,7 +376,7 @@ PDWTAGS = \ 2>/dev/null | sed -n 1p`; \ test -f "$$o" || { echo ".o for $< not found" >&2; exit 1; }; \ pdwtags --verbose $$o > $(@F)-t1 2> $(@F)-t2; \ - if test -s $(@F)-t2; then \ + if test ! -s $(@F)-t1 && test -s $(@F)-t2; then \ rm -rf $(@F)-t?; \ echo 'WARNING: pdwtags appears broken; skipping the $@ test' >&2;\ else \ @@ -426,12 +426,17 @@ check-symsorting: $(srcdir) $(SYM_FILES) EXTRA_DIST += check-symfile.pl check-symsorting.pl +# Keep this list synced with RPC_PROBE_FILES PROTOCOL_STRUCTS = \ $(srcdir)/remote_protocol-structs \ $(srcdir)/lxc_protocol-structs \ $(srcdir)/qemu_protocol-structs \ $(srcdir)/virnetprotocol-structs \ - $(srcdir)/virkeepaliveprotocol-structs + $(srcdir)/virkeepaliveprotocol-structs \ + $(srcdir)/lxc_monitor_protocol-structs \ + $(srcdir)/lock_protocol-structs \ + $(NULL) + if WITH_REMOTE check-protocol: $(PROTOCOL_STRUCTS) $(PROTOCOL_STRUCTS:structs=struct) @@ -445,6 +450,13 @@ $(srcdir)/remote_protocol-struct \ $(srcdir)/virnetprotocol-struct $(srcdir)/virkeepaliveprotocol-struct: \ $(srcdir)/%-struct: libvirt_net_rpc_la-%.lo $(PDWTAGS) +$(srcdir)/lxc_monitor_protocol-struct: \ + $(srcdir)/%-struct: libvirt_driver_lxc_impl_la-%.lo + $(PDWTAGS) +$(srcdir)/lock_protocol-struct: \ + $(srcdir)/%-struct: lockd_la-%.lo + $(PDWTAGS) + else !WITH_REMOTE # The $(PROTOCOL_STRUCTS) files must live in git, because they cannot be # re-generated when configured --without-remote. @@ -1886,6 +1898,7 @@ tapset_DATA = libvirt_probes.stp libvirt_qemu_probes.stp libvirt_functions.stp "non_pic_object='$<'" \ > $@ +# Keep this list synced with PROTOCOL_STRUCTS RPC_PROBE_FILES = $(srcdir)/rpc/virnetprotocol.x \ $(srcdir)/rpc/virkeepaliveprotocol.x \ $(srcdir)/remote/remote_protocol.x \ diff --git a/src/lock_protocol-structs b/src/lock_protocol-structs new file mode 100644 index 0000000..8e8b84f --- /dev/null +++ b/src/lock_protocol-structs @@ -0,0 +1,55 @@ +/* -*- c -*- */ +struct virLockSpaceProtocolOwner { + virLockSpaceProtocolUUID uuid; + virLockSpaceProtocolNonNullString name; + u_int id; + u_int pid; +}; +struct virLockSpaceProtocolRegisterArgs { + virLockSpaceProtocolOwner owner; + u_int flags; +}; +struct virLockSpaceProtocolRestrictArgs { + u_int flags; +}; +struct virLockSpaceProtocolNewArgs { + virLockSpaceProtocolNonNullString path; + u_int flags; +}; +struct virLockSpaceProtocolCreateResourceArgs { + virLockSpaceProtocolNonNullString path; + virLockSpaceProtocolNonNullString name; + u_int flags; +}; +struct virLockSpaceProtocolDeleteResourceArgs { + virLockSpaceProtocolNonNullString path; + virLockSpaceProtocolNonNullString name; + u_int flags; +}; +enum virLockSpaceProtocolAcquireResourceFlags { + VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED = 1, + VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE = 2, +}; +struct virLockSpaceProtocolAcquireResourceArgs { + virLockSpaceProtocolNonNullString path; + virLockSpaceProtocolNonNullString name; + u_int flags; +}; +struct virLockSpaceProtocolReleaseResourceArgs { + virLockSpaceProtocolNonNullString path; + virLockSpaceProtocolNonNullString name; + u_int flags; +}; +struct virLockSpaceProtocolCreateLockSpaceArgs { + virLockSpaceProtocolNonNullString path; +}; +enum virLockSpaceProtocolProcedure { + VIR_LOCK_SPACE_PROTOCOL_PROC_REGISTER = 1, + VIR_LOCK_SPACE_PROTOCOL_PROC_RESTRICT = 2, + VIR_LOCK_SPACE_PROTOCOL_PROC_NEW = 3, + VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_RESOURCE = 4, + VIR_LOCK_SPACE_PROTOCOL_PROC_DELETE_RESOURCE = 5, + VIR_LOCK_SPACE_PROTOCOL_PROC_ACQUIRE_RESOURCE = 6, + VIR_LOCK_SPACE_PROTOCOL_PROC_RELEASE_RESOURCE = 7, + VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_LOCKSPACE = 8, +}; diff --git a/src/lxc_monitor_protocol-structs b/src/lxc_monitor_protocol-structs new file mode 100644 index 0000000..da72ec0 --- /dev/null +++ b/src/lxc_monitor_protocol-structs @@ -0,0 +1,16 @@ +/* -*- c -*- */ +enum virLXCMonitorExitStatus { + VIR_LXC_MONITOR_EXIT_STATUS_ERROR = 0, + VIR_LXC_MONITOR_EXIT_STATUS_SHUTDOWN = 1, + VIR_LXC_MONITOR_EXIT_STATUS_REBOOT = 2, +}; +struct virLXCMonitorExitEventMsg { + enum virLXCMonitorExitStatus status; +}; +struct virLXCMonitorInitEventMsg { + uint64_t initpid; +}; +enum virLXCMonitorProcedure { + VIR_LXC_MONITOR_PROC_EXIT_EVENT = 1, + VIR_LXC_MONITOR_PROC_INIT_EVENT = 2, +}; -- 1.8.3.1

Automake 2.0 will enable subdir-objects by default; in preparation for that change, automake 1.14 outputs LOADS of warnings: daemon/Makefile.am:38: warning: source file '../src/remote/remote_protocol.c' is in a subdirectory, daemon/Makefile.am:38: but option 'subdir-objects' is disabled automake-1.14: warning: possible forward-incompatibility. automake-1.14: At least a source file is in a subdirectory, but the 'subdir-objects' automake-1.14: automake option hasn't been enabled. For now, the corresponding output automake-1.14: object file(s) will be placed in the top-level directory. However, automake-1.14: this behaviour will change in future Automake versions: they will automake-1.14: unconditionally cause object files to be placed in the same subdirectory automake-1.14: of the corresponding sources. automake-1.14: You are advised to start using 'subdir-objects' option throughout your automake-1.14: project, to avoid future incompatibilities. daemon/Makefile.am:38: warning: source file '../src/remote/lxc_protocol.c' is in a subdirectory, daemon/Makefile.am:38: but option 'subdir-objects' is disabled ... As automake 1.9 also supported this option, and the previous patches fixed up the code base to work with it, it is safe to now turn it on unconditionally. * configure.ac (AM_INIT_AUTOMAKE): Enable subdir-objects. * .gitignore: Ignore .dirstamp directories. * src/Makefile.am (PDWTAGS, *-protocol-struct): Adjust to new subdir-object location of .lo files. Signed-off-by: Eric Blake <eblake@redhat.com> --- .gitignore | 1 + configure.ac | 2 +- src/Makefile.am | 15 ++++++++------- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index 8a94748..2b8652f 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ *~ .#* .deps +.dirstamp .gdb_history .git .git-module-status diff --git a/configure.ac b/configure.ac index a639f59..a5d0373 100644 --- a/configure.ac +++ b/configure.ac @@ -23,7 +23,7 @@ AC_CONFIG_HEADERS([config.h]) AC_CONFIG_MACRO_DIR([m4]) dnl Make automake keep quiet about wildcards & other GNUmake-isms; also keep dnl quiet about the fact that we intentionally cater to automake 1.9 -AM_INIT_AUTOMAKE([-Wno-portability -Wno-obsolete tar-ustar]) +AM_INIT_AUTOMAKE([-Wno-portability -Wno-obsolete tar-ustar subdir-objects]) AM_MAINTAINER_MODE([enable]) # Maintainer note - comment this line out if you plan to rerun diff --git a/src/Makefile.am b/src/Makefile.am index 90b7554..711da32 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -367,12 +367,13 @@ r2 = /\* <[[:xdigit:]]+> \S+:\d+ \*/ struct_prefix = (remote_|qemu_|lxc_|keepalive|vir(Net|LockSpace|LXCMonitor)) # Depending on configure options, libtool creates one or both of -# {,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want the -# newest of the two, in case configure options changed and a stale +# remote/{,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want +# the newest of the two, in case configure options changed and a stale # file is left around from an earlier build. PDWTAGS = \ $(AM_V_GEN)if (pdwtags --help) > /dev/null 2>&1; then \ - o=`ls -t $(<:.lo=.$(OBJEXT)) .libs/$(<:.lo=.$(OBJEXT)) \ + o=`ls -t $(<:.lo=.$(OBJEXT)) \ + $(subst /,/.libs/,$(<:.lo=.$(OBJEXT))) \ 2>/dev/null | sed -n 1p`; \ test -f "$$o" || { echo ".o for $< not found" >&2; exit 1; }; \ pdwtags --verbose $$o > $(@F)-t1 2> $(@F)-t2; \ @@ -445,16 +446,16 @@ check-protocol: $(PROTOCOL_STRUCTS) $(PROTOCOL_STRUCTS:structs=struct) $(srcdir)/remote_protocol-struct \ $(srcdir)/qemu_protocol-struct \ $(srcdir)/lxc_protocol-struct: \ - $(srcdir)/%-struct: libvirt_driver_remote_la-%.lo + $(srcdir)/%-struct: remote/libvirt_driver_remote_la-%.lo $(PDWTAGS) $(srcdir)/virnetprotocol-struct $(srcdir)/virkeepaliveprotocol-struct: \ - $(srcdir)/%-struct: libvirt_net_rpc_la-%.lo + $(srcdir)/%-struct: rpc/libvirt_net_rpc_la-%.lo $(PDWTAGS) $(srcdir)/lxc_monitor_protocol-struct: \ - $(srcdir)/%-struct: libvirt_driver_lxc_impl_la-%.lo + $(srcdir)/%-struct: lxc/libvirt_driver_lxc_impl_la-%.lo $(PDWTAGS) $(srcdir)/lock_protocol-struct: \ - $(srcdir)/%-struct: lockd_la-%.lo + $(srcdir)/%-struct: locking/lockd_la-%.lo $(PDWTAGS) else !WITH_REMOTE -- 1.8.3.1

On 09.09.2013 17:51, Eric Blake wrote:
Automake 1.14 is annoyingly loud about warning that the future automake 2.0 will turn on subdir-objects by default. Since automake 1.9 also supports subdir-objects, the best course of action is to enable the feature. But we have to fix some problems before doing so. Also, I noticed some issues with .x files while searching for places that needed fixing.
Eric Blake (4): build: avoid $(srcdir) in *_SOURCES build: use library rather than cross-directory compilation tests: check remaining .x files build: use automake subdir-objects
.gitignore | 1 + cfg.mk | 3 +- configure.ac | 2 +- daemon/Makefile.am | 55 +++++++++++----- src/Makefile.am | 139 +++++++++++++++++++++++---------------- src/lock_protocol-structs | 55 ++++++++++++++++ src/lxc_monitor_protocol-structs | 16 +++++ tests/Makefile.am | 4 +- 8 files changed, 198 insertions(+), 77 deletions(-) create mode 100644 src/lock_protocol-structs create mode 100644 src/lxc_monitor_protocol-structs
ACK series. Michal
participants (2)
-
Eric Blake
-
Michal Privoznik