[PATCH 00/24] NetBSD fixes

[PATCH 00/24] NetBSD fixes

[Manuel Bouyer]

Hello,
here is a set of 24 patches, which are needed to build and run the
tools on NetBSD. They are extracted from NetBSD's pkgsrc repository for
Xen 4.13, and ported to 4.15. 

Manuel Bouyer (24):
  Fix lock directory path for NetBSD
  NetBSD doens't need xenbackendd with xl toolstack
  Fix lock directory path for NetBSD
  Make it build on NetBSD
  Introduce locking functions for block device setup on NetBSD
  Handle the case where vifname is not present in xenstore.
  Remove NetBSD's system headers. We'll use the system-provided ones,
    which are up to date.
  Make it build on NetBSD
  Use xen/xenio.h on NetBSD
  Make it build on NetBSD
  Implement foreignmemory on NetBSD
  Implement gnttab on NetBSD
  Don't assume tv_sec is a unsigned long (for NetBSD)
  Pass bridge name to qemu When starting qemu, set an environnement
    variable XEN_DOMAIN_ID, to be used by qemu helper scripts
  Make it build on NetBSD
  Switch NetBSD to QEMU_XEN (!traditional)
  Make it build on NetBSD
  This doens't need xen/sys/evtchn.h (NetBSD fix)
  errno may not be a gobal R/W variable, use a local variable instead
    (fix build on NetBSD)
  If FILENAME_MAX is defined, use it instead of arbitrary value (fix
    format-truncation errors with GCC >= 7)
  Fix unused functions/variables error
  If PTHREAD_STACK_MIN is not defined, use DEFAULT_THREAD_STACKSIZE
  Use xen/xenio.h on NetBSD
  Fix error: array subscript has type 'char' [-Werror=char-subscripts]

 m4/paths.m4                                   |   2 +-
 tools/Makefile                                |   1 -
 tools/configure                               |   2 +-
 tools/debugger/gdbsx/xg/xg_main.c             |  11 +
 tools/hotplug/NetBSD/Makefile                 |   1 +
 tools/hotplug/NetBSD/block                    |   5 +-
 tools/hotplug/NetBSD/locking.sh               |  72 +++++
 tools/hotplug/NetBSD/vif-bridge               |   5 +-
 tools/hotplug/NetBSD/vif-ip                   |   4 +
 tools/include/Makefile                        |   2 +-
 tools/include/xen-sys/NetBSD/evtchn.h         |  86 ------
 tools/include/xen-sys/NetBSD/privcmd.h        | 106 -------
 tools/libs/call/netbsd.c                      |  18 +-
 tools/libs/call/private.h                     |   8 +-
 tools/libs/ctrl/xc_private.h                  |   4 +
 tools/libs/evtchn/netbsd.c                    |   8 +-
 tools/libs/foreignmemory/Makefile             |   2 +-
 tools/libs/foreignmemory/netbsd.c             |  76 ++++-
 tools/libs/foreignmemory/private.h            |  10 +-
 tools/libs/gnttab/Makefile                    |   2 +-
 tools/libs/gnttab/netbsd.c                    | 267 ++++++++++++++++++
 tools/libs/light/libxl_create.c               |   8 +-
 tools/libs/light/libxl_dm.c                   |  19 ++
 tools/libs/light/libxl_netbsd.c               |   2 +-
 tools/libs/light/libxl_qmp.c                  |   2 +-
 tools/libs/light/libxl_uuid.c                 |   4 +-
 tools/libs/stat/xenstat_netbsd.c              |  11 -
 tools/libs/store/xs.c                         |   4 +
 tools/ocaml/libs/eventchn/xeneventchn_stubs.c |   1 -
 tools/xenpaging/xenpaging.c                   |   5 +-
 tools/xenpmd/xenpmd.c                         |   4 +
 tools/xentrace/xentrace.c                     |   2 +-
 xen/tools/symbols.c                           |   4 +-
 33 files changed, 508 insertions(+), 250 deletions(-)
 create mode 100644 tools/hotplug/NetBSD/locking.sh
 delete mode 100644 tools/include/xen-sys/NetBSD/evtchn.h
 delete mode 100644 tools/include/xen-sys/NetBSD/privcmd.h
 create mode 100644 tools/libs/gnttab/netbsd.c

-- 
2.28.0

[PATCH 01/24] Fix lock directory path for NetBSD

[Manuel Bouyer]

---
 m4/paths.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/m4/paths.m4 b/m4/paths.m4
index 89d3bb8312..1c107b1a61 100644
--- a/m4/paths.m4
+++ b/m4/paths.m4
@@ -142,7 +142,7 @@ AC_SUBST(XEN_SCRIPT_DIR)
 
 case "$host_os" in
 *freebsd*) XEN_LOCK_DIR=$localstatedir/lib ;;
-*netbsd*) XEN_LOCK_DIR=$localstatedir/lib ;;
+*netbsd*) XEN_LOCK_DIR=$rundir_path ;;
 *) XEN_LOCK_DIR=$localstatedir/lock ;;
 esac
 AC_SUBST(XEN_LOCK_DIR)
-- 
2.28.0

[PATCH 02/24] NetBSD doens't need xenbackendd with xl toolstack

[Manuel Bouyer]

---
 tools/Makefile | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/Makefile b/tools/Makefile
index ed71474421..757a560be0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -18,7 +18,6 @@ SUBDIRS-$(CONFIG_X86) += firmware
 SUBDIRS-y += console
 SUBDIRS-y += xenmon
 SUBDIRS-y += xentop
-SUBDIRS-$(CONFIG_NetBSD) += xenbackendd
 SUBDIRS-y += libfsimage
 SUBDIRS-$(CONFIG_Linux) += vchan
 
-- 
2.28.0

[PATCH 03/24] Fix lock directory path for NetBSD

[Manuel Bouyer]

---
 tools/configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/configure b/tools/configure
index 8a708e9baa..131112c41e 100755
--- a/tools/configure
+++ b/tools/configure
@@ -4030,7 +4030,7 @@ XEN_SCRIPT_DIR=$XEN_CONFIG_DIR/scripts
 
 case "$host_os" in
 *freebsd*) XEN_LOCK_DIR=$localstatedir/lib ;;
-*netbsd*) XEN_LOCK_DIR=$localstatedir/lib ;;
+*netbsd*) XEN_LOCK_DIR=$localstatedir/run ;;
 *) XEN_LOCK_DIR=$localstatedir/lock ;;
 esac
 
-- 
2.28.0

[PATCH 04/24] Make xg_main.c build on NetBSD

[Manuel Bouyer]

---
 tools/debugger/gdbsx/xg/xg_main.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/debugger/gdbsx/xg/xg_main.c b/tools/debugger/gdbsx/xg/xg_main.c
index a4e8653168..fa2741ccf8 100644
--- a/tools/debugger/gdbsx/xg/xg_main.c
+++ b/tools/debugger/gdbsx/xg/xg_main.c
@@ -49,7 +49,11 @@
 #include "xg_public.h"
 #include <xen/version.h>
 #include <xen/domctl.h>
+#ifdef __NetBSD__
+#include <xen/xenio.h>
+#else
 #include <xen/sys/privcmd.h>
+#endif
 #include <xen/foreign/x86_32.h>
 #include <xen/foreign/x86_64.h>
 
@@ -126,12 +130,19 @@ xg_init()
     int flags, saved_errno;
 
     XGTRC("E\n");
