[OpenWrt-Devel,v2] procd: delay inittab workers until the tty exists.
diff mbox

Message ID 54BD9F8A.2020907@exegin.com
State Changes Requested
Headers show

Commit Message

Owen Kirby Jan. 20, 2015, 12:21 a.m. UTC
Add hotplug support for tty devices in /etc/inittab that are specified by the askfirst
keyword so that terminals attached after boot time get console processes started.

This was tested on an AT91 target using the gadget serial subsystem and on an WNDR3800
with a USB serial adapter. One possible weirdness I encountered was that the baud rates
and control modes sometimes need adjusting with a hotplug script after reconnecting
the adapter. This is also only implemented for askfirst, but it might also make sense
to do the same thing for respawn and askconsole.

Signed-off-by: Owen Kirby <osk@exegin.com>
---
  inittab.c      | 35 ++++++++++++++++++++++++++++++++---
  plug/hotplug.c | 40 +++++++++++++++++++++++++++++++++++-----
  plug/hotplug.h | 15 +++++++++++++++
  3 files changed, 82 insertions(+), 8 deletions(-)

Comments

John Crispin Jan. 20, 2015, 6:34 a.m. UTC | #1
On 20/01/2015 01:21, Owen Kirby wrote:
> Add hotplug support for tty devices in /etc/inittab that are specified
> by the askfirst
> keyword so that terminals attached after boot time get console processes
> started.
> 
> This was tested on an AT91 target using the gadget serial subsystem and
> on an WNDR3800
> with a USB serial adapter. One possible weirdness I encountered was that
> the baud rates
> and control modes sometimes need adjusting with a hotplug script after
> reconnecting
> the adapter. This is also only implemented for askfirst, but it might
> also make sense
> to do the same thing for respawn and askconsole.
> 


i think it would be a lot simpler if you ...

* add a new cmd_handler inside hotplug/json_script called console or
spawn_console
* add a 2nd parameter to procd_inittab_run called char *tty that can be
NULL or act as a filter
* call procd_inittab_run with the 2nd parameter set from the new
json_script handler
* add a new if clause to package/system/procd/files/hotplug.json




> Signed-off-by: Owen Kirby <osk@exegin.com>
> ---
>  inittab.c      | 35 ++++++++++++++++++++++++++++++++---
>  plug/hotplug.c | 40 +++++++++++++++++++++++++++++++++++-----
>  plug/hotplug.h | 15 +++++++++++++++
>  3 files changed, 82 insertions(+), 8 deletions(-)
> 
> diff --git a/inittab.c b/inittab.c
> index 623103d..c9310e5 100644
> --- a/inittab.c
> +++ b/inittab.c
> @@ -26,6 +26,7 @@
>  #include <libubox/utils.h>
>  #include <libubox/list.h>
>  
> +#include "plug/hotplug.h"
>  #include "utils/utils.h"
>  #include "procd.h"
>  #include "rcS.h"
> @@ -55,6 +56,7 @@ struct init_action {
>  
>      struct init_handler *handler;
>      struct uloop_process proc;
> +    struct hotplug_event event;
>  
>      int respawn;
>      struct uloop_timeout tout;
> @@ -90,6 +92,29 @@ static int dev_exist(const char *dev)
>      return (res != -1);
>  }
>  
> +static void dev_hotplug(struct hotplug_event *event, struct blob_attr
> *msg)
> +{
> +    struct init_action *a = container_of(event, struct init_action,
> event);
> +    char *action = hotplug_msg_find_var(msg, "ACTION");
> +    char *subsystem = hotplug_msg_find_var(msg, "SUBSYSTEM");
> +    char *devname = hotplug_msg_find_var(msg, "DEVNAME");
> +   
> +    if (!action || !subsystem || !devname)
> +        return;
> +    if (strcmp(subsystem, "tty") || strcmp(devname, a->id))
> +        return;
> +   
> +    DEBUG(4, "inittab hotplug: ACTION=\"%s\", SUBSYSTEM=\"%s\",
> DEVNAME=\"%s\"\n",
> +        action, subsystem, devname);
> +    if (!strcmp(action, "add")) {
> +        uloop_timeout_set(&a->tout, a->respawn);
> +    }
> +    else if (!strcmp(action, "remove")) {
> +        uloop_process_delete(&a->proc);
> +        uloop_timeout_cancel(&a->tout);
> +    }
> +}
> +
>  static void fork_worker(struct init_action *a)
>  {
>      int fd;
> @@ -130,7 +155,7 @@ static void child_exit(struct uloop_process *proc,
> int ret)
>      struct init_action *a = container_of(proc, struct init_action, proc);
>  
>      DEBUG(4, "pid:%d\n", proc->pid);
> -        uloop_timeout_set(&a->tout, a->respawn);
> +        uloop_timeout_set(&a->tout, a->respawn);
>  }
>  
>  static void respawn(struct uloop_timeout *tout)
> @@ -157,7 +182,7 @@ static void askfirst(struct init_action *a)
>  {
>      int i;
>  
> -    if (!dev_exist(a->id) || (console && !strcmp(console, a->id))) {
> +    if (console && !strcmp(console, a->id)) {
>          DEBUG(4, "Skipping %s\n", a->id);
>          return;
>      }
> @@ -168,8 +193,12 @@ static void askfirst(struct init_action *a)
>      a->argv[0] = ask;
>      a->respawn = 500;
>  
> +    a->event.cb = dev_hotplug;
> +    hotplug_event_add(&a->event);
> +
>      a->proc.cb = child_exit;
> -    fork_worker(a);
> +    if (dev_exist(a->id))
> +        fork_worker(a);
>  }
>  
>  static void askconsole(struct init_action *a)
> diff --git a/plug/hotplug.c b/plug/hotplug.c
> index 061833a..3aa87c0 100644
> --- a/plug/hotplug.c
> +++ b/plug/hotplug.c
> @@ -44,13 +44,14 @@ struct cmd_queue {
>  };
>  
>  static LIST_HEAD(cmd_queue);
> +static LIST_HEAD(events);
>  static struct uloop_process queue_proc;
>  static struct uloop_timeout last_event;
>  static struct blob_buf b;
>  static char *rule_file;
>  static struct blob_buf script;
>  
> -static char *hotplug_msg_find_var(struct blob_attr *msg, const char *name)
> +char *hotplug_msg_find_var(struct blob_attr *msg, const char *name)
>  {
>      struct blob_attr *cur;
>      int rem;
> @@ -393,24 +394,30 @@ static struct json_script_ctx jctx = {
>      .handle_file = rule_handle_file,
>  };
>  
> -static void hotplug_handler_debug(struct blob_attr *data)
> +static void hotplug_handler_debug(struct hotplug_event *e, struct
> blob_attr *msg)
>  {
>      char *str;
>  
>      if (debug < 3)
>          return;
>  
> -    str = blobmsg_format_json(data, true);
> +    str = blobmsg_format_json(msg, true);
>      DEBUG(3, "%s\n", str);
>      free(str);
>  }
>  
> +static void hotplug_handler_script(struct hotplug_event *e, struct
> blob_attr *msg)
> +{
> +    json_script_run(&jctx, rule_file, msg);
> +}
> +
>  static void hotplug_handler(struct uloop_fd *u, unsigned int ev)
>  {
>      int i = 0;
>      static char buf[4096];
>      int len = recv(u->fd, buf, sizeof(buf), MSG_DONTWAIT);
>      void *index;
> +    struct hotplug_event *e, *tmp;
>      if (len < 1)
>          return;
>  
> @@ -427,10 +434,16 @@ static void hotplug_handler(struct uloop_fd *u,
> unsigned int ev)
>          i += l;
>      }
>      blobmsg_close_table(&b, index);
> -    hotplug_handler_debug(b.head);
> -    json_script_run(&jctx, rule_file, blob_data(b.head));
> +    list_for_each_entry_safe(e, tmp, &events, list)
> +        e->cb(e, blob_data(b.head));
>  }
>  
> +static struct hotplug_event script_event = {
> +    .cb = hotplug_handler_script,
> +};
> +static struct hotplug_event debug_event = {
> +    .cb = hotplug_handler_debug,
> +};
>  static struct uloop_fd hotplug_fd = {
>      .cb = hotplug_handler,
>  };
> @@ -470,6 +483,8 @@ void hotplug(char *rules)
>      json_script_init(&jctx);
>      queue_proc.cb = queue_proc_cb;
>      uloop_fd_add(&hotplug_fd, ULOOP_READ);
> +    hotplug_event_add(&debug_event);
> +    hotplug_event_add(&script_event);
>  }
>  
>  int hotplug_run(char *rules)
> @@ -486,3 +501,18 @@ void hotplug_shutdown(void)
>      uloop_fd_delete(&hotplug_fd);
>      close(hotplug_fd.fd);
>  }
> +
> +int hotplug_event_add(struct hotplug_event *e)
> +{
> +    list_add_tail(&e->list, &events);
> +   
> +    return 0;
> +}
> +
> +int hotplug_event_delete(struct hotplug_event *e)
> +{
> +    list_del(&e->list);
> +   
> +    return 0;
> +}
> +
> diff --git a/plug/hotplug.h b/plug/hotplug.h
> index 2a44442..4870476 100644
> --- a/plug/hotplug.h
> +++ b/plug/hotplug.h
> @@ -16,10 +16,25 @@
>  #define __PROCD_HOTPLUG_H
>  
>  #include <libubox/uloop.h>
> +#include <libubox/list.h>
> +#include "../utils/utils.h"
> +
> +struct hotplug_event;
> +
> +typedef void (*hotplug_event_handler)(struct hotplug_event *event,
> struct blob_attr *msg);
> +
> +struct hotplug_event {
> +    struct list_head list;
> +
> +    hotplug_event_handler cb;
> +};
>  
>  void hotplug(char *rules);
>  int hotplug_run(char *rules);
>  void hotplug_shutdown(void);
>  void hotplug_last_event(uloop_timeout_handler handler);
> +char *hotplug_msg_find_var(struct blob_attr *msg, const char *name);
> +int hotplug_event_add(struct hotplug_event *event);
> +int hotplug_event_delete(struct hotplug_event *event);
>  
>  #endif
Owen Kirby Jan. 21, 2015, 7:34 p.m. UTC | #2
John,

On 15-01-19 10:34 PM, John Crispin wrote:
>
> On 20/01/2015 01:21, Owen Kirby wrote:
>> Add hotplug support for tty devices in /etc/inittab that are specified
>> by the askfirst
>> keyword so that terminals attached after boot time get console processes
>> started.
>>
>> This was tested on an AT91 target using the gadget serial subsystem and
>> on an WNDR3800
>> with a USB serial adapter. One possible weirdness I encountered was that
>> the baud rates
>> and control modes sometimes need adjusting with a hotplug script after
>> reconnecting
>> the adapter. This is also only implemented for askfirst, but it might
>> also make sense
>> to do the same thing for respawn and askconsole.
>>
>
> i think it would be a lot simpler if you ...
>
> * add a new cmd_handler inside hotplug/json_script called console or
> spawn_console
> * add a 2nd parameter to procd_inittab_run called char *tty that can be
> NULL or act as a filter
> * call procd_inittab_run with the 2nd parameter set from the new
> json_script handler
> * add a new if clause to package/system/procd/files/hotplug.json
I'm not sure that would really be any simpler. The cmd_handler would 
need to deal with respawning the console process and would wind up 
duplicating a the code already in inittab.c. It seemed like less work to 
simply extend the hotplug system to add a way for the inittab.c code to 
receive notifications of hotplug events. There is also the question of 
where this 2nd parameter to procd_inittab_run would come from, would we 
need to parse the /etc/hotplug.json script for anything that might spawn 
a console, and what if there were multiple consoles created this way in 
the hotplug script?

