Skip to contentSkip to conversation

fix xendomains service

PATCH New Zhigang Wang 2 participants 9 messages Jun 8, 2011 – Jun 21, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fix xendomains service

This series of patches fix some xendomains service issues.

#1 lore raw

PATCH 1/1 fix state

# HG changeset patch
# User Zhigang Wang <[email protected]> # Date 1307569476 14400
# Node ID be4f09fff517c1973d643635c9f40574a693da1a # Parent 37c77bacb52aa7795978b994f9d371b979b2cb07 fix state

This patch fixes the state problem of xendomains service:

1. Bash regex:

       if [[ "(state -b----)" =~ '(state' ]]; then echo "works"; fi

works on bash 4.x, not 3.x.

Put it into a variable works for both:

       state_re="\(state"; if [[ "(state -b----)" =~ $state_re ]]; then echo "works"; fi

2. Fix: "state" as well as "name" and "id", is used but never defined.

tools/hotplug/Linux/init.d/xendomains +15 -9
diff -r 37c77bacb52a -r be4f09fff517 tools/hotplug/Linux/init.d/xendomains
@@ -204,15 +204,21 @@ rdnames()
 
 parseln()
 {
-    if [[ "$1" =~ '(domain' ]]; then
+    domain_re="\(domain"
+    name_re="\(name"
+    domid_re="\(domid"
+    state_re="\(state"
+    if [[ "$1" =~ $domain_re ]]; then
         name=;id=
-    else if [[ "$1" =~ '(name' ]]; then
+    else if [[ "$1" =~ $name_re ]]; then
         name=$(echo $1 | sed -e 's/^.*(name \(.*\))$/\1/')
-    else if [[ "$1" =~ '(domid' ]]; then
+    else if [[ "$1" =~ $domid_re ]]; then
         id=$(echo $1 | sed -e 's/^.*(domid \(.*\))$/\1/')
-    fi; fi; fi
+    else if [[ "$1" =~ $state_re ]]; then
+        state=$(echo $1 | sed -e 's/^.*(state \(.*\))$/\1/')
+    fi; fi; fi; fi
 
-    [ -n "$name" -a -n "$id" ] && return 0 || return 1
+    [ -n "$name" -a -n "$id" -a -n "$state" ] && return 0 || return 1
 }
 
 is_running()
@@ -228,7 +234,7 @@ is_running()
 		RC=0
 		;;
 	esac
-    done < <($CMD list -l | grep '(\(domain\|domid\|name\)')
+    done < <($CMD list -l | grep '(\(domain\|domid\|name\|state\)')
     return $RC
 }
 
@@ -310,7 +316,7 @@ all_zombies()
 	if test "$state" != "-b---d" -a "$state" != "-----d"; then
 	    return 1;
 	fi
-    done < <($CMD list -l | grep '(\(domain\|domid\|name\)')
+    done < <($CMD list -l | grep '(\(domain\|domid\|name\|state\)')
     return 0
 }
 
@@ -441,7 +447,7 @@ stop()
 	    fi
 	    kill $WDOG_PID >/dev/null 2>&1
 	fi
-    done < <($CMD list -l | grep '(\(domain\|domid\|name\)')
+    done < <($CMD list -l | grep '(\(domain\|domid\|name\|state\)')
 
     # NB. this shuts down ALL Xen domains (politely), not just the ones in
     # AUTODIR/*
@@ -478,7 +484,7 @@ check_domain_up()
 		return 0
 		;;
 	esac
-    done < <($CMD list -l | grep '(\(domain\|domid\|name\)')
+    done < <($CMD list -l | grep '(\(domain\|domid\|name\|state\)')
     return 1
 }
#2 lore raw

Re: [PATCH 1 of 4] fix state

Zhigang Wang writes ("[Xen-devel] [PATCH 1 of 4] fix state"):

tools/hotplug/Linux/init.d/xendomains rdnames() · line 213

Quoted 2 lines
tools/hotplug/Linux/init.d/xendomains +1 -1
-    else if [[ "$1" =~ '(name' ]]; then
+    else if [[ "$1" =~ $name_re ]]; then

This is really shockingly ugly. What would be wrong with using case ?

Since this is a bash-specific script already, we can use ${..#...} ${..##...} ${..%...} ${..%%...} too.

Ian.

#3 lore raw

PATCH 1/1 fix not functional eval

In reply to Zhigang Wang

# HG changeset patch
# User Zhigang Wang <[email protected]> # Date 1307569509 14400
# Node ID 4982815ee816edd44a85c27e885437cfcd227612 # Parent be4f09fff517c1973d643635c9f40574a693da1a fix not functional eval

I don't understand why use eval here. It doesn't work and the case syntax seems not right.

Remove the eval and fix the case syntax works fine.

tools/hotplug/Linux/init.d/xendomains +3 -5
diff -r be4f09fff517 -r 4982815ee816 tools/hotplug/Linux/init.d/xendomains
@@ -365,17 +365,15 @@ stop()
 	if test $id = 0; then continue; fi
 	echo -n " $name"
 	if test "$XENDOMAINS_AUTO_ONLY" = "true"; then
-	    eval "
-	    case \"\$name\" in
-		($NAMES)
+	    case "$name" in
+		$NAMES)
 		    # nothing
 		    ;;
