Libguestfs - Oct 2015 - [PATCH 0/2] New APIs: set-identifier, get-identifier

This is very useful for debugging multithreaded programs.

Rich.

Add a per-handle identifier.  The main effect of this is to change
trace output from:

  libguestfs: trace: foo

to:

  libguestfs: trace: ID: foo

which makes it simpler to follow traces from multi-threaded programs.
---
 generator/actions.ml   | 43 +++++++++++++++++++++++++++++++++++++++++++
 src/events.c           | 10 ++++++++--
 src/guestfs-internal.h |  1 +
 src/handle.c           | 32 ++++++++++++++++++++++++++++++++
 src/launch.c           |  5 +++++
 5 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/generator/actions.ml b/generator/actions.ml
index c1755b9..ae4bb0c 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -3369,6 +3369,49 @@ To download to the current directory, use C<.> as
in:
 
 Wildcards cannot be used." };
 
+  { defaults with
+    name = "set_identifier"; added = (1, 31, 14);
+    style = RErr, [String "identifier"], [];
+    fish_alias = ["identifier"];
+    blocking = false;
+    shortdesc = "set the handle identifier";
+    longdesc = "\
+This is an informative string which the caller may optionally
+set in the handle.  It is printed in various places, allowing
+the current handle to be identified in debugging output.
+
+One important place is when tracing is enabled.  If the
+identifier string is not an empty string, then trace messages
+change from this:
+
+ libguestfs: trace: get_tmpdir
+ libguestfs: trace: get_tmpdir = \"/tmp\"
+
+to this:
+
+ libguestfs: trace: ID: get_tmpdir
+ libguestfs: trace: ID: get_tmpdir = \"/tmp\"
+
+where C<ID> is the identifier string set by this call.
+
+The identifier must only contain alphanumeric ASCII characters,
+underscore and minus sign.  The default is the empty string.
+
+See also C<guestfs_set_program>, C<guestfs_set_trace>,
+C<guestfs_get_identifier>" };
+
+  { defaults with
+    name = "get_identifier"; added = (1, 31, 14);
+    style = RConstString "identifier", [], [];
+    blocking = false;
+    tests = [
+      InitNone, Always, TestRun (
+        [["get_identifier"]]), []
+    ];
+    shortdesc = "get the handle identifier";
+    longdesc = "\
+Get the handle identifier.  See C<guestfs_set_identifier>." };
+
 ]
 
 (* daemon_functions are any functions which cause some action
diff --git a/src/events.c b/src/events.c
index d5efefe..5fed0c0 100644
--- a/src/events.c
+++ b/src/events.c
@@ -20,6 +20,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <assert.h>
 
 #include "c-ctype.h"
@@ -136,7 +137,7 @@ guestfs_int_call_callbacks_message (guestfs_h *g, uint64_t
event,
     /* APPLIANCE =>  <buf>
      * LIBRARY =>    libguestfs: <buf>\n
      * WARNING =>    libguestfs: warning: <buf>\n
-     * TRACE =>      libguestfs: trace: <buf>\n  (RHBZ#673479)
+     * TRACE =>      libguestfs: trace: [<ID>:] <buf>\n 
(RHBZ#673479)
      */
 
     if (event != GUESTFS_EVENT_APPLIANCE)
@@ -145,8 +146,13 @@ guestfs_int_call_callbacks_message (guestfs_h *g, uint64_t
event,
     if (event == GUESTFS_EVENT_WARNING)
       fputs ("warning: ", stderr);
 
-    if (event == GUESTFS_EVENT_TRACE)
+    if (event == GUESTFS_EVENT_TRACE) {
       fputs ("trace: ", stderr);
+      if (STRNEQ (g->identifier, "")) {
+        fputs (g->identifier, stderr);
+        fputs (": ", stderr);
+      }
+    }
 
     /* Special or non-printing characters in the buffer must be
      * escaped (RHBZ#731744).  The buffer can contain any 8 bit
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 72f9968..34b5f1d 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -371,6 +371,7 @@ struct guestfs_h
   struct hv_param *hv_params;   /* Extra hv parameters. */
 
   char *program;                /* Program name. */
+  char *identifier;             /* Handle identifier. */
 
   /* Array of drives added by add-drive* APIs.
    *
diff --git a/src/handle.c b/src/handle.c
index e2d0aa3..ff109ea 100644
--- a/src/handle.c
+++ b/src/handle.c
@@ -32,6 +32,7 @@
 
 #include "glthread/lock.h"
 #include "ignore-value.h"
+#include "c-ctype.h"
 
 #include "guestfs.h"
 #include "guestfs-internal.h"
@@ -128,6 +129,9 @@ guestfs_create_flags (unsigned flags, ...)
 #endif
   if (!g->program) goto error;
 
+  g->identifier = strdup ("");
+  if (!g->identifier) goto error;
+
   if (guestfs_int_set_backend (g, DEFAULT_BACKEND) == -1) {
     warning (g, _("libguestfs was built with an invalid default backend,
using 'direct' instead"));
     if (guestfs_int_set_backend (g, "direct") == -1) {
@@ -667,6 +671,34 @@ guestfs_impl_get_program (guestfs_h *g)
 }
 
 int
+guestfs_impl_set_identifier (guestfs_h *g, const char *identifier)
+{
+  size_t i, len;
+
+  /* Check the identifier contains only permitted characters. */
+  len = strlen (identifier);
+  for (i = 0; i < len; ++i) {
+    char c = identifier[i];
+
+    if (!c_isalnum (c) && c != '_' && c != '-')
{
+      error (g, _("identifier must contain only alphanumeric characters,
underscore or minus sign"));
+      return -1;
+    }
+  }
+
+  free (g->identifier);
+  g->identifier = safe_strdup (g, identifier);
+
+  return 0;
+}
+
+const char *
+guestfs_impl_get_identifier (guestfs_h *g)
+{
+  return g->identifier;
+}
+
+int
 guestfs_impl_set_backend (guestfs_h *g, const char *method)
 {
   if (guestfs_int_set_backend (g, method) == -1) {
diff --git a/src/launch.c b/src/launch.c
index 99c5582..ddd7091 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -75,6 +75,8 @@ guestfs_impl_launch (guestfs_h *g)
     CLEANUP_FREE char *backend = guestfs_get_backend (g);
 
     debug (g, "launch: program=%s", g->program);
+    if (STRNEQ (g->identifier, ""))
+      debug (g, "launch: identifier=%s", g->identifier);
     debug (g, "launch:
version=%"PRIi64".%"PRIi64".%"PRIi64"%s",
            v->major, v->minor, v->release, v->extra);
 
@@ -359,6 +361,7 @@ guestfs_int_appliance_command_line (guestfs_h *g, const char
*appliance_dev,
      "%s"                       /* verbose */
      "%s"                       /* network */
      " TERM=%s"                 /* TERM environment variable */
+     "%s%s"                     /* handle identifier */
      "%s%s",                    /* append */
 #ifdef __arm__
      g->memsize,
@@ -369,6 +372,8 @@ guestfs_int_appliance_command_line (guestfs_h *g, const char
*appliance_dev,
      g->verbose ? " guestfs_verbose=1" : "",
      g->enable_network ? " guestfs_network=1" : "",
      term ? term : "linux",
+     STRNEQ (g->identifier, "") ? " guestfs_identifier="
: "",
+     g->identifier,
      g->append ? " " : "", g->append ? g->append :
"");
 
   return ret;
-- 
2.5.0

---
 df/parallel.c                                 | 6 ++++++
 src/guestfs.pod                               | 3 +++
 tests/mount-local/test-parallel-mount-local.c | 2 ++
 tests/parallel/test-parallel.c                | 6 ++++++
 4 files changed, 17 insertions(+)

diff --git a/df/parallel.c b/df/parallel.c
index 2d1d1d3..114cc27 100644
--- a/df/parallel.c
+++ b/df/parallel.c
@@ -149,6 +149,7 @@ worker_thread (void *thread_data_vp)
     size_t output_len = 0;
     guestfs_h *g;
     int err;
+    char id[64];
 
     /* Take the next domain from the list. */
     if (thread_data->verbose)
@@ -191,6 +192,11 @@ worker_thread (void *thread_data_vp)
       return &thread_data->r;
     }
 
+    /* Set the handle identifier so we can tell threads apart. */
+    snprintf (id, sizeof id, "thread_%zu_domain_%zu",
+              thread_data->thread_num, i);
+    guestfs_set_identifier (g, id);
+
     /* Copy some settings from the options guestfs handle. */
     guestfs_set_trace (g, thread_data->trace);
     guestfs_set_verbose (g, thread_data->verbose);
diff --git a/src/guestfs.pod b/src/guestfs.pod
index 248a4d0..349aa8d 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -1328,6 +1328,9 @@ safe to be called from multiple threads without a mutex.
 See the graphical program guestfs-browser for one possible
 architecture for multithreaded programs using libvirt and libguestfs.
 
+Use L</guestfs_set_identifier> to make it simpler to identify threads
+in trace output.
+
 =head2 PATH
 
 Libguestfs needs a supermin appliance, which it finds by looking along
diff --git a/tests/mount-local/test-parallel-mount-local.c
b/tests/mount-local/test-parallel-mount-local.c
index 0210330..873d20a 100644
--- a/tests/mount-local/test-parallel-mount-local.c
+++ b/tests/mount-local/test-parallel-mount-local.c
@@ -177,6 +177,8 @@ start_thread (void *statevp)
     pthread_exit (&state->exit_status);
   }
 
+  guestfs_set_identifier (g, state->mp);
+
   if (guestfs_add_drive_scratch (g, 512*1024*1024, -1) == -1)
     goto error;
   if (guestfs_launch (g) == -1)
diff --git a/tests/parallel/test-parallel.c b/tests/parallel/test-parallel.c
index d018ec7..85182f2 100644
--- a/tests/parallel/test-parallel.c
+++ b/tests/parallel/test-parallel.c
@@ -43,6 +43,7 @@
 #define NR_THREADS 5
 
 struct thread_state {
+  size_t thread_num;            /* Thread number. */
   pthread_t thread;             /* Thread handle. */
   int exit_status;              /* Thread exit status. */
 };
@@ -89,6 +90,7 @@ main (int argc, char *argv[])
   sigaction (SIGQUIT, &sa, NULL);
 
   for (i = 0; i < NR_THREADS; ++i) {
+    threads[i].thread_num = i;
     /* Start the thread. */
     r = pthread_create (&threads[i].thread, NULL, start_thread,
                         &threads[i]);
@@ -117,6 +119,7 @@ start_thread (void *statevp)
   struct thread_state *state = statevp;
   guestfs_h *g;
   time_t start_t, t;
+  char id[64];
 
   time (&start_t);
 
@@ -133,6 +136,9 @@ start_thread (void *statevp)
       pthread_exit (&state->exit_status);
     }
 
+    snprintf (id, sizeof id, "%zu", state->thread_num);
+    guestfs_set_identifier (g, id);
+
     if (guestfs_add_drive_opts (g, "/dev/null",
                                 GUESTFS_ADD_DRIVE_OPTS_FORMAT, "raw",
                                 GUESTFS_ADD_DRIVE_OPTS_READONLY, 1,
-- 
2.5.0