[v2] NetBSD hotplug: fix block unconfigure on destroy

Commit Message

Manuel Bouyer Jan. 26, 2021, 10:47 p.m. UTC

When a domain is destroyed, xparams may not be available any more when
the block script is called to unconfigure the vnd.
Check xparam only at configure time, and just unconfigure any vnd present
in the xenstore.

Signed-off-by: Manuel Bouyer <[e-mail]>
Reviewed-by: Roger Pau Monné <[e-mail]>
---
 tools/hotplug/NetBSD/block | 37 ++++++++++++++-----------------------
 1 file changed, 14 insertions(+), 23 deletions(-)

Comments

Roger Pau Monné Jan. 28, 2021, 11:45 a.m. UTC | #1

On Tue, Jan 26, 2021 at 11:47:49PM +0100, Manuel Bouyer wrote:
> When a domain is destroyed, xparams may not be available any more when
> the block script is called to unconfigure the vnd.
> Check xparam only at configure time, and just unconfigure any vnd present
> in the xenstore.

Can you paste the output of `xl -vvv destroy ...` when this happens?
I'm puzzled as to why the nodes would be removed before hotplug script
execution. Could this somehow be done by NetBSD blkback?

Thanks, Roger.

Manuel Bouyer Jan. 29, 2021, 11 a.m. UTC | #2

On Thu, Jan 28, 2021 at 12:45:09PM +0100, Roger Pau Monné wrote:
> On Tue, Jan 26, 2021 at 11:47:49PM +0100, Manuel Bouyer wrote:
> > When a domain is destroyed, xparams may not be available any more when
> > the block script is called to unconfigure the vnd.
> > Check xparam only at configure time, and just unconfigure any vnd present
> > in the xenstore.
> 
> Can you paste the output of `xl -vvv destroy ...` when this happens?
> I'm puzzled as to why the nodes would be removed before hotplug script
> execution. Could this somehow be done by NetBSD blkback?

I'll try. I've seen it mostly while running automated tests, so I'm
not sure I can reliably reproduce it.

Patch

diff --git a/tools/hotplug/NetBSD/block b/tools/hotplug/NetBSD/block
index 2a0516f436..c8b31a7b2b 100644
--- a/tools/hotplug/NetBSD/block
+++ b/tools/hotplug/NetBSD/block
@@ -20,37 +20,28 @@  error() {
 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)
-		vnd=$(xenstore-read "$xpath/vnd" || echo none)
-		if [ $vnd != none ]; then
-			vnconfig -u $vnd
-		fi
-		;;
-	phy)
-		;;
-	*)
-		echo "unknown type $xtype" >&2
-		;;
-	esac
+	vnd=$(xenstore-read "$xpath/vnd" || echo none)
+	if [ $vnd != none ]; then
+		vnconfig -u $vnd
+	fi
 	xenstore-rm $xpath
 	exit 0
 	;;
 2)
+	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 $xtype in
 	file)
 		# Store the list of available vnd(4) devices in