[PATCH 0/6] Use system blktap

[PATCH 0/6] Use system blktap

[George Dunlap]

Switch to using a blktap provided by the system, rather than building
it in-tree.

Do this by adding a block-tap script which calls the tap-ctl binary,
rather than linking against a library.

This requires the use of an externally-built tap-ctl binary, which an
be built from the master branch of the XenServer blktap tree at
https://github.com/xapi-project/blktap.git

The first two patches and the final patch are general libxl disk
handling improvements which can be applied independently of the core.

The third patch (tools: Add a block-tap script for setting up tapdisks
via tap-ctl) can also be checked in independently from the fourth and
fifth, as it allows someone to disable blktap support in configure but
still use externally-built tap-ctl binaries (albeit with a different
disk string).

The fifth patch is a straight rm -rf of toosl/blktap2.  Rather than
spam everyone's mailboxes I've clipped the message here, and provided
a tree which can be pulled from:

git://xenbits.xen.org/people/gdunlap/xen.git out/blktap-system/v1

This series also exposes blktap to a bug that prevents HVM guests from
using disks created with block scripts.  This should be a blocker for
the 4.6 release, so I think we should consider checking this series in
as is, assuming that bug will be addressed before the release.

George Dunlap (6): 
  libxl: Make local_initiate_attach more rational
  libxl: Remove linux udev rules
  tools: Add a block-tap script for setting up tapdisks via tap-ctl
  libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP
  tools: Remove in-tree blktap2
  libxl: Add more logging to hotplug script path

 tools/Makefile                               |    1 -
 [tools/blktap2/* removal trimmed]
 tools/config.h.in                            |    3 -
 tools/configure                              |   29 +-
 tools/configure.ac                           |   13 +-
 tools/examples/README                        |    1 -
 tools/hotplug/Linux/Makefile                 |   15 +-
 tools/hotplug/Linux/block-tap                |  123 +
 tools/hotplug/Linux/xen-backend.rules.in     |   13 -
 tools/hotplug/Linux/xen-hotplug-common.sh.in |    7 -
 tools/libxl/Makefile                         |    5 -
 tools/libxl/libxl.c                          |  139 +-
 tools/libxl/libxl_blktap2.c                  |   93 -
 tools/libxl/libxl_create.c                   |   27 -
 tools/libxl/libxl_device.c                   |   34 +-
 tools/libxl/libxl_dm.c                       |    5 +-
 tools/libxl/libxl_internal.c                 |   19 -
 tools/libxl/libxl_internal.h                 |   27 -
 tools/libxl/libxl_linux.c                    |   12 +-
 tools/libxl/libxl_netbsd.c                   |    7 -
 tools/libxl/libxl_noblktap2.c                |   42 -
 146 files changed, 194 insertions(+), 40550 deletions(-)
 [delete mode 100644 tools/blktap2/* trimmed]

-- 
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>
CC: Felipe Franciosi <felipe.franciosi@citrix.com>
CC: Dave Scott <Dave.Scott@eu.citrix.com>
CC: Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>

1.9.1

[PATCH 1/6] libxl: Make local_initiate_attach more rational

[George Dunlap]

There are a lot of paths through
libxl__device_disk_local_initiate_attach(), but they all really boil
down to one thing: Can we just access the file directly, or do we need
to attach it?

The requirements for direct access are fairly simple:
* Is this local (as opposed to a driver domain)?
* Is this a raw format (as opposed to cooked)?
* Does this have no scripts associated with it?

If it meets all those requirements, we can access it directly;
otherwise we need to attach it.

This fixes a bug where bootloader execution fails for disks with
hotplug scripts.

This should fix a theoretical bug when using a qdisk backend in a
driver domain. (Not tested.)

Based on a patch by Roger Pau Monne <roger.pau@citrix.com>.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>

This is identical to the patch I sent earlier, with the exception that
I have fixed a build by removing the (now) unused ctx variable.  Given
the circumstances I have taken the liberty to retain the Acks above.
---
 tools/libxl/libxl.c | 98 ++++++++++++++---------------------------------------
 1 file changed, 26 insertions(+), 72 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e9a2d26..92563db 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3058,7 +3058,6 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
                                      libxl__disk_local_state *dls)
 {
     STATE_AO_GC(dls->ao);
-    libxl_ctx *ctx = CTX;
     char *dev = NULL;
     int rc;
     const libxl_device_disk *in_disk = dls->in_disk;
@@ -3076,55 +3075,21 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
     rc = libxl__device_disk_setdefault(gc, disk);
     if (rc) goto out;
 
-    switch (disk->backend) {
-        case LIBXL_DISK_BACKEND_PHY:
-            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching PHY disk %s",
-                       disk->pdev_path);
-            dev = disk->pdev_path;
-            break;
-        case LIBXL_DISK_BACKEND_TAP:
-            switch (disk->format) {
-            case LIBXL_DISK_FORMAT_RAW:
-                /* optimise away the early tapdisk attach in this case */
-                LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching"
-                           " tap disk %s directly (ie without using blktap)",
-                           disk->pdev_path);
-                dev = disk->pdev_path;
-                break;
-            case LIBXL_DISK_FORMAT_VHD:
-                dev = libxl__blktap_devpath(gc, disk->pdev_path,
-                                            disk->format);
-                break;
-            case LIBXL_DISK_FORMAT_QCOW:
-            case LIBXL_DISK_FORMAT_QCOW2:
-                abort(); /* prevented by libxl__device_disk_set_backend */
-            default:
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                           "unrecognized disk format: %d", disk->format);
-                rc = ERROR_FAIL;
-                goto out;
-            }
-            break;
-        case LIBXL_DISK_BACKEND_QDISK:
-            if (disk->format != LIBXL_DISK_FORMAT_RAW) {
-                libxl__prepare_ao_device(ao, &dls->aodev);
-                dls->aodev.callback = local_device_attach_cb;
-                device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk,
-                                &dls->aodev, libxl__alloc_vdev,
-                                (void *) blkdev_start);
-                return;
-            } else {
-                dev = disk->pdev_path;
-            }
-            LOG(DEBUG, "locally attaching qdisk %s", dev);
-            break;
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
-                "type: %d", disk->backend);
-            rc = ERROR_FAIL;
-            goto out;
+    /* If this is in a driver domain, or it's not a raw format, or it involves
+     * running a script, we have to do a local attach. */
+    if (disk->backend_domname != NULL
+        || disk->format != LIBXL_DISK_FORMAT_RAW
+        || disk->script != NULL) {
+        libxl__prepare_ao_device(ao, &dls->aodev);
+        dls->aodev.callback = local_device_attach_cb;
+        device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk, &dls->aodev,
+                        libxl__alloc_vdev, (void *) blkdev_start);
+        return;
     }
 
+    LOG(DEBUG, "locally attaching RAW disk %s", disk->pdev_path);
+    dev = disk->pdev_path;
+
     if (dev != NULL)
         dls->diskpath = libxl__strdup(gc, dev);
 
@@ -3157,7 +3122,7 @@ static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev)
     }
 
     dev = GCSPRINTF("/dev/%s", disk->vdev);
-    LOG(DEBUG, "locally attaching qdisk %s", dev);
+    LOG(DEBUG, "locally attaching disk %s", dev);
 
     rc = libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID, disk, &device);
     if (rc < 0)
@@ -3196,29 +3161,18 @@ void libxl__device_disk_local_initiate_detach(libxl__egc *egc,
 
     if (!dls->diskpath) goto out;
 
-    switch (disk->backend) {
-        case LIBXL_DISK_BACKEND_QDISK:
-            if (disk->vdev != NULL) {
-                GCNEW(device);
-                rc = libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID,
-                                             disk, device);
-                if (rc != 0) goto out;
-
-                aodev->action = LIBXL__DEVICE_ACTION_REMOVE;
-                aodev->dev = device;
-                aodev->callback = local_device_detach_cb;
-                aodev->force = 0;
-                libxl__initiate_device_remove(egc, aodev);
-                return;
-            }
-            /* disk->vdev == NULL; fall through */
-        default:
-            /*
-             * Nothing to do for PHYSTYPE_PHY.
-             * For other device types assume that the blktap2 process is
-             * needed by the soon to be started domain and do nothing.
-             */
-            goto out;
+    if (disk->vdev != NULL) {
+        GCNEW(device);
+        rc = libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID,
+                                     disk, device);
+        if (rc != 0) goto out;
+        
+        aodev->action = LIBXL__DEVICE_ACTION_REMOVE;
+        aodev->dev = device;
+        aodev->callback = local_device_detach_cb;
+        aodev->force = 0;
+        libxl__initiate_device_remove(egc, aodev);
+        return;
     }
 
 out:
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 2/6] libxl: Remove linux udev rules

[George Dunlap]

They are no longer needed, having been replaced by a daemon for
driverdomains which will run scripts as necessary.

Worse yet, they seem to be broken for script-based block devices, such
as block-iscsi.  This wouldn't matter so much if they were never run
by default; but if you run block-attach without having created a
domain, then the appropriate node to disable running udev scripts will
not have been written yet, and the attach will silently fail.

Rather than try to sort out that issue, just remove them entirely.

Note: This changes tools/configure.ac, so autogen.sh may need to be
re-run.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>

There was some concern that old udev scripts may end up lying around
in /etc which will still trigger and run unless libxl/disable_udev
exists.

If that's a concern, then for one release we might consider writing
libxl/disable_udev manually in xencommons or something to make sure
that they don't trigger.
---
 tools/configure                              | 13 ++++++-------
 tools/configure.ac                           |  1 -
 tools/examples/README                        |  1 -
 tools/hotplug/Linux/Makefile                 | 14 +-------------
 tools/hotplug/Linux/xen-backend.rules.in     | 13 -------------
 tools/hotplug/Linux/xen-hotplug-common.sh.in |  7 -------
 tools/libxl/libxl.c                          | 13 -------------
 tools/libxl/libxl_create.c                   | 27 ---------------------------
 tools/libxl/libxl_internal.c                 | 19 -------------------
 tools/libxl/libxl_internal.h                 |  1 -
 tools/libxl/libxl_linux.c                    |  7 -------
 tools/libxl/libxl_netbsd.c                   |  7 -------
 12 files changed, 7 insertions(+), 116 deletions(-)

diff --git a/tools/configure b/tools/configure
index c940dd1..5138f3d 100755
--- a/tools/configure
+++ b/tools/configure
@@ -2403,7 +2403,7 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
 
 
 
-ac_config_files="$ac_config_files ../config/Tools.mk hotplug/FreeBSD/rc.d/xencommons hotplug/Linux/init.d/sysconfig.xencommons hotplug/Linux/init.d/xen-watchdog hotplug/Linux/init.d/xencommons hotplug/Linux/init.d/xendomains hotplug/Linux/vif-setup hotplug/Linux/xen-backend.rules hotplug/Linux/xen-hotplug-common.sh hotplug/Linux/xendomains hotplug/NetBSD/rc.d/xencommons libxl/xenlight.pc.in libxl/xlutil.pc.in"
+ac_config_files="$ac_config_files ../config/Tools.mk hotplug/FreeBSD/rc.d/xencommons hotplug/Linux/init.d/sysconfig.xencommons hotplug/Linux/init.d/xen-watchdog hotplug/Linux/init.d/xencommons hotplug/Linux/init.d/xendomains hotplug/Linux/vif-setup hotplug/Linux/xen-hotplug-common.sh hotplug/Linux/xendomains hotplug/NetBSD/rc.d/xencommons libxl/xenlight.pc.in libxl/xlutil.pc.in"
 
 ac_config_headers="$ac_config_headers config.h"
 
@@ -3343,7 +3343,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -3389,7 +3389,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -3413,7 +3413,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -3458,7 +3458,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -3482,7 +3482,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -10050,7 +10050,6 @@ do
     "hotplug/Linux/init.d/xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xencommons" ;;
     "hotplug/Linux/init.d/xendomains") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xendomains" ;;
     "hotplug/Linux/vif-setup") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/vif-setup" ;;
-    "hotplug/Linux/xen-backend.rules") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/xen-backend.rules" ;;
     "hotplug/Linux/xen-hotplug-common.sh") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/xen-hotplug-common.sh" ;;
     "hotplug/Linux/xendomains") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/xendomains" ;;
     "hotplug/NetBSD/rc.d/xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/NetBSD/rc.d/xencommons" ;;
diff --git a/tools/configure.ac b/tools/configure.ac
index 2d371f4..8bfdfcb 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -13,7 +13,6 @@ hotplug/Linux/init.d/xen-watchdog
 hotplug/Linux/init.d/xencommons
 hotplug/Linux/init.d/xendomains
 hotplug/Linux/vif-setup
-hotplug/Linux/xen-backend.rules
 hotplug/Linux/xen-hotplug-common.sh
 hotplug/Linux/xendomains
 hotplug/NetBSD/rc.d/xencommons
diff --git a/tools/examples/README b/tools/examples/README
index 115ca02..13380a4 100644
--- a/tools/examples/README
+++ b/tools/examples/README
@@ -24,7 +24,6 @@ vif-nat             - xen virtual network start/stop script in NAT mode
 vif-route           - xen virtual network start/stop script in routed mode
 xen-backend.agent   - calls block, vif-* scripts to add, remove, hotplug
                       devices  
-xen-backend.rules   - hotplug script rules
 xen-hotplug-common.sh - sourced by vif-common.sh
 xen-network-common.sh - sourced by vif-common.sh
 xen-script-common.sh  - sourced by xen-hotplug-common.sh
diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
index d94a9cb..8bb2316 100644
--- a/tools/hotplug/Linux/Makefile
+++ b/tools/hotplug/Linux/Makefile
@@ -32,9 +32,6 @@ XEN_SCRIPT_DATA = xen-script-common.sh locking.sh logging.sh
 XEN_SCRIPT_DATA += xen-hotplug-common.sh xen-network-common.sh vif-common.sh
 XEN_SCRIPT_DATA += block-common.sh
 
-UDEV_RULES_DIR = $(CONFIG_DIR)/udev
-UDEV_RULES = xen-backend.rules $(UDEV_RULES-y)
-
 .PHONY: all
 all: subdirs-all
 
@@ -42,7 +39,7 @@ all: subdirs-all
 build:
 
 .PHONY: install
-install: install-initd install-scripts install-udev subdirs-install
+install: install-initd install-scripts subdirs-install
 
 # See docs/misc/distro_mapping.txt for INITD_DIR location
 .PHONY: install-initd
@@ -70,15 +67,6 @@ install-scripts:
 	    $(INSTALL_DATA) $$i $(DESTDIR)$(XEN_SCRIPT_DIR); \
 	done
 
-.PHONY: install-udev
-install-udev:
-	[ -d $(DESTDIR)$(UDEV_RULES_DIR) ] || \
-		$(INSTALL_DIR) $(DESTDIR)$(UDEV_RULES_DIR)/rules.d
-	set -e; for i in $(UDEV_RULES); \
-	    do \
-	    $(INSTALL_DATA) $$i $(DESTDIR)$(UDEV_RULES_DIR)/rules.d; \
-	done
-
 .PHONY: clean
 clean: subdirs-clean
 
diff --git a/tools/hotplug/Linux/xen-backend.rules.in b/tools/hotplug/Linux/xen-backend.rules.in
deleted file mode 100644
index ee107af..0000000
--- a/tools/hotplug/Linux/xen-backend.rules.in
+++ /dev/null
@@ -1,13 +0,0 @@
-SUBSYSTEM=="xen-backend", KERNEL=="vbd*", ENV{UDEV_CALL}="1", RUN+="@XEN_SCRIPT_DIR@/block $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vif2-*", RUN+="@XEN_SCRIPT_DIR@/vif2 $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ENV{UDEV_CALL}="1", ACTION=="online", RUN+="@XEN_SCRIPT_DIR@/vif-setup online type_if=vif"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ENV{UDEV_CALL}="1", ACTION=="offline", RUN+="@XEN_SCRIPT_DIR@/vif-setup offline type_if=vif"
-SUBSYSTEM=="xen-backend", KERNEL=="vscsi*", RUN+="@XEN_SCRIPT_DIR@/vscsi $env{ACTION}"
-SUBSYSTEM=="xen-backend", ACTION=="remove", ENV{UDEV_CALL}="1", RUN+="@XEN_SCRIPT_DIR@/xen-hotplug-cleanup"
-KERNEL=="evtchn", NAME="xen/%k"
-SUBSYSTEM=="blktap2", KERNEL=="blktap[0-9]*", NAME="xen/blktap-2/%k", MODE="0600"
-KERNEL=="blktap-control", NAME="xen/blktap-2/control", MODE="0600"
-KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
-KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
-KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
-SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add", ENV{UDEV_CALL}="1", RUN+="@XEN_SCRIPT_DIR@/vif-setup $env{ACTION} type_if=tap"
diff --git a/tools/hotplug/Linux/xen-hotplug-common.sh.in b/tools/hotplug/Linux/xen-hotplug-common.sh.in
index 40b933e..afb240f 100644
--- a/tools/hotplug/Linux/xen-hotplug-common.sh.in
+++ b/tools/hotplug/Linux/xen-hotplug-common.sh.in
@@ -15,13 +15,6 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 #
 
-# Hack to prevent the execution of hotplug scripts from udev if the domain
-# has been launched from libxl
-if [ -n "${UDEV_CALL}" ] && \
-   xenstore-read "libxl/disable_udev" >/dev/null 2>&1; then
-    exit 0
-fi
-
 dir=$(dirname "$0")
 . "$dir/hotplugpath.sh"
 . "$dir/logging.sh"
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 92563db..3a83903 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3211,7 +3211,6 @@ out:
 int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
                                  uint32_t domid)
 {
-    int run_hotplug_scripts;
     int rc;
 
     if (!nic->mtu)
@@ -3242,12 +3241,6 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
                                   libxl__xen_script_dir_path()) < 0 )
         return ERROR_FAIL;
 
-    run_hotplug_scripts = libxl__hotplug_settings(gc, XBT_NULL);
-    if (run_hotplug_scripts < 0) {
-        LOG(ERROR, "unable to get current hotplug scripts execution setting");
-        return run_hotplug_scripts;
-    }
-
     rc = libxl__resolve_domid(gc, nic->backend_domname, &nic->backend_domid);
     if (rc < 0) return rc;
 
@@ -4550,12 +4543,6 @@ int libxl_device_events_handler(libxl_ctx *ctx,
         goto out;
     }
 
-    rc = libxl__xs_write_checked(gc, XBT_NULL, DISABLE_UDEV_PATH, "1");
-    if (rc) {
-        LOGE(ERROR, "unable to write %s = 1", DISABLE_UDEV_PATH);
-        goto out;
-    }
-
     /*
      * We use absolute paths because we want xswatch to also return
      * absolute paths that can be parsed by libxl__parse_backend_path.
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index f799081..9c2303c 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -682,33 +682,6 @@ retry_transaction:
         goto out;
     }
     libxl_vminfo_list_free(vm_list, nb_vm);
-    int hotplug_setting = libxl__hotplug_settings(gc, t);
-    if (hotplug_setting < 0) {
-        LOG(ERROR, "unable to get current hotplug scripts execution setting");
-        rc = ERROR_FAIL;
-        goto out;
-    }
-    if (libxl_defbool_val(info->run_hotplug_scripts) != hotplug_setting &&
-        (nb_vm - 1)) {
-        LOG(ERROR, "cannot change hotplug execution option once set, "
-                    "please shutdown all guests before changing it");
-        rc = ERROR_FAIL;
-        goto out;
-    }
-
-    if (libxl_defbool_val(info->run_hotplug_scripts)) {
-        rc = libxl__xs_write_checked(gc, t, DISABLE_UDEV_PATH, "1");
-        if (rc) {
-            LOGE(ERROR, "unable to write %s = 1", DISABLE_UDEV_PATH);
-            goto out;
-        }
-    } else {
-        rc = libxl__xs_rm_checked(gc, t, DISABLE_UDEV_PATH);
-        if (rc) {
-            LOGE(ERROR, "unable to delete %s", DISABLE_UDEV_PATH);
-            goto out;
-        }
-    }
 
     xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/uuid", vm_path), uuid_string, strlen(uuid_string));
     xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/name", vm_path), info->name, strlen(info->name));
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index c2c67bd..42d548e 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -376,25 +376,6 @@ int libxl__device_model_version_running(libxl__gc *gc, uint32_t domid)
     return value;
 }
 
-int libxl__hotplug_settings(libxl__gc *gc, xs_transaction_t t)
-{
-    int rc = 0;
-    char *val;
-
-    val = libxl__xs_read(gc, t, DISABLE_UDEV_PATH);
-    if (!val && errno != ENOENT) {
-        LOGE(ERROR, "cannot read %s from xenstore", DISABLE_UDEV_PATH);
-        rc = ERROR_FAIL;
-        goto out;
-    }
-    if (!val) val = "0";
-
-    rc = !!atoi(val);
-
-out:
-    return rc;
-}
-
 /* Portability note: this lock utilises flock(2) so a proper implementation of
  * flock(2) is required.
  */
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index d52589e..7129aee 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -104,7 +104,6 @@
 #define STUBDOM_CONSOLE_SERIAL 3
 #define STUBDOM_SPECIAL_CONSOLES 3
 #define TAP_DEVICE_SUFFIX "-emu"
-#define DISABLE_UDEV_PATH "libxl/disable_udev"
 #define DOMID_XS_PATH "domid"
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index f42a89a..512d2c9 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -236,15 +236,8 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
                                    libxl__device_action action,
                                    int num_exec)
 {
-    char *disable_udev = libxl__xs_read(gc, XBT_NULL, DISABLE_UDEV_PATH);
     int rc;
 
-    /* Check if we have to run hotplug scripts */
-    if (!disable_udev) {
-        rc = 0;
-        goto out;
-    }
-
     switch (dev->backend_kind) {
     case LIBXL__DEVICE_KIND_VBD:
         if (num_exec != 0) {
diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
index a2a962e..096c057 100644
--- a/tools/libxl/libxl_netbsd.c
+++ b/tools/libxl/libxl_netbsd.c
@@ -64,15 +64,8 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
                                    libxl__device_action action,
                                    int num_exec)
 {
-    char *disable_udev = libxl__xs_read(gc, XBT_NULL, DISABLE_UDEV_PATH);
     int rc;
 
-    /* Check if we have to run hotplug scripts */
-    if (!disable_udev || num_exec > 0) {
-        rc = 0;
-        goto out;
-    }
-
     switch (dev->backend_kind) {
     case LIBXL__DEVICE_KIND_VBD:
     case LIBXL__DEVICE_KIND_VIF:
-- 
1.9.1

Re: [PATCH 2/6] libxl: Remove linux udev rules

[Wei Liu]

On Mon, Jul 06, 2015 at 11:51:39AM +0100, George Dunlap wrote:
> They are no longer needed, having been replaced by a daemon for
> driverdomains which will run scripts as necessary.
> 
> Worse yet, they seem to be broken for script-based block devices, such
> as block-iscsi.  This wouldn't matter so much if they were never run
> by default; but if you run block-attach without having created a
> domain, then the appropriate node to disable running udev scripts will
> not have been written yet, and the attach will silently fail.
> 
> Rather than try to sort out that issue, just remove them entirely.
> 
> Note: This changes tools/configure.ac, so autogen.sh may need to be
> re-run.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Roger Pau Monne <roger.pau@citrix.com>
> 
> There was some concern that old udev scripts may end up lying around
> in /etc which will still trigger and run unless libxl/disable_udev
> exists.
> 

It's certainly not making things worse, right?

I think it's user's responsibility to make sure the installation is
clean. It's the same as he / she should avoid having stale libraries
around.

> If that's a concern, then for one release we might consider writing
> libxl/disable_udev manually in xencommons or something to make sure
> that they don't trigger.
> ---
>  tools/configure                              | 13 ++++++-------
[...]
>      }
>      libxl_vminfo_list_free(vm_list, nb_vm);
> -    int hotplug_setting = libxl__hotplug_settings(gc, t);
> -    if (hotplug_setting < 0) {
> -        LOG(ERROR, "unable to get current hotplug scripts execution setting");
> -        rc = ERROR_FAIL;
> -        goto out;
> -    }
> -    if (libxl_defbool_val(info->run_hotplug_scripts) != hotplug_setting &&
> -        (nb_vm - 1)) {
> -        LOG(ERROR, "cannot change hotplug execution option once set, "
> -                    "please shutdown all guests before changing it");
> -        rc = ERROR_FAIL;
> -        goto out;
> -    }
> -
> -    if (libxl_defbool_val(info->run_hotplug_scripts)) {
> -        rc = libxl__xs_write_checked(gc, t, DISABLE_UDEV_PATH, "1");
> -        if (rc) {
> -            LOGE(ERROR, "unable to write %s = 1", DISABLE_UDEV_PATH);
> -            goto out;
> -        }
> -    } else {
> -        rc = libxl__xs_rm_checked(gc, t, DISABLE_UDEV_PATH);
> -        if (rc) {
> -            LOGE(ERROR, "unable to delete %s", DISABLE_UDEV_PATH);
> -            goto out;
> -        }

Heh, looks like this is buggy as it is -- every guest shares the same
node. I.e. if a guest has run_hotplug_script in xl config file it can
overwrite that node easily.

Wei.

Re: [PATCH 2/6] libxl: Remove linux udev rules

[Konrad Rzeszutek Wilk]

On Tue, Jul 07, 2015 at 12:39:52PM +0100, Wei Liu wrote:
> On Mon, Jul 06, 2015 at 11:51:39AM +0100, George Dunlap wrote:
> > They are no longer needed, having been replaced by a daemon for
> > driverdomains which will run scripts as necessary.

This introduces an regression. The 'daemon for driverdomains'
is xenbackendd - which only gets built for NetBSD.

That means we can't do Linux driver domains anymore with this patch.


> > 
> > Worse yet, they seem to be broken for script-based block devices, such
> > as block-iscsi.  This wouldn't matter so much if they were never run
> > by default; but if you run block-attach without having created a
> > domain, then the appropriate node to disable running udev scripts will
> > not have been written yet, and the attach will silently fail.
> > 
> > Rather than try to sort out that issue, just remove them entirely.
> > 
> > Note: This changes tools/configure.ac, so autogen.sh may need to be
> > re-run.
> > 
> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> > ---
> > CC: Ian Campbell <ian.campbell@citrix.com>
> > CC: Ian Jackson <ian.jackson@citrix.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> > CC: Roger Pau Monne <roger.pau@citrix.com>
> > 
> > There was some concern that old udev scripts may end up lying around
> > in /etc which will still trigger and run unless libxl/disable_udev
> > exists.
> > 
> 
> It's certainly not making things worse, right?
> 
> I think it's user's responsibility to make sure the installation is
> clean. It's the same as he / she should avoid having stale libraries
> around.
> 
> > If that's a concern, then for one release we might consider writing
> > libxl/disable_udev manually in xencommons or something to make sure
> > that they don't trigger.
> > ---
> >  tools/configure                              | 13 ++++++-------
> [...]
> >      }
> >      libxl_vminfo_list_free(vm_list, nb_vm);
> > -    int hotplug_setting = libxl__hotplug_settings(gc, t);
> > -    if (hotplug_setting < 0) {
> > -        LOG(ERROR, "unable to get current hotplug scripts execution setting");
> > -        rc = ERROR_FAIL;
> > -        goto out;
> > -    }
> > -    if (libxl_defbool_val(info->run_hotplug_scripts) != hotplug_setting &&
> > -        (nb_vm - 1)) {
> > -        LOG(ERROR, "cannot change hotplug execution option once set, "
> > -                    "please shutdown all guests before changing it");
> > -        rc = ERROR_FAIL;
> > -        goto out;
> > -    }
> > -
> > -    if (libxl_defbool_val(info->run_hotplug_scripts)) {
> > -        rc = libxl__xs_write_checked(gc, t, DISABLE_UDEV_PATH, "1");
> > -        if (rc) {
> > -            LOGE(ERROR, "unable to write %s = 1", DISABLE_UDEV_PATH);
> > -            goto out;
> > -        }
> > -    } else {
> > -        rc = libxl__xs_rm_checked(gc, t, DISABLE_UDEV_PATH);
> > -        if (rc) {
> > -            LOGE(ERROR, "unable to delete %s", DISABLE_UDEV_PATH);
> > -            goto out;
> > -        }
> 
> Heh, looks like this is buggy as it is -- every guest shares the same
> node. I.e. if a guest has run_hotplug_script in xl config file it can
> overwrite that node easily.
> 
> Wei.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 2/6] libxl: Remove linux udev rules

[Ian Campbell]

On Tue, 2015-07-14 at 12:13 -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Jul 07, 2015 at 12:39:52PM +0100, Wei Liu wrote:
> > On Mon, Jul 06, 2015 at 11:51:39AM +0100, George Dunlap wrote:
> > > They are no longer needed, having been replaced by a daemon for
> > > driverdomains which will run scripts as necessary.
> 
> This introduces an regression. The 'daemon for driverdomains'
> is xenbackendd - which only gets built for NetBSD.

It's not (any more), it's "xl devd", which should be built everywhere.

One thing which is missing is an initscript to launch this, it probably
wants to be a new xendriverdomain or something since dom0 doesn't need
this so xencommons would be wrong.

Ian.

Re: [PATCH 2/6] libxl: Remove linux udev rules

[George Dunlap]

On 07/14/2015 05:21 PM, Ian Campbell wrote:
> On Tue, 2015-07-14 at 12:13 -0400, Konrad Rzeszutek Wilk wrote:
>> On Tue, Jul 07, 2015 at 12:39:52PM +0100, Wei Liu wrote:
>>> On Mon, Jul 06, 2015 at 11:51:39AM +0100, George Dunlap wrote:
>>>> They are no longer needed, having been replaced by a daemon for
>>>> driverdomains which will run scripts as necessary.
>>
>> This introduces an regression. The 'daemon for driverdomains'
>> is xenbackendd - which only gets built for NetBSD.
> 
> It's not (any more), it's "xl devd", which should be built everywhere.

Aha!  I knew I'd seen it somewhere.

In that case, do we actually still need xenbackendd?  Or does
xenbackendd do something slightly different?

> One thing which is missing is an initscript to launch this, it probably
> wants to be a new xendriverdomain or something since dom0 doesn't need
> this so xencommons would be wrong.

...and xencommons starts a bunch of things which should *not* be in
driver domains, like xenstored, xenconsoled, &c.

 -George

Re: [PATCH 2/6] libxl: Remove linux udev rules

[Wei Liu]

On Tue, Jul 14, 2015 at 05:35:23PM +0100, George Dunlap wrote:
> On 07/14/2015 05:21 PM, Ian Campbell wrote:
> > On Tue, 2015-07-14 at 12:13 -0400, Konrad Rzeszutek Wilk wrote:
> >> On Tue, Jul 07, 2015 at 12:39:52PM +0100, Wei Liu wrote:
> >>> On Mon, Jul 06, 2015 at 11:51:39AM +0100, George Dunlap wrote:
> >>>> They are no longer needed, having been replaced by a daemon for
> >>>> driverdomains which will run scripts as necessary.
> >>
> >> This introduces an regression. The 'daemon for driverdomains'
> >> is xenbackendd - which only gets built for NetBSD.
> > 
> > It's not (any more), it's "xl devd", which should be built everywhere.
> 
> Aha!  I knew I'd seen it somewhere.
> 
> In that case, do we actually still need xenbackendd?  Or does
> xenbackendd do something slightly different?
> 

That's NetBSD-only thing as far as I remember. Not sure if what it does
is NetBSD specific.

Wei.

Re: [PATCH 2/6] libxl: Remove linux udev rules

[Ian Campbell]

On Tue, 2015-07-14 at 17:40 +0100, Wei Liu wrote:
> On Tue, Jul 14, 2015 at 05:35:23PM +0100, George Dunlap wrote:
> > On 07/14/2015 05:21 PM, Ian Campbell wrote:
> > > On Tue, 2015-07-14 at 12:13 -0400, Konrad Rzeszutek Wilk wrote:
> > >> On Tue, Jul 07, 2015 at 12:39:52PM +0100, Wei Liu wrote:
> > >>> On Mon, Jul 06, 2015 at 11:51:39AM +0100, George Dunlap wrote:
> > >>>> They are no longer needed, having been replaced by a daemon for
> > >>>> driverdomains which will run scripts as necessary.
> > >>
> > >> This introduces an regression. The 'daemon for driverdomains'
> > >> is xenbackendd - which only gets built for NetBSD.
> > > 
> > > It's not (any more), it's "xl devd", which should be built everywhere.
> > 
> > Aha!  I knew I'd seen it somewhere.
> > 
> > In that case, do we actually still need xenbackendd?  Or does
> > xenbackendd do something slightly different?
> > 
> 
> That's NetBSD-only thing as far as I remember. Not sure if what it does
> is NetBSD specific.

My understanding was it had been subsumed by "xl devd", but I don't know
why it hasn't been removed. Probably best to wait until Roger is back...

Ian.

Re: [PATCH 2/6] libxl: Remove linux udev rules

[Roger Pau Monné]

El 14/07/15 a les 18.48, Ian Campbell ha escrit:
> On Tue, 2015-07-14 at 17:40 +0100, Wei Liu wrote:
>> On Tue, Jul 14, 2015 at 05:35:23PM +0100, George Dunlap wrote:
>>> On 07/14/2015 05:21 PM, Ian Campbell wrote:
>>>> On Tue, 2015-07-14 at 12:13 -0400, Konrad Rzeszutek Wilk wrote:
>>>>> On Tue, Jul 07, 2015 at 12:39:52PM +0100, Wei Liu wrote:
>>>>>> On Mon, Jul 06, 2015 at 11:51:39AM +0100, George Dunlap wrote:
>>>>>>> They are no longer needed, having been replaced by a daemon for
>>>>>>> driverdomains which will run scripts as necessary.
>>>>>
>>>>> This introduces an regression. The 'daemon for driverdomains'
>>>>> is xenbackendd - which only gets built for NetBSD.
>>>>
>>>> It's not (any more), it's "xl devd", which should be built everywhere.
>>>
>>> Aha!  I knew I'd seen it somewhere.
>>>
>>> In that case, do we actually still need xenbackendd?  Or does
>>> xenbackendd do something slightly different?
>>>
>>
>> That's NetBSD-only thing as far as I remember. Not sure if what it does
>> is NetBSD specific.
> 
> My understanding was it had been subsumed by "xl devd", but I don't know
> why it hasn't been removed. Probably best to wait until Roger is back...

Some time ago I proposed to the NetBSD folks to remove xenbackendd,
because I think it's useless now, but the reply was to leave it as is:

http://mail-index.netbsd.org/port-xen/2015/01/29/msg008483.html

Roger.

Re: [PATCH 2/6] libxl: Remove linux udev rules

[Ian Campbell]

On Thu, 2015-07-23 at 13:55 +0200, Roger Pau Monné wrote:
> El 14/07/15 a les 18.48, Ian Campbell ha escrit:
> > On Tue, 2015-07-14 at 17:40 +0100, Wei Liu wrote:
> > > On Tue, Jul 14, 2015 at 05:35:23PM +0100, George Dunlap wrote:
> > > > On 07/14/2015 05:21 PM, Ian Campbell wrote:
> > > > > On Tue, 2015-07-14 at 12:13 -0400, Konrad Rzeszutek Wilk 
> > > > > wrote:
> > > > > > On Tue, Jul 07, 2015 at 12:39:52PM +0100, Wei Liu wrote:
> > > > > > > On Mon, Jul 06, 2015 at 11:51:39AM +0100, George Dunlap 
> > > > > > > wrote:
> > > > > > > > They are no longer needed, having been replaced by a 
> > > > > > > > daemon for
> > > > > > > > driverdomains which will run scripts as necessary.
> > > > > > 
> > > > > > This introduces an regression. The 'daemon for 
> > > > > > driverdomains'
> > > > > > is xenbackendd - which only gets built for NetBSD.
> > > > > 
> > > > > It's not (any more), it's "xl devd", which should be built 
> > > > > everywhere.
> > > > 
> > > > Aha!  I knew I'd seen it somewhere.
> > > > 
> > > > In that case, do we actually still need xenbackendd?  Or does
> > > > xenbackendd do something slightly different?
> > > > 
> > > 
> > > That's NetBSD-only thing as far as I remember. Not sure if what 
> > > it does
> > > is NetBSD specific.
> > 
> > My understanding was it had been subsumed by "xl devd", but I don't 
> > know
> > why it hasn't been removed. Probably best to wait until Roger is 
> > back...
> 
> Some time ago I proposed to the NetBSD folks to remove xenbackendd,
> because I think it's useless now, but the reply was to leave it as 
> is:
> 
> http://mail-index.netbsd.org/port-xen/2015/01/29/msg008483.html

It's redundant with "xl devd", I don't see any reason for use to
continue carrying it in xen.git. If xl devd has some short coming
relative to xenbackendd then we should address that. But otherwise
if folks want to keep xenbackend despite it having been replaced then
they can carry the patch IMHO.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH for-4.6] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[Ian Campbell]

The removal of the udev rules highlighted that although it has been
replaced by "xl devd" there isn't an initscript to replace it.

To enable this add a --pidfile option to xl devd.

Tested on Linux by running the script in dom0 and checking the daemon
was started/stopped, but not in an actual driver domain environment
since I don't have one conveniently available. I also checked that
running without the --pidfile option still works.

Scripts mainly cribbed from the xencommons for each platform.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
---
For 4.6: I think having removed the udev rules we ought to include
this, it's basically zero risk to normal operation.

Nothing seems to start xl devd for domain 0 in *BSD, nor is xenbackend
started. Is that correct?
---
 .gitignore                                    |  3 ++
 tools/configure                               |  5 +-
 tools/configure.ac                            |  3 ++
 tools/hotplug/FreeBSD/Makefile                |  2 +-
 tools/hotplug/FreeBSD/rc.d/xendriverdomain.in | 48 +++++++++++++++++
 tools/hotplug/Linux/Makefile                  |  3 ++
 tools/hotplug/Linux/init.d/xendriverdomain.in | 77 +++++++++++++++++++++++++++
 tools/hotplug/NetBSD/Makefile                 |  2 +-
 tools/hotplug/NetBSD/rc.d/xendriverdomain.in  | 49 +++++++++++++++++
 tools/libxl/xl_cmdimpl.c                      | 42 +++++++++++++--
 tools/libxl/xl_cmdtable.c                     |  3 +-
 11 files changed, 229 insertions(+), 8 deletions(-)
 create mode 100644 tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
 create mode 100644 tools/hotplug/Linux/init.d/xendriverdomain.in
 create mode 100644 tools/hotplug/NetBSD/rc.d/xendriverdomain.in

diff --git a/.gitignore b/.gitignore
index 3f42ded..bab67e7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -133,10 +133,12 @@ tools/flask/utils/flask-set-bool
 tools/flask/utils/flask-label-pci
 tools/hotplug/common/hotplugpath.sh
 tools/hotplug/FreeBSD/rc.d/xencommons
+tools/hotplug/FreeBSD/rc.d/xendriverdomain
 tools/hotplug/Linux/init.d/sysconfig.xencommons
 tools/hotplug/Linux/init.d/xen-watchdog
 tools/hotplug/Linux/init.d/xencommons
 tools/hotplug/Linux/init.d/xendomains
+tools/hotplug/Linux/init.d/xendriverdomain
 tools/hotplug/Linux/systemd/*.conf
 tools/hotplug/Linux/systemd/*.mount
 tools/hotplug/Linux/systemd/*.socket
@@ -146,6 +148,7 @@ tools/hotplug/Linux/xen-backend.rules
 tools/hotplug/Linux/xen-hotplug-common.sh
 tools/hotplug/Linux/xendomains
 tools/hotplug/NetBSD/rc.d/xencommons
+tools/hotplug/NetBSD/rc.d/xendriverdomain
 tools/include/xen/*
 tools/include/xen-xsm/*
 tools/include/xen-foreign/*.(c|h|size)
diff --git a/tools/configure b/tools/configure
index 5138f3d..d90db47 100755
--- a/tools/configure
+++ b/tools/configure
@@ -2403,7 +2403,7 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
 
 
 
-ac_config_files="$ac_config_files ../config/Tools.mk hotplug/FreeBSD/rc.d/xencommons hotplug/Linux/init.d/sysconfig.xencommons hotplug/Linux/init.d/xen-watchdog hotplug/Linux/init.d/xencommons hotplug/Linux/init.d/xendomains hotplug/Linux/vif-setup hotplug/Linux/xen-hotplug-common.sh hotplug/Linux/xendomains hotplug/NetBSD/rc.d/xencommons libxl/xenlight.pc.in libxl/xlutil.pc.in"
+ac_config_files="$ac_config_files ../config/Tools.mk hotplug/FreeBSD/rc.d/xencommons hotplug/FreeBSD/rc.d/xendriverdomain hotplug/Linux/init.d/sysconfig.xencommons hotplug/Linux/init.d/xen-watchdog hotplug/Linux/init.d/xencommons hotplug/Linux/init.d/xendomains hotplug/Linux/init.d/xendriverdomain hotplug/Linux/vif-setup hotplug/Linux/xen-hotplug-common.sh hotplug/Linux/xendomains hotplug/NetBSD/rc.d/xencommons hotplug/NetBSD/rc.d/xendriverdomain libxl/xenlight.pc.in libxl/xlutil.pc.in"
 
 ac_config_headers="$ac_config_headers config.h"
 
@@ -10045,14 +10045,17 @@ do
   case $ac_config_target in
     "../config/Tools.mk") CONFIG_FILES="$CONFIG_FILES ../config/Tools.mk" ;;
     "hotplug/FreeBSD/rc.d/xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/FreeBSD/rc.d/xencommons" ;;
+    "hotplug/FreeBSD/rc.d/xendriverdomain") CONFIG_FILES="$CONFIG_FILES hotplug/FreeBSD/rc.d/xendriverdomain" ;;
     "hotplug/Linux/init.d/sysconfig.xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/sysconfig.xencommons" ;;
     "hotplug/Linux/init.d/xen-watchdog") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xen-watchdog" ;;
     "hotplug/Linux/init.d/xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xencommons" ;;
     "hotplug/Linux/init.d/xendomains") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xendomains" ;;
+    "hotplug/Linux/init.d/xendriverdomain") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xendriverdomain" ;;
     "hotplug/Linux/vif-setup") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/vif-setup" ;;
     "hotplug/Linux/xen-hotplug-common.sh") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/xen-hotplug-common.sh" ;;
     "hotplug/Linux/xendomains") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/xendomains" ;;
     "hotplug/NetBSD/rc.d/xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/NetBSD/rc.d/xencommons" ;;
+    "hotplug/NetBSD/rc.d/xendriverdomain") CONFIG_FILES="$CONFIG_FILES hotplug/NetBSD/rc.d/xendriverdomain" ;;
     "libxl/xenlight.pc.in") CONFIG_FILES="$CONFIG_FILES libxl/xenlight.pc.in" ;;
     "libxl/xlutil.pc.in") CONFIG_FILES="$CONFIG_FILES libxl/xlutil.pc.in" ;;
     "config.h") CONFIG_HEADERS="$CONFIG_HEADERS config.h" ;;
diff --git a/tools/configure.ac b/tools/configure.ac
index 8bfdfcb..f6a79c2 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -8,14 +8,17 @@ AC_CONFIG_SRCDIR([libxl/libxl.c])
 AC_CONFIG_FILES([
 ../config/Tools.mk
 hotplug/FreeBSD/rc.d/xencommons
+hotplug/FreeBSD/rc.d/xendriverdomain
 hotplug/Linux/init.d/sysconfig.xencommons
 hotplug/Linux/init.d/xen-watchdog
 hotplug/Linux/init.d/xencommons
 hotplug/Linux/init.d/xendomains
+hotplug/Linux/init.d/xendriverdomain
 hotplug/Linux/vif-setup
 hotplug/Linux/xen-hotplug-common.sh
 hotplug/Linux/xendomains
 hotplug/NetBSD/rc.d/xencommons
+hotplug/NetBSD/rc.d/xendriverdomain
 libxl/xenlight.pc.in
 libxl/xlutil.pc.in
 ])
diff --git a/tools/hotplug/FreeBSD/Makefile b/tools/hotplug/FreeBSD/Makefile
index 8dfc90a..10fce4f 100644
--- a/tools/hotplug/FreeBSD/Makefile
+++ b/tools/hotplug/FreeBSD/Makefile
@@ -6,7 +6,7 @@ XEN_SCRIPTS = vif-bridge
 
 XEN_SCRIPT_DATA =
 
-XEN_RCD_PROG = rc.d/xencommons
+XEN_RCD_PROG = rc.d/xencommons rc.d/xendriverdomain
 
 .PHONY: all
 all:
diff --git a/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
new file mode 100644
index 0000000..25e3edd
--- /dev/null
+++ b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
@@ -0,0 +1,48 @@
+#!/bin/sh
+#
+# PROVIDE: xendriverdomain
+# REQUIRE: DAEMON
+#
+# Should be run in a driver domain, but not in domain 0.
+
+. /etc/rc.subr
+
+. /etc/xen/scripts/hotplugpath.sh
+
+LD_LIBRARY_PATH="${libdir}"
+export LD_LIBRARY_PATH
+
+name="xendriverdomain"
+start_precmd="xendriverdomain_precmd"
+start_cmd="xendriverdomain_startcmd"
+stop_cmd="xendriverdomain_stop"
+extra_commands=""
+
+XLDEVD_PIDFILE="/var/run/xldevd.pid"
+
+xendriverdomain_precmd()
+{
+	:
+}
+
+xendriverdomain_startcmd()
+{
+	printf "Starting xenservices: xl devd."
+
+	${sbindir}/xl devd --pidfile=$XLDEVD_PIDFILE ${XLDEVD_ARGS}
+
+	printf "\n"
+}
+
+xendriverdomain_stop()
+{
+	printf "Stopping xl devd.\n"
+
+	rc_pid=$(check_pidfile ${XLDEVD_PIDFILE} ${sbindir}/xl)
+
+	kill -${sig_stop:-TERM} $rc_pids
+	wait_for_pids $rc_pids
+}
+
+load_rc_config $name
+run_rc_command "$1"
diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
index bc8ee5e..6e10118 100644
--- a/tools/hotplug/Linux/Makefile
+++ b/tools/hotplug/Linux/Makefile
@@ -9,6 +9,8 @@ XENDOMAINS_SYSCONFIG = init.d/sysconfig.xendomains
 XENCOMMONS_INITD = init.d/xencommons
 XENCOMMONS_SYSCONFIG = init.d/sysconfig.xencommons
 
+XENDRIVERDOMAIN_INITD = init.d/xendriverdomain
+
 # Xen script dir and scripts to go there.
 XEN_SCRIPTS = vif-bridge
 XEN_SCRIPTS += vif-route
@@ -53,6 +55,7 @@ install-initd:
 	$(INSTALL_DATA) $(XENDOMAINS_SYSCONFIG) $(DESTDIR)$(SYSCONFIG_DIR)/xendomains
 	$(INSTALL_PROG) $(XENCOMMONS_INITD) $(DESTDIR)$(INITD_DIR)
 	$(INSTALL_DATA) $(XENCOMMONS_SYSCONFIG) $(DESTDIR)$(SYSCONFIG_DIR)/xencommons
+	$(INSTALL_PROG) $(XENDRIVERDOMAIN_INITD) $(DESTDIR)$(INITD_DIR)
 	$(INSTALL_PROG) init.d/xen-watchdog $(DESTDIR)$(INITD_DIR)
 
 .PHONY: install-scripts
diff --git a/tools/hotplug/Linux/init.d/xendriverdomain.in b/tools/hotplug/Linux/init.d/xendriverdomain.in
new file mode 100644
index 0000000..2ea1113
--- /dev/null
+++ b/tools/hotplug/Linux/init.d/xendriverdomain.in
@@ -0,0 +1,77 @@
+
+#!/bin/bash
+#
+# xendriverdomain    Script to start services needed in a Xen driver domain
+#
+# NOTE: This initscript is not needed on dom0.
+
+# chkconfig: 2345 70 10
+# description: Starts and stops xen driver domain daemon
+### BEGIN INIT INFO
+# Provides:          xendevd
+# Required-Start:    $syslog $remote_fs
+# Should-Start:
+# Required-Stop:     $syslog $remote_fs
+# Should-Stop:
+# Default-Start:     2 3 5
+# Default-Stop:      0 1 6
+# Short-Description: Start/stop xen driver domain daemon
+# Description:       Starts and stops the daemons neeeded for a xen driver domain
+### END INIT INFO
+
+. @XEN_SCRIPT_DIR@/hotplugpath.sh
+
+xendriverdomain_config=@CONFIG_DIR@/@CONFIG_LEAF_DIR@
+
+test -f $xendriverdomain_config/xendriverdomain && . $xendriverdomain_config/xendriverdomain
+
+XLDEVD_PIDFILE=/var/run/xldevd.pid
+
+# not running in Xen dom0 or domU
+if ! test -d /proc/xen ; then
+	exit 0
+fi
+
+# mount xenfs in dom0 or domU with a pv_ops kernel
+if test "x$1" = xstart && \
+   ! test -f /proc/xen/capabilities && \
+   ! grep '^xenfs ' /proc/mounts >/dev/null;
+then
+	mount -t xenfs xenfs /proc/xen
+fi
+
+do_start () {
+	echo Starting xl devd...
+	${sbindir}/xl devd --pidfile=$XLDEVD_PIDFILE $XLDEVD_ARGS
+}
+do_stop () {
+        echo Stopping xl devd...
+	if read 2>/dev/null <$XLDEVD_PIDFILE pid; then
+		kill $pid
+		while kill -9 $pid >/dev/null 2>&1; do sleep 0.1; done
+		rm -f $XLDEVD_PIDFILE
+	fi
+}
+
+case "$1" in
+  start)
+	do_start
+	;;
+  stop)
+	do_stop
+	;;
+  reload)
+	echo >&2 'Reload not available; use force-reload'; exit 1
+	;;
+  force-reload|restart)
+        do_stop
+	do_start
+	;;
+  *)
+	# do not advertise unreasonable commands that there is no reason
+	# to use with this device
+	echo $"Usage: $0 {start|stop|restart|force-reload}"
+	exit 1
+esac
+
+exit $?
diff --git a/tools/hotplug/NetBSD/Makefile b/tools/hotplug/NetBSD/Makefile
index 4e609e3..d01aabf 100644
--- a/tools/hotplug/NetBSD/Makefile
+++ b/tools/hotplug/NetBSD/Makefile
@@ -8,7 +8,7 @@ XEN_SCRIPTS += vif-bridge
 XEN_SCRIPTS += vif-ip
 
 XEN_SCRIPT_DATA =
-XEN_RCD_PROG = rc.d/xencommons rc.d/xendomains rc.d/xen-watchdog
+XEN_RCD_PROG = rc.d/xencommons rc.d/xendomains rc.d/xen-watchdog rc.d/xendriverdomain
 
 .PHONY: all
 all:
diff --git a/tools/hotplug/NetBSD/rc.d/xendriverdomain.in b/tools/hotplug/NetBSD/rc.d/xendriverdomain.in
new file mode 100644
index 0000000..5062a71
--- /dev/null
+++ b/tools/hotplug/NetBSD/rc.d/xendriverdomain.in
@@ -0,0 +1,49 @@
+#!/bin/sh
+#
+# PROVIDE: xendriverdomain
+# REQUIRE: DAEMON
+#
+# Should be run in a driver domain, but not in domain 0.
+
+. /etc/rc.subr
+
+DIR=$(dirname "$0")
+. "${DIR}/xen-hotplugpath.sh"
+
+LD_LIBRARY_PATH="${libdir}"
+export LD_LIBRARY_PATH
+
+name="xendriverdomain"
+start_precmd="xendriverdomain_precmd"
+start_cmd="xendriverdomain_startcmd"
+stop_cmd="xendriverdomain_stop"
+extra_commands=""
+
+XLDEVD_PIDFILE="/var/run/xldevd.pid"
+
+xendriverdomain_precmd()
+{
+	:
+}
+
+xendriverdomain_startcmd()
+{
+	printf "Starting xenservices: xl devd."
+
+	${sbindir}/xl devd --pidfile=$XLDEVD_PIDFILE ${XLDEVD_ARGS}
+
+	printf "\n"
+}
+
+xendriverdomain_stop()
+{
+	printf "Stopping xl devd.\n"
+
+	rc_pid=$(check_pidfile ${XLDEVD_PIDFILE} ${sbindir}/xl)
+
+	kill -${sig_stop:-TERM} $rc_pids
+	wait_for_pids $rc_pids
+}
+
+load_rc_config $name
+run_rc_command "$1"
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5f7a712..17e6e7d 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -445,7 +445,7 @@ out:
     flush_stream(fh);
 }
 
-static int do_daemonize(char *name)
+static int do_daemonize(char *name, const char *pidfile)
 {
     char *fullname;
     pid_t child1;
@@ -477,6 +477,31 @@ static int do_daemonize(char *name)
 
     CHK_SYSCALL(daemon(0, 1));
 
+    if (pidfile) {
+        int fd = open(pidfile, O_RDWR | O_CREAT, S_IRUSR|S_IWUSR);
+        char *pid = NULL;
+
+        if (fd == -1) {
+            perror("Unable to open pidfile");
+            exit(1);
+        }
+
+        if (asprintf(&pid, "%ld\n", (long)getpid()) == -1) {
+            perror("Formatting pid");
+            exit(1);
+        }
+
+        if (write(fd, pid, strlen(pid)) < 0) {
+            perror("Writing pid");
+            exit(1);
+        }
+
+        if ( close(fd) < 0 ) {
+            perror("Closing pidfile");
+            exit(1);
+        }
+    }
+
 out:
     return ret;
 }
@@ -2798,7 +2823,7 @@ start:
             LOG("Failed to allocate memory in asprintf");
             exit(1);
         }
-        ret = do_daemonize(name);
+        ret = do_daemonize(name, NULL);
         free(name);
         if (ret) {
             ret = (ret == 1) ? domid : ret;
@@ -7972,15 +7997,24 @@ int main_remus(int argc, char **argv)
 int main_devd(int argc, char **argv)
 {
     int ret = 0, opt = 0, daemonize = 1;
+    const char *pidfile = NULL;
+    static const struct option opts[] = {
+        {"pidfile", 1, 0, 'p'},
+        COMMON_LONG_OPTS,
+        {0, 0, 0, 0}
+    };
 
-    SWITCH_FOREACH_OPT(opt, "F", NULL, "devd", 0) {
+    SWITCH_FOREACH_OPT(opt, "Fp:", opts, "devd", 0) {
     case 'F':
         daemonize = 0;
         break;
+    case 'p':
+        pidfile = optarg;
+        break;
     }
 
     if (daemonize) {
-        ret = do_daemonize("xldevd");
+        ret = do_daemonize("xldevd", pidfile);
         if (ret) {
             ret = (ret == 1) ? 0 : ret;
             goto out;
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 54dbecc..0071f12 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -506,7 +506,8 @@ struct cmd_spec cmd_table[] = {
       &main_devd, 0, 1,
       "Daemon that listens for devices and launches backends",
       "[options]",
-      "-F                      Run in the foreground",
+      "-F                      Run in the foreground.\n"
+      "-p, --pidfile [FILE]    Write PID to pidfile when daemonizing.",
     },
 #ifdef LIBXL_HAVE_PSR_CMT
     { "psr-hwinfo",
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH for-4.6] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[Wei Liu]

On Wed, Jul 15, 2015 at 12:07:30PM +0100, Ian Campbell wrote:
> The removal of the udev rules highlighted that although it has been
> replaced by "xl devd" there isn't an initscript to replace it.
> 
> To enable this add a --pidfile option to xl devd.
> 
> Tested on Linux by running the script in dom0 and checking the daemon
> was started/stopped, but not in an actual driver domain environment
> since I don't have one conveniently available. I also checked that
> running without the --pidfile option still works.
> 
> Scripts mainly cribbed from the xencommons for each platform.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> For 4.6: I think having removed the udev rules we ought to include
> this, it's basically zero risk to normal operation.
> 
> Nothing seems to start xl devd for domain 0 in *BSD, nor is xenbackend
> started. Is that correct?
> ---
>  .gitignore                                    |  3 ++
>  tools/configure                               |  5 +-
>  tools/configure.ac                            |  3 ++
>  tools/hotplug/FreeBSD/Makefile                |  2 +-
>  tools/hotplug/FreeBSD/rc.d/xendriverdomain.in | 48 +++++++++++++++++
>  tools/hotplug/Linux/Makefile                  |  3 ++
>  tools/hotplug/Linux/init.d/xendriverdomain.in | 77 +++++++++++++++++++++++++++
>  tools/hotplug/NetBSD/Makefile                 |  2 +-
>  tools/hotplug/NetBSD/rc.d/xendriverdomain.in  | 49 +++++++++++++++++
>  tools/libxl/xl_cmdimpl.c                      | 42 +++++++++++++--
>  tools/libxl/xl_cmdtable.c                     |  3 +-

Systemd unit file is missing.

Wei.

Re: [PATCH for-4.6] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[Ian Campbell]

On Wed, 2015-07-15 at 14:26 +0100, Wei Liu wrote:
> On Wed, Jul 15, 2015 at 12:07:30PM +0100, Ian Campbell wrote:
> > The removal of the udev rules highlighted that although it has been
> > replaced by "xl devd" there isn't an initscript to replace it.
> > 
> > To enable this add a --pidfile option to xl devd.
> > 
> > Tested on Linux by running the script in dom0 and checking the daemon
> > was started/stopped, but not in an actual driver domain environment
> > since I don't have one conveniently available. I also checked that
> > running without the --pidfile option still works.
> > 
> > Scripts mainly cribbed from the xencommons for each platform.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Roger Pau Monné <roger.pau@citrix.com>
> > Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> > Cc: George Dunlap <george.dunlap@eu.citrix.com>
> > ---
> > For 4.6: I think having removed the udev rules we ought to include
> > this, it's basically zero risk to normal operation.
> > 
> > Nothing seems to start xl devd for domain 0 in *BSD, nor is xenbackend
> > started. Is that correct?
> > ---
> >  .gitignore                                    |  3 ++
> >  tools/configure                               |  5 +-
> >  tools/configure.ac                            |  3 ++
> >  tools/hotplug/FreeBSD/Makefile                |  2 +-
> >  tools/hotplug/FreeBSD/rc.d/xendriverdomain.in | 48 +++++++++++++++++
> >  tools/hotplug/Linux/Makefile                  |  3 ++
> >  tools/hotplug/Linux/init.d/xendriverdomain.in | 77 +++++++++++++++++++++++++++
> >  tools/hotplug/NetBSD/Makefile                 |  2 +-
> >  tools/hotplug/NetBSD/rc.d/xendriverdomain.in  | 49 +++++++++++++++++
> >  tools/libxl/xl_cmdimpl.c                      | 42 +++++++++++++--
> >  tools/libxl/xl_cmdtable.c                     |  3 +-
> 
> Systemd unit file is missing.

Yes, it is.

I don't think it matters, systemd is supposed to have support for
"legacy" init scripts and this one is simple enough IMHO to not need any
of the special sauce that a unit would give.

If some systemd person wants to add a unit file then they can do so.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH for-4.6] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[George Dunlap]

On 07/15/2015 12:07 PM, Ian Campbell wrote:
> The removal of the udev rules highlighted that although it has been
> replaced by "xl devd" there isn't an initscript to replace it.
> 
> To enable this add a --pidfile option to xl devd.
> 
> Tested on Linux by running the script in dom0 and checking the daemon
> was started/stopped, but not in an actual driver domain environment
> since I don't have one conveniently available. I also checked that
> running without the --pidfile option still works.
> 
> Scripts mainly cribbed from the xencommons for each platform.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>

Looks OK to me:

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

> ---
> For 4.6: I think having removed the udev rules we ought to include
> this, it's basically zero risk to normal operation.
> 
> Nothing seems to start xl devd for domain 0 in *BSD, nor is xenbackend
> started. Is that correct?
> ---
>  .gitignore                                    |  3 ++
>  tools/configure                               |  5 +-
>  tools/configure.ac                            |  3 ++
>  tools/hotplug/FreeBSD/Makefile                |  2 +-
>  tools/hotplug/FreeBSD/rc.d/xendriverdomain.in | 48 +++++++++++++++++
>  tools/hotplug/Linux/Makefile                  |  3 ++
>  tools/hotplug/Linux/init.d/xendriverdomain.in | 77 +++++++++++++++++++++++++++
>  tools/hotplug/NetBSD/Makefile                 |  2 +-
>  tools/hotplug/NetBSD/rc.d/xendriverdomain.in  | 49 +++++++++++++++++
>  tools/libxl/xl_cmdimpl.c                      | 42 +++++++++++++--
>  tools/libxl/xl_cmdtable.c                     |  3 +-
>  11 files changed, 229 insertions(+), 8 deletions(-)
>  create mode 100644 tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
>  create mode 100644 tools/hotplug/Linux/init.d/xendriverdomain.in
>  create mode 100644 tools/hotplug/NetBSD/rc.d/xendriverdomain.in
> 
> diff --git a/.gitignore b/.gitignore
> index 3f42ded..bab67e7 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -133,10 +133,12 @@ tools/flask/utils/flask-set-bool
>  tools/flask/utils/flask-label-pci
>  tools/hotplug/common/hotplugpath.sh
>  tools/hotplug/FreeBSD/rc.d/xencommons
> +tools/hotplug/FreeBSD/rc.d/xendriverdomain
>  tools/hotplug/Linux/init.d/sysconfig.xencommons
>  tools/hotplug/Linux/init.d/xen-watchdog
>  tools/hotplug/Linux/init.d/xencommons
>  tools/hotplug/Linux/init.d/xendomains
> +tools/hotplug/Linux/init.d/xendriverdomain
>  tools/hotplug/Linux/systemd/*.conf
>  tools/hotplug/Linux/systemd/*.mount
>  tools/hotplug/Linux/systemd/*.socket
> @@ -146,6 +148,7 @@ tools/hotplug/Linux/xen-backend.rules
>  tools/hotplug/Linux/xen-hotplug-common.sh
>  tools/hotplug/Linux/xendomains
>  tools/hotplug/NetBSD/rc.d/xencommons
> +tools/hotplug/NetBSD/rc.d/xendriverdomain
>  tools/include/xen/*
>  tools/include/xen-xsm/*
>  tools/include/xen-foreign/*.(c|h|size)
> diff --git a/tools/configure b/tools/configure
> index 5138f3d..d90db47 100755
> --- a/tools/configure
> +++ b/tools/configure
> @@ -2403,7 +2403,7 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
>  
>  
>  
> -ac_config_files="$ac_config_files ../config/Tools.mk hotplug/FreeBSD/rc.d/xencommons hotplug/Linux/init.d/sysconfig.xencommons hotplug/Linux/init.d/xen-watchdog hotplug/Linux/init.d/xencommons hotplug/Linux/init.d/xendomains hotplug/Linux/vif-setup hotplug/Linux/xen-hotplug-common.sh hotplug/Linux/xendomains hotplug/NetBSD/rc.d/xencommons libxl/xenlight.pc.in libxl/xlutil.pc.in"
> +ac_config_files="$ac_config_files ../config/Tools.mk hotplug/FreeBSD/rc.d/xencommons hotplug/FreeBSD/rc.d/xendriverdomain hotplug/Linux/init.d/sysconfig.xencommons hotplug/Linux/init.d/xen-watchdog hotplug/Linux/init.d/xencommons hotplug/Linux/init.d/xendomains hotplug/Linux/init.d/xendriverdomain hotplug/Linux/vif-setup hotplug/Linux/xen-hotplug-common.sh hotplug/Linux/xendomains hotplug/NetBSD/rc.d/xencommons hotplug/NetBSD/rc.d/xendriverdomain libxl/xenlight.pc.in libxl/xlutil.pc.in"
>  
>  ac_config_headers="$ac_config_headers config.h"
>  
> @@ -10045,14 +10045,17 @@ do
>    case $ac_config_target in
>      "../config/Tools.mk") CONFIG_FILES="$CONFIG_FILES ../config/Tools.mk" ;;
>      "hotplug/FreeBSD/rc.d/xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/FreeBSD/rc.d/xencommons" ;;
> +    "hotplug/FreeBSD/rc.d/xendriverdomain") CONFIG_FILES="$CONFIG_FILES hotplug/FreeBSD/rc.d/xendriverdomain" ;;
>      "hotplug/Linux/init.d/sysconfig.xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/sysconfig.xencommons" ;;
>      "hotplug/Linux/init.d/xen-watchdog") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xen-watchdog" ;;
>      "hotplug/Linux/init.d/xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xencommons" ;;
>      "hotplug/Linux/init.d/xendomains") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xendomains" ;;
> +    "hotplug/Linux/init.d/xendriverdomain") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xendriverdomain" ;;
>      "hotplug/Linux/vif-setup") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/vif-setup" ;;
>      "hotplug/Linux/xen-hotplug-common.sh") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/xen-hotplug-common.sh" ;;
>      "hotplug/Linux/xendomains") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/xendomains" ;;
>      "hotplug/NetBSD/rc.d/xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/NetBSD/rc.d/xencommons" ;;
> +    "hotplug/NetBSD/rc.d/xendriverdomain") CONFIG_FILES="$CONFIG_FILES hotplug/NetBSD/rc.d/xendriverdomain" ;;
>      "libxl/xenlight.pc.in") CONFIG_FILES="$CONFIG_FILES libxl/xenlight.pc.in" ;;
>      "libxl/xlutil.pc.in") CONFIG_FILES="$CONFIG_FILES libxl/xlutil.pc.in" ;;
>      "config.h") CONFIG_HEADERS="$CONFIG_HEADERS config.h" ;;
> diff --git a/tools/configure.ac b/tools/configure.ac
> index 8bfdfcb..f6a79c2 100644
> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -8,14 +8,17 @@ AC_CONFIG_SRCDIR([libxl/libxl.c])
>  AC_CONFIG_FILES([
>  ../config/Tools.mk
>  hotplug/FreeBSD/rc.d/xencommons
> +hotplug/FreeBSD/rc.d/xendriverdomain
>  hotplug/Linux/init.d/sysconfig.xencommons
>  hotplug/Linux/init.d/xen-watchdog
>  hotplug/Linux/init.d/xencommons
>  hotplug/Linux/init.d/xendomains
> +hotplug/Linux/init.d/xendriverdomain
>  hotplug/Linux/vif-setup
>  hotplug/Linux/xen-hotplug-common.sh
>  hotplug/Linux/xendomains
>  hotplug/NetBSD/rc.d/xencommons
> +hotplug/NetBSD/rc.d/xendriverdomain
>  libxl/xenlight.pc.in
>  libxl/xlutil.pc.in
>  ])
> diff --git a/tools/hotplug/FreeBSD/Makefile b/tools/hotplug/FreeBSD/Makefile
> index 8dfc90a..10fce4f 100644
> --- a/tools/hotplug/FreeBSD/Makefile
> +++ b/tools/hotplug/FreeBSD/Makefile
> @@ -6,7 +6,7 @@ XEN_SCRIPTS = vif-bridge
>  
>  XEN_SCRIPT_DATA =
>  
> -XEN_RCD_PROG = rc.d/xencommons
> +XEN_RCD_PROG = rc.d/xencommons rc.d/xendriverdomain
>  
>  .PHONY: all
>  all:
> diff --git a/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
> new file mode 100644
> index 0000000..25e3edd
> --- /dev/null
> +++ b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
> @@ -0,0 +1,48 @@
> +#!/bin/sh
> +#
> +# PROVIDE: xendriverdomain
> +# REQUIRE: DAEMON
> +#
> +# Should be run in a driver domain, but not in domain 0.
> +
> +. /etc/rc.subr
> +
> +. /etc/xen/scripts/hotplugpath.sh
> +
> +LD_LIBRARY_PATH="${libdir}"
> +export LD_LIBRARY_PATH
> +
> +name="xendriverdomain"
> +start_precmd="xendriverdomain_precmd"
> +start_cmd="xendriverdomain_startcmd"
> +stop_cmd="xendriverdomain_stop"
> +extra_commands=""
> +
> +XLDEVD_PIDFILE="/var/run/xldevd.pid"
> +
> +xendriverdomain_precmd()
> +{
> +	:
> +}
> +
> +xendriverdomain_startcmd()
> +{
> +	printf "Starting xenservices: xl devd."
> +
> +	${sbindir}/xl devd --pidfile=$XLDEVD_PIDFILE ${XLDEVD_ARGS}
> +
> +	printf "\n"
> +}
> +
> +xendriverdomain_stop()
> +{
> +	printf "Stopping xl devd.\n"
> +
> +	rc_pid=$(check_pidfile ${XLDEVD_PIDFILE} ${sbindir}/xl)
> +
> +	kill -${sig_stop:-TERM} $rc_pids
> +	wait_for_pids $rc_pids
> +}
> +
> +load_rc_config $name
> +run_rc_command "$1"
> diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
> index bc8ee5e..6e10118 100644
> --- a/tools/hotplug/Linux/Makefile
> +++ b/tools/hotplug/Linux/Makefile
> @@ -9,6 +9,8 @@ XENDOMAINS_SYSCONFIG = init.d/sysconfig.xendomains
>  XENCOMMONS_INITD = init.d/xencommons
>  XENCOMMONS_SYSCONFIG = init.d/sysconfig.xencommons
>  
> +XENDRIVERDOMAIN_INITD = init.d/xendriverdomain
> +
>  # Xen script dir and scripts to go there.
>  XEN_SCRIPTS = vif-bridge
>  XEN_SCRIPTS += vif-route
> @@ -53,6 +55,7 @@ install-initd:
>  	$(INSTALL_DATA) $(XENDOMAINS_SYSCONFIG) $(DESTDIR)$(SYSCONFIG_DIR)/xendomains
>  	$(INSTALL_PROG) $(XENCOMMONS_INITD) $(DESTDIR)$(INITD_DIR)
>  	$(INSTALL_DATA) $(XENCOMMONS_SYSCONFIG) $(DESTDIR)$(SYSCONFIG_DIR)/xencommons
> +	$(INSTALL_PROG) $(XENDRIVERDOMAIN_INITD) $(DESTDIR)$(INITD_DIR)
>  	$(INSTALL_PROG) init.d/xen-watchdog $(DESTDIR)$(INITD_DIR)
>  
>  .PHONY: install-scripts
> diff --git a/tools/hotplug/Linux/init.d/xendriverdomain.in b/tools/hotplug/Linux/init.d/xendriverdomain.in
> new file mode 100644
> index 0000000..2ea1113
> --- /dev/null
> +++ b/tools/hotplug/Linux/init.d/xendriverdomain.in
> @@ -0,0 +1,77 @@
> +
> +#!/bin/bash
> +#
> +# xendriverdomain    Script to start services needed in a Xen driver domain
> +#
> +# NOTE: This initscript is not needed on dom0.
> +
> +# chkconfig: 2345 70 10
> +# description: Starts and stops xen driver domain daemon
> +### BEGIN INIT INFO
> +# Provides:          xendevd
> +# Required-Start:    $syslog $remote_fs
> +# Should-Start:
> +# Required-Stop:     $syslog $remote_fs
> +# Should-Stop:
> +# Default-Start:     2 3 5
> +# Default-Stop:      0 1 6
> +# Short-Description: Start/stop xen driver domain daemon
> +# Description:       Starts and stops the daemons neeeded for a xen driver domain
> +### END INIT INFO
> +
> +. @XEN_SCRIPT_DIR@/hotplugpath.sh
> +
> +xendriverdomain_config=@CONFIG_DIR@/@CONFIG_LEAF_DIR@
> +
> +test -f $xendriverdomain_config/xendriverdomain && . $xendriverdomain_config/xendriverdomain
> +
> +XLDEVD_PIDFILE=/var/run/xldevd.pid
> +
> +# not running in Xen dom0 or domU
> +if ! test -d /proc/xen ; then
> +	exit 0
> +fi
> +
> +# mount xenfs in dom0 or domU with a pv_ops kernel
> +if test "x$1" = xstart && \
> +   ! test -f /proc/xen/capabilities && \
> +   ! grep '^xenfs ' /proc/mounts >/dev/null;
> +then
> +	mount -t xenfs xenfs /proc/xen
> +fi
> +
> +do_start () {
> +	echo Starting xl devd...
> +	${sbindir}/xl devd --pidfile=$XLDEVD_PIDFILE $XLDEVD_ARGS
> +}
> +do_stop () {
> +        echo Stopping xl devd...
> +	if read 2>/dev/null <$XLDEVD_PIDFILE pid; then
> +		kill $pid
> +		while kill -9 $pid >/dev/null 2>&1; do sleep 0.1; done
> +		rm -f $XLDEVD_PIDFILE
> +	fi
> +}
> +
> +case "$1" in
> +  start)
> +	do_start
> +	;;
> +  stop)
> +	do_stop
> +	;;
> +  reload)
> +	echo >&2 'Reload not available; use force-reload'; exit 1
> +	;;
> +  force-reload|restart)
> +        do_stop
> +	do_start
> +	;;
> +  *)
> +	# do not advertise unreasonable commands that there is no reason
> +	# to use with this device
> +	echo $"Usage: $0 {start|stop|restart|force-reload}"
> +	exit 1
> +esac
> +
> +exit $?
> diff --git a/tools/hotplug/NetBSD/Makefile b/tools/hotplug/NetBSD/Makefile
> index 4e609e3..d01aabf 100644
> --- a/tools/hotplug/NetBSD/Makefile
> +++ b/tools/hotplug/NetBSD/Makefile
> @@ -8,7 +8,7 @@ XEN_SCRIPTS += vif-bridge
>  XEN_SCRIPTS += vif-ip
>  
>  XEN_SCRIPT_DATA =
> -XEN_RCD_PROG = rc.d/xencommons rc.d/xendomains rc.d/xen-watchdog
> +XEN_RCD_PROG = rc.d/xencommons rc.d/xendomains rc.d/xen-watchdog rc.d/xendriverdomain
>  
>  .PHONY: all
>  all:
> diff --git a/tools/hotplug/NetBSD/rc.d/xendriverdomain.in b/tools/hotplug/NetBSD/rc.d/xendriverdomain.in
> new file mode 100644
> index 0000000..5062a71
> --- /dev/null
> +++ b/tools/hotplug/NetBSD/rc.d/xendriverdomain.in
> @@ -0,0 +1,49 @@
> +#!/bin/sh
> +#
> +# PROVIDE: xendriverdomain
> +# REQUIRE: DAEMON
> +#
> +# Should be run in a driver domain, but not in domain 0.
> +
> +. /etc/rc.subr
> +
> +DIR=$(dirname "$0")
> +. "${DIR}/xen-hotplugpath.sh"
> +
> +LD_LIBRARY_PATH="${libdir}"
> +export LD_LIBRARY_PATH
> +
> +name="xendriverdomain"
> +start_precmd="xendriverdomain_precmd"
> +start_cmd="xendriverdomain_startcmd"
> +stop_cmd="xendriverdomain_stop"
> +extra_commands=""
> +
> +XLDEVD_PIDFILE="/var/run/xldevd.pid"
> +
> +xendriverdomain_precmd()
> +{
> +	:
> +}
> +
> +xendriverdomain_startcmd()
> +{
> +	printf "Starting xenservices: xl devd."
> +
> +	${sbindir}/xl devd --pidfile=$XLDEVD_PIDFILE ${XLDEVD_ARGS}
> +
> +	printf "\n"
> +}
> +
> +xendriverdomain_stop()
> +{
> +	printf "Stopping xl devd.\n"
> +
> +	rc_pid=$(check_pidfile ${XLDEVD_PIDFILE} ${sbindir}/xl)
> +
> +	kill -${sig_stop:-TERM} $rc_pids
> +	wait_for_pids $rc_pids
> +}
> +
> +load_rc_config $name
> +run_rc_command "$1"
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 5f7a712..17e6e7d 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -445,7 +445,7 @@ out:
>      flush_stream(fh);
>  }
>  
> -static int do_daemonize(char *name)
> +static int do_daemonize(char *name, const char *pidfile)
>  {
>      char *fullname;
>      pid_t child1;
> @@ -477,6 +477,31 @@ static int do_daemonize(char *name)
>  
>      CHK_SYSCALL(daemon(0, 1));
>  
> +    if (pidfile) {
> +        int fd = open(pidfile, O_RDWR | O_CREAT, S_IRUSR|S_IWUSR);
> +        char *pid = NULL;
> +
> +        if (fd == -1) {
> +            perror("Unable to open pidfile");
> +            exit(1);
> +        }
> +
> +        if (asprintf(&pid, "%ld\n", (long)getpid()) == -1) {
> +            perror("Formatting pid");
> +            exit(1);
> +        }
> +
> +        if (write(fd, pid, strlen(pid)) < 0) {
> +            perror("Writing pid");
> +            exit(1);
> +        }
> +
> +        if ( close(fd) < 0 ) {
> +            perror("Closing pidfile");
> +            exit(1);
> +        }
> +    }
> +
>  out:
>      return ret;
>  }
> @@ -2798,7 +2823,7 @@ start:
>              LOG("Failed to allocate memory in asprintf");
>              exit(1);
>          }
> -        ret = do_daemonize(name);
> +        ret = do_daemonize(name, NULL);
>          free(name);
>          if (ret) {
>              ret = (ret == 1) ? domid : ret;
> @@ -7972,15 +7997,24 @@ int main_remus(int argc, char **argv)
>  int main_devd(int argc, char **argv)
>  {
>      int ret = 0, opt = 0, daemonize = 1;
> +    const char *pidfile = NULL;
> +    static const struct option opts[] = {
> +        {"pidfile", 1, 0, 'p'},
> +        COMMON_LONG_OPTS,
> +        {0, 0, 0, 0}
> +    };
>  
> -    SWITCH_FOREACH_OPT(opt, "F", NULL, "devd", 0) {
> +    SWITCH_FOREACH_OPT(opt, "Fp:", opts, "devd", 0) {
>      case 'F':
>          daemonize = 0;
>          break;
> +    case 'p':
> +        pidfile = optarg;
> +        break;
>      }
>  
>      if (daemonize) {
> -        ret = do_daemonize("xldevd");
> +        ret = do_daemonize("xldevd", pidfile);
>          if (ret) {
>              ret = (ret == 1) ? 0 : ret;
>              goto out;
> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index 54dbecc..0071f12 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -506,7 +506,8 @@ struct cmd_spec cmd_table[] = {
>        &main_devd, 0, 1,
>        "Daemon that listens for devices and launches backends",
>        "[options]",
> -      "-F                      Run in the foreground",
> +      "-F                      Run in the foreground.\n"
> +      "-p, --pidfile [FILE]    Write PID to pidfile when daemonizing.",
>      },
>  #ifdef LIBXL_HAVE_PSR_CMT
>      { "psr-hwinfo",
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH for-4.6] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[Ian Jackson]

Ian Campbell writes ("[PATCH for-4.6] tools/hotplug: Add an initscript to start "xl devd" in a driver domain"):
> The removal of the udev rules highlighted that although it has been
> replaced by "xl devd" there isn't an initscript to replace it.

Despite

> +# NOTE: This initscript is not needed on dom0.

I didn't see anything which would stop it running on dom0.  I don't
think running it on dom0 is a good idea; it might fight with libxl.

Ian.

Re: [PATCH for-4.6] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[George Dunlap]

On 07/15/2015 04:32 PM, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH for-4.6] tools/hotplug: Add an initscript to start "xl devd" in a driver domain"):
>> The removal of the udev rules highlighted that although it has been
>> replaced by "xl devd" there isn't an initscript to replace it.
> 
> Despite
> 
>> +# NOTE: This initscript is not needed on dom0.
> 
> I didn't see anything which would stop it running on dom0.  I don't
> think running it on dom0 is a good idea; it might fight with libxl.

I think Ian's idea was that the admin should know not to run it in dom0,
and only enable it in driver domains.

Do you think we should do something to actively prevent it running in
dom0?  Do we have a good way to determine whether we're in dom0 or not?
 (Or more apropos, whether we're running in the toolstack domain?)

 -George

Re: [PATCH for-4.6] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[Ian Campbell]

On Wed, 2015-07-15 at 16:35 +0100, George Dunlap wrote:
> On 07/15/2015 04:32 PM, Ian Jackson wrote:
> > Ian Campbell writes ("[PATCH for-4.6] tools/hotplug: Add an initscript to start "xl devd" in a driver domain"):
> >> The removal of the udev rules highlighted that although it has been
> >> replaced by "xl devd" there isn't an initscript to replace it.
> > 
> > Despite
> > 
> >> +# NOTE: This initscript is not needed on dom0.
> > 
> > I didn't see anything which would stop it running on dom0.  I don't
> > think running it on dom0 is a good idea; it might fight with libxl.

Good point.

> I think Ian's idea was that the admin should know not to run it in dom0,
> and only enable it in driver domains.
> 
> Do you think we should do something to actively prevent it running in
> dom0?  Do we have a good way to determine whether we're in dom0 or not?
>  (Or more apropos, whether we're running in the toolstack domain?)

Actually, when I cribbed from xencommons I removed the test for "am I in
dom0". I think what I should have done was invert it...

I don't know of an equivalent test for the *BSDs.

Ian.

[PATCH for-4.6 v2] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[Ian Campbell]

The removal of the udev rules highlighted that although it has been
replaced by "xl devd" there isn't an initscript to replace it.

To enable this add a --pidfile option to xl devd.

Tested on Linux by running the script in dom0 and checking the daemon
was started/stopped, but not in an actual driver domain environment
since I don't have one conveniently available. I also checked that
running without the --pidfile option still works.

Scripts mainly cribbed from the xencommons for each platform.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
---
For 4.6: I think having removed the udev rules we ought to include
this, it's basically zero risk to normal operation.

Nothing seems to start xl devd for domain 0 in *BSD, nor is xenbackend
started. Is that correct?

v2: Check in Linux script that we don't start on dom0. I don't know
the equivalent for *BSD.
---
 .gitignore                                    |  3 +
 tools/configure                               |  5 +-
 tools/configure.ac                            |  3 +
 tools/hotplug/FreeBSD/Makefile                |  2 +-
 tools/hotplug/FreeBSD/rc.d/xendriverdomain.in | 48 +++++++++++++++
 tools/hotplug/Linux/Makefile                  |  3 +
 tools/hotplug/Linux/init.d/xendriverdomain.in | 85 +++++++++++++++++++++++++++
 tools/hotplug/NetBSD/Makefile                 |  2 +-
 tools/hotplug/NetBSD/rc.d/xendriverdomain.in  | 49 +++++++++++++++
 tools/libxl/xl_cmdimpl.c                      | 42 +++++++++++--
 tools/libxl/xl_cmdtable.c                     |  3 +-
 11 files changed, 237 insertions(+), 8 deletions(-)
 create mode 100644 tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
 create mode 100644 tools/hotplug/Linux/init.d/xendriverdomain.in
 create mode 100644 tools/hotplug/NetBSD/rc.d/xendriverdomain.in

diff --git a/.gitignore b/.gitignore
index 464f3f4..f6ddb00 100644
--- a/.gitignore
+++ b/.gitignore
@@ -133,10 +133,12 @@ tools/flask/utils/flask-set-bool
 tools/flask/utils/flask-label-pci
 tools/hotplug/common/hotplugpath.sh
 tools/hotplug/FreeBSD/rc.d/xencommons
+tools/hotplug/FreeBSD/rc.d/xendriverdomain
 tools/hotplug/Linux/init.d/sysconfig.xencommons
 tools/hotplug/Linux/init.d/xen-watchdog
 tools/hotplug/Linux/init.d/xencommons
 tools/hotplug/Linux/init.d/xendomains
+tools/hotplug/Linux/init.d/xendriverdomain
 tools/hotplug/Linux/systemd/*.conf
 tools/hotplug/Linux/systemd/*.mount
 tools/hotplug/Linux/systemd/*.socket
@@ -146,6 +148,7 @@ tools/hotplug/Linux/xen-backend.rules
 tools/hotplug/Linux/xen-hotplug-common.sh
 tools/hotplug/Linux/xendomains
 tools/hotplug/NetBSD/rc.d/xencommons
+tools/hotplug/NetBSD/rc.d/xendriverdomain
 tools/include/xen/*
 tools/include/xen-xsm/*
 tools/include/xen-foreign/*.(c|h|size)
diff --git a/tools/configure b/tools/configure
index 5138f3d..d90db47 100755
--- a/tools/configure
+++ b/tools/configure
@@ -2403,7 +2403,7 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
 
 
 
-ac_config_files="$ac_config_files ../config/Tools.mk hotplug/FreeBSD/rc.d/xencommons hotplug/Linux/init.d/sysconfig.xencommons hotplug/Linux/init.d/xen-watchdog hotplug/Linux/init.d/xencommons hotplug/Linux/init.d/xendomains hotplug/Linux/vif-setup hotplug/Linux/xen-hotplug-common.sh hotplug/Linux/xendomains hotplug/NetBSD/rc.d/xencommons libxl/xenlight.pc.in libxl/xlutil.pc.in"
+ac_config_files="$ac_config_files ../config/Tools.mk hotplug/FreeBSD/rc.d/xencommons hotplug/FreeBSD/rc.d/xendriverdomain hotplug/Linux/init.d/sysconfig.xencommons hotplug/Linux/init.d/xen-watchdog hotplug/Linux/init.d/xencommons hotplug/Linux/init.d/xendomains hotplug/Linux/init.d/xendriverdomain hotplug/Linux/vif-setup hotplug/Linux/xen-hotplug-common.sh hotplug/Linux/xendomains hotplug/NetBSD/rc.d/xencommons hotplug/NetBSD/rc.d/xendriverdomain libxl/xenlight.pc.in libxl/xlutil.pc.in"
 
 ac_config_headers="$ac_config_headers config.h"
 
@@ -10045,14 +10045,17 @@ do
   case $ac_config_target in
     "../config/Tools.mk") CONFIG_FILES="$CONFIG_FILES ../config/Tools.mk" ;;
     "hotplug/FreeBSD/rc.d/xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/FreeBSD/rc.d/xencommons" ;;
+    "hotplug/FreeBSD/rc.d/xendriverdomain") CONFIG_FILES="$CONFIG_FILES hotplug/FreeBSD/rc.d/xendriverdomain" ;;
     "hotplug/Linux/init.d/sysconfig.xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/sysconfig.xencommons" ;;
     "hotplug/Linux/init.d/xen-watchdog") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xen-watchdog" ;;
     "hotplug/Linux/init.d/xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xencommons" ;;
     "hotplug/Linux/init.d/xendomains") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xendomains" ;;
+    "hotplug/Linux/init.d/xendriverdomain") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xendriverdomain" ;;
     "hotplug/Linux/vif-setup") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/vif-setup" ;;
     "hotplug/Linux/xen-hotplug-common.sh") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/xen-hotplug-common.sh" ;;
     "hotplug/Linux/xendomains") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/xendomains" ;;
     "hotplug/NetBSD/rc.d/xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/NetBSD/rc.d/xencommons" ;;
+    "hotplug/NetBSD/rc.d/xendriverdomain") CONFIG_FILES="$CONFIG_FILES hotplug/NetBSD/rc.d/xendriverdomain" ;;
     "libxl/xenlight.pc.in") CONFIG_FILES="$CONFIG_FILES libxl/xenlight.pc.in" ;;
     "libxl/xlutil.pc.in") CONFIG_FILES="$CONFIG_FILES libxl/xlutil.pc.in" ;;
     "config.h") CONFIG_HEADERS="$CONFIG_HEADERS config.h" ;;
diff --git a/tools/configure.ac b/tools/configure.ac
index 8bfdfcb..f6a79c2 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -8,14 +8,17 @@ AC_CONFIG_SRCDIR([libxl/libxl.c])
 AC_CONFIG_FILES([
 ../config/Tools.mk
 hotplug/FreeBSD/rc.d/xencommons
+hotplug/FreeBSD/rc.d/xendriverdomain
 hotplug/Linux/init.d/sysconfig.xencommons
 hotplug/Linux/init.d/xen-watchdog
 hotplug/Linux/init.d/xencommons
 hotplug/Linux/init.d/xendomains
+hotplug/Linux/init.d/xendriverdomain
 hotplug/Linux/vif-setup
 hotplug/Linux/xen-hotplug-common.sh
 hotplug/Linux/xendomains
 hotplug/NetBSD/rc.d/xencommons
+hotplug/NetBSD/rc.d/xendriverdomain
 libxl/xenlight.pc.in
 libxl/xlutil.pc.in
 ])
diff --git a/tools/hotplug/FreeBSD/Makefile b/tools/hotplug/FreeBSD/Makefile
index 8dfc90a..10fce4f 100644
--- a/tools/hotplug/FreeBSD/Makefile
+++ b/tools/hotplug/FreeBSD/Makefile
@@ -6,7 +6,7 @@ XEN_SCRIPTS = vif-bridge
 
 XEN_SCRIPT_DATA =
 
-XEN_RCD_PROG = rc.d/xencommons
+XEN_RCD_PROG = rc.d/xencommons rc.d/xendriverdomain
 
 .PHONY: all
 all:
diff --git a/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
new file mode 100644
index 0000000..25e3edd
--- /dev/null
+++ b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
@@ -0,0 +1,48 @@
+#!/bin/sh
+#
+# PROVIDE: xendriverdomain
+# REQUIRE: DAEMON
+#
+# Should be run in a driver domain, but not in domain 0.
+
+. /etc/rc.subr
+
+. /etc/xen/scripts/hotplugpath.sh
+
+LD_LIBRARY_PATH="${libdir}"
+export LD_LIBRARY_PATH
+
+name="xendriverdomain"
+start_precmd="xendriverdomain_precmd"
+start_cmd="xendriverdomain_startcmd"
+stop_cmd="xendriverdomain_stop"
+extra_commands=""
+
+XLDEVD_PIDFILE="/var/run/xldevd.pid"
+
+xendriverdomain_precmd()
+{
+	:
+}
+
+xendriverdomain_startcmd()
+{
+	printf "Starting xenservices: xl devd."
+
+	${sbindir}/xl devd --pidfile=$XLDEVD_PIDFILE ${XLDEVD_ARGS}
+
+	printf "\n"
+}
+
+xendriverdomain_stop()
+{
+	printf "Stopping xl devd.\n"
+
+	rc_pid=$(check_pidfile ${XLDEVD_PIDFILE} ${sbindir}/xl)
+
+	kill -${sig_stop:-TERM} $rc_pids
+	wait_for_pids $rc_pids
+}
+
+load_rc_config $name
+run_rc_command "$1"
diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
index bc8ee5e..6e10118 100644
--- a/tools/hotplug/Linux/Makefile
+++ b/tools/hotplug/Linux/Makefile
@@ -9,6 +9,8 @@ XENDOMAINS_SYSCONFIG = init.d/sysconfig.xendomains
 XENCOMMONS_INITD = init.d/xencommons
 XENCOMMONS_SYSCONFIG = init.d/sysconfig.xencommons
 
+XENDRIVERDOMAIN_INITD = init.d/xendriverdomain
+
 # Xen script dir and scripts to go there.
 XEN_SCRIPTS = vif-bridge
 XEN_SCRIPTS += vif-route
@@ -53,6 +55,7 @@ install-initd:
 	$(INSTALL_DATA) $(XENDOMAINS_SYSCONFIG) $(DESTDIR)$(SYSCONFIG_DIR)/xendomains
 	$(INSTALL_PROG) $(XENCOMMONS_INITD) $(DESTDIR)$(INITD_DIR)
 	$(INSTALL_DATA) $(XENCOMMONS_SYSCONFIG) $(DESTDIR)$(SYSCONFIG_DIR)/xencommons
+	$(INSTALL_PROG) $(XENDRIVERDOMAIN_INITD) $(DESTDIR)$(INITD_DIR)
 	$(INSTALL_PROG) init.d/xen-watchdog $(DESTDIR)$(INITD_DIR)
 
 .PHONY: install-scripts
diff --git a/tools/hotplug/Linux/init.d/xendriverdomain.in b/tools/hotplug/Linux/init.d/xendriverdomain.in
new file mode 100644
index 0000000..dd5f3a3
--- /dev/null
+++ b/tools/hotplug/Linux/init.d/xendriverdomain.in
@@ -0,0 +1,85 @@
+
+#!/bin/bash
+#
+# xendriverdomain    Script to start services needed in a Xen driver domain
+#
+# NOTE: This initscript is not needed on dom0.
+
+# chkconfig: 2345 70 10
+# description: Starts and stops xen driver domain daemon
+### BEGIN INIT INFO
+# Provides:          xendevd
+# Required-Start:    $syslog $remote_fs
+# Should-Start:
+# Required-Stop:     $syslog $remote_fs
+# Should-Stop:
+# Default-Start:     2 3 5
+# Default-Stop:      0 1 6
+# Short-Description: Start/stop xen driver domain daemon
+# Description:       Starts and stops the daemons neeeded for a xen driver domain
+### END INIT INFO
+
+. @XEN_SCRIPT_DIR@/hotplugpath.sh
+
+xendriverdomain_config=@CONFIG_DIR@/@CONFIG_LEAF_DIR@
+
+test -f $xendriverdomain_config/xendriverdomain && . $xendriverdomain_config/xendriverdomain
+
+XLDEVD_PIDFILE=/var/run/xldevd.pid
+
+# not running in Xen dom0 or domU
+if ! test -d /proc/xen ; then
+	exit 0
+fi
+
+# mount xenfs in dom0 or domU with a pv_ops kernel
+if test "x$1" = xstart && \
+   ! test -f /proc/xen/capabilities && \
+   ! grep '^xenfs ' /proc/mounts >/dev/null;
+then
+	mount -t xenfs xenfs /proc/xen
+fi
+
+# run this script only in domU:
+# no capabilities file in xenlinux domU kernel
+# empty capabilities file in pv_ops domU kernel
+if ! test -f /proc/xen/capabilities || \
+   grep -q "control_d" /proc/xen/capabilities ; then
+	exit 0
+fi
+
+do_start () {
+	echo Starting xl devd...
+	${sbindir}/xl devd --pidfile=$XLDEVD_PIDFILE $XLDEVD_ARGS
+}
+do_stop () {
+        echo Stopping xl devd...
+	if read 2>/dev/null <$XLDEVD_PIDFILE pid; then
+		kill $pid
+		while kill -9 $pid >/dev/null 2>&1; do sleep 0.1; done
+		rm -f $XLDEVD_PIDFILE
+	fi
+}
+
+case "$1" in
+  start)
+	do_start
+	;;
+  stop)
+	do_stop
+	;;
+  reload)
+	echo >&2 'Reload not available; use force-reload'; exit 1
+	;;
+  force-reload|restart)
+        do_stop
+	do_start
+	;;
+  *)
+	# do not advertise unreasonable commands that there is no reason
+	# to use with this device
+	echo $"Usage: $0 {start|stop|restart|force-reload}"
+	exit 1
+esac
+
+exit $?
diff --git a/tools/hotplug/NetBSD/Makefile b/tools/hotplug/NetBSD/Makefile
index 4e609e3..d01aabf 100644
--- a/tools/hotplug/NetBSD/Makefile
+++ b/tools/hotplug/NetBSD/Makefile
@@ -8,7 +8,7 @@ XEN_SCRIPTS += vif-bridge
 XEN_SCRIPTS += vif-ip
 
 XEN_SCRIPT_DATA =
-XEN_RCD_PROG = rc.d/xencommons rc.d/xendomains rc.d/xen-watchdog
+XEN_RCD_PROG = rc.d/xencommons rc.d/xendomains rc.d/xen-watchdog rc.d/xendriverdomain
 
 .PHONY: all
 all:
diff --git a/tools/hotplug/NetBSD/rc.d/xendriverdomain.in b/tools/hotplug/NetBSD/rc.d/xendriverdomain.in
new file mode 100644
index 0000000..5062a71
--- /dev/null
+++ b/tools/hotplug/NetBSD/rc.d/xendriverdomain.in
@@ -0,0 +1,49 @@
+#!/bin/sh
+#
+# PROVIDE: xendriverdomain
+# REQUIRE: DAEMON
+#
+# Should be run in a driver domain, but not in domain 0.
+
+. /etc/rc.subr
+
+DIR=$(dirname "$0")
+. "${DIR}/xen-hotplugpath.sh"
+
+LD_LIBRARY_PATH="${libdir}"
+export LD_LIBRARY_PATH
+
+name="xendriverdomain"
+start_precmd="xendriverdomain_precmd"
+start_cmd="xendriverdomain_startcmd"
+stop_cmd="xendriverdomain_stop"
+extra_commands=""
+
+XLDEVD_PIDFILE="/var/run/xldevd.pid"
+
+xendriverdomain_precmd()
+{
+	:
+}
+
+xendriverdomain_startcmd()
+{
+	printf "Starting xenservices: xl devd."
+
+	${sbindir}/xl devd --pidfile=$XLDEVD_PIDFILE ${XLDEVD_ARGS}
+
+	printf "\n"
+}
+
+xendriverdomain_stop()
+{
+	printf "Stopping xl devd.\n"
+
+	rc_pid=$(check_pidfile ${XLDEVD_PIDFILE} ${sbindir}/xl)
+
+	kill -${sig_stop:-TERM} $rc_pids
+	wait_for_pids $rc_pids
+}
+
+load_rc_config $name
+run_rc_command "$1"
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index df89777..483eb71 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -477,7 +477,7 @@ out:
     flush_stream(fh);
 }
 
-static int do_daemonize(char *name)
+static int do_daemonize(char *name, const char *pidfile)
 {
     char *fullname;
     pid_t child1;
@@ -509,6 +509,31 @@ static int do_daemonize(char *name)
 
     CHK_SYSCALL(daemon(0, 1));
 
+    if (pidfile) {
+        int fd = open(pidfile, O_RDWR | O_CREAT, S_IRUSR|S_IWUSR);
+        char *pid = NULL;
+
+        if (fd == -1) {
+            perror("Unable to open pidfile");
+            exit(1);
+        }
+
+        if (asprintf(&pid, "%ld\n", (long)getpid()) == -1) {
+            perror("Formatting pid");
+            exit(1);
+        }
+
+        if (write(fd, pid, strlen(pid)) < 0) {
+            perror("Writing pid");
+            exit(1);
+        }
+
+        if ( close(fd) < 0 ) {
+            perror("Closing pidfile");
+            exit(1);
+        }
+    }
+
 out:
     return ret;
 }
@@ -2815,7 +2840,7 @@ start:
         char *name;
 
         xasprintf(&name, "xl-%s", d_config.c_info.name);
-        ret = do_daemonize(name);
+        ret = do_daemonize(name, NULL);
         free(name);
         if (ret) {
             ret = (ret == 1) ? domid : ret;
@@ -7984,15 +8009,24 @@ int main_remus(int argc, char **argv)
 int main_devd(int argc, char **argv)
 {
     int ret = 0, opt = 0, daemonize = 1;
+    const char *pidfile = NULL;
+    static const struct option opts[] = {
+        {"pidfile", 1, 0, 'p'},
+        COMMON_LONG_OPTS,
+        {0, 0, 0, 0}
+    };
 
-    SWITCH_FOREACH_OPT(opt, "F", NULL, "devd", 0) {
+    SWITCH_FOREACH_OPT(opt, "Fp:", opts, "devd", 0) {
     case 'F':
         daemonize = 0;
         break;
+    case 'p':
+        pidfile = optarg;
+        break;
     }
 
     if (daemonize) {
-        ret = do_daemonize("xldevd");
+        ret = do_daemonize("xldevd", pidfile);
         if (ret) {
             ret = (ret == 1) ? 0 : ret;
             goto out;
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 54dbecc..0071f12 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -506,7 +506,8 @@ struct cmd_spec cmd_table[] = {
       &main_devd, 0, 1,
       "Daemon that listens for devices and launches backends",
       "[options]",
-      "-F                      Run in the foreground",
+      "-F                      Run in the foreground.\n"
+      "-p, --pidfile [FILE]    Write PID to pidfile when daemonizing.",
     },
 #ifdef LIBXL_HAVE_PSR_CMT
     { "psr-hwinfo",
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH for-4.6 v2] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[Ian Jackson]

Ian Campbell writes ("[PATCH for-4.6 v2] tools/hotplug: Add an initscript to start "xl devd" in a driver domain"):
> The removal of the udev rules highlighted that although it has been
> replaced by "xl devd" there isn't an initscript to replace it.

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

Re: [PATCH for-4.6 v2] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[Wei Liu]

On Thu, Jul 16, 2015 at 05:58:27PM +0100, Ian Campbell wrote:
> The removal of the udev rules highlighted that although it has been
> replaced by "xl devd" there isn't an initscript to replace it.
> 
> To enable this add a --pidfile option to xl devd.
> 
> Tested on Linux by running the script in dom0 and checking the daemon
> was started/stopped, but not in an actual driver domain environment
> since I don't have one conveniently available. I also checked that
> running without the --pidfile option still works.
> 
> Scripts mainly cribbed from the xencommons for each platform.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH for-4.6 v2] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[Wei Liu]

On Thu, Jul 16, 2015 at 05:58:27PM +0100, Ian Campbell wrote:
> The removal of the udev rules highlighted that although it has been
> replaced by "xl devd" there isn't an initscript to replace it.
> 
> To enable this add a --pidfile option to xl devd.
> 
> Tested on Linux by running the script in dom0 and checking the daemon
> was started/stopped, but not in an actual driver domain environment
> since I don't have one conveniently available. I also checked that
> running without the --pidfile option still works.
> 
> Scripts mainly cribbed from the xencommons for each platform.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> For 4.6: I think having removed the udev rules we ought to include
> this, it's basically zero risk to normal operation.
> 
> Nothing seems to start xl devd for domain 0 in *BSD, nor is xenbackend
> started. Is that correct?
> 

With my RM hat on:

Release-acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH for-4.6 v2] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[Ian Campbell]

On Fri, 2015-07-17 at 10:05 +0100, Wei Liu wrote:
> On Thu, Jul 16, 2015 at 05:58:27PM +0100, Ian Campbell wrote:
> > The removal of the udev rules highlighted that although it has been
> > replaced by "xl devd" there isn't an initscript to replace it.
> > 
> > To enable this add a --pidfile option to xl devd.
> > 
> > Tested on Linux by running the script in dom0 and checking the daemon
> > was started/stopped, but not in an actual driver domain environment
> > since I don't have one conveniently available. I also checked that
> > running without the --pidfile option still works.
> > 
> > Scripts mainly cribbed from the xencommons for each platform.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Roger Pau Monné <roger.pau@citrix.com>
> > Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> > Cc: George Dunlap <george.dunlap@eu.citrix.com>
> > ---
> > For 4.6: I think having removed the udev rules we ought to include
> > this, it's basically zero risk to normal operation.
> > 
> > Nothing seems to start xl devd for domain 0 in *BSD, nor is xenbackend
> > started. Is that correct?
> > 
> 
> With my RM hat on:
> 
> Release-acked-by: Wei Liu <wei.liu2@citrix.com>

Applied, thanks,



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH for-4.6 v2] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[Roger Pau Monné]

El 16/07/15 a les 18.58, Ian Campbell ha escrit:
> The removal of the udev rules highlighted that although it has been
> replaced by "xl devd" there isn't an initscript to replace it.
> 
> To enable this add a --pidfile option to xl devd.
> 
> Tested on Linux by running the script in dom0 and checking the daemon
> was started/stopped, but not in an actual driver domain environment
> since I don't have one conveniently available. I also checked that
> running without the --pidfile option still works.
> 
> Scripts mainly cribbed from the xencommons for each platform.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> For 4.6: I think having removed the udev rules we ought to include
> this, it's basically zero risk to normal operation.
> 
> Nothing seems to start xl devd for domain 0 in *BSD, nor is xenbackend
> started. Is that correct?
> 
> v2: Check in Linux script that we don't start on dom0. I don't know
> the equivalent for *BSD.

At least on FreeBSD there's no easy way to know if we are running on
Dom0 from userspace (unless we start using hypercalls with privcmd).

I could add a dummy device (let's say /dev/xen/control) that's only
present if we are running on Dom0, does that sound reasonable?

Another option would be to add a small C program that can use hypercalls
and privcmd to detect if we are running on Dom0 or not, but that seems
over engineering it.

[...]

> diff --git a/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
> new file mode 100644
> index 0000000..25e3edd
> --- /dev/null
> +++ b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
> @@ -0,0 +1,48 @@
> +#!/bin/sh
> +#
> +# PROVIDE: xendriverdomain
> +# REQUIRE: DAEMON
> +#
> +# Should be run in a driver domain, but not in domain 0.
> +
> +. /etc/rc.subr
> +
> +. /etc/xen/scripts/hotplugpath.sh

This should have been:

. @XEN_SCRIPT_DIR@/hotplugpath.sh

I will send a patch ASAP to fix it.

[...]

> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index df89777..483eb71 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -477,7 +477,7 @@ out:
>      flush_stream(fh);
>  }
>  
> -static int do_daemonize(char *name)
> +static int do_daemonize(char *name, const char *pidfile)
>  {
>      char *fullname;
>      pid_t child1;
> @@ -509,6 +509,31 @@ static int do_daemonize(char *name)
>  
>      CHK_SYSCALL(daemon(0, 1));
>  
> +    if (pidfile) {
> +        int fd = open(pidfile, O_RDWR | O_CREAT, S_IRUSR|S_IWUSR);
> +        char *pid = NULL;
> +
> +        if (fd == -1) {
> +            perror("Unable to open pidfile");
> +            exit(1);
> +        }
> +
> +        if (asprintf(&pid, "%ld\n", (long)getpid()) == -1) {
> +            perror("Formatting pid");
> +            exit(1);
> +        }
> +
> +        if (write(fd, pid, strlen(pid)) < 0) {
> +            perror("Writing pid");
> +            exit(1);
> +        }
> +
> +        if ( close(fd) < 0 ) {

<pedantic>
The line above doesn't follow libxl coding style.
</pedantic>

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH for-4.6 v2] tools/hotplug: Add an initscript to start "xl devd" in a driver domain

[Ian Campbell]

On Mon, 2015-07-20 at 16:16 +0200, Roger Pau Monné wrote:
> El 16/07/15 a les 18.58, Ian Campbell ha escrit:
> > The removal of the udev rules highlighted that although it has been
> > replaced by "xl devd" there isn't an initscript to replace it.
> > 
> > To enable this add a --pidfile option to xl devd.
> > 
> > Tested on Linux by running the script in dom0 and checking the daemon
> > was started/stopped, but not in an actual driver domain environment
> > since I don't have one conveniently available. I also checked that
> > running without the --pidfile option still works.
> > 
> > Scripts mainly cribbed from the xencommons for each platform.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Roger Pau Monné <roger.pau@citrix.com>
> > Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> > Cc: George Dunlap <george.dunlap@eu.citrix.com>
> > ---
> > For 4.6: I think having removed the udev rules we ought to include
> > this, it's basically zero risk to normal operation.
> > 
> > Nothing seems to start xl devd for domain 0 in *BSD, nor is xenbackend
> > started. Is that correct?
> > 
> > v2: Check in Linux script that we don't start on dom0. I don't know
> > the equivalent for *BSD.
> 
> At least on FreeBSD there's no easy way to know if we are running on
> Dom0 from userspace (unless we start using hypercalls with privcmd).
> 
> I could add a dummy device (let's say /dev/xen/control) that's only
> present if we are running on Dom0, does that sound reasonable?

FWIW on Linux there is a capabilities "device" (actually procfs thing,
close enough) which contains "control_d" in dom0. Not sure if that is
helpful to follow?

> Another option would be to add a small C program that can use hypercalls
> and privcmd to detect if we are running on Dom0 or not, but that seems
> over engineering it.

Agreed.

> [...]
> 
> > diff --git a/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
> > new file mode 100644
> > index 0000000..25e3edd
> > --- /dev/null
> > +++ b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
> > @@ -0,0 +1,48 @@
> > +#!/bin/sh
> > +#
> > +# PROVIDE: xendriverdomain
> > +# REQUIRE: DAEMON
> > +#
> > +# Should be run in a driver domain, but not in domain 0.
> > +
> > +. /etc/rc.subr
> > +
> > +. /etc/xen/scripts/hotplugpath.sh
> 
> This should have been:
> 
> . @XEN_SCRIPT_DIR@/hotplugpath.sh
> 
> I will send a patch ASAP to fix it.

Sorry, and please do, thanks.

> [...]

> > +        if ( close(fd) < 0 ) {
> 
> <pedantic>
> The line above doesn't follow libxl coding style.
> </pedantic>

Damn, sorry.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 3/6] tools: Add a block-tap script for setting up tapdisks via tap-ctl

[George Dunlap]

The blocktap library isn't really necessary; all the necessary functionality
is available via the tap-ctl binary.

To use:

script=block-tap,vdev=[whatever],target=vhd:/path/to/file.vhd

script=block-tap,vdev=[whatever],target=aio:/path/to/file.raw

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>
CC: Felipe Franciosi <felipe.franciosi@citrix.com>
CC: Dave Scott <Dave.Scott@eu.citrix.com>
CC: Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>
---
 tools/hotplug/Linux/Makefile  |   1 +
 tools/hotplug/Linux/block-tap | 123 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)

diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
index 8bb2316..bc8ee5e 100644
--- a/tools/hotplug/Linux/Makefile
+++ b/tools/hotplug/Linux/Makefile
@@ -23,6 +23,7 @@ XEN_SCRIPTS += xen-hotplug-cleanup
 XEN_SCRIPTS += external-device-migrate
 XEN_SCRIPTS += vscsi
 XEN_SCRIPTS += block-iscsi
+XEN_SCRIPTS += block-tap
 XEN_SCRIPTS += block-drbd-probe
 XEN_SCRIPTS += $(XEN_SCRIPTS-y)
 
diff --git a/tools/hotplug/Linux/block-tap b/tools/hotplug/Linux/block-tap
new file mode 100755
index 0000000..8924792
--- /dev/null
+++ b/tools/hotplug/Linux/block-tap
@@ -0,0 +1,123 @@
+#!/bin/bash -e
+#
+# tapdisk Xen block device hotplug script
+#
+# Author George Dunlap <george.dunlap@eu.citrix.com>
+#
+# Based on block-iscsi by Roger Pau Monné <roger.pau@citrix.com>
+#
+# This program 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 only. with the special
+# exception on linking described in file LICENSE.
+#
+# This program 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.
+#
+# Usage:
+#
+# Target should be specified using the following syntax:
+#
+# script=block-tap,vdev=xvda,target=<type>:<file>
+#
+# Type is either "aio" (for raw files), or "vhd"
+
+dir=$(dirname "$0")
+. "$dir/block-common.sh"
+
+remove_label()
+{
+    echo $1 | sed "s/^\("$2"\)//"
+}
+
+check_tools()
+{
+    if ! command -v tap-ctl > /dev/null 2>&1; then
+        fatal "Unable to find tap-ctl tool"
+    fi
+    modprobe blktap
+    if ! tap-ctl check >& /dev/null ; then
+	fatal "Blocktap kernel module not available"
+    fi
+}
+
+# Sets the following global variables based on the params field passed in as
+# a parameter: type file
+parse_target()
+{
+    params=($(echo "$1" | tr ":" "\n"))
+
+    type=${params[0]}
+    file=${params[1]}
+    if [ -z "$type" ] || [ -z "$file" ]; then
+        fatal "Cannot parse required parameters"
+    fi
+}
+
+# Sets $pid and $minor to point to the device associated with the target
+find_device()
+{
+    local info
+    local param
+
+    if [ -z "$type" ] || [ -z "$file" ]; then
+        fatal "required parameters not set"
+    fi
+
+    info=$(tap-ctl list -t $type -f $file)
+
+    for param in $(echo "$info" | tr "," "\n")
+    do
+        case $param in
+        pid=*)
+            pid=$(remove_label $param "pid=")
+            ;;
+        minor=*)
+            minor=$(remove_label $param "minor=")
+            ;;
+        esac
+    done
+
+    if [ -z "$pid" ] || [ -z "$minor" ]; then
+        fatal "cannot find required parameters"
+    fi
+}
+
+# Attaches the device and writes xenstore backend entries to connect
+# the device
+add()
+{
+    dev=$(tap-ctl create -a $target)
+    write_dev $dev
+}
+
+# Disconnects the device
+remove()
+{
+    find_device
+    do_or_die tap-ctl destroy -p ${pid} -m ${minor} > /dev/null
+}
+
+command=$1
+target=$(xenstore-read $XENBUS_PATH/params || true)
+if [ -z "$target" ]; then
+    fatal "No information about the target"
+fi
+
+parse_target "$target"
+
+check_tools || exit 1
+
+case $command in
+add)
+    add
+    ;;
+remove)
+    remove
+    ;;
+*)
+    exit 1
+    ;;
+esac
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 3/6] tools: Add a block-tap script for setting up tapdisks via tap-ctl

[Wei Liu]

On Mon, Jul 06, 2015 at 11:51:40AM +0100, George Dunlap wrote:
> The blocktap library isn't really necessary; all the necessary functionality
> is available via the tap-ctl binary.
> 
> To use:
> 
> script=block-tap,vdev=[whatever],target=vhd:/path/to/file.vhd
> 
> script=block-tap,vdev=[whatever],target=aio:/path/to/file.raw
> 

Is "scirpt=block-tap" the only change required for current syntax?

Does this still supports deprecated syntax like

  vhd:disk.img,hda,rw

I'm assuming in later patch you will massage relevant bits into
parameter string to this script.

Wei.

Re: [PATCH 3/6] tools: Add a block-tap script for setting up tapdisks via tap-ctl

[Wei Liu]

On Tue, Jul 07, 2015 at 01:03:24PM +0100, Wei Liu wrote:
> On Mon, Jul 06, 2015 at 11:51:40AM +0100, George Dunlap wrote:
> > The blocktap library isn't really necessary; all the necessary functionality
> > is available via the tap-ctl binary.
> > 
> > To use:
> > 
> > script=block-tap,vdev=[whatever],target=vhd:/path/to/file.vhd
> > 
> > script=block-tap,vdev=[whatever],target=aio:/path/to/file.raw
> > 
> 
> Is "scirpt=block-tap" the only change required for current syntax?
> 
> Does this still supports deprecated syntax like
> 
>   vhd:disk.img,hda,rw
> 
> I'm assuming in later patch you will massage relevant bits into
> parameter string to this script.
> 

And yes, you actually did that in later patch.

Acked-by: Wei Liu <wei.liu2@citrix.com>

> Wei.

[PATCH 4/6] libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP

[George Dunlap]

The block-tap script can now do everything needed for libxl; no need
to link against the blktap library.

To do this:

* Set disk->script to "block-tap" and dev to "format:pdev_path" in
  device_disk_add for LIBXL_DISK_BACKEND_TAP

* Remove libxl_blktap2.o and libxl_noblktap2.o and all code depending
  on them

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Dave Scott <Dave.Scott@eu.citrix.com>
CC: Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>
CC: Wen Congyang <wency@cn.fujitsu.com>
CC: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Felipe Franciosi <felipe.franciosi@citrix.com>

Note: This is broken for HVM domains at the moment because all block
scripts are broken for HVM domains.  That is a bug which should be a
blocker for the 4.6 release.  As such, I think there is justification
for checking in this "feature" (enabling use of a 'system' blktap)
with the assumption that the FIXME will go away before the release.

That will technically break bisectability; but only for users of
blktap (which does not include osstest).
---
 tools/libxl/Makefile          |  5 ---
 tools/libxl/libxl.c           | 28 ++++---------
 tools/libxl/libxl_blktap2.c   | 93 -------------------------------------------
 tools/libxl/libxl_device.c    | 14 -------
 tools/libxl/libxl_dm.c        |  5 ++-
 tools/libxl/libxl_internal.h  | 26 ------------
 tools/libxl/libxl_noblktap2.c | 42 -------------------
 7 files changed, 11 insertions(+), 202 deletions(-)

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index cc9c152..43ab243 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -44,11 +44,6 @@ LIBXL_LIBS += $(LIBXL_LIBS-y)
 LIBXLU_LIBS = libxenlight.so
 
 LIBXL_OBJS-y = osdeps.o libxl_paths.o libxl_bootloader.o flexarray.o
-ifeq ($(LIBXL_BLKTAP),y)
-LIBXL_OBJS-y += libxl_blktap2.o
-else
-LIBXL_OBJS-y += libxl_noblktap2.o
-endif
 
 ifeq ($(CONFIG_REMUS_NETBUF),y)
 LIBXL_OBJS-y += libxl_netbuffer.o
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3a83903..2aab0c2 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2469,13 +2469,10 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
         switch (disk->backend) {
             case LIBXL_DISK_BACKEND_PHY:
                 dev = disk->pdev_path;
-
-        do_backend_phy:
-                flexarray_append(back, "params");
-                flexarray_append(back, dev);
-
                 script = libxl__abs_path(gc, disk->script?: "block",
                                          libxl__xen_script_dir_path());
+        do_backend_phy:
+                flexarray_append_pair(back, "params", dev);
                 flexarray_append_pair(back, "script", script);
 
                 /* If the user did not supply a block script then we
@@ -2497,25 +2494,16 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
                 break;
 
             case LIBXL_DISK_BACKEND_TAP:
-                if (dev == NULL) {
-                    dev = libxl__blktap_devpath(gc, disk->pdev_path,
-                                                disk->format);
-                    if (!dev) {
-                        LOG(ERROR, "failed to get blktap devpath for %p\n",
-                            disk->pdev_path);
-                        rc = ERROR_FAIL;
-                        goto out;
-                    }
-                }
-                flexarray_append(back, "tapdisk-params");
-                flexarray_append(back, libxl__sprintf(gc, "%s:%s",
-                    libxl__device_disk_string_of_format(disk->format),
-                    disk->pdev_path));
-
                 /* tap backends with scripts are rejected by
                  * libxl__device_disk_set_backend */
                 assert(!disk->script);
 
+                script = libxl__abs_path(gc, "block-tap",
+                                         libxl__xen_script_dir_path());
+                dev = libxl__sprintf(gc, "%s:%s",
+                             libxl__device_disk_string_of_format(disk->format),
+                             disk->pdev_path);
+ 
                 /* now create a phy device to export the device to the guest */
                 goto do_backend_phy;
             case LIBXL_DISK_BACKEND_QDISK:
diff --git a/tools/libxl/libxl_blktap2.c b/tools/libxl/libxl_blktap2.c
deleted file mode 100644
index 2053403..0000000
--- a/tools/libxl/libxl_blktap2.c
+++ /dev/null
@@ -1,93 +0,0 @@
-/*
- * Copyright (C) 2010      Advanced Micro Devices
- * Author Christoph Egger <Christoph.Egger@amd.com>
- *
- * This program 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 only.
- *
- * This program 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.
- */
-
-#include "libxl_osdeps.h" /* must come before any other headers */
-#include "libxl_internal.h"
-
-#include "tap-ctl.h"
-
-int libxl__blktap_enabled(libxl__gc *gc)
-{
-    const char *msg;
-    return !tap_ctl_check(&msg);
-}
-
-char *libxl__blktap_devpath(libxl__gc *gc,
-                            const char *disk,
-                            libxl_disk_format format)
-{
-    const char *type;
-    char *params, *devname = NULL;
-    tap_list_t tap;
-    int err;
-
-    type = libxl__device_disk_string_of_format(format);
-    err = tap_ctl_find(type, disk, &tap);
-    if (err == 0) {
-        devname = libxl__sprintf(gc, "/dev/xen/blktap-2/tapdev%d", tap.minor);
-        if (devname)
-            return devname;
-    }
-
-    params = libxl__sprintf(gc, "%s:%s", type, disk);
-    err = tap_ctl_create(params, &devname);
-    if (!err) {
-        libxl__ptr_add(gc, devname);
-        return devname;
-    }
-
-    return NULL;
-}
-
-
-int libxl__device_destroy_tapdisk(libxl__gc *gc, const char *params)
-{
-    char *type, *disk;
-    int err;
-    tap_list_t tap;
-
-    type = libxl__strdup(gc, params);
-
-    disk = strchr(type, ':');
-    if (!disk) {
-        LOG(ERROR, "Unable to parse params %s", params);
-        return ERROR_INVAL;
-    }
-
-    *disk++ = '\0';
-
-    err = tap_ctl_find(type, disk, &tap);
-    if (err < 0) {
-        /* returns -errno */
-        LOGEV(ERROR, -err, "Unable to find type %s disk %s", type, disk);
-        return ERROR_FAIL;
-    }
-
-    err = tap_ctl_destroy(tap.id, tap.minor);
-    if (err < 0) {
-        LOGEV(ERROR, -err, "Failed to destroy tap device id %d minor %d",
-              tap.id, tap.minor);
-        return ERROR_FAIL;
-    }
-
-    return 0;
-}
-
-/*
- * Local variables:
- * mode: C
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 8209629..c1ca07b 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -223,11 +223,6 @@ static int disk_try_backend(disk_try_backend_args *a,
                        a->disk->vdev);
             return 0;
         }
-        if (!libxl__blktap_enabled(a->gc)) {
-            LOG(DEBUG, "Disk vdev=%s, backend tap unsuitable because blktap "
-                       "not available", a->disk->vdev);
-            return 0;
-        }
         if (!(a->disk->format == LIBXL_DISK_FORMAT_RAW ||
               a->disk->format == LIBXL_DISK_FORMAT_VHD)) {
             goto bad_format;
@@ -572,8 +567,6 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
 {
     const char *be_path = libxl__device_backend_path(gc, dev);
     const char *fe_path = libxl__device_frontend_path(gc, dev);
-    const char *tapdisk_path = GCSPRINTF("%s/%s", be_path, "tapdisk-params");
-    const char *tapdisk_params;
     xs_transaction_t t = 0;
     int rc;
     uint32_t domid;
@@ -585,10 +578,6 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
         rc = libxl__xs_transaction_start(gc, &t);
         if (rc) goto out;
 
-        /* May not exist if this is not a tap device */
-        rc = libxl__xs_read_checked(gc, t, tapdisk_path, &tapdisk_params);
-        if (rc) goto out;
-
         if (domid == LIBXL_TOOLSTACK_DOMID) {
             /*
              * The toolstack domain is in charge for removing both the
@@ -609,9 +598,6 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
         if (rc < 0) goto out;
     }
 
-    if (tapdisk_params)
-        rc = libxl__device_destroy_tapdisk(gc, tapdisk_params);
-
 out:
     libxl__xs_transaction_abort(gc, &t);
     return rc;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 317a8eb..9ac6029 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -856,8 +856,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
 
                 if (disks[i].backend == LIBXL_DISK_BACKEND_TAP) {
                     format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
-                    pdev_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
-                                                      disks[i].format);
+                    /* FIXME This won't work at the moment, because
+                     * all block scripts are broken for qemu */
+                    pdev_path = disks[i].pdev_path;
                 } else {
                     pdev_path = disks[i].pdev_path;
                 }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 7129aee..0d15397 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1615,32 +1615,6 @@ struct libxl__cpuid_policy {
     char *policy[4];
 };
 
-/*
- * blktap2 support
- */
-
-/* libxl__blktap_enabled:
- *    return true if blktap/blktap2 support is available.
- */
-_hidden int libxl__blktap_enabled(libxl__gc *gc);
-
-/* libxl__blktap_devpath:
- *    Argument: path and disk image as specified in config file.
- *      The type specifies whether this is aio, qcow, qcow2, etc.
- *    returns device path xenstore wants to have. returns NULL
- *      if no device corresponds to the disk.
- */
-_hidden char *libxl__blktap_devpath(libxl__gc *gc,
-                                    const char *disk,
-                                    libxl_disk_format format);
-
-/* libxl__device_destroy_tapdisk:
- *   Destroys any tapdisk process associated with the backend represented
- *   by be_path.
- *   Always logs on failure.
- */
-_hidden int libxl__device_destroy_tapdisk(libxl__gc *gc, const char *params);
-
 _hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
                                    libxl_device_disk *disk,
                                    libxl__device *device);
diff --git a/tools/libxl/libxl_noblktap2.c b/tools/libxl/libxl_noblktap2.c
deleted file mode 100644
index 5a86ed1..0000000
--- a/tools/libxl/libxl_noblktap2.c
+++ /dev/null
@@ -1,42 +0,0 @@
-/*
- * Copyright (C) 2010      Advanced Micro Devices
- * Author Christoph Egger <Christoph.Egger@amd.com>
- *
- * This program 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 only.
- *
- * This program 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.
- */
-
-#include "libxl_osdeps.h" /* must come before any other headers */
-
-#include "libxl_internal.h"
-
-int libxl__blktap_enabled(libxl__gc *gc)
-{
-    return 0;
-}
-
-char *libxl__blktap_devpath(libxl__gc *gc,
-                            const char *disk,
-                            libxl_disk_format format)
-{
-    return NULL;
-}
-
-int libxl__device_destroy_tapdisk(libxl__gc *gc, const char *params)
-{
-    return 0;
-}
-
-/*
- * Local variables:
- * mode: C
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
-- 
1.9.1

Re: [PATCH 4/6] libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP

[Andrew Cooper]

On 06/07/15 11:51, George Dunlap wrote:
> The block-tap script can now do everything needed for libxl; no need
> to link against the blktap library.
>
> To do this:
>
> * Set disk->script to "block-tap" and dev to "format:pdev_path" in
>   device_disk_add for LIBXL_DISK_BACKEND_TAP
>
> * Remove libxl_blktap2.o and libxl_noblktap2.o and all code depending
>   on them
>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Dave Scott <Dave.Scott@eu.citrix.com>
> CC: Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>
> CC: Wen Congyang <wency@cn.fujitsu.com>
> CC: Yang Hongyang <yanghy@cn.fujitsu.com>
> CC: Felipe Franciosi <felipe.franciosi@citrix.com>
>
> Note: This is broken for HVM domains at the moment because all block
> scripts are broken for HVM domains.  That is a bug which should be a
> blocker for the 4.6 release.  As such, I think there is justification
> for checking in this "feature" (enabling use of a 'system' blktap)
> with the assumption that the FIXME will go away before the release.
>
> That will technically break bisectability; but only for users of
> blktap (which does not include osstest).
> ---
>  tools/libxl/Makefile          |  5 ---
>  tools/libxl/libxl.c           | 28 ++++---------
>  tools/libxl/libxl_blktap2.c   | 93 -------------------------------------------
>  tools/libxl/libxl_device.c    | 14 -------
>  tools/libxl/libxl_dm.c        |  5 ++-
>  tools/libxl/libxl_internal.h  | 26 ------------
>  tools/libxl/libxl_noblktap2.c | 42 -------------------
>  7 files changed, 11 insertions(+), 202 deletions(-)
>
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index cc9c152..43ab243 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -44,11 +44,6 @@ LIBXL_LIBS += $(LIBXL_LIBS-y)
>  LIBXLU_LIBS = libxenlight.so
>  
>  LIBXL_OBJS-y = osdeps.o libxl_paths.o libxl_bootloader.o flexarray.o
> -ifeq ($(LIBXL_BLKTAP),y)

You will also want to remove reference to LIBXL_BLKTAP from Rules.mk

~Andrew

Re: [PATCH 4/6] libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP

[George Dunlap]

On Mon, Jul 6, 2015 at 12:01 PM, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> On 06/07/15 11:51, George Dunlap wrote:
>> The block-tap script can now do everything needed for libxl; no need
>> to link against the blktap library.
>>
>> To do this:
>>
>> * Set disk->script to "block-tap" and dev to "format:pdev_path" in
>>   device_disk_add for LIBXL_DISK_BACKEND_TAP
>>
>> * Remove libxl_blktap2.o and libxl_noblktap2.o and all code depending
>>   on them
>>
>> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>> ---
>> CC: Ian Campbell <ian.campbell@citrix.com>
>> CC: Ian Jackson <ian.jackson@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Dave Scott <Dave.Scott@eu.citrix.com>
>> CC: Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>
>> CC: Wen Congyang <wency@cn.fujitsu.com>
>> CC: Yang Hongyang <yanghy@cn.fujitsu.com>
>> CC: Felipe Franciosi <felipe.franciosi@citrix.com>
>>
>> Note: This is broken for HVM domains at the moment because all block
>> scripts are broken for HVM domains.  That is a bug which should be a
>> blocker for the 4.6 release.  As such, I think there is justification
>> for checking in this "feature" (enabling use of a 'system' blktap)
>> with the assumption that the FIXME will go away before the release.
>>
>> That will technically break bisectability; but only for users of
>> blktap (which does not include osstest).
>> ---
>>  tools/libxl/Makefile          |  5 ---
>>  tools/libxl/libxl.c           | 28 ++++---------
>>  tools/libxl/libxl_blktap2.c   | 93 -------------------------------------------
>>  tools/libxl/libxl_device.c    | 14 -------
>>  tools/libxl/libxl_dm.c        |  5 ++-
>>  tools/libxl/libxl_internal.h  | 26 ------------
>>  tools/libxl/libxl_noblktap2.c | 42 -------------------
>>  7 files changed, 11 insertions(+), 202 deletions(-)
>>
>> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
>> index cc9c152..43ab243 100644
>> --- a/tools/libxl/Makefile
>> +++ b/tools/libxl/Makefile
>> @@ -44,11 +44,6 @@ LIBXL_LIBS += $(LIBXL_LIBS-y)
>>  LIBXLU_LIBS = libxenlight.so
>>
>>  LIBXL_OBJS-y = osdeps.o libxl_paths.o libxl_bootloader.o flexarray.o
>> -ifeq ($(LIBXL_BLKTAP),y)
>
> You will also want to remove reference to LIBXL_BLKTAP from Rules.mk

Indeed, there is quite a bit I seem to have missed... I'll add a patch
exising the configure- and make-related bits, separately from both
this patch and the rm -rf tools/blktap2 patch.

 -George

Re: [PATCH 4/6] libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP

[Wei Liu]

On Mon, Jul 06, 2015 at 11:51:41AM +0100, George Dunlap wrote:
> The block-tap script can now do everything needed for libxl; no need
> to link against the blktap library.
> 
> To do this:
> 
> * Set disk->script to "block-tap" and dev to "format:pdev_path" in
>   device_disk_add for LIBXL_DISK_BACKEND_TAP
> 
> * Remove libxl_blktap2.o and libxl_noblktap2.o and all code depending
>   on them
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Dave Scott <Dave.Scott@eu.citrix.com>
> CC: Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>
> CC: Wen Congyang <wency@cn.fujitsu.com>
> CC: Yang Hongyang <yanghy@cn.fujitsu.com>
> CC: Felipe Franciosi <felipe.franciosi@citrix.com>
> 
> Note: This is broken for HVM domains at the moment because all block
> scripts are broken for HVM domains.  That is a bug which should be a
> blocker for the 4.6 release.  As such, I think there is justification

This bug is partially fixed by this particular patch. I think making
things partially work is better than having it not work at all.

> for checking in this "feature" (enabling use of a 'system' blktap)
> with the assumption that the FIXME will go away before the release.
> 

I have the impression that to make QEMU work with block script (the
FIXME) would require us to devise a way to get back the path of block
device node. Am I right? How much work do you envisage is needed to make
that FIXME go away?

On the flip side we're not making anything worse than before so we can
probably get by by writing that down in known issues section of release
note.

Wei.

Re: [PATCH 4/6] libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP

[George Dunlap]

On 07/07/2015 01:29 PM, Wei Liu wrote:
> On Mon, Jul 06, 2015 at 11:51:41AM +0100, George Dunlap wrote:
>> The block-tap script can now do everything needed for libxl; no need
>> to link against the blktap library.
>>
>> To do this:
>>
>> * Set disk->script to "block-tap" and dev to "format:pdev_path" in
>>   device_disk_add for LIBXL_DISK_BACKEND_TAP
>>
>> * Remove libxl_blktap2.o and libxl_noblktap2.o and all code depending
>>   on them
>>
>> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>> ---
>> CC: Ian Campbell <ian.campbell@citrix.com>
>> CC: Ian Jackson <ian.jackson@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Dave Scott <Dave.Scott@eu.citrix.com>
>> CC: Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>
>> CC: Wen Congyang <wency@cn.fujitsu.com>
>> CC: Yang Hongyang <yanghy@cn.fujitsu.com>
>> CC: Felipe Franciosi <felipe.franciosi@citrix.com>
>>
>> Note: This is broken for HVM domains at the moment because all block
>> scripts are broken for HVM domains.  That is a bug which should be a
>> blocker for the 4.6 release.  As such, I think there is justification
> 
> This bug is partially fixed by this particular patch. I think making
> things partially work is better than having it not work at all.
> 
>> for checking in this "feature" (enabling use of a 'system' blktap)
>> with the assumption that the FIXME will go away before the release.
>>
> 
> I have the impression that to make QEMU work with block script (the
> FIXME) would require us to devise a way to get back the path of block
> device node. Am I right? How much work do you envisage is needed to make
> that FIXME go away?

For block scripts where dom0 is the backend, I *think* we should in
theory be able change block-common to store a pathname to the device
node in xenstore and hand it to qemu.  Alternately, we could fish the
existing major:minor numbers out of xenstore and make our own node to
pass to qemu.

> On the flip side we're not making anything worse than before so we can
> probably get by by writing that down in known issues section of release
> note.

We do technically cause a regression at the moment.  Before this patch,
someone who has access to the blktap kernel module, and is using the
in-tree blktap, will be able to use a blktap backend with HVM guests.
Once this patch is accepted, blktap will work for PV guests, but not for
HVM guests -- at least until we fix the bug with local block scripts.

 -George

Re: [PATCH 4/6] libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP

[Ian Campbell]

On Tue, 2015-07-07 at 14:41 +0100, George Dunlap wrote:
> On 07/07/2015 01:29 PM, Wei Liu wrote:
> > On Mon, Jul 06, 2015 at 11:51:41AM +0100, George Dunlap wrote:
> >> The block-tap script can now do everything needed for libxl; no need
> >> to link against the blktap library.
> >>
> >> To do this:
> >>
> >> * Set disk->script to "block-tap" and dev to "format:pdev_path" in
> >>   device_disk_add for LIBXL_DISK_BACKEND_TAP
> >>
> >> * Remove libxl_blktap2.o and libxl_noblktap2.o and all code depending
> >>   on them
> >>
> >> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> >> ---
> >> CC: Ian Campbell <ian.campbell@citrix.com>
> >> CC: Ian Jackson <ian.jackson@citrix.com>
> >> CC: Wei Liu <wei.liu2@citrix.com>
> >> CC: Dave Scott <Dave.Scott@eu.citrix.com>
> >> CC: Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>
> >> CC: Wen Congyang <wency@cn.fujitsu.com>
> >> CC: Yang Hongyang <yanghy@cn.fujitsu.com>
> >> CC: Felipe Franciosi <felipe.franciosi@citrix.com>
> >>
> >> Note: This is broken for HVM domains at the moment because all block
> >> scripts are broken for HVM domains.  That is a bug which should be a
> >> blocker for the 4.6 release.  As such, I think there is justification
> > 
> > This bug is partially fixed by this particular patch. I think making
> > things partially work is better than having it not work at all.
> > 
> >> for checking in this "feature" (enabling use of a 'system' blktap)
> >> with the assumption that the FIXME will go away before the release.
> >>
> > 
> > I have the impression that to make QEMU work with block script (the
> > FIXME) would require us to devise a way to get back the path of block
> > device node. Am I right? How much work do you envisage is needed to make
> > that FIXME go away?
> 
> For block scripts where dom0 is the backend, I *think* we should in
> theory be able change block-common to store a pathname to the device
> node in xenstore and hand it to qemu.  Alternately, we could fish the
> existing major:minor numbers out of xenstore and make our own node to
> pass to qemu.
> 
> > On the flip side we're not making anything worse than before so we can
> > probably get by by writing that down in known issues section of release
> > note.
> 
> We do technically cause a regression at the moment.  Before this patch,
> someone who has access to the blktap kernel module, and is using the
> in-tree blktap, will be able to use a blktap backend with HVM guests.
> Once this patch is accepted, blktap will work for PV guests, but not for
> HVM guests -- at least until we fix the bug with local block scripts.

Unfortunately that does sound like something which needs to be fixed
before we accept this patch, doesn't it?

Re: [PATCH 4/6] libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP

[George Dunlap]

On 07/07/2015 03:20 PM, Ian Campbell wrote:
> On Tue, 2015-07-07 at 14:41 +0100, George Dunlap wrote:
>> On 07/07/2015 01:29 PM, Wei Liu wrote:
>>> On Mon, Jul 06, 2015 at 11:51:41AM +0100, George Dunlap wrote:
>>>> The block-tap script can now do everything needed for libxl; no need
>>>> to link against the blktap library.
>>>>
>>>> To do this:
>>>>
>>>> * Set disk->script to "block-tap" and dev to "format:pdev_path" in
>>>>   device_disk_add for LIBXL_DISK_BACKEND_TAP
>>>>
>>>> * Remove libxl_blktap2.o and libxl_noblktap2.o and all code depending
>>>>   on them
>>>>
>>>> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>>>> ---
>>>> CC: Ian Campbell <ian.campbell@citrix.com>
>>>> CC: Ian Jackson <ian.jackson@citrix.com>
>>>> CC: Wei Liu <wei.liu2@citrix.com>
>>>> CC: Dave Scott <Dave.Scott@eu.citrix.com>
>>>> CC: Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>
>>>> CC: Wen Congyang <wency@cn.fujitsu.com>
>>>> CC: Yang Hongyang <yanghy@cn.fujitsu.com>
>>>> CC: Felipe Franciosi <felipe.franciosi@citrix.com>
>>>>
>>>> Note: This is broken for HVM domains at the moment because all block
>>>> scripts are broken for HVM domains.  That is a bug which should be a
>>>> blocker for the 4.6 release.  As such, I think there is justification
>>>
>>> This bug is partially fixed by this particular patch. I think making
>>> things partially work is better than having it not work at all.
>>>
>>>> for checking in this "feature" (enabling use of a 'system' blktap)
>>>> with the assumption that the FIXME will go away before the release.
>>>>
>>>
>>> I have the impression that to make QEMU work with block script (the
>>> FIXME) would require us to devise a way to get back the path of block
>>> device node. Am I right? How much work do you envisage is needed to make
>>> that FIXME go away?
>>
>> For block scripts where dom0 is the backend, I *think* we should in
>> theory be able change block-common to store a pathname to the device
>> node in xenstore and hand it to qemu.  Alternately, we could fish the
>> existing major:minor numbers out of xenstore and make our own node to
>> pass to qemu.
>>
>>> On the flip side we're not making anything worse than before so we can
>>> probably get by by writing that down in known issues section of release
>>> note.
>>
>> We do technically cause a regression at the moment.  Before this patch,
>> someone who has access to the blktap kernel module, and is using the
>> in-tree blktap, will be able to use a blktap backend with HVM guests.
>> Once this patch is accepted, blktap will work for PV guests, but not for
>> HVM guests -- at least until we fix the bug with local block scripts.
> 
> Unfortunately that does sound like something which needs to be fixed
> before we accept this patch, doesn't it?

Since the bug is in block scripts in general, and should (I think) be a
blocker, it can go in after the feature freeze.  This being a new
feature, it can't (or at least, that's how I was doing my calculations).
 So I didn't want to block the thing I can't work on during the feature
freeze for the thing I can.

On the other hand, there's always a risk that the fix will be somewhat
complicated, and we may decide that since nobody has been shouting about
it so far, we could just put the fix off until 4.6.1 -- or at least, if
we don't have any visible regressions, we could do that.  Checking this
in might burn our bridges somewhat.

I think that risk is small, so I'm still in favor of checking it in, but
I could certainly see the the argument against it.

 -George

[PATCH 5/6] tools: Remove in-tree blktap2

[George Dunlap]

Now that libxl has been configured to use a block script for tapdisk,
we can remove the in-tree blktap and let the user build an out-of-tree
blktap.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Dave Scott <Dave.Scott@eu.citrix.com>
CC: Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>
CC: Wen Congyang <wency@cn.fujitsu.com>
CC: Yang Hongyang <yanghy@cn.fujitsu.com>
---

[Patch trimmed]

Re: [PATCH 5/6] tools: Remove in-tree blktap2

[Wei Liu]

On Mon, Jul 06, 2015 at 11:51:42AM +0100, George Dunlap wrote:
> Now that libxl has been configured to use a block script for tapdisk,
> we can remove the in-tree blktap and let the user build an out-of-tree
> blktap.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> 

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Dave Scott <Dave.Scott@eu.citrix.com>
> CC: Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>
> CC: Wen Congyang <wency@cn.fujitsu.com>
> CC: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
> 
> [Patch trimmed]

[PATCH 6/6] libxl: Add more logging to hotplug script path

[George Dunlap]

This was useful in tracking down bugs.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_device.c | 20 +++++++++++++++++---
 tools/libxl/libxl_linux.c  |  5 +++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index c1ca07b..d2ae42f 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -920,11 +920,13 @@ static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
     libxl__ao_device *aodev = CONTAINER_OF(ds, *aodev, backend_ds);
     STATE_AO_GC(aodev->ao);
 
+    LOG(DEBUG, "calling device_backend_cleanup");
     device_backend_cleanup(gc, aodev);
 
     if (rc == ERROR_TIMEDOUT &&
         aodev->action == LIBXL__DEVICE_ACTION_REMOVE &&
         !aodev->force) {
+        LOG(DEBUG, "Timeout reached, initiating forced remove");
         aodev->force = 1;
         libxl__initiate_device_remove(egc, aodev);
         return;
@@ -967,10 +969,18 @@ static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev)
      * hotplug scripts
      */
     rc = libxl__get_domid(gc, &domid);
-    if (rc) goto out;
+    if (rc) {
+        LOG(ERROR, "Failed to get domid");
+        goto out;
+    }
     if (aodev->dev->backend_domid != domid) {
-        if (aodev->action != LIBXL__DEVICE_ACTION_REMOVE)
+        LOG(DEBUG, "Backend domid %d, domid %d, assuming driver domains", 
+            aodev->dev->backend_domid, domid);
+
+        if (aodev->action != LIBXL__DEVICE_ACTION_REMOVE) {
+            LOG(DEBUG, "Not a remove, not executing hotplug scripts");
             goto out;
+        }
 
         aodev->xswait.ao = ao;
         aodev->xswait.what = "removal of backend path";
@@ -978,8 +988,11 @@ static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev)
         aodev->xswait.timeout_ms = LIBXL_DESTROY_TIMEOUT * 1000;
         aodev->xswait.callback = device_destroy_be_watch_cb;
         rc = libxl__xswait_start(gc, &aodev->xswait);
-        if (rc)
+        if (rc) {
+            LOG(ERROR, "Setup of backend removal watch failed (path %s)", be_path);
             goto out;
+        }
+
         return;
     }
 
@@ -991,6 +1004,7 @@ static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev)
     switch (hotplug) {
     case 0:
         /* no hotplug script to execute */
+        LOG(DEBUG, "No hotplug script to execute");
         goto out;
     case 1:
         /* execute hotplug script */
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 512d2c9..4fbcba1 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -214,6 +214,7 @@ static int libxl__hotplug_disk(libxl__gc *gc, libxl__device *dev,
 
     *env = get_hotplug_env(gc, script, dev);
     if (!*env) {
+        LOG(ERROR, "Failed to get hotplug environment");
         rc = ERROR_FAIL;
         goto error;
     }
@@ -225,6 +226,7 @@ static int libxl__hotplug_disk(libxl__gc *gc, libxl__device *dev,
     (*args)[nr++] = NULL;
     assert(nr == arraysize);
 
+    LOG(DEBUG, "Args and environment ready");
     rc = 1;
 
 error:
@@ -241,6 +243,7 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
     switch (dev->backend_kind) {
     case LIBXL__DEVICE_KIND_VBD:
         if (num_exec != 0) {
+            LOG(DEBUG, "num_exec %d, not running hotplug scripts\n", num_exec);
             rc = 0;
             goto out;
         }
@@ -253,6 +256,7 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
          */
         if ((num_exec > 1) ||
             (libxl_get_stubdom_id(CTX, dev->domid) && num_exec)) {
+            LOG(DEBUG, "num_exec %d, not running hotplug scripts\n", num_exec);
             rc = 0;
             goto out;
         }
@@ -260,6 +264,7 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
         break;
     default:
         /* No need to execute any hotplug scripts */
+        LOG(DEBUG, "backend_kind %d, no need to execute scripts", dev->backend_kind);
         rc = 0;
         break;
     }
-- 
1.9.1

Re: [PATCH 6/6] libxl: Add more logging to hotplug script path

[Wei Liu]

On Mon, Jul 06, 2015 at 11:51:43AM +0100, George Dunlap wrote:
> This was useful in tracking down bugs.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH 0/6] Use system blktap

[Ian Campbell]

On Mon, 2015-07-06 at 11:51 +0100, George Dunlap wrote:

> George Dunlap (6): 
>   libxl: Make local_initiate_attach more rational
>   libxl: Remove linux udev rules
>   tools: Add a block-tap script for setting up tapdisks via tap-ctl
>   libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP
>   tools: Remove in-tree blktap2
>   libxl: Add more logging to hotplug script path

Could some subset of this series safely go in now? I'm thinking in
particular of:
  libxl: Make local_initiate_attach more rational
  libxl: Add more logging to hotplug script path

Which look useful and standalone, assuming the first doesn't need any of
the rest.

I'm not sure if the udev stuff makes sense without the blktap script
stuff (which appears to need a little more discussion if nothing else).
I'll take it if you say it is safe to do so...

Ian.

Re: [PATCH 0/6] Use system blktap

[George Dunlap]

On 07/07/2015 03:21 PM, Ian Campbell wrote:
> On Mon, 2015-07-06 at 11:51 +0100, George Dunlap wrote:
> 
>> George Dunlap (6): 
>>   libxl: Make local_initiate_attach more rational
>>   libxl: Remove linux udev rules
>>   tools: Add a block-tap script for setting up tapdisks via tap-ctl
>>   libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP
>>   tools: Remove in-tree blktap2
>>   libxl: Add more logging to hotplug script path
> 
> Could some subset of this series safely go in now? I'm thinking in
> particular of:
>   libxl: Make local_initiate_attach more rational
>   libxl: Add more logging to hotplug script path
> 
> Which look useful and standalone, assuming the first doesn't need any of
> the rest.
> 
> I'm not sure if the udev stuff makes sense without the blktap script
> stuff (which appears to need a little more discussion if nothing else).
> I'll take it if you say it is safe to do so...

The first three can go in without any regressions, and I think would be
useful.  The udev stuff fixes a bug for *all* block scripts --
including, block-iscsi and block-nbd.

 -George

Re: [PATCH 0/6] Use system blktap

[Ian Campbell]

On Tue, 2015-07-07 at 15:24 +0100, George Dunlap wrote:
> On 07/07/2015 03:21 PM, Ian Campbell wrote:
> > On Mon, 2015-07-06 at 11:51 +0100, George Dunlap wrote:
> > 
> >> George Dunlap (6): 
> >>   libxl: Make local_initiate_attach more rational
> >>   libxl: Remove linux udev rules
> >>   tools: Add a block-tap script for setting up tapdisks via tap-ctl
> >>   libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP
> >>   tools: Remove in-tree blktap2
> >>   libxl: Add more logging to hotplug script path
> > 
> > Could some subset of this series safely go in now? I'm thinking in
> > particular of:
> >   libxl: Make local_initiate_attach more rational
> >   libxl: Add more logging to hotplug script path
> > 
> > Which look useful and standalone, assuming the first doesn't need any of
> > the rest.
> > 
> > I'm not sure if the udev stuff makes sense without the blktap script
> > stuff (which appears to need a little more discussion if nothing else).
> > I'll take it if you say it is safe to do so...
> 
> The first three can go in without any regressions,

ITYM the first two and sixth? i.e.

b1882a4 libxl: Make local_initiate_attach more rational
2ba368d libxl: Remove linux udev rules
eff228d libxl: Add more logging to hotplug script path

So that is what I'm in the middle of applying...

Ian.

Re: [PATCH 0/6] Use system blktap

[George Dunlap]

On 07/07/2015 03:52 PM, Ian Campbell wrote:
> On Tue, 2015-07-07 at 15:24 +0100, George Dunlap wrote:
>> On 07/07/2015 03:21 PM, Ian Campbell wrote:
>>> On Mon, 2015-07-06 at 11:51 +0100, George Dunlap wrote:
>>>
>>>> George Dunlap (6): 
>>>>   libxl: Make local_initiate_attach more rational
>>>>   libxl: Remove linux udev rules
>>>>   tools: Add a block-tap script for setting up tapdisks via tap-ctl
>>>>   libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP
>>>>   tools: Remove in-tree blktap2
>>>>   libxl: Add more logging to hotplug script path
>>>
>>> Could some subset of this series safely go in now? I'm thinking in
>>> particular of:
>>>   libxl: Make local_initiate_attach more rational
>>>   libxl: Add more logging to hotplug script path
>>>
>>> Which look useful and standalone, assuming the first doesn't need any of
>>> the rest.
>>>
>>> I'm not sure if the udev stuff makes sense without the blktap script
>>> stuff (which appears to need a little more discussion if nothing else).
>>> I'll take it if you say it is safe to do so...
>>
>> The first three can go in without any regressions,
> 
> ITYM the first two and sixth? i.e.
> 
> b1882a4 libxl: Make local_initiate_attach more rational
> 2ba368d libxl: Remove linux udev rules
> eff228d libxl: Add more logging to hotplug script path
> 
> So that is what I'm in the middle of applying...

Well actually I meant the first three. :-)

To quote myself from the cover letter:

"The third patch (tools: Add a block-tap script for setting up tapdisks
via tap-ctl) can also be checked in independently from the fourth and
fifth, as it allows someone to disable blktap support in configure but
still use externally-built tap-ctl binaries (albeit with a different
disk string)."

 -George

