Re: [Xen-devel] [PATCH v4 05/15] oxenstored: add support for systemd active sockets

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Luis R. Rodriguez writes ("[PATCH v4 05/15] oxenstored: add support for systemd 
active sockets"):
...
> +/* Conforms to what we should send sd_is_socket_unix() */
> +struct xen_systemd_active_socket {
> +     int fd;
> +     int type;
> +     int listening;
> +     const char *path;
> +     size_t length;
> +};

This is quite a lot of extraneous stuff, AFAICT.  For example, we
always set listening to 0.

Why is this array not const ?

> +/*
> + * We list stdin, stdout and stderr simply for documentation purposes
> + * and to help our array size fit the number of expected sockets we
> + * as sd_listen_fds() will return 5 for example if you set the socket
> + * service with 2 sockets.
> + */

This doesn't seem very convincing to me.

> +CAMLprim value ocaml_sd_listen_fds(value connect_to)
> +{
> +     CAMLparam1(connect_to);
> +     CAMLlocal1(sock_ret);
> +     int n, r;
> +     struct xen_systemd_active_socket *active_socket;
> +
> +     active_socket = get_xen_active_socket((const char *) 
> String_val(connect_to));
> +     if (!active_socket)
> +             caml_failwith("ocaml_sd_listen_fds() got invalid request");

The sole purpose of this is to convert the string to an entry in the
the table ?  I.e., essentially, just the table index ?

> +/*
> + * If xenstored was built to depend on systemd libraries
> + * we assume you want all the bells and whistles with
> + * systemd.
> + */

This is not correct.  A distro might want to use the same binary for
both systemd and non-systemd installations.  You need to use a runtime
test.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel