[LEDE-DEV] [PATCH][RFC] procd: service: add data within the service itself

[John Crispin]

Hi Pierre

sorry for the late reply, comments inline


On 04/10/17 12:50, pme.lebleu at gmail.com wrote:
> From: Pierre Lebleu <pme.lebleu at gmail.com>
>
> It gives the ability to create firewall data within the
> service itself rather than within an instance.
>
> Signed-off-by: Pierre Lebleu <pme.lebleu at gmail.com>
> ---
>   service/service.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
>   service/service.h |  2 ++
>   2 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/service/service.c b/service/service.c
> index 9c798aa..595f6f7 100644
> --- a/service/service.c
> +++ b/service/service.c
> @@ -84,6 +84,7 @@ service_alloc(const char *name)
>   	s->name = new_name;
>   	s->avl.key = s->name;
>   	INIT_LIST_HEAD(&s->validators);
> +	blobmsg_list_simple_init(&s->data);
>   
>   	return s;
>   }
> @@ -95,6 +96,7 @@ enum {
>   	SERVICE_SET_TRIGGER,
>   	SERVICE_SET_VALIDATE,
>   	SERVICE_SET_AUTOSTART,
> +	SERVICE_SET_DATA,
>   	__SERVICE_SET_MAX
>   };
>   
> @@ -105,8 +107,20 @@ static const struct blobmsg_policy service_set_attrs[__SERVICE_SET_MAX] = {
>   	[SERVICE_SET_TRIGGER] = { "triggers", BLOBMSG_TYPE_ARRAY },
>   	[SERVICE_SET_VALIDATE] = { "validate", BLOBMSG_TYPE_ARRAY },
>   	[SERVICE_SET_AUTOSTART] = { "autostart", BLOBMSG_TYPE_BOOL },
> +	[SERVICE_SET_DATA] = { "data", BLOBMSG_TYPE_TABLE },
>   };
>   
> +static void
> +service_fill_any(struct blobmsg_list *l, struct blob_attr *cur)
> +{
> +	if (!cur)
> +		return;
> +
> +	DEBUG(2, "Add data for service\n");
> +
> +	blobmsg_list_fill(l, blobmsg_data(cur), blobmsg_data_len(cur), false);
> +}
> +
>   static int
>   service_update(struct service *s, struct blob_attr **tb, bool add)
>   {
> @@ -148,6 +162,8 @@ service_update(struct service *s, struct blob_attr **tb, bool add)
>   			vlist_flush(&s->instances);
>   	}
>   
> +	service_fill_any(&s->data, tb[SERVICE_SET_DATA]);
> +
>   	s->deleted = false;
>   
>   	rc(s->name, "running");
> @@ -159,6 +175,7 @@ static void
>   service_delete(struct service *s)
>   {
>   	vlist_flush_all(&s->instances);
> +	blobmsg_list_free(&s->data);
>   	s->deleted = true;
>   	service_stopped(s);
>   }
> @@ -316,6 +333,13 @@ service_dump(struct service *s, bool verbose)
>   		blobmsg_add_blob(&b, s->trigger);
>   	if (verbose && !list_empty(&s->validators))
>   		service_validate_dump(&b, s);
> +	if (!avl_is_empty(&s->data.avl)) {
> +		struct blobmsg_list_node *var;
> +		void *e = blobmsg_open_table(&b, "data");
> +		blobmsg_list_for_each(&s->data, var)
> +			blobmsg_add_blob(&b, var->data);
> +		blobmsg_close_table(&b, e);
> +	}
>   	blobmsg_close_table(&b, c);
>   }
>   
> @@ -598,13 +622,30 @@ service_get_data(struct ubus_context *ctx, struct ubus_object *obj,
>   	blob_buf_init(&b, 0);
>   	avl_for_each_element(&services, s, avl) {
>   		void *cs = NULL;
> +		void *ci = NULL;
> +		struct blobmsg_list_node *var;
>   
>   		if (name && strcmp(name, s->name))
>   			continue;
>   
> +		blobmsg_list_for_each(&s->data, var) {
> +			if (type && strcmp(blobmsg_name(var->data), type))
> +				continue;
> +
> +			if (!cs)
> +				cs = blobmsg_open_table(&b, s->name);
> +
> +			if (!ci)
> +				ci = blobmsg_open_table(&b, "*");
> +
> +			blobmsg_add_blob(&b, var->data);
> +		}
> +
> +		if (ci)
> +			blobmsg_close_table(&b, ci), ci = NULL;
> +

maybe i am reading the code wrong but it looks to me as if this would 
result int he service and instance data being intermingled. not sure if 
this is good or bad. could you possibly post a json dump of what the 
generated blob would looks like ?

     John

>   		vlist_for_each_element(&s->instances, in, node) {
> -			struct blobmsg_list_node *var;
> -			void *ci = NULL;
> +			ci = NULL;
>   
>   			if (instance && strcmp(instance, in->name))
>   				continue;
> diff --git a/service/service.h b/service/service.h
> index a433c9f..15333c4 100644
> --- a/service/service.h
> +++ b/service/service.h
> @@ -18,6 +18,7 @@
>   #include <libubox/avl.h>
>   #include <libubox/vlist.h>
>   #include <libubox/list.h>
> +#include "../utils/utils.h"
>   
>   extern struct avl_tree services;
>   
> @@ -46,6 +47,7 @@ struct service {
>   	struct blob_attr *trigger;
>   	struct vlist_tree instances;
>   	struct list_head validators;
> +	struct blobmsg_list data;
>   };
>   
>   void service_validate_add(struct service *s, struct blob_attr *attr);