+#ifdef __NetBSD__
+    if ((_dom0_fd=open("/kern/xen/privcmd", O_RDWR)) == -1) {
+        perror("Failed to open /kern/xen/privcmd\n");
+        return -1;
+    }
+#else
     if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1) {
         if ((_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1) {
             perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd\n");
             return -1;
         }
     }
+#endif
     /* Although we return the file handle as the 'xc handle' the API
      * does not specify / guarentee that this integer is in fact
      * a file handle. Thus we must take responsiblity to ensure
-- 
2.28.0

[PATCH 05/24] Introduce locking functions for block device setup on NetBSD

[Manuel Bouyer]

---
 tools/hotplug/NetBSD/Makefile   |  1 +
 tools/hotplug/NetBSD/block      |  5 ++-
 tools/hotplug/NetBSD/locking.sh | 72 +++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 1 deletion(-)
 create mode 100644 tools/hotplug/NetBSD/locking.sh

diff --git a/tools/hotplug/NetBSD/Makefile b/tools/hotplug/NetBSD/Makefile
index 6926885ab8..114b223207 100644
--- a/tools/hotplug/NetBSD/Makefile
+++ b/tools/hotplug/NetBSD/Makefile
@@ -3,6 +3,7 @@ include $(XEN_ROOT)/tools/Rules.mk
 
 # Xen script dir and scripts to go there.
 XEN_SCRIPTS =
+XEN_SCRIPTS += locking.sh
 XEN_SCRIPTS += block
 XEN_SCRIPTS += vif-bridge
 XEN_SCRIPTS += vif-ip
diff --git a/tools/hotplug/NetBSD/block b/tools/hotplug/NetBSD/block
index 32c20b6c89..23c8e38ebf 100644
--- a/tools/hotplug/NetBSD/block
+++ b/tools/hotplug/NetBSD/block
@@ -6,6 +6,7 @@
 
 DIR=$(dirname "$0")
 . "${DIR}/hotplugpath.sh"
+. "${DIR}/locking.sh"
 
 PATH=${bindir}:${sbindir}:${LIBEXEC_BIN}:/bin:/usr/bin:/sbin:/usr/sbin
 export PATH
@@ -62,6 +63,7 @@ case $xstatus in
 			available_disks="$available_disks $disk"
 			eval $disk=free
 		done
+		claim_lock block
 		# Mark the used vnd(4) devices as ``used''.
 		for disk in `sysctl hw.disknames`; do
 			case $disk in
@@ -77,6 +79,7 @@ case $xstatus in
 				break	
 			fi
 		done
+		release_lock block
 		if [ x$device = x ] ; then
 			error "no available vnd device"
 		fi
@@ -86,7 +89,7 @@ case $xstatus in
 		device=$xparams
 		;;
 	esac
-	physical_device=$(stat -f '%r' "$device")
+	physical_device=$(stat -L -f '%r' "$device")
 	xenstore-write $xpath/physical-device $physical_device
 	xenstore-write $xpath/hotplug-status connected
 	exit 0
diff --git a/tools/hotplug/NetBSD/locking.sh b/tools/hotplug/NetBSD/locking.sh
new file mode 100644
index 0000000000..88257f62b7
--- /dev/null
+++ b/tools/hotplug/NetBSD/locking.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+#
+# Copyright (c) 2016, Christoph Badura.  All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1. Redistributions of source code must retain the above copyright
+#    notice, this list of conditions and the following disclaimer.
+# 2. Redistributions in binary form must reproduce the above copyright
+#    notice, this list of conditions and the following disclaimer in the
+#    documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE AUTHOR(S) ``AS IS'' AND ANY EXPRESS
+# OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR(S) BE LIABLE FOR ANY DIRECT,
+# INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+#
+
+LOCK_BASEDIR="$XEN_LOCK_DIR/xen-hotplug"
+
+_lockfd=9
+_have_lock=0	# lock not taken yet.
+
+SHLOCK="shlock ${_shlock_debug-}"
+
+_lock_set_vars() {
+	_lockfile="$LOCK_BASEDIR/$1.lock"
+	_lockfifo="$LOCK_BASEDIR/$1.fifo"
+}
+
+_lock_init() {
+	mkdir -p "$LOCK_BASEDIR" 2>/dev/null || true
+	mkfifo $_lockfifo 2>/dev/null || true
+}
+
+#
+# use a named pipe as condition variable
+# opening for read-only blocks when there's no writer.
+# opening for read-write never blocks but unblocks any waiting readers.
+# 
+_lock_wait_cv() {
+	eval "exec $_lockfd<  $_lockfifo ; exec $_lockfd<&-"
+}
+_lock_signal_cv() {
+	eval "exec $_lockfd<> $_lockfifo ; exec $_lockfd<&-"
+}
+
+claim_lock() {
+	_lock_set_vars $1
+	_lock_init
+	until $SHLOCK -f $_lockfile -p $$; do
+		_lock_wait_cv
+	done
+	_have_lock=1
+	# be sure to release the lock when the shell exits
+	trap "release_lock $1" 0 1 2 15
+}
+
+release_lock() {
+	_lock_set_vars $1
+	[ "$_have_lock" != 0 -a -f $_lockfile ] && rm $_lockfile
+	_have_lock=0
+	_lock_signal_cv;
+}
-- 
2.28.0

[PATCH 06/24] Handle the case where vifname is not present in xenstore.

[Manuel Bouyer]

---
 tools/hotplug/NetBSD/vif-bridge | 5 ++++-
 tools/hotplug/NetBSD/vif-ip     | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/hotplug/NetBSD/vif-bridge b/tools/hotplug/NetBSD/vif-bridge
index b58e922601..cd428b5936 100644
--- a/tools/hotplug/NetBSD/vif-bridge
+++ b/tools/hotplug/NetBSD/vif-bridge
@@ -23,7 +23,10 @@ case $xstatus in
 	xbridge=$(xenstore-read "$xpath/bridge")
 	xfid=$(xenstore-read "$xpath/frontend-id")
 	xhandle=$(xenstore-read "$xpath/handle")
-	iface=$(xenstore-read "$xpath/vifname")
+	iface=$(xenstore-read "$xpath/vifname") || true
+	if [ x${iface} = "x" ] ; then
+		iface=xvif$xfid.$xhandle
+	fi
 	ifconfig $iface up
 	brconfig $xbridge add $iface
 	xenstore-write $xpath/hotplug-status connected
diff --git a/tools/hotplug/NetBSD/vif-ip b/tools/hotplug/NetBSD/vif-ip
index 83cbfe20e2..944f50f881 100644
--- a/tools/hotplug/NetBSD/vif-ip
+++ b/tools/hotplug/NetBSD/vif-ip
@@ -24,6 +24,10 @@ case $xstatus in
 	xfid=$(xenstore-read "$xpath/frontend-id")
 	xhandle=$(xenstore-read "$xpath/handle")
 	iface=$(xenstore-read "$xpath/vifname")
+	iface=$(xenstore-read "$xpath/vifname") || true
+	if [ x${iface} = "x" ] ; then
+		iface=xvif$xfid.$xhandle
+	fi
 	ifconfig $iface $xip up
 	xenstore-write $xpath/hotplug-status connected
 	exit 0
-- 
2.28.0

[PATCH 07/24] Remove NetBSD's system headers. We'll use the system-provided ones, which are up to date.

[Manuel Bouyer]

---
 tools/include/Makefile                 |   2 +-
 tools/include/xen-sys/NetBSD/evtchn.h  |  86 --------------------
 tools/include/xen-sys/NetBSD/privcmd.h | 106 -------------------------
 3 files changed, 1 insertion(+), 193 deletions(-)
 delete mode 100644 tools/include/xen-sys/NetBSD/evtchn.h
 delete mode 100644 tools/include/xen-sys/NetBSD/privcmd.h

diff --git a/tools/include/Makefile b/tools/include/Makefile
index 4d4ec5f974..5e90179e66 100644
--- a/tools/include/Makefile
+++ b/tools/include/Makefile
@@ -68,7 +68,7 @@ install: all
 	$(INSTALL_DATA) xen/foreign/*.h $(DESTDIR)$(includedir)/xen/foreign
 	$(INSTALL_DATA) xen/hvm/*.h $(DESTDIR)$(includedir)/xen/hvm
 	$(INSTALL_DATA) xen/io/*.h $(DESTDIR)$(includedir)/xen/io
-	$(INSTALL_DATA) xen/sys/*.h $(DESTDIR)$(includedir)/xen/sys
+	$(INSTALL_DATA) xen/sys/*.h $(DESTDIR)$(includedir)/xen/sys || true
 	$(INSTALL_DATA) xen/xsm/*.h $(DESTDIR)$(includedir)/xen/xsm
 
 .PHONY: uninstall
diff --git a/tools/include/xen-sys/NetBSD/evtchn.h b/tools/include/xen-sys/NetBSD/evtchn.h
deleted file mode 100644
index 2d8a1f9164..0000000000
--- a/tools/include/xen-sys/NetBSD/evtchn.h
+++ /dev/null
@@ -1,86 +0,0 @@
-/* $NetBSD: evtchn.h,v 1.1.1.1 2007/06/14 19:39:45 bouyer Exp $ */
-/******************************************************************************
- * evtchn.h
- * 
- * Interface to /dev/xen/evtchn.
- * 
- * Copyright (c) 2003-2005, K A Fraser
- * 
- * This file may be distributed separately from the Linux kernel, or
- * incorporated into other software packages, subject to the following license:
- * 
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this source file (the "Software"), to deal in the Software without
- * restriction, including without limitation the rights to use, copy, modify,
- * merge, publish, distribute, sublicense, and/or sell copies of the Software,
- * and to permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- * 
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- * 
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- */
-
-#ifndef __NetBSD_EVTCHN_H__
-#define __NetBSD_EVTCHN_H__
-
-/*
- * Bind a fresh port to VIRQ @virq.
- */
-#define IOCTL_EVTCHN_BIND_VIRQ				\
-	_IOWR('E', 4, struct ioctl_evtchn_bind_virq)
-struct ioctl_evtchn_bind_virq {
-	unsigned int virq;
-	unsigned int port;
-};
-
-/*
- * Bind a fresh port to remote <@remote_domain, @remote_port>.
- */
-#define IOCTL_EVTCHN_BIND_INTERDOMAIN			\
-	_IOWR('E', 5, struct ioctl_evtchn_bind_interdomain)
-struct ioctl_evtchn_bind_interdomain {
-	unsigned int remote_domain, remote_port;
-	unsigned int port;
-};
-
-/*
- * Allocate a fresh port for binding to @remote_domain.
- */
-#define IOCTL_EVTCHN_BIND_UNBOUND_PORT			\
-	_IOWR('E', 6, struct ioctl_evtchn_bind_unbound_port)
-struct ioctl_evtchn_bind_unbound_port {
-	unsigned int remote_domain;
-	unsigned int port;
-};
-
-/*
- * Unbind previously allocated @port.
- */
-#define IOCTL_EVTCHN_UNBIND				\
-	_IOW('E', 7, struct ioctl_evtchn_unbind)
-struct ioctl_evtchn_unbind {
-	unsigned int port;
-};
-
-/*
- * Send event to previously allocated @port.
- */
-#define IOCTL_EVTCHN_NOTIFY				\
-	_IOW('E', 8, struct ioctl_evtchn_notify)
-struct ioctl_evtchn_notify {
-	unsigned int port;
-};
-
-/* Clear and reinitialise the event buffer. Clear error condition. */
-#define IOCTL_EVTCHN_RESET				\
-	_IO('E', 9)
-
-#endif /* __NetBSD_EVTCHN_H__ */
diff --git a/tools/include/xen-sys/NetBSD/privcmd.h b/tools/include/xen-sys/NetBSD/privcmd.h
deleted file mode 100644
index 555bad973e..0000000000
--- a/tools/include/xen-sys/NetBSD/privcmd.h
+++ /dev/null
@@ -1,106 +0,0 @@
-/*	NetBSD: xenio.h,v 1.3 2005/05/24 12:07:12 yamt Exp $	*/
-
-/******************************************************************************
- * privcmd.h
- * 
- * Copyright (c) 2003-2004, K A Fraser
- * 
- * This file may be distributed separately from the Linux kernel, or
- * incorporated into other software packages, subject to the following license:
- * 
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this source file (the "Software"), to deal in the Software without
- * restriction, including without limitation the rights to use, copy, modify,
- * merge, publish, distribute, sublicense, and/or sell copies of the Software,
- * and to permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- * 
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- * 
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- */
-
-#ifndef __NetBSD_PRIVCMD_H__
-#define __NetBSD_PRIVCMD_H__
-
-/* Interface to /dev/xen/privcmd */
-
-typedef struct privcmd_hypercall
-{
-    unsigned long op;
-    unsigned long arg[5];
-    long retval;
-} privcmd_hypercall_t;
-
-typedef struct privcmd_mmap_entry {
-    unsigned long va;
-    unsigned long mfn;
-    unsigned long npages;
-} privcmd_mmap_entry_t; 
-
-typedef struct privcmd_mmap {
-    int num;
-    domid_t dom; /* target domain */
-    privcmd_mmap_entry_t *entry;
-} privcmd_mmap_t; 
-
-typedef struct privcmd_mmapbatch {
-    int num;     /* number of pages to populate */
-    domid_t dom; /* target domain */
-    unsigned long addr;  /* virtual address */
-    unsigned long *arr; /* array of mfns - top nibble set on err */
-} privcmd_mmapbatch_t; 
-
-typedef struct privcmd_blkmsg
-{
-    unsigned long op;
-    void         *buf;
-    int           buf_size;
-} privcmd_blkmsg_t;
-
-/*
- * @cmd: IOCTL_PRIVCMD_HYPERCALL
- * @arg: &privcmd_hypercall_t
- * Return: Value returned from execution of the specified hypercall.
- */
-#define IOCTL_PRIVCMD_HYPERCALL         \
-    _IOWR('P', 0, privcmd_hypercall_t)
-
-#if defined(_KERNEL)
-/* compat */
-#define IOCTL_PRIVCMD_INITDOMAIN_EVTCHN_OLD \
-    _IO('P', 1)
-#endif /* defined(_KERNEL) */
-    
-#define IOCTL_PRIVCMD_MMAP             \
-    _IOW('P', 2, privcmd_mmap_t)
-#define IOCTL_PRIVCMD_MMAPBATCH        \
-    _IOW('P', 3, privcmd_mmapbatch_t)
-#define IOCTL_PRIVCMD_GET_MACH2PHYS_START_MFN \
-    _IOR('P', 4, unsigned long)
-
-/*
- * @cmd: IOCTL_PRIVCMD_INITDOMAIN_EVTCHN
- * @arg: n/a
- * Return: Port associated with domain-controller end of control event channel
- *         for the initial domain.
- */
-#define IOCTL_PRIVCMD_INITDOMAIN_EVTCHN \
-    _IOR('P', 5, int)
-
-/* Interface to /dev/xenevt */
-/* EVTCHN_RESET: Clear and reinit the event buffer. Clear error condition. */
-#define EVTCHN_RESET  _IO('E', 1)
-/* EVTCHN_BIND: Bind to the specified event-channel port. */
-#define EVTCHN_BIND   _IOW('E', 2, unsigned long)
-/* EVTCHN_UNBIND: Unbind from the specified event-channel port. */
-#define EVTCHN_UNBIND _IOW('E', 3, unsigned long)
-
-#endif /* __NetBSD_PRIVCMD_H__ */
-- 
2.28.0

[PATCH 08/24] Make libs/call build on NetBSD

[Manuel Bouyer]

---
 tools/libs/call/netbsd.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/tools/libs/call/netbsd.c b/tools/libs/call/netbsd.c
index a5502da377..1a771e9928 100644
--- a/tools/libs/call/netbsd.c
+++ b/tools/libs/call/netbsd.c
@@ -19,12 +19,14 @@
  * Split from xc_netbsd.c
  */
 
-#include "xc_private.h"
 
 #include <unistd.h>
 #include <fcntl.h>
 #include <malloc.h>
+#include <errno.h>
 #include <sys/mman.h>
+#include <sys/ioctl.h>
+#include "private.h"
 
 int osdep_xencall_open(xencall_handle *xcall)
 {
@@ -69,12 +71,13 @@ int osdep_xencall_close(xencall_handle *xcall)
     return close(fd);
 }
 
-void *osdep_alloc_hypercall_buffer(xencall_handle *xcall, size_t npages)
+void *osdep_alloc_pages(xencall_handle *xcall, size_t npages)
 {
-    size_t size = npages * XC_PAGE_SIZE;
+    size_t size = npages * PAGE_SIZE;
     void *p;
+    int ret;
 
-    ret = posix_memalign(&p, XC_PAGE_SIZE, size);
+    ret = posix_memalign(&p, PAGE_SIZE, size);
     if ( ret != 0 || !p )
         return NULL;
 
@@ -86,14 +89,13 @@ void *osdep_alloc_hypercall_buffer(xencall_handle *xcall, size_t npages)
     return p;
 }
 
-void osdep_free_hypercall_buffer(xencall_handle *xcall, void *ptr,
-                                 size_t npages)
+void osdep_free_pages(xencall_handle *xcall, void *ptr, size_t npages)
 {
-    (void) munlock(ptr, npages * XC_PAGE_SIZE);
+    (void) munlock(ptr, npages * PAGE_SIZE);
     free(ptr);
 }
 
-int do_xen_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall)
+int osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall)
 {
     int fd = xcall->fd;
     int error = ioctl(fd, IOCTL_PRIVCMD_HYPERCALL, hypercall);
-- 
2.28.0

[PATCH 09/24] Use xen/xenio.h on NetBSD

[Manuel Bouyer]

---
 tools/libs/call/private.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/libs/call/private.h b/tools/libs/call/private.h
index 21f992b37e..96922e03d5 100644
--- a/tools/libs/call/private.h
+++ b/tools/libs/call/private.h
@@ -7,13 +7,19 @@
 #include <xencall.h>
 
 #include <xen/xen.h>
+#ifdef __NetBSD__
+#include <xen/xenio.h>
+#else
 #include <xen/sys/privcmd.h>
+#endif
 
 #ifndef PAGE_SHIFT /* Mini-os, Yukk */
 #define PAGE_SHIFT           12
 #endif
-#ifndef __MINIOS__ /* Yukk */
+#ifndef PAGE_SIZE
 #define PAGE_SIZE            (1UL << PAGE_SHIFT)
+#endif
+#ifndef PAGE_MASK
 #define PAGE_MASK            (~(PAGE_SIZE-1))
 #endif
 
-- 
2.28.0

[PATCH 10/24] Make libs/evtchn build on NetBSD

[Manuel Bouyer]

---
 tools/libs/evtchn/netbsd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
index 8b8545d2f9..6d4ce28011 100644
--- a/tools/libs/evtchn/netbsd.c
+++ b/tools/libs/evtchn/netbsd.c
@@ -25,10 +25,10 @@
 
 #include <sys/ioctl.h>
 
-#include <xen/sys/evtchn.h>
-
 #include "private.h"
 
+#include <xen/xenio3.h>
+
 #define EVTCHN_DEV_NAME  "/dev/xenevt"
 
 int osdep_evtchn_open(xenevtchn_handle *xce)
@@ -131,7 +131,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
     int fd = xce->fd;
     evtchn_port_t port;
 
-    if ( read_exact(fd, (char *)&port, sizeof(port)) == -1 )
+    if ( read(fd, (char *)&port, sizeof(port)) == -1 )
         return -1;
 
     return port;
@@ -140,7 +140,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
 int xenevtchn_unmask(xenevtchn_handle *xce, evtchn_port_t port)
 {
     int fd = xce->fd;
-    return write_exact(fd, (char *)&port, sizeof(port));
+    return write(fd, (char *)&port, sizeof(port));
 }
 
 /*
-- 
2.28.0

[PATCH 11/24] Implement foreignmemory on NetBSD

[Manuel Bouyer]

---
 tools/libs/foreignmemory/Makefile  |  2 +-
 tools/libs/foreignmemory/netbsd.c  | 76 ++++++++++++++++++++++++++----
 tools/libs/foreignmemory/private.h | 10 +++-
 3 files changed, 75 insertions(+), 13 deletions(-)

diff --git a/tools/libs/foreignmemory/Makefile b/tools/libs/foreignmemory/Makefile
index 13850f7988..f191cdbed0 100644
--- a/tools/libs/foreignmemory/Makefile
+++ b/tools/libs/foreignmemory/Makefile
@@ -8,7 +8,7 @@ SRCS-y                 += core.c
 SRCS-$(CONFIG_Linux)   += linux.c
 SRCS-$(CONFIG_FreeBSD) += freebsd.c
 SRCS-$(CONFIG_SunOS)   += compat.c solaris.c
-SRCS-$(CONFIG_NetBSD)  += compat.c netbsd.c
+SRCS-$(CONFIG_NetBSD)  += netbsd.c
 SRCS-$(CONFIG_MiniOS)  += minios.c
 
 include $(XEN_ROOT)/tools/libs/libs.mk
diff --git a/tools/libs/foreignmemory/netbsd.c b/tools/libs/foreignmemory/netbsd.c
index 54a418ebd6..6d740ec2a3 100644
--- a/tools/libs/foreignmemory/netbsd.c
+++ b/tools/libs/foreignmemory/netbsd.c
@@ -19,7 +19,9 @@
 
 #include <unistd.h>
 #include <fcntl.h>
+#include <errno.h>
 #include <sys/mman.h>
+#include <sys/ioctl.h>
 
 #include "private.h"
 
@@ -66,15 +68,17 @@ int osdep_xenforeignmemory_close(xenforeignmemory_handle *fmem)
     return close(fd);
 }
 
-void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
-                              void *addr, int prot, int flags,
-                              xen_pfn_t *arr, int num)
+void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
+                                 uint32_t dom, void *addr,
+				 int prot, int flags, size_t num,
+				 const xen_pfn_t arr[/*num*/], int err[/*num*/])
+
 {
     int fd = fmem->fd;
-    privcmd_mmapbatch_t ioctlx;
-    addr = mmap(addr, num*XC_PAGE_SIZE, prot, flags | MAP_ANON | MAP_SHARED, -1, 0);
+    privcmd_mmapbatch_v2_t ioctlx;
+    addr = mmap(addr, num*PAGE_SIZE, prot, flags | MAP_ANON | MAP_SHARED, -1, 0);
     if ( addr == MAP_FAILED ) {
-        PERROR("osdep_map_foreign_batch: mmap failed");
+        PERROR("osdep_xenforeignmemory_map: mmap failed");
         return NULL;
     }
 
@@ -82,11 +86,12 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
     ioctlx.dom=dom;
     ioctlx.addr=(unsigned long)addr;
     ioctlx.arr=arr;
-    if ( ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH, &ioctlx) < 0 )
+    ioctlx.err=err;
+    if ( ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH_V2, &ioctlx) < 0 )
     {
         int saved_errno = errno;
-        PERROR("osdep_map_foreign_batch: ioctl failed");
-        (void)munmap(addr, num*XC_PAGE_SIZE);
+        PERROR("osdep_xenforeignmemory_map: ioctl failed");
+        (void)munmap(addr, num*PAGE_SIZE);
         errno = saved_errno;
         return NULL;
     }
