remoting/host/installer/linux/debian/postinst - Issue 10857046: Stop remoting host on install.

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: remoting/host/installer/linux/debian/postinst

Issue 10857046: Stop remoting host on install. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Created 8 years, 4 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch | Annotate | Revision Log

OLD	NEW
(Empty)
	1 #!/bin/bash

	2

	3 set -e

	4

	5 case $1 in

	6 "configure")

	7 echo "Shutting down remoting hosts (they will restart automatically)..."

	8 killall -q -s USR1 remoting_me2me_host \|\| true
	Lambros 2012/08/17 00:14:11 Are you sure about USR1, doesn't that just tell it Are you sure about USR1, doesn't that just tell it to reload its config? Just a plain SIGTERM seems to be the right thing here. Jamie 2012/08/17 00:21:13 I think you're thinking of SIGHUP. The reason I we Show quoted text On 2012/08/17 00:14:11, Lambros wrote: > Are you sure about USR1, doesn't that just tell it to reload its config? Just a > plain SIGTERM seems to be the right thing here. I think you're thinking of SIGHUP. The reason I went with SIGUSR1 was so that we could handle in the host in a future version and perhaps close the connection more cleanly with an error message. However, the only value in sending the signal before we have code to process it is that downgrades would work correctly, which I don't think we care about. I'll change it to SIGTERM. Sergey Ulanov 2012/08/17 00:23:59 This will only restart the host. If the daemon cod Show quoted text On 2012/08/17 00:14:11, Lambros wrote: > Are you sure about USR1, doesn't that just tell it to reload its config? Just a > plain SIGTERM seems to be the right thing here. This will only restart the host. If the daemon code (the python script) is updated too, shouldn't we restart it too? Also if we use SIGTERM to kill the process it might be a good idea to handle it explicitly in the host, e.g. to shutdown all current sessions cleanly. Jamie 2012/08/17 01:21:28 That's right. I'm planning a follow-up CL to handl Show quoted text On 2012/08/17 00:23:59, sergeyu wrote: > On 2012/08/17 00:14:11, Lambros wrote: > > Are you sure about USR1, doesn't that just tell it to reload its config? Just > a > > plain SIGTERM seems to be the right thing here. > > This will only restart the host. If the daemon code (the python script) is > updated too, shouldn't we restart it too? That's right. I'm planning a follow-up CL to handle the more complex case of an updated script or Xvfb. The problem there is that simply restarting the affected process will kill the user's desktop and might result in data loss. I wanted to handle this simple case first. Show quoted text > Also if we use SIGTERM to kill the process it might be a good idea to handle it > explicitly in the host, e.g. to shutdown all current sessions cleanly. I'm not averse to doing that, but as Lambros says, I think it can be a follow-up (especially as it conflicts with your signal handling CL).
	9 ;;

	10 esac

	11

	12 #DEBHELPER#

OLD	NEW

« no previous file with comments | « remoting/host/installer/linux/debian/.gitignore ('k') | no next file » | no next file with comments »