On 08/05/2015 07:19 PM, Wei Liu wrote:
> On Wed, Aug 05, 2015 at 06:24:37PM +0100, Wei Liu wrote:
> [...]
>>>
>>
>> Right. I misinterpreted sd_boot.
>>
>> You patch, however, has the undesirable effect that it fails to report
>> error if xenstored is started by systemd but couldn't claim the
>> socket. I don't think this is the correct behaviour.
>>
>> After consulting with systemd manual [0], I think we should check
>> sd_listen_fds return value to determine if it is started by systemd.
>> Currently it only checks for <= 0, which covers 1) not started by
>> systemd 2) an error occurs.
>>
>> Hopefully I interpret the doc correctly this time. I will prepare a
>> patch shortly.
>>
>> Wei.
>>
>> [0] http://www.freedesktop.org/software/systemd/man/sd_listen_fds.html
>
> Patch attached. I start cxenstored by hand and it seems to works fine --
> now it fails with other errors. Can you test this patch to see if it
> works?
>
> Apparently oxenstored has similar issues. If this patch works I will
> need to fix oxenstored, too. Time to test my ocaml-fu! :-/
>
> ---8<---
> From a03eba6e258d8097b974366abb50b39af9e9abbf Mon Sep 17 00:00:00 2001
> From: Wei Liu <wei.l...@citrix.com>
> Date: Wed, 5 Aug 2015 19:02:34 +0100
> Subject: [PATCH] cxenstored: fix systemd socket activation
>
> There were two problems with original code:
>
> 1. sd_booted() was used to determined if the process was started by
> systemd, which was wrong.
> 2. Exit with error if pidfile was specified, which was too harsh.
>
> These two combined made cxenstored unable to start by hand if it ran
> on a system which had systemd.
>
> Fix issues with following changes:
>
> 1. Use sd_listen_fds to determine if the process is started by systemd.
> 2. Don't exit if pidfile is specified.
>
> Rename function and restructure code to make things clearer.
>
> Signed-off-by: Wei Liu <wei.l...@citrix.com>
I've booted with this patch using both the chkconfig+xencommons sysvinit
script, and the systemd method, and both work for me:
Tested-by: George Dunlap <george.dun...@eu.citrix.com>
Thanks,
-George
> ---
> tools/xenstore/xenstored_core.c | 27 +++++++++++++++++++--------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index b7e4936..57581e0 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -1781,7 +1781,10 @@ static int xs_validate_active_socket(const char
> *connect_to)
> return xs_get_sd_fd(connect_to);
> }
>
> -static void xen_claim_active_sockets(int **psock, int **pro_sock)
> +/* Return true if started by systemd and false if not. Exit with
> + * error if things go wrong.
> + */
> +static bool systemd_checkin(int **psock, int **pro_sock)
> {
> int *sock, *ro_sock;
> const char *soc_str = xs_daemon_socket();
> @@ -1789,7 +1792,11 @@ static void xen_claim_active_sockets(int **psock, int
> **pro_sock)
> int n;
>
> n = sd_listen_fds(0);
> - if (n <= 0) {
> +
> + if (n == 0)
> + return false;
> +
> + if (n < 0) {
> sd_notifyf(0, "STATUS=Failed to get any active sockets: %s\n"
> "ERRNO=%i",
> strerror(errno),
> @@ -1816,6 +1823,8 @@ static void xen_claim_active_sockets(int **psock, int
> **pro_sock)
>
> talloc_set_destructor(sock, destroy_fd);
> talloc_set_destructor(ro_sock, destroy_fd);
> +
> + return true;
> }
> #endif
>
> @@ -1929,6 +1938,9 @@ int main(int argc, char *argv[])
> bool no_domain_init = false;
> const char *pidfile = NULL;
> int timeout;
> +#if defined(XEN_SYSTEMD_ENABLED)
> + bool systemd;
> +#endif
>
> while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:", options,
> NULL)) != -1) {
> @@ -1990,10 +2002,11 @@ int main(int argc, char *argv[])
> barf("%s: No arguments desired", argv[0]);
>
> #if defined(XEN_SYSTEMD_ENABLED)
> - if (sd_booted()) {
> + systemd = systemd_checkin(&sock, &ro_sock);
> + if (systemd) {
> dofork = false;
> if (pidfile)
> - barf("%s: PID file not needed on systemd", argv[0]);
> + xprintf("%s: PID file not needed on systemd", argv[0]);
> pidfile = NULL;
> }
> #endif
> @@ -2020,9 +2033,7 @@ int main(int argc, char *argv[])
> signal(SIGPIPE, SIG_IGN);
>
> #if defined(XEN_SYSTEMD_ENABLED)
> - if (sd_booted())
> - xen_claim_active_sockets(&sock, &ro_sock);
> - else
> + if (!systemd)
> #endif
> init_sockets(&sock, &ro_sock);
>
> @@ -2057,7 +2068,7 @@ int main(int argc, char *argv[])
> xenbus_notify_running();
>
> #if defined(XEN_SYSTEMD_ENABLED)
> - if (sd_booted()) {
> + if (systemd) {
> sd_notify(1, "READY=1");
> fprintf(stderr, SD_NOTICE "xenstored is ready\n");
> }
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
