subdaemon: callback to handle subdaemon status updates.

It's a bit messy, since some status messages are accompanied by an FD:
in this case, the handler returns STATUS_NEED_FD and we read that then
re-call the handler.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/hsm_control.c

@@ -1,10 +1,12 @@
 #include "hsm_control.h"
 #include "lightningd.h"
+#include "peer_control.h"
 #include "subdaemon.h"
 #include <ccan/err/err.h>
 #include <ccan/io/io.h>
 #include <ccan/take/take.h>
 #include <daemon/log.h>
+#include <inttypes.h>
 #include <lightningd/hsm/gen_hsm_control_wire.h>
 #include <lightningd/hsm/gen_hsm_status_wire.h>
 
@@ -26,6 +28,40 @@ static void hsm_finished(struct subdaemon *hsm, int status)
 	errx(1, "HSM failed (signal %u), exiting.", WTERMSIG(status));
 }
 
+static enum subdaemon_status hsm_status(struct subdaemon *hsm, const u8 *msg,
+					int fd)
+{
+	enum hsm_status_wire_type t = fromwire_peektype(msg);
+	u8 *badmsg;
+	struct peer *peer;
+	u64 id;
+
+	switch (t) {
+	case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST:
+		if (!fromwire_hsmstatus_client_bad_request(msg, msg, NULL,
+							   &id, &badmsg))
+			errx(1, "HSM bad status %s", tal_hex(msg, msg));
+		peer = peer_by_unique_id(hsm->ld, id);
+
+		/* "Shouldn't happen" */
+		errx(1, "HSM says bad cmd from %"PRIu64" (%s): %s",
+		     id,
+		     peer ? (peer->id ? type_to_string(msg, struct pubkey,
+						       peer->id)
+			     : "pubkey not yet known")
+		     : "unknown peer",
+		     tal_hex(msg, badmsg));
+
+	/* We don't get called for failed status. */
+	case WIRE_HSMSTATUS_INIT_FAILED:
+	case WIRE_HSMSTATUS_WRITEMSG_FAILED:
+	case WIRE_HSMSTATUS_BAD_REQUEST:
+	case WIRE_HSMSTATUS_FD_FAILED:
+		break;
+	}
+	return STATUS_COMPLETE;
+}
+
 void hsm_init(struct lightningd *ld, bool newdir)
 {
 	bool create;
@@ -33,7 +69,7 @@ void hsm_init(struct lightningd *ld, bool newdir)
 	ld->hsm = new_subdaemon(ld, ld, "lightningd_hsm",
 				hsm_status_wire_type_name,
 				hsm_control_wire_type_name,
-				hsm_finished, -1);
+				hsm_status, hsm_finished, -1);
 	if (!ld->hsm)
 		err(1, "Could not subdaemon hsm");
 

lightningd/peer_control.c

@@ -16,28 +16,6 @@
 #include <sys/socket.h>
 #include <sys/types.h>
 
