[RFC] new initctl command

[Surbhi Palande]

Hi James,

I have uploaded another set of changes with most of the recommendations.

On 06/27/2011 01:05 PM, James Hunt wrote:
> -----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.
>
I have created bugs on launchpad - bug  #802465, #802477, #802487 
against libnih.

> 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>
Thanks! I have added these instead.
>
> * 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().
This is because of the stat returning a 0 on /proc/cmdline.

> * 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.

Also adding the license to the files. Please do let me know if that 
looks ok.

Thanks a lot!

Warm Regards,
Surbhi.