On Thu, Aug 6, 2009 at 2:41 AM, sudhir kumar<smalik...@gmail.com> wrote:
> 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.

Ok folks, fair enough :) Let's keep it bash, in this case I think it won't hurt.
Thanks for your input!

> On Wed, Aug 5, 2009 at 6:21 PM, Michael Goldish<mgold...@redhat.com> wrote:
>>
>> ----- "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
>> _______________________________________________
>> Autotest mailing list
>> autot...@test.kernel.org
>> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>>
>
>
>
> --
> Sudhir Kumar
> _______________________________________________
> 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

Reply via email to