Nir Soffer has uploaded a new change for review.
Change subject: debug: Integrate manhole debugging service ......................................................................
debug: Integrate manhole debugging service
Manhole is in-process service that will accept unix domain socket connections and present the stacktraces for all threads and an interactive prompt.
Usage:
0. Install it
pip install manhole
1. Enable it in vdsm.conf:
[vars] manhole_enable = true
2. Dive into vdsm:
$ nc -U /tmp/manhole-1234 Python 2.7.5 (default, Jun 26 2014, 11:55:39) [GCC 4.8.2 20131212 (Red Hat 4.8.2-7)] on linux2 Type "help", "copyright", "credits" or "license" for more information.
Change-Id: I8a6cdf97ddce446bea527e771eb641f969b1a532 Signed-off-by: Nir Soffer nsoffer@redhat.com --- M lib/vdsm/config.py.in M vdsm/vdsm 2 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/47/32147/1
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in index 69185e0..d085890 100644 --- a/lib/vdsm/config.py.in +++ b/lib/vdsm/config.py.in @@ -42,6 +42,9 @@ ('profile_clock', 'cpu', 'Sets the underlying clock type (cpu, wall)'),
+ ('manhole_enable', 'false', + 'Enable manhole debugging service (requires manhole package).'), + ('host_mem_reserve', '256', 'Reserves memory for the host to prevent VMs from using all the ' 'physical pages. The values are in Mbytes.'), diff --git a/vdsm/vdsm b/vdsm/vdsm index e575295..9fdf72f 100755 --- a/vdsm/vdsm +++ b/vdsm/vdsm @@ -70,6 +70,10 @@
profile.start()
+ if config.getboolean('vars', 'manhole_enable'): + import manhole + manhole.install() + libvirtconnection.start_event_loop()
if config.getboolean('irs', 'irs_enable'):
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 1:
Untested - posted so people can try this awesome thing.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11199/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12141/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11988/ : SUCCESS
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/32147/1//COMMIT_MSG Commit Message:
Line 22: manhole_enable = true Line 23: Line 24: 2. Dive into vdsm: Line 25: Line 26: $ nc -U /tmp/manhole-1234 1234 is the process pid
Should replace with:
$ nc -U /tmp/manhole-`cat /var/run/vdsm/vdsm.pid`
Or find how to configure it instead to create the socket at:
/var/run/vdsm/manhole Line 27: Python 2.7.5 (default, Jun 26 2014, 11:55:39) Line 28: [GCC 4.8.2 20131212 (Red Hat 4.8.2-7)] on linux2 Line 29: Type "help", "copyright", "credits" or "license" for more information. Line 30: >>>
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/32147/1//COMMIT_MSG Commit Message:
Line 22: manhole_enable = true Line 23: Line 24: 2. Dive into vdsm: Line 25: Line 26: $ nc -U /tmp/manhole-1234
1234 is the process pid
It is not possible to configure manhole socket. I sent a patch for adding this feature: https://github.com/nirs/python-manhole/commit/7755009b2140414c0786675c624f0e... Line 27: Python 2.7.5 (default, Jun 26 2014, 11:55:39) Line 28: [GCC 4.8.2 20131212 (Red Hat 4.8.2-7)] on linux2 Line 29: Type "help", "copyright", "credits" or "license" for more information. Line 30: >>>
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 2:
Use safer and more convenient /var/run/vdsm/vdsmd.manhole
oVirt Jenkins CI Server has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 2:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11216/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12158/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12005/ : SUCCESS
Antoni Segura Puimedon has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 2:
Have you considered instead of making it be a config value that it there would be a verb that one can trigger from vdsClient that installs and starts manhole?
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 2:
No, but why make it more complex? If we had to restart vdsm to enable it, I would like to use it without further actions.
Also this allows anyone to start the manhole, I would not want this on production machine.
Antoni Segura Puimedon has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 2:
Why would we need to restart vdsm to enable it.
What I propose is
1. Enable it:
vdsClient -s 0 manholeEnable
2. Dive into vdsm: nc -U /var/run/vdsm/vdsmd.manhole
3. Disable again vdsClient -s 0 manholeDisable
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 2:
What you suggest is convenient, but it gives someone out of the machine too much control.
We can use the activate_on=SIGUSR2 option to start the manhole when receiving a singal. To stop the manhole thread, we can arrange a function in the manhole shell that stop the manhole thread.
But I don't think this complexity is needed. Restarting vdsm is a safe and fast operation, and a thread waiting on accept have practically zero cost.
Antoni Segura Puimedon has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 2:
I would like the signal because to investigate problems that accumulate over time, it is much more convenient. If you restart vdsm you may lose them and it can take days to reproduce (e.g. current memory leak).
Francesco Romani has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 2:
I see Nir's point, but signal is very convenient.
A good way to accomodate both convenience and safety could be
* config option to enable/disable the service; so, in development or when debug is required, the admin must be aware of the presence of the manhole * a signal to 'open' the manhole, so it could be enabled on demand and we gain agility on complex scenarios like hunting the memleaks.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11683/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12627/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12472/ : SUCCESS
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 3: Verified+1
It works - and reveal that we have serious threading issue - we get this warning to stderr every few seconds:
Exception RuntimeError: RuntimeError('cannot notify on un-acquired lock',) in <module 'threading' from '/usr/lib64/python2.7/threading.pyc'> ignored
I'm not convinced that we need a signal to open the manhole, and I also never tested this feature.
Please try before your review.
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 3:
The console is unusable now because of the exceptions Python prints to stderr every few seconds.
I think this comes from bad code that raises exceptions in __del__, or maybe Python interpreter shutdown issues in Python child process inheriting stderr from vdsm. These issues should be fixed, but a new redirect_stderr option in manhole allow using manhole in the meantime.
To get this option please checkout and install the redirect_stderr branch from: https://github.com/nirs/python-manhole
And add this option when installing manhole: manhole.install(redirect_stderr=False)
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 4: Verified+1
This version works with current vdsm using my git.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 4:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11684/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12628/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12473/ : SUCCESS
Dan Kenigsberg has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 4: Code-Review+1
I believe this deprecates the debugPlugin. Let's drop it in a subsequent patch.
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 5:
Upstream manhole supports now redirect_stderr, no need to use my repository.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11685/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12629/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12474/ : SUCCESS
Francesco Romani has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 5: Code-Review+1
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 6:
Version 6 is rebase on master and update the commit message.
manhole-1.0.0 supports now the required features, so we don't need to use the git version any more.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 6:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12984/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12826/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/12035/ : FAILURE
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 6: Verified+1
Tested on rhel 6.6 and 7.
Francesco Romani has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 6: Code-Review+1
Useful and helpful addition for debugging, seems harmless in other scenarios, so solid +1.
Saggi Mizrahi has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 6:
How is this better than https://github.com/ficoos/gdb-python-scripts Which doesn't need any configuration change in VDSM and works for all python apps. Also includes running pyrdbg (while in gdb) which will start winpdb remote debugging session.
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 6:
Saggi, it looks that manhole integration and improving gdb python support are different things, and I don't see how can we compare them.
Having an integrated shell in vdsm means you can connect to vdsm and run any python code you like while the application is running. For example, testing functions that do not have public API (e.g. getHostLeaseStatus).
Can you do this using gdb?
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 6:
ping?
Francesco Romani has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 6:
ping for other reviews - I think this is an useful addition
Dan Kenigsberg has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 6:
I'm happy to include this, please rebase. And in another patch - remove the debug plugin
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 6:
Will rebase soon, thanks.
automation@ovirt.org has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 7:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Nir Soffer has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 7:
Rebased and tested again.
Dan Kenigsberg has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 7: Code-Review+2
raising
Dan Kenigsberg has submitted this change and it was merged.
Change subject: debug: Integrate manhole debugging service ......................................................................
debug: Integrate manhole debugging service
Manhole is in-process service that will accept unix domain socket connections and present the stacktraces for all threads and an interactive prompt.
Usage:
0. Install it
pip install manhole
1. Enable it in vdsm.conf:
[devel] manhole_enable = true
2. Dive into vdsm:
$ nc -U /var/run/vdsm/vdsmd.manhole Python 2.7.5 (default, Jun 26 2014, 11:55:39) [GCC 4.8.2 20131212 (Red Hat 4.8.2-7)] on linux2 Type "help", "copyright", "credits" or "license" for more information.
dir()
['__builtins__', 'cif', 'dump_stacktraces', 'irs', 'os', 'socket', 'sys', 'traceback']
irs.ready
True
For more info see http://python-manhole.readthedocs.org/
Change-Id: I8a6cdf97ddce446bea527e771eb641f969b1a532 Signed-off-by: Nir Soffer nsoffer@redhat.com Reviewed-on: https://gerrit.ovirt.org/32147 Reviewed-by: Francesco Romani fromani@redhat.com Reviewed-by: Dan Kenigsberg danken@redhat.com --- M lib/vdsm/config.py.in M vdsm/vdsm 2 files changed, 29 insertions(+), 0 deletions(-)
Approvals: Nir Soffer: Verified Dan Kenigsberg: Looks good to me, approved Francesco Romani: Looks good to me, but someone else must approve
automation@ovirt.org has posted comments on this change.
Change subject: debug: Integrate manhole debugging service ......................................................................
Patch Set 8:
* Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found.
vdsm-patches@lists.fedorahosted.org
-
asegurap＠redhat.com -
automation＠ovirt.org -
Dan Kenigsberg -
fromani＠redhat.com -
Jenkins CI RO -
Nir Soffer -
smizrahi＠redhat.com