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

> 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?