[OpenWrt-Devel] [PATCH] service: Support writing pidfiles

[Yousong Zhou]

Hi, Karl

On 2 March 2016 at 17:07, Karl Palsson <karlp at tweak.net.au> wrote:
> From: Karl Palsson <karlp at remake.is>
>
> Use the "pidfile" attribute of a service to decide whether to write a
> pidfile or not.
>
> Files are removed on stop/restart, and correctly created if the config
> has changed.
>
> Signed-off-by: Karl Palsson <karlp at etactica.com>
> ---
>  service/instance.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  service/instance.h |  1 +
>  2 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/service/instance.c b/service/instance.c
> index b8602b8..fee6c78 100644
> --- a/service/instance.c
> +++ b/service/instance.c
> @@ -53,6 +53,7 @@ enum {
>         INSTANCE_ATTR_JAIL,
>         INSTANCE_ATTR_TRACE,
>         INSTANCE_ATTR_SECCOMP,
> +       INSTANCE_ATTR_PIDFILE,
>         __INSTANCE_ATTR_MAX
>  };
>
> @@ -75,6 +76,7 @@ static const struct blobmsg_policy instance_attr[__INSTANCE_ATTR_MAX] = {
>         [INSTANCE_ATTR_JAIL] = { "jail", BLOBMSG_TYPE_TABLE },
>         [INSTANCE_ATTR_TRACE] = { "trace", BLOBMSG_TYPE_BOOL },
>         [INSTANCE_ATTR_SECCOMP] = { "seccomp", BLOBMSG_TYPE_STRING },
> +       [INSTANCE_ATTR_PIDFILE] = { "pidfile", BLOBMSG_TYPE_STRING },
>  };
>
>  enum {
> @@ -230,6 +232,46 @@ jail_run(struct service_instance *in, char **argv)
>         return argc;
>  }
>
> +static int
> +instance_removepid(struct service_instance *in) {
> +       if (in->pidfile) {
> +               if (unlink(in->pidfile)) {
> +                       ERROR("Failed to removed pidfile: %s: %d - %s\n",
> +                               in->pidfile, errno, strerror(errno));
> +                       return 1;
> +               }
> +       }
> +       return 0;
> +}
> +
> +static int
> +instance_writepid(struct service_instance *in)
> +{
> +       FILE *_pidfile;
> +
> +       if (!in->pidfile) {
> +               return 0;
> +       }
> +       _pidfile = fopen(in->pidfile, "w");
> +       if (_pidfile < 0) {

Error check should be done against NULL

> +               ERROR("failed to open pidfile for writing: %s: %d (%s)",
> +                       in->pidfile, errno, strerror(errno));
> +               return 1;
> +       }
> +       if (fprintf(_pidfile, "%d\n", in->proc.pid) < 0) {
> +               ERROR("failed to write pidfile: %s: %d (%s)",
> +                       in->pidfile, errno, strerror(errno));
> +               return 2;
> +       }
> +       if (fclose(_pidfile)) {
> +               ERROR("failed to close pidfile: %s: %d (%s)",
> +                       in->pidfile, errno, strerror(errno));
> +               return 3;
> +       }
> +
> +       return 0;
> +}
> +
>  static void
>  instance_run(struct service_instance *in, int _stdout, int _stderr)
>  {
> @@ -378,8 +420,9 @@ instance_start(struct service_instance *in)
>                 return;
>         }
>
> -       DEBUG(2, "Started instance %s::%s\n", in->srv->name, in->name);
> +       DEBUG(2, "Started instance %s::%s[%d]\n", in->srv->name, in->name, pid);
>         in->proc.pid = pid;
> +       instance_writepid(in);
>         clock_gettime(CLOCK_MONOTONIC, &in->start);
>         uloop_process_add(&in->proc);
>
> @@ -496,6 +539,7 @@ instance_stop(struct service_instance *in)
>         in->halt = true;
>         in->restart = in->respawn = false;
>         kill(in->proc.pid, SIGTERM);
> +       instance_removepid(in);
>  }
>
>  static void
> @@ -506,6 +550,7 @@ instance_restart(struct service_instance *in)
>         in->halt = false;
>         in->restart = true;
>         kill(in->proc.pid, SIGTERM);
> +       instance_removepid(in);
>  }
>
>  static bool
> @@ -538,6 +583,16 @@ instance_config_changed(struct service_instance *in, struct service_instance *in
>         if (in->gid != in_new->gid)
>                 return true;
>
> +       if (in->pidfile && in_new->pidfile)
> +               if (strcmp(in->pidfile, in_new->pidfile))
> +                       return true;
> +
> +       if (in->pidfile && !in_new->pidfile)
> +               return true;
> +
> +       if (!in->pidfile && in_new->pidfile)
> +               return true;
> +
>         if (!blobmsg_list_equal(&in->limits, &in_new->limits))
>                 return true;
>
> @@ -780,6 +835,12 @@ instance_config_parse(struct service_instance *in)
>                 else
>                         in->seccomp = seccomp;
>         }
> +       if (tb[INSTANCE_ATTR_PIDFILE]) {
> +               char *pidfile = blobmsg_get_string(tb[INSTANCE_ATTR_PIDFILE]);
> +               if (pidfile) {

Well, empty string should also be checked after the NULL check...

                yousong

> +                       in->pidfile = pidfile;
> +               }
> +       }
>         if (!in->trace && tb[INSTANCE_ATTR_JAIL])
>                 in->has_jail = instance_jail_parse(in, tb[INSTANCE_ATTR_JAIL]);
>
> @@ -834,6 +895,7 @@ instance_config_move(struct service_instance *in, struct service_instance *in_sr
>         blobmsg_list_move(&in->jail.mount, &in_src->jail.mount);
>         in->trigger = in_src->trigger;
>         in->command = in_src->command;
> +       in->pidfile = in_src->pidfile;
>         in->name = in_src->name;
>         in->node.avl.key = in_src->node.avl.key;
>
> @@ -966,6 +1028,9 @@ void instance_dump(struct blob_buf *b, struct service_instance *in, int verbose)
>         if (in->seccomp)
>                 blobmsg_add_string(b, "seccomp", in->seccomp);
>
> +       if (in->pidfile)
> +               blobmsg_add_string(b, "pidfile", in->pidfile);
> +
>         if (in->has_jail) {
>                 void *r = blobmsg_open_table(b, "jail");
>                 if (in->jail.name)
> diff --git a/service/instance.h b/service/instance.h
> index 7490462..1ee0429 100644
> --- a/service/instance.h
> +++ b/service/instance.h
> @@ -56,6 +56,7 @@ struct service_instance {
>         bool no_new_privs;
>         struct jail jail;
>         char *seccomp;
> +       char *pidfile;
>
>         uint32_t respawn_timeout;
>         uint32_t respawn_threshold;
> --
> 2.4.3
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel