All of lore.kernel.org
 help / color / mirror / Atom feed
* [rteval PATCH] rteval: stressng.py: Fix argument passing to Popen
@ 2021-07-28 11:19 Atsushi Nemoto
  2021-07-28 21:38 ` John Kacur
  0 siblings, 1 reply; 6+ messages in thread
From: Atsushi Nemoto @ 2021-07-28 11:19 UTC (permalink / raw)
  To: linux-rt-users, John Kacur

The self.args is a list of strings which may contains spaces.
Use shell style and pass self.args as a joined string.

Signed-off-by: Atsushi Nemoto <atsushi.nemoto@sord.co.jp>
---
 rteval/modules/loads/stressng.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rteval/modules/loads/stressng.py b/rteval/modules/loads/stressng.py
index 926de38..927bee5 100644
--- a/rteval/modules/loads/stressng.py
+++ b/rteval/modules/loads/stressng.py
@@ -84,7 +84,7 @@ class Stressng(CommandLineLoad):
 
         self._log(Log.DEBUG, "starting with %s" % " ".join(self.args))
         try:
-            self.process = subprocess.Popen(self.args,
+            self.process = subprocess.Popen(" ".join(self.args), shell=True,
                                             stdout=self.__out,
                                             stderr=self.__err,
                                             stdin=self.__in)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [rteval PATCH] rteval: stressng.py: Fix argument passing to Popen
  2021-07-28 11:19 [rteval PATCH] rteval: stressng.py: Fix argument passing to Popen Atsushi Nemoto
@ 2021-07-28 21:38 ` John Kacur
  2021-07-29  0:23   ` Atsushi Nemoto
  0 siblings, 1 reply; 6+ messages in thread
From: John Kacur @ 2021-07-28 21:38 UTC (permalink / raw)
  To: Atsushi Nemoto; +Cc: linux-rt-users



On Wed, 28 Jul 2021, Atsushi Nemoto wrote:

> The self.args is a list of strings which may contains spaces.
> Use shell style and pass self.args as a joined string.
> 
> Signed-off-by: Atsushi Nemoto <atsushi.nemoto@sord.co.jp>
> ---
>  rteval/modules/loads/stressng.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/rteval/modules/loads/stressng.py b/rteval/modules/loads/stressng.py
> index 926de38..927bee5 100644
> --- a/rteval/modules/loads/stressng.py
> +++ b/rteval/modules/loads/stressng.py
> @@ -84,7 +84,7 @@ class Stressng(CommandLineLoad):
>  
>          self._log(Log.DEBUG, "starting with %s" % " ".join(self.args))
>          try:
> -            self.process = subprocess.Popen(self.args,
> +            self.process = subprocess.Popen(" ".join(self.args), shell=True,
>                                              stdout=self.__out,
>                                              stderr=self.__err,
>                                              stdin=self.__in)
> -- 
> 2.11.0
> 

I don't see the need to do this here and in fact there are some security 
implications to using shell=True. Is there a reason you want to do this?

John


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [rteval PATCH] rteval: stressng.py: Fix argument passing to Popen
  2021-07-28 21:38 ` John Kacur
@ 2021-07-29  0:23   ` Atsushi Nemoto
  2021-07-29 23:10     ` John Kacur
  0 siblings, 1 reply; 6+ messages in thread
From: Atsushi Nemoto @ 2021-07-29  0:23 UTC (permalink / raw)
  To: jkacur; +Cc: linux-rt-users

