[Ovirt-devel] [PATCH trivial] Create developer appliances under Ubuntu v2

[Jim Meyering]

"Jeff Schroeder" <jeffschroed at gmail.com> wrote:
> On Fri, Jun 27, 2008 at 4:13 PM, Jeff Schroeder <jeffschroed at gmail.com> wrote:
> ...snip...
>> An updated patch is attached that hopefully addresses your concerns.
>
> Ok I changed something after testing the previous patch and flipped
> the logic for some
> stupid reason. This makes more sense and fixes the package dependency check.
>
> Please ignore the last email and take a look at this patch instead.
>
>
> diff --git a/wui-appliance/create-wui-appliance.sh b/wui-appliance/create-wui-appliance.sh
> index 18d7983..fdf1206 100755
> --- a/wui-appliance/create-wui-appliance.sh
> +++ b/wui-appliance/create-wui-appliance.sh
> @@ -122,7 +122,7 @@ gen_fake_managed_node() {
>    <on_reboot>restart</on_reboot>
>    <on_crash>destroy</on_crash>
>    <devices>
> -    <emulator>$KVM_BINARY</emulator>
> +    <emulator>$KVM</emulator>
>      <interface type='network'>
>        <mac address='00:16:3e:12:34:$last_mac'/>
>        <source network='dummybridge'/>
> @@ -155,7 +155,7 @@ gen_app() {
>    <on_reboot>restart</on_reboot>
>    <on_crash>destroy</on_crash>
>    <devices>
> -    <emulator>$KVM_BINARY</emulator>
> +    <emulator>$KVM</emulator>
>      <disk type='file' device='disk'>
>        <source file='$disk'/>
>        <target dev='hda'/>
> @@ -180,23 +180,21 @@ fi
>
>  # now make sure the packages we need are installed
>  if [ -e /etc/redhat-release ]; then
> -    PACKAGES="libvirt kvm virt-manager virt-viewer"
> -    CHECK=$(rpm $(printf " -q %s " "$PACKAGES")  &> /dev/null; echo $?)
> -    KVM_BINARY=/usr/bin/qemu-kvm
> +    PACKAGES='libvirt kvm virt-manager virt-viewer'
> +    rpm $(printf " -q %s " "$PACKAGES") > /dev/null 2>&1 && DEPS_MET=1
> +    KVM=/usr/bin/qemu-kvm
>  elif [ -e /etc/debian_version ]; then
>      # Works in Ubuntu 8.04. Still needs testing in Debian
> -    PACKAGES="libvirt0 libvirt-bin kvm qemu virt-manager virt-viewer"
> -    CHECK=$(dpkg -l $PACKAGES &> /dev/null; echo $?)
> -    KVM_BINARY=/usr/bin/kvm
> +    PACKAGES='libvirt0 libvirt-bin kvm qemu virt-manager virt-viewer'
> +    dpkg -l $PACKAGES > /dev/null 2>&1 && DEPS_MET=1

The idiom I suggested, "; some_var=$?", guaranteed that
$some_var would be defined.
Since you've opted to use " && DEPS_MET=1",
it's your responsibility to define DEPS_MET when they're not met.
Otherwise, if DEPS_MET=1 happens to be set in the environment
(yeah, not likely), it will stay that way, even when the dependencies
are not met.

> +    KVM=/usr/bin/kvm
>  else
>      die "Not a supported system"
>  fi
> -
> -if [ $CHECK -ne 0 ]; then
> +if [ ${DEPS_MET:-0} -ne 1 ]; then

Then you can use a cleaner comparison:

  if [ $DEPS_MET = 0 ]; then

By the way, please don't use "-ne" for arithmetic comparisons.
"=" works fine and its shorter and more readable.

>      # one of the previous packages wasn't installed; bail out
> -    die "Must have the libvirt, kvm, virt-manager, and virt-viewer packages installed"
> +    die "Missing a package from this list: $PACKAGES"
>  fi
> -
>  if [ $devel = 1 ]; then
>      NAME=developer
>      BRIDGENAME=dummybridge