subd: make functions more generic, don't assume 'struct channel'.

This means the caller needs to supply an explicit log to base the
subd log on, and also a callback for error handling.

The callback is kind of ugly, but it gets reworked towards the end
of this series.

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

lightningd/log.c

@@ -161,6 +161,11 @@ new_log(const tal_t *ctx, struct log_book *record, const char *fmt, ...)
 	return log;
 }
 
+struct log_book *get_log_book(const struct log *log)
+{
+	return log->lr;
+}
+
 enum log_level get_log_level(struct log_book *lr)
 {
 	return lr->print_level;

lightningd/log.h

@@ -39,6 +39,8 @@ enum log_level get_log_level(struct log_book *lr);
 void set_log_level(struct log_book *lr, enum log_level level);
 void set_log_prefix(struct log *log, const char *prefix);
 const char *log_prefix(const struct log *log);
+struct log_book *get_log_book(const struct log *log);
+
 #define set_log_outfn(lr, print, arg)					\
 	set_log_outfn_((lr),						\
 		       typesafe_cb_preargs(void, void *, (print), (arg),\

lightningd/peer_control.c

@@ -277,6 +277,29 @@ static void channel_config(struct lightningd *ld,
 	 ours->channel_reserve_satoshis = 0;
 };
 
+static void channel_errmsg(struct channel *channel,
+			   enum side sender,
+			   const struct channel_id *channel_id,
+			   const char *desc,
+			   const u8 *errmsg)
+{
+	if (sender == LOCAL) {
+		/* If this is NULL, it means subd died. */
+		if (!errmsg) {
+			channel_fail_transient(channel, "%s", desc);
+			return;
+		}
+
+		/* Otherwise overwrite any error we have */
+		if (!channel->error)
+			channel->error = tal_dup_arr(channel, u8,
+						     errmsg, tal_len(errmsg), 0);
+	}
+
+	channel_fail_permanent(channel, "%s ERROR %s",
+			       sender == LOCAL ? "sent" : "received", desc);
+}
+
 /* Gossipd tells us a peer has connected */
 void peer_connected(struct lightningd *ld, const u8 *msg,
 		    int peer_fd, int gossip_fd)
@@ -1229,6 +1252,17 @@ static bool tell_if_missing(const struct channel *channel,
 	return true;
 }
 
+/* Only error onchaind can get is if it dies. */
+static void onchain_error(struct channel *channel,
+			  enum side sender,
+			  const struct channel_id *channel_id,
+			  const char *desc,
+			  const u8 *errmsg)
+{
+	/* FIXME: re-launch? */
+	log_broken(channel->log, "%s", desc);
+}
+
 /* With a reorg, this can get called multiple times; each time we'll kill
  * onchaind (like any other owner), and restart */
 static enum watch_result funding_spent(struct channel *channel,
@@ -1253,8 +1287,10 @@ static enum watch_result funding_spent(struct channel *channel,
 	channel_set_owner(channel, new_channel_subd(ld,
 						    "lightning_onchaind",
 						    channel,
+						    channel->log,
 						    onchain_wire_type_name,
 						    onchain_msg,
+						    onchain_error,
 						    NULL));
 
 	if (!channel->owner) {
@@ -1720,8 +1756,10 @@ static void peer_start_closingd(struct channel *channel,
 	}
 
 	channel_set_owner(channel, new_channel_subd(ld,
-					   "lightning_closingd", channel,
+					   "lightning_closingd",
+					   channel, channel->log,
 					   closing_wire_type_name, closing_msg,
+					   channel_errmsg,
 					   take(&peer_fd), take(&gossip_fd),
 					   NULL));
 	if (!channel->owner) {
@@ -1946,8 +1984,10 @@ static bool peer_start_channeld(struct channel *channel,
 
 	channel_set_owner(channel, new_channel_subd(ld,
 					   "lightning_channeld", channel,
+					   channel->log,
 					   channel_wire_type_name,
 					   channel_msg,
+					   channel_errmsg,
 					   take(&peer_fd),
 					   take(&gossip_fd),
 					   take(&hsmfd), NULL));
@@ -2325,8 +2365,10 @@ static void peer_accept_channel(struct lightningd *ld,
 	channel_set_state(channel, UNINITIALIZED, OPENINGD);
 	channel_set_owner(channel,
 			  new_channel_subd(ld, "lightning_openingd", channel,
+					   channel->log,
 					   opening_wire_type_name,
 					   opening_negotiation_failed,
+					   channel_errmsg,
 					   take(&peer_fd), take(&gossip_fd),
 					   NULL));
 	if (!channel->owner) {
@@ -2394,8 +2436,10 @@ static void peer_offer_channel(struct lightningd *ld,
 	channel_set_owner(fc->channel,
 			  new_channel_subd(ld,
 					   "lightning_openingd", fc->channel,
+					   fc->channel->log,
 					   opening_wire_type_name,
 					   opening_negotiation_failed,
+					   channel_errmsg,
 					   take(&peer_fd), take(&gossip_fd),
 					   NULL));
 	if (!fc->channel->owner) {

lightningd/subd.c

@@ -10,7 +10,6 @@
 #include <common/gen_status_wire.h>
 #include <errno.h>
 #include <fcntl.h>
-#include <lightningd/channel.h>
 #include <lightningd/lightningd.h>
 #include <lightningd/log.h>
 #include <lightningd/log_status.h>
@@ -389,7 +388,7 @@ static bool log_status_fail(struct subd *sd, const u8 *msg)
 
 static bool handle_received_errmsg(struct subd *sd, const u8 *msg)
 {
-	struct channel *channel = sd->channel;
+	void *channel = sd->channel;
 	struct channel_id channel_id;
 	char *desc;
 
@@ -401,14 +400,14 @@ static bool handle_received_errmsg(struct subd *sd, const u8 *msg)
 
 	/* Don't free sd; we're may be about to free channel. */
 	sd->channel = NULL;
-	channel_fail_permanent(channel,
+	if (sd->errcb)
-			       "%s: received ERROR %s", sd->name, desc);
+		sd->errcb(channel, REMOTE, &channel_id, desc, NULL);
 	return true;
 }
 
 static bool handle_sent_errmsg(struct subd *sd, const u8 *msg)
 {
-	struct channel *channel = sd->channel;
+	void *channel = sd->channel;
 	struct channel_id channel_id;
 	char *desc;
 	u8 *errmsg;
@@ -418,13 +417,11 @@ static bool handle_sent_errmsg(struct subd *sd, const u8 *msg)
 		return false;
 
 	/* FIXME: if not all channels failed, hand back to gossipd! */
-	if (!channel->error)
-		channel->error = tal_steal(channel, errmsg);
 
 	/* Don't free sd; we're may be about to free channel. */
 	sd->channel = NULL;
-	channel_fail_permanent(channel,
+	if (sd->errcb)
-			       "%s: sent ERROR %s", sd->name, desc);
+		sd->errcb(channel, LOCAL, &channel_id, desc, errmsg);
 	return true;
 }
 
@@ -573,7 +570,7 @@ static void destroy_subd(struct subd *sd)
 	/* Peer still attached? */
 	if (sd->channel) {
 		/* Don't loop back when we fail it. */
-		struct channel *channel = sd->channel;
+		void *channel = sd->channel;
 		struct db *db = sd->ld->wallet->db;
 		bool outer_transaction;
 
@@ -583,9 +580,11 @@ static void destroy_subd(struct subd *sd)
 		outer_transaction = db->in_transaction;
 		if (!outer_transaction)
 			db_begin_transaction(db);
-		channel_fail_transient(channel,
+		if (sd->errcb)
-				       "Owning subdaemon %s died (%i)",
+			sd->errcb(channel, LOCAL, NULL,
-				       sd->name, status);
+				  tal_fmt(sd, "Owning subdaemon %s died (%i)",
+					  sd->name, status),
+				  NULL);
 		if (!outer_transaction)
 			db_commit_transaction(db);
 	}
@@ -625,10 +624,16 @@ static struct io_plan *msg_setup(struct io_conn *conn, struct subd *sd)
 
 static struct subd *new_subd(struct lightningd *ld,
 			     const char *name,
-			     struct channel *channel,
+			     void *channel,
+			     struct log *base_log,
 			     const char *(*msgname)(int msgtype),
 			     unsigned int (*msgcb)(struct subd *,
 						   const u8 *, const int *fds),
+			     void (*errcb)(void *channel,
+					   enum side sender,
+					   const struct channel_id *channel_id,
+					   const char *desc,
+					   const u8 *errmsg),
 			     va_list *ap)
 {
 	struct subd *sd = tal(ld, struct subd);
@@ -649,9 +654,9 @@ static struct subd *new_subd(struct lightningd *ld,
 		return tal_free(sd);
 	}
 	sd->ld = ld;
-	if (channel) {
+	if (base_log) {
-		sd->log = new_log(sd, channel->peer->log_book, "%s-%s", name,
+		sd->log = new_log(sd, get_log_book(base_log), "%s-%s", name,
-				  log_prefix(channel->log));
+				  log_prefix(base_log));
 	} else {
 		sd->log = new_log(sd, ld->log_book, "%s(%u):", name, sd->pid);
 	}
@@ -660,6 +665,7 @@ static struct subd *new_subd(struct lightningd *ld,
 	sd->must_not_exit = false;
 	sd->msgname = msgname;
 	sd->msgcb = msgcb;
+	sd->errcb = errcb;
 	sd->fds_in = NULL;
 	msg_queue_init(&sd->outq, sd);
 	tal_add_destructor(sd, destroy_subd);
@@ -686,25 +692,32 @@ struct subd *new_global_subd(struct lightningd *ld,
 	struct subd *sd;
 
 	va_start(ap, msgcb);
-	sd = new_subd(ld, name, NULL, msgname, msgcb, &ap);
+	sd = new_subd(ld, name, NULL, NULL, msgname, msgcb, NULL, &ap);
 	va_end(ap);
 
 	sd->must_not_exit = true;
 	return sd;
 }
 
-struct subd *new_channel_subd(struct lightningd *ld,
+struct subd *new_channel_subd_(struct lightningd *ld,
 			       const char *name,
-			   struct channel *channel,
+			       void *channel,
+			       struct log *base_log,
 			       const char *(*msgname)(int msgtype),
 			       unsigned int (*msgcb)(struct subd *, const u8 *,
-						 const int *fds), ...)
+						     const int *fds),
+			       void (*errcb)(void *channel,
+					     enum side sender,
+					     const struct channel_id *channel_id,
+					     const char *desc,
+					     const u8 *errmsg),
+			       ...)
 {
 	va_list ap;
 	struct subd *sd;
 
-	va_start(ap, msgcb);
+	va_start(ap, errcb);
-	sd = new_subd(ld, name, channel, msgname, msgcb, &ap);
+	sd = new_subd(ld, name, channel, base_log, msgname, msgcb, errcb, &ap);
 	va_end(ap);
 	return sd;
 }
@@ -764,7 +777,7 @@ void subd_shutdown(struct subd *sd, unsigned int seconds)
 	tal_free(sd);
 }
 
-void subd_release_channel(struct subd *owner, struct channel *channel)
+void subd_release_channel(struct subd *owner, void *channel)
 {
 	/* If owner is a per-peer-daemon, and not already freeing itself... */
 	if (owner->channel) {

lightningd/subd.h

@@ -5,6 +5,7 @@
 #include <ccan/list/list.h>
 #include <ccan/short_types/short_types.h>
 #include <ccan/tal/tal.h>
+#include <ccan/typesafe_cb/typesafe_cb.h>
 #include <common/msg_queue.h>
 
 struct io_conn;
@@ -26,7 +27,7 @@ struct subd {
 	struct io_conn *conn;
 
 	/* If we are associated with a single channel, this points to it. */
-	struct channel *channel;
+	void *channel;
 
 	/* For logging */
 	struct log *log;
@@ -35,6 +36,13 @@ struct subd {
 	unsigned (*msgcb)(struct subd *, const u8 *, const int *);
 	const char *(*msgname)(int msgtype);
 
+	/* Callback when an errormsg sent/received, or subd died. */
+	void (*errcb)(void *channel,
+		      enum side sender,
+		      const struct channel_id *channel_id,
+		      const char *desc,
+		      const u8 *errmsg);
+
 	/* Buffer for input. */
 	u8 *msg_in;
 
@@ -77,6 +85,7 @@ struct subd *new_global_subd(struct lightningd *ld,
  * @ld: global state
  * @name: basename of daemon
  * @channel: channel to associate.
+ * @base_log: log to use (actually makes a copy so it has name in prefix)
  * @msgname: function to get name from messages
  * @msgcb: function to call (inside db transaction) when non-fatal message received (or NULL)
  * @...: NULL-terminated list of pointers to  fds to hand as fd 3, 4...
@@ -86,14 +95,27 @@ struct subd *new_global_subd(struct lightningd *ld,
  * that many @fds are received before calling again.  If it returns -1, the
  * subdaemon is shutdown.
  */
-struct subd *new_channel_subd(struct lightningd *ld,
+struct subd *new_channel_subd_(struct lightningd *ld,
 			       const char *name,
-			      struct channel *channel,
+			       void *channel,
+			       struct log *base_log,
 			       const char *(*msgname)(int msgtype),
 			       unsigned int (*msgcb)(struct subd *, const u8 *,
 						     const int *fds),
+			       void (*errcb)(void *channel,
+					     enum side sender,
+					     const struct channel_id *channel_id,
+					     const char *desc,
+					     const u8 *errmsg),
 			       ...);
 
+#define new_channel_subd(ld, name, channel, log, msgname, msgcb, errcb, ...) \
+	new_channel_subd_((ld), (name), (channel), (log), (msgname), (msgcb), \
+			  typesafe_cb_postargs(void, void *, (errcb),	\
+					       (channel), enum side,	\
+					       const struct channel_id *, \
+					       const char *, const u8 *), \
+			  __VA_ARGS__)
 /**
  * subd_raw - raw interface to get a subdaemon on an fd (for HSM)
  * @ld: global state
@@ -144,14 +166,14 @@ void subd_req_(const tal_t *ctx,
 	       void *replycb_data);
 
 /**
- * subd_release_channel - shut down a subdaemon which no longer owns the channel.
+ * subd_release_channel - shut down a subdaemon which no longer owns the channe;.
  * @owner: subd which owned channel.
  * @channel: channel to release.
  *
  * If the subdaemon is not already shutting down, and it is a per-channel
  * subdaemon, this shuts it down.
  */
-void subd_release_channel(struct subd *owner, struct channel *channel);
+void subd_release_channel(struct subd *owner, void *channel);
 
 /**
  * subd_shutdown - try to politely shut down a subdaemon.

wallet/test/run-wallet.c

@@ -325,15 +325,21 @@ void log_io(struct log *log UNNEEDED, enum log_level dir UNNEEDED, const char *c
 /* Generated stub for logv_add */
 void logv_add(struct log *log UNNEEDED, const char *fmt UNNEEDED, va_list ap UNNEEDED)
 { fprintf(stderr, "logv_add called!\n"); abort(); }
-/* Generated stub for new_channel_subd */
+/* Generated stub for new_channel_subd_ */
-struct subd *new_channel_subd(struct lightningd *ld UNNEEDED,
+struct subd *new_channel_subd_(struct lightningd *ld UNNEEDED,
 			       const char *name UNNEEDED,
-			      struct channel *channel UNNEEDED,
+			       void *channel UNNEEDED,
+			       struct log *base_log UNNEEDED,
 			       const char *(*msgname)(int msgtype) UNNEEDED,
 			       unsigned int (*msgcb)(struct subd * UNNEEDED, const u8 * UNNEEDED,
 						     const int *fds) UNNEEDED,
+			       void (*errcb)(void *channel UNNEEDED,
+					     enum side sender UNNEEDED,
+					     const struct channel_id *channel_id UNNEEDED,
+					     const char *desc UNNEEDED,
+					     const u8 *errmsg) UNNEEDED,
 			       ...)
-{ fprintf(stderr, "new_channel_subd called!\n"); abort(); }
+{ fprintf(stderr, "new_channel_subd_ called!\n"); abort(); }
 /* Generated stub for new_json_result */
 struct json_result *new_json_result(const tal_t *ctx UNNEEDED)
 { fprintf(stderr, "new_json_result called!\n"); abort(); }
@@ -379,7 +385,7 @@ char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED,
 		     struct channel_id *channel_id UNNEEDED)
 { fprintf(stderr, "sanitize_error called!\n"); abort(); }
 /* Generated stub for subd_release_channel */
-void subd_release_channel(struct subd *owner UNNEEDED, struct channel *channel UNNEEDED)
+void subd_release_channel(struct subd *owner UNNEEDED, void *channel UNNEEDED)
 { fprintf(stderr, "subd_release_channel called!\n"); abort(); }
 /* Generated stub for subd_req_ */
 void subd_req_(const tal_t *ctx UNNEEDED,