[KVM-AUTOTEST,03/12] KVM test: add sample 'qemu-ifup' script

Commit Message

Michael Goldish Aug. 2, 2009, 11:58 p.m. UTC

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 <[e-mail]>
---
 client/tests/kvm/qemu-ifup |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
 create mode 100644 client/tests/kvm/qemu-ifup

Comments

Lucas Meneghel Rodrigues Aug. 5, 2009, 12:10 p.m. UTC | #1

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!

On Sun, Aug 2, 2009 at 8:58 PM, Michael Goldish<[e-mail]> 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 <[e-mail]>
> ---
>  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
> [e-mail]
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>

Michael Goldish Aug. 5, 2009, 12:51 p.m. UTC | #2

----- "Lucas Meneghel Rodrigues" <[e-mail]> 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<[e-mail]>
> 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 <[e-mail]>
> > ---
> >  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
> > [e-mail]
> > 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 [e-mail]
> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

sudhir kumar Aug. 6, 2009, 5:41 a.m. UTC | #3

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<[e-mail]> wrote:
>
> ----- "Lucas Meneghel Rodrigues" <[e-mail]> 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<[e-mail]>
>> 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 <[e-mail]>
>> > ---
>> >  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
>> > [e-mail]
>> > 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 [e-mail]
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> _______________________________________________
> Autotest mailing list
> [e-mail]
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>

Lucas Meneghel Rodrigues Aug. 6, 2009, 11:42 a.m. UTC | #4

On Thu, Aug 6, 2009 at 2:41 AM, sudhir kumar<[e-mail]> 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<[e-mail]> wrote:
>>
>> ----- "Lucas Meneghel Rodrigues" <[e-mail]> 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<[e-mail]>
>>> 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 <[e-mail]>
>>> > ---
>>> >  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
>>> > [e-mail]
>>> > 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 [e-mail]
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> _______________________________________________
>> Autotest mailing list
>> [e-mail]
>> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>>
>
>
>
> --
> Sudhir Kumar
> _______________________________________________
> Autotest mailing list
> [e-mail]
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>

Patch

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