[PATCH 0/9] NetBSD: minor fixes and hotplug execution

[PATCH 0/9] NetBSD: minor fixes and hotplug execution

[Roger Pau Monne]

This series contains minor fixes to build xen on NetBSD and adds 
hotplug support to NetBSD. After this, xen-unstable should build 
and work out-of-the-box on NetBSD.

This fixes are not related, and as far as I know, can be applied 
sepparetedly:

[PATCH 1/9] xenstore: don't print an error when gntdev cannot be
[PATCH 2/9] tools/build: fix pygrub linking
[PATCH 3/9] pygrub: don't leave fds open
[PATCH 4/9] build: include Tools.mk first in tools/Rules.mk
[PATCH 5/9] libxl: react correctly to POLLHUP
[PATCH 6/9] libxl: check backend state before setting it to
[PATCH 7/9] hotplug/NetBSD: check type of file to attach from params

This two should be applied together:

[PATCH 8/9] libxl: call hotplug scripts from xl for NetBSD
[PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

Thanks, Roger.

[PATCH 1/9] xenstore: don't print an error when gntdev cannot be opened

[Roger Pau Monne]

NetBSD doesn't have a gntdev, but we should not print an error when
falling back to the previous implementation.

Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxc/xc_private.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
index 3ceed3e..3e03a91 100644
--- a/tools/libxc/xc_private.c
+++ b/tools/libxc/xc_private.c
@@ -174,7 +174,7 @@ static struct xc_interface_core *xc_interface_open_common(xentoollog_logger *log
         xch->ops = xch->osdep.init(xch, type);
         if ( xch->ops == NULL )
         {
-            ERROR("OSDEP: interface %d (%s) not supported on this platform",
+            DPRINTF("OSDEP: interface %d (%s) not supported on this platform",
                   type, xc_osdep_type_name(type));
             goto err_put_iface;
         }
-- 
1.7.7.5 (Apple Git-26)

[PATCH 2/9] tools/build: fix pygrub linking

[Roger Pau Monne]

Prevent creating a symlink to $(DESTDIR)/$(BINDIR) if it is the same
as $(PRIVATE_BINDIR)

This fixes NetBSD install, where $(DESTDIR)/$(BINDIR) ==
$(PRIVATE_BINDIR).

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Christoph Egger <Christoph.Egger@amd.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/pygrub/Makefile |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
index bd22dd4..835fd43 100644
--- a/tools/pygrub/Makefile
+++ b/tools/pygrub/Makefile
@@ -14,7 +14,9 @@ install: all
 		$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" \
 		--install-scripts=$(PRIVATE_BINDIR) --force
 	$(INSTALL_DIR) $(DESTDIR)/var/run/xend/boot
+#ifneq ($(readlink -f $(PRIVATE_BINDIR)), $(readlink -f $(DESTDIR)/$(BINDIR)))
 	ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR)
+#endif
 
 .PHONY: clean
 clean:
-- 
1.7.7.5 (Apple Git-26)

[PATCH 3/9] pygrub: don't leave fds open

[Roger Pau Monne]

On NetBSD a block device can only be opened once, so make sure pygrub
closes it every time, if this is not done libfsimage is not able to
open the disk later.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Christoph Egger <Christoph.Egger@amd.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/pygrub/src/pygrub |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
index 6dd44ac..ad78c22 100644
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -67,6 +67,7 @@ def get_solaris_slice(file, offset):
     fd = os.open(file, os.O_RDONLY)
     os.lseek(fd, offset + (DK_LABEL_LOC * SECTOR_SIZE), 0)
     buf = os.read(fd, 512)
+    os.close(fd)
     if struct.unpack("<H", buf[508:510])[0] != DKL_MAGIC:
         raise RuntimeError, "Invalid disklabel magic"
 
@@ -93,6 +94,7 @@ def get_fs_offset_gpt(file):
         buf = os.read(fd, partsize)
         offsets.append(struct.unpack("<Q", buf[32:40])[0] * SECTOR_SIZE)
         i -= 1
+    os.close(fd)
     return offsets
 
 FDISK_PART_SOLARIS=0xbf
@@ -116,6 +118,7 @@ def get_partition_offsets(file):
 
     fd = os.open(file, os.O_RDONLY)
     buf = os.read(fd, 512)
+    os.close(fd)
     for poff in (446, 462, 478, 494): # partition offsets
 
         # MBR contains a 16 byte descriptor per partition
-- 
1.7.7.5 (Apple Git-26)

[PATCH 4/9] build: include Tools.mk first in tools/Rules.mk

[Roger Pau Monne]

Tools.mk should be included first, or PREFIX is not honoured in the
other conf/ files that define the paths of several tools.

Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/Rules.mk |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/Rules.mk b/tools/Rules.mk
index 1e928b7..2dd8ed3 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -3,8 +3,8 @@
 # `all' is the default target
 all:
 
-include $(XEN_ROOT)/Config.mk
 -include $(XEN_ROOT)/config/Tools.mk
+include $(XEN_ROOT)/Config.mk
 
 export _INSTALL := $(INSTALL)
 INSTALL = $(XEN_ROOT)/tools/cross-install
-- 
1.7.7.5 (Apple Git-26)

[PATCH 5/9] libxl: react correctly to POLLHUP

[Roger Pau Monne]

On datacopier_readable ignore PULLHUP, since we will shortly process
the death of the underlying process.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl_aoutils.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 99972a2..fa07154 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -102,6 +102,14 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
     libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, toread);
     STATE_AO_GC(dc->ao);
 
+    if (revents & POLLHUP) {
+        LOG(DEBUG, "received POLLHUP on %s during copy of %s, "
+                   "waiting for process to die",
+                   dc->readwhat, dc->copywhat);
+        libxl__ev_fd_deregister(gc, &dc->toread);
+        return;
+    }
+
     if (revents & ~POLLIN) {
         LOG(ERROR, "unexpected poll event 0x%x (should be POLLIN)"
             " on %s during copy of %s", revents, dc->readwhat, dc->copywhat);
-- 
1.7.7.5 (Apple Git-26)

[PATCH 6/9] libxl: check backend state before setting it to "closing"

[Roger Pau Monne]

Check backend state before unconditionally setting it to "closing"
(5), since it might already be in "closed" (6).

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl_device.c |   53 +++++++++++++++++++++++++++++++------------
 1 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 8ae2bb5..18002a0 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -681,10 +681,11 @@ void libxl__initiate_device_remove(libxl__egc *egc,
                                    libxl__ao_device *aodev)
 {
     STATE_AO_GC(aodev->ao);
-    libxl_ctx *ctx = libxl__gc_owner(gc);
     xs_transaction_t t = 0;
     char *be_path = libxl__device_backend_path(gc, aodev->dev);
     char *state_path = libxl__sprintf(gc, "%s/state", be_path);
+    char *online_path = GCSPRINTF("%s/online", be_path);
+    const char *state;
     int rc = 0;
 
     /* We init this here because we might call device_hotplug_done
@@ -701,23 +702,44 @@ void libxl__initiate_device_remove(libxl__egc *egc,
         goto out;
     }
 
-retry_transaction:
-    t = xs_transaction_start(ctx->xsh);
-    if (aodev->force)
-        libxl__xs_path_cleanup(gc, t,
-                               libxl__device_frontend_path(gc, aodev->dev));
-    xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/online", be_path), "0", strlen("0"));
-    xs_write(ctx->xsh, t, state_path, "5", strlen("5"));
-    if (!xs_transaction_end(ctx->xsh, t, 0)) {
-        if (errno == EAGAIN)
-            goto retry_transaction;
-        else {
-            rc = ERROR_FAIL;
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) {
+            LOG(ERROR, "unable to start transaction");
+            goto out;
+        }
+
+        rc = libxl__xs_read_checked(gc, t, state_path, &state);
+        if (rc) {
+            LOG(ERROR, "unable to read device state from path %s", state_path);
             goto out;
         }
+
+        if (aodev->force)
+            libxl__xs_path_cleanup(gc, t,
+                                   libxl__device_frontend_path(gc, aodev->dev));
+
+        /*
+         * Check if device is already in "closed" state, in which case
+         * it should not be changed.
+         */
+        if (atoi(state) != XenbusStateClosed) {
+            rc = libxl__xs_write_checked(gc, t, online_path, "0");
+            if (rc) {
+                LOG(ERROR, "unable to write to xenstore path %s", online_path);
+                goto out;
+            }
+            rc = libxl__xs_write_checked(gc, t, state_path, "5");
+            if (rc) {
+                LOG(ERROR, "unable to write to xenstore path %s", state_path);
+                goto out;
+            }
+        }
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
     }
-    /* mark transaction as ended, to prevent double closing it on out_ok */
-    t = 0;
 
     libxl__device_destroy_tapdisk(gc, be_path);
 
@@ -733,6 +755,7 @@ retry_transaction:
     return;
 
 out:
+    libxl__xs_transaction_abort(gc, &t);
     aodev->rc = rc;
     device_hotplug_done(egc, aodev);
     return;
-- 
1.7.7.5 (Apple Git-26)

[PATCH 7/9] hotplug/NetBSD: check type of file to attach from params

[Roger Pau Monne]

xend used to set the xenbus backend entry "type" to either "phy" or
"file", but now libxl sets it to "phy" for both file and block device.
We have to manually check for the type of the "param" filed in order
to detect if we are trying to attach a file or a block device.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Christoph Egger <Christoph.Egger@amd.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/hotplug/NetBSD/block |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/tools/hotplug/NetBSD/block b/tools/hotplug/NetBSD/block
index cf5ff3a..95acaa0 100644
--- a/tools/hotplug/NetBSD/block
+++ b/tools/hotplug/NetBSD/block
@@ -19,8 +19,12 @@ error() {
 
 xpath=$1
 xstatus=$2
-xtype=$(xenstore-read "$xpath/type")
 xparams=$(xenstore-read "$xpath/params")
+if [ -b $xparams ]; then
+	xtype="phy"
+else
+	xtype="file"
+fi
 
 case $xstatus in
 6)
-- 
1.7.7.5 (Apple Git-26)

[PATCH 8/9] libxl: call hotplug scripts from xl for NetBSD

[Roger Pau Monne]

Add the missing NetBSD functions to call hotplug scripts, and disable
xenbackendd if libxl/disable_udev is not set.

Cc: Christoph Egger <Christoph.Egger@amd.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl_netbsd.c      |   57 ++++++++++++++++++++++++++++++++++++++-
 tools/xenbackendd/xenbackendd.c |    8 +++++-
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
index 28cdf21..9587833 100644
--- a/tools/libxl/libxl_netbsd.c
+++ b/tools/libxl/libxl_netbsd.c
@@ -32,10 +32,65 @@ char *libxl__devid_to_localdev(libxl__gc *gc, int devid)
 }
 
 /* Hotplug scripts caller functions */
+static int libxl__hotplug(libxl__gc *gc, libxl__device *dev, char ***args,
+                          libxl__device_action action)
+{
+    char *be_path = libxl__device_backend_path(gc, dev);
+    char *script;
+    int nr = 0, rc = 0, arraysize = 4;
+
+    script = libxl__xs_read(gc, XBT_NULL,
+                            GCSPRINTF("%s/%s", be_path, "script"));
+    if (!script) {
+        LOGEV(ERROR, errno, "unable to read script from %s", be_path);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    GCNEW_ARRAY(*args, arraysize);
+    (*args)[nr++] = script;
+    (*args)[nr++] = be_path;
+    (*args)[nr++] = GCSPRINTF("%d", action == DEVICE_CONNECT ?
+                                    XenbusStateInitWait : XenbusStateClosed);
+    (*args)[nr++] = NULL;
+    assert(nr == arraysize);
+
+out:
+    return rc;
+}
+
 int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
                                    char ***args, char ***env,
                                    libxl__device_action action,
                                    int num_exec)
 {
-    return 0;
+    char *disable_udev = libxl__xs_read(gc, XBT_NULL, DISABLE_UDEV_PATH);
+    int rc;
+
+    /* Check if we have to run hotplug scripts */
+    if (!disable_udev || num_exec > 0) {
+        rc = 0;
+        goto out;
+    }
+
+    switch (dev->backend_kind) {
+    case LIBXL__DEVICE_KIND_VBD:
+    case LIBXL__DEVICE_KIND_VIF:
+        if (num_exec != 0) {
+            rc = 0;
+            goto out;
+        }
+        rc = libxl__hotplug(gc, dev, args, action);
+        if (!rc) rc = 1;
+        break;
+    default:
+        /* If no need to execute any hotplug scripts,
+         * call the callback manually
+         */
+        rc = 0;
+        break;
+    }
+
+out:
+    return rc;
 }
diff --git a/tools/xenbackendd/xenbackendd.c b/tools/xenbackendd/xenbackendd.c
index 6b5bb8e..5381a2a 100644
--- a/tools/xenbackendd/xenbackendd.c
+++ b/tools/xenbackendd/xenbackendd.c
@@ -33,6 +33,7 @@
 #define DEVTYPE_UNKNOWN 0
 #define DEVTYPE_VIF 1
 #define DEVTYPE_VBD 2
+#define DISABLE_EXEC "libxl/disable_udev"
 
 #define DOMAIN_PATH "/local/domain/0"
 
@@ -149,7 +150,7 @@ main(int argc, char * const argv[])
 	unsigned int num;
 	char *s;
 	int state;
-	char *sstate;
+	char *sstate, *sdisable;
 	char *p;
 	char buf[80];
 	int type;
@@ -245,6 +246,10 @@ main(int argc, char * const argv[])
 			continue;
 		}
 
