[libvirt] [PATCH 1/3] Add APIs for talking to init via /dev/initctl

[Eric Blake]

> To be able todo controlled shutdown/reboot of containers an

s/todo/to do/

> API to talk to init via /dev/initctl is required. Fortunately
> this is quite straightforward to implement, and is supported
> by both sysvinit and systemd. Upstart support for /dev/initctl
> is unclear.
> 

> +++ b/src/util/virinitctl.c
> @@ -0,0 +1,161 @@

> +
> +/* These constants & struct definitions are taken from
> + * systemd, under terms of LGPLv2+
> + *
> + * initreq.h    Interface to talk to init through /dev/initctl.
> + *
> + *              Copyright (C) 1995-2004 Miquel van Smoorenburg
> + */

Thankfully, compatible with our usage.

> +
> +#if defined(__FreeBSD_kernel__)
> +# define VIR_INITCTL_FIFO  "/etc/.initctl"
> +#else
> +# define VIR_INITCTL_FIFO  "/dev/initctl"
> +#endif
> +
> +#define VIR_INITCTL_MAGIC 0x03091969

Wonder if the original author of this code picked a birthdate.  Gotta
love the Easter eggs present in open source software :)

> +
> +/*
> + *      Because of legacy interfaces, "runlevel" and "sleeptime"
> + *      aren't in a separate struct in the union.
> + *
> + *      The weird sizes are because init expects the whole
> + *      struct to be 384 bytes.
> + */
> +struct virInitctlRequest {
> +    int     magic;                  /* Magic number
>                 */

'int' is not necessarily 4 bytes; I would feel slightly more
comfortable had upstream used int32_t.  I know you are just copying
code from an existing header (so don't change the struct itself),
but wonder if we should at least add our own sanity checking:

verify(sizeof(virInitctlRequest)) == 384);

> +
> +    if ((fd = open(path, O_WRONLY|O_NDELAY|O_CLOEXEC|O_NOCTTY)) < 0)

O_NDELAY is non-standard.  I would spell it according to POSIX as
O_NONBLOCK.

> +typedef enum virInitctlRunLevel virInitctlRunLevel;
> +enum virInitctlRunLevel {
> +    VIR_INITCTL_RUNLEVEL_POWEROFF = 0,
> +    VIR_INITCTL_RUNLEVEL_1 = 1,
> +    VIR_INITCTL_RUNLEVEL_2 = 2,
> +    VIR_INITCTL_RUNLEVEL_3 = 3,
> +    VIR_INITCTL_RUNLEVEL_4 = 4,
> +    VIR_INITCTL_RUNLEVEL_5 = 5,
> +    VIR_INITCTL_RUNLEVEL_REBOOT = 6,

Should you add VIR_INITCTL_RUNLEVEL_LAST, in case we ever
expand this list?

I've never used messages over the socket to initctl myself, but
I'm assuming your code works.  ACK once you fix the nits I've
pointed out above.