@@ -97,7 +102,58 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
 int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
                                  void *addr, size_t num)
 {
-    return munmap(addr, num*XC_PAGE_SIZE);
+    return munmap(addr, num*PAGE_SIZE);
+}
+
+int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
+                                    domid_t domid)
+{
+    return 0;
+}
+
+int osdep_xenforeignmemory_unmap_resource(
+    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
+{
+    return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) : 0;
+}
+
+int osdep_xenforeignmemory_map_resource(
+    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
+{
+    privcmd_mmap_resource_t mr = {
+        .dom = fres->domid,
+        .type = fres->type,
+        .id = fres->id,
+        .idx = fres->frame,
+        .num = fres->nr_frames,
+    };
+    int rc;
+
+    fres->addr = mmap(fres->addr, fres->nr_frames << PAGE_SHIFT,
+                      fres->prot, fres->flags | MAP_ANON | MAP_SHARED, -1, 0);
+    if ( fres->addr == MAP_FAILED )
+        return -1;
+
+    mr.addr = (uintptr_t)fres->addr;
+
+    rc = ioctl(fmem->fd, IOCTL_PRIVCMD_MMAP_RESOURCE, &mr);
+    if ( rc )
+    {
+        int saved_errno;
+
+        if ( errno != fmem->unimpl_errno && errno != EOPNOTSUPP )
+            PERROR("ioctl failed");
+        else
+            errno = EOPNOTSUPP;
+
+        saved_errno = errno;
+        (void)osdep_xenforeignmemory_unmap_resource(fmem, fres);
+        errno = saved_errno;
+
+        return -1;
+    }
+
+    return 0;
 }
 
 /*
diff --git a/tools/libs/foreignmemory/private.h b/tools/libs/foreignmemory/private.h
index 8f1bf081ed..abeceb8720 100644
--- a/tools/libs/foreignmemory/private.h
+++ b/tools/libs/foreignmemory/private.h
@@ -8,7 +8,13 @@
 #include <xentoolcore_internal.h>
 
 #include <xen/xen.h>
+
+#ifdef __NetBSD__
+#include <xen/xen.h>
+#include <xen/xenio.h>
+#else
 #include <xen/sys/privcmd.h>
+#endif
 
 #ifndef PAGE_SHIFT /* Mini-os, Yukk */
 #define PAGE_SHIFT           12
@@ -38,7 +44,7 @@ int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
 
 #if defined(__NetBSD__) || defined(__sun__)
 /* Strictly compat for those two only only */
-void *compat_mapforeign_batch(xenforeignmem_handle *fmem, uint32_t dom,
+void *osdep_map_foreign_batch(xenforeignmemory_handle *fmem, uint32_t dom,
                               void *addr, int prot, int flags,
                               xen_pfn_t *arr, int num);
 #endif
@@ -54,7 +60,7 @@ struct xenforeignmemory_resource_handle {
     int flags;
 };
 
-#ifndef __linux__
+#if  !defined(__linux__) && !defined(__NetBSD__)
 static inline int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
                                                   domid_t domid)
 {
-- 
2.28.0

[PATCH 12/24] Implement gnttab on NetBSD

[Manuel Bouyer]

---
 tools/libs/gnttab/Makefile |   2 +-
 tools/libs/gnttab/netbsd.c | 267 +++++++++++++++++++++++++++++++++++++
 2 files changed, 268 insertions(+), 1 deletion(-)
 create mode 100644 tools/libs/gnttab/netbsd.c

diff --git a/tools/libs/gnttab/Makefile b/tools/libs/gnttab/Makefile
index d86c49d243..ae390ce60f 100644
--- a/tools/libs/gnttab/Makefile
+++ b/tools/libs/gnttab/Makefile
@@ -10,7 +10,7 @@ SRCS-GNTSHR            += gntshr_core.c
 SRCS-$(CONFIG_Linux)   += $(SRCS-GNTTAB) $(SRCS-GNTSHR) linux.c
 SRCS-$(CONFIG_MiniOS)  += $(SRCS-GNTTAB) gntshr_unimp.c minios.c
 SRCS-$(CONFIG_FreeBSD) += $(SRCS-GNTTAB) $(SRCS-GNTSHR) freebsd.c
+SRCS-$(CONFIG_NetBSD)  += $(SRCS-GNTTAB) $(SRCS-GNTSHR) netbsd.c
 SRCS-$(CONFIG_SunOS)   += gnttab_unimp.c gntshr_unimp.c
-SRCS-$(CONFIG_NetBSD)  += gnttab_unimp.c gntshr_unimp.c
 
 include $(XEN_ROOT)/tools/libs/libs.mk
diff --git a/tools/libs/gnttab/netbsd.c b/tools/libs/gnttab/netbsd.c
new file mode 100644
index 0000000000..2df7058cd7
--- /dev/null
+++ b/tools/libs/gnttab/netbsd.c
@@ -0,0 +1,267 @@
+/*
+ * Copyright (c) 2007-2008, D G Murray <Derek.Murray@cl.cam.ac.uk>
+ * Copyright (c) 2016-2017, Akshay Jaggi <jaggi@FreeBSD.org>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Split out from linux.c
+ */
+
+#include <fcntl.h>
+#include <errno.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+
+#include <xen/xen.h>
+#include <xen/xenio.h>
+
+#include "private.h"
+
+#define PAGE_SHIFT           12
+#define PAGE_SIZE            (1UL << PAGE_SHIFT)
+#define PAGE_MASK            (~(PAGE_SIZE-1))
+
+#define DEVXEN "/kern/xen/privcmd"
+
+int osdep_gnttab_open(xengnttab_handle *xgt)
+{
+    int fd = open(DEVXEN, O_RDWR|O_CLOEXEC);
+
+    if ( fd == -1 )
+        return -1;
+    xgt->fd = fd;
+
+    return 0;
+}
+
+int osdep_gnttab_close(xengnttab_handle *xgt)
+{
+    if ( xgt->fd == -1 )
+        return 0;
+
+    return close(xgt->fd);
+}
+
+int osdep_gnttab_set_max_grants(xengnttab_handle *xgt, uint32_t count)
+{
+    return 0;
+}
+
+void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
+                             uint32_t count, int flags, int prot,
+                             uint32_t *domids, uint32_t *refs,
+                             uint32_t notify_offset,
+                             evtchn_port_t notify_port)
+{
+    uint32_t i;
+    int fd = xgt->fd;
+    struct ioctl_gntdev_mmap_grant_ref map;
+    void *addr = NULL;
+    int domids_stride;
+    unsigned int refs_size = count * sizeof(struct ioctl_gntdev_grant_ref);
+    int rv;
+
+    domids_stride = (flags & XENGNTTAB_GRANT_MAP_SINGLE_DOMAIN) ? 0 : 1;
+    map.refs = malloc(refs_size);
+
+    for ( i = 0; i < count; i++ )
+    {
+        map.refs[i].domid = domids[i * domids_stride];
+        map.refs[i].ref = refs[i];
+    }
+
+    map.count = count;
+    addr = mmap(NULL, count * PAGE_SIZE,
+	prot, flags | MAP_ANON | MAP_SHARED, -1, 0);
+
+    if (map.va == MAP_FAILED) {
+        GTERROR(xgt->logger, "osdep_gnttab_grant_map: mmap failed");
+	munmap((void *)map.va, count * PAGE_SIZE);
+        addr = MAP_FAILED;
+    }
+    map.va = addr;
+
+    map.notify.offset = 0;
+    map.notify.action = 0;
+    if ( notify_offset < PAGE_SIZE * count )
+    {
+	map.notify.offset = notify_offset;
+	map.notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
+    }
+    if ( notify_port != -1 )
+    {
+       map.notify.event_channel_port = notify_port;
+       map.notify.action |= UNMAP_NOTIFY_SEND_EVENT;
+    }
+
+    rv = ioctl(fd, IOCTL_GNTDEV_MMAP_GRANT_REF, &map);
+    if ( rv )
+    {
+        GTERROR(xgt->logger,
+	    "ioctl IOCTL_GNTDEV_MMAP_GRANT_REF failed: %d", rv);
+        munmap(addr, count * PAGE_SIZE);
+        addr = MAP_FAILED;
+    }
+    free(map.refs);
+    return addr;
+}
+
+int osdep_gnttab_unmap(xengnttab_handle *xgt,
+                       void *start_address,
+                       uint32_t count)
+{
+    int rc;
+    if ( start_address == NULL )
+    {
+        errno = EINVAL;
+        return -1;
+    }
+
+    /* Next, unmap the memory. */
+    rc = munmap(start_address, count * PAGE_SIZE);
+
+    return rc;
+}
+
+int osdep_gnttab_grant_copy(xengnttab_handle *xgt,
+                            uint32_t count,
+                            xengnttab_grant_copy_segment_t *segs)
+{
+    errno = ENOSYS;
+    return -1;
+}
+
+int osdep_gntshr_open(xengntshr_handle *xgs)
+{
+
+    int fd = open(DEVXEN, O_RDWR);
+
+    if ( fd == -1 )
+        return -1;
+    xgs->fd = fd;
+
+    return 0;
+}
+
+int osdep_gntshr_close(xengntshr_handle *xgs)
+{
+    if ( xgs->fd == -1 )
+        return 0;
+
+    return close(xgs->fd);
+}
+
+void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
+                               uint32_t domid, int count,
+                               uint32_t *refs, int writable,
+                               uint32_t notify_offset,
+                               evtchn_port_t notify_port)
+{
+    int err;
+    int fd = xgs->fd;
+    void *area = NULL;
+    struct ioctl_gntdev_alloc_grant_ref alloc;
+
+    alloc.gref_ids = malloc(count * sizeof(uint32_t));
+    if ( alloc.gref_ids == NULL )
+        return NULL;
+    alloc.domid = domid;
+    alloc.flags = writable ? GNTDEV_ALLOC_FLAG_WRITABLE : 0;
+    alloc.count = count;
+    area = mmap(NULL, count * PAGE_SIZE,
+	PROT_READ | PROT_WRITE, MAP_ANON | MAP_SHARED, -1, 0);
+
+    if (area == MAP_FAILED) {
+        GTERROR(xgs->logger, "osdep_gnttab_grant_map: mmap failed");
+        area = MAP_FAILED;
+	goto out;
+    }
+    alloc.va = area;
+
+    alloc.notify.offset = 0;
+    alloc.notify.action = 0;
+    if ( notify_offset < PAGE_SIZE * count )
+    {
+	alloc.notify.offset = notify_offset;
+	alloc.notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
+    }
+    if ( notify_port != -1 )
+    {
+       alloc.notify.event_channel_port = notify_port;
+       alloc.notify.action |= UNMAP_NOTIFY_SEND_EVENT;
+    }
+    err = ioctl(fd, IOCTL_GNTDEV_ALLOC_GRANT_REF, &alloc);
+    if ( err )
+    {
+        GSERROR(xgs->logger, "IOCTL_GNTDEV_ALLOC_GRANT_REF failed");
+	munmap(area, count * PAGE_SIZE);
+	area = MAP_FAILED;
+        goto out;
+    }
+    memcpy(refs, alloc.gref_ids, count * sizeof(uint32_t));
+
+ out:
+    free(alloc.gref_ids);
+    return area;
+}
+
+int osdep_gntshr_unshare(xengntshr_handle *xgs,
+                         void *start_address, uint32_t count)
+{
+    return munmap(start_address, count * PAGE_SIZE);
+}
+
+/*
+ * The functions below are Linux-isms that will likely never be implemented
+ * on FreeBSD unless FreeBSD also implements something akin to Linux dmabuf.
+ */
+int osdep_gnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
+                                      uint32_t flags, uint32_t count,
+                                      const uint32_t *refs,
+                                      uint32_t *dmabuf_fd)
+{
+    abort();
+}
+
+int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
+                                          uint32_t fd, uint32_t wait_to_ms)
+{
+    abort();
+}
+
+int osdep_gnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
+                                    uint32_t fd, uint32_t count, uint32_t *refs)
+{
+    abort();
+}
+
+int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
+{
+    abort();
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.28.0

[PATCH 13/24] Don't assume tv_sec is a unsigned long (for NetBSD)

[Manuel Bouyer]

---
 tools/libs/light/libxl_create.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 321a13e519..44691010bc 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -496,7 +496,7 @@ int libxl__domain_build(libxl__gc *gc,
         vments[2] = "image/ostype";
         vments[3] = "hvm";
         vments[4] = "start_time";
-        vments[5] = GCSPRINTF("%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
+        vments[5] = GCSPRINTF("%jd.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
 
         localents = libxl__calloc(gc, 13, sizeof(char *));
         i = 0;
@@ -535,7 +535,7 @@ int libxl__domain_build(libxl__gc *gc,
         vments[i++] = "image/kernel";
         vments[i++] = (char *) state->pv_kernel.path;
         vments[i++] = "start_time";
-        vments[i++] = GCSPRINTF("%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
+        vments[i++] = GCSPRINTF("%jd.%02d", (intmax_t)start_time.tv_sec,(int)start_time.tv_usec/10000);
         if (state->pv_ramdisk.path) {
             vments[i++] = "image/ramdisk";
             vments[i++] = (char *) state->pv_ramdisk.path;
@@ -1502,7 +1502,7 @@ static void domcreate_stream_done(libxl__egc *egc,
         vments[2] = "image/ostype";
         vments[3] = "hvm";
         vments[4] = "start_time";
-        vments[5] = GCSPRINTF("%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
+        vments[5] = GCSPRINTF("%jd.%02d", (intmax_t)start_time.tv_sec,(int)start_time.tv_usec/10000);
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         vments = libxl__calloc(gc, 11, sizeof(char *));
@@ -1512,7 +1512,7 @@ static void domcreate_stream_done(libxl__egc *egc,
         vments[i++] = "image/kernel";
         vments[i++] = (char *) state->pv_kernel.path;
         vments[i++] = "start_time";
-        vments[i++] = GCSPRINTF("%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
+        vments[i++] = GCSPRINTF("%jd.%02d", (intmax_t)start_time.tv_sec,(int)start_time.tv_usec/10000);
         if (state->pv_ramdisk.path) {
             vments[i++] = "image/ramdisk";
             vments[i++] = (char *) state->pv_ramdisk.path;
-- 
2.28.0

[PATCH 14/24] Pass bridge name to qemu and set XEN_DOMAIN_ID

[Manuel Bouyer]

Pass bridge name to qemu
When starting qemu, set an environnement variable XEN_DOMAIN_ID,
to be used by qemu helper scripts

---
 tools/libs/light/libxl_dm.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 3da83259c0..5948ace60d 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -761,6 +761,10 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
         int nr_set_cpus = 0;
         char *s;
 
+	static char buf[12];
+	snprintf(buf, sizeof(buf), "%d", domid);
+        flexarray_append_pair(dm_envs, "XEN_DOMAIN_ID", buf);
+
         if (b_info->kernel) {
             LOGD(ERROR, domid, "HVM direct kernel boot is not supported by "
                  "qemu-xen-traditional");
@@ -1547,8 +1551,10 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                 flexarray_append(dm_args, "-netdev");
                 flexarray_append(dm_args,
                                  GCSPRINTF("type=tap,id=net%d,ifname=%s,"
+					   "br=%s,"
                                            "script=%s,downscript=%s",
                                            nics[i].devid, ifname,
+					   nics[i].bridge,
                                            libxl_tapif_script(gc),
                                            libxl_tapif_script(gc)));
 
@@ -1825,6 +1831,10 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
     flexarray_append(dm_args, GCSPRINTF("%"PRId64, ram_size));
 
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
+	static char buf[12];
+	snprintf(buf, sizeof(buf), "%d", guest_domid);
+        flexarray_append_pair(dm_envs, "XEN_DOMAIN_ID", buf);
+
         if (b_info->u.hvm.hdtype == LIBXL_HDTYPE_AHCI)
             flexarray_append_pair(dm_args, "-device", "ahci,id=ahci0");
         for (i = 0; i < num_disks; i++) {
-- 
2.28.0

[PATCH 15/24] Make libs/light build on NetBSD

[Manuel Bouyer]

---
 tools/libs/light/libxl_dm.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 5948ace60d..c93bdf2cc9 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -3659,6 +3659,14 @@ static int kill_device_model_uid_child(libxl__destroy_devicemodel_state *ddms,
 
     LOGD(DEBUG, domid, "DM reaper: calling setresuid(%d, %d, 0)",
          reaper_uid, dm_kill_uid);
+#ifdef __NetBSD__
+    r = setuid(dm_kill_uid);
+    if (r) {
+        LOGED(ERROR, domid, "setuid to %d", dm_kill_uid);
+        rc = rc ?: ERROR_FAIL;
+        goto out;
+    }
+#else /* __NetBSD__ */
     r = setresuid(reaper_uid, dm_kill_uid, 0);
     if (r) {
         LOGED(ERROR, domid, "setresuid to (%d, %d, 0)",
@@ -3666,6 +3674,7 @@ static int kill_device_model_uid_child(libxl__destroy_devicemodel_state *ddms,
         rc = rc ?: ERROR_FAIL;
         goto out;
     }
+#endif /* __NetBSD__ */
 
     /*
      * And kill everyone but me.
-- 
2.28.0

[PATCH 16/24] Switch NetBSD to QEMU_XEN (!traditional)

[Manuel Bouyer]

---
 tools/libs/light/libxl_netbsd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libs/light/libxl_netbsd.c b/tools/libs/light/libxl_netbsd.c
index e66a393d7f..31334f932c 100644
--- a/tools/libs/light/libxl_netbsd.c
+++ b/tools/libs/light/libxl_netbsd.c
@@ -110,7 +110,7 @@ out:
 
 libxl_device_model_version libxl__default_device_model(libxl__gc *gc)
 {
-    return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+    return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
 }
 
 int libxl__pci_numdevs(libxl__gc *gc)
-- 
2.28.0

[PATCH 17/24] Make libs/light build on NetBSD

[Manuel Bouyer]

---
 tools/libs/light/libxl_uuid.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libs/light/libxl_uuid.c b/tools/libs/light/libxl_uuid.c
index dadb79bad8..a8ee5f253e 100644
--- a/tools/libs/light/libxl_uuid.c
+++ b/tools/libs/light/libxl_uuid.c
@@ -82,7 +82,7 @@ void libxl_uuid_generate(libxl_uuid *uuid)
     uuid_enc_be(uuid->uuid, &nat_uuid);
 }
 
-#ifdef __FreeBSD__
+#if defined(__FreeBSD__) || defined(__NetBSD__)
 int libxl_uuid_from_string(libxl_uuid *uuid, const char *in)
 {
     uint32_t status;
@@ -120,7 +120,7 @@ void libxl_uuid_clear(libxl_uuid *uuid)
     memset(&uuid->uuid, 0, sizeof(uuid->uuid));
 }
 
-#ifdef __FreeBSD__
+#if defined(__FreeBSD__) || defined(__NetBSD__)
 int libxl_uuid_compare(const libxl_uuid *uuid1, const libxl_uuid *uuid2)
 {
     uuid_t nat_uuid1, nat_uuid2;
-- 
2.28.0

[PATCH 18/24] xeneventchn_stubs.c doens't need xen/sys/evtchn.h (NetBSD fix)

[Manuel Bouyer]

---
 tools/ocaml/libs/eventchn/xeneventchn_stubs.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/ocaml/libs/eventchn/xeneventchn_stubs.c b/tools/ocaml/libs/eventchn/xeneventchn_stubs.c
index ba40078d09..f889a7a2e4 100644
--- a/tools/ocaml/libs/eventchn/xeneventchn_stubs.c
+++ b/tools/ocaml/libs/eventchn/xeneventchn_stubs.c
@@ -22,7 +22,6 @@
 #include <stdint.h>
 #include <sys/ioctl.h>
 #include <xen/xen.h>
-#include <xen/sys/evtchn.h>
 #include <xenevtchn.h>
 
 #define CAML_NAME_SPACE
-- 
2.28.0

[PATCH 19/24] errno may not be a gobal R/W variable, use a local variable instead (fix build on NetBSD)

[Manuel Bouyer]

---
 tools/xenpaging/xenpaging.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
index 33098046c2..39c8c83b4b 100644
--- a/tools/xenpaging/xenpaging.c
+++ b/tools/xenpaging/xenpaging.c
@@ -180,10 +180,11 @@ static int xenpaging_get_tot_pages(struct xenpaging *paging)
 static void *init_page(void)
 {
     void *buffer;
+    int rc;
 
     /* Allocated page memory */
-    errno = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
-    if ( errno != 0 )
+    rc = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
+    if ( rc != 0 )
         return NULL;
 
     /* Lock buffer in memory so it can't be paged out */
-- 
2.28.0

[PATCH 20/24] If FILENAME_MAX is defined, use it instead of arbitrary value (fix format-truncation errors with GCC >= 7)

[Manuel Bouyer]

---
 tools/xenpmd/xenpmd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/xenpmd/xenpmd.c b/tools/xenpmd/xenpmd.c
index 12b82cf43e..cfd22e64e3 100644
--- a/tools/xenpmd/xenpmd.c
+++ b/tools/xenpmd/xenpmd.c
@@ -101,7 +101,11 @@ FILE *get_next_battery_file(DIR *battery_dir,
 {
     FILE *file = 0;
     struct dirent *dir_entries;
+#ifdef FILENAME_MAX
+    char file_name[FILENAME_MAX];
+#else
     char file_name[284];
+#endif
     int ret;
     
     do 
-- 
2.28.0

[PATCH 21/24] Fix unused functions/variables error

[Manuel Bouyer]

---
 tools/libs/stat/xenstat_netbsd.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/tools/libs/stat/xenstat_netbsd.c b/tools/libs/stat/xenstat_netbsd.c
index 6e9d6aee10..64eda9e1ae 100644
--- a/tools/libs/stat/xenstat_netbsd.c
+++ b/tools/libs/stat/xenstat_netbsd.c
@@ -55,11 +55,6 @@ get_priv_data(xenstat_handle *handle)
 }
 
 /* Expected format of /proc/net/dev */
-static const char PROCNETDEV_HEADER[] =
-    "Inter-|   Receive                                                |"
-    "  Transmit\n"
-    " face |bytes    packets errs drop fifo frame compressed multicast|"
-    "bytes    packets errs drop fifo colls carrier compressed\n";
 
 /* Collect information about networks */
 int xenstat_collect_networks(xenstat_node * node)
@@ -76,12 +71,6 @@ void xenstat_uninit_networks(xenstat_handle * handle)
 		fclose(priv->procnetdev);
 }
 
-static int read_attributes_vbd(const char *vbd_directory, const char *what, char *ret, int cap)
-{
-	/* XXX implement */
-	return 0;
-}
-
 /* Collect information about VBDs */
 int xenstat_collect_vbds(xenstat_node * node)
 {
-- 
2.28.0

[PATCH 22/24] If PTHREAD_STACK_MIN is not defined, use DEFAULT_THREAD_STACKSIZE

[Manuel Bouyer]

---
 tools/libs/store/xs.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
index 4ac73ec317..8e646b98d6 100644
--- a/tools/libs/store/xs.c
+++ b/tools/libs/store/xs.c
@@ -811,9 +811,13 @@ bool xs_watch(struct xs_handle *h, const char *path, const char *token)
 
 #ifdef USE_PTHREAD
 #define DEFAULT_THREAD_STACKSIZE (16 * 1024)
+#ifndef PTHREAD_STACK_MIN
+#define READ_THREAD_STACKSIZE DEFAULT_THREAD_STACKSIZE
+#else
 #define READ_THREAD_STACKSIZE 					\
 	((DEFAULT_THREAD_STACKSIZE < PTHREAD_STACK_MIN) ? 	\
 	PTHREAD_STACK_MIN : DEFAULT_THREAD_STACKSIZE)
+#endif
 
 	/* We dynamically create a reader thread on demand. */
 	mutex_lock(&h->request_mutex);
-- 
2.28.0

[PATCH 23/24] Use xen/xenio.h on NetBSD

[Manuel Bouyer]

---
 tools/libs/ctrl/xc_private.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/libs/ctrl/xc_private.h b/tools/libs/ctrl/xc_private.h
index f0b5f83ac8..68e388f488 100644
--- a/tools/libs/ctrl/xc_private.h
+++ b/tools/libs/ctrl/xc_private.h
@@ -39,7 +39,11 @@
 #include <xenforeignmemory.h>
 #include <xendevicemodel.h>
 
+#ifdef __NetBSD__
+#include <xen/xenio.h>
+#else
 #include <xen/sys/privcmd.h>
+#endif
 
 #include <xen-tools/libs.h>
 
-- 
2.28.0

[PATCH 24/24] Fix error: array subscript has type 'char' [-Werror=char-subscripts]

[Manuel Bouyer]

---
 tools/libs/light/libxl_qmp.c | 2 +-
 tools/xentrace/xentrace.c    | 2 +-
 xen/tools/symbols.c          | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/libs/light/libxl_qmp.c b/tools/libs/light/libxl_qmp.c
index c394000ea9..9b638e6f54 100644
--- a/tools/libs/light/libxl_qmp.c
+++ b/tools/libs/light/libxl_qmp.c
@@ -1249,7 +1249,7 @@ static int qmp_error_class_to_libxl_error_code(libxl__gc *gc,
                 se++;
                 continue;
             }
-            if (tolower(*s) != tolower(*se))
+            if (tolower((unsigned char)*s) != tolower((unsigned char)*se))
                 break;
             s++, se++;
         }
diff --git a/tools/xentrace/xentrace.c b/tools/xentrace/xentrace.c
index 4b50b8a53e..a8903ebf46 100644
--- a/tools/xentrace/xentrace.c
+++ b/tools/xentrace/xentrace.c
@@ -957,7 +957,7 @@ static int parse_cpumask_range(const char *mask_str, xc_cpumap_t map)
 {
     unsigned int a, b;
     int nmaskbits;
-    char c;
+    unsigned char c;
     int in_range;
     const char *s;
 
diff --git a/xen/tools/symbols.c b/xen/tools/symbols.c
index 9f9e2c9900..0b12452616 100644
--- a/xen/tools/symbols.c
+++ b/xen/tools/symbols.c
@@ -173,11 +173,11 @@ static int read_symbol(FILE *in, struct sym_entry *s)
 	/* include the type field in the symbol name, so that it gets
 	 * compressed together */
 	s->len = strlen(str) + 1;
-	if (islower(stype) && filename)
+	if (islower((unsigned char)stype) && filename)
 		s->len += strlen(filename) + 1;
 	s->sym = malloc(s->len + 1);
 	sym = SYMBOL_NAME(s);
-	if (islower(stype) && filename) {
+	if (islower((unsigned char)stype) && filename) {
 		sym = stpcpy(sym, filename);
 		*sym++ = '#';
 	}
-- 
2.28.0

Re: [PATCH 14/24] Pass bridge name to qemu and set XEN_DOMAIN_ID

[Manuel Bouyer]

On Mon, Dec 14, 2020 at 05:36:13PM +0100, Manuel Bouyer wrote:
> Pass bridge name to qemu
> When starting qemu, set an environnement variable XEN_DOMAIN_ID,
> to be used by qemu helper scripts

This one is not NetBSD related, I should have sent is as a separate
git mail ... I guess (i'm not familiar with git, sorry).

But I think it can be usefull for the comunity.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 12/24] Implement gnttab on NetBSD

[Roger Pau Monné]

Might need some kind of log message, and will also required your
Signed-off-by (or from the original author of the patch).

On Mon, Dec 14, 2020 at 05:36:11PM +0100, Manuel Bouyer wrote:
> ---
>  tools/libs/gnttab/Makefile |   2 +-
>  tools/libs/gnttab/netbsd.c | 267 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 268 insertions(+), 1 deletion(-)
>  create mode 100644 tools/libs/gnttab/netbsd.c
> 
> diff --git a/tools/libs/gnttab/Makefile b/tools/libs/gnttab/Makefile
> index d86c49d243..ae390ce60f 100644
> --- a/tools/libs/gnttab/Makefile
> +++ b/tools/libs/gnttab/Makefile
> @@ -10,7 +10,7 @@ SRCS-GNTSHR            += gntshr_core.c
>  SRCS-$(CONFIG_Linux)   += $(SRCS-GNTTAB) $(SRCS-GNTSHR) linux.c
>  SRCS-$(CONFIG_MiniOS)  += $(SRCS-GNTTAB) gntshr_unimp.c minios.c
>  SRCS-$(CONFIG_FreeBSD) += $(SRCS-GNTTAB) $(SRCS-GNTSHR) freebsd.c
> +SRCS-$(CONFIG_NetBSD)  += $(SRCS-GNTTAB) $(SRCS-GNTSHR) netbsd.c
>  SRCS-$(CONFIG_SunOS)   += gnttab_unimp.c gntshr_unimp.c
> -SRCS-$(CONFIG_NetBSD)  += gnttab_unimp.c gntshr_unimp.c
>  
>  include $(XEN_ROOT)/tools/libs/libs.mk
> diff --git a/tools/libs/gnttab/netbsd.c b/tools/libs/gnttab/netbsd.c
> new file mode 100644
> index 0000000000..2df7058cd7
> --- /dev/null
> +++ b/tools/libs/gnttab/netbsd.c

I think this is mostly (if not equal) to the FreeBSD version, in which
case we could rename freebsd.c to plain bsd.c and use it for
both FreeBSD and NetBSD?

Thanks, Roger.

Re: [PATCH 02/24] NetBSD doens't need xenbackendd with xl toolstack

[Roger Pau Monné]

On Mon, Dec 14, 2020 at 05:36:01PM +0100, Manuel Bouyer wrote:
> ---
>  tools/Makefile | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index ed71474421..757a560be0 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -18,7 +18,6 @@ SUBDIRS-$(CONFIG_X86) += firmware
>  SUBDIRS-y += console
>  SUBDIRS-y += xenmon
>  SUBDIRS-y += xentop
> -SUBDIRS-$(CONFIG_NetBSD) += xenbackendd

I think we also want to remove the directory itself, as it would be
dead code at this point.

Thanks, Roger.

Re: [PATCH 04/24] Make xg_main.c build on NetBSD

[Roger Pau Monné]

On Mon, Dec 14, 2020 at 05:36:03PM +0100, Manuel Bouyer wrote:
> ---
>  tools/debugger/gdbsx/xg/xg_main.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/tools/debugger/gdbsx/xg/xg_main.c b/tools/debugger/gdbsx/xg/xg_main.c
> index a4e8653168..fa2741ccf8 100644
> --- a/tools/debugger/gdbsx/xg/xg_main.c
> +++ b/tools/debugger/gdbsx/xg/xg_main.c
> @@ -49,7 +49,11 @@
>  #include "xg_public.h"
>  #include <xen/version.h>
>  #include <xen/domctl.h>
> +#ifdef __NetBSD__
> +#include <xen/xenio.h>
> +#else
>  #include <xen/sys/privcmd.h>
> +#endif
>  #include <xen/foreign/x86_32.h>
>  #include <xen/foreign/x86_64.h>
>  
> @@ -126,12 +130,19 @@ xg_init()
>      int flags, saved_errno;
>  
>      XGTRC("E\n");
> +#ifdef __NetBSD__
> +    if ((_dom0_fd=open("/kern/xen/privcmd", O_RDWR)) == -1) {
> +        perror("Failed to open /kern/xen/privcmd\n");
> +        return -1;
> +    }
> +#else
>      if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1) {
>          if ((_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1) {
>              perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd\n");
>              return -1;
>          }
>      }
> +#endif

I don't think you need to ifdef here, instead just add to the existing
if, ie:

    if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1 &&
        (_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1 &&
	(_dom0_fd=open("/kern/xen/privcmd", O_RDWR)) == -1) {
        perror("Failed to open /dev/xen/privcmd, /proc/xen/privcmd or /kern/xen/privcmd\n");
        return -1;
    }

Thanks, Roger.

Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD

[Roger Pau Monné]

I think you want tot CC the tools dev on this one, specially Ian who
knows how the Linux one is implemented and can likely give valuable
input.

On Mon, Dec 14, 2020 at 05:36:04PM +0100, Manuel Bouyer wrote:
> ---
>  tools/hotplug/NetBSD/Makefile   |  1 +
>  tools/hotplug/NetBSD/block      |  5 ++-
>  tools/hotplug/NetBSD/locking.sh | 72 +++++++++++++++++++++++++++++++++

Seeing the file itself, I don't think there's any NetBSD specific
stuff, so we might want to consider putting it in BSD/ instead, so it
can be used by FreeBSD also?

I'm also not sure if it would be useful to Linux folks?

Thanks, Roger.

Re: [PATCH 06/24] Handle the case where vifname is not present in xenstore.

[Roger Pau Monné]

Maybe it would be easier to just fix libxl to always set the vifname
in xenstore?

FWIW, on FreeBSD I'm passing the vifname as an environment parameter
to the script.

Thanks, Roger.

Re: [PATCH 07/24] Remove NetBSD's system headers. We'll use the system-provided ones, which are up to date.

[Roger Pau Monné]

What would happen when a new device (or ioctl to and existing one) is
added?

You would then run into issues of newer versions of Xen not building on
older NetBSD systems, or would have to appropriately gate the newly
added code to only be built when the headers are available.

Thanks, Roger.

Re: [PATCH 08/24] Make libs/call build on NetBSD

[Roger Pau Monné]

LGTM.

On Mon, Dec 14, 2020 at 05:36:07PM +0100, Manuel Bouyer wrote:
> ---
>  tools/libs/call/netbsd.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/libs/call/netbsd.c b/tools/libs/call/netbsd.c
> index a5502da377..1a771e9928 100644
> --- a/tools/libs/call/netbsd.c
> +++ b/tools/libs/call/netbsd.c
> @@ -19,12 +19,14 @@
>   * Split from xc_netbsd.c
>   */
>  
> -#include "xc_private.h"
>  
>  #include <unistd.h>
>  #include <fcntl.h>
>  #include <malloc.h>
> +#include <errno.h>
>  #include <sys/mman.h>
> +#include <sys/ioctl.h>
> +#include "private.h"

Please leave a newline before including private.h.

>  
>  int osdep_xencall_open(xencall_handle *xcall)
>  {
> @@ -69,12 +71,13 @@ int osdep_xencall_close(xencall_handle *xcall)
>      return close(fd);
>  }
>  
> -void *osdep_alloc_hypercall_buffer(xencall_handle *xcall, size_t npages)
> +void *osdep_alloc_pages(xencall_handle *xcall, size_t npages)
>  {
> -    size_t size = npages * XC_PAGE_SIZE;
> +    size_t size = npages * PAGE_SIZE;
>      void *p;
> +    int ret;
>  
> -    ret = posix_memalign(&p, XC_PAGE_SIZE, size);
> +    ret = posix_memalign(&p, PAGE_SIZE, size);
>      if ( ret != 0 || !p )
>          return NULL;
>  
> @@ -86,14 +89,13 @@ void *osdep_alloc_hypercall_buffer(xencall_handle *xcall, size_t npages)
>      return p;
>  }
>  
> -void osdep_free_hypercall_buffer(xencall_handle *xcall, void *ptr,
> -                                 size_t npages)
> +void osdep_free_pages(xencall_handle *xcall, void *ptr, size_t npages)
>  {
> -    (void) munlock(ptr, npages * XC_PAGE_SIZE);
> +    (void) munlock(ptr, npages * PAGE_SIZE);

I think you can drop the (void) cast here.

Thanks, Roger.

Re: [PATCH 10/24] Make libs/evtchn build on NetBSD

[Roger Pau Monné]

On Mon, Dec 14, 2020 at 05:36:09PM +0100, Manuel Bouyer wrote:
> ---
>  tools/libs/evtchn/netbsd.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
> index 8b8545d2f9..6d4ce28011 100644
> --- a/tools/libs/evtchn/netbsd.c
> +++ b/tools/libs/evtchn/netbsd.c
> @@ -25,10 +25,10 @@
>  
>  #include <sys/ioctl.h>
>  
> -#include <xen/sys/evtchn.h>
> -
>  #include "private.h"
>  
> +#include <xen/xenio3.h>
> +
>  #define EVTCHN_DEV_NAME  "/dev/xenevt"
>  
>  int osdep_evtchn_open(xenevtchn_handle *xce)
> @@ -131,7 +131,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
>      int fd = xce->fd;
>      evtchn_port_t port;
>  
> -    if ( read_exact(fd, (char *)&port, sizeof(port)) == -1 )
> +    if ( read(fd, (char *)&port, sizeof(port)) == -1 )
>          return -1;
>  
>      return port;
> @@ -140,7 +140,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
>  int xenevtchn_unmask(xenevtchn_handle *xce, evtchn_port_t port)
>  {
>      int fd = xce->fd;
> -    return write_exact(fd, (char *)&port, sizeof(port));
> +    return write(fd, (char *)&port, sizeof(port));

I'm afraid we will need some context as to why {read/write}_exact
doesn't work here.

Thanks, Roger.

Re: [PATCH 11/24] Implement foreignmemory on NetBSD

[Roger Pau Monné]

On Mon, Dec 14, 2020 at 05:36:10PM +0100, Manuel Bouyer wrote:
> ---
>  tools/libs/foreignmemory/Makefile  |  2 +-
>  tools/libs/foreignmemory/netbsd.c  | 76 ++++++++++++++++++++++++++----
>  tools/libs/foreignmemory/private.h | 10 +++-
>  3 files changed, 75 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/libs/foreignmemory/Makefile b/tools/libs/foreignmemory/Makefile
> index 13850f7988..f191cdbed0 100644
> --- a/tools/libs/foreignmemory/Makefile
> +++ b/tools/libs/foreignmemory/Makefile
> @@ -8,7 +8,7 @@ SRCS-y                 += core.c
>  SRCS-$(CONFIG_Linux)   += linux.c
>  SRCS-$(CONFIG_FreeBSD) += freebsd.c
>  SRCS-$(CONFIG_SunOS)   += compat.c solaris.c
> -SRCS-$(CONFIG_NetBSD)  += compat.c netbsd.c
> +SRCS-$(CONFIG_NetBSD)  += netbsd.c
>  SRCS-$(CONFIG_MiniOS)  += minios.c
>  
>  include $(XEN_ROOT)/tools/libs/libs.mk
> diff --git a/tools/libs/foreignmemory/netbsd.c b/tools/libs/foreignmemory/netbsd.c
> index 54a418ebd6..6d740ec2a3 100644
> --- a/tools/libs/foreignmemory/netbsd.c
> +++ b/tools/libs/foreignmemory/netbsd.c
> @@ -19,7 +19,9 @@
>  
>  #include <unistd.h>
>  #include <fcntl.h>
> +#include <errno.h>
>  #include <sys/mman.h>
> +#include <sys/ioctl.h>
>  
>  #include "private.h"
>  
> @@ -66,15 +68,17 @@ int osdep_xenforeignmemory_close(xenforeignmemory_handle *fmem)
>      return close(fd);
>  }
>  
> -void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
> -                              void *addr, int prot, int flags,
> -                              xen_pfn_t *arr, int num)
> +void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
> +                                 uint32_t dom, void *addr,
> +				 int prot, int flags, size_t num,
> +				 const xen_pfn_t arr[/*num*/], int err[/*num*/])
> +
>  {
>      int fd = fmem->fd;
> -    privcmd_mmapbatch_t ioctlx;
> -    addr = mmap(addr, num*XC_PAGE_SIZE, prot, flags | MAP_ANON | MAP_SHARED, -1, 0);
> +    privcmd_mmapbatch_v2_t ioctlx;
> +    addr = mmap(addr, num*PAGE_SIZE, prot, flags | MAP_ANON | MAP_SHARED, -1, 0);
>      if ( addr == MAP_FAILED ) {
> -        PERROR("osdep_map_foreign_batch: mmap failed");
> +        PERROR("osdep_xenforeignmemory_map: mmap failed");
>          return NULL;
>      }
>  
> @@ -82,11 +86,12 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
>      ioctlx.dom=dom;
>      ioctlx.addr=(unsigned long)addr;
>      ioctlx.arr=arr;
> -    if ( ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH, &ioctlx) < 0 )
> +    ioctlx.err=err;
> +    if ( ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH_V2, &ioctlx) < 0 )
>      {
>          int saved_errno = errno;
> -        PERROR("osdep_map_foreign_batch: ioctl failed");
> -        (void)munmap(addr, num*XC_PAGE_SIZE);
> +        PERROR("osdep_xenforeignmemory_map: ioctl failed");
> +        (void)munmap(addr, num*PAGE_SIZE);
>          errno = saved_errno;
>          return NULL;
>      }
> @@ -97,7 +102,58 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
>  int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
>                                   void *addr, size_t num)
>  {
> -    return munmap(addr, num*XC_PAGE_SIZE);
> +    return munmap(addr, num*PAGE_SIZE);
> +}
> +
> +int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
> +                                    domid_t domid)
> +{
> +    return 0;

Returning 0 here is wrong, since you are not implementing it. You
should return -1 and set errno to EOPNOTSUPP.

> +}
> +
> +int osdep_xenforeignmemory_unmap_resource(
> +    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
> +{
> +    return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) : 0;
> +}
> +
> +int osdep_xenforeignmemory_map_resource(
> +    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
> +{
> +    privcmd_mmap_resource_t mr = {
> +        .dom = fres->domid,
> +        .type = fres->type,
> +        .id = fres->id,
> +        .idx = fres->frame,
> +        .num = fres->nr_frames,
> +    };
> +    int rc;
> +
> +    fres->addr = mmap(fres->addr, fres->nr_frames << PAGE_SHIFT,
> +                      fres->prot, fres->flags | MAP_ANON | MAP_SHARED, -1, 0);
> +    if ( fres->addr == MAP_FAILED )
> +        return -1;
> +
> +    mr.addr = (uintptr_t)fres->addr;
> +
> +    rc = ioctl(fmem->fd, IOCTL_PRIVCMD_MMAP_RESOURCE, &mr);
> +    if ( rc )
> +    {
> +        int saved_errno;
> +
> +        if ( errno != fmem->unimpl_errno && errno != EOPNOTSUPP )

Maybe this is set in another patch, but I don't seem to spot where
fmem->unimpl_errno is set on NetBSD (seems to be set only for Linux
AFAICT).

> +            PERROR("ioctl failed");
> +        else
> +            errno = EOPNOTSUPP;
> +
> +        saved_errno = errno;
> +        (void)osdep_xenforeignmemory_unmap_resource(fmem, fres);
> +        errno = saved_errno;
> +
> +        return -1;
> +    }
> +
> +    return 0;
>  }
>  
>  /*
> diff --git a/tools/libs/foreignmemory/private.h b/tools/libs/foreignmemory/private.h
> index 8f1bf081ed..abeceb8720 100644
> --- a/tools/libs/foreignmemory/private.h
> +++ b/tools/libs/foreignmemory/private.h
> @@ -8,7 +8,13 @@
>  #include <xentoolcore_internal.h>
>  
>  #include <xen/xen.h>
> +
> +#ifdef __NetBSD__
> +#include <xen/xen.h>
> +#include <xen/xenio.h>
> +#else
>  #include <xen/sys/privcmd.h>
> +#endif
>  
>  #ifndef PAGE_SHIFT /* Mini-os, Yukk */
>  #define PAGE_SHIFT           12
> @@ -38,7 +44,7 @@ int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
>  
>  #if defined(__NetBSD__) || defined(__sun__)
>  /* Strictly compat for those two only only */
> -void *compat_mapforeign_batch(xenforeignmem_handle *fmem, uint32_t dom,
> +void *osdep_map_foreign_batch(xenforeignmemory_handle *fmem, uint32_t dom,
>                                void *addr, int prot, int flags,
>                                xen_pfn_t *arr, int num);

You will have to split this into NetBSD and Solaris variants now if
this is really required, but AFAICT you replace
osdep_map_foreign_batch with osdep_xenforeignmemory_map, so this
prototype is stale?

Also a little bit below these prototypes are the dummy implementations
of osdep_xenforeignmemory_restrict,
osdep_xenforeignmemory_map_resource and
osdep_xenforeignmemory_unmap_resource. I think you at least need to
modify the condition below so that on NetBSD the dummy inlines are not
used?

Thanks, Roger.

Re: [PATCH 13/24] Don't assume tv_sec is a unsigned long (for NetBSD)

[Roger Pau Monné]

On Mon, Dec 14, 2020 at 05:36:12PM +0100, Manuel Bouyer wrote:
> ---
>  tools/libs/light/libxl_create.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> index 321a13e519..44691010bc 100644
> --- a/tools/libs/light/libxl_create.c
> +++ b/tools/libs/light/libxl_create.c
> @@ -496,7 +496,7 @@ int libxl__domain_build(libxl__gc *gc,
>          vments[2] = "image/ostype";
>          vments[3] = "hvm";
>          vments[4] = "start_time";
> -        vments[5] = GCSPRINTF("%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
> +        vments[5] = GCSPRINTF("%jd.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);

You don't cast tv_sec to intmax_t here...

>  
>          localents = libxl__calloc(gc, 13, sizeof(char *));
>          i = 0;
> @@ -535,7 +535,7 @@ int libxl__domain_build(libxl__gc *gc,
>          vments[i++] = "image/kernel";
>          vments[i++] = (char *) state->pv_kernel.path;
>          vments[i++] = "start_time";
> -        vments[i++] = GCSPRINTF("%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
> +        vments[i++] = GCSPRINTF("%jd.%02d", (intmax_t)start_time.tv_sec,(int)start_time.tv_usec/10000);

... yet you do it here. I think the first occurrence is missing the
cast?

Thanks, Roger.

Re: [PATCH 15/24] Make libs/light build on NetBSD

[Roger Pau Monné]

On Mon, Dec 14, 2020 at 05:36:14PM +0100, Manuel Bouyer wrote:
> ---
>  tools/libs/light/libxl_dm.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> index 5948ace60d..c93bdf2cc9 100644
> --- a/tools/libs/light/libxl_dm.c
> +++ b/tools/libs/light/libxl_dm.c
> @@ -3659,6 +3659,14 @@ static int kill_device_model_uid_child(libxl__destroy_devicemodel_state *ddms,
>  
>      LOGD(DEBUG, domid, "DM reaper: calling setresuid(%d, %d, 0)",

For correctness you should change this log message also on NetBSD.

>           reaper_uid, dm_kill_uid);
> +#ifdef __NetBSD__
> +    r = setuid(dm_kill_uid);
> +    if (r) {
> +        LOGED(ERROR, domid, "setuid to %d", dm_kill_uid);
> +        rc = rc ?: ERROR_FAIL;
> +        goto out;
> +    }
> +#else /* __NetBSD__ */
>      r = setresuid(reaper_uid, dm_kill_uid, 0);
>      if (r) {
>          LOGED(ERROR, domid, "setresuid to (%d, %d, 0)",
> @@ -3666,6 +3674,7 @@ static int kill_device_model_uid_child(libxl__destroy_devicemodel_state *ddms,
>          rc = rc ?: ERROR_FAIL;
>          goto out;
>      }
> +#endif /* __NetBSD__ */

Instead of adding this NetBSD specific bodge here I would add a test
for setresuid in tools/configure.ac using AC_CHECK_FUNCS and use the
result from that. Then if/when NetBSD implements setresuid the switch
will be done transparently.

Thanks, Roger.

Re: [PATCH 16/24] Switch NetBSD to QEMU_XEN (!traditional)

[Roger Pau Monné]

On Mon, Dec 14, 2020 at 05:36:15PM +0100, Manuel Bouyer wrote:
> ---
>  tools/libs/light/libxl_netbsd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libs/light/libxl_netbsd.c b/tools/libs/light/libxl_netbsd.c
> index e66a393d7f..31334f932c 100644
> --- a/tools/libs/light/libxl_netbsd.c
> +++ b/tools/libs/light/libxl_netbsd.c
> @@ -110,7 +110,7 @@ out:
>  
>  libxl_device_model_version libxl__default_device_model(libxl__gc *gc)
>  {
> -    return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> +    return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
>  }

All libxl supported OSes will now be using upstream QEMU as default,
maybe it's best to just move libxl__default_device_model into
libxl_dm.c instead of having 3 equal copies in OS specific files.

Thanks, Roger.

Re: [PATCH 17/24] Make libs/light build on NetBSD

[Roger Pau Monné]

There's already a patch with the same subject in the series. I would
recommend to be a bit more specific with the fixes, specally if
there's no log message. This for example would better be:

tools/libxl: fix uuid build on NetBSD

On Mon, Dec 14, 2020 at 05:36:16PM +0100, Manuel Bouyer wrote:
> ---
>  tools/libs/light/libxl_uuid.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_uuid.c b/tools/libs/light/libxl_uuid.c
> index dadb79bad8..a8ee5f253e 100644
> --- a/tools/libs/light/libxl_uuid.c
> +++ b/tools/libs/light/libxl_uuid.c
> @@ -82,7 +82,7 @@ void libxl_uuid_generate(libxl_uuid *uuid)
>      uuid_enc_be(uuid->uuid, &nat_uuid);
>  }
>  
> -#ifdef __FreeBSD__
> +#if defined(__FreeBSD__) || defined(__NetBSD__)
>  int libxl_uuid_from_string(libxl_uuid *uuid, const char *in)
>  {
>      uint32_t status;
> @@ -120,7 +120,7 @@ void libxl_uuid_clear(libxl_uuid *uuid)
>      memset(&uuid->uuid, 0, sizeof(uuid->uuid));
>  }
>  
> -#ifdef __FreeBSD__
> +#if defined(__FreeBSD__) || defined(__NetBSD__)

There's no need to add NetBSD here, just remove the #ifdef altogether
and the content of the #else branch, since this section is already
only for FreeBSD and NetBSD (the #else variant was only used by
NetBSD, see the #elif defined(__FreeBSD__) || defined(__NetBSD__) up
on the file).

Thanks, Roger.

Re: [PATCH 18/24] xeneventchn_stubs.c doens't need xen/sys/evtchn.h (NetBSD fix)

[Roger Pau Monné]

If it builds on other OSes:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Subject would better be:

tools/evtchn: remove unneeded evtchn.h include

Thanks, Roger.

Re: [PATCH 19/24] errno may not be a gobal R/W variable, use a local variable instead (fix build on NetBSD)

[Roger Pau Monné]

On Mon, Dec 14, 2020 at 05:36:18PM +0100, Manuel Bouyer wrote:
> ---
>  tools/xenpaging/xenpaging.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
> index 33098046c2..39c8c83b4b 100644
> --- a/tools/xenpaging/xenpaging.c
> +++ b/tools/xenpaging/xenpaging.c
> @@ -180,10 +180,11 @@ static int xenpaging_get_tot_pages(struct xenpaging *paging)
>  static void *init_page(void)
>  {
>      void *buffer;
> +    int rc;
>  
>      /* Allocated page memory */
> -    errno = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
> -    if ( errno != 0 )
> +    rc = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
> +    if ( rc != 0 )

I think the point of setting errno here is because posix_memalign
doesn't set it and instead returns an error code. The caller of
init_page uses PERROR in order to print the error which his expected to
be in errno.

I don't think this is the only place in Xen code that errno is set, why
are the others fine but not this instance?

Thanks, Roger.

Re: [PATCH 20/24] If FILENAME_MAX is defined, use it instead of arbitrary value (fix format-truncation errors with GCC >= 7)

[Roger Pau Monné]

On Mon, Dec 14, 2020 at 05:36:19PM +0100, Manuel Bouyer wrote:
> ---
>  tools/xenpmd/xenpmd.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/xenpmd/xenpmd.c b/tools/xenpmd/xenpmd.c
> index 12b82cf43e..cfd22e64e3 100644
> --- a/tools/xenpmd/xenpmd.c
> +++ b/tools/xenpmd/xenpmd.c
> @@ -101,7 +101,11 @@ FILE *get_next_battery_file(DIR *battery_dir,
>  {
>      FILE *file = 0;
>      struct dirent *dir_entries;
> +#ifdef FILENAME_MAX
> +    char file_name[FILENAME_MAX];
> +#else
>      char file_name[284];
> +#endif
>      int ret;

I think it's dangerous to do this, specially on the stack, GNU libc
manual states:

Usage Note: Don’t use FILENAME_MAX as the size of an array in which to
store a file name! You can’t possibly make an array that big! Use
dynamic allocation (see Memory Allocation) instead.

I think it would be better to replace the snprintf calls with asprintf
and free the buffer afterwards. Setting file_name to 284 should be
fine however, as d_name is 256 max and the paths above are 26 maximum
I think (27 with the nul character).

Thanks, Roger.

Re: [PATCH 22/24] If PTHREAD_STACK_MIN is not defined, use DEFAULT_THREAD_STACKSIZE

[Roger Pau Monné]

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

I've been told that PTHREAD_STACK_MIN is not designed to be used like
this anyway, and that what we do could be dangerous:

https://lists.freebsd.org/pipermail/freebsd-current/2014-March/048885.html

Please adjust the subject line to mention this applies to
tools/xenstore and a bit of commit message saying it's not available
on NetBSD.

Thanks.

Re: [PATCH 14/24] Pass bridge name to qemu and set XEN_DOMAIN_ID

[Roger Pau Monné]

On Mon, Dec 14, 2020 at 05:36:13PM +0100, Manuel Bouyer wrote:
> Pass bridge name to qemu
> When starting qemu, set an environnement variable XEN_DOMAIN_ID,
> to be used by qemu helper scripts

NetBSD is the only one to use QEMU nic scripts, both FreeBSD and Linux
don't use up/down scripts with QEMU.

> ---
>  tools/libs/light/libxl_dm.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> index 3da83259c0..5948ace60d 100644
> --- a/tools/libs/light/libxl_dm.c
> +++ b/tools/libs/light/libxl_dm.c
> @@ -761,6 +761,10 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
>          int nr_set_cpus = 0;
>          char *s;
>  
> +	static char buf[12];
> +	snprintf(buf, sizeof(buf), "%d", domid);
> +        flexarray_append_pair(dm_envs, "XEN_DOMAIN_ID", buf);

Indentation, here and below.

Also just use:

flexarray_append_pair(dm_envs, "XEN_DOMAIN_ID",
                      GCSPRINTF("%d", domid);

Here and below.

Thanks, Roger.

Re: [PATCH 00/24] NetBSD fixes

[Roger Pau Monné]

On Mon, Dec 14, 2020 at 05:35:59PM +0100, Manuel Bouyer wrote:
> Hello,
> here is a set of 24 patches, which are needed to build and run the
> tools on NetBSD. They are extracted from NetBSD's pkgsrc repository for
> Xen 4.13, and ported to 4.15. 

Thanks! I think they are mostly fine. All of them are however missing
a Signed-off-by tag, which is mandatory for getting them accepted.
Also you should Cc the maintainers on patches, so they can review
them. There's a script in tree to do that, you can use it against the
patch files and it will append the appropriate Cc's:

% ./scripts/add_maintainers.pl -d patch/directory

Optionally use the --reroll-count when sending new versions of the
series (2,3...)

Thanks, Roger.

Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD

[Manuel Bouyer]

On Tue, Dec 29, 2020 at 12:29:09PM +0100, Roger Pau Monné wrote:
> I think you want tot CC the tools dev on this one, specially Ian who
> knows how the Linux one is implemented and can likely give valuable
> input.
> 
> On Mon, Dec 14, 2020 at 05:36:04PM +0100, Manuel Bouyer wrote:
> > ---
> >  tools/hotplug/NetBSD/Makefile   |  1 +
> >  tools/hotplug/NetBSD/block      |  5 ++-
> >  tools/hotplug/NetBSD/locking.sh | 72 +++++++++++++++++++++++++++++++++
> 
> Seeing the file itself, I don't think there's any NetBSD specific
> stuff, so we might want to consider putting it in BSD/ instead, so it
> can be used by FreeBSD also?

I'm not sure if FreeBSD needs the locking stuff.
Also, there are certainly differences in block device handling between
FreeBSD and NetBSD. Both OSes have diverged in this area.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 06/24] Handle the case where vifname is not present in xenstore.

[Manuel Bouyer]

On Tue, Dec 29, 2020 at 12:43:02PM +0100, Roger Pau Monné wrote:
> Maybe it would be easier to just fix libxl to always set the vifname
> in xenstore?
> 
> FWIW, on FreeBSD I'm passing the vifname as an environment parameter
> to the script.

Actually I don't know why the vifname is not always set. It may depend on
the Xen version ...

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 07/24] Remove NetBSD's system headers. We'll use the system-provided ones, which are up to date.

[Manuel Bouyer]

On Tue, Dec 29, 2020 at 12:46:38PM +0100, Roger Pau Monné wrote:
> What would happen when a new device (or ioctl to and existing one) is
> added?
> 
> You would then run into issues of newer versions of Xen not building on
> older NetBSD systems, or would have to appropriately gate the newly
> added code to only be built when the headers are available.

I prefer to have the build fail if the system isn't new enough, than
have it build and then fail to run. We already have version requirements
for e.g. bug fixes.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 10/24] Make libs/evtchn build on NetBSD

[Manuel Bouyer]

On Tue, Dec 29, 2020 at 12:52:43PM +0100, Roger Pau Monné wrote:
> On Mon, Dec 14, 2020 at 05:36:09PM +0100, Manuel Bouyer wrote:
> > ---
> >  tools/libs/evtchn/netbsd.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
> > index 8b8545d2f9..6d4ce28011 100644
> > --- a/tools/libs/evtchn/netbsd.c
> > +++ b/tools/libs/evtchn/netbsd.c
> > @@ -25,10 +25,10 @@
> >  
> >  #include <sys/ioctl.h>
> >  
> > -#include <xen/sys/evtchn.h>
> > -
> >  #include "private.h"
> >  
> > +#include <xen/xenio3.h>
> > +
> >  #define EVTCHN_DEV_NAME  "/dev/xenevt"
> >  
> >  int osdep_evtchn_open(xenevtchn_handle *xce)
> > @@ -131,7 +131,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> >      int fd = xce->fd;
> >      evtchn_port_t port;
> >  
> > -    if ( read_exact(fd, (char *)&port, sizeof(port)) == -1 )
> > +    if ( read(fd, (char *)&port, sizeof(port)) == -1 )
> >          return -1;
> >  
> >      return port;
> > @@ -140,7 +140,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> >  int xenevtchn_unmask(xenevtchn_handle *xce, evtchn_port_t port)
> >  {
> >      int fd = xce->fd;
> > -    return write_exact(fd, (char *)&port, sizeof(port));
> > +    return write(fd, (char *)&port, sizeof(port));
> 
> I'm afraid we will need some context as to why {read/write}_exact
> doesn't work here.

It just doesn't exists on NetBSD

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 12/24] Implement gnttab on NetBSD

[Manuel Bouyer]

On Tue, Dec 29, 2020 at 12:16:01PM +0100, Roger Pau Monné wrote:
> Might need some kind of log message, and will also required your
> Signed-off-by (or from the original author of the patch).
> 
> On Mon, Dec 14, 2020 at 05:36:11PM +0100, Manuel Bouyer wrote:
> > ---
> >  tools/libs/gnttab/Makefile |   2 +-
> >  tools/libs/gnttab/netbsd.c | 267 +++++++++++++++++++++++++++++++++++++
> >  2 files changed, 268 insertions(+), 1 deletion(-)
> >  create mode 100644 tools/libs/gnttab/netbsd.c
> > 
> > diff --git a/tools/libs/gnttab/Makefile b/tools/libs/gnttab/Makefile
> > index d86c49d243..ae390ce60f 100644
> > --- a/tools/libs/gnttab/Makefile
> > +++ b/tools/libs/gnttab/Makefile
> > @@ -10,7 +10,7 @@ SRCS-GNTSHR            += gntshr_core.c
> >  SRCS-$(CONFIG_Linux)   += $(SRCS-GNTTAB) $(SRCS-GNTSHR) linux.c
> >  SRCS-$(CONFIG_MiniOS)  += $(SRCS-GNTTAB) gntshr_unimp.c minios.c
> >  SRCS-$(CONFIG_FreeBSD) += $(SRCS-GNTTAB) $(SRCS-GNTSHR) freebsd.c
> > +SRCS-$(CONFIG_NetBSD)  += $(SRCS-GNTTAB) $(SRCS-GNTSHR) netbsd.c
> >  SRCS-$(CONFIG_SunOS)   += gnttab_unimp.c gntshr_unimp.c
> > -SRCS-$(CONFIG_NetBSD)  += gnttab_unimp.c gntshr_unimp.c
> >  
> >  include $(XEN_ROOT)/tools/libs/libs.mk
> > diff --git a/tools/libs/gnttab/netbsd.c b/tools/libs/gnttab/netbsd.c
> > new file mode 100644
> > index 0000000000..2df7058cd7
> > --- /dev/null
> > +++ b/tools/libs/gnttab/netbsd.c
> 
> I think this is mostly (if not equal) to the FreeBSD version, in which
> case we could rename freebsd.c to plain bsd.c and use it for
> both FreeBSD and NetBSD?

I can't see why they won't diverge in the future ...
I prefer to keep them separate.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 13/24] Don't assume tv_sec is a unsigned long (for NetBSD)

[Manuel Bouyer]

On Tue, Dec 29, 2020 at 03:02:15PM +0100, Roger Pau Monné wrote:
> On Mon, Dec 14, 2020 at 05:36:12PM +0100, Manuel Bouyer wrote:
> > ---
> >  tools/libs/light/libxl_create.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> > index 321a13e519..44691010bc 100644
> > --- a/tools/libs/light/libxl_create.c
> > +++ b/tools/libs/light/libxl_create.c
> > @@ -496,7 +496,7 @@ int libxl__domain_build(libxl__gc *gc,
> >          vments[2] = "image/ostype";
> >          vments[3] = "hvm";
> >          vments[4] = "start_time";
> > -        vments[5] = GCSPRINTF("%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
> > +        vments[5] = GCSPRINTF("%jd.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
> 
> You don't cast tv_sec to intmax_t here...
> 
> >  
> >          localents = libxl__calloc(gc, 13, sizeof(char *));
> >          i = 0;
> > @@ -535,7 +535,7 @@ int libxl__domain_build(libxl__gc *gc,
> >          vments[i++] = "image/kernel";
> >          vments[i++] = (char *) state->pv_kernel.path;
> >          vments[i++] = "start_time";
> > -        vments[i++] = GCSPRINTF("%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
> > +        vments[i++] = GCSPRINTF("%jd.%02d", (intmax_t)start_time.tv_sec,(int)start_time.tv_usec/10000);
> 
> ... yet you do it here. I think the first occurrence is missing the
> cast?

thanks, fixed.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 14/24] Pass bridge name to qemu and set XEN_DOMAIN_ID

[Manuel Bouyer]

On Tue, Dec 29, 2020 at 04:19:58PM +0100, Roger Pau Monné wrote:
> On Mon, Dec 14, 2020 at 05:36:13PM +0100, Manuel Bouyer wrote:
> > Pass bridge name to qemu
> > When starting qemu, set an environnement variable XEN_DOMAIN_ID,
> > to be used by qemu helper scripts
> 
> NetBSD is the only one to use QEMU nic scripts, both FreeBSD and Linux
> don't use up/down scripts with QEMU.

I think the scripts are more flexible though; they can be hacked for
local needs ...

> 
> > ---
> >  tools/libs/light/libxl_dm.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> > index 3da83259c0..5948ace60d 100644
> > --- a/tools/libs/light/libxl_dm.c
> > +++ b/tools/libs/light/libxl_dm.c
> > @@ -761,6 +761,10 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
> >          int nr_set_cpus = 0;
> >          char *s;
> >  
> > +	static char buf[12];
> > +	snprintf(buf, sizeof(buf), "%d", domid);
> > +        flexarray_append_pair(dm_envs, "XEN_DOMAIN_ID", buf);
> 
> Indentation, here and below.
> 
> Also just use:
> 
> flexarray_append_pair(dm_envs, "XEN_DOMAIN_ID",
>                       GCSPRINTF("%d", domid);
> 
> Here and below.

fixed, thanks

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 16/24] Switch NetBSD to QEMU_XEN (!traditional)

[Manuel Bouyer]

On Tue, Dec 29, 2020 at 03:19:14PM +0100, Roger Pau Monné wrote:
> On Mon, Dec 14, 2020 at 05:36:15PM +0100, Manuel Bouyer wrote:
> > ---
> >  tools/libs/light/libxl_netbsd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/libs/light/libxl_netbsd.c b/tools/libs/light/libxl_netbsd.c
> > index e66a393d7f..31334f932c 100644
> > --- a/tools/libs/light/libxl_netbsd.c
> > +++ b/tools/libs/light/libxl_netbsd.c
> > @@ -110,7 +110,7 @@ out:
> >  
> >  libxl_device_model_version libxl__default_device_model(libxl__gc *gc)
> >  {
> > -    return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> > +    return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
> >  }
> 
> All libxl supported OSes will now be using upstream QEMU as default,
> maybe it's best to just move libxl__default_device_model into
> libxl_dm.c instead of having 3 equal copies in OS specific files.

I guess it would be better, yes. But I'll let the Xen developpers decide :)

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 19/24] errno may not be a gobal R/W variable, use a local variable instead (fix build on NetBSD)

[Manuel Bouyer]

On Tue, Dec 29, 2020 at 03:38:53PM +0100, Roger Pau Monné wrote:
> On Mon, Dec 14, 2020 at 05:36:18PM +0100, Manuel Bouyer wrote:
> > ---
> >  tools/xenpaging/xenpaging.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
> > index 33098046c2..39c8c83b4b 100644
> > --- a/tools/xenpaging/xenpaging.c
> > +++ b/tools/xenpaging/xenpaging.c
> > @@ -180,10 +180,11 @@ static int xenpaging_get_tot_pages(struct xenpaging *paging)
> >  static void *init_page(void)
> >  {
> >      void *buffer;
> > +    int rc;
> >  
> >      /* Allocated page memory */
> > -    errno = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
> > -    if ( errno != 0 )
> > +    rc = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
> > +    if ( rc != 0 )
> 
> I think the point of setting errno here is because posix_memalign
> doesn't set it and instead returns an error code. The caller of
> init_page uses PERROR in order to print the error which his expected to
> be in errno.

I understand this. But on NetBSD, errno is:
#define errno (*__errno())

(I think this is related to thread-safety).

> 
> I don't think this is the only place in Xen code that errno is set, why
> are the others fine but not this instance?

probably this code is not used on NetBSD ?

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 20/24] If FILENAME_MAX is defined, use it instead of arbitrary value (fix format-truncation errors with GCC >= 7)

[Manuel Bouyer]

On Tue, Dec 29, 2020 at 03:51:55PM +0100, Roger Pau Monné wrote:
> I think it's dangerous to do this, specially on the stack, GNU libc
> manual states:
> 
> Usage Note: Don?t use FILENAME_MAX as the size of an array in which to
> store a file name! You can?t possibly make an array that big! Use
> dynamic allocation (see Memory Allocation) instead.
> 
> I think it would be better to replace the snprintf calls with asprintf
> and free the buffer afterwards.

I went this route, thanks

> Setting file_name to 284 should be
> fine however, as d_name is 256 max and the paths above are 26 maximum
> I think (27 with the nul character).

On NetBSD d_name is 512 ... I guess this is why gcc complains.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 11/24] Implement foreignmemory on NetBSD

[Manuel Bouyer]

On Tue, Dec 29, 2020 at 01:46:30PM +0100, Roger Pau Monné wrote:
> [...]
> > diff --git a/tools/libs/foreignmemory/private.h b/tools/libs/foreignmemory/private.h
> > index 8f1bf081ed..abeceb8720 100644
> > --- a/tools/libs/foreignmemory/private.h
> > +++ b/tools/libs/foreignmemory/private.h
> > @@ -8,7 +8,13 @@
> >  #include <xentoolcore_internal.h>
> >  
> >  #include <xen/xen.h>
> > +
> > +#ifdef __NetBSD__
> > +#include <xen/xen.h>
> > +#include <xen/xenio.h>
> > +#else
> >  #include <xen/sys/privcmd.h>
> > +#endif
> >  
> >  #ifndef PAGE_SHIFT /* Mini-os, Yukk */
> >  #define PAGE_SHIFT           12
> > @@ -38,7 +44,7 @@ int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
> >  
> >  #if defined(__NetBSD__) || defined(__sun__)
> >  /* Strictly compat for those two only only */
> > -void *compat_mapforeign_batch(xenforeignmem_handle *fmem, uint32_t dom,
> > +void *osdep_map_foreign_batch(xenforeignmemory_handle *fmem, uint32_t dom,
> >                                void *addr, int prot, int flags,
> >                                xen_pfn_t *arr, int num);
> 
> You will have to split this into NetBSD and Solaris variants now if
> this is really required, but AFAICT you replace
> osdep_map_foreign_batch with osdep_xenforeignmemory_map, so this
> prototype is stale?

I think on solaris too, compat_mapforeign_batch() has to be changed to
osdep_map_foreign_batch() as compat.c and solaris.c has something called
osdep_map_foreign_batch(), but nothing defines compat_mapforeign_batch().
Looks like a rename that missed private.h

Indeed osdep_map_foreign_batch() should not be needed on NetBSD now

> 
> Also a little bit below these prototypes are the dummy implementations
> of osdep_xenforeignmemory_restrict,
> osdep_xenforeignmemory_map_resource and
> osdep_xenforeignmemory_unmap_resource. I think you at least need to
> modify the condition below so that on NetBSD the dummy inlines are not
> used?

This is !defined(__NetBSD__) so it should not be used ?

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 19/24] errno may not be a gobal R/W variable, use a local variable instead (fix build on NetBSD)

[Manuel Bouyer]

On Mon, Jan 04, 2021 at 11:56:14AM +0100, Manuel Bouyer wrote:
> On Tue, Dec 29, 2020 at 03:38:53PM +0100, Roger Pau Monné wrote:
> > On Mon, Dec 14, 2020 at 05:36:18PM +0100, Manuel Bouyer wrote:
> > > ---
> > >  tools/xenpaging/xenpaging.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
> > > index 33098046c2..39c8c83b4b 100644
> > > --- a/tools/xenpaging/xenpaging.c
> > > +++ b/tools/xenpaging/xenpaging.c
> > > @@ -180,10 +180,11 @@ static int xenpaging_get_tot_pages(struct xenpaging *paging)
> > >  static void *init_page(void)
> > >  {
> > >      void *buffer;
> > > +    int rc;
> > >  
> > >      /* Allocated page memory */
> > > -    errno = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
> > > -    if ( errno != 0 )
> > > +    rc = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
> > > +    if ( rc != 0 )
> > 
> > I think the point of setting errno here is because posix_memalign
> > doesn't set it and instead returns an error code. The caller of
> > init_page uses PERROR in order to print the error which his expected to
> > be in errno.
> 
> I understand this. But on NetBSD, errno is:
> #define errno (*__errno())
> 
> (I think this is related to thread-safety).
> 
> > 
> > I don't think this is the only place in Xen code that errno is set, why
> > are the others fine but not this instance?
> 
> probably this code is not used on NetBSD ?

I was wrong, errno is writable but it needs #include <errno.h>
I reverted this a fixed it the right way

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 07/24] Remove NetBSD's system headers. We'll use the system-provided ones, which are up to date.

[Roger Pau Monné]

On Mon, Jan 04, 2021 at 11:25:52AM +0100, Manuel Bouyer wrote:
> On Tue, Dec 29, 2020 at 12:46:38PM +0100, Roger Pau Monné wrote:
> > What would happen when a new device (or ioctl to and existing one) is
> > added?
> > 
> > You would then run into issues of newer versions of Xen not building on
> > older NetBSD systems, or would have to appropriately gate the newly
> > added code to only be built when the headers are available.
> 
> I prefer to have the build fail if the system isn't new enough, than
> have it build and then fail to run. We already have version requirements
> for e.g. bug fixes.

We usually take a different approach for Linux and FreeBSD in
order to support all kernels: test if the new ioctl is available, or
else fallback to the old implementation. But this requires having the
new header even on old systems in order to have the ioctl definition
for the build, even if it's not implemented on the currently running
kernel.

I guess you would have to use preprocessor conditionals in order to
keep the build on older versions when adding the new features.

Roger.

Re: [PATCH 10/24] Make libs/evtchn build on NetBSD

[Roger Pau Monné]

On Mon, Jan 04, 2021 at 11:26:45AM +0100, Manuel Bouyer wrote:
> On Tue, Dec 29, 2020 at 12:52:43PM +0100, Roger Pau Monné wrote:
> > On Mon, Dec 14, 2020 at 05:36:09PM +0100, Manuel Bouyer wrote:
> > > ---
> > >  tools/libs/evtchn/netbsd.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
> > > index 8b8545d2f9..6d4ce28011 100644
> > > --- a/tools/libs/evtchn/netbsd.c
> > > +++ b/tools/libs/evtchn/netbsd.c
> > > @@ -25,10 +25,10 @@
> > >  
> > >  #include <sys/ioctl.h>
> > >  
> > > -#include <xen/sys/evtchn.h>
> > > -
> > >  #include "private.h"
> > >  
> > > +#include <xen/xenio3.h>
> > > +
> > >  #define EVTCHN_DEV_NAME  "/dev/xenevt"
> > >  
> > >  int osdep_evtchn_open(xenevtchn_handle *xce)
> > > @@ -131,7 +131,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> > >      int fd = xce->fd;
> > >      evtchn_port_t port;
> > >  
> > > -    if ( read_exact(fd, (char *)&port, sizeof(port)) == -1 )
> > > +    if ( read(fd, (char *)&port, sizeof(port)) == -1 )
> > >          return -1;
> > >  
> > >      return port;
> > > @@ -140,7 +140,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> > >  int xenevtchn_unmask(xenevtchn_handle *xce, evtchn_port_t port)
> > >  {
> > >      int fd = xce->fd;
> > > -    return write_exact(fd, (char *)&port, sizeof(port));
> > > +    return write(fd, (char *)&port, sizeof(port));
> > 
> > I'm afraid we will need some context as to why {read/write}_exact
> > doesn't work here.
> 
> It just doesn't exists on NetBSD

But those are not part of libc or any external library, they are
implemented in tools/libs/ctrl/xc_private.c and should be available to
the NetBSD build AFAICT.

They are just helpers build on top of the standard read/write calls.

Roger.

Re: [PATCH 07/24] Remove NetBSD's system headers. We'll use the system-provided ones, which are up to date.

[Manuel Bouyer]

On Mon, Jan 04, 2021 at 06:09:07PM +0100, Roger Pau Monné wrote:
> We usually take a different approach for Linux and FreeBSD in
> order to support all kernels: test if the new ioctl is available, or
> else fallback to the old implementation. But this requires having the
> new header even on old systems in order to have the ioctl definition
> for the build, even if it's not implemented on the currently running
> kernel.
> 
> I guess you would have to use preprocessor conditionals in order to
> keep the build on older versions when adding the new features.

Or just assume that version X of Xen needs version Y of NetBSD.
This is already the case, because of bug fixes.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 12/24] Implement gnttab on NetBSD

[Roger Pau Monné]

On Mon, Jan 04, 2021 at 11:29:51AM +0100, Manuel Bouyer wrote:
> On Tue, Dec 29, 2020 at 12:16:01PM +0100, Roger Pau Monné wrote:
> > Might need some kind of log message, and will also required your
> > Signed-off-by (or from the original author of the patch).
> > 
> > On Mon, Dec 14, 2020 at 05:36:11PM +0100, Manuel Bouyer wrote:
> > > ---
> > >  tools/libs/gnttab/Makefile |   2 +-
> > >  tools/libs/gnttab/netbsd.c | 267 +++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 268 insertions(+), 1 deletion(-)
> > >  create mode 100644 tools/libs/gnttab/netbsd.c
> > > 
> > > diff --git a/tools/libs/gnttab/Makefile b/tools/libs/gnttab/Makefile
> > > index d86c49d243..ae390ce60f 100644
> > > --- a/tools/libs/gnttab/Makefile
> > > +++ b/tools/libs/gnttab/Makefile
> > > @@ -10,7 +10,7 @@ SRCS-GNTSHR            += gntshr_core.c
> > >  SRCS-$(CONFIG_Linux)   += $(SRCS-GNTTAB) $(SRCS-GNTSHR) linux.c
> > >  SRCS-$(CONFIG_MiniOS)  += $(SRCS-GNTTAB) gntshr_unimp.c minios.c
> > >  SRCS-$(CONFIG_FreeBSD) += $(SRCS-GNTTAB) $(SRCS-GNTSHR) freebsd.c
> > > +SRCS-$(CONFIG_NetBSD)  += $(SRCS-GNTTAB) $(SRCS-GNTSHR) netbsd.c
> > >  SRCS-$(CONFIG_SunOS)   += gnttab_unimp.c gntshr_unimp.c
> > > -SRCS-$(CONFIG_NetBSD)  += gnttab_unimp.c gntshr_unimp.c
> > >  
> > >  include $(XEN_ROOT)/tools/libs/libs.mk
> > > diff --git a/tools/libs/gnttab/netbsd.c b/tools/libs/gnttab/netbsd.c
> > > new file mode 100644
> > > index 0000000000..2df7058cd7
> > > --- /dev/null
> > > +++ b/tools/libs/gnttab/netbsd.c
> > 
> > I think this is mostly (if not equal) to the FreeBSD version, in which
> > case we could rename freebsd.c to plain bsd.c and use it for
> > both FreeBSD and NetBSD?
> 
> I can't see why they won't diverge in the future ...

True, but then let's diverge when we have to cross that bridge I would
say.

There's IMO no point in having two verbatim copies of the same code in
different places, it's just more churn to maintain and to remember to
apply duplicate fixes.

Roger.

Re: [PATCH 13/24] Don't assume tv_sec is a unsigned long (for NetBSD)

[Roger Pau Monné]

On Mon, Jan 04, 2021 at 11:31:56AM +0100, Manuel Bouyer wrote:
> On Tue, Dec 29, 2020 at 03:02:15PM +0100, Roger Pau Monné wrote:
> > On Mon, Dec 14, 2020 at 05:36:12PM +0100, Manuel Bouyer wrote:
> > > ---
> > >  tools/libs/light/libxl_create.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> > > index 321a13e519..44691010bc 100644
> > > --- a/tools/libs/light/libxl_create.c
> > > +++ b/tools/libs/light/libxl_create.c
> > > @@ -496,7 +496,7 @@ int libxl__domain_build(libxl__gc *gc,
> > >          vments[2] = "image/ostype";
> > >          vments[3] = "hvm";
> > >          vments[4] = "start_time";
> > > -        vments[5] = GCSPRINTF("%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
> > > +        vments[5] = GCSPRINTF("%jd.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
> > 
> > You don't cast tv_sec to intmax_t here...
> > 
> > >  
> > >          localents = libxl__calloc(gc, 13, sizeof(char *));
> > >          i = 0;
> > > @@ -535,7 +535,7 @@ int libxl__domain_build(libxl__gc *gc,
> > >          vments[i++] = "image/kernel";
> > >          vments[i++] = (char *) state->pv_kernel.path;
> > >          vments[i++] = "start_time";
> > > -        vments[i++] = GCSPRINTF("%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
> > > +        vments[i++] = GCSPRINTF("%jd.%02d", (intmax_t)start_time.tv_sec,(int)start_time.tv_usec/10000);
> > 
> > ... yet you do it here. I think the first occurrence is missing the
> > cast?
> 
> thanks, fixed.

Add my:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Then.

Thanks, Roger.

Re: [PATCH 11/24] Implement foreignmemory on NetBSD

[Roger Pau Monné]

On Mon, Jan 04, 2021 at 12:30:44PM +0100, Manuel Bouyer wrote:
> On Tue, Dec 29, 2020 at 01:46:30PM +0100, Roger Pau Monné wrote:
> > Also a little bit below these prototypes are the dummy implementations
> > of osdep_xenforeignmemory_restrict,
> > osdep_xenforeignmemory_map_resource and
> > osdep_xenforeignmemory_unmap_resource. I think you at least need to
> > modify the condition below so that on NetBSD the dummy inlines are not
> > used?
> 
> This is !defined(__NetBSD__) so it should not be used ?

Right, I think I somehow overlooked that part of the patch. You are
indeed tweaking the define so it should be fine.

Thanks, Roger.

Re: [PATCH 10/24] Make libs/evtchn build on NetBSD

[Manuel Bouyer]

On Mon, Jan 04, 2021 at 06:15:24PM +0100, Roger Pau Monné wrote:
> On Mon, Jan 04, 2021 at 11:26:45AM +0100, Manuel Bouyer wrote:
> > On Tue, Dec 29, 2020 at 12:52:43PM +0100, Roger Pau Monné wrote:
> > > On Mon, Dec 14, 2020 at 05:36:09PM +0100, Manuel Bouyer wrote:
> > > > ---
> > > >  tools/libs/evtchn/netbsd.c | 8 ++++----
> > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
> > > > index 8b8545d2f9..6d4ce28011 100644
> > > > --- a/tools/libs/evtchn/netbsd.c
> > > > +++ b/tools/libs/evtchn/netbsd.c
> > > > @@ -25,10 +25,10 @@
> > > >  
> > > >  #include <sys/ioctl.h>
> > > >  
> > > > -#include <xen/sys/evtchn.h>
> > > > -
> > > >  #include "private.h"
> > > >  
> > > > +#include <xen/xenio3.h>
> > > > +
> > > >  #define EVTCHN_DEV_NAME  "/dev/xenevt"
> > > >  
> > > >  int osdep_evtchn_open(xenevtchn_handle *xce)
> > > > @@ -131,7 +131,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> > > >      int fd = xce->fd;
> > > >      evtchn_port_t port;
> > > >  
> > > > -    if ( read_exact(fd, (char *)&port, sizeof(port)) == -1 )
> > > > +    if ( read(fd, (char *)&port, sizeof(port)) == -1 )
> > > >          return -1;
> > > >  
> > > >      return port;
> > > > @@ -140,7 +140,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> > > >  int xenevtchn_unmask(xenevtchn_handle *xce, evtchn_port_t port)
> > > >  {
> > > >      int fd = xce->fd;
> > > > -    return write_exact(fd, (char *)&port, sizeof(port));
> > > > +    return write(fd, (char *)&port, sizeof(port));
> > > 
> > > I'm afraid we will need some context as to why {read/write}_exact
> > > doesn't work here.
> > 
> > It just doesn't exists on NetBSD
> 
> But those are not part of libc or any external library, they are
> implemented in tools/libs/ctrl/xc_private.c and should be available to
> the NetBSD build AFAICT.
> 
> They are just helpers build on top of the standard read/write calls.

Yes, I misremembered (I have this patch for a long time, since 4.11 at last,
maybe even older).
Anyway the build fails with:
netbsd.c: In function 'xenevtchn_pending':
netbsd.c:134:10: error: implicit declaration of function 'read_exact'; did you mean 'readlinkat'? [-Werror=implicit-function-declaration]

The only header where I see this function defined is
tools/libs/ctrl/xc_private.h, so I would need something like
#include "../../ctrl/xc_private.h"
but this doesn't look right.

I didn't find where other OSes are getting the prototype from (or maybe
they just have this -Werror turned off ?)

Anyway I think NetBSD doesn't need this read_exact/write_exact thing,
the underlying pseudo-device won't to partial read/write.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 12/24] Implement gnttab on NetBSD

[Manuel Bouyer]

On Mon, Jan 04, 2021 at 06:24:11PM +0100, Roger Pau Monné wrote:
> On Mon, Jan 04, 2021 at 11:29:51AM +0100, Manuel Bouyer wrote:
> > On Tue, Dec 29, 2020 at 12:16:01PM +0100, Roger Pau Monné wrote:
> > > Might need some kind of log message, and will also required your
> > > Signed-off-by (or from the original author of the patch).
> > > 
> > > On Mon, Dec 14, 2020 at 05:36:11PM +0100, Manuel Bouyer wrote:
> > > > ---
> > > >  tools/libs/gnttab/Makefile |   2 +-
> > > >  tools/libs/gnttab/netbsd.c | 267 +++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 268 insertions(+), 1 deletion(-)
> > > >  create mode 100644 tools/libs/gnttab/netbsd.c
> > > > 
> > > > diff --git a/tools/libs/gnttab/Makefile b/tools/libs/gnttab/Makefile
> > > > index d86c49d243..ae390ce60f 100644
> > > > --- a/tools/libs/gnttab/Makefile
> > > > +++ b/tools/libs/gnttab/Makefile
> > > > @@ -10,7 +10,7 @@ SRCS-GNTSHR            += gntshr_core.c
> > > >  SRCS-$(CONFIG_Linux)   += $(SRCS-GNTTAB) $(SRCS-GNTSHR) linux.c
> > > >  SRCS-$(CONFIG_MiniOS)  += $(SRCS-GNTTAB) gntshr_unimp.c minios.c
> > > >  SRCS-$(CONFIG_FreeBSD) += $(SRCS-GNTTAB) $(SRCS-GNTSHR) freebsd.c
> > > > +SRCS-$(CONFIG_NetBSD)  += $(SRCS-GNTTAB) $(SRCS-GNTSHR) netbsd.c
> > > >  SRCS-$(CONFIG_SunOS)   += gnttab_unimp.c gntshr_unimp.c
> > > > -SRCS-$(CONFIG_NetBSD)  += gnttab_unimp.c gntshr_unimp.c
> > > >  
> > > >  include $(XEN_ROOT)/tools/libs/libs.mk
> > > > diff --git a/tools/libs/gnttab/netbsd.c b/tools/libs/gnttab/netbsd.c
> > > > new file mode 100644
> > > > index 0000000000..2df7058cd7
> > > > --- /dev/null
> > > > +++ b/tools/libs/gnttab/netbsd.c
> > > 
> > > I think this is mostly (if not equal) to the FreeBSD version, in which
> > > case we could rename freebsd.c to plain bsd.c and use it for
> > > both FreeBSD and NetBSD?
> > 
> > I can't see why they won't diverge in the future ...
> 
> True, but then let's diverge when we have to cross that bridge I would
> say.
> 
> There's IMO no point in having two verbatim copies of the same code in
> different places, it's just more churn to maintain and to remember to
> apply duplicate fixes.

Actually I just checked, the files are quite different, because the
GNTTAB ioctls are not the same, and it seems they don't work the same way
either. FreeBSD does mmap against the gnttab device; this is not supported
on NetBSD. Merging the two would cause an #ifdef maze.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 12/24] Implement gnttab on NetBSD

[Roger Pau Monné]

On Sun, Jan 10, 2021 at 01:40:50PM +0100, Manuel Bouyer wrote:
> On Mon, Jan 04, 2021 at 06:24:11PM +0100, Roger Pau Monné wrote:
> > On Mon, Jan 04, 2021 at 11:29:51AM +0100, Manuel Bouyer wrote:
> > > On Tue, Dec 29, 2020 at 12:16:01PM +0100, Roger Pau Monné wrote:
> > > > Might need some kind of log message, and will also required your
> > > > Signed-off-by (or from the original author of the patch).
> > > > 
> > > > On Mon, Dec 14, 2020 at 05:36:11PM +0100, Manuel Bouyer wrote:
> > > > > ---
> > > > >  tools/libs/gnttab/Makefile |   2 +-
> > > > >  tools/libs/gnttab/netbsd.c | 267 +++++++++++++++++++++++++++++++++++++
> > > > >  2 files changed, 268 insertions(+), 1 deletion(-)
> > > > >  create mode 100644 tools/libs/gnttab/netbsd.c
> > > > > 
> > > > > diff --git a/tools/libs/gnttab/Makefile b/tools/libs/gnttab/Makefile
> > > > > index d86c49d243..ae390ce60f 100644
> > > > > --- a/tools/libs/gnttab/Makefile
> > > > > +++ b/tools/libs/gnttab/Makefile
> > > > > @@ -10,7 +10,7 @@ SRCS-GNTSHR            += gntshr_core.c
> > > > >  SRCS-$(CONFIG_Linux)   += $(SRCS-GNTTAB) $(SRCS-GNTSHR) linux.c
> > > > >  SRCS-$(CONFIG_MiniOS)  += $(SRCS-GNTTAB) gntshr_unimp.c minios.c
> > > > >  SRCS-$(CONFIG_FreeBSD) += $(SRCS-GNTTAB) $(SRCS-GNTSHR) freebsd.c
> > > > > +SRCS-$(CONFIG_NetBSD)  += $(SRCS-GNTTAB) $(SRCS-GNTSHR) netbsd.c
> > > > >  SRCS-$(CONFIG_SunOS)   += gnttab_unimp.c gntshr_unimp.c
> > > > > -SRCS-$(CONFIG_NetBSD)  += gnttab_unimp.c gntshr_unimp.c
> > > > >  
> > > > >  include $(XEN_ROOT)/tools/libs/libs.mk
> > > > > diff --git a/tools/libs/gnttab/netbsd.c b/tools/libs/gnttab/netbsd.c
> > > > > new file mode 100644
> > > > > index 0000000000..2df7058cd7
> > > > > --- /dev/null
> > > > > +++ b/tools/libs/gnttab/netbsd.c
> > > > 
> > > > I think this is mostly (if not equal) to the FreeBSD version, in which
> > > > case we could rename freebsd.c to plain bsd.c and use it for
> > > > both FreeBSD and NetBSD?
> > > 
> > > I can't see why they won't diverge in the future ...
> > 
> > True, but then let's diverge when we have to cross that bridge I would
> > say.
> > 
> > There's IMO no point in having two verbatim copies of the same code in
> > different places, it's just more churn to maintain and to remember to
> > apply duplicate fixes.
> 
> Actually I just checked, the files are quite different, because the
> GNTTAB ioctls are not the same, and it seems they don't work the same way
> either. FreeBSD does mmap against the gnttab device; this is not supported
> on NetBSD. Merging the two would cause an #ifdef maze.

Ack, my bad. Then it's fine. I got to think they where the same by the
copyright message.

Thanks, Roger.

Re: [PATCH 10/24] Make libs/evtchn build on NetBSD

[Roger Pau Monné]

On Sun, Jan 10, 2021 at 01:22:50PM +0100, Manuel Bouyer wrote:
> On Mon, Jan 04, 2021 at 06:15:24PM +0100, Roger Pau Monné wrote:
> > On Mon, Jan 04, 2021 at 11:26:45AM +0100, Manuel Bouyer wrote:
> > > On Tue, Dec 29, 2020 at 12:52:43PM +0100, Roger Pau Monné wrote:
> > > > On Mon, Dec 14, 2020 at 05:36:09PM +0100, Manuel Bouyer wrote:
> > > > > ---
> > > > >  tools/libs/evtchn/netbsd.c | 8 ++++----
> > > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
> > > > > index 8b8545d2f9..6d4ce28011 100644
> > > > > --- a/tools/libs/evtchn/netbsd.c
> > > > > +++ b/tools/libs/evtchn/netbsd.c
> > > > > @@ -25,10 +25,10 @@
> > > > >  
> > > > >  #include <sys/ioctl.h>
> > > > >  
> > > > > -#include <xen/sys/evtchn.h>
> > > > > -
> > > > >  #include "private.h"
> > > > >  
> > > > > +#include <xen/xenio3.h>
> > > > > +
> > > > >  #define EVTCHN_DEV_NAME  "/dev/xenevt"
> > > > >  
> > > > >  int osdep_evtchn_open(xenevtchn_handle *xce)
> > > > > @@ -131,7 +131,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> > > > >      int fd = xce->fd;
> > > > >      evtchn_port_t port;
> > > > >  
> > > > > -    if ( read_exact(fd, (char *)&port, sizeof(port)) == -1 )
> > > > > +    if ( read(fd, (char *)&port, sizeof(port)) == -1 )
> > > > >          return -1;
> > > > >  
> > > > >      return port;
> > > > > @@ -140,7 +140,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> > > > >  int xenevtchn_unmask(xenevtchn_handle *xce, evtchn_port_t port)
> > > > >  {
> > > > >      int fd = xce->fd;
> > > > > -    return write_exact(fd, (char *)&port, sizeof(port));
> > > > > +    return write(fd, (char *)&port, sizeof(port));
> > > > 
> > > > I'm afraid we will need some context as to why {read/write}_exact
> > > > doesn't work here.
> > > 
> > > It just doesn't exists on NetBSD
> > 
> > But those are not part of libc or any external library, they are
> > implemented in tools/libs/ctrl/xc_private.c and should be available to
> > the NetBSD build AFAICT.
> > 
> > They are just helpers build on top of the standard read/write calls.
> 
> Yes, I misremembered (I have this patch for a long time, since 4.11 at last,
> maybe even older).
> Anyway the build fails with:
> netbsd.c: In function 'xenevtchn_pending':
> netbsd.c:134:10: error: implicit declaration of function 'read_exact'; did you mean 'readlinkat'? [-Werror=implicit-function-declaration]
> 
> The only header where I see this function defined is
> tools/libs/ctrl/xc_private.h, so I would need something like
> #include "../../ctrl/xc_private.h"
> but this doesn't look right.
> 
> I didn't find where other OSes are getting the prototype from (or maybe
> they just have this -Werror turned off ?)
> 
> Anyway I think NetBSD doesn't need this read_exact/write_exact thing,
> the underlying pseudo-device won't to partial read/write.

The usage of {read/write}_exact there is indeed a mistake, when the
evtchn library was split from libxc no one realized that the
{read/write}_exact where no longer available to that code.

Could you please add:

Fixes: b7f76a699dc ('tools: Refactor /dev/xen/evtchn wrappers into libxenevtchn.')

To the commit message?

And also:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD

[Ian Jackson]

Manuel Bouyer writes ("Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD"):
> On Tue, Dec 29, 2020 at 12:29:09PM +0100, Roger Pau Monné wrote:
> > I think you want tot CC the tools dev on this one, specially Ian who
> > knows how the Linux one is implemented and can likely give valuable
> > input.
...
> > Seeing the file itself, I don't think there's any NetBSD specific
> > stuff, so we might want to consider putting it in BSD/ instead, so it
> > can be used by FreeBSD also?
> 
> I'm not sure if FreeBSD needs the locking stuff.
> Also, there are certainly differences in block device handling between
> FreeBSD and NetBSD. Both OSes have diverged in this area.

I think most operating systems will want some kind of locking here.

I loooked at the code in the new tools/hotplug/NetBSD/locking.sh.
Unfortunately this area is complex and the available APIs and tools
are awkard, and the field is troubled by broken "traditional"
approaches involving O_EXCL or the moral equivalent, which cannot be
made reliable (if you think reliability implies never being broken due
to stale lock).

I doubt that the code in this patch is correct.  It uses shlock(1)
which is based on link(2) and kill(2) and so on, which I think is
basically an O_EXCL-based approach as I discuss above.  (I don't have
a formal proof of this contention.)  The presence of an invocation of
the "trap" shell builtin in the new NetBSD script is a bad sign - a
reliable locking protocol would need that.

I see from https://man.netbsd.org that NetBSD has flock(1) and
stat(1).  I think this means we could reuse the code in
tools/hotplug/Linux/locking.sh.  Maybe it will need to be lightly
adapted, to NetBSD's flock(1) and stat(1).  Perhaps via some kind of
substitution to avoid all the clone-and-hack.

Regards,
Ian.

Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD

[Manuel Bouyer]

On Wed, Jan 20, 2021 at 03:13:22PM +0000, Ian Jackson wrote:
> Manuel Bouyer writes ("Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD"):
> > On Tue, Dec 29, 2020 at 12:29:09PM +0100, Roger Pau Monné wrote:
> > > I think you want tot CC the tools dev on this one, specially Ian who
> > > knows how the Linux one is implemented and can likely give valuable
> > > input.
> ...
> > > Seeing the file itself, I don't think there's any NetBSD specific
> > > stuff, so we might want to consider putting it in BSD/ instead, so it
> > > can be used by FreeBSD also?
> > 
> > I'm not sure if FreeBSD needs the locking stuff.
> > Also, there are certainly differences in block device handling between
> > FreeBSD and NetBSD. Both OSes have diverged in this area.
> 
> I think most operating systems will want some kind of locking here.
> 
> I loooked at the code in the new tools/hotplug/NetBSD/locking.sh.
> Unfortunately this area is complex and the available APIs and tools
> are awkard, and the field is troubled by broken "traditional"
> approaches involving O_EXCL or the moral equivalent, which cannot be
> made reliable (if you think reliability implies never being broken due
> to stale lock).
> 
> I doubt that the code in this patch is correct.  It uses shlock(1)
> which is based on link(2) and kill(2) and so on, which I think is
> basically an O_EXCL-based approach as I discuss above.  (I don't have
> a formal proof of this contention.)  The presence of an invocation of
> the "trap" shell builtin in the new NetBSD script is a bad sign - a
> reliable locking protocol would need that.

Actually this patch is old - since Xen 4.8 at last.

> 
> I see from https://man.netbsd.org that NetBSD has flock(1) and
> stat(1).  I think this means we could reuse the code in
> tools/hotplug/Linux/locking.sh.  Maybe it will need to be lightly
> adapted, to NetBSD's flock(1) and stat(1).  Perhaps via some kind of
> substitution to avoid all the clone-and-hack.

Yes, at last the stat call will need to be patched.
But it seems to rely on a linux-specific behavoir, which is that
/dev/stdin points to the real file on redirection:
>ls -l /dev/stdin /proc/self/fd/0 < /etc/passwd
lrwxrwxrwx 1 root   root      15 Apr 30  2019 /dev/stdin -> /proc/self/fd/0
lr-x------ 1 bouyer ita-iatos 64 Jan 20 16:54 /proc/self/fd/0 -> /etc/passwd

On NetBSD (and I guess other BSDs) this won't work, as /dev/stdin is a
specific device:
>ls -l /dev/stdin 
crw-rw-rw-  1 root  wheel  22, 0 Nov 15  2007 /dev/stdin

so stat -L will always return the same data. We can't use the same protocol.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD

[Ian Jackson]

Manuel Bouyer writes ("Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD"):
> Yes, at last the stat call will need to be patched.
> But it seems to rely on a linux-specific behavoir, which is that
> /dev/stdin points to the real file on redirection:
> >ls -l /dev/stdin /proc/self/fd/0 < /etc/passwd
> lrwxrwxrwx 1 root   root      15 Apr 30  2019 /dev/stdin -> /proc/self/fd/0
> lr-x------ 1 bouyer ita-iatos 64 Jan 20 16:54 /proc/self/fd/0 -> /etc/passwd
> 
> On NetBSD (and I guess other BSDs) this won't work, as /dev/stdin is a
> specific device:
> >ls -l /dev/stdin 
> crw-rw-rw-  1 root  wheel  22, 0 Nov 15  2007 /dev/stdin
> 
> so stat -L will always return the same data. We can't use the same
> protocol.

Ah.

The manpage I am looking at says:

     If no argument is given, stat displays information about the
     file descriptor for standard input.

Here NetBSD has a better command line API than Linux - Linux requires
pratting about with /dev/stdin and NetBSD doesn't.  So I think
where on Linux we have
   stat .... /dev/stdin
on NetBsd we can simply have
   stat ....
with no filename argument.

I think NetBSD's stat(1) also takes different argumnts to specify the
format.  NetBSD uses -f, whereas Linux uses -c.  So the exact rune
will have to be different.

Ian.

Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD

[Manuel Bouyer]

On Wed, Jan 20, 2021 at 04:12:29PM +0000, Ian Jackson wrote:
> Manuel Bouyer writes ("Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD"):
> > Yes, at last the stat call will need to be patched.
> > But it seems to rely on a linux-specific behavoir, which is that
> > /dev/stdin points to the real file on redirection:
> > >ls -l /dev/stdin /proc/self/fd/0 < /etc/passwd
> > lrwxrwxrwx 1 root   root      15 Apr 30  2019 /dev/stdin -> /proc/self/fd/0
> > lr-x------ 1 bouyer ita-iatos 64 Jan 20 16:54 /proc/self/fd/0 -> /etc/passwd
> > 
> > On NetBSD (and I guess other BSDs) this won't work, as /dev/stdin is a
> > specific device:
> > >ls -l /dev/stdin 
> > crw-rw-rw-  1 root  wheel  22, 0 Nov 15  2007 /dev/stdin
> > 
> > so stat -L will always return the same data. We can't use the same
> > protocol.
> 
> Ah.
> 
> The manpage I am looking at says:
> 
>      If no argument is given, stat displays information about the
>      file descriptor for standard input.
> 
> Here NetBSD has a better command line API than Linux - Linux requires
> pratting about with /dev/stdin and NetBSD doesn't.  So I think
> where on Linux we have
>    stat .... /dev/stdin
> on NetBsd we can simply have
>    stat ....
> with no filename argument.

Right, thanks. Then it would need to be done with 2 different calls
but I don't think that's a problem (even with the linux version it would
not be atomic anyway).

> 
> I think NetBSD's stat(1) also takes different argumnts to specify the
> format.  NetBSD uses -f, whereas Linux uses -c.  So the exact rune
> will have to be different.

Yes, and NetBSD doens't have %D (only %d)

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD

[Ian Jackson]

Manuel Bouyer writes ("Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD"):
> On Wed, Jan 20, 2021 at 04:12:29PM +0000, Ian Jackson wrote:
> > I think NetBSD's stat(1) also takes different argumnts to specify the
> > format.  NetBSD uses -f, whereas Linux uses -c.  So the exact rune
> > will have to be different.
> 
> Yes, and NetBSD doens't have %D (only %d)

What's really important here is the inode number.  But %d will do
nicely.  I only used %D on Linux since device numbers are two-element
bitfields so displaying them in hex makes more sense to humans who
might be debugging this.

Ian.

Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD

[Ian Jackson]

Manuel Bouyer writes ("Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD"):
> Right, thanks. Then it would need to be done with 2 different calls
> but I don't think that's a problem (even with the linux version it would
> not be atomic anyway).

Sorry, I forgot to reply to this.  Yes, it would need to invocations
of stat(1).  You are correct that the Linux version is not atomic in
that sense.

Ian.