Perry Myers
2008-Jun-28 22:19 UTC
[Ovirt-devel] [PATCH] Merge developer/bundled appliance into a single appliance: ovirt-appliance
The separation of the ovirt appliance image into developer/bundled versions is not necessary, since the only differences between them are in how the host bridge is configured. Developer assumes that you only have a single NIC on your host so you can't have a separate ovirt network managed by the appliance. Bundled assumes you have the second NIC available. These patches remove the separation between bundled/developer. The new appliance is called ovirt-appliance and can be installed with or without a bridge to a real network device. In either case dummy managed nodes are allowed (they can be used in conjunction with the real managed nodes or just ignored in that case) Both build-all.sh and create-wui-appliance.sh have the developer/bundled flags removed, and replaced with -e flag to indicate the ethernet device to use as the bridge. If this is omitted it is assumed that the ovirtbr will not bridge to a physical network. The bridge (if to a physical net) is made persistent in rc.local (eventually libvirt should handle this) If the device to use on the bridge is used already, the script aborts (we don't want to mess up someones eth0 config by accident) Because the appliance name, bridge name and appliance image name have changed (ovirt-appliance, ovirtbr, ovirt-appliance.img) there are sections of the script that clean up the older named appliances. Signed-off-by: Perry Myers <pmyers at redhat.com> --- build-all.sh | 34 ++++----- wui-appliance/create-wui-appliance.sh | 124 ++++++++++++++++++++------------ 2 files changed, 93 insertions(+), 65 deletions(-) diff --git a/build-all.sh b/build-all.sh index 6e27957..d231241 100755 --- a/build-all.sh +++ b/build-all.sh @@ -29,16 +29,15 @@ DEP_RPMS="createrepo httpd kvm libvirt livecd-tools pungi-1.2.18.1" usage() { case $# in 1) warn "$1"; try_h; exit 1;; esac cat <<EOF -Usage: $ME [-w] [-n] [-p init|update] [-s] [-d|-b] [-a] [-c] [-v git|release|none] +Usage: $ME [-w] [-n] [-p init|update] [-s] [-a] [-c] [-v git|release|none] -w: update oVirt WUI RPMs -n: update oVirt Managed Node RPMs -p: update pungi repository (init or update) -s: include SRPMs and produce source ISO - -d: update developer appliance - -b: update bundled appliance - -a: updates all (WUI, Node, App), requires -d or -b + -a: updates all (WUI, Node, Appliance) -c: cleanup old repos (pungi and ovirt) -v: update version type (git, release, none) default is git + -e: ethernet device to use as bridge (i.e. eth1) -h: display this help and exit EOF } @@ -57,19 +56,18 @@ update_wui=0 update_node=0 update_pungi=0 update_app=0 include_src=0 cleanup=0 -app_type version_type=git +bridge err=0 help=0 -while getopts wnp:sdbahcv: c; do +while getopts wnp:sahcv:e: c; do case $c in w) update_wui=1;; n) update_node=1;; p) update_pungi=$OPTARG;; s) include_src=1;; - d) update_app=1; app_type="-v";; - b) update_app=1; app_type="-b";; a) update_wui=1; update_node=1; update_app=1; update_pungi=init;; c) cleanup=1;; + e) bridge=$OPTARG;; v) version_type=$OPTARG;; h) help=1;; '?') err=1; warn "invalid option: \`-$OPTARG'";; @@ -79,7 +77,6 @@ while getopts wnp:sdbahcv: c; do done test $err = 1 && { try_h; exit 1; } test $help = 1 && { usage; exit 0; } -test $update_app = 1 -a -z "$app_type" && usage "Need to specify -d or -b" test $include_src = 1 -a "$update_pungi" = 0 && usage "Need to specify -p when including source" test "$update_pungi" != 0 -a "$update_pungi" != "init" \ @@ -262,17 +259,18 @@ repo --name=ovirt-org --baseurl=http://ovirt.org/repos/ovirt/$F_REL/x86_64 $excl EOF make cp wui-rel-*.ks $OVIRT + + bridge_flag+ if [ -n "$bridge" ]; then + bridge_flag="-e $bridge" + fi + ./create-wui-appliance.sh \ - -t http://$VIRBR/pungi/$F_REL/$ARCH/os \ - -k http://$VIRBR/ovirt/wui-rel-$ARCH.ks $app_type + -t http://$VIRBR/pungi/$F_REL/$ARCH/os \ + -k http://$VIRBR/ovirt/wui-rel-$ARCH.ks \ + $bridge_flag set +x echo "oVirt appliance setup started, check progress with:" - echo -n " virt-viewer " - if [[ "$app_type" == "-b" ]]; then - echo "bundled" - else - echo "developer" - fi - + echo -n " virt-viewer ovirt-appliance" fi diff --git a/wui-appliance/create-wui-appliance.sh b/wui-appliance/create-wui-appliance.sh index 18d7983..d51c59f 100755 --- a/wui-appliance/create-wui-appliance.sh +++ b/wui-appliance/create-wui-appliance.sh @@ -10,36 +10,34 @@ IMGSIZE=6000M ISO IMGDIR_DEFAULT=/var/lib/libvirt/images +NET_SCRIPTS=/etc/sysconfig/network-scripts ARCH_DEFAULT=$(uname -m) +NAME=ovirt-appliance +BRIDGENAME=ovirtbr ARCH=$ARCH_DEFAULT IMGDIR=$IMGDIR_DEFAULT CONSOLE_FLAG=--noautoconsole -# stupid bridge name so that if all of our checks below fail, we will still -# fail the install -BRIDGENAME=failme - usage() { case $# in 1) warn "$1"; try_h; exit 1;; esac cat <<EOF -Usage: $ME [-i install_iso | -t install_tree] [-d image_dir] [-a x86_64|i686] [-k kickstart] -v -b +Usage: $ME [-i install_iso | -t install_tree] [-d image_dir] [-a x86_64|i686] [-k kickstart] [-e eth] -i: location of installation ISO -t: location of installation tree -k: URL of kickstart file for use with installation tree -o: Display virt-viewer window during install (implied by -i option) -d: directory to place virtual disk (default: $IMGDIR_DEFAULT) -a: architecture for the virtual machine (default: $ARCH_DEFAULT) - -v: Install in developer mode (see http://ovirt.org for details) - -b: Install in bundled mode (see http://ovirt.org for details) + -e: ethernet device to use as bridge (i.e. eth1) -h: display this help and exit EOF } err=0 help=0 -devel=0 bundled=0 viewer=0 -while getopts :a:d:i:t:k:ohvb c; do +bridge+while getopts :a:d:i:t:k:ohe: c; do case $c in i) ISO=$OPTARG;; t) TREE=$OPTARG;; @@ -47,9 +45,8 @@ while getopts :a:d:i:t:k:ohvb c; do d) IMGDIR=$OPTARG;; a) ARCH=$OPTARG;; o) CONSOLE_FLAG=;; + e) bridge=$OPTARG;; h) help=1;; - v) devel=1;; - b) bundled=1;; '?') err=1; warn "invalid option: \`-$OPTARG'";; :) err=1; warn "missing argument to \`-$OPTARG' option";; *) err=1; warn "internal error: \`-$OPTARG' not handled";; @@ -83,9 +80,6 @@ else CONSOLE_FLAG fi -test $devel = 1 -a $bundled = 1 && usage "Can only specify one of -v and -b" -test $devel = 0 -a $bundled = 0 && usage "Must specify one of -v or -b" - case $ARCH in i686|x86_64);; *) usage "invalid architecture: \`$ARCH'";; @@ -125,7 +119,7 @@ gen_fake_managed_node() { <emulator>$KVM_BINARY</emulator> <interface type='network'> <mac address='00:16:3e:12:34:$last_mac'/> - <source network='dummybridge'/> + <source network='$BRIDGENAME'/> </interface> <input type='mouse' bus='ps2'/> <graphics type='vnc' port='-1' listen='127.0.0.1'/> @@ -135,14 +129,12 @@ EOF } gen_app() { - local name=$1 - local disk=$2 - local bridge=$3 - local ram=$4 + local disk=$1 + local ram=$2 cat<<EOF <domain type='kvm'> - <name>$name</name> + <name>$NAME</name> <memory>$(( $ram * 1024 ))</memory> <currentMemory>$(( $ram * 1024 ))</currentMemory> <vcpu>1</vcpu> @@ -164,7 +156,7 @@ gen_app() { <source network='default'/> </interface> <interface type='network'> - <source network='$bridge'/> + <source network='$BRIDGENAME'/> </interface> <input type='mouse' bus='ps2'/> <graphics type='vnc' port='-1' listen='127.0.0.1'/> @@ -197,24 +189,35 @@ if [ $CHECK -ne 0 ]; then die "Must have the libvirt, kvm, virt-manager, and virt-viewer packages installed" fi -if [ $devel = 1 ]; then - NAME=developer - BRIDGENAME=dummybridge - - # define the fake managed nodes we will use - for i in `seq 3 5` ; do - virsh destroy node$i >& /dev/null - virsh undefine node$i >& /dev/null - TMPXML=$(mktemp) - gen_fake_managed_node $i > $TMPXML - virsh define $TMPXML - rm $TMPXML - done -elif [ $bundled = 1 ]; then - NAME=bundled - BRIDGENAME=eth1bridge +service libvirtd status > /dev/null 2>&1 || service libvirtd start > /dev/null 2>&1 +chkconfig libvirtd on + +if [ -n "$bridge" ]; then + ifconfig $bridge > /dev/null 2>&1 ; bridge_dev_present=$? + test $bridge_dev_present != 0 && die "$bridge device not present, aborting!" + test -f $NET_SCRIPTS/ifcfg-$bridge && die "$bridge defined in $NET_SCRIPTS, aborting!" fi +# Cleanup to handle older version of script that used these bridge names +{ + virsh net-destroy dummybridge + virsh net-undefine dummybridge + brctl delif eth1bridge eth1 + virsh net-destroy eth1bridge + virsh net-undefine eth1bridge +} > /dev/null 2>&1 + +# define the fake managed nodes we will use. These can be used for both +# developer and bundled, since the bridge name/network config is the same +for i in `seq 3 5` ; do + virsh destroy node$i >& /dev/null + virsh undefine node$i >& /dev/null + TMPXML=$(mktemp) + gen_fake_managed_node $i > $TMPXML + virsh define $TMPXML + rm $TMPXML +done + virsh net-dumpxml $BRIDGENAME >& /dev/null RETVAL=$? if [ $( brctl show | grep -c $BRIDGENAME ) -ne 0 -a $RETVAL -ne 0 ]; then @@ -225,10 +228,19 @@ if [ $( brctl show | grep -c $BRIDGENAME ) -ne 0 -a $RETVAL -ne 0 ]; then exit 1 fi +# Remove old bridge device if it exists +sed -i "/# $BRIDGENAME/d" /etc/rc.d/rc.local +old_bridge=$(brctl show | awk -v BRIDGENAME=$BRIDGENAME '$1~BRIDGENAME {print $4}') +if [ -n "$old_bridge" ]; then + echo "Removing old bridge $old_bridge" + ifconfig $old_bridge down + brctl delif $BRIDGENAME $old_bridge +fi + # TODO when virFileReadAll is fixed for stdin #virsh net-define <(gen_dummy) -virsh net-destroy $BRIDGENAME -virsh net-undefine $BRIDGENAME +virsh net-destroy $BRIDGENAME > /dev/null 2>&1 +virsh net-undefine $BRIDGENAME > /dev/null 2>&1 TMPXML=$(mktemp) || exit 1 gen_bridge $BRIDGENAME > $TMPXML virsh net-define $TMPXML @@ -236,14 +248,32 @@ rm $TMPXML virsh net-start $BRIDGENAME virsh net-autostart $BRIDGENAME -if [ $bundled = 1 ]; then - # unfortunately, these two can't be done by libvirt at the moment, so - # we do them by hand here - # FIXME: how do we make this persistent, so that we survive reboots? - /usr/sbin/brctl addif $BRIDGENAME eth1 - /sbin/ifconfig eth1 up +if [ -n "$bridge" ]; then + # FIXME: unfortunately, these two can't be done by libvirt at the + # moment, so we do them by hand here and persist the config by + # by adding to rc.local + echo "Adding new bridge $bridge" + TMPBRCTL=$(mktemp) || exit 1 + cat > $TMPBRCTL << EOF +brctl addif $BRIDGENAME $bridge # $BRIDGENAME +ifconfig $bridge up # $BRIDGENAME +EOF + chmod a+x $TMPBRCTL + + cat $TMPBRCTL >> /etc/rc.d/rc.local + + $TMPBRCTL + rm $TMPBRCTL fi +# Cleanup to handle older version of script that used these domain names +{ + virsh destroy developer + virsh undefine developer + virsh destroy bundled + virsh undefine bundled +} > /dev/null 2>&1 + IMGNAME=$NAME.img mkdir -p $IMGDIR virsh destroy $NAME > /dev/null 2>&1 @@ -259,8 +289,8 @@ if [ $do_install = 1 ]; then else test ! -r $IMGDIR/$IMGNAME && die "Disk image not found at $IMGDIR/$IMGNAME" - TMPXML=$(mktemp) - gen_app $NAME $IMGDIR/$IMGNAME $BRIDGENAME $RAM > $TMPXML + TMPXML=$(mktemp) || exit 1 + gen_app $IMGDIR/$IMGNAME $RAM > $TMPXML virsh define $TMPXML rm $TMPXML echo "Application defined using disk located at $IMGDIR/$IMGNAME." -- 1.5.5.1
Chris Lalancette
2008-Jun-30 07:00 UTC
[Ovirt-devel] [PATCH] Merge developer/bundled appliance into a single appliance: ovirt-appliance
Perry Myers wrote:> The separation of the ovirt appliance image into developer/bundled versions > is not necessary, since the only differences between them are in how the > host bridge is configured. Developer assumes that you only have a single > NIC on your host so you can't have a separate ovirt network managed by > the appliance. Bundled assumes you have the second NIC available.Yes, very good point. I totally agree; the only real difference is in the underlying setup, so unifying them is good.> > These patches remove the separation between bundled/developer. The new > appliance is called ovirt-appliance and can be installed with or without > a bridge to a real network device. In either case dummy managed nodes > are allowed (they can be used in conjunction with the real managed nodes > or just ignored in that case)Yep, also a good point.> Both build-all.sh and create-wui-appliance.sh have the developer/bundled > flags removed, and replaced with -e flag to indicate the ethernet device > to use as the bridge. If this is omitted it is assumed that the ovirtbr > will not bridge to a physical network. The bridge (if to a physical net) > is made persistent in rc.local (eventually libvirt should handle this) > If the device to use on the bridge is used already, the script aborts > (we don't want to mess up someones eth0 config by accident)Yay! Removal of a command-line switch :). And yes, totally agreed about the eth0 checking; we can't fix it up (and really probably shouldn't blindly override the user's decision for eth0), so just detecting with a good error is the best we can do here.> diff --git a/build-all.sh b/build-all.sh > index 6e27957..d231241 100755 > --- a/build-all.sh > +++ b/build-all.sh > @@ -29,16 +29,15 @@ DEP_RPMS="createrepo httpd kvm libvirt livecd-tools pungi-1.2.18.1" > usage() { > case $# in 1) warn "$1"; try_h; exit 1;; esac > cat <<EOF > -Usage: $ME [-w] [-n] [-p init|update] [-s] [-d|-b] [-a] [-c] [-v git|release|none] > +Usage: $ME [-w] [-n] [-p init|update] [-s] [-a] [-c] [-v git|release|none]Minor nit; you forgot to put the [-e nic] as part of the initial Usage string. <snip>> @@ -57,19 +56,18 @@ update_wui=0 update_node=0 > update_pungi=0 update_app=0 > include_src=0 > cleanup=0 > -app_type> version_type=git > +bridge> err=0 help=0 > -while getopts wnp:sdbahcv: c; do > +while getopts wnp:sahcv:e: c; do > case $c in > w) update_wui=1;; > n) update_node=1;; > p) update_pungi=$OPTARG;; > s) include_src=1;; > - d) update_app=1; app_type="-v";; > - b) update_app=1; app_type="-b";;Not 100% certain of this, but don't you need to set the "update_app" flag somewhere now that you've removed the options that set it? <snip>> +if [ -n "$bridge" ]; then > + ifconfig $bridge > /dev/null 2>&1 ; bridge_dev_present=$? > + test $bridge_dev_present != 0 && die "$bridge device not present, aborting!" > + test -f $NET_SCRIPTS/ifcfg-$bridge && die "$bridge defined in $NET_SCRIPTS, aborting!" > fiUnless I'm missing something (and please point out if I am), this looks to be the only check for already existing networking devices. If this is the only check, I don't think it is strong enough. In particular, if you are doing this over a remote link, and happen to pass eth0 as the device you want to bridge, I think you will still make it by this check and then totally hose up your remote connection. I think we need to check both if the device is present, and if the device is currently configured with an IP address. In that case, we would abort, since we can't make any guarantees about not hosing up the connection. <snip>> +if [ -n "$bridge" ]; then > + # FIXME: unfortunately, these two can't be done by libvirt at the > + # moment, so we do them by hand here and persist the config by > + # by adding to rc.local > + echo "Adding new bridge $bridge" > + TMPBRCTL=$(mktemp) || exit 1 > + cat > $TMPBRCTL << EOF > +brctl addif $BRIDGENAME $bridge # $BRIDGENAME > +ifconfig $bridge up # $BRIDGENAME > +EOF > + chmod a+x $TMPBRCTL > + > + cat $TMPBRCTL >> /etc/rc.d/rc.local > + > + $TMPBRCTL > + rm $TMPBRCTLIs there any reason you need the TMPBRCTL file at all? Can't you just: cat >> /etc/rc.d/rc.local << EOF brctl addif $BRIDGENAME $bridge # $BRIDGENAME ifconfig $bridge up # $BRIDGENAME EOF and be done with it? Chris Lalancette
Perry N. Myers
2008-Jul-01 00:51 UTC
[Ovirt-devel] [PATCH] Merge developer/bundled appliance into a single appliance: ovirt-appliance
Chris Lalancette wrote:> Perry Myers wrote: >> The separation of the ovirt appliance image into developer/bundled versions >> is not necessary, since the only differences between them are in how the >> host bridge is configured. Developer assumes that you only have a single >> NIC on your host so you can't have a separate ovirt network managed by >> the appliance. Bundled assumes you have the second NIC available. > > Yes, very good point. I totally agree; the only real difference is in the > underlying setup, so unifying them is good. > >> These patches remove the separation between bundled/developer. The new >> appliance is called ovirt-appliance and can be installed with or without >> a bridge to a real network device. In either case dummy managed nodes >> are allowed (they can be used in conjunction with the real managed nodes >> or just ignored in that case) > > Yep, also a good point. > >> Both build-all.sh and create-wui-appliance.sh have the developer/bundled >> flags removed, and replaced with -e flag to indicate the ethernet device >> to use as the bridge. If this is omitted it is assumed that the ovirtbr >> will not bridge to a physical network. The bridge (if to a physical net) >> is made persistent in rc.local (eventually libvirt should handle this) >> If the device to use on the bridge is used already, the script aborts >> (we don't want to mess up someones eth0 config by accident) > > Yay! Removal of a command-line switch :). And yes, totally agreed about the > eth0 checking; we can't fix it up (and really probably shouldn't blindly > override the user's decision for eth0), so just detecting with a good error is > the best we can do here. > >> diff --git a/build-all.sh b/build-all.sh >> index 6e27957..d231241 100755 >> --- a/build-all.sh >> +++ b/build-all.sh >> @@ -29,16 +29,15 @@ DEP_RPMS="createrepo httpd kvm libvirt livecd-tools pungi-1.2.18.1" >> usage() { >> case $# in 1) warn "$1"; try_h; exit 1;; esac >> cat <<EOF >> -Usage: $ME [-w] [-n] [-p init|update] [-s] [-d|-b] [-a] [-c] [-v git|release|none] >> +Usage: $ME [-w] [-n] [-p init|update] [-s] [-a] [-c] [-v git|release|none] > > Minor nit; you forgot to put the [-e nic] as part of the initial Usage string.Will fix.> <snip> > >> @@ -57,19 +56,18 @@ update_wui=0 update_node=0 >> update_pungi=0 update_app=0 >> include_src=0 >> cleanup=0 >> -app_type>> version_type=git >> +bridge>> err=0 help=0 >> -while getopts wnp:sdbahcv: c; do >> +while getopts wnp:sahcv:e: c; do >> case $c in >> w) update_wui=1;; >> n) update_node=1;; >> p) update_pungi=$OPTARG;; >> s) include_src=1;; >> - d) update_app=1; app_type="-v";; >> - b) update_app=1; app_type="-b";; > > Not 100% certain of this, but don't you need to set the "update_app" flag > somewhere now that you've removed the options that set it?It's part of the -a flag to the script. -a is to 'rebuild all' and in reality if you're going to rebuild the appliance, you probably should always rebuild the node/wui RPMs, etc.> <snip> > >> +if [ -n "$bridge" ]; then >> + ifconfig $bridge > /dev/null 2>&1 ; bridge_dev_present=$? >> + test $bridge_dev_present != 0 && die "$bridge device not present, aborting!" >> + test -f $NET_SCRIPTS/ifcfg-$bridge && die "$bridge defined in $NET_SCRIPTS, aborting!" >> fi > > Unless I'm missing something (and please point out if I am), this looks to be > the only check for already existing networking devices. If this is the only > check, I don't think it is strong enough. In particular, if you are doing this > over a remote link, and happen to pass eth0 as the device you want to bridge, I > think you will still make it by this check and then totally hose up your remote > connection. I think we need to check both if the device is present, and if the > device is currently configured with an IP address. In that case, we would > abort, since we can't make any guarantees about not hosing up the connection.We talked about this offline, and you were misreading the code. $bridge here refers to eth1, eth2 (i.e. whatever you passed in on -e flag) However you did point out two things here, first is that the check for ifcfg-eth1 may not be sufficient to detect whether or not the designated interface is running. Secondly there may be portability issues with using ifconfig. I'll see if I can refactor these checks to use something like ip. Suggestions from our friends working in other distros would be appreciated here.> <snip> > >> +if [ -n "$bridge" ]; then >> + # FIXME: unfortunately, these two can't be done by libvirt at the >> + # moment, so we do them by hand here and persist the config by >> + # by adding to rc.local >> + echo "Adding new bridge $bridge" >> + TMPBRCTL=$(mktemp) || exit 1 >> + cat > $TMPBRCTL << EOF >> +brctl addif $BRIDGENAME $bridge # $BRIDGENAME >> +ifconfig $bridge up # $BRIDGENAME >> +EOF >> + chmod a+x $TMPBRCTL >> + >> + cat $TMPBRCTL >> /etc/rc.d/rc.local >> + >> + $TMPBRCTL >> + rm $TMPBRCTL > > Is there any reason you need the TMPBRCTL file at all? Can't you just: > > cat >> /etc/rc.d/rc.local << EOF > brctl addif $BRIDGENAME $bridge # $BRIDGENAME > ifconfig $bridge up # $BRIDGENAME > EOFThere are two things that need to be done. First is to start the bridge *now*, second is to put the stuff in rc.local to have it start on reboot. We can't just put it in rc.local and then execute rc.local because we don't know what else is in rc.local and it might be bad to reexecute that file. So rather than duplicating the brctl and ifconfig lines in the script, I write it once to a temp file, put the contents of the temp file in rc.local and then execute the temp file now so that the bridge comes up immediately. Make more sense now? Again, if ifconfig is a bad command to use for portability suggestions for more portable commands would be appreciated. Perry> and be done with it? > > Chris Lalancette-- |=- Red Hat, Engineering, Emerging Technologies, Boston -=| |=- Email: pmyers at redhat.com -=| |=- Office: +1 412 474 3552 Mobile: +1 703 362 9622 -=| |=- GnuPG: E65E4F3D 88F9 F1C9 C2F3 1303 01FE 817C C5D2 8B91 E65E 4F3D -=|