Re: locked hotplug scripts

[Christoph Badura <bad%bsd.de@localhost>]

On Wed, Mar 30, 2016 at 11:23:52AM +0200, Manuel Bouyer wrote:
> > I think you don't need to lock at all when unconfiguring (case 6).
> > In the 2) case, I think you only need locking in the file) case.
> > The xenstore-write probably can be done unlocked.
> 
> I've installed the attached script on my test server. This is the dom0
> doing nightly anita xen runs, so we should have several domains
> creation/shutdown per day, with 2 file-backed virtual disks.

I appreciate the effort.  But I would like to see the tests done with
more concurrency in the vnd allocation.  Could you add more file-backed
virtual disks to the domUs?  They can be small.  And if you are using
xl to create the domUs you can re-use a single file.

Also, for testing the locking implementation it is better to claim the
locks longer so that we have more contention for the lock.

I've done my tests with 4, 5, and 10 virtual disks.  In part to check
that the problems in port-xen/50659 have gone away.

I also did tests with a faster version of our "block" hotplug script, if
you're up for more tests.  Not adapted to your locking scheme yet.

Personally I think doing the locking as close as possible to the Linux
scripts is the better choice.

--chris

#!/bin/sh -e

# $NetBSD: block-nbsd,v 1.1.1.1 2008/08/07 20:26:57 cegger Exp $
# Called by xenbackendd
# Usage: block xsdir_backend_path state

DIR=$(dirname "$0")
. "${DIR}/hotplugpath.sh"
. "${DIR}/locking.sh"

PATH=${BINDIR}:${SBINDIR}:${LIBEXEC_BIN}:/bin:/usr/bin:/sbin:/usr/sbin
export PATH

error() {
	echo "$@" >&2
	xenstore-write $xpath/hotplug-status error \
	               $xpath/hotplug-error "$@"
	release_lock block
	exit 1
}
	

xpath=$1
xstatus=$2
xparams=$(xenstore-read "$xpath/params")
if [ -b "$xparams" ]; then
	xtype="phy"
elif [ -f "$xparams" ]; then
	xtype="file"
elif [ -z "$xparams" ]; then
	error "$xpath/params is empty, unable to attach block device."
else
	error "$xparams is not a valid file type to use as block device." \
	      "Only block and regular image files accepted."
fi

case $xstatus in
6)
	# device removed
	case $xtype in
	file)
		claim_lock block
		vnd=$(xenstore-read "$xpath/vnd" || echo none)
		if [ $vnd != none ]; then
			vnconfig -u $vnd
		fi
		release_lock block
		;;
	phy)
		;;
	*)
		echo "unknown type $xtype" >&2
		;;
	esac
	xenstore-rm $xpath
	exit 0
	;;
2)
	claim_lock block
	case $xtype in
	file)
		# Mark the used vnd(4) devices as ``used''.
		for disk in $(sysctl hw.disknames); do
			case $disk in
			vnd[0-9]*) eval ${disk}d=used ;;
			esac
		done
		# Store the list of available vnd(4) devices in
		#``available_disks''.
		available_disks=$( ( cd /dev && ls vnd[0-9]*d ) | sort -k 1.4n )
		# Configure the first free vnd(4) device.
		for disk in $available_disks; do
			eval status=\${${disk}:-free}
			if [ "$status" = "free" ] && \
			    vnconfig /dev/${disk} $xparams >/dev/null; then
				device=/dev/${disk}
				break	
			fi
		done
		if [ x$device = x ] ; then
			release_lock block
			error "no available vnd device"
		fi
		xenstore-write $xpath/vnd $device
		;;
	phy)
		device=$xparams
		;;
	esac
	physical_device=$(stat -f '%r' "$device")
	xenstore-write $xpath/physical-device $physical_device
	xenstore-write $xpath/hotplug-status connected
	release_lock block
	exit 0
	;;
*)
	exit 0
	;;
esac