+		sdisable = xs_read(xs, XBT_NULL, DISABLE_EXEC, 0);
+		if (sdisable)
+			goto next1;
+
 		if (strlen(vec[XS_WATCH_PATH]) < sizeof("state"))
 			goto next1;
 
@@ -314,6 +319,7 @@ next2:
 		free(sstate);
 
 next1:
+		free(sdisable);
 		free(vec);
 	}
 
-- 
1.7.7.5 (Apple Git-26)

[PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

[Roger Pau Monne]

xenbackendd is not needed by the xl toolstack, so move it's launch to
the xend script.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Christoph Egger <Christoph.Egger@amd.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/hotplug/NetBSD/rc.d/xencommons |   29 ++---------------
 tools/hotplug/NetBSD/rc.d/xend       |   57 +++++++++++++++++++++++++++++++--
 2 files changed, 57 insertions(+), 29 deletions(-)

diff --git a/tools/hotplug/NetBSD/rc.d/xencommons b/tools/hotplug/NetBSD/rc.d/xencommons
index c0d87bf..fe4c9ac 100644
--- a/tools/hotplug/NetBSD/rc.d/xencommons
+++ b/tools/hotplug/NetBSD/rc.d/xencommons
@@ -29,8 +29,6 @@ XENBACKENDD_PIDFILE="/var/run/xenbackendd.pid"
 
 xen_precmd()
 {
-	mkdir -p /var/run/xend || exit 1
-	mkdir -p /var/run/xend/boot || exit 1
 	mkdir -p /var/run/xenstored || exit 1
 }
 
@@ -46,7 +44,7 @@ xen_startcmd()
 			XENSTORED_ROOTDIR="/var/lib/xenstored"
 		fi
 		rm -f ${XENSTORED_ROOTDIR}/tdb* >/dev/null 2>&1
-		printf "Starting xenservices: xenstored, xenconsoled, xenbackendd."
+		printf "Starting xenservices: xenstored, xenconsoled."
 		XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}"
 		if [ -n "${XENSTORED_TRACE}" ]; then
 			XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log"
@@ -58,7 +56,7 @@ xen_startcmd()
 			sleep 1
 		done
 	else
-		printf "Starting xenservices: xenconsoled, xenbackendd."
+		printf "Starting xenservices: xenconsoled."
 	fi
 
 	XENCONSOLED_ARGS=""
@@ -68,13 +66,6 @@ xen_startcmd()
 
 	${SBINDIR}/xenconsoled ${XENCONSOLED_ARGS}
 
-	XENBACKENDD_ARGS=""
-	if [ -n "${XENBACKENDD_DEBUG}" ]; then
-		XENBACKENDD_ARGS="${XENBACKENDD_ARGS} -d"
-	fi
-
-	${SBINDIR}/xenbackendd ${XENBACKENDD_ARGS}
-
 	printf "\n"
 
 	printf "Setting domain 0 name.\n"
@@ -87,8 +78,6 @@ xen_stop()
 	printf "Stopping xencommons.\n"
 	printf "WARNING: Not stopping xenstored, as it cannot be restarted.\n"
 
-	rc_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
-	pids="$pids $rc_pid"
 	rc_pid=$(check_pidfile ${XENCONSOLED_PIDFILE} ${SBINDIR}/xenconsoled)
 	pids="$pids $rc_pid"
 
@@ -108,17 +97,12 @@ xen_status()
 		pids="$pids $xenconsoled_pid"
 	fi
 
-	xenbackend_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
-	if test -n ${xenbackend_pid}; then
-		pids="$pids $xenbackend_pid"
-	fi
-
-	if test -n "$xenbackend_pid" -a -n "$xenconsoled_pid" -a -n "$xenstored_pid";
+	if test -n "$xenconsoled_pid" -a -n "$xenstored_pid";
 	then
 		echo "xencommons are running as pids $pids."
 		return 0
 	fi
-	if test -z "$xenbackend_pid" -a -z "$xenconsoled_pid" -a -z "$xenstored_pid";
+	if test -z "$xenconsoled_pid" -a -z "$xenstored_pid";
 	then
 		echo "xencommons are not running."
 		return 0
@@ -134,11 +118,6 @@ xen_status()
 	else
 		echo "xenconsoled is not running."
 	fi
-	if test -n $xenbackend_pid; then
-		echo "xenbackendd is running as pid $xenbackend_pid."
-	else
-		echo "xenbackendd is not running."
-	fi
 }
 
 load_rc_config $name
diff --git a/tools/hotplug/NetBSD/rc.d/xend b/tools/hotplug/NetBSD/rc.d/xend
index ead9ee0..7fd9bc0 100644
--- a/tools/hotplug/NetBSD/rc.d/xend
+++ b/tools/hotplug/NetBSD/rc.d/xend
@@ -15,10 +15,59 @@ export PATH
 
 name="xend"
 rcvar=$name
