Change in vdsm[master]: config: introduce a devel section

Francesco Romani has uploaded a new change for review.

Change subject: config: introduce a devel section
......................................................................

config: introduce a devel section

This patch introduces a 'devel' configuration section,
in order to unclutter a bit the already crowded 'vars'
section.

An additional important benefit of this move is to clearly
mark that some tunables should never turned on in production
environments, unless exceptional circumstances, like a
performance problem being debugged.

For the same reason, is acceptable to move these options
across minor versions (e.g. 3.5 to 3.6).

This patch also makes room for future options which are
currently being discussed, like memory profiling or
a debug manhole service.

Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345
Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt;
---
M lib/vdsm/config.py.in
M lib/vdsm/profile.py
M tests/profileTests.py
3 files changed, 24 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/64/32664/1

diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index 69185e0..63b82d5 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -29,19 +29,6 @@
         ('core_dump_enable', 'true',
             'Enable core dump.'),
 
-        ('profile_enable', 'false',
-            'Enable whole process profiling (requires yappi profiler).'),
-
-        ('profile_format', 'pstat',
-            'Profile file format (pstat, callgrind, ystat)'),
-
-        ('profile_builtins', 'false',
-            'Profile builtin functions used by standard Python modules. '
-            'false by default.'),
-
-        ('profile_clock', 'cpu',
-            'Sets the underlying clock type (cpu, wall)'),
-
         ('host_mem_reserve', '256',
             'Reserves memory for the host to prevent VMs from using all the '
             'physical pages. The values are in Mbytes.'),
@@ -349,6 +336,25 @@
         ('guests_gateway_ip', '', None),
 
     ]),
+
+    # Section: [devel]
+    ('devel', [
+
+        ('profile_enable', 'false',
+            'Enable whole process profiling (requires yappi profiler).'),
+
+        ('profile_format', 'pstat',
+            'Profile file format (pstat, callgrind, ystat)'),
+
+        ('profile_builtins', 'false',
+            'Profile builtin functions used by standard Python modules. '
+            'false by default.'),
+
+        ('profile_clock', 'cpu',
+            'Sets the underlying clock type (cpu, wall)'),
+
+    ]),
+
 ]
 
 
diff --git a/lib/vdsm/profile.py b/lib/vdsm/profile.py
index 93dfacc..9710225 100644
--- a/lib/vdsm/profile.py
+++ b/lib/vdsm/profile.py
@@ -36,9 +36,9 @@
 # Defaults
 
 _FILENAME = os.path.join(constants.P_VDSM_RUN, 'vdsmd.prof')
-_FORMAT = config.get('vars', 'profile_format')
-_BUILTINS = config.getboolean('vars', 'profile_builtins')
-_CLOCK = config.get('vars', 'profile_clock')
+_FORMAT = config.get('devel', 'profile_format')
+_BUILTINS = config.getboolean('devel', 'profile_builtins')
+_CLOCK = config.get('devel', 'profile_clock')
 _THREADS = True
 
 _lock = threading.Lock()
@@ -61,7 +61,7 @@
 
 
 def is_enabled():
-    return config.getboolean('vars', 'profile_enable')
+    return config.getboolean('devel', 'profile_enable')
 
 
 def is_running():
diff --git a/tests/profileTests.py b/tests/profileTests.py
index 4e9ff7c..95bcfb1 100644
--- a/tests/profileTests.py
+++ b/tests/profileTests.py
@@ -44,7 +44,7 @@
 def make_config(enable='false'):
     cfg = ConfigParser.ConfigParser()
     config.set_defaults(cfg)
-    cfg.set('vars', 'profile_enable', enable)
+    cfg.set('devel', 'profile_enable', enable)
     return cfg
 
 


-- 
To view, visit http://gerrit.ovirt.org/32664
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: config: introduce a devel section
......................................................................


Patch Set 2:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1842/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged_test_debug/183/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5816/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3975/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/32664
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: config: introduce a devel section
......................................................................


config: introduce a devel section

This patch introduces a 'devel' configuration section,
in order to unclutter a bit the already crowded 'vars'
section.

An additional important benefit of this move is to clearly
mark that some tunables should never turned on in production
environments, unless exceptional circumstances, like a
performance problem being debugged.

For the same reason, is acceptable to move these options
across minor versions (e.g. 3.5 to 3.6).

This patch also makes room for future options which are
currently being discussed, like memory profiling or
a debug manhole service.

Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345
Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/32664
Reviewed-by: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com&gt;
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M lib/vdsm/config.py.in
M lib/vdsm/profile.py
M tests/profileTests.py
3 files changed, 24 insertions(+), 18 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, but someone else must approve
  Antoni Segura Puimedon: Looks good to me, but someone else must approve
  Dan Kenigsberg: Looks good to me, approved
  Francesco Romani: Verified



-- 
To view, visit http://gerrit.ovirt.org/32664
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: config: introduce a devel section
......................................................................


Patch Set 1: Code-Review+2

-- 
To view, visit http://gerrit.ovirt.org/32664
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: config: introduce a devel section
......................................................................


Patch Set 1: Verified+1

verified by enabling the profiler and running a few test runs.

-- 
To view, visit http://gerrit.ovirt.org/32664
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: config: introduce a devel section
......................................................................


Patch Set 1:

I will implement Nir suggestion in a followup patch.

-- 
To view, visit http://gerrit.ovirt.org/32664
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: config: introduce a devel section
......................................................................


Patch Set 1:

(2 comments)

Looks fine but can do more - I think we can do all changes needed for additional profiler
in one step, simplifying review, verification and even backporting.

http://gerrit.ovirt.org/#/c/32664/1//COMMIT_MSG
Commit Message:

Line 19: across minor versions (e.g. 3.5 to 3.6).
Line 20: 
Line 21: This patch also makes room for future options which are
Line 22: currently being discussed, like memory profiling or
Line 23: a debug manhole service.
To do this we much change the config names so we can have many profile_xxx_enable
options,

And make the lib/vdsm/profile.py a package: lib/vdsm/profile/cpu.py
Line 24: 
Line 25: Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345


http://gerrit.ovirt.org/#/c/32664/1/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:

Line 339: 
Line 340:     # Section: [devel]
Line 341:     ('devel', [
Line 342: 
Line 343:         ('profile_enable', 'false',
Lests make room for the memory profiler by renaming all "profile_" configs to
"profile_cpu". We can do this in another patch, but I don't a need.

This patch: make room for more profilers
Next patch: add memory profiler

What do you think?
Line 344:             'Enable whole process profiling (requires yappi
profiler).'),
Line 345: 
Line 346:         ('profile_format', 'pstat',
Line 347:             'Profile file format (pstat, callgrind, ystat)'),


-- 
To view, visit http://gerrit.ovirt.org/32664
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Antoni Segura Puimedon has posted comments on this change.

Change subject: config: introduce a devel section
......................................................................


Patch Set 1: Code-Review+1

-- 
To view, visit http://gerrit.ovirt.org/32664
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: config: introduce a devel section
......................................................................


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12345/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11401/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12190/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/32664
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: config: introduce a devel section
......................................................................


Patch Set 1: Code-Review+1

On the other hand, we can keep this small and add another patch for the package.

-- 
To view, visit http://gerrit.ovirt.org/32664
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f9fc14c6fffd7ba111af985586eada83ec3c345
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No