[PATCH v2] tools/power/x86/intel_pstate_tracer: Adjust directory ownership

[PATCH v2] tools/power/x86/intel_pstate_tracer: Adjust directory ownership

[Doug Smythies]

The intel_pstate_tracer.py script only needs to be run as root
when it is also used to actually acquire the trace data that
it will post process. Otherwise it is generally preferable
that it be run as a regular user.
If run the first time as root the results directory will be
incorrect for any subsequent run as a regular user. For any run
as root the specific testname subdirectory will not allow any
subsequent file saves by a regular user. Typically, and for example,
the regular user might be attempting to save a .csv file converted to
a spreadsheet with added calculations or graphs.

Set the directories and files owner and groups IDs to be the regular
user, if required.

Signed-off-by: Doug Smythies <dsmythies@telus.net>
---
 .../x86/intel_pstate_tracer/intel_pstate_tracer.py      | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
index fd706ac..0b24dd9 100755
--- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
+++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
@@ -353,6 +353,14 @@ def split_csv():
                 os.system('grep -m 1 common_cpu cpu.csv > cpu{:0>3}.csv'.format(index))
                 os.system('grep CPU_{:0>3} cpu.csv >> cpu{:0>3}.csv'.format(index, index))
 
+def fix_ownership(path):
+    """Change the owner of the file to SUDO_UID, if required"""
+
+    uid = os.environ.get('SUDO_UID')
+    gid = os.environ.get('SUDO_GID')
+    if uid is not None:
+        os.chown(path, int(uid), int(gid))
+
 def cleanup_data_files():
     """ clean up existing data files """
 
@@ -518,12 +526,16 @@ else:
 
 if not os.path.exists('results'):
     os.mkdir('results')
+    # The regular user needs to own the directory, not root.
+    fix_ownership('results')
 
 os.chdir('results')
 if os.path.exists(testname):
     print('The test name directory already exists. Please provide a unique test name. Test re-run not supported, yet.')
     sys.exit()
 os.mkdir(testname)
+# The regular user needs to own the directory, not root.
+fix_ownership(testname)
 os.chdir(testname)
 
 # Temporary (or perhaps not)
@@ -566,4 +578,9 @@ plot_scaled_cpu()
 plot_boost_cpu()
 plot_ghz_cpu()
 
+# It is preferrable, but not necessary, that the regular user owns the files, not root.
+for root, dirs, files in os.walk('.'):
+    for f in files:
+        fix_ownership(f)
+
 os.chdir('../../')
-- 
2.7.4

Re: [PATCH v2] tools/power/x86/intel_pstate_tracer: Adjust directory ownership

[Rafael J. Wysocki]

On Monday, April 17, 2017 05:12:13 PM Doug Smythies wrote:
> The intel_pstate_tracer.py script only needs to be run as root
> when it is also used to actually acquire the trace data that
> it will post process. Otherwise it is generally preferable
> that it be run as a regular user.
> If run the first time as root the results directory will be
> incorrect for any subsequent run as a regular user. For any run
> as root the specific testname subdirectory will not allow any
> subsequent file saves by a regular user. Typically, and for example,
> the regular user might be attempting to save a .csv file converted to
> a spreadsheet with added calculations or graphs.
> 
> Set the directories and files owner and groups IDs to be the regular
> user, if required.
> 
> Signed-off-by: Doug Smythies <dsmythies@telus.net>

Srinivas, any concerns?

