All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Update sar command and handle OSError error.
@ 2010-03-30  5:24 Feng Yang
  2010-04-01  1:27 ` [Autotest] " Lucas Meneghel Rodrigues
  0 siblings, 1 reply; 3+ messages in thread
From: Feng Yang @ 2010-03-30  5:24 UTC (permalink / raw)
  To: autotest; +Cc: kvm

This patch do following things:
1. Update sar command in start function in /profilers/sar/sar.py,
because when i manual run '/usr/bin/sar -o %s %d 0' command, help
message is show up. Sames count number could not be 0, so use default
count.
2. Put os.kill in sar.py into try block to avoid traceback.
Sometimes it tried to kill an already terminated process which can
cause a traceback.

Signed-off-by: Feng Yang <fyang@redhat.com>
---
 client/profilers/sar/sar.py |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/client/profilers/sar/sar.py b/client/profilers/sar/sar.py
index fbe0639..e10156f 100644
--- a/client/profilers/sar/sar.py
+++ b/client/profilers/sar/sar.py
@@ -21,14 +21,17 @@ class sar(profiler.profiler):
         logfile = open(os.path.join(test.profdir, "sar"), 'w')
         # Save the sar data as binary, convert to text after the test.
         raw = os.path.join(test.profdir, "sar.raw")
-        cmd = "/usr/bin/sar -o %s %d 0" % (raw, self.interval)
+        cmd = "/usr/bin/sar -o %s %d " % (raw, self.interval)
         p = subprocess.Popen(cmd, shell=True, stdout=logfile, \
                              stderr=subprocess.STDOUT)
         self.pid = p.pid
 
 
     def stop(self, test):
-        os.kill(self.pid, 15)
+        try:
+            os.kill(self.pid, 15)
+        except OSError:
+            pass
 
 
     def report(self, test):
-- 
1.5.5.6

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [Autotest] [PATCH] Update sar command and handle OSError error.
  2010-03-30  5:24 [PATCH] Update sar command and handle OSError error Feng Yang
@ 2010-04-01  1:27 ` Lucas Meneghel Rodrigues
  2010-04-01  1:59   ` Feng Yang
  0 siblings, 1 reply; 3+ messages in thread
From: Lucas Meneghel Rodrigues @ 2010-04-01  1:27 UTC (permalink / raw)
  To: Feng Yang; +Cc: autotest, kvm

On Tue, Mar 30, 2010 at 2:24 AM, Feng Yang <fyang@redhat.com> wrote:
> This patch do following things:
> 1. Update sar command in start function in /profilers/sar/sar.py,
> because when i manual run '/usr/bin/sar -o %s %d 0' command, help
> message is show up. Sames count number could not be 0, so use default
> count.

The problem here is that the sar command changed its behavior: if you
check man pages for older versions of sar (see
http://linux.die.net/man/1/sar), you'll see:

"The default value for the count parameter is 1. If its value is set
to zero, then reports are generated continuously."

For newer versions of sar like the ones shipped in recent Fedoras, the
behavior is conflicting. From the man page:

"If the interval parameter is specified without the count parameter,
then reports are generated continuously."

So, it's clear here that we have to handle the differences, otherwise
the profiler won't work on older systems.

> 2. Put os.kill in sar.py into try block to avoid traceback.
> Sometimes it tried to kill an already terminated process which can
> cause a traceback.

It just occurred to me that since we're using subprocess, we can use
the terminate() method of the subprocess object, that doesn't raise
exceptions when the process already returned, instead of just picking
the PID and sending a sigterm on that PID. I just sent a version of
this patch that does things the way I explained, and plan on change
the subprocess part for the kvm_stat test profiler as well.

> Signed-off-by: Feng Yang <fyang@redhat.com>
> ---
>  client/profilers/sar/sar.py |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/client/profilers/sar/sar.py b/client/profilers/sar/sar.py
> index fbe0639..e10156f 100644
> --- a/client/profilers/sar/sar.py
> +++ b/client/profilers/sar/sar.py
> @@ -21,14 +21,17 @@ class sar(profiler.profiler):
>         logfile = open(os.path.join(test.profdir, "sar"), 'w')
>         # Save the sar data as binary, convert to text after the test.
>         raw = os.path.join(test.profdir, "sar.raw")
> -        cmd = "/usr/bin/sar -o %s %d 0" % (raw, self.interval)
> +        cmd = "/usr/bin/sar -o %s %d " % (raw, self.interval)
>         p = subprocess.Popen(cmd, shell=True, stdout=logfile, \
>                              stderr=subprocess.STDOUT)
>         self.pid = p.pid
>
>
>     def stop(self, test):
> -        os.kill(self.pid, 15)
> +        try:
> +            os.kill(self.pid, 15)
> +        except OSError:
> +            pass
>
>
>     def report(self, test):
> --
> 1.5.5.6
>
> _______________________________________________
> Autotest mailing list
> Autotest@test.kernel.org
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>



-- 
Lucas

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] Update sar command and handle OSError error.
  2010-04-01  1:27 ` [Autotest] " Lucas Meneghel Rodrigues