-command="${SBINDIR}/xend"
-command_args="start"
-command_interpreter=`head -n 1 ${command} | awk '{ print substr($0,3) }'`
-sig_stop="SIGKILL"
+start_precmd="xend_precmd"
+start_cmd="xend_startcmd"
+stop_cmd="xend_stop"
+status_cmd="xend_status"
+extra_commands="status"
+required_files="/kern/xen/privcmd"
+
+XENBACKENDD_PIDFILE="/var/run/xenbackendd.pid"
+#XENBACKENDD_DEBUG=1
+
+xend_precmd()
+{
+	mkdir -p /var/run/xend || exit 1
+	mkdir -p /var/run/xend/boot || exit 1
+}
+
+xend_startcmd()
+{
+	printf "Starting xenbackendd.\n"
+
+	XENBACKENDD_ARGS=""
+	if [ -n "${XENBACKENDD_DEBUG}" ]; then
+		XENBACKENDD_ARGS="${XENBACKENDD_ARGS} -d"
+	fi
+
+	${SBINDIR}/xenbackendd ${XENBACKENDD_ARGS}
+
+	printf "Starting xend.\n"
+	${SBINDIR}/xend start
+}
+
+xend_stop()
+{
+	printf "Stopping xenbackendd, xend\n"
+	xb_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
+	xend_pid=`ps x -o pid,command|grep ${SBINDIR}/xend|awk '{ print $1 }'`
+	if test -n "$xb_pid";
+	then
+		kill -${sig_stop:-TERM} $xb_pid
+	fi
+	if test -n "$xend_pid";
+	then
+		kill -${sig_stop:-KILL} $xend_pid
+	fi
+	wait_for_pids $xb_pid $xend_pid
+	rm -f /var/lock/subsys/xend /var/lock/xend /var/run/xenbackendd.pid
+}
+
+xend_status()
+{
+	${SBINDIR}/xend status
+}
 
 load_rc_config $name
 run_rc_command "$1"
+
-- 
1.7.7.5 (Apple Git-26)

Re: [PATCH 2/9] tools/build: fix pygrub linking

[Christoph Egger]

On 07/11/12 12:23, Roger Pau Monne wrote:

> Prevent creating a symlink to $(DESTDIR)/$(BINDIR) if it is the same
> as $(PRIVATE_BINDIR)
> 
> This fixes NetBSD install, where $(DESTDIR)/$(BINDIR) ==
> $(PRIVATE_BINDIR).
> 
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Christoph Egger <Christoph.Egger@amd.com>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>


Acked-by: Christoph Egger <Christoph.Egger@amd.com>

> ---
>  tools/pygrub/Makefile |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
> index bd22dd4..835fd43 100644
> --- a/tools/pygrub/Makefile
> +++ b/tools/pygrub/Makefile
> @@ -14,7 +14,9 @@ install: all
>  		$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" \
>  		--install-scripts=$(PRIVATE_BINDIR) --force
>  	$(INSTALL_DIR) $(DESTDIR)/var/run/xend/boot
> +#ifneq ($(readlink -f $(PRIVATE_BINDIR)), $(readlink -f $(DESTDIR)/$(BINDIR)))
>  	ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR)
> +#endif
>  
>  .PHONY: clean
>  clean:



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 3/9] pygrub: don't leave fds open

[Christoph Egger]

On 07/11/12 12:23, Roger Pau Monne wrote:

> On NetBSD a block device can only be opened once, so make sure pygrub
> closes it every time, if this is not done libfsimage is not able to
> open the disk later.
> 
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Christoph Egger <Christoph.Egger@amd.com>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>


Acked-by: Christoph Egger <Christoph.Egger@amd.com>

> ---
>  tools/pygrub/src/pygrub |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
> index 6dd44ac..ad78c22 100644
> --- a/tools/pygrub/src/pygrub
> +++ b/tools/pygrub/src/pygrub
> @@ -67,6 +67,7 @@ def get_solaris_slice(file, offset):
>      fd = os.open(file, os.O_RDONLY)
>      os.lseek(fd, offset + (DK_LABEL_LOC * SECTOR_SIZE), 0)
>      buf = os.read(fd, 512)
> +    os.close(fd)
>      if struct.unpack("<H", buf[508:510])[0] != DKL_MAGIC:
>          raise RuntimeError, "Invalid disklabel magic"
>  
> @@ -93,6 +94,7 @@ def get_fs_offset_gpt(file):
>          buf = os.read(fd, partsize)
>          offsets.append(struct.unpack("<Q", buf[32:40])[0] * SECTOR_SIZE)
>          i -= 1
> +    os.close(fd)
>      return offsets
>  
>  FDISK_PART_SOLARIS=0xbf
> @@ -116,6 +118,7 @@ def get_partition_offsets(file):
>  
>      fd = os.open(file, os.O_RDONLY)
>      buf = os.read(fd, 512)
> +    os.close(fd)
>      for poff in (446, 462, 478, 494): # partition offsets
>  
>          # MBR contains a 16 byte descriptor per partition



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 4/9] build: include Tools.mk first in tools/Rules.mk

[Christoph Egger]

On 07/11/12 12:23, Roger Pau Monne wrote:

> Tools.mk should be included first, or PREFIX is not honoured in the
> other conf/ files that define the paths of several tools.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>


Acked-by: Christoph Egger <Christoph.Egger@amd.com>

> ---
>  tools/Rules.mk |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 1e928b7..2dd8ed3 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -3,8 +3,8 @@
>  # `all' is the default target
>  all:
>  
> -include $(XEN_ROOT)/Config.mk
>  -include $(XEN_ROOT)/config/Tools.mk
> +include $(XEN_ROOT)/Config.mk
>  
>  export _INSTALL := $(INSTALL)
>  INSTALL = $(XEN_ROOT)/tools/cross-install



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 7/9] hotplug/NetBSD: check type of file to attach from params

[Christoph Egger]

On 07/11/12 12:23, Roger Pau Monne wrote:

> xend used to set the xenbus backend entry "type" to either "phy" or
> "file", but now libxl sets it to "phy" for both file and block device.
> We have to manually check for the type of the "param" filed in order
> to detect if we are trying to attach a file or a block device.
> 
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Christoph Egger <Christoph.Egger@amd.com>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>


Acked-by: Christoph Egger <Christoph.Egger@amd.com>

> ---
>  tools/hotplug/NetBSD/block |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/hotplug/NetBSD/block b/tools/hotplug/NetBSD/block
> index cf5ff3a..95acaa0 100644
> --- a/tools/hotplug/NetBSD/block
> +++ b/tools/hotplug/NetBSD/block
> @@ -19,8 +19,12 @@ error() {
>  
>  xpath=$1
>  xstatus=$2
> -xtype=$(xenstore-read "$xpath/type")
>  xparams=$(xenstore-read "$xpath/params")
> +if [ -b $xparams ]; then
> +	xtype="phy"
> +else
> +	xtype="file"
> +fi
>  
>  case $xstatus in
>  6)



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

[Christoph Egger]

On 07/11/12 12:23, Roger Pau Monne wrote:

> xenbackendd is not needed by the xl toolstack, so move it's launch to
> the xend script.
> 
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Christoph Egger <Christoph.Egger@amd.com>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>


Acked-by: Christoph Egger <Christoph.Egger@amd.com>

> ---
>  tools/hotplug/NetBSD/rc.d/xencommons |   29 ++---------------
>  tools/hotplug/NetBSD/rc.d/xend       |   57 +++++++++++++++++++++++++++++++--
>  2 files changed, 57 insertions(+), 29 deletions(-)
> 
> diff --git a/tools/hotplug/NetBSD/rc.d/xencommons b/tools/hotplug/NetBSD/rc.d/xencommons
> index c0d87bf..fe4c9ac 100644
> --- a/tools/hotplug/NetBSD/rc.d/xencommons
> +++ b/tools/hotplug/NetBSD/rc.d/xencommons
> @@ -29,8 +29,6 @@ XENBACKENDD_PIDFILE="/var/run/xenbackendd.pid"
>  
>  xen_precmd()
>  {
> -	mkdir -p /var/run/xend || exit 1
> -	mkdir -p /var/run/xend/boot || exit 1
>  	mkdir -p /var/run/xenstored || exit 1
>  }
>  
> @@ -46,7 +44,7 @@ xen_startcmd()
>  			XENSTORED_ROOTDIR="/var/lib/xenstored"
>  		fi
>  		rm -f ${XENSTORED_ROOTDIR}/tdb* >/dev/null 2>&1
> -		printf "Starting xenservices: xenstored, xenconsoled, xenbackendd."
> +		printf "Starting xenservices: xenstored, xenconsoled."
>  		XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}"
>  		if [ -n "${XENSTORED_TRACE}" ]; then
>  			XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log"
> @@ -58,7 +56,7 @@ xen_startcmd()
>  			sleep 1
>  		done
>  	else
> -		printf "Starting xenservices: xenconsoled, xenbackendd."
> +		printf "Starting xenservices: xenconsoled."
>  	fi
>  
>  	XENCONSOLED_ARGS=""
> @@ -68,13 +66,6 @@ xen_startcmd()
>  
>  	${SBINDIR}/xenconsoled ${XENCONSOLED_ARGS}
>  
> -	XENBACKENDD_ARGS=""
> -	if [ -n "${XENBACKENDD_DEBUG}" ]; then
> -		XENBACKENDD_ARGS="${XENBACKENDD_ARGS} -d"
> -	fi
> -
> -	${SBINDIR}/xenbackendd ${XENBACKENDD_ARGS}
> -
>  	printf "\n"
>  
>  	printf "Setting domain 0 name.\n"
> @@ -87,8 +78,6 @@ xen_stop()
>  	printf "Stopping xencommons.\n"
>  	printf "WARNING: Not stopping xenstored, as it cannot be restarted.\n"
>  
> -	rc_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
> -	pids="$pids $rc_pid"
>  	rc_pid=$(check_pidfile ${XENCONSOLED_PIDFILE} ${SBINDIR}/xenconsoled)
>  	pids="$pids $rc_pid"
>  
> @@ -108,17 +97,12 @@ xen_status()
>  		pids="$pids $xenconsoled_pid"
>  	fi
>  
> -	xenbackend_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
> -	if test -n ${xenbackend_pid}; then
> -		pids="$pids $xenbackend_pid"
> -	fi
> -
> -	if test -n "$xenbackend_pid" -a -n "$xenconsoled_pid" -a -n "$xenstored_pid";
> +	if test -n "$xenconsoled_pid" -a -n "$xenstored_pid";
>  	then
>  		echo "xencommons are running as pids $pids."
>  		return 0
>  	fi
> -	if test -z "$xenbackend_pid" -a -z "$xenconsoled_pid" -a -z "$xenstored_pid";
> +	if test -z "$xenconsoled_pid" -a -z "$xenstored_pid";
>  	then
>  		echo "xencommons are not running."
>  		return 0
> @@ -134,11 +118,6 @@ xen_status()
>  	else
>  		echo "xenconsoled is not running."
>  	fi
> -	if test -n $xenbackend_pid; then
> -		echo "xenbackendd is running as pid $xenbackend_pid."
> -	else
> -		echo "xenbackendd is not running."
> -	fi
>  }
>  
>  load_rc_config $name
> diff --git a/tools/hotplug/NetBSD/rc.d/xend b/tools/hotplug/NetBSD/rc.d/xend
> index ead9ee0..7fd9bc0 100644
> --- a/tools/hotplug/NetBSD/rc.d/xend
> +++ b/tools/hotplug/NetBSD/rc.d/xend
> @@ -15,10 +15,59 @@ export PATH
>  
>  name="xend"
>  rcvar=$name
> -command="${SBINDIR}/xend"
> -command_args="start"
> -command_interpreter=`head -n 1 ${command} | awk '{ print substr($0,3) }'`
> -sig_stop="SIGKILL"
> +start_precmd="xend_precmd"
> +start_cmd="xend_startcmd"
> +stop_cmd="xend_stop"
> +status_cmd="xend_status"
> +extra_commands="status"
> +required_files="/kern/xen/privcmd"
> +
> +XENBACKENDD_PIDFILE="/var/run/xenbackendd.pid"
> +#XENBACKENDD_DEBUG=1
> +
> +xend_precmd()
> +{
> +	mkdir -p /var/run/xend || exit 1
> +	mkdir -p /var/run/xend/boot || exit 1
> +}
> +
> +xend_startcmd()
> +{
> +	printf "Starting xenbackendd.\n"
> +
> +	XENBACKENDD_ARGS=""
> +	if [ -n "${XENBACKENDD_DEBUG}" ]; then
> +		XENBACKENDD_ARGS="${XENBACKENDD_ARGS} -d"
> +	fi
> +
> +	${SBINDIR}/xenbackendd ${XENBACKENDD_ARGS}
> +
> +	printf "Starting xend.\n"
> +	${SBINDIR}/xend start
> +}
> +
> +xend_stop()
> +{
> +	printf "Stopping xenbackendd, xend\n"
> +	xb_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
> +	xend_pid=`ps x -o pid,command|grep ${SBINDIR}/xend|awk '{ print $1 }'`
> +	if test -n "$xb_pid";
> +	then
> +		kill -${sig_stop:-TERM} $xb_pid
> +	fi
> +	if test -n "$xend_pid";
> +	then
> +		kill -${sig_stop:-KILL} $xend_pid
> +	fi
> +	wait_for_pids $xb_pid $xend_pid
> +	rm -f /var/lock/subsys/xend /var/lock/xend /var/run/xenbackendd.pid
> +}
> +
> +xend_status()
> +{
> +	${SBINDIR}/xend status
> +}
>  
>  load_rc_config $name
>  run_rc_command "$1"
> +



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 2/9] tools/build: fix pygrub linking

[Roger Pau Monne]

Christoph Egger wrote:
> On 07/11/12 12:23, Roger Pau Monne wrote:
> 
>> Prevent creating a symlink to $(DESTDIR)/$(BINDIR) if it is the same
>> as $(PRIVATE_BINDIR)
>>
>> This fixes NetBSD install, where $(DESTDIR)/$(BINDIR) ==
>> $(PRIVATE_BINDIR).
>>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Christoph Egger <Christoph.Egger@amd.com>
>> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> 
> 
> Acked-by: Christoph Egger <Christoph.Egger@amd.com>
> 
>> ---
>>  tools/pygrub/Makefile |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
>> index bd22dd4..835fd43 100644
>> --- a/tools/pygrub/Makefile
>> +++ b/tools/pygrub/Makefile
>> @@ -14,7 +14,9 @@ install: all
>>  		$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" \
>>  		--install-scripts=$(PRIVATE_BINDIR) --force
>>  	$(INSTALL_DIR) $(DESTDIR)/var/run/xend/boot
>> +#ifneq ($(readlink -f $(PRIVATE_BINDIR)), $(readlink -f $(DESTDIR)/$(BINDIR)))
>>  	ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR)
>> +#endif

I'm afraid I've not removed the "#" here (don't know why I've put them
on the first place), and also $(shell ...) should be used to get the
output. This is the correct path:

8<---------------------------------------------------------------------

Subject: [PATCH] tools/build: fix pygrub linking

Prevent creating a symlink to $(DESTDIR)/$(BINDIR) if it is the same
as $(PRIVATE_BINDIR)

This fixes NetBSD install, where $(DESTDIR)/$(BINDIR) ==
$(PRIVATE_BINDIR).

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Christoph Egger <Christoph.Egger@amd.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/pygrub/Makefile |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
index bd22dd4..182afdd 100644
--- a/tools/pygrub/Makefile
+++ b/tools/pygrub/Makefile
@@ -14,7 +14,9 @@ install: all
 		$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" \
 		--install-scripts=$(PRIVATE_BINDIR) --force
 	$(INSTALL_DIR) $(DESTDIR)/var/run/xend/boot
+ifneq ($(shell readlink -f $(DESTDIR)/$(BINDIR)), $(shell readlink -f
$(PRIVATE_BINDIR)))
 	ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR)
+endif

 .PHONY: clean
 clean:
-- 
1.7.7.5 (Apple Git-26)

Re: [PATCH 3/9] pygrub: don't leave fds open

[Ian Jackson]

Roger Pau Monne writes ("[PATCH 3/9] pygrub: don't leave fds open"):
> On NetBSD a block device can only be opened once, so make sure pygrub
> closes it every time, if this is not done libfsimage is not able to
> open the disk later.
> 
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Christoph Egger <Christoph.Egger@amd.com>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>

Looks plausible to me.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Although:

>      fd = os.open(file, os.O_RDONLY)
>      os.lseek(fd, offset + (DK_LABEL_LOC * SECTOR_SIZE), 0)
>      buf = os.read(fd, 512)
> +    os.close(fd)
...
>      fd = os.open(file, os.O_RDONLY)
>      buf = os.read(fd, 512)
> +    os.close(fd)

These are two remarkably similar bits of code which might deserve a
function.

Ian.

Re: [PATCH 5/9] libxl: react correctly to POLLHUP

[Ian Jackson]

Roger Pau Monne writes ("[PATCH 5/9] libxl: react correctly to POLLHUP"):
> On datacopier_readable ignore PULLHUP, since we will shortly process
> the death of the underlying process.

As discussed,

Nacked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.

Re: [PATCH 6/9] libxl: check backend state before setting it to "closing"

[Ian Jackson]

Roger Pau Monne writes ("[PATCH 6/9] libxl: check backend state before setting it to "closing""):
> Check backend state before unconditionally setting it to "closing"
> (5), since it might already be in "closed" (6).

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

> +        if (atoi(state) != XenbusStateClosed) {
> +            rc = libxl__xs_write_checked(gc, t, online_path, "0");
> +            if (rc) {
> +                LOG(ERROR, "unable to write to xenstore path %s", online_path);
> +                goto out;
> +            }
> +            rc = libxl__xs_write_checked(gc, t, state_path, "5");

This business where we use XenbusStateClosed some of the time and "5",
"6", etc. the rest of the time, is incredibly confusing.  But now
probably isn't the time to open that can of worms.

Ian.

Re: [PATCH 7/9] hotplug/NetBSD: check type of file to attach from params

[Ian Jackson]

Roger Pau Monne writes ("[PATCH 7/9] hotplug/NetBSD: check type of file to attach from params"):
> xend used to set the xenbus backend entry "type" to either "phy" or
> "file", but now libxl sets it to "phy" for both file and block device.
> We have to manually check for the type of the "param" filed in order
> to detect if we are trying to attach a file or a block device.

This sounds and looks plausible but what if the type is something
else ?

Ian.

Re: [PATCH 8/9] libxl: call hotplug scripts from xl for NetBSD

[Ian Jackson]

Roger Pau Monne writes ("[PATCH 8/9] libxl: call hotplug scripts from xl for NetBSD"):
> Add the missing NetBSD functions to call hotplug scripts, and disable
> xenbackendd if libxl/disable_udev is not set.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

I haven't checked this for correctness (knowing little about NetBSD)
but it looks about right and doesn't touch non-BSD code.

Ian.

Re: [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

[Ian Jackson]

Roger Pau Monne writes ("[PATCH 9/9] init/NetBSD: move xenbackendd to xend init script"):
> xenbackendd is not needed by the xl toolstack, so move it's launch to
> the xend script.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

on the basis that it doesn't touch non-BSD code.  But:

> +	printf "Stopping xenbackendd, xend\n"
> +	xb_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
> +	xend_pid=`ps x -o pid,command|grep ${SBINDIR}/xend|awk '{ print $1 }'`
> +	if test -n "$xb_pid";
> +	then
> +		kill -${sig_stop:-TERM} $xb_pid

This is pretty horrid.  Does BSD not have better ways to manage
daemons ?

Ian.

Re: [PATCH 2/9] tools/build: fix pygrub linking

[Ian Jackson]

Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
> diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
> index bd22dd4..182afdd 100644
> --- a/tools/pygrub/Makefile
> +++ b/tools/pygrub/Makefile
> @@ -14,7 +14,9 @@ install: all
>  		$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" \
>  		--install-scripts=$(PRIVATE_BINDIR) --force
>  	$(INSTALL_DIR) $(DESTDIR)/var/run/xend/boot
> +ifneq ($(shell readlink -f $(DESTDIR)/$(BINDIR)), $(shell readlink -f
> $(PRIVATE_BINDIR)))
>  	ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR)
> +endif

Why are we doing all this work in make with $(shell...) rather than
simply in the command stanza ?

Also I'm a bit confused.  This problem occurs only when DESTDIR="" ?
In which case perhaps the right answer is something like
        set -e; if ! test -e $(DESTDIR)/$(BINDIR)/pygrub; then \
            ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR); \
        fi

Ian.

Re: [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

[Roger Pau Monne]

Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH 9/9] init/NetBSD: move xenbackendd to xend init script"):
>> xenbackendd is not needed by the xl toolstack, so move it's launch to
>> the xend script.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> on the basis that it doesn't touch non-BSD code.  But:
> 
>> +	printf "Stopping xenbackendd, xend\n"
>> +	xb_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
>> +	xend_pid=`ps x -o pid,command|grep ${SBINDIR}/xend|awk '{ print $1 }'`
>> +	if test -n "$xb_pid";
>> +	then
>> +		kill -${sig_stop:-TERM} $xb_pid
> 
> This is pretty horrid.  Does BSD not have better ways to manage
> daemons ?

Yes, but xend seems to spawn two processes that regenerate, so you have
to kill them with one shot, or it will be useless. "xend stop" doesn't
work either, but since xend is deprecated I think it's rather useless to
try to fix this properly now.

Re: [PATCH 2/9] tools/build: fix pygrub linking

[Roger Pau Monne]

Ian Jackson wrote:
> Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
>> diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
>> index bd22dd4..182afdd 100644
>> --- a/tools/pygrub/Makefile
>> +++ b/tools/pygrub/Makefile
>> @@ -14,7 +14,9 @@ install: all
>>  		$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" \
>>  		--install-scripts=$(PRIVATE_BINDIR) --force
>>  	$(INSTALL_DIR) $(DESTDIR)/var/run/xend/boot
>> +ifneq ($(shell readlink -f $(DESTDIR)/$(BINDIR)), $(shell readlink -f
>> $(PRIVATE_BINDIR)))
>>  	ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR)
>> +endif
> 
> Why are we doing all this work in make with $(shell...) rather than
> simply in the command stanza ?
> 
> Also I'm a bit confused.  This problem occurs only when DESTDIR="" ?

No, this occurs when $(BINDIR) == $(PRIVATE_BINDIR), but we cannot use
the "test -e", because we might have previous versions of pygrub
installed which should also be overwritten by the link, or the user will
keep using the old version.

> In which case perhaps the right answer is something like
>         set -e; if ! test -e $(DESTDIR)/$(BINDIR)/pygrub; then \
>             ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR); \
>         fi
> 
> Ian.

Re: [PATCH 7/9] hotplug/NetBSD: check type of file to attach from params

[Roger Pau Monne]

Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH 7/9] hotplug/NetBSD: check type of file to attach from params"):
>> xend used to set the xenbus backend entry "type" to either "phy" or
>> "file", but now libxl sets it to "phy" for both file and block device.
>> We have to manually check for the type of the "param" filed in order
>> to detect if we are trying to attach a file or a block device.
> 
> This sounds and looks plausible but what if the type is something
> else ?

According to the checks we perform in libxl, it can't be anything else,
but it might be good to add an exit 1 if it's neither 'phy' nor 'file'.

Re: [PATCH 2/9] tools/build: fix pygrub linking

[Ian Jackson]

Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
> Ian Jackson wrote:
> > Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
> >> +ifneq ($(shell readlink -f $(DESTDIR)/$(BINDIR)), $(shell readlink -f
> >> $(PRIVATE_BINDIR)))
> >>  	ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR)
> >> +endif
> > 
> > Why are we doing all this work in make with $(shell...) rather than
> > simply in the command stanza ?
> > 
> > Also I'm a bit confused.  This problem occurs only when DESTDIR="" ?
> 
> No, this occurs when $(BINDIR) == $(PRIVATE_BINDIR), but we cannot use
> the "test -e", because we might have previous versions of pygrub
> installed which should also be overwritten by the link, or the user will
> keep using the old version.

If DESTDIR is not "" then $(PRIVATE_BINDIR)/pygrub is not the same
directory as $(DESTDIR)/$(BINDIR).

Ian.

Re: [PATCH 2/9] tools/build: fix pygrub linking

[Roger Pau Monne]

Ian Jackson wrote:
> Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
>> Ian Jackson wrote:
>>> Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
>>>> +ifneq ($(shell readlink -f $(DESTDIR)/$(BINDIR)), $(shell readlink -f
>>>> $(PRIVATE_BINDIR)))
>>>>  	ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR)
>>>> +endif
>>> Why are we doing all this work in make with $(shell...) rather than
>>> simply in the command stanza ?
>>>
>>> Also I'm a bit confused.  This problem occurs only when DESTDIR="" ?
>> No, this occurs when $(BINDIR) == $(PRIVATE_BINDIR), but we cannot use
>> the "test -e", because we might have previous versions of pygrub
>> installed which should also be overwritten by the link, or the user will
>> keep using the old version.
> 
> If DESTDIR is not "" then $(PRIVATE_BINDIR)/pygrub is not the same
> directory as $(DESTDIR)/$(BINDIR).

Ok, both conditions have to be meet, that DESTDIR == "" and
PRIVATE_BINDIR == BINDIR (so $(DESTDIR)/$(BINDIR) == $(PRIVATE_BINDIR)).

Re: [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

[Christoph Egger]

On 07/17/12 18:08, Ian Jackson wrote:

> 
> Roger Pau Monne writes ("[PATCH 9/9] init/NetBSD: move xenbackendd to xend init script"):
>> xenbackendd is not needed by the xl toolstack, so move it's launch to
>> the xend script.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> on the basis that it doesn't touch non-BSD code.  But:
> 
>> +	printf "Stopping xenbackendd, xend\n"
>> +	xb_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
>> +	xend_pid=`ps x -o pid,command|grep ${SBINDIR}/xend|awk '{ print $1 }'`
>> +	if test -n "$xb_pid";
>> +	then
>> +		kill -${sig_stop:-TERM} $xb_pid
>
> This is pretty horrid.  Does BSD not have better ways to manage
> daemons ?


Does a

xend_pid=`pgrep xend`

work ?

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

[Christoph Egger]

On 07/17/12 18:23, Roger Pau Monne wrote:

> Ian Jackson wrote:
>> Roger Pau Monne writes ("[PATCH 9/9] init/NetBSD: move xenbackendd to xend init script"):
>>> xenbackendd is not needed by the xl toolstack, so move it's launch to
>>> the xend script.
>>
>> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>>
>> on the basis that it doesn't touch non-BSD code.  But:
>>
>>> +	printf "Stopping xenbackendd, xend\n"
>>> +	xb_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
>>> +	xend_pid=`ps x -o pid,command|grep ${SBINDIR}/xend|awk '{ print $1 }'`
>>> +	if test -n "$xb_pid";
>>> +	then
>>> +		kill -${sig_stop:-TERM} $xb_pid
>>
>> This is pretty horrid.  Does BSD not have better ways to manage
>> daemons ?
> 
> Yes, but xend seems to spawn two processes that regenerate, so you have
> to kill them with one shot, or it will be useless. "xend stop" doesn't
> work either, but since xend is deprecated I think it's rather useless to
> try to fix this properly now.


pkill xend  should do it. :)

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

[Roger Pau Monne]

Christoph Egger wrote:
> On 07/17/12 18:23, Roger Pau Monne wrote:
> 
>> Ian Jackson wrote:
>>> Roger Pau Monne writes ("[PATCH 9/9] init/NetBSD: move xenbackendd to xend init script"):
>>>> xenbackendd is not needed by the xl toolstack, so move it's launch to
>>>> the xend script.
>>> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>>>
>>> on the basis that it doesn't touch non-BSD code.  But:
>>>
>>>> +	printf "Stopping xenbackendd, xend\n"
>>>> +	xb_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
>>>> +	xend_pid=`ps x -o pid,command|grep ${SBINDIR}/xend|awk '{ print $1 }'`
>>>> +	if test -n "$xb_pid";
>>>> +	then
>>>> +		kill -${sig_stop:-TERM} $xb_pid
>>> This is pretty horrid.  Does BSD not have better ways to manage
>>> daemons ?
>> Yes, but xend seems to spawn two processes that regenerate, so you have
>> to kill them with one shot, or it will be useless. "xend stop" doesn't
>> work either, but since xend is deprecated I think it's rather useless to
>> try to fix this properly now.
> 
> 
> pkill xend  should do it. :)

Neither pgrep or pkill work, because the process command is registered
as "/usr/pkg/bin/python2.7 /usr/xen42/sbin/xend start". I've tried:

pgrep /usr/xen42/sbin/xend
pgrep xend
pkill /usr/xen42/sbin/xend
pkill xend

And none of them work :(

Re: [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

[Christoph Egger]

On 07/18/12 11:43, Roger Pau Monne wrote:

> Christoph Egger wrote:
>> On 07/17/12 18:23, Roger Pau Monne wrote:
>>
>>> Ian Jackson wrote:
>>>> Roger Pau Monne writes ("[PATCH 9/9] init/NetBSD: move xenbackendd to xend init script"):
>>>>> xenbackendd is not needed by the xl toolstack, so move it's launch to
>>>>> the xend script.
>>>> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>>>>
>>>> on the basis that it doesn't touch non-BSD code.  But:
>>>>
>>>>> +	printf "Stopping xenbackendd, xend\n"
>>>>> +	xb_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
>>>>> +	xend_pid=`ps x -o pid,command|grep ${SBINDIR}/xend|awk '{ print $1 }'`
>>>>> +	if test -n "$xb_pid";
>>>>> +	then
>>>>> +		kill -${sig_stop:-TERM} $xb_pid
>>>> This is pretty horrid.  Does BSD not have better ways to manage
>>>> daemons ?
>>> Yes, but xend seems to spawn two processes that regenerate, so you have
>>> to kill them with one shot, or it will be useless. "xend stop" doesn't
>>> work either, but since xend is deprecated I think it's rather useless to
>>> try to fix this properly now.
>>
>>
>> pkill xend  should do it. :)
> 
> Neither pgrep or pkill work, because the process command is registered
> as "/usr/pkg/bin/python2.7 /usr/xen42/sbin/xend start".


Oh, I see.

> I've tried:
> 
> pgrep /usr/xen42/sbin/xend
> pgrep xend
> pkill /usr/xen42/sbin/xend
> pkill xend
> 
> And none of them work :(
> 
> 



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

[Roger Pau Monne]

Roger Pau Monne wrote:
> Christoph Egger wrote:
>> On 07/17/12 18:23, Roger Pau Monne wrote:
>>
>>> Ian Jackson wrote:
>>>> Roger Pau Monne writes ("[PATCH 9/9] init/NetBSD: move xenbackendd to xend init script"):
>>>>> xenbackendd is not needed by the xl toolstack, so move it's launch to
>>>>> the xend script.
>>>> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>>>>
>>>> on the basis that it doesn't touch non-BSD code.  But:
>>>>
>>>>> +	printf "Stopping xenbackendd, xend\n"
>>>>> +	xb_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
>>>>> +	xend_pid=`ps x -o pid,command|grep ${SBINDIR}/xend|awk '{ print $1 }'`
>>>>> +	if test -n "$xb_pid";
>>>>> +	then
>>>>> +		kill -${sig_stop:-TERM} $xb_pid
>>>> This is pretty horrid.  Does BSD not have better ways to manage
>>>> daemons ?
>>> Yes, but xend seems to spawn two processes that regenerate, so you have
>>> to kill them with one shot, or it will be useless. "xend stop" doesn't
>>> work either, but since xend is deprecated I think it's rather useless to
>>> try to fix this properly now.
>>
>> pkill xend  should do it. :)
> 
> Neither pgrep or pkill work, because the process command is registered
> as "/usr/pkg/bin/python2.7 /usr/xen42/sbin/xend start". I've tried:
> 
> pgrep /usr/xen42/sbin/xend
> pgrep xend
> pkill /usr/xen42/sbin/xend
> pkill xend
> 
> And none of them work :(

Now I got it, `pgrep -f` does the trick, thanks Christoph!

Re: [PATCH 2/9] tools/build: fix pygrub linking

[Ian Jackson]

Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
> Ian Jackson wrote:
> > If DESTDIR is not "" then $(PRIVATE_BINDIR)/pygrub is not the same
> > directory as $(DESTDIR)/$(BINDIR).
> 
> Ok, both conditions have to be meet, that DESTDIR == "" and
> PRIVATE_BINDIR == BINDIR (so $(DESTDIR)/$(BINDIR) == $(PRIVATE_BINDIR)).

Right.  OK.  I see now why my test -e is wrong.

But I still think this should be done with an if in the rule's
command, rather than a makefile conditional.

Ian.

Re: [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

[Ian Jackson]

Christoph Egger writes ("Re: [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script"):
> On 07/17/12 18:23, Roger Pau Monne wrote:
> > Yes, but xend seems to spawn two processes that regenerate, so you have
> > to kill them with one shot, or it will be useless. "xend stop" doesn't
> > work either, but since xend is deprecated I think it's rather useless to
> > try to fix this properly now.
> 
> pkill xend  should do it. :)

What if some other thing on the system has a process called `xend' ?
Does it limit it to processes owned by root or something ?

Or is it just expected that on *BSD we will occasionally kill the
wrong process ?

Ian.

Re: [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

[Roger Pau Monne]

Ian Jackson wrote:
> Christoph Egger writes ("Re: [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script"):
>> On 07/17/12 18:23, Roger Pau Monne wrote:
>>> Yes, but xend seems to spawn two processes that regenerate, so you have
>>> to kill them with one shot, or it will be useless. "xend stop" doesn't
>>> work either, but since xend is deprecated I think it's rather useless to
>>> try to fix this properly now.
>> pkill xend  should do it. :)
> 
> What if some other thing on the system has a process called `xend' ?
> Does it limit it to processes owned by root or something ?
> 
> Or is it just expected that on *BSD we will occasionally kill the
> wrong process ?

I'm using the full path (pkill -f /usr/xen42/sbin/xend), so unless
another process has an argument that's /usr/xen42/sbin/xend (which I've
never seen) it will only kill xend.

Re: [PATCH 2/9] tools/build: fix pygrub linking

[Roger Pau Monne]

Ian Jackson wrote:
> Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
>> Ian Jackson wrote:
>>> If DESTDIR is not "" then $(PRIVATE_BINDIR)/pygrub is not the same
>>> directory as $(DESTDIR)/$(BINDIR).
>> Ok, both conditions have to be meet, that DESTDIR == "" and
>> PRIVATE_BINDIR == BINDIR (so $(DESTDIR)/$(BINDIR) == $(PRIVATE_BINDIR)).
> 
> Right.  OK.  I see now why my test -e is wrong.
> 
> But I still think this should be done with an if in the rule's
> command, rather than a makefile conditional.

What do you think about:

set -e; if [ `readlink -f $(DESTDIR)/$(BINDIR)` != \
             `readlink -f $(PRIVATE_BINDIR)` ]; then \
                ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR); \
fi

Re: [PATCH 2/9] tools/build: fix pygrub linking

[Ian Jackson]

Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
> Ian Jackson wrote:
> > Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
> >> Ian Jackson wrote:
> >>> If DESTDIR is not "" then $(PRIVATE_BINDIR)/pygrub is not the same
> >>> directory as $(DESTDIR)/$(BINDIR).
> >> Ok, both conditions have to be meet, that DESTDIR == "" and
> >> PRIVATE_BINDIR == BINDIR (so $(DESTDIR)/$(BINDIR) == $(PRIVATE_BINDIR)).
> > 
> > Right.  OK.  I see now why my test -e is wrong.
> > 
> > But I still think this should be done with an if in the rule's
> > command, rather than a makefile conditional.
> 
> What do you think about:
> 
> set -e; if [ `readlink -f $(DESTDIR)/$(BINDIR)` != \
>              `readlink -f $(PRIVATE_BINDIR)` ]; then \
>                 ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR); \
> fi

That looks plausible.

Can we assume that we are using bash as our shell for these commands ?
If so we can use `test A -ef B'.

Ian.

Re: [PATCH 2/9] tools/build: fix pygrub linking

[Roger Pau Monne]

Ian Jackson wrote:
> Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
>> Ian Jackson wrote:
>>> Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
>>>> Ian Jackson wrote:
>>>>> If DESTDIR is not "" then $(PRIVATE_BINDIR)/pygrub is not the same
>>>>> directory as $(DESTDIR)/$(BINDIR).
>>>> Ok, both conditions have to be meet, that DESTDIR == "" and
>>>> PRIVATE_BINDIR == BINDIR (so $(DESTDIR)/$(BINDIR) == $(PRIVATE_BINDIR)).
>>> Right.  OK.  I see now why my test -e is wrong.
>>>
>>> But I still think this should be done with an if in the rule's
>>> command, rather than a makefile conditional.
>> What do you think about:
>>
>> set -e; if [ `readlink -f $(DESTDIR)/$(BINDIR)` != \
>>              `readlink -f $(PRIVATE_BINDIR)` ]; then \
>>                 ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR); \
>> fi
> 
> That looks plausible.
> 
> Can we assume that we are using bash as our shell for these commands ?
> If so we can use `test A -ef B'.