-struct peer {
-	struct lightningd *ld;
-
-	/* Unique ID (works before we know their pubkey) */
-	u64 unique_id;
-
-	/* Inside ld->peers. */
-	struct list_node list;
-
-	/* What stage is this in? */
-	struct subdaemon *owner;
-
-	/* ID of peer (NULL before initial handshake). */
-	struct pubkey *id;
-
-	/* Our fd to the peer. */
-	int fd;
-
-	/* HSM connection for this peer. */
-	int hsmfd;
-};
-
 static void destroy_peer(struct peer *peer)
 {
 	list_del_from(&peer->ld->peers, &peer->list);
@@ -123,7 +101,7 @@ static void peer_got_hsmfd(struct subdaemon *hsm, const u8 *msg,
 				    "lightningd_handshake",
 				    handshake_status_wire_type_name,
 				    handshake_control_wire_type_name,
-				    NULL,
+				    NULL, NULL,
 				    peer->hsmfd, -1);
 	if (!peer->owner) {
 		log_unusual(peer->ld->log, "Could not subdaemon handshake: %s",

lightningd/peer_control.h

@@ -3,7 +3,27 @@
 #include "config.h"
 #include <stdbool.h>
 
-struct lightningd;
+struct peer {
+	struct lightningd *ld;
+
+	/* Unique ID (works before we know their pubkey) */
+	u64 unique_id;
+
+	/* Inside ld->peers. */
+	struct list_node list;
+
+	/* What stage is this in? */
+	struct subdaemon *owner;
+
+	/* ID of peer (NULL before initial handshake). */
+	struct pubkey *id;
+
+	/* Our fd to the peer. */
+	int fd;
+
+	/* HSM connection for this peer. */
+	int hsmfd;
+};
 
 struct peer *peer_by_unique_id(struct lightningd *ld, u64 unique_id);
 

lightningd/subdaemon.c

@@ -151,17 +151,34 @@ fail:
 
 static struct io_plan *status_read(struct io_conn *conn, struct subdaemon *sd);
 
+static struct io_plan *status_process_fd(struct io_conn *conn,
+					 struct subdaemon *sd)
+{
+	const tal_t *tmpctx = tal_tmpctx(sd);
+
+	/* Ensure we free it iff callback doesn't tal_steal it. */
+	tal_steal(tmpctx, sd->status_in);
+	sd->statuscb(sd, sd->status_in, sd->status_fd_in);
+	tal_free(tmpctx);
+	sd->status_in = NULL;
+	return status_read(conn, sd);
+}
+
 static struct io_plan *status_process(struct io_conn *conn, struct subdaemon *sd)
 {
 	int type = fromwire_peektype(sd->status_in);
 	const char *str;
 	int str_len;
+	const tal_t *tmpctx = tal_tmpctx(sd);
 
 	if (type == -1) {
 		log_unusual(sd->log, "ERROR: Invalid status output");
 		return io_close(conn);
 	}
 
+	/* If not stolen, we'll free this below. */
+	tal_steal(tmpctx, sd->status_in);
+
 	/* If it's a string. */
 	str_len = tal_count(sd->status_in) - sizeof(be16);
 	str = (const char *)sd->status_in + sizeof(be16);
@@ -173,12 +190,26 @@ static struct io_plan *status_process(struct io_conn *conn, struct subdaemon *sd
 			    sd->statusname(type), str_len, str);
 	else {
 		log_info(sd->log, "UPDATE %s", sd->statusname(type));
-		tal_free(sd->last_status);
+		if (sd->statuscb) {
-		/* Keep last status around. */
+			enum subdaemon_status s = sd->statuscb(sd,
-		sd->last_status = tal_steal(sd, sd->status_in);
+							       sd->status_in,
-		sd->status_in = NULL;
+							       -1);
+			switch (s) {
+			case STATUS_NEED_FD:
+				tal_steal(sd, sd->status_in);
+				tal_free(tmpctx);
+				return io_recv_fd(conn, &sd->status_fd_in,
+						  status_process_fd, sd);
+			case STATUS_COMPLETE:
+				break;
+			default:
+				fatal("Unknown statuscb return for %s:%s: %u",
+				      sd->name, sd->statusname(type), s);
+			}
+		}
 	}
-	sd->status_in = tal_free(sd->status_in);
+	sd->status_in = NULL;
+	tal_free(tmpctx);
 	return status_read(conn, sd);
 }
 
@@ -214,6 +245,8 @@ struct subdaemon *new_subdaemon(const tal_t *ctx,
 				const char *name,
 				const char *(*statusname)(int status),
 				const char *(*reqname)(int req),
+				enum subdaemon_status (*statuscb)
+				(struct subdaemon *, const u8 *, int fd),
 				void (*finished)(struct subdaemon *, int),
 				...)
 {
@@ -235,7 +268,7 @@ struct subdaemon *new_subdaemon(const tal_t *ctx,
 	sd->name = name;
 	sd->finished = finished;
 	sd->statusname = statusname;
-	sd->last_status = NULL;
+	sd->statuscb = statuscb;
 	list_head_init(&sd->reqs);
 	tal_add_destructor(sd, destroy_subdaemon);
 

lightningd/subdaemon.h

@@ -8,6 +8,11 @@
 
 struct io_conn;
 
+enum subdaemon_status {
+	STATUS_NEED_FD,
+	STATUS_COMPLETE
+};
+
 /* One of our subdaemons. */
 struct subdaemon {
 	/* Name, like John, or "lightningd_hsm" */
@@ -24,15 +29,15 @@ struct subdaemon {
 	/* For logging */
 	struct log *log;
 
+	/* Callback when status comes in. */
+	enum subdaemon_status (*statuscb)(struct subdaemon *, const u8 *, int);
 	const char *(*statusname)(int status);
 	const char *(*reqname)(int req);
 	void (*finished)(struct subdaemon *sd, int status);
 
 	/* Buffer for input. */
 	u8 *status_in;
-
+	int status_fd_in;
-	/* Status handler puts last status msg here. */
-	u8 *last_status;
 
 	/* Requests queue up here. */
 	struct list_head reqs;
@@ -45,16 +50,21 @@ struct subdaemon {
  * @name: basename of daemon
  * @statusname: function to get name from status messages
  * @reqname: function to get name from request messages, or NULL if no requests.
+ * @statuscb: function to call when status message received (or NULL)
  * @finished: function to call when it's finished (with exit status).
  * @...: the fds to hand as fd 3, 4... terminated with -1.
  *
- * You should free it from finished().
+ * @statuscb is called with fd == -1 when a status message is
+ * received; if it returns STATUS_NEED_FD, we read an fd from the
+ * daemon and call it again with that as the third arg.
  */
 struct subdaemon *new_subdaemon(const tal_t *ctx,
 				struct lightningd *ld,
 				const char *name,
 				const char *(*statusname)(int status),
 				const char *(*reqname)(int req),
+				enum subdaemon_status (*statuscb)
+				(struct subdaemon *, const u8 *, int fd),
 				void (*finished)(struct subdaemon *, int), ...);
 
 /**