[PATCH 00/12] more tools changes to honor --prefix=

[PATCH 00/12] more tools changes to honor --prefix=

[Olaf Hering]

In my attempt to get a xen.rpm from 'make rpmball' which operates
entirely below the configured --prefix= I came up with these changes.
Up to now there was very little runtime testing. A PV and HVM guest
starts fine.

Outstanding is a change to pygrub, which does not find the site-packages
below --prefix= if PYTHONPATH is not set.


This series contains also two unrelated cleanup changes at the end.

Olaf Hering (12):
  Config.mk: move directory list into BUILD_MAKE_VARS
  Config.mk: replace dependency to genpath with actual target
  Config.mk: add new macro buildmakevars2header
  tools/libxl: use buildmakevars2header to create _paths.h
  tools/libxc: provide variable paths to libxc
  tools/libxc: use XEN_RUN_DIR for SUSPEND_LOCK_FILE
  tools/pygrub: store kernels in /var/run/xen/pygrub
  tools/hotplug: use XEN_SCRIPT_DIR instead of hardcoded path
  tools/hotplug: create XEN_RUN_DIR at runtime
  tools/hotplug: create XEN_LOCK_DIR at runtime
  tools/examples: remove obsolete install targets
  remove obsolete SUBSYS_DIR variable

 Config.mk                                 | 22 ++++++++++++++++++----
 docs/misc/distro_mapping.txt              |  1 -
 stubdom/Makefile                          | 16 ++++++++--------
 tools/examples/Makefile                   | 21 +--------------------
 tools/hotplug/Linux/Makefile              | 30 ++++++++++++++++++++++--------
 tools/hotplug/Linux/init.d/xen-watchdog   |  2 +-
 tools/hotplug/Linux/init.d/xencommons     |  5 +++--
 tools/hotplug/Linux/init.d/xendomains     |  2 +-
 tools/hotplug/Linux/vif-setup             |  2 +-
 tools/hotplug/Linux/xen-backend.rules     | 16 ++++++++--------
 tools/hotplug/Linux/xen-hotplug-common.sh |  2 +-
 tools/hotplug/common/Makefile             |  2 +-
 tools/libxc/Makefile                      |  8 ++++++++
 tools/libxc/xc_private.h                  |  1 +
 tools/libxc/xc_suspend.c                  |  2 +-
 tools/libxl/Makefile                      |  7 +------
 tools/pygrub/src/pygrub                   | 13 ++++++++++++-
 tools/python/Makefile                     |  2 +-
 18 files changed, 89 insertions(+), 65 deletions(-)

[PATCH 01/12] Config.mk: move directory list into BUILD_MAKE_VARS

[Olaf Hering]

To maintain the list of directories in a single place, move the existing
list into its own variable and use it in buildmakevars2file.
Required for upcoming changes.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 Config.mk | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Config.mk b/Config.mk
index 6a93533..a26e964 100644
--- a/Config.mk
+++ b/Config.mk
@@ -166,15 +166,17 @@ define move-if-changed
 	if ! cmp -s $(1) $(2); then mv -f $(1) $(2); else rm -f $(1); fi
 endef
 
+BUILD_MAKE_VARS := SBINDIR BINDIR LIBEXEC LIBDIR SHAREDIR PRIVATE_BINDIR \
+                   XENFIRMWAREDIR XEN_CONFIG_DIR XEN_SCRIPT_DIR XEN_LOCK_DIR \
+                   XEN_RUN_DIR XEN_PAGING_DIR
+
 buildmakevars2file = $(eval $(call buildmakevars2file-closure,$(1)))
 define buildmakevars2file-closure
     .PHONY: genpath
     genpath:
 	rm -f $(1).tmp;                                                     \
 	$(foreach var,                                                      \
-	          SBINDIR BINDIR LIBEXEC LIBDIR SHAREDIR PRIVATE_BINDIR     \
-	          XENFIRMWAREDIR XEN_CONFIG_DIR XEN_SCRIPT_DIR XEN_LOCK_DIR \
-	          XEN_RUN_DIR XEN_PAGING_DIR,                               \
+	          $(BUILD_MAKE_VARS),                                       \
 	          echo "$(var)=\"$($(var))\"" >>$(1).tmp;)        \
 	$(call move-if-changed,$(1).tmp,$(1))
 endef

[PATCH 02/12] Config.mk: replace dependency to genpath with actual target

[Olaf Hering]

genpath is a detail of buildmakevars2file. Replace the dependency to
genpath with the actual buildmakevars2file target. This change by itself
does not fix any bug. Upcoming changes will add dependencies to
$(target), but no rule exist to create $(target).

With this change a rule for $(target) is added, which in turn depends on
genpath. No change in behaviour is expected by this patch.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 Config.mk                     |  1 +
 stubdom/Makefile              | 16 ++++++++--------
 tools/hotplug/common/Makefile |  2 +-
 tools/python/Makefile         |  2 +-
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/Config.mk b/Config.mk
index a26e964..eb3937c 100644
--- a/Config.mk
+++ b/Config.mk
@@ -173,6 +173,7 @@ BUILD_MAKE_VARS := SBINDIR BINDIR LIBEXEC LIBDIR SHAREDIR PRIVATE_BINDIR \
 buildmakevars2file = $(eval $(call buildmakevars2file-closure,$(1)))
 define buildmakevars2file-closure
     .PHONY: genpath
