Code review comment for lp:~jamesodhunt/upstart/bug-901038

Revision history for this message
Steve Langasek (vorlon) wrote :

> 1) Where telinit is attempting to talk to an old version of upstart that doesn't support
> re-exec. We can ignore this scenario as we don't support down-grade scenarios.

> 2) Where telinit is attempting to talk to a non-Upstart init daemon.

> I guess this could be an issue in Ubuntu, depending on exactly how we manage the init
> transition. However, we should be able to arrange for maintainer scripts to set
> UPSTART_TELINIT_U_NO_WAIT to avoid that issue.

I don't think any solution should require maintainer scripts to set an extra variable to get sane behavior out of telinit.

If telinit is trying to talk to a non-upstart init, shouldn't it know it? In that case, it surely should not wait for a reconnect on an upstart-specific private socket, but should automatically fall back gracefully to a non-blocking restart.

However, the current upstart 'telinit u' code simply exits non-zero when pid 1 is not upstart - as does the proposed new code. So this is something we need to deal with if/when adding support for telinit u on top of non-upstart pid 1, but not something that should block this change now.

Given this, what is the expected use case for "UPSTART_TELINIT_U_NO_WAIT"? If you don't have a concrete user for this, please omit. Unnecessary interfaces always carry an incremental cost, and I can't see any case where it would ever be correct to use this interface, so this looks like overengineering to me.

Otherwise, this looks fine to me for merging.

review: Needs Fixing

« Back to merge proposal