[RFC] new initctl command

James Hunt james.hunt at canonical.com
Mon Jun 27 10:05:50 UTC 2011


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 22/06/11 14:01, Surbhi Palande wrote:
> Hi James,
>
> Thanks a lot for the review and the suggestions.
>
> I have incorporated most of the changes you suggested and pushed them to:
> https://code.launchpad.net/~csurbhi/upstart/upstart-add-pivot-handling/

Hi Surbhi,

Thanks very much for making the changes. I think it's getting pretty close now. Could you raise bugs
on NIH re exposing nih_dir_walk_scan() and the nih_file_read() stat() issue.

The remaining comments I have:

* init/pivot.h:
  - If there is a chance that MS_MOVE will not already be defined, I'd
    suggested documenting why and adding a line such as below in that #if
    block:

      #warning WARNING: MS_MOVE not defined

    However, can't we just do the following to get all 3 of these
    defines? This would be much safer should those magic numbers ever
    change.

      #include <linux/magic.h>
      #include <linux/fs.h>

* init/pivot.c:
  - line 73: del_dir(): "while ( (" should be "while (("
  - Would be good to add some nih_debug() calls to del_dir()
    so you can see what files / directories are being deleted?
  - is_init_ok(): Typo "exsists".
  - get_init_args():
    - Still hard-coding size of CMDLINE. Could break on x86_64.
    - Still not using nih_file_read() rather than fopen() and fgets().
* init/conf.c: delete_conf_watches():
  - NIH_LIST_FOREACH() indent wrong (and line below).
  - The use of inotify_rm_watch() makes me wonder if you should raise a
    bug on NIH since that functionality isn't available in watch.c.
* init/control.c: control_pivot_to_new_rootfs(): Could just remove the
  'ret' variable and make last line:

    return pivot_to_new_rootfs (message, rootfs, init);

* init/control.h: Formatting of control_emit_event_with_file()
  prototype.
* init/events.h: As mentioned, PIVOT_FAILED_EVENT should be
  "pivot-failed" for consistency with other events (see
  upstart-events(7) on Ubuntu).
* util/initctl.c: pivot_action(): Doc says returns -1 on error, but it
  is returning 1 on error consistently.

Kind regards,

James.

James Hunt
____________________________________
Ubuntu Foundations Team, Canonical.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk4IVfcACgkQYBWEaHcQG9eaAACfY3HBZXXfk6dPXbyEU2ECEU/6
y8kAmwVnr+VbI7KnuEvEtje2AsKetGUt
=LzrA
-----END PGP SIGNATURE-----



More information about the upstart-devel mailing list