+    $(1): genpath
     genpath:
 	rm -f $(1).tmp;                                                     \
 	$(foreach var,                                                      \
diff --git a/stubdom/Makefile b/stubdom/Makefile
index c41de27..5f25c20 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -48,18 +48,18 @@ TARGET_LDFLAGS += -nostdlib -L$(CROSS_PREFIX)/$(GNU_TARGET_ARCH)-xen-elf/lib
 
 TARGETS=$(STUBDOM_TARGETS)
 
+STUBDOMPATH="stubdompath.sh"
+genpath-target = $(call buildmakevars2file,$(STUBDOMPATH))
+$(eval $(genpath-target))
+
 .PHONY: all
 all: build
 ifeq ($(STUBDOM_SUPPORTED),1)
-build: genpath $(STUBDOM_BUILD)
+build: $(STUBDOMPATH) $(STUBDOM_BUILD)
 else
-build: genpath
+build: $(STUBDOMPATH)
 endif
 
-STUBDOMPATH="stubdompath.sh"
-genpath-target = $(call buildmakevars2file,$(STUBDOMPATH))
-$(eval $(genpath-target))
-
 ##############
 # Cross-newlib
 ##############
@@ -448,9 +448,9 @@ xenstore-stubdom: mini-os-$(XEN_TARGET_ARCH)-xenstore libxc xenstore
 #########
 
 ifeq ($(STUBDOM_SUPPORTED),1)
-install: genpath install-readme $(STUBDOM_INSTALL)
+install: $(STUBDOMPATH) install-readme $(STUBDOM_INSTALL)
 else
-install: genpath
+install: $(STUBDOMPATH)
 endif
 
 install-readme:
diff --git a/tools/hotplug/common/Makefile b/tools/hotplug/common/Makefile
index 18d87aa..657a8e3 100644
--- a/tools/hotplug/common/Makefile
+++ b/tools/hotplug/common/Makefile
@@ -16,7 +16,7 @@ $(eval $(genpath-target))
 all: build
 
 .PHONY: build
-build: genpath
+build: $(HOTPLUGPATH)
 
 .PHONY: install
 install: all install-scripts
diff --git a/tools/python/Makefile b/tools/python/Makefile
index c433cbe..8c4b86e 100644
--- a/tools/python/Makefile
+++ b/tools/python/Makefile
@@ -10,7 +10,7 @@ genpath-target = $(call buildmakevars2file,$(XENPATH))
 $(eval $(genpath-target))
 
 .PHONY: build
-build: genpath genwrap.py $(XEN_ROOT)/tools/libxl/libxl_types.idl \
+build: $(XENPATH) genwrap.py $(XEN_ROOT)/tools/libxl/libxl_types.idl \
 		$(XEN_ROOT)/tools/libxl/idl.py
 	PYTHONPATH=$(XEN_ROOT)/tools/libxl $(PYTHON) genwrap.py \
 		$(XEN_ROOT)/tools/libxl/libxl_types.idl \

[PATCH 03/12] Config.mk: add new macro buildmakevars2header

[Olaf Hering]

This macro is similar to buildmakevars2file, it just creates a C header
file instead of shell style syntax. Upcoming changes will use this macro
in libxl and libxc.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 Config.mk | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Config.mk b/Config.mk
index eb3937c..d8fe87b 100644
--- a/Config.mk
+++ b/Config.mk
@@ -182,6 +182,18 @@ define buildmakevars2file-closure
 	$(call move-if-changed,$(1).tmp,$(1))
 endef
 
+buildmakevars2header = $(eval $(call buildmakevars2header-closure,$(1)))
+define buildmakevars2header-closure
+    .PHONY: genpath
+    $(1): genpath
+    genpath:
+	rm -f $(1).tmp;                                                     \
+	$(foreach var,                                                      \
+	          $(BUILD_MAKE_VARS),                                       \
+	          echo "#define $(var) \"$($(var))\"" >>$(1).tmp;)\
+	$(call move-if-changed,$(1).tmp,$(1))
+endef
+
 ifeq ($(debug_symbols),y)
 CFLAGS += -g
 endif

[PATCH 04/12] tools/libxl: use buildmakevars2header to create _paths.h

[Olaf Hering]

Replace usage of buildmakevars2file with buildmakevars2header. The macro
generates a C header file, so remove code which converts shell variables
into C defines. Also update the dependency, the macro itself creates a
dependency to genpath for _paths.h. A temporary file is not needed
anymore.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/libxl/Makefile | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 4cfa275..6c8a14b 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -141,7 +141,7 @@ $(LIBXL_OBJS) $(LIBXLU_OBJS) $(XL_OBJS) $(SAVE_HELPER_OBJS) \
 	@rm -f $*.[ch]
 	$(FLEX) --header-file=$*.h --outfile=$*.c $<
 
-genpath-target = $(call buildmakevars2file,_paths.h.tmp)
+genpath-target = $(call buildmakevars2header,_paths.h)
 $(eval $(genpath-target))
 
 libxl.api-ok: check-libxl-api-rules _libxl.api-for-check
@@ -154,11 +154,6 @@ _%.api-for-check: %.h $(AUTOINCS)
 		>$@.new
 	mv -f $@.new $@
 
-_paths.h: genpath
-	sed -e "s/\([^=]*\)=\(.*\)/#define \1 \2/g" $@.tmp >$@.2.tmp
-	rm -f $@.tmp
-	$(call move-if-changed,$@.2.tmp,$@)
-
 _libxl_list.h: $(XEN_INCLUDE)/xen-external/bsd-sys-queue-h-seddery $(XEN_INCLUDE)/xen-external/bsd-sys-queue.h
 	$(PERL) $^ --prefix=libxl >$@.new
 	$(call move-if-changed,$@.new,$@)

[PATCH 05/12] tools/libxc: provide variable paths to libxc

[Olaf Hering]

In preparation to remove hardcoded /var/run/xen paths, provide
XEN_RUN_DIR and related directories to xc_private.h. Similar code exists
already for libxl, stubdom and other parts.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/libxc/Makefile     | 8 ++++++++
 tools/libxc/xc_private.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
index a74b19e..cbba502 100644
--- a/tools/libxc/Makefile
+++ b/tools/libxc/Makefile
@@ -118,6 +118,13 @@ ifneq ($(stubdom),y)
 LIB += xenctrl_osdep_ENOSYS.so
 endif
 
+genpath-target = $(call buildmakevars2header,_paths.h)
+$(eval $(genpath-target))
+
+xc_private.h: _paths.h
+
+$(CTRL_LIB_OBJS) $(GUEST_LIB_OBJS) $(OSDEP_LIB_OBJS): xc_private.h
+
 .PHONY: all
 all: build
 
@@ -150,6 +157,7 @@ TAGS:
 .PHONY: clean
 clean:
 	rm -rf *.rpm $(LIB) *~ $(DEPS) \
+            _paths.h \
             $(CTRL_LIB_OBJS) $(CTRL_PIC_OBJS) \
             $(GUEST_LIB_OBJS) $(GUEST_PIC_OBJS) \
             $(OSDEP_LIB_OBJS) $(OSDEP_PIC_OBJS)
diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
index 670a82d..786dc61 100644
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -29,6 +29,7 @@
 #include <stdlib.h>
 #include <sys/ioctl.h>
 
+#include "_paths.h"
 #include "xenctrl.h"
 #include "xenctrlosdep.h"

[PATCH 06/12] tools/libxc: use XEN_RUN_DIR for SUSPEND_LOCK_FILE

[Olaf Hering]

Remove hardcoded /var/run/xen directory path, use XEN_RUN_DIR instead.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxc/xc_suspend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxc/xc_suspend.c b/tools/libxc/xc_suspend.c
index 200d381..e22f4ac 100644
--- a/tools/libxc/xc_suspend.c
+++ b/tools/libxc/xc_suspend.c
@@ -20,7 +20,7 @@
 #include "xc_private.h"
 #include "xenguest.h"
 
-#define SUSPEND_LOCK_FILE "/var/run/xen/suspend-evtchn-%d.lock"
+#define SUSPEND_LOCK_FILE    XEN_RUN_DIR "/suspend-evtchn-%d.lock"
 
 /*
  * locking

[PATCH 07/12] tools/pygrub: store kernels in /var/run/xen/pygrub

[Olaf Hering]

Move location of temporary bootfiles from /var/run/xend/boot to
/var/run/xen/pygrub. Create the subdirectory if does not exist, unless
--output-directory=<dir> was specified.

The reason for this change is that all entrys below /var/run have to be
created at runtime in case /var/run is cleared on every boot.

v3:
 use /var/run/xen/pygrub instead of /var/run/pygrub
v2:
 update exception handling for python 2.4

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/pygrub/src/pygrub | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
index 54fecee..7759b1a 100644
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -14,6 +14,7 @@
 #
 
 import os, sys, string, struct, tempfile, re, traceback
+import errno
 import copy
 import logging
 import platform
@@ -757,7 +758,7 @@ if __name__ == "__main__":
     debug = False
     not_really = False
     output_format = "sxp"
-    output_directory = "/var/run/xend/boot"
+    output_directory = None
 
     # what was passed in
     incfg = { "kernel": None, "ramdisk": None, "args": "" }
@@ -813,6 +814,16 @@ if __name__ == "__main__":
     if debug:
 	logging.basicConfig(level=logging.DEBUG)
 
+    if output_directory is None:
+        output_directory = "/var/run/xen/pygrub"
+        try:
+            os.mkdir(output_directory, 0700)
+        except OSError, exc:
+            if exc.errno == errno.EEXIST and os.path.isdir(output_directory):
+                pass
+            else:
+                raise
+
     if output is None or output == "-":
         fd = sys.stdout.fileno()
     else:

[PATCH 08/12] tools/hotplug: use XEN_SCRIPT_DIR instead of hardcoded path

[Olaf Hering]

Helper scripts get installed into XEN_SCRIPT_DIR, but initscripts,
helper scripts and udev rules still refer to the hardcoded location
/etc/xen/scripts/. Update scripts, rules and Makefile to refer to
@@XEN_SCRIPT_DIR@@ instead.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/hotplug/Linux/Makefile              | 30 ++++++++++++++++++++++--------
 tools/hotplug/Linux/init.d/xen-watchdog   |  2 +-
 tools/hotplug/Linux/init.d/xencommons     |  2 +-
 tools/hotplug/Linux/init.d/xendomains     |  2 +-
 tools/hotplug/Linux/vif-setup             |  2 +-
 tools/hotplug/Linux/xen-backend.rules     | 16 ++++++++--------
 tools/hotplug/Linux/xen-hotplug-common.sh |  2 +-
 7 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
index 4874ec5..aae7f9e 100644
--- a/tools/hotplug/Linux/Makefile
+++ b/tools/hotplug/Linux/Makefile
@@ -2,10 +2,11 @@ XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
 # Init scripts.
-XENDOMAINS_INITD = init.d/xendomains
+XEN_INIT_SCRIPTS = xen-watchdog
+XEN_INIT_SCRIPTS += xendomains
 XENDOMAINS_SYSCONFIG = init.d/sysconfig.xendomains
 
-XENCOMMONS_INITD = init.d/xencommons
+XEN_INIT_SCRIPTS += xencommons
 XENCOMMONS_SYSCONFIG = init.d/sysconfig.xencommons
 
 # Xen script dir and scripts to go there.
@@ -45,11 +46,15 @@ install: all install-initd install-scripts install-udev
 install-initd:
 	[ -d $(DESTDIR)$(INITD_DIR) ] || $(INSTALL_DIR) $(DESTDIR)$(INITD_DIR)
 	[ -d $(DESTDIR)$(SYSCONFIG_DIR) ] || $(INSTALL_DIR) $(DESTDIR)$(SYSCONFIG_DIR)
-	$(INSTALL_PROG) $(XENDOMAINS_INITD) $(DESTDIR)$(INITD_DIR)
+	set -e; for i in $(XEN_INIT_SCRIPTS); \
+	    do \
+		o="$(DESTDIR)$(INITD_DIR)/$$i"; \
+		sed 's|@@XEN_SCRIPT_DIR@@|$(XEN_SCRIPT_DIR)|g' \
+		init.d/$$i > "$$o"; \
+		chmod 755 "$$o"; \
+	done
 	$(INSTALL_DATA) $(XENDOMAINS_SYSCONFIG) $(DESTDIR)$(SYSCONFIG_DIR)/xendomains
-	$(INSTALL_PROG) $(XENCOMMONS_INITD) $(DESTDIR)$(INITD_DIR)
 	$(INSTALL_DATA) $(XENCOMMONS_SYSCONFIG) $(DESTDIR)$(SYSCONFIG_DIR)/xencommons
-	$(INSTALL_PROG) init.d/xen-watchdog $(DESTDIR)$(INITD_DIR)
 
 .PHONY: install-scripts
 install-scripts:
@@ -57,11 +62,17 @@ install-scripts:
 		$(INSTALL_DIR) $(DESTDIR)$(XEN_SCRIPT_DIR)
 	set -e; for i in $(XEN_SCRIPTS); \
 	    do \
-	    $(INSTALL_PROG) $$i $(DESTDIR)$(XEN_SCRIPT_DIR); \
+		o="$(DESTDIR)$(XEN_SCRIPT_DIR)/$$i"; \
+		sed 's|@@XEN_SCRIPT_DIR@@|$(XEN_SCRIPT_DIR)|g' \
+		$$i > "$$o"; \
+		chmod 755 "$$o"; \
 	done
 	set -e; for i in $(XEN_SCRIPT_DATA); \
 	    do \
-	    $(INSTALL_DATA) $$i $(DESTDIR)$(XEN_SCRIPT_DIR); \
+		o="$(DESTDIR)$(XEN_SCRIPT_DIR)/$$i"; \
+		sed 's|@@XEN_SCRIPT_DIR@@|$(XEN_SCRIPT_DIR)|g' \
+		$$i > "$$o"; \
+		chmod 644 "$$o"; \
 	done
 
 .PHONY: install-udev
@@ -70,7 +81,10 @@ install-udev:
 		$(INSTALL_DIR) $(DESTDIR)$(UDEV_RULES_DIR)/rules.d
 	set -e; for i in $(UDEV_RULES); \
 	    do \
-	    $(INSTALL_DATA) $$i $(DESTDIR)$(UDEV_RULES_DIR)/rules.d; \
+		o="$(DESTDIR)$(UDEV_RULES_DIR)/rules.d/$$i"; \
+		sed 's|@@XEN_SCRIPT_DIR@@|$(XEN_SCRIPT_DIR)|g' \
+		$$i > "$$o"; \
+		chmod 644 "$$o"; \
 	done
 
 .PHONY: clean
diff --git a/tools/hotplug/Linux/init.d/xen-watchdog b/tools/hotplug/Linux/init.d/xen-watchdog
index 3592fda..d0d7966 100644
--- a/tools/hotplug/Linux/init.d/xen-watchdog
+++ b/tools/hotplug/Linux/init.d/xen-watchdog
@@ -17,7 +17,7 @@
 ### END INIT INFO
 #
 
-. /etc/xen/scripts/hotplugpath.sh
+. @@XEN_SCRIPT_DIR@@/hotplugpath.sh
 
 DAEMON=${SBINDIR}/xenwatchdogd
 base=$(basename $DAEMON)
diff --git a/tools/hotplug/Linux/init.d/xencommons b/tools/hotplug/Linux/init.d/xencommons
index 4ebd636..c62bdb8 100644
--- a/tools/hotplug/Linux/init.d/xencommons
+++ b/tools/hotplug/Linux/init.d/xencommons
@@ -18,7 +18,7 @@
 # Description:       Starts and stops the daemons neeeded for xl/xend
 ### END INIT INFO
 
-. /etc/xen/scripts/hotplugpath.sh
+. @@XEN_SCRIPT_DIR@@/hotplugpath.sh
 
 if [ -d /etc/sysconfig ]; then
 	xencommons_config=/etc/sysconfig
diff --git a/tools/hotplug/Linux/init.d/xendomains b/tools/hotplug/Linux/init.d/xendomains
index 59f1e3d..66f3f9a 100644
--- a/tools/hotplug/Linux/init.d/xendomains
+++ b/tools/hotplug/Linux/init.d/xendomains
@@ -27,7 +27,7 @@
 #                    boots / shuts down.
 ### END INIT INFO
 
-. /etc/xen/scripts/hotplugpath.sh
+. @@XEN_SCRIPT_DIR@@/hotplugpath.sh
 
 CMD=${SBINDIR}/xm
 HEADCOMP="LinuxGuestRecord"
diff --git a/tools/hotplug/Linux/vif-setup b/tools/hotplug/Linux/vif-setup
index bcc462e..ce1ba56 100644
--- a/tools/hotplug/Linux/vif-setup
+++ b/tools/hotplug/Linux/vif-setup
@@ -4,6 +4,6 @@ if test "$script"
 then
     exec $script $*
 else
-    exec /etc/xen/scripts/vif-bridge $*
+    exec @@XEN_SCRIPT_DIR@@/vif-bridge $*
 fi
 
diff --git a/tools/hotplug/Linux/xen-backend.rules b/tools/hotplug/Linux/xen-backend.rules
index a0d409e..8e01533 100644
--- a/tools/hotplug/Linux/xen-backend.rules
+++ b/tools/hotplug/Linux/xen-backend.rules
@@ -1,10 +1,10 @@
-SUBSYSTEM=="xen-backend", KERNEL=="tap*", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/blktap $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vbd*", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/block $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vif2-*", RUN+="/etc/xen/scripts/vif2 $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ENV{UDEV_CALL}="1", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online type_if=vif"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ENV{UDEV_CALL}="1", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline type_if=vif"
-SUBSYSTEM=="xen-backend", KERNEL=="vscsi*", RUN+="/etc/xen/scripts/vscsi $env{ACTION}"
-SUBSYSTEM=="xen-backend", ACTION=="remove", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
+SUBSYSTEM=="xen-backend", KERNEL=="tap*", ENV{UDEV_CALL}="1", RUN+="@@XEN_SCRIPT_DIR@@/blktap $env{ACTION}"
+SUBSYSTEM=="xen-backend", KERNEL=="vbd*", ENV{UDEV_CALL}="1", RUN+="@@XEN_SCRIPT_DIR@@/block $env{ACTION}"
+SUBSYSTEM=="xen-backend", KERNEL=="vif2-*", RUN+="@@XEN_SCRIPT_DIR@@/vif2 $env{ACTION}"
+SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ENV{UDEV_CALL}="1", ACTION=="online", RUN+="@@XEN_SCRIPT_DIR@@/vif-setup online type_if=vif"
+SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ENV{UDEV_CALL}="1", ACTION=="offline", RUN+="@@XEN_SCRIPT_DIR@@/vif-setup offline type_if=vif"
+SUBSYSTEM=="xen-backend", KERNEL=="vscsi*", RUN+="@@XEN_SCRIPT_DIR@@/vscsi $env{ACTION}"
+SUBSYSTEM=="xen-backend", ACTION=="remove", ENV{UDEV_CALL}="1", RUN+="@@XEN_SCRIPT_DIR@@/xen-hotplug-cleanup"
 KERNEL=="evtchn", NAME="xen/%k"
 SUBSYSTEM=="xen", KERNEL=="blktap[0-9]*", NAME="xen/%k", MODE="0600"
 SUBSYSTEM=="blktap2", KERNEL=="blktap[0-9]*", NAME="xen/blktap-2/%k", MODE="0600"
@@ -12,4 +12,4 @@ KERNEL=="blktap-control", NAME="xen/blktap-2/control", MODE="0600"
 KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
 KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
 KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
-SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
+SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add", ENV{UDEV_CALL}="1", RUN+="@@XEN_SCRIPT_DIR@@/vif-setup $env{ACTION} type_if=tap"
diff --git a/tools/hotplug/Linux/xen-hotplug-common.sh b/tools/hotplug/Linux/xen-hotplug-common.sh
index 7af4688..8128b9e 100644
--- a/tools/hotplug/Linux/xen-hotplug-common.sh
+++ b/tools/hotplug/Linux/xen-hotplug-common.sh
@@ -112,7 +112,7 @@ xenstore_write() {
 # Execute each hook in the <hook> directory.
 #
 call_hooks() {
-  for f in /etc/xen/scripts/${1}-${2}.d/*.hook; do
+  for f in @@XEN_SCRIPT_DIR@@/${1}-${2}.d/*.hook; do
     if [ -x "$f" ]; then . "$f"; fi
   done
 }

[PATCH 09/12] tools/hotplug: create XEN_RUN_DIR at runtime

[Olaf Hering]

Create XEN_RUN_DIR instead of hardcoded path at runtime.
/var/run might be empty on startup.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/hotplug/Linux/init.d/xencommons | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/hotplug/Linux/init.d/xencommons b/tools/hotplug/Linux/init.d/xencommons
index c62bdb8..2326a9c 100644
--- a/tools/hotplug/Linux/init.d/xencommons
+++ b/tools/hotplug/Linux/init.d/xencommons
@@ -72,7 +72,7 @@ do_start () {
 	modprobe pciback 2>/dev/null
 	modprobe xen-acpi-processor 2>/dev/null
 	modprobe blktap2 2>/dev/null || modprobe blktap 2>/dev/null
-	mkdir -p /var/run/xen
+	mkdir -p ${XEN_RUN_DIR}
 
 	if ! `${BINDIR}/xenstore-read -s / >/dev/null 2>&1`
 	then

[PATCH 10/12] tools/hotplug: create XEN_LOCK_DIR at runtime

[Olaf Hering]

XEN_LOCK_DIR is a compile-time setting and may not exist on the target
host.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/hotplug/Linux/init.d/xencommons | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/hotplug/Linux/init.d/xencommons b/tools/hotplug/Linux/init.d/xencommons
index 2326a9c..0b14568 100644
--- a/tools/hotplug/Linux/init.d/xencommons
+++ b/tools/hotplug/Linux/init.d/xencommons
@@ -73,6 +73,7 @@ do_start () {
 	modprobe xen-acpi-processor 2>/dev/null
 	modprobe blktap2 2>/dev/null || modprobe blktap 2>/dev/null
 	mkdir -p ${XEN_RUN_DIR}
+	mkdir -p ${XEN_LOCK_DIR}
 
 	if ! `${BINDIR}/xenstore-read -s / >/dev/null 2>&1`
 	then

[PATCH 11/12] tools/examples: remove obsolete install targets

[Olaf Hering]

install-hotplug and install-udev are obsolete since commit 57bcfa11
("tools/hotplug: Separate OS-specific scripts.")

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/examples/Makefile | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/tools/examples/Makefile b/tools/examples/Makefile
index 86d9cc1..473580e 100644
--- a/tools/examples/Makefile
+++ b/tools/examples/Makefile
@@ -19,7 +19,7 @@ all:
 build:
 
 .PHONY: install
-install: all install-readmes install-configs $(HOTPLUGS)
+install: all install-readmes install-configs
 
 .PHONY: install-readmes
 install-readmes:
@@ -41,24 +41,5 @@ install-configs: $(XEN_CONFIGS)
 	    $(INSTALL_DATA) $$i $(DESTDIR)$(XEN_CONFIG_DIR); \
 	done
 
-.PHONY: install-hotplug
-install-hotplug:
-	[ -d $(DESTDIR)$(XEN_HOTPLUG_DIR) ] || \
-		$(INSTALL_DIR) $(DESTDIR)$(XEN_HOTPLUG_DIR)
-	set -e; for i in $(XEN_HOTPLUG_SCRIPTS); \
-	    do \
-	    $(INSTALL_PROG) $$i $(DESTDIR)$(XEN_HOTPLUG_DIR); \
-	done
-
-.PHONY: install-udev
-install-udev:
-	[ -d $(DESTDIR)$(UDEV_RULES_DIR) ] || \
-		$(INSTALL_DIR) $(DESTDIR)$(UDEV_RULES_DIR)/rules.d
-	set -e; for i in $(UDEV_RULES); \
-	    do \
-	    $(INSTALL_DATA) $$i $(DESTDIR)$(UDEV_RULES_DIR); \
-	    ln -sf ../$$i $(DESTDIR)$(UDEV_RULES_DIR)/rules.d; \
-	done
-
 .PHONY: clean
 clean:

[PATCH 12/12] remove obsolete SUBSYS_DIR variable

[Olaf Hering]

/var/run is a runtime directory. It is not supposed to be packaged. The
xendomains runlevel script checks at runtime if /var/run/subsys or
/var/run has to be used. Remove the unused SUBSYS_DIR variable from
Config.mk and distro_mapping.txt.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 Config.mk                    | 1 -
 docs/misc/distro_mapping.txt | 1 -
 2 files changed, 2 deletions(-)

diff --git a/Config.mk b/Config.mk
index d8fe87b..3cc1ab1 100644
--- a/Config.mk
+++ b/Config.mk
@@ -75,7 +75,6 @@ endef
 
 # See distro_mapping.txt for other options
 $(eval $(call setvar_dir,CONFIG_LEAF_DIR,,/etc/sysconfig,sysconfig,default))
-$(eval $(call setvar_dir,SUBSYS_DIR,/var/run,/subsys,/subsys,))
 $(eval $(call setvar_dir,INITD_DIR,/etc,/rc.d/init.d,/rc.d/init.d,/init.d))
 
 ifneq ($(EXTRA_PREFIX),)
diff --git a/docs/misc/distro_mapping.txt b/docs/misc/distro_mapping.txt
index f56e281..f849d07 100644
--- a/docs/misc/distro_mapping.txt
+++ b/docs/misc/distro_mapping.txt
@@ -5,7 +5,6 @@ other distros one needs to set the variables for the elements below
                  | Red Hat          |  Debian       |  Suse          |
 -----------------+------------------+---------------+----------------+
 CONFIG_LEAF_DIR  | sysconfig        | default       | sysconfig      |
-SUBSYS_DIR       | /var/run/subsys  | /var/run      | /var/run       |
 INITD_DIR        | /etc/rc.d/init.d | /etc/init.d   | /etc/init.d    |
 -----------------+------------------+---------------+----------------+

Re: [PATCH 07/12] tools/pygrub: store kernels in /var/run/xen/pygrub

[Andrew Cooper]

On 24/04/14 09:36, Olaf Hering wrote:
> Move location of temporary bootfiles from /var/run/xend/boot to
> /var/run/xen/pygrub. Create the subdirectory if does not exist, unless
> --output-directory=<dir> was specified.
>
> The reason for this change is that all entrys below /var/run have to be
> created at runtime in case /var/run is cleared on every boot.
>
> v3:
>  use /var/run/xen/pygrub instead of /var/run/pygrub
> v2:
>  update exception handling for python 2.4
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  tools/pygrub/src/pygrub | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
> index 54fecee..7759b1a 100644
> --- a/tools/pygrub/src/pygrub
> +++ b/tools/pygrub/src/pygrub
> @@ -14,6 +14,7 @@
>  #
>  
>  import os, sys, string, struct, tempfile, re, traceback
> +import errno
>  import copy
>  import logging
>  import platform
> @@ -757,7 +758,7 @@ if __name__ == "__main__":
>      debug = False
>      not_really = False
>      output_format = "sxp"
> -    output_directory = "/var/run/xend/boot"
> +    output_directory = None
>  
>      # what was passed in
>      incfg = { "kernel": None, "ramdisk": None, "args": "" }
> @@ -813,6 +814,16 @@ if __name__ == "__main__":
>      if debug:
>  	logging.basicConfig(level=logging.DEBUG)
>  
> +    if output_directory is None:
> +        output_directory = "/var/run/xen/pygrub"
> +        try:
> +            os.mkdir(output_directory, 0700)
> +        except OSError, exc:
> +            if exc.errno == errno.EEXIST and os.path.isdir(output_directory):
> +                pass
> +            else:
> +                raise

if not (exc.errno == errno.EEXIST and os.path.isdir(output_directory)):
    raise

There is no need for the pass statement.

~Andrew

[PATCH 07/12 v4] tools/pygrub: store kernels in /var/run/xen/pygrub

[Olaf Hering]

Move location of temporary bootfiles from /var/run/xend/boot to
/var/run/xen/pygrub. Create the subdirectory if does not exist, unless
--output-directory=<dir> was specified.

The reason for this change is that all entrys below /var/run have to be
created at runtime in case /var/run is cleared on every boot.

v4:
 update mkdir exception handling
v3:
 use /var/run/xen/pygrub instead of /var/run/pygrub
v2:
 update exception handling for python 2.4

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---

In case the whole series can/will go in, address a comment from Andrew.

 tools/pygrub/src/pygrub | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
index 54fecee..8781d83 100644
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -14,6 +14,7 @@
 #
 
 import os, sys, string, struct, tempfile, re, traceback
+import errno
 import copy
 import logging
 import platform
@@ -757,7 +758,7 @@ if __name__ == "__main__":
     debug = False
     not_really = False
     output_format = "sxp"
-    output_directory = "/var/run/xend/boot"
+    output_directory = None
 
     # what was passed in
     incfg = { "kernel": None, "ramdisk": None, "args": "" }
@@ -813,6 +814,14 @@ if __name__ == "__main__":
     if debug:
 	logging.basicConfig(level=logging.DEBUG)
 
+    if output_directory is None:
+        output_directory = "/var/run/xen/pygrub"
+        try:
+            os.mkdir(output_directory, 0700)
+        except OSError, exc:
+            if not (exc.errno == errno.EEXIST and os.path.isdir(output_directory)):
+                raise
+
     if output is None or output == "-":
         fd = sys.stdout.fileno()
     else:

Re: [PATCH 01/12] Config.mk: move directory list into BUILD_MAKE_VARS

[Ian Campbell]

On Thu, 2014-04-24 at 10:36 +0200, Olaf Hering wrote:
> To maintain the list of directories in a single place, move the existing
> list into its own variable and use it in buildmakevars2file.
> Required for upcoming changes.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

You haven't really CCd the right people though, this is not strictly
just a tools change. Apart from the path of the file being touched the
make function in question is also used by the stubdom stuff.

Probably doesn't matter for this particular patch, but I fear it might
for the upcoming changes you refer to. Did you test a stubdom build with
them?

> ---
>  Config.mk | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Config.mk b/Config.mk
> index 6a93533..a26e964 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -166,15 +166,17 @@ define move-if-changed
>  	if ! cmp -s $(1) $(2); then mv -f $(1) $(2); else rm -f $(1); fi
>  endef
>  
> +BUILD_MAKE_VARS := SBINDIR BINDIR LIBEXEC LIBDIR SHAREDIR PRIVATE_BINDIR \
> +                   XENFIRMWAREDIR XEN_CONFIG_DIR XEN_SCRIPT_DIR XEN_LOCK_DIR \
> +                   XEN_RUN_DIR XEN_PAGING_DIR
> +
>  buildmakevars2file = $(eval $(call buildmakevars2file-closure,$(1)))
>  define buildmakevars2file-closure
>      .PHONY: genpath
>      genpath:
>  	rm -f $(1).tmp;                                                     \
>  	$(foreach var,                                                      \
> -	          SBINDIR BINDIR LIBEXEC LIBDIR SHAREDIR PRIVATE_BINDIR     \
> -	          XENFIRMWAREDIR XEN_CONFIG_DIR XEN_SCRIPT_DIR XEN_LOCK_DIR \
> -	          XEN_RUN_DIR XEN_PAGING_DIR,                               \
> +	          $(BUILD_MAKE_VARS),                                       \

I think this can now go on the same line as the foreach. Possibly the
whole thing might fit on one line now too?

>  	          echo "$(var)=\"$($(var))\"" >>$(1).tmp;)        \
>  	$(call move-if-changed,$(1).tmp,$(1))
>  endef

Re: [PATCH 02/12] Config.mk: replace dependency to genpath with actual target

[Ian Campbell]

On Thu, 2014-04-24 at 10:36 +0200, Olaf Hering wrote:
> genpath is a detail of buildmakevars2file. Replace the dependency to
> genpath with the actual buildmakevars2file target. This change by itself
> does not fix any bug. Upcoming changes will add dependencies to
> $(target), but no rule exist to create $(target).
> 
> With this change a rule for $(target) is added, which in turn depends on
> genpath. No change in behaviour is expected by this patch.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Wrong CC again.

> ---
>  Config.mk                     |  1 +
>  stubdom/Makefile              | 16 ++++++++--------
>  tools/hotplug/common/Makefile |  2 +-
>  tools/python/Makefile         |  2 +-
>  4 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/Config.mk b/Config.mk
> index a26e964..eb3937c 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -173,6 +173,7 @@ BUILD_MAKE_VARS := SBINDIR BINDIR LIBEXEC LIBDIR SHAREDIR PRIVATE_BINDIR \
>  buildmakevars2file = $(eval $(call buildmakevars2file-closure,$(1)))
>  define buildmakevars2file-closure
>      .PHONY: genpath
> +    $(1): genpath
>      genpath:

This seems like an odd construction. The genpath in the middle seems
redundant with you change. Do you not just want the result to be:
	$(1):
		rm -f $(1).tmp
		$(foreach, etc...

with no .PHONY-ness?

Ian.

Re: [PATCH 03/12] Config.mk: add new macro buildmakevars2header

[Ian Campbell]

On Thu, 2014-04-24 at 10:36 +0200, Olaf Hering wrote:
> This macro is similar to buildmakevars2file, it just creates a C header
> file instead of shell style syntax. Upcoming changes will use this macro
> in libxl and libxc.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

All the same comments as to #2 wrt CCs, folding into a single line and
the .PHONY stuff.

Re: [PATCH 04/12] tools/libxl: use buildmakevars2header to create _paths.h

[Ian Campbell]

On Thu, 2014-04-24 at 10:36 +0200, Olaf Hering wrote:
> Replace usage of buildmakevars2file with buildmakevars2header. The macro
> generates a C header file, so remove code which converts shell variables
> into C defines. Also update the dependency, the macro itself creates a
> dependency to genpath for _paths.h. A temporary file is not needed
> anymore.

The penultimate sentence mentions genpath which per my comments on #2 I
think should go away. But other than that the patch is OK

> Signed-off-by: Olaf Hering <olaf@aepfle.de>

When you've made the genpath change in the previous patches and updated
this commit message to reflect that:
        Acked-by: Ian Campbell <ian.campbell@citrix.com> 

Thanks,
Ian

Re: [PATCH 05/12] tools/libxc: provide variable paths to libxc

[Ian Campbell]

On Thu, 2014-04-24 at 10:36 +0200, Olaf Hering wrote:
> In preparation to remove hardcoded /var/run/xen paths, provide
> XEN_RUN_DIR and related directories to xc_private.h. Similar code exists
> already for libxl, stubdom and other parts.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  tools/libxc/Makefile     | 8 ++++++++
>  tools/libxc/xc_private.h | 1 +

Please update .*ignore too. libxl has a blanket "tools/libxl/_*.[ch]"
which I think could be duplicate for libxc too, unless you find a file
named tools/libxc/_foo which shouldn't be ignored.

Other than that:
Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.

Re: [PATCH 07/12 v4] tools/pygrub: store kernels in /var/run/xen/pygrub

[Ian Campbell]

On Thu, 2014-04-24 at 14:12 +0200, Olaf Hering wrote:
> Move location of temporary bootfiles from /var/run/xend/boot to
> /var/run/xen/pygrub. Create the subdirectory if does not exist, unless
> --output-directory=<dir> was specified.

I'm not at all opposed to this change but why this last condition? Any
reason not to always create the directory if it doesn't exist?

Or should there be an existence check when you specify
--output-directory?


> The reason for this change is that all entrys below /var/run have to be
> created at runtime in case /var/run is cleared on every boot.
> 
> v4:
>  update mkdir exception handling
> v3:
>  use /var/run/xen/pygrub instead of /var/run/pygrub
> v2:
>  update exception handling for python 2.4
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
> 
> In case the whole series can/will go in, address a comment from Andrew.
> 
>  tools/pygrub/src/pygrub | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
> index 54fecee..8781d83 100644
> --- a/tools/pygrub/src/pygrub
> +++ b/tools/pygrub/src/pygrub
> @@ -14,6 +14,7 @@
>  #
>  
>  import os, sys, string, struct, tempfile, re, traceback
> +import errno
>  import copy
>  import logging
>  import platform
> @@ -757,7 +758,7 @@ if __name__ == "__main__":
>      debug = False
>      not_really = False
>      output_format = "sxp"
> -    output_directory = "/var/run/xend/boot"
> +    output_directory = None
>  
>      # what was passed in
>      incfg = { "kernel": None, "ramdisk": None, "args": "" }
> @@ -813,6 +814,14 @@ if __name__ == "__main__":
>      if debug:
>  	logging.basicConfig(level=logging.DEBUG)
>  
> +    if output_directory is None:
> +        output_directory = "/var/run/xen/pygrub"
> +        try:
> +            os.mkdir(output_directory, 0700)
> +        except OSError, exc:
> +            if not (exc.errno == errno.EEXIST and os.path.isdir(output_directory)):
> +                raise
> +
>      if output is None or output == "-":
>          fd = sys.stdout.fileno()
>      else:

Re: [PATCH 08/12] tools/hotplug: use XEN_SCRIPT_DIR instead of hardcoded path

[Ian Campbell]

On Thu, 2014-04-24 at 10:36 +0200, Olaf Hering wrote:
> @@ -45,11 +46,15 @@ install: all install-initd install-scripts install-udev
>  install-initd:
>  	[ -d $(DESTDIR)$(INITD_DIR) ] || $(INSTALL_DIR) $(DESTDIR)$(INITD_DIR)
>  	[ -d $(DESTDIR)$(SYSCONFIG_DIR) ] || $(INSTALL_DIR) $(DESTDIR)$(SYSCONFIG_DIR)
> -	$(INSTALL_PROG) $(XENDOMAINS_INITD) $(DESTDIR)$(INITD_DIR)
> +	set -e; for i in $(XEN_INIT_SCRIPTS); \
> +	    do \
> +		o="$(DESTDIR)$(INITD_DIR)/$$i"; \
> +		sed 's|@@XEN_SCRIPT_DIR@@|$(XEN_SCRIPT_DIR)|g' \
> +		init.d/$$i > "$$o"; \
> +		chmod 755 "$$o"; \
> +	done

I'm not entirely comfortable with doing this sort of thing at install,
rather than build, time. It has subtle issues with partial success and
the chmod etc.

I think you should rename all the input files foo.in and generate foo
from that at build time, then install as normal here using INSTALL_PROG.

Don't forget to update .*ignore.

Ian.

Re: [PATCH 09/12] tools/hotplug: create XEN_RUN_DIR at runtime

[Ian Campbell]

On Thu, 2014-04-24 at 10:36 +0200, Olaf Hering wrote:
> Create XEN_RUN_DIR instead of hardcoded path at runtime.
> /var/run might be empty on startup.

The last sentence is a bit odd in the context, surely the point is that
XEN_RUN_DIR might != /var/run? /var/run being empty is already handled.

> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Acked-by: Ian Campbell <ian.campbell@citrix.com> (please consider
changing the last sentence though)

Re: [PATCH 10/12] tools/hotplug: create XEN_LOCK_DIR at runtime

[Ian Campbell]

On Thu, 2014-04-24 at 10:36 +0200, Olaf Hering wrote:
> XEN_LOCK_DIR is a compile-time setting and may not exist on the target
> host.

Maybe insert some words to the affect that it might be in a tmpfs and
therefore need creating on boot (otherwise the question becomes why
isn't this done at install time)

> Signed-off-by: Olaf Hering <olaf@aepfle.de>

With that clarified:
        Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  tools/hotplug/Linux/init.d/xencommons | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/hotplug/Linux/init.d/xencommons b/tools/hotplug/Linux/init.d/xencommons
> index 2326a9c..0b14568 100644
> --- a/tools/hotplug/Linux/init.d/xencommons
> +++ b/tools/hotplug/Linux/init.d/xencommons
> @@ -73,6 +73,7 @@ do_start () {
>  	modprobe xen-acpi-processor 2>/dev/null
>  	modprobe blktap2 2>/dev/null || modprobe blktap 2>/dev/null
>  	mkdir -p ${XEN_RUN_DIR}
> +	mkdir -p ${XEN_LOCK_DIR}
>  
>  	if ! `${BINDIR}/xenstore-read -s / >/dev/null 2>&1`
>  	then

Re: [PATCH 11/12] tools/examples: remove obsolete install targets

[Ian Campbell]

On Thu, 2014-04-24 at 10:36 +0200, Olaf Hering wrote:
> install-hotplug and install-udev are obsolete since commit 57bcfa11
> ("tools/hotplug: Separate OS-specific scripts.")
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

I think this can go in independent of the stuff before it in this
series. Please confirm.

Re: [PATCH 01/12] Config.mk: move directory list into BUILD_MAKE_VARS

[Olaf Hering]

On Fri, May 02, Ian Campbell wrote:

> Probably doesn't matter for this particular patch, but I fear it might
> for the upcoming changes you refer to. Did you test a stubdom build with
> them?

Yes.


> >  	$(foreach var,                                                      \
> > -	          SBINDIR BINDIR LIBEXEC LIBDIR SHAREDIR PRIVATE_BINDIR     \
> > -	          XENFIRMWAREDIR XEN_CONFIG_DIR XEN_SCRIPT_DIR XEN_LOCK_DIR \
> > -	          XEN_RUN_DIR XEN_PAGING_DIR,                               \
> > +	          $(BUILD_MAKE_VARS),                                       \
> 
> I think this can now go on the same line as the foreach. Possibly the
> whole thing might fit on one line now too?

It was too long, but I have to double check.

Olaf

Re: [PATCH 05/12] tools/libxc: provide variable paths to libxc

[Olaf Hering]

On Fri, May 02, Ian Campbell wrote:

> On Thu, 2014-04-24 at 10:36 +0200, Olaf Hering wrote:
> > In preparation to remove hardcoded /var/run/xen paths, provide
> > XEN_RUN_DIR and related directories to xc_private.h. Similar code exists
> > already for libxl, stubdom and other parts.
> > 
> > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > ---
> >  tools/libxc/Makefile     | 8 ++++++++
> >  tools/libxc/xc_private.h | 1 +
> 
> Please update .*ignore too. libxl has a blanket "tools/libxl/_*.[ch]"
> which I think could be duplicate for libxc too, unless you find a file
> named tools/libxc/_foo which shouldn't be ignored.

I have noticed that, will update the patch.

Olaf

Re: [PATCH 12/12] remove obsolete SUBSYS_DIR variable

[Ian Campbell]

On Thu, 2014-04-24 at 10:36 +0200, Olaf Hering wrote:
> /var/run is a runtime directory. It is not supposed to be packaged. The
> xendomains runlevel script checks at runtime if /var/run/subsys or
> /var/run has to be used. Remove the unused SUBSYS_DIR variable from
> Config.mk and distro_mapping.txt.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  Config.mk                    | 1 -
>  docs/misc/distro_mapping.txt | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/Config.mk b/Config.mk
> index d8fe87b..3cc1ab1 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -75,7 +75,6 @@ endef
>  
>  # See distro_mapping.txt for other options
>  $(eval $(call setvar_dir,CONFIG_LEAF_DIR,,/etc/sysconfig,sysconfig,default))
> -$(eval $(call setvar_dir,SUBSYS_DIR,/var/run,/subsys,/subsys,))
>  $(eval $(call setvar_dir,INITD_DIR,/etc,/rc.d/init.d,/rc.d/init.d,/init.d))
>  
>  ifneq ($(EXTRA_PREFIX),)
> diff --git a/docs/misc/distro_mapping.txt b/docs/misc/distro_mapping.txt
> index f56e281..f849d07 100644
> --- a/docs/misc/distro_mapping.txt
> +++ b/docs/misc/distro_mapping.txt

I think you also want to remove the comment from xendomains:
        # See docs/misc/distro_mapping.txt
        if [ -d /var/lock/subsys ]; then
                LOCKFILE=/var/lock/subsys/xendomains
        else
                LOCKFILE=/var/lock/xendomains
        fi

Other than that I think this one can go in independently of the rest of
this series, right?

Re: [PATCH 07/12 v4] tools/pygrub: store kernels in /var/run/xen/pygrub

[Olaf Hering]

On Fri, May 02, Ian Campbell wrote:

> On Thu, 2014-04-24 at 14:12 +0200, Olaf Hering wrote:
> > Move location of temporary bootfiles from /var/run/xend/boot to
> > /var/run/xen/pygrub. Create the subdirectory if does not exist, unless
> > --output-directory=<dir> was specified.
> 
> I'm not at all opposed to this change but why this last condition? Any
> reason not to always create the directory if it doesn't exist?
> 
> Or should there be an existence check when you specify
> --output-directory?

Without --output-directory the default should be created internally.
With --output-directory the specified dir should exist already.
Perhaps new code can be added to check if the given path is a directory.


Olaf

Re: [PATCH 01/12] Config.mk: move directory list into BUILD_MAKE_VARS

[Ian Campbell]

On Fri, 2014-05-02 at 12:57 +0200, Olaf Hering wrote:
> On Fri, May 02, Ian Campbell wrote:
> 
> > Probably doesn't matter for this particular patch, but I fear it might
> > for the upcoming changes you refer to. Did you test a stubdom build with
> > them?
> 
> Yes.
> 
> 
> > >  	$(foreach var,                                                      \
> > > -	          SBINDIR BINDIR LIBEXEC LIBDIR SHAREDIR PRIVATE_BINDIR     \
> > > -	          XENFIRMWAREDIR XEN_CONFIG_DIR XEN_SCRIPT_DIR XEN_LOCK_DIR \
> > > -	          XEN_RUN_DIR XEN_PAGING_DIR,                               \
> > > +	          $(BUILD_MAKE_VARS),                                       \
> > 
> > I think this can now go on the same line as the foreach. Possibly the
> > whole thing might fit on one line now too?
> 
> It was too long, but I have to double check.

If it is too long then:
        $(foreach var,$(BUILD_MAKE_VARS),                                \
                         echo "$(var)=\"$($(var))\"" >>$(1).tmp;)
        \
would be OK (but with proper indentation)

Re: [PATCH 07/12 v4] tools/pygrub: store kernels in /var/run/xen/pygrub

[Ian Campbell]

On Fri, 2014-05-02 at 13:02 +0200, Olaf Hering wrote:
> On Fri, May 02, Ian Campbell wrote:
> 
> > On Thu, 2014-04-24 at 14:12 +0200, Olaf Hering wrote:
> > > Move location of temporary bootfiles from /var/run/xend/boot to
> > > /var/run/xen/pygrub. Create the subdirectory if does not exist, unless
> > > --output-directory=<dir> was specified.
> > 
> > I'm not at all opposed to this change but why this last condition? Any
> > reason not to always create the directory if it doesn't exist?
> > 
> > Or should there be an existence check when you specify
> > --output-directory?
> 
> Without --output-directory the default should be created internally.
> With --output-directory the specified dir should exist already.
> Perhaps new code can be added to check if the given path is a directory.

Probably. That would also lead to a potentially simplicifaction and
keeping the default up at the top:

        initially:
	output_directory = "/var/path/todefault"

        then when handling --output:
		fi not os.path.isdir(argument):
			ARGH!
		output_directory = argument

        eventually:
        
        if not os.path.isdir(argument): # will never hit if --output-directory was given
                os.path.mkdir
                
What do you think?