|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] fix xendomains service
On Tue, 2011-06-07 at 22:02 +0100, Zhigang Wang wrote:
> # HG changeset patch
> # User Zhigang Wang <zhigang.x.wang@xxxxxxxxxx>
> # Date 1307480010 14400
> # Node ID 72a098061cbd894976b0bf9e0f96cc1e1c2897b5
> # Parent 37c77bacb52aa7795978b994f9d371b979b2cb07
> fix xendomains service
>
> This patch fixes the following problems of xendomains service:
In general we would prefer each unrelated change in an individual patch.
If your commit message is a list of unrelated issues then that is a good
sign you need to consider splitting them up.
>
> 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
This is pretty nasty looking in both cases, but OK, I guess we don't
have anything much better available.
> 2. 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.
Why is this change in default behaviour desirable? What makes it better
in the general case than the existing default?
> 3. Fix: "state" as well as "name" and "id", is used but never defined.
OK.
> 4. Remove the incorrect eval.
Which incorrect eval? (this might be obvious if it was a separate
changeset but it is hidden in the rest of the changes in this changes).
How is it incorrect and what makes it correct now?
> Signed-off-by: Zhigang Wang <zhigang.x.wang@xxxxxxxxxx>
>
> diff -r 37c77bacb52a -r 72a098061cbd
> tools/hotplug/Linux/init.d/sysconfig.xendomains
> --- a/tools/hotplug/Linux/init.d/sysconfig.xendomains Mon May 23 17:38:28
> 2011 +0100
> +++ b/tools/hotplug/Linux/init.d/sysconfig.xendomains Tue Jun 07 16:53:30
> 2011 -0400
> @@ -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
> diff -r 37c77bacb52a -r 72a098061cbd tools/hotplug/Linux/init.d/xendomains
> --- a/tools/hotplug/Linux/init.d/xendomains Mon May 23 17:38:28 2011 +0100
> +++ b/tools/hotplug/Linux/init.d/xendomains Tue Jun 07 16:53:30 2011 -0400
> @@ -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
> }
>
> @@ -243,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
> @@ -270,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
> @@ -299,6 +302,8 @@ start()
> fi
> done
> fi
> + mkdir -p $(dirname "$LOCKFILE")
> + touch $LOCKFILE
> }
>
> all_zombies()
> @@ -310,7 +315,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
> }
>
> @@ -359,17 +364,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"
> @@ -441,7 +444,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 +481,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
> }
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |