Re: [LEDE-DEV] [PATCH v2] ubox/logread: add re-connect capability

[John Crispin]

On 24/11/17 14:06, Zefir Kurtisi wrote:

When logd is restarted while 'logread -f' is running, the
logread process terminates, which cumbers debugging in
different use-cases.

This patch adds re-connect functionality to logread. In
follow mode, when the ustream to logd is disconnected,
instead of terminating, it tries to re-connect to logd
and re-issue the original request.

Signed-off-by: Zefir Kurtisi 


Hi Zefir,

I have just pushed a different version of the patch, thanks for 
reporting the problem


    John


---
v2: in follow mode, don't exit if the initial
 ubus lookup for logd fails

  log/logread.c | 143 +-
  1 file changed, 91 insertions(+), 52 deletions(-)

diff --git a/log/logread.c b/log/logread.c
index ad06f2a..8229c98 100644
--- a/log/logread.c
+++ b/log/logread.c
@@ -65,6 +65,10 @@ static int log_type = LOG_STDOUT;
  static int log_size, log_udp, log_follow, log_trailer_null = 0;
  static int log_timestamp;
  static int last_errno = 0;
+static struct ubus_context *ctx;
+static int lines;
+
+static void logread_reconnect_cb(struct uloop_timeout *timeout);
  
  static const char* getcodetext(int value, CODE *codetable) {

CODE *i;
@@ -268,29 +272,82 @@ static void logread_fd_data_cb(struct ustream *s, int 
bytes)
}
  }
  
+/*

+ * Reconnect Handling
+ * while following log
+ * - after logd removal
+ *   - destroy ustream
+ *   - cyclically try to re-connect to new logd object
+ *   - re-issue original request
+ *
+ * Note: if a re-connection appears while a 'logread -l' request is active, the
+ *   number of returned lines will not match (i.e. you get some lines from
+ *   the old instance plus the number of lines requested from the new one)
+ */
+
+/* flag to prevent printing error messages during reconnect cycles */
+static int object_reconnect_active;
+
+static void logread_restart_reconnect_timer(struct uloop_timeout *timeout)
+{
+   const int LOG_RECONNECT_TIMEOUT_MS = 250;
+   uloop_timeout_set(timeout, LOG_RECONNECT_TIMEOUT_MS);
+}
+
  static void logread_fd_state_cb(struct ustream *s)
  {
-   uloop_end();
+   static struct uloop_timeout ubus_timer;
+   /* force re-opening of stream */
+   s->free(s);
+
+   object_reconnect_active = 1;
+   ubus_timer.cb = logread_reconnect_cb;
+   logread_restart_reconnect_timer(_timer);
  }
  
  static void logread_fd_cb(struct ubus_request *req, int fd)

  {
static struct ustream_fd test_fd;
-
test_fd.stream.notify_read = logread_fd_data_cb;
test_fd.stream.notify_state = logread_fd_state_cb;
ustream_fd_init(_fd, fd);
  }
  
-int main(int argc, char **argv)

+static int logread_process(void)
  {
+   static struct blob_buf b;
static struct ubus_request req;
-   struct ubus_context *ctx;
uint32_t id;
+   int ret = ubus_lookup_id(ctx, "log", );
+   if (ret) {
+   if (!object_reconnect_active)
+   fprintf(stderr, "Failed to find log object\n");
+   return ret;
+   }
+   blob_buf_init(, 0);
+   blobmsg_add_u8(, "stream", 1);
+   if (lines)
+   blobmsg_add_u32(, "lines", lines);
+   else if (log_follow)
+   blobmsg_add_u32(, "lines", 0);
+
+   ubus_invoke_async(ctx, id, "read", b.head, );
+   req.fd_cb = logread_fd_cb;
+   ubus_complete_request_async(ctx, );
+
+   return 0;
+}
+
+static void logread_reconnect_cb(struct uloop_timeout *timeout)
+{
+   if (logread_process())
+   logread_restart_reconnect_timer(timeout);
+}
+
+int main(int argc, char **argv)
+{
const char *ubus_socket = NULL;
-   int ch, ret, lines = 0;
-   static struct blob_buf b;
-   int tries = 5;
+   int ch, ret;
  
  	signal(SIGPIPE, SIG_IGN);
  
@@ -354,58 +411,40 @@ int main(int argc, char **argv)

}
ubus_add_uloop(ctx);
  
-	/* ugly ugly ugly ... we need a real reconnect logic */

-   do {
-   ret = ubus_lookup_id(ctx, "log", );
-   if (ret) {
-   fprintf(stderr, "Failed to find log object: %s\n", 
ubus_strerror(ret));
-   sleep(1);
-   continue;
-   }
-
-   blob_buf_init(, 0);
-   blobmsg_add_u8(, "stream", 1);
-   blobmsg_add_u8(, "oneshot", !log_follow);
-   if (lines)
-   blobmsg_add_u32(, "lines", lines);
-   else if (log_follow)
-   blobmsg_add_u32(, "lines", 0);
-   if (log_follow) {
-   if (pid_file) {
-   FILE *fp = fopen(pid_file, "w+");
-   if (fp) {
-   fprintf(fp, "%d", getpid());
-   fclose(fp);
-   }
+   if 

[LEDE-DEV] [PATCH v2] ubox/logread: add re-connect capability

[Zefir Kurtisi]

When logd is restarted while 'logread -f' is running, the
logread process terminates, which cumbers debugging in
different use-cases.

This patch adds re-connect functionality to logread. In
follow mode, when the ustream to logd is disconnected,
instead of terminating, it tries to re-connect to logd
and re-issue the original request.

Signed-off-by: Zefir Kurtisi 
---
v2: in follow mode, don't exit if the initial
ubus lookup for logd fails

 log/logread.c | 143 +-
 1 file changed, 91 insertions(+), 52 deletions(-)

diff --git a/log/logread.c b/log/logread.c
index ad06f2a..8229c98 100644
--- a/log/logread.c
+++ b/log/logread.c
@@ -65,6 +65,10 @@ static int log_type = LOG_STDOUT;
 static int log_size, log_udp, log_follow, log_trailer_null = 0;
 static int log_timestamp;
 static int last_errno = 0;
+static struct ubus_context *ctx;
+static int lines;
+
+static void logread_reconnect_cb(struct uloop_timeout *timeout);
 
 static const char* getcodetext(int value, CODE *codetable) {
CODE *i;
@@ -268,29 +272,82 @@ static void logread_fd_data_cb(struct ustream *s, int 
bytes)
}
 }
 
+/*
+ * Reconnect Handling
+ * while following log
+ * - after logd removal
+ *   - destroy ustream
+ *   - cyclically try to re-connect to new logd object
+ *   - re-issue original request
+ *
+ * Note: if a re-connection appears while a 'logread -l' request is active, the
+ *   number of returned lines will not match (i.e. you get some lines from
+ *   the old instance plus the number of lines requested from the new one)
+ */
+
+/* flag to prevent printing error messages during reconnect cycles */
+static int object_reconnect_active;
+
+static void logread_restart_reconnect_timer(struct uloop_timeout *timeout)
+{
+   const int LOG_RECONNECT_TIMEOUT_MS = 250;
+   uloop_timeout_set(timeout, LOG_RECONNECT_TIMEOUT_MS);
+}
+
 static void logread_fd_state_cb(struct ustream *s)
 {
-   uloop_end();
+   static struct uloop_timeout ubus_timer;
+   /* force re-opening of stream */
+   s->free(s);
+
+   object_reconnect_active = 1;
+   ubus_timer.cb = logread_reconnect_cb;
+   logread_restart_reconnect_timer(_timer);
 }
 
 static void logread_fd_cb(struct ubus_request *req, int fd)
 {
static struct ustream_fd test_fd;
-
test_fd.stream.notify_read = logread_fd_data_cb;
test_fd.stream.notify_state = logread_fd_state_cb;
ustream_fd_init(_fd, fd);
 }
 
-int main(int argc, char **argv)
+static int logread_process(void)
 {
+   static struct blob_buf b;
static struct ubus_request req;
-   struct ubus_context *ctx;
uint32_t id;
+   int ret = ubus_lookup_id(ctx, "log", );
+   if (ret) {
+   if (!object_reconnect_active)
+   fprintf(stderr, "Failed to find log object\n");
+   return ret;
+   }
+   blob_buf_init(, 0);
+   blobmsg_add_u8(, "stream", 1);
+   if (lines)
+   blobmsg_add_u32(, "lines", lines);
+   else if (log_follow)
+   blobmsg_add_u32(, "lines", 0);
+
+   ubus_invoke_async(ctx, id, "read", b.head, );
+   req.fd_cb = logread_fd_cb;
+   ubus_complete_request_async(ctx, );
+
+   return 0;
+}
+
+static void logread_reconnect_cb(struct uloop_timeout *timeout)
+{
+   if (logread_process())
+   logread_restart_reconnect_timer(timeout);
+}
+
+int main(int argc, char **argv)
+{
const char *ubus_socket = NULL;
-   int ch, ret, lines = 0;
-   static struct blob_buf b;
-   int tries = 5;
+   int ch, ret;
 
signal(SIGPIPE, SIG_IGN);
 
@@ -354,58 +411,40 @@ int main(int argc, char **argv)
}
ubus_add_uloop(ctx);
 
-   /* ugly ugly ugly ... we need a real reconnect logic */
-   do {
-   ret = ubus_lookup_id(ctx, "log", );
-   if (ret) {
-   fprintf(stderr, "Failed to find log object: %s\n", 
ubus_strerror(ret));
-   sleep(1);
-   continue;
-   }
-
-   blob_buf_init(, 0);
-   blobmsg_add_u8(, "stream", 1);
-   blobmsg_add_u8(, "oneshot", !log_follow);
-   if (lines)
-   blobmsg_add_u32(, "lines", lines);
-   else if (log_follow)
-   blobmsg_add_u32(, "lines", 0);
-   if (log_follow) {
-   if (pid_file) {
-   FILE *fp = fopen(pid_file, "w+");
-   if (fp) {
-   fprintf(fp, "%d", getpid());
-   fclose(fp);
-   }
+   if (log_follow) {
+   if (pid_file) {
+   FILE *fp = fopen(pid_file, "w+");
+   if (fp) {
+   fprintf(fp,