Code review comment for lp:~ricmm/ubuntu-touch-session/migrate-to-upstart-session

Revision history for this message
James Hunt (jamesodhunt) wrote :

Hi Ricardo - this is great! A couple of comments:

1) I really would like to see those sleeps removed if at all possible:

- If the sleeps are all required to wait "a reasonable time" for android services in the lxc container to start (?), we should instead use some sort of pre-start script to detect that via polling in preference to sleeping I think.

- upstart-session/maliit-server.conf: Is the app mgr /system/bin/ubuntuappmanager running in the container? If so, until we can either get it to talk 'out' to ubuntu or we can prod it to determine when it is actually ready, it would probably be better to:
  - remove the sleep.
  - keep the 'respawn' but also add a 'respawn limit ...' stanza to allow it to be restarted by upstart when it initially exits but with a wait period (see init(5)).

- upstart-session/unity8.conf: 14 seconds of sleep is a loooong time. In particular does that first 12 second sleep
  imply that no calls can be made with the phone until 12 seconds have passed... even emergency calls?

2) Setting XDG_RUNTIME_DIR

   I recently added a section to the Upstart Cookbook on how to run a Session Init in a touch-like environment:

   http://upstart.ubuntu.com/cookbook/#non-graphical-sessions-ubuntu-specific

   Specifically, if we can it would be good to set XDG_RUNTIME_DIR since that would:

   - Allow the Session Init to create the session file which would then allow 'initctl list-sessions' to work (try
     running it on a desktop system). This could be extremely useful for debugging since if the session file is
     created, you can then "join a session":

     http://upstart.ubuntu.com/cookbook/#joining-a-session

   Note: Upstart running as the Session Init doesn't actually care either way if a session file exists or not (by design), but it would provide a consistent environment if we do set XDG_RUNTIME_DIR.

« Back to merge proposal