On Wed, 28 Jul 2021 17:38:05 -0400 (EDT), John Kacur <jkacur@redhat.com> wrote:
>> -            self.process = subprocess.Popen(self.args,
>> +            self.process = subprocess.Popen(" ".join(self.args), shell=True,
>>                                              stdout=self.__out,
>>                                              stderr=self.__err,
>>                                              stdin=self.__in)
>> -- 
>> 2.11.0
>> 
> 
> I don't see the need to do this here and in fact there are some security 
> implications to using shell=True. Is there a reason you want to do this?

I want to pass multiple options using --stressng-arg.

And also, there are elements with spaces in self.args already:
"--timeout %s" and "--taskset %s".

---
Atsushi Nemoto

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [rteval PATCH] rteval: stressng.py: Fix argument passing to Popen
  2021-07-29  0:23   ` Atsushi Nemoto
@ 2021-07-29 23:10     ` John Kacur
  2021-07-30  4:35       ` Atsushi Nemoto
  0 siblings, 1 reply; 6+ messages in thread
From: John Kacur @ 2021-07-29 23:10 UTC (permalink / raw)
  To: Atsushi Nemoto; +Cc: linux-rt-users



On Thu, 29 Jul 2021, Atsushi Nemoto wrote:

> On Wed, 28 Jul 2021 17:38:05 -0400 (EDT), John Kacur <jkacur@redhat.com> wrote:
> >> -            self.process = subprocess.Popen(self.args,
> >> +            self.process = subprocess.Popen(" ".join(self.args), shell=True,
> >>                                              stdout=self.__out,
> >>                                              stderr=self.__err,
> >>                                              stdin=self.__in)
> >> -- 
> >> 2.11.0
> >> 
> > 
> > I don't see the need to do this here and in fact there are some security 
> > implications to using shell=True. Is there a reason you want to do this?
> 
> I want to pass multiple options using --stressng-arg.
> 
> And also, there are elements with spaces in self.args already:
> "--timeout %s" and "--taskset %s".
> 
> ---
> Atsushi Nemoto
> 

Running stress-ng as a load in rteval is fairly new and purposely 
contrained so far. So, you can't run multiple tests right now.
However most of what you want to do is already possible.

stress-ng is like any other load so you can specify the taskset like this
--loads-cpulist='0-4'

If you want for example to run the stress-ng memcpy test with an argument 
of N=8 for 8 workers with a timeout of ten seconds you do this

rteval -d1m --loads-cpulist='0-4' --stressng-option=memcpy 
--stressng-arg=8 --stressng-timeout=10

which will result in rteval running the following command
stress-ng --memcpy 8 --timeout 10 --taskset 0,1,2,3,4

John


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [rteval PATCH] rteval: stressng.py: Fix argument passing to Popen
  2021-07-29 23:10     ` John Kacur
@ 2021-07-30  4:35       ` Atsushi Nemoto
  2021-08-06  8:45         ` Atsushi Nemoto
  0 siblings, 1 reply; 6+ messages in thread
From: Atsushi Nemoto @ 2021-07-30  4:35 UTC (permalink / raw)
  To: jkacur; +Cc: linux-rt-users

On Thu, 29 Jul 2021 19:10:37 -0400 (EDT), John Kacur <jkacur@redhat.com> wrote:
> On Thu, 29 Jul 2021, Atsushi Nemoto wrote:
>> On Wed, 28 Jul 2021 17:38:05 -0400 (EDT), John Kacur <jkacur@redhat.com> wrote:
>> >> -            self.process = subprocess.Popen(self.args,
>> >> +            self.process = subprocess.Popen(" ".join(self.args), shell=True,
>> >>                                              stdout=self.__out,
>> >>                                              stderr=self.__err,
>> >>                                              stdin=self.__in)
>> >> -- 
>> > 
>> > I don't see the need to do this here and in fact there are some security 
>> > implications to using shell=True. Is there a reason you want to do this?
>> 
>> I want to pass multiple options using --stressng-arg.
>> 
>> And also, there are elements with spaces in self.args already:
>> "--timeout %s" and "--taskset %s".
> 
> Running stress-ng as a load in rteval is fairly new and purposely 
> contrained so far. So, you can't run multiple tests right now.
> However most of what you want to do is already possible.

I just want to pass some additional option like "--memcpy-method=libc"
using stressng-arg.

    rteval --stressng-option=memcpy --stressng-arg="8 --memcpy-method=libc"

> stress-ng is like any other load so you can specify the taskset like this
> --loads-cpulist='0-4'
> 
> If you want for example to run the stress-ng memcpy test with an argument 
> of N=8 for 8 workers with a timeout of ten seconds you do this
> 
> rteval -d1m --loads-cpulist='0-4' --stressng-option=memcpy 
> --stressng-arg=8 --stressng-timeout=10
> 
> which will result in rteval running the following command
> stress-ng --memcpy 8 --timeout 10 --taskset 0,1,2,3,4

In this case, the self.args will be:

	["stress-ng", "--memcpy", "8", "--timeout 10", "--taskset 0,1,2,3,4"]

and will cause "stress-ng: unrecognized option '--timeout 10'" error.

If "shell=True" is not acceptable, how about this?

    self.process = subprocess.Popen(" ".join(self.args).split(),

---
Atsushi Nemoto

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [rteval PATCH] rteval: stressng.py: Fix argument passing to Popen
  2021-07-30  4:35       ` Atsushi Nemoto
@ 2021-08-06  8:45         ` Atsushi Nemoto
  0 siblings, 0 replies; 6+ messages in thread
From: Atsushi Nemoto @ 2021-08-06  8:45 UTC (permalink / raw)
  To: jkacur; +Cc: linux-rt-users

On Fri, 30 Jul 2021 13:35:08 +0900 (JST), Atsushi Nemoto <atsushi.nemoto@sord.co.jp> wrote:
>> If you want for example to run the stress-ng memcpy test with an argument 
>> of N=8 for 8 workers with a timeout of ten seconds you do this
>> 
>> rteval -d1m --loads-cpulist='0-4' --stressng-option=memcpy 
>> --stressng-arg=8 --stressng-timeout=10
>> 
>> which will result in rteval running the following command
>> stress-ng --memcpy 8 --timeout 10 --taskset 0,1,2,3,4
> 
> In this case, the self.args will be:
> 
> 	["stress-ng", "--memcpy", "8", "--timeout 10", "--taskset 0,1,2,3,4"]
> 
> and will cause "stress-ng: unrecognized option '--timeout 10'" error.
> 
> If "shell=True" is not acceptable, how about this?
> 
>     self.process = subprocess.Popen(" ".join(self.args).split(),

Instead of this, spliting "--timeout 10" to ["--timeout", "10"] would
be better?

I will post another patch.

---
Atsushi Nemoto

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-08-06  8:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-28 11:19 [rteval PATCH] rteval: stressng.py: Fix argument passing to Popen Atsushi Nemoto
2021-07-28 21:38 ` John Kacur
2021-07-29  0:23   ` Atsushi Nemoto
2021-07-29 23:10     ` John Kacur
2021-07-30  4:35       ` Atsushi Nemoto
2021-08-06  8:45         ` Atsushi Nemoto

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.