> ---
>  .../x86/intel_pstate_tracer/intel_pstate_tracer.py      | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> index fd706ac..0b24dd9 100755
> --- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> +++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> @@ -353,6 +353,14 @@ def split_csv():
>                  os.system('grep -m 1 common_cpu cpu.csv > cpu{:0>3}.csv'.format(index))
>                  os.system('grep CPU_{:0>3} cpu.csv >> cpu{:0>3}.csv'.format(index, index))
>  
> +def fix_ownership(path):
> +    """Change the owner of the file to SUDO_UID, if required"""
> +
> +    uid = os.environ.get('SUDO_UID')
> +    gid = os.environ.get('SUDO_GID')
> +    if uid is not None:
> +        os.chown(path, int(uid), int(gid))
> +
>  def cleanup_data_files():
>      """ clean up existing data files """
>  
> @@ -518,12 +526,16 @@ else:
>  
>  if not os.path.exists('results'):
>      os.mkdir('results')
> +    # The regular user needs to own the directory, not root.
> +    fix_ownership('results')
>  
>  os.chdir('results')
>  if os.path.exists(testname):
>      print('The test name directory already exists. Please provide a unique test name. Test re-run not supported, yet.')
>      sys.exit()
>  os.mkdir(testname)
> +# The regular user needs to own the directory, not root.
> +fix_ownership(testname)
>  os.chdir(testname)
>  
>  # Temporary (or perhaps not)
> @@ -566,4 +578,9 @@ plot_scaled_cpu()
>  plot_boost_cpu()
>  plot_ghz_cpu()
>  
> +# It is preferrable, but not necessary, that the regular user owns the files, not root.
> +for root, dirs, files in os.walk('.'):
> +    for f in files:
> +        fix_ownership(f)
> +
>  os.chdir('../../')
> 

Re: [PATCH v2] tools/power/x86/intel_pstate_tracer: Adjust directory ownership

[Srinivas Pandruvada]

On Tue, 2017-04-18 at 15:58 +0200, Rafael J. Wysocki wrote:
> On Monday, April 17, 2017 05:12:13 PM Doug Smythies wrote:
> > 
> > The intel_pstate_tracer.py script only needs to be run as root
> > when it is also used to actually acquire the trace data that
> > it will post process. Otherwise it is generally preferable
> > that it be run as a regular user.
> > If run the first time as root the results directory will be
> > incorrect for any subsequent run as a regular user. For any run
> > as root the specific testname subdirectory will not allow any
> > subsequent file saves by a regular user. Typically, and for
> > example,
> > the regular user might be attempting to save a .csv file converted
> > to
> > a spreadsheet with added calculations or graphs.
> > 
> > Set the directories and files owner and groups IDs to be the
> > regular
> > user, if required.
> > 
> > Signed-off-by: Doug Smythies <dsmythies@telus.net>
 Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

> 
> Srinivas, any concerns?
No, I am fine with the change.

Thanks,
Srinivas

> 
> > 
> > ---
> >  .../x86/intel_pstate_tracer/intel_pstate_tracer.py      | 17
> > +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git
> > a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> > b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> > index fd706ac..0b24dd9 100755
> > --- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> > +++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> > @@ -353,6 +353,14 @@ def split_csv():
> >                  os.system('grep -m 1 common_cpu cpu.csv >
> > cpu{:0>3}.csv'.format(index))
> >                  os.system('grep CPU_{:0>3} cpu.csv >>
> > cpu{:0>3}.csv'.format(index, index))
> >  
> > +def fix_ownership(path):
> > +    """Change the owner of the file to SUDO_UID, if required"""
> > +
> > +    uid = os.environ.get('SUDO_UID')
> > +    gid = os.environ.get('SUDO_GID')
> > +    if uid is not None:
> > +        os.chown(path, int(uid), int(gid))
> > +
> >  def cleanup_data_files():
> >      """ clean up existing data files """
> >  
> > @@ -518,12 +526,16 @@ else:
> >  
> >  if not os.path.exists('results'):
> >      os.mkdir('results')
> > +    # The regular user needs to own the directory, not root.
> > +    fix_ownership('results')
> >  
> >  os.chdir('results')
> >  if os.path.exists(testname):
> >      print('The test name directory already exists. Please provide
> > a unique test name. Test re-run not supported, yet.')
> >      sys.exit()
> >  os.mkdir(testname)
> > +# The regular user needs to own the directory, not root.
> > +fix_ownership(testname)
> >  os.chdir(testname)
> >  
> >  # Temporary (or perhaps not)
> > @@ -566,4 +578,9 @@ plot_scaled_cpu()
> >  plot_boost_cpu()
> >  plot_ghz_cpu()
> >  
> > +# It is preferrable, but not necessary, that the regular user owns
> > the files, not root.
> > +for root, dirs, files in os.walk('.'):
> > +    for f in files:
> > +        fix_ownership(f)
> > +
> >  os.chdir('../../')
> > 
>