I wouldn't make such an assumption, I've build Xen on systems without
bash (Alpine Linux and NetBSD).

Re: [PATCH 2/9] tools/build: fix pygrub linking

[Ian Jackson]

Roger Pau Monne writes ("Re: [PATCH 2/9] tools/build: fix pygrub linking"):
> Ian Jackson wrote:
> > Can we assume that we are using bash as our shell for these commands ?
> > If so we can use `test A -ef B'.
> 
> I wouldn't make such an assumption, I've build Xen on systems without
> bash (Alpine Linux and NetBSD).

OK then, the rune you posted looks reasonable to me assuming someone
has tested it :-).  I'll ack the full patch when I see it.

Ian.

Re: [PATCH 4/9] build: include Tools.mk first in tools/Rules.mk

[Ian Campbell]

On Wed, 2012-07-11 at 11:23 +0100, Roger Pau Monne wrote:
> Tools.mk should be included first, or PREFIX is not honoured in the
> other conf/ files that define the paths of several tools.

Isn't it a bug for anything which is included/defined via
$(XEN_ROOT)/Config.mk to depend on the contents of
$(XEN_ROOT)/config/Tools.mk since for anything non-tools Tools.mk would
never be included at all?

> 
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> ---
>  tools/Rules.mk |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 1e928b7..2dd8ed3 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -3,8 +3,8 @@
>  # `all' is the default target
>  all:
>  
> -include $(XEN_ROOT)/Config.mk
>  -include $(XEN_ROOT)/config/Tools.mk
> +include $(XEN_ROOT)/Config.mk
>  
>  export _INSTALL := $(INSTALL)
>  INSTALL = $(XEN_ROOT)/tools/cross-install

Re: [PATCH 1/9] xenstore: don't print an error when gntdev cannot be opened

[Ian Campbell]

On Wed, 2012-07-11 at 11:23 +0100, Roger Pau Monne wrote:
> NetBSD doesn't have a gntdev, but we should not print an error when
> falling back to the previous implementation.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>

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

> ---
>  tools/libxc/xc_private.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
> index 3ceed3e..3e03a91 100644
> --- a/tools/libxc/xc_private.c
> +++ b/tools/libxc/xc_private.c
> @@ -174,7 +174,7 @@ static struct xc_interface_core *xc_interface_open_common(xentoollog_logger *log
>          xch->ops = xch->osdep.init(xch, type);
>          if ( xch->ops == NULL )
>          {
> -            ERROR("OSDEP: interface %d (%s) not supported on this platform",
> +            DPRINTF("OSDEP: interface %d (%s) not supported on this platform",
>                    type, xc_osdep_type_name(type));
>              goto err_put_iface;
>          }

Re: [PATCH 4/9] build: include Tools.mk first in tools/Rules.mk

[Christoph Egger]

On 07/23/12 13:34, Ian Campbell wrote:

> On Wed, 2012-07-11 at 11:23 +0100, Roger Pau Monne wrote:
>> Tools.mk should be included first, or PREFIX is not honoured in the
>> other conf/ files that define the paths of several tools.
> 
> Isn't it a bug for anything which is included/defined via
> $(XEN_ROOT)/Config.mk to depend on the contents of
> $(XEN_ROOT)/config/Tools.mk since for anything non-tools Tools.mk would
> never be included at all?

When you specify --prefix to configure then this has no effect
w/o this patch.

Christoph


>>
>> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
>> ---
>>  tools/Rules.mk |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/tools/Rules.mk b/tools/Rules.mk
>> index 1e928b7..2dd8ed3 100644
>> --- a/tools/Rules.mk
>> +++ b/tools/Rules.mk
>> @@ -3,8 +3,8 @@
>>  # `all' is the default target
>>  all:
>>  
>> -include $(XEN_ROOT)/Config.mk
>>  -include $(XEN_ROOT)/config/Tools.mk
>> +include $(XEN_ROOT)/Config.mk
>>  
>>  export _INSTALL := $(INSTALL)
>>  INSTALL = $(XEN_ROOT)/tools/cross-install
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 0/9] NetBSD: minor fixes and hotplug execution

[Ian Campbell]

On Wed, 2012-07-11 at 11:23 +0100, Roger Pau Monne wrote:
> This series contains minor fixes to build xen on NetBSD and adds 
> hotplug support to NetBSD. After this, xen-unstable should build 
> and work out-of-the-box on NetBSD.
> 
> This fixes are not related, and as far as I know, can be applied 
> sepparetedly:

I've applied:

> [PATCH 1/9] xenstore: don't print an error when gntdev cannot be
> [PATCH 3/9] pygrub: don't leave fds open

The rest I skipped because they weren't yet acked. Apart from:

> [PATCH 6/9] libxl: check backend state before setting it to

Which I already applied from your V10 hotplug script series.

And:
> [PATCH 8/9] libxl: call hotplug scripts from xl for NetBSD
> [PATCH 9/9] init/NetBSD: move xenbackendd to xend init script

which were Acked but didn't apply cleanly. I suspect conflicts with the
parts of your hotplug series that I've just applied.

Ian.

Re: [PATCH 4/9] build: include Tools.mk first in tools/Rules.mk

[Roger Pau Monne]

Ian Campbell wrote:
> On Wed, 2012-07-11 at 11:23 +0100, Roger Pau Monne wrote:
>> Tools.mk should be included first, or PREFIX is not honoured in the
>> other conf/ files that define the paths of several tools.
> 
> Isn't it a bug for anything which is included/defined via
> $(XEN_ROOT)/Config.mk to depend on the contents of
> $(XEN_ROOT)/config/Tools.mk since for anything non-tools Tools.mk would
> never be included at all?

$(XEN_ROOT)/Config.mk includes config/{Linux.mk/NetBSD.mk} and
config/StdGNU.mk, which contain a bunch of paths, specially
config/StdGNU.mk. If the tools config file is not loaded before, all
this paths get set to the default value (which is probably fine in
Linux), but not desirable in NetBSD if the user has specified a custom
prefix.

So the main problem is that the tools Makefile should include
config/Tools.mk before config/StdGNU.mk, but since config/StdGNU.mk is
included in $(XEN_ROOT)/Config.mk we have to either include Tools.mk
before $(XEN_ROOT)/Config.mk or modify $(XEN_ROOT)/Config.mk to include
Tools.mk (which is not desirable at all). I don't care that
$(XEN_ROOT)/Config.mk uses the default paths when building the xen
kernel, but we should honour the user set paths when installing the tools.

One effect of this patch is that etc and var is installed inside of
$(PREFIX)/etc, which is normal in BSD systems, but I'm quite sure it is
not correct on Linux, so I should add the following chunk:

--- a/config/Linux.mk
+++ b/config/Linux.mk
@@ -6,3 +6,8 @@ KERNELS ?=
 XKERNELS := $(foreach kernel, $(KERNELS), \
               $(patsubst buildconfigs/mk.%,%, \
                 $(wildcard buildconfigs/mk.$(kernel))) )
+
+CONFIG_DIR = /etc
+XEN_LOCK_DIR = /var/lock
+XEN_RUN_DIR = /var/run/xen
+XEN_PAGING_DIR = /var/lib/xen/xenpaging

>> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
>> ---
>>  tools/Rules.mk |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/tools/Rules.mk b/tools/Rules.mk
>> index 1e928b7..2dd8ed3 100644
>> --- a/tools/Rules.mk
>> +++ b/tools/Rules.mk
>> @@ -3,8 +3,8 @@
>>  # `all' is the default target
>>  all:
>>  
>> -include $(XEN_ROOT)/Config.mk
>>  -include $(XEN_ROOT)/config/Tools.mk
>> +include $(XEN_ROOT)/Config.mk
>>  
>>  export _INSTALL := $(INSTALL)
>>  INSTALL = $(XEN_ROOT)/tools/cross-install
> 
> 

Re: [PATCH 4/9] build: include Tools.mk first in tools/Rules.mk

[Ian Campbell]

On Wed, 2012-07-25 at 10:32 +0100, Roger Pau Monne wrote:
> Ian Campbell wrote:
> > On Wed, 2012-07-11 at 11:23 +0100, Roger Pau Monne wrote:
> >> Tools.mk should be included first, or PREFIX is not honoured in the
> >> other conf/ files that define the paths of several tools.
> > 
> > Isn't it a bug for anything which is included/defined via
> > $(XEN_ROOT)/Config.mk to depend on the contents of
> > $(XEN_ROOT)/config/Tools.mk since for anything non-tools Tools.mk would
> > never be included at all?
> 
> $(XEN_ROOT)/Config.mk includes config/{Linux.mk/NetBSD.mk} and
> config/StdGNU.mk, which contain a bunch of paths, specially
> config/StdGNU.mk. If the tools config file is not loaded before, all
> this paths get set to the default value (which is probably fine in
> Linux), but not desirable in NetBSD if the user has specified a custom
> prefix.
> 
> So the main problem is that the tools Makefile should include
> config/Tools.mk before config/StdGNU.mk, but since config/StdGNU.mk is
> included in $(XEN_ROOT)/Config.mk we have to either include Tools.mk
> before $(XEN_ROOT)/Config.mk or modify $(XEN_ROOT)/Config.mk to include
> Tools.mk (which is not desirable at all). I don't care that
> $(XEN_ROOT)/Config.mk uses the default paths when building the xen
> kernel, but we should honour the user set paths when installing the tools.

My point was that really all of these paths should move to autoconf and
Tools.mk.in. But for 4.2 I guess your proposed change is more
appropriate.

> One effect of this patch is that etc and var is installed inside of
> $(PREFIX)/etc, which is normal in BSD systems, but I'm quite sure it is
> not correct on Linux, so I should add the following chunk:

StdGNU explicitly has:
        ifeq ($(PREFIX),/usr)
        CONFIG_DIR = /etc
        XEN_LOCK_DIR = /var/lock
        XEN_RUN_DIR = /var/run/xen
        XEN_PAGING_DIR = /var/lib/xen/xenpaging
        else
        CONFIG_DIR = $(PREFIX)/etc
        XEN_LOCK_DIR = $(PREFIX)/var/lock
        XEN_RUN_DIR = $(PREFIX)/var/run/xen
        XEN_PAGING_DIR = $(PREFIX)/var/lib/xen/xenpaging
        endif
so it looks like this was at least deliberate.

> 
> --- a/config/Linux.mk
> +++ b/config/Linux.mk
> @@ -6,3 +6,8 @@ KERNELS ?=
>  XKERNELS := $(foreach kernel, $(KERNELS), \
>                $(patsubst buildconfigs/mk.%,%, \
>                  $(wildcard buildconfigs/mk.$(kernel))) )
> +
> +CONFIG_DIR = /etc
> +XEN_LOCK_DIR = /var/lock
> +XEN_RUN_DIR = /var/run/xen
> +XEN_PAGING_DIR = /var/lib/xen/xenpaging
> 
> >> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> >> ---
> >>  tools/Rules.mk |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/tools/Rules.mk b/tools/Rules.mk
> >> index 1e928b7..2dd8ed3 100644
> >> --- a/tools/Rules.mk
> >> +++ b/tools/Rules.mk
> >> @@ -3,8 +3,8 @@
> >>  # `all' is the default target
> >>  all:
> >>  
> >> -include $(XEN_ROOT)/Config.mk
> >>  -include $(XEN_ROOT)/config/Tools.mk
> >> +include $(XEN_ROOT)/Config.mk
> >>  
> >>  export _INSTALL := $(INSTALL)
> >>  INSTALL = $(XEN_ROOT)/tools/cross-install
> > 
> > 
> 

Re: [PATCH 4/9] build: include Tools.mk first in tools/Rules.mk

[Roger Pau Monne]

Ian Campbell wrote:
> On Wed, 2012-07-25 at 10:32 +0100, Roger Pau Monne wrote:
>> Ian Campbell wrote:
>>> On Wed, 2012-07-11 at 11:23 +0100, Roger Pau Monne wrote:
>>>> Tools.mk should be included first, or PREFIX is not honoured in the
>>>> other conf/ files that define the paths of several tools.
>>> Isn't it a bug for anything which is included/defined via
>>> $(XEN_ROOT)/Config.mk to depend on the contents of
>>> $(XEN_ROOT)/config/Tools.mk since for anything non-tools Tools.mk would
>>> never be included at all?
>> $(XEN_ROOT)/Config.mk includes config/{Linux.mk/NetBSD.mk} and
>> config/StdGNU.mk, which contain a bunch of paths, specially
>> config/StdGNU.mk. If the tools config file is not loaded before, all
>> this paths get set to the default value (which is probably fine in
>> Linux), but not desirable in NetBSD if the user has specified a custom
>> prefix.
>>
>> So the main problem is that the tools Makefile should include
>> config/Tools.mk before config/StdGNU.mk, but since config/StdGNU.mk is
>> included in $(XEN_ROOT)/Config.mk we have to either include Tools.mk
>> before $(XEN_ROOT)/Config.mk or modify $(XEN_ROOT)/Config.mk to include
>> Tools.mk (which is not desirable at all). I don't care that
>> $(XEN_ROOT)/Config.mk uses the default paths when building the xen
>> kernel, but we should honour the user set paths when installing the tools.
> 
> My point was that really all of these paths should move to autoconf and
> Tools.mk.in. But for 4.2 I guess your proposed change is more
> appropriate.

Yes, this is just a bandaid to hide a much bigger problem we have with
paths & makefiles in general.

>> One effect of this patch is that etc and var is installed inside of
>> $(PREFIX)/etc, which is normal in BSD systems, but I'm quite sure it is
>> not correct on Linux, so I should add the following chunk:
> 
> StdGNU explicitly has:
>         ifeq ($(PREFIX),/usr)
>         CONFIG_DIR = /etc
>         XEN_LOCK_DIR = /var/lock
>         XEN_RUN_DIR = /var/run/xen
>         XEN_PAGING_DIR = /var/lib/xen/xenpaging
>         else
>         CONFIG_DIR = $(PREFIX)/etc
>         XEN_LOCK_DIR = $(PREFIX)/var/lock
>         XEN_RUN_DIR = $(PREFIX)/var/run/xen
>         XEN_PAGING_DIR = $(PREFIX)/var/lib/xen/xenpaging
>         endif
> so it looks like this was at least deliberate.

Yes, this was the behaviour we had before the autoconf patch, when
PREFIX was specified in .config (which I find strange in a Linux
system). So I'm not going to add this chunk and leave the patch as-is.

Thanks.

> 
>> --- a/config/Linux.mk
>> +++ b/config/Linux.mk
>> @@ -6,3 +6,8 @@ KERNELS ?=
>>  XKERNELS := $(foreach kernel, $(KERNELS), \
>>                $(patsubst buildconfigs/mk.%,%, \
>>                  $(wildcard buildconfigs/mk.$(kernel))) )
>> +
>> +CONFIG_DIR = /etc
>> +XEN_LOCK_DIR = /var/lock
>> +XEN_RUN_DIR = /var/run/xen
>> +XEN_PAGING_DIR = /var/lib/xen/xenpaging
>>
>>>> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
>>>> ---
>>>>  tools/Rules.mk |    2 +-
>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/tools/Rules.mk b/tools/Rules.mk
>>>> index 1e928b7..2dd8ed3 100644
>>>> --- a/tools/Rules.mk
>>>> +++ b/tools/Rules.mk
>>>> @@ -3,8 +3,8 @@
>>>>  # `all' is the default target
>>>>  all:
>>>>  
>>>> -include $(XEN_ROOT)/Config.mk
>>>>  -include $(XEN_ROOT)/config/Tools.mk
>>>> +include $(XEN_ROOT)/Config.mk
>>>>  
>>>>  export _INSTALL := $(INSTALL)
>>>>  INSTALL = $(XEN_ROOT)/tools/cross-install
>>>
> 
> 

Re: [PATCH 4/9] build: include Tools.mk first in tools/Rules.mk

[Ian Campbell]

On Wed, 2012-07-25 at 11:56 +0100, Roger Pau Monne wrote:
> Ian Campbell wrote:
> > On Wed, 2012-07-25 at 10:32 +0100, Roger Pau Monne wrote:
> >> Ian Campbell wrote:
> >>> On Wed, 2012-07-11 at 11:23 +0100, Roger Pau Monne wrote:
> >>>> Tools.mk should be included first, or PREFIX is not honoured in the
> >>>> other conf/ files that define the paths of several tools.
> >>> Isn't it a bug for anything which is included/defined via
> >>> $(XEN_ROOT)/Config.mk to depend on the contents of
> >>> $(XEN_ROOT)/config/Tools.mk since for anything non-tools Tools.mk would
> >>> never be included at all?
> >> $(XEN_ROOT)/Config.mk includes config/{Linux.mk/NetBSD.mk} and
> >> config/StdGNU.mk, which contain a bunch of paths, specially
> >> config/StdGNU.mk. If the tools config file is not loaded before, all
> >> this paths get set to the default value (which is probably fine in
> >> Linux), but not desirable in NetBSD if the user has specified a custom
> >> prefix.
> >>
> >> So the main problem is that the tools Makefile should include
> >> config/Tools.mk before config/StdGNU.mk, but since config/StdGNU.mk is
> >> included in $(XEN_ROOT)/Config.mk we have to either include Tools.mk
> >> before $(XEN_ROOT)/Config.mk or modify $(XEN_ROOT)/Config.mk to include
> >> Tools.mk (which is not desirable at all). I don't care that
> >> $(XEN_ROOT)/Config.mk uses the default paths when building the xen
> >> kernel, but we should honour the user set paths when installing the tools.
> > 
> > My point was that really all of these paths should move to autoconf and
> > Tools.mk.in. But for 4.2 I guess your proposed change is more
> > appropriate.
> 
> Yes, this is just a bandaid to hide a much bigger problem we have with
> paths & makefiles in general.

OK then.

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

and applied, thanks.