Lets not make it a python script. Since the purpose of providing this
script is that the user can copy it to /etc also and not bother
updating it to kvm_tests.cfg, so let us keep it bash only. Also as
Michael pointed there is nothing much pythonic even if we write it in
python, so better keep it bash.
On Wed, Aug 5, 2009 at 6:21 PM, Michael Goldish<mgoldish@xxxxxxxxxx> wrote:
>
> ----- "Lucas Meneghel Rodrigues" <lmr@xxxxxxxxxx> 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<mgoldish@xxxxxxxxxx>
>> 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 <mgoldish@xxxxxxxxxx>
>> > ---
>> > 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
>> > Autotest@xxxxxxxxxxxxxxx
>> > 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 majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> _______________________________________________
> Autotest mailing list
> Autotest@xxxxxxxxxxxxxxx
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>
--
Sudhir Kumar
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