-		(*)
+		*)
 		    echo -e '(skip)'
 		    continue
 		    ;;
 	    esac
-	    "
 	fi
 	# XENDOMAINS_SYSRQ chould be something like just "s" 
 	# or "s e i u" or even "s e s i u o"
#4 lore raw

Re: [PATCH 2 of 4] fix not functional eval

Zhigang Wang writes ("[Xen-devel] [PATCH 2 of 4] fix not functional eval"):

Quoted 4 lines

fix not functional eval

I don't understand why use eval here. It doesn't work and the case syntax seems not right.

You are right that the case syntax is wrong. But the eval is needed because
case "$name" in
$NAMES)
doesn't look into $NAMES to see the |s.

The existing script is a perfectly hideous piece of code.

Ian.

#5 lore raw

PATCH 1/1 make xendomains service save/migrate vms under /etc/xen/auto only

In reply to Zhigang Wang

# HG changeset patch
# User Zhigang Wang <[email protected]>
# Date 1307569509 14400
# Node ID b944852b97898ed11ea10eb9301efadc19ec50ca
# Parent 4982815ee816edd44a85c27e885437cfcd227612
make xendomains service save/migrate vms under /etc/xen/auto only

Currently, xendomains service will save or migrate all the running domains in the host on stop.

Change XENDOMAINS_AUTO_ONLY=true. Then on service stop, it will only save or migrate all VMs under XENDOMAINS_AUTO (/etc/xen/auto), but always shutdown all the running VMs.

It's a way to cleanly shutdown all the running VMs on system reboot. If you want to save/migrate a VM, create a link to /etc/xen/auto.

tools/hotplug/Linux/init.d/sysconfig.xendomains +2 -2
diff -r 4982815ee816 -r b944852b9789 tools/hotplug/Linux/init.d/sysconfig.xendomains
@@ -103,7 +103,7 @@ XENDOMAINS_RESTORE=true
 XENDOMAINS_AUTO=/etc/xen/auto
 
 ## Type: boolean
-## Default: false
+## Default: true
 # 
 # If this variable is set to "true", only the domains started via config 
 # files in XENDOMAINS_AUTO will be treated according to XENDOMAINS_SYSRQ,
@@ -111,7 +111,7 @@ XENDOMAINS_AUTO=/etc/xen/auto
 # all running domains will be. 
 # Note that the name matching is somewhat fuzzy.
 # 
-XENDOMAINS_AUTO_ONLY=false
+XENDOMAINS_AUTO_ONLY=true
 
 ## Type: integer
 ## Default: 300
Applied
#6 lore raw

Re: [PATCH 3 of 4] make xendomains service save/migrate vms under /etc/xen/auto only

Zhigang Wang writes ("[Xen-devel] [PATCH 3 of 4] make xendomains service save/migrate vms under /etc/xen/auto only"):

Quoted 1 line

make xendomains service save/migrate vms under /etc/xen/auto only

Applied, thanks.

Ian.

#7 lore raw

PATCH 1/1 always create a lockfile on xendomains service start

In reply to Zhigang Wang

# HG changeset patch
# User Zhigang Wang <[email protected]> # Date 1307569509 14400
# Node ID ab0a7d100668a25f0e6f025b474694962e2e8aef # Parent b944852b97898ed11ea10eb9301efadc19ec50ca always create a lockfile on xendomains service start

Currently if there's no saved VMs or VMs under /etc/xen/auto, the lockfile will not be created.

If no lockfile, when system reboot, system will not wait xendomains service to stop. Thus any running VMs without a link to /etc/xen/auto will not be cleanly shutdown.

tools/hotplug/Linux/init.d/xendomains +2 -3
diff -r b944852b9789 -r ab0a7d100668 tools/hotplug/Linux/init.d/xendomains
@@ -249,8 +249,6 @@ start()
     if [ "$XENDOMAINS_RESTORE" = "true" ] &&
        contains_something "$XENDOMAINS_SAVE"
     then
-	mkdir -p $(dirname "$LOCKFILE")
-	touch $LOCKFILE
 	echo -n "Restoring Xen domains:"
 	saved_domains=`ls $XENDOMAINS_SAVE`
         for dom in $XENDOMAINS_SAVE/*; do
@@ -276,7 +274,6 @@ start()
 
     if contains_something "$XENDOMAINS_AUTO"
     then
-	touch $LOCKFILE
 	echo -n "Starting auto Xen domains:"
 	# We expect config scripts for auto starting domains to be in
 	# XENDOMAINS_AUTO - they could just be symlinks to files elsewhere
@@ -305,6 +302,8 @@ start()
 	    fi
 	done
     fi
+	mkdir -p $(dirname "$LOCKFILE")
+	touch $LOCKFILE
 }
 
 all_zombies()
#8 lore raw

Re: [PATCH 4 of 4] always create a lockfile on xendomains service start

Zhigang Wang writes ("[Xen-devel] [PATCH 4 of 4] always create a lockfile on xendomains service start"):

Quoted 1 line

always create a lockfile on xendomains service start

What does this do if the system is booted with a non-xen-capable kernel ?

Ian.