Furthermore, it seems like the description of which tty devices should 
spawn consoles and which devices don't is really a board-specific 
configuration item that seems inappropriate to put into /etc/hotplug.json.

Although, adding a new cmd_handler for creating consoles is an 
interesting idea. We could get rid of /etc/inittab entirely by creating 
all the consoles that way.

But, if you really think that's the best approach then I'll code up a 
3rd version of the patch for you.

Cheers,
Owen

Patch
diff mbox

diff --git a/inittab.c b/inittab.c
index 623103d..c9310e5 100644
--- a/inittab.c
+++ b/inittab.c
@@ -26,6 +26,7 @@ 
  #include <libubox/utils.h>
  #include <libubox/list.h>
  
+#include "plug/hotplug.h"
  #include "utils/utils.h"
  #include "procd.h"
  #include "rcS.h"
@@ -55,6 +56,7 @@  struct init_action {
  
  	struct init_handler *handler;
  	struct uloop_process proc;
+	struct hotplug_event event;
  
  	int respawn;
  	struct uloop_timeout tout;
@@ -90,6 +92,29 @@  static int dev_exist(const char *dev)
  	return (res != -1);
  }
  
+static void dev_hotplug(struct hotplug_event *event, struct blob_attr *msg)
+{
+	struct init_action *a = container_of(event, struct init_action, event);
+	char *action = hotplug_msg_find_var(msg, "ACTION");
+	char *subsystem = hotplug_msg_find_var(msg, "SUBSYSTEM");
+	char *devname = hotplug_msg_find_var(msg, "DEVNAME");
+	
+	if (!action || !subsystem || !devname)
+		return;
+	if (strcmp(subsystem, "tty") || strcmp(devname, a->id))
+		return;
+	
+	DEBUG(4, "inittab hotplug: ACTION=\"%s\", SUBSYSTEM=\"%s\", DEVNAME=\"%s\"\n",
+		action, subsystem, devname);
+	if (!strcmp(action, "add")) {
+		uloop_timeout_set(&a->tout, a->respawn);
+	}
+	else if (!strcmp(action, "remove")) {
+		uloop_process_delete(&a->proc);
+		uloop_timeout_cancel(&a->tout);
+	}
+}
+
  static void fork_worker(struct init_action *a)
  {
  	int fd;
@@ -130,7 +155,7 @@  static void child_exit(struct uloop_process *proc, int ret)
  	struct init_action *a = container_of(proc, struct init_action, proc);
  
  	DEBUG(4, "pid:%d\n", proc->pid);
-        uloop_timeout_set(&a->tout, a->respawn);
+		uloop_timeout_set(&a->tout, a->respawn);
  }
  
  static void respawn(struct uloop_timeout *tout)
