----- "Lucas Meneghel Rodrigues" <l...@redhat.com> wrote: > I am taking some time to review your patches, and likewise you > mentioned revising my unattended patchset, it's going to take > sometime > for me to go trough all the code. Starting with the low hanging > fruit, > this little setup script could be turned into a python script as > well!
qemu-ifup is a traditional qemu script. The one in this patch is almost identical to the ones included in KVM releases. Also, it's meant to be modified by the user -- the user may want to replace the 'brctl show | awk' expression with the name of a bridge, especially if the host has more than one. I think a python script will be awkward to modify. Also, traditionally this script resides in /etc, and this one is provided only in case the user doesn't have a better one in /etc. The script in /etc is normally a bash script. I have no problem with rewriting this as a python script -- I just think it's more natural to keep this one in bash. In python it would look something like: import sys, os, commands switch = commands.getoutput("/usr/sbin/brctl show").split()[1].split()[0] os.system("/sbin/ifconfig %s 0.0.0.0 up" % sys.argv[1]) os.system("/usr/sbin/brctl addif %s %s" % (switch, sys.argv[1])) There's nothing 'pythonic' about this. It looks like it should be a bash script. It also looks simpler in bash. Anyway, if you like this better, or if you think the 'python only' policy should apply here, no problem. > On Sun, Aug 2, 2009 at 8:58 PM, Michael Goldish<mgold...@redhat.com> > wrote: > > The script adds a requested interface to an existing bridge. It is > meant to be > > used by qemu when running in TAP mode. > > > > Note: the user is responsible for setting up the bridge before > running any > > tests. This can be done with brctl or in any manner that is > appropriate for > > the host OS. It can be done inside 'qemu-ifup' as well, but this > sample script > > doesn't do it. > > > > Signed-off-by: Michael Goldish <mgold...@redhat.com> > > --- > > client/tests/kvm/qemu-ifup | 8 ++++++++ > > 1 files changed, 8 insertions(+), 0 deletions(-) > > create mode 100644 client/tests/kvm/qemu-ifup > > > > diff --git a/client/tests/kvm/qemu-ifup > b/client/tests/kvm/qemu-ifup > > new file mode 100644 > > index 0000000..bcd9a7a > > --- /dev/null > > +++ b/client/tests/kvm/qemu-ifup > > @@ -0,0 +1,8 @@ > > +#!/bin/sh > > + > > +# The following expression selects the first bridge listed by > 'brctl show'. > > +# Modify it to suit your needs. > > +switch=$(/usr/sbin/brctl show | awk 'NR==2 { print $1 }') > > + > > +/sbin/ifconfig $1 0.0.0.0 up > > +/usr/sbin/brctl addif ${switch} $1 > > -- > > 1.5.4.1 > > > > _______________________________________________ > > Autotest mailing list > > autot...@test.kernel.org > > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest > > > > > > -- > Lucas Meneghel > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html