@ 2010-04-01  1:59   ` Feng Yang
  0 siblings, 0 replies; 3+ messages in thread
From: Feng Yang @ 2010-04-01  1:59 UTC (permalink / raw)
  To: Lucas Meneghel Rodrigues; +Cc: autotest, kvm

Hi, Lucas

Thanks very much!


----- "Lucas Meneghel Rodrigues" <lmr@redhat.com> wrote:

> From: "Lucas Meneghel Rodrigues" <lmr@redhat.com>
> To: "Feng Yang" <fyang@redhat.com>
> Cc: autotest@test.kernel.org, kvm@vger.kernel.org
> Sent: Thursday, April 1, 2010 9:27:44 AM GMT +08:00 Beijing / Chongqing / Hong Kong / Urumqi
> Subject: Re: [Autotest] [PATCH] Update sar command and handle OSError error.
>
> On Tue, Mar 30, 2010 at 2:24 AM, Feng Yang <fyang@redhat.com> wrote:
> > This patch do following things:
> > 1. Update sar command in start function in /profilers/sar/sar.py,
> > because when i manual run '/usr/bin/sar -o %s %d 0' command, help
> > message is show up. Sames count number could not be 0, so use
> default
> > count.
> 
> The problem here is that the sar command changed its behavior: if you
> check man pages for older versions of sar (see
> http://linux.die.net/man/1/sar), you'll see:
> 
> "The default value for the count parameter is 1. If its value is set
> to zero, then reports are generated continuously."
> 
> For newer versions of sar like the ones shipped in recent Fedoras,
> the
> behavior is conflicting. From the man page:
> 
> "If the interval parameter is specified without the count parameter,
> then reports are generated continuously."
> 
> So, it's clear here that we have to handle the differences, otherwise
> the profiler won't work on older systems.
> 
> > 2. Put os.kill in sar.py into try block to avoid traceback.
> > Sometimes it tried to kill an already terminated process which can
> > cause a traceback.
> 
> It just occurred to me that since we're using subprocess, we can use
> the terminate() method of the subprocess object, that doesn't raise
> exceptions when the process already returned, instead of just picking
> the PID and sending a sigterm on that PID. I just sent a version of
> this patch that does things the way I explained, and plan on change
> the subprocess part for the kvm_stat test profiler as well.
> 
> > Signed-off-by: Feng Yang <fyang@redhat.com>
> > ---
> >  client/profilers/sar/sar.py |    7 +++++--
> >  1 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/client/profilers/sar/sar.py
> b/client/profilers/sar/sar.py
> > index fbe0639..e10156f 100644
> > --- a/client/profilers/sar/sar.py
> > +++ b/client/profilers/sar/sar.py
> > @@ -21,14 +21,17 @@ class sar(profiler.profiler):
> >         logfile = open(os.path.join(test.profdir, "sar"), 'w')
> >         # Save the sar data as binary, convert to text after the
> test.
> >         raw = os.path.join(test.profdir, "sar.raw")
> > -        cmd = "/usr/bin/sar -o %s %d 0" % (raw, self.interval)
> > +        cmd = "/usr/bin/sar -o %s %d " % (raw, self.interval)
> >         p = subprocess.Popen(cmd, shell=True, stdout=logfile, \
> >                              stderr=subprocess.STDOUT)
> >         self.pid = p.pid
> >
> >
> >     def stop(self, test):
> > -        os.kill(self.pid, 15)
> > +        try:
> > +            os.kill(self.pid, 15)
> > +        except OSError:
> > +            pass
> >
> >
> >     def report(self, test):
> > --
> > 1.5.5.6
> >
> > _______________________________________________
> > Autotest mailing list
> > Autotest@test.kernel.org
> > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
> >
> 
> 
> 
> -- 
> Lucas
_______________________________________________
Autotest mailing list
Autotest@test.kernel.org
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-04-01  1:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-30  5:24 [PATCH] Update sar command and handle OSError error Feng Yang
2010-04-01  1:27 ` [Autotest] " Lucas Meneghel Rodrigues
2010-04-01  1:59   ` Feng Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.