thr3ads.net - Xen devel - [PATCH 0/9] NetBSD: minor fixes and hotplug execution [Jul 2012]

If this information is useful, please help other people find it:
Share via:

Roger Pau Monne

2012-Jul-11 10:23 UTC

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

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.

Roger Pau Monne

2012-Jul-11 10:23 UTC

head link

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

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)

Roger Pau Monne

2012-Jul-11 10:23 UTC

head link

[PATCH 2/9] 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..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)

Roger Pau Monne

2012-Jul-11 10:23 UTC

head link

[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>
---
 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)

Roger Pau Monne

2012-Jul-11 10:23 UTC

head link

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

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)

Roger Pau Monne

2012-Jul-11 10:23 UTC

head link

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

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)

Roger Pau Monne

2012-Jul-11 10:23 UTC

head link

[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).

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)

Roger Pau Monne

2012-Jul-11 10:23 UTC

head link

[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.

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)

Roger Pau Monne

2012-Jul-11 10:23 UTC

head link

[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.

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)

Roger Pau Monne

2012-Jul-11 10:23 UTC

head link

[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.

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)

Christoph Egger

2012-Jul-11 11:00 UTC

head link

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

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

Christoph Egger

2012-Jul-11 11:02 UTC

head link

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

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

Christoph Egger

2012-Jul-11 11:52 UTC

head link

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

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

Christoph Egger

2012-Jul-11 11:54 UTC

head link

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

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

Christoph Egger

2012-Jul-11 12:02 UTC

head link

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

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

Roger Pau Monne

2012-Jul-12 16:30 UTC

head link

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

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)

Ian Jackson

2012-Jul-17 16:01 UTC

head link

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

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.

Ian Jackson

2012-Jul-17 16:01 UTC

head link

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

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.

Ian Jackson

2012-Jul-17 16:05 UTC

head link

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

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.

Ian Jackson

2012-Jul-17 16:06 UTC

head link

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

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.

Ian Jackson

2012-Jul-17 16:07 UTC

head link

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

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.

Ian Jackson

2012-Jul-17 16:08 UTC

head link

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

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.

Ian Jackson

2012-Jul-17 16:22 UTC

head link

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

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.

Roger Pau Monne

2012-Jul-17 16:23 UTC

head link

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

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.

Roger Pau Monne

2012-Jul-17 16:34 UTC

head link

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"):
>> 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.

Roger Pau Monne

2012-Jul-17 16:35 UTC

head link

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

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''.

Ian Jackson

2012-Jul-17 18:17 UTC

head link

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

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.

Roger Pau Monne

2012-Jul-18 08:26 UTC

head link

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:
>>> 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)).

Christoph Egger

2012-Jul-18 09:02 UTC

head link

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

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

Christoph Egger

2012-Jul-18 09:04 UTC

head link

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

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

Roger Pau Monne

2012-Jul-18 09:43 UTC

head link

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

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 :(

Christoph Egger

2012-Jul-18 09:46 UTC

head link

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

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

Roger Pau Monne

2012-Jul-18 09:46 UTC

head link

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

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!

Ian Jackson

2012-Jul-18 10:48 UTC

head link

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

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.

Ian Jackson

2012-Jul-18 10:50 UTC

head link

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

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.

Roger Pau Monne

2012-Jul-18 11:02 UTC

head link

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

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.

Roger Pau Monne

2012-Jul-18 13:55 UTC

head link

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

Ian Jackson

2012-Jul-19 14:48 UTC

head link

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

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.

Roger Pau Monne

2012-Jul-19 15:00 UTC

head link

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:
>>> 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).

Ian Jackson

2012-Jul-19 15:03 UTC

head link

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

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.

Ian Campbell

2012-Jul-23 11:34 UTC

head link

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

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

Ian Campbell

2012-Jul-23 11:42 UTC

head link

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

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;
>          }

Christoph Egger

2012-Jul-23 12:19 UTC

head link

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

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

Ian Campbell

2012-Jul-23 12:20 UTC

head link

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

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.

Roger Pau Monne

2012-Jul-25 09:32 UTC

head link

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

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
> 
>

Ian Campbell

2012-Jul-25 10:39 UTC

head link

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

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
> > 
> > 
>

Roger Pau Monne

2012-Jul-25 10:56 UTC

head link

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

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
>>>
> 
>

Ian Campbell

2012-Jul-25 16:43 UTC

head link

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

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.

Xen devel - Jul 2012 - [PATCH 0/9] NetBSD: minor fixes and hotplug execution

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

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

[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 "closing"

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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