@@ -157,7 +182,7 @@  static void askfirst(struct init_action *a)
  {
  	int i;
  
-	if (!dev_exist(a->id) || (console && !strcmp(console, a->id))) {
+	if (console && !strcmp(console, a->id)) {
  		DEBUG(4, "Skipping %s\n", a->id);
  		return;
  	}
@@ -168,8 +193,12 @@  static void askfirst(struct init_action *a)
  	a->argv[0] = ask;
  	a->respawn = 500;
  
+	a->event.cb = dev_hotplug;
+	hotplug_event_add(&a->event);
+
  	a->proc.cb = child_exit;
-	fork_worker(a);
+	if (dev_exist(a->id))
+		fork_worker(a);
  }
  
  static void askconsole(struct init_action *a)
diff --git a/plug/hotplug.c b/plug/hotplug.c
index 061833a..3aa87c0 100644
--- a/plug/hotplug.c
+++ b/plug/hotplug.c
@@ -44,13 +44,14 @@  struct cmd_queue {
  };
  
  static LIST_HEAD(cmd_queue);
+static LIST_HEAD(events);
  static struct uloop_process queue_proc;
  static struct uloop_timeout last_event;
  static struct blob_buf b;
  static char *rule_file;
  static struct blob_buf script;
  
-static char *hotplug_msg_find_var(struct blob_attr *msg, const char *name)
+char *hotplug_msg_find_var(struct blob_attr *msg, const char *name)
  {
  	struct blob_attr *cur;
  	int rem;
@@ -393,24 +394,30 @@  static struct json_script_ctx jctx = {
  	.handle_file = rule_handle_file,
  };
  
-static void hotplug_handler_debug(struct blob_attr *data)
+static void hotplug_handler_debug(struct hotplug_event *e, struct blob_attr *msg)
  {
  	char *str;
  
  	if (debug < 3)
  		return;
  
-	str = blobmsg_format_json(data, true);
+	str = blobmsg_format_json(msg, true);
  	DEBUG(3, "%s\n", str);
  	free(str);
  }
  
+static void hotplug_handler_script(struct hotplug_event *e, struct blob_attr *msg)
+{
+	json_script_run(&jctx, rule_file, msg);
+}
+
  static void hotplug_handler(struct uloop_fd *u, unsigned int ev)
  {
  	int i = 0;
  	static char buf[4096];
  	int len = recv(u->fd, buf, sizeof(buf), MSG_DONTWAIT);
  	void *index;
+	struct hotplug_event *e, *tmp;
  	if (len < 1)
  		return;
  
@@ -427,10 +434,16 @@  static void hotplug_handler(struct uloop_fd *u, unsigned int ev)
  		i += l;
  	}
  	blobmsg_close_table(&b, index);
-	hotplug_handler_debug(b.head);
-	json_script_run(&jctx, rule_file, blob_data(b.head));
+	list_for_each_entry_safe(e, tmp, &events, list)
+		e->cb(e, blob_data(b.head));
  }
  
+static struct hotplug_event script_event = {
+	.cb = hotplug_handler_script,
+};
+static struct hotplug_event debug_event = {
+	.cb = hotplug_handler_debug,
+};
  static struct uloop_fd hotplug_fd = {
  	.cb = hotplug_handler,
  };
@@ -470,6 +483,8 @@  void hotplug(char *rules)
  	json_script_init(&jctx);
  	queue_proc.cb = queue_proc_cb;
  	uloop_fd_add(&hotplug_fd, ULOOP_READ);
+	hotplug_event_add(&debug_event);
+	hotplug_event_add(&script_event);
  }
  
  int hotplug_run(char *rules)
@@ -486,3 +501,18 @@  void hotplug_shutdown(void)
  	uloop_fd_delete(&hotplug_fd);
  	close(hotplug_fd.fd);
  }
+
+int hotplug_event_add(struct hotplug_event *e)
+{
+	list_add_tail(&e->list, &events);
+	
+	return 0;
+}
+
+int hotplug_event_delete(struct hotplug_event *e)
+{
+	list_del(&e->list);
+	
+	return 0;
+}
+
diff --git a/plug/hotplug.h b/plug/hotplug.h
index 2a44442..4870476 100644
--- a/plug/hotplug.h
+++ b/plug/hotplug.h
@@ -16,10 +16,25 @@ 
  #define __PROCD_HOTPLUG_H
  
  #include <libubox/uloop.h>
+#include <libubox/list.h>
+#include "../utils/utils.h"
+
+struct hotplug_event;
+
+typedef void (*hotplug_event_handler)(struct hotplug_event *event, struct blob_attr *msg);
+
+struct hotplug_event {
+	struct list_head list;
+
+	hotplug_event_handler cb;
+};
  
  void hotplug(char *rules);
  int hotplug_run(char *rules);
  void hotplug_shutdown(void);
  void hotplug_last_event(uloop_timeout_handler handler);
+char *hotplug_msg_find_var(struct blob_attr *msg, const char *name);
+int hotplug_event_add(struct hotplug_event *event);
+int hotplug_event_delete(struct hotplug_event *event);
  
  #endif