libxl: support custom block hotplug scripts
authorIan Campbell <ian.campbell@citrix.com>
Fri, 3 Aug 2012 10:59:12 +0000 (11:59 +0100)
committerIan Campbell <ian.campbell@citrix.com>
Fri, 3 Aug 2012 10:59:12 +0000 (11:59 +0100)
commit1a3e84bd85c6804668fe32f83c0c2912956b9ff6
tree3f8fe4f4439da80ce6553c66a502a88df503cc17
parent5b4dce05268f74ef8d11b0d5f8a64e18f6f27f5f
libxl: support custom block hotplug scripts

These are provided using the "script=" syntax described in
docs/misc/xl-disk-configuration.txt.

The existing hotplug scripts currently conflate two different
concepts, namely that of making a datapath available in the backend
domain (logging into iSCSI LUNs and the like) and that of actually
connecting that datapath to a Xen backend path (e.g. writing
"physical-device" node in xenstore to bring up blkback).

For this reason the script support implemented here is only supported
in conjunction with backendtype=phy.

Eventually we hope to rework the hotplug scripts to separate the to
concepts, but that is not 4.2 material.

In addition there are some other subtleties:

 - Previously in the blktap case we would add "script = .../blktap" to
   the backend flex array, but then jumped to the PHY case which added
   "script = .../block" too. The block one takes precendence since it
   comes second.

   This was, accidentally, correct. The blktap script is for blktap1
   devices and not blktap2 devices. libxl completely manages the
   blktap2 side of things without resorting to hotplug scripts and
   creates a blkback device directly.  Therefore the "block" script is
   always the correct one to call. Custom script are not supported in
   this context.

 - libxl should not write the "physical-device" node. This is the
   responsibility of the block script. Writing the "physical-device"
   node in libxl basically completely short-cuts the standard block
   hotplug script which uses "physical-device" to know if it has run
   already or not.

   In the case of more complex scripts libxl cannot know the right
   value to write here anyway, in particular the device may not exist
   until after the script is called.

   This change has the side effect of re-enabling the checks for
   device sharing aspect of the default block script, which I have tested
   and which now cause libxl to properly abort now that libxl properly
   checks for hotplug script errors.

   There is no sharing check for blktap2 since even if you reuse the
   same vhd the resulting tap device is different. I would have preferred
   to simply write the "physical-device" node for the blktap2 case but
   the hotplug script infrastructure is not currently setup to handle
   LIBXL__DEVICE_KIND_VBD
   devices without a hotplug script (backendtype phy and tap both end
   up as KIND_VBD). Changing this was more surgery than I was happy doing
   for 4.2 and therefore I have simply hardcoded to the block script for
   the LIBXL_DISK_BACKEND_TAP case.

 - libxl__device_disk_set_backend running against a phy device with a
   script cannot stat the device to check its properties since it may
   not exist until the script is run. Therefore I have special cased
   this in disk_try_backend to simply assume that backend == phy is
   always ok if a script was
   configured.  Similarly the other backend types are always rejected
   if a script was configured.

   Note that the reason for implementing the default script behaviour
   in device_disk_add instead of libxl__device_disk_setdefault is
   because we need to be able to tell when the script was
   user-supplied rather than defaulted by libxl in order to correctly
   implement the above. The setdefault function must be idempotent so
   we cannot simply update disk->script.

   I suspect that for 4.3 a script member should be added to
   libxl__device, this would also help in the case above of handling
   devices with no script in a consistent manner. This is not 4.2
   material.

 - When the block script falls through and shells out to a block-$type
   script it used to pass "$node" however the only place this was
   assigned was in the remove+phy case (in which case it contains the
   file:// derived /dev/loopN device), and in that case the script
   exits without falling through to the block-$type case.

   Since libxl never creates a type other than phy this never happens
   in practice anyway and we now call the correct block-$type script
   directly.  But fix it up anyway since it is confusing.

 - The block-nbd and block-enbd scripts which we supply appear to be
   broken WRT the hotplug calling convention, in that they seem to
   expect a command line parameter (perhaps the $node described above)
   rather than reading the appropriate node from xenstore.

   I rather suspect this was broken by 7774:e2e7f47e6f79 in November
   2005. I think it is safe to say no one is using these scripts! I
   haven't fixed this here. It would be good to track down some working
   scripts and either incorproate them or defer to them in their existing
   home (e.g. if they live somewhere useful like the nbd tools
   package).

 - Added a few block script related entries to check-xl-disk-parse
   from http://backdrift.org/xen-block-iscsi-script-with-multipath-support
   and http://lists.linbit.com/pipermail/drbd-user/2008-September/010221.html /
   http://www.drbd.org/users-guide-emb/s-xen-configure-domu.html (and
   snuck in another interesting empty CDROM case)

   This highlighted two bugs in the libxlu disk parser handling of the
   deprecated "<script>:" prefix:

   - It was failing to prefix with "block-" to construct the actual
     script name

   - The regex for matching iscsi or drdb or e?nbd was incorrect

 - Use libxl__abs_path for the nic script too. Just because the
   existing code nearly tricked me into repeating the mistake

I have tested with a custom block script which uses "lvchange -a" to
dynamically add remove the referenced device (simulates iSCSI
login/logout without requiring me to faff around setting up an iSCSI
target). I also tested on a blktap2 system.

I haven't directly tested anything more complex like iscsi: or nbd:
other than what check-xl-disk-parse exercises.

[ reran flex/bison -iwj ]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
docs/misc/xl-disk-configuration.txt
tools/hotplug/Linux/block
tools/libxl/check-xl-disk-parse
tools/libxl/libxl.c
tools/libxl/libxl_device.c
tools/libxl/libxlu_disk_i.h
tools/libxl/libxlu_disk_l.c
tools/libxl/libxlu_disk_l.h
tools/libxl/libxlu_disk_l.l