Mir

Merge lp:~thomas-voss/mir/revert-process-group-leader-patch into lp:~mir-team/mir/trunk

Proposed by Thomas Voß on 2013-07-30
Status: Merged
Approved by: kevin gunn on 2013-07-30
Approved revision: 897
Merged at revision: 898
Proposed branch: lp:~thomas-voss/mir/revert-process-group-leader-patch
Merge into: lp:~mir-team/mir/trunk
Diff against target: 30 lines (+0/-20)
1 file modified
src/server/graphics/gbm/linux_virtual_terminal.cpp (+0/-20)
To merge this branch: bzr merge lp:~thomas-voss/mir/revert-process-group-leader-patch
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve on 2013-07-30
kevin gunn (community) Approve on 2013-07-30
Maarten Lankhorst (community) 2013-07-30 Disapprove on 2013-07-30
Review via email: mp+177645@code.launchpad.net

Commit message

Don't make Mir become a process group leader, to avoid CTRL-C killing the X server.

Description of the change

Don't make Mir become a process group leader, to avoid CTRL-C killing the X server.

To post a comment you must log in.
Robert Ancell (robert-ancell) wrote :

For the record, this is reverting this change: https://code.launchpad.net/~mlankhorst/mir/setsid/+merge/176676

Robert Ancell (robert-ancell) wrote :

This change was to fix the issue where the VT couldn't be acquired on startup, i.e.

"Failed to set the current VT mode: Input/output error (5)"

It's not clear to me how this change fixed that, but we should be aware that reverting this change might cause that problem again.

Maarten Lankhorst (mlankhorst) wrote :

See xorg-server/hw/xfree86/os-support/linux/lnx_init.c xf86OpenConsole and do the same for mir. Use tcgetattr/tcsetattr to set the console parameters to IGNBRK, this should fix xorg-server. You probably also need to set KDSKBMUTE to prevent characters from being visible afterwards.

review: Disapprove
kevin gunn (kgunn72) wrote :

for the moment - we are trying to update the system-compositor-testing ppa & this cntl+c issue is worse than the original bug that https://code.launchpad.net/~mlankhorst/mir/setsid/+merge/176676 fixed

top approving this, we can revisit this topic after we update the system-compositor-testing ppa

kevin gunn (kgunn72) wrote :

and an approve...

review: Approve
kevin gunn (kgunn72) wrote :

so we just verified through some additional testing that this change is only needed when the xmir stack fails to open the hw gpu driver & falls back to sw rendering for the x stack (not mir, but x)
So, the bug is still relevant but this change should be abandoned in favor of a solution which addresses both bugs

PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
Approved revid is not set in launchpad. This is most likely a launchpad issue and re-approve should fix it. There is also a chance (although a very small one) this is a permission problem of the ps-jenkins bot.
http://jenkins.qa.ubuntu.com/job/mir-autolanding/506/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1490
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1375
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-autolanding/160
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-autolanding/160/artifact/work/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
kevin gunn (kgunn72) wrote :

oops...hit save too soon...
these are the 2 relevant competing bugs...
https://bugs.launchpad.net/xmir/+bug/1206508 "Xorg crash on xmir punts user back out to greeter" which is the ctl+c issue on sw rendering - which this mp was intending to fix

and https://bugs.launchpad.net/mir/+bug/1195509 "System compositor fails to start - Failed to set the current VT mode: Input/output error (5) " which is a infrequent occuring bug...and results in fall back, which is fixed by https://code.launchpad.net/~mlankhorst/mir/setsid/+merge/176676

review: Disapprove
kevin gunn (kgunn72) wrote :

ok, after more testing - this was relevant to when x is 3D rendering, not tied to sw rendering as first thot
approving

still need to solve the conflict of desire to fix aforementioned bugs

review: Approve
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/graphics/gbm/linux_virtual_terminal.cpp'
2--- src/server/graphics/gbm/linux_virtual_terminal.cpp 2013-07-26 05:12:14 +0000
3+++ src/server/graphics/gbm/linux_virtual_terminal.cpp 2013-07-30 16:37:23 +0000
4@@ -186,26 +186,6 @@
5
6 std::string const active_vt_path{vt_path_stream.str()};
7
8- if (activate)
9- {
10- if (getpid() == getpgid(0) && setpgid(0, getpgid(getppid())) < 0)
11- {
12- BOOST_THROW_EXCEPTION(
13- boost::enable_error_info(
14- std::runtime_error("Failed to stop being a process group"))
15- << boost::errinfo_errno(errno));
16- }
17-
18- /* become process group leader */
19- if (setsid() < 0)
20- {
21- BOOST_THROW_EXCEPTION(
22- boost::enable_error_info(
23- std::runtime_error("Failed to become session leader"))
24- << boost::errinfo_errno(errno));
25- }
26- }
27-
28 auto vt_fd = fops->open(active_vt_path.c_str(), O_RDONLY | O_NDELAY);
29
30 if (vt_fd < 0)

Subscribers

People subscribed via source and target branches