Re: [PATCH 0/6] Use system blktap

[Ian Campbell]

On Tue, 2015-07-07 at 15:59 +0100, George Dunlap wrote:
> On 07/07/2015 03:52 PM, Ian Campbell wrote:
> > On Tue, 2015-07-07 at 15:24 +0100, George Dunlap wrote:
> >> On 07/07/2015 03:21 PM, Ian Campbell wrote:
> >>> On Mon, 2015-07-06 at 11:51 +0100, George Dunlap wrote:
> >>>
> >>>> George Dunlap (6): 
> >>>>   libxl: Make local_initiate_attach more rational
> >>>>   libxl: Remove linux udev rules
> >>>>   tools: Add a block-tap script for setting up tapdisks via tap-ctl
> >>>>   libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP
> >>>>   tools: Remove in-tree blktap2
> >>>>   libxl: Add more logging to hotplug script path
> >>>
> >>> Could some subset of this series safely go in now? I'm thinking in
> >>> particular of:
> >>>   libxl: Make local_initiate_attach more rational
> >>>   libxl: Add more logging to hotplug script path
> >>>
> >>> Which look useful and standalone, assuming the first doesn't need any of
> >>> the rest.
> >>>
> >>> I'm not sure if the udev stuff makes sense without the blktap script
> >>> stuff (which appears to need a little more discussion if nothing else).
> >>> I'll take it if you say it is safe to do so...
> >>
> >> The first three can go in without any regressions,
> > 
> > ITYM the first two and sixth? i.e.
> > 
> > b1882a4 libxl: Make local_initiate_attach more rational
> > 2ba368d libxl: Remove linux udev rules
> > eff228d libxl: Add more logging to hotplug script path
> > 
> > So that is what I'm in the middle of applying...
> 
> Well actually I meant the first three. :-)
> 
> To quote myself from the cover letter:
> 
> "The third patch (tools: Add a block-tap script for setting up tapdisks
> via tap-ctl) can also be checked in independently from the fourth and
> fifth, as it allows someone to disable blktap support in configure but
> still use externally-built tap-ctl binaries (albeit with a different
> disk string)."

Sorry, I was getting confused between this and the next patch.

I'll apply the three above plus the third one.

Ian.

Re: [PATCH 0/6] Use system blktap

[Ian Campbell]

On Mon, 2015-07-06 at 11:51 +0100, George Dunlap wrote:

> George Dunlap (6): 
>   libxl: Make local_initiate_attach more rational
>   libxl: Remove linux udev rules
>   tools: Add a block-tap script for setting up tapdisks via tap-ctl
[...snip 2 patches...]
>   libxl: Add more logging to hotplug script path

Applied these 4, thanks.