]> git.ipfire.org Git - thirdparty/elfutils.git/commitdiff
debuginfod: usability tweaks, incl. $DEBUGINFOD_PROGRESS client support
authorFrank Ch. Eigler <fche@redhat.com>
Wed, 4 Dec 2019 20:51:12 +0000 (15:51 -0500)
committerFrank Ch. Eigler <fche@redhat.com>
Thu, 19 Dec 2019 14:57:13 +0000 (09:57 -0500)
This facility allows a default progress-printing function to be
installed if the given environment variable is set.  Some larger usage
experience (systemtap fetching kernels) indicates the default timeout
is too short, so forked it into a connection timeout (default short)
and a transfer timeout (default unlimited).

Signed-off-by: Frank Ch. Eigler <fche@redhat.com>
debuginfod/ChangeLog
debuginfod/debuginfod-client.c
debuginfod/debuginfod.h
doc/ChangeLog
doc/debuginfod-find.1
doc/debuginfod_find_debuginfo.3
tests/ChangeLog
tests/run-debuginfod-find.sh

index bb7c5687bed182cf00a2cb9df32cb5a17254dc82..28a507196cacd1ab73957ca3652fb2da7eb7a819 100644 (file)
@@ -1,3 +1,13 @@
+2019-12-19  Frank Ch. Eigler  <fche@redhat.com>
+
+       * debuginfod-client.c (default_progressfn): New function.
+       (debuginfod_begin): Use it if $DEBUGINFOD_PROGRESS set.
+       (server_timeout): Bump to 30 seconds.
+       (debuginfod_query_server): Call progressfn -after- rather than
+       before curl ops, to make it likely that a successful transfer
+       results in final a=b call.  Tweak cleanup sequence.
+       * debuginfod.h: Document $DEBUGINFOD_PROGRESS name.
+
 2019-12-09  Mark Wielaard  <mark@klomp.org>
 
        * debuginfod-client.c (debuginfod_query_server): Check
index ab7b4e1346d65409687fdd1614f5d9356170473f..9a4a0e0fc421a8cf96daf8d71c340ca0bcf0b1f0 100644 (file)
@@ -40,6 +40,7 @@
 
 #include "config.h"
 #include "debuginfod.h"
+#include "system.h"
 #include <assert.h>
 #include <dirent.h>
 #include <stdio.h>
@@ -98,16 +99,16 @@ static const time_t cache_default_max_unused_age_s = 604800; /* 1 week */
 static const char *cache_default_name = ".debuginfod_client_cache";
 static const char *cache_path_envvar = DEBUGINFOD_CACHE_PATH_ENV_VAR;
 
-/* URLs of debuginfods, separated by url_delim.
-   This env var must be set for debuginfod-client to run.  */
+/* URLs of debuginfods, separated by url_delim. */
 static const char *server_urls_envvar = DEBUGINFOD_URLS_ENV_VAR;
 static const char *url_delim =  " ";
 static const char url_delim_char = ' ';
 
-/* Timeout for debuginfods, in seconds.
-   This env var must be set for debuginfod-client to run.  */
+/* Timeout for debuginfods, in seconds. */
 static const char *server_timeout_envvar = DEBUGINFOD_TIMEOUT_ENV_VAR;
-static int server_timeout = 5;
+static const long default_connect_timeout = 5;
+static const long default_transfer_timeout = -1; /* unlimited */
+
 
 /* Data associated with a particular CURL easy handle. Passed to
    the write callback.  */
@@ -400,8 +401,18 @@ debuginfod_query_server (debuginfod_client *c,
       return fd;
     }
 
-  if (getenv(server_timeout_envvar))
-    server_timeout = atoi (getenv(server_timeout_envvar));
+  long connect_timeout = default_connect_timeout;
+  long transfer_timeout = default_transfer_timeout;
+  const char* timeout_envvar = getenv(server_timeout_envvar);
+  if (timeout_envvar != NULL)
+    {
+      long ct, tt;
+      rc = sscanf(timeout_envvar, "%ld,%ld", &ct, &tt);
+      if (rc >= 1)
+        connect_timeout = ct;
+      if (rc >= 2)
+        transfer_timeout = tt;
+    }
 
   /* make a copy of the envvar so it can be safely modified.  */
   server_urls = strdup(urls_envvar);
@@ -493,7 +504,10 @@ debuginfod_query_server (debuginfod_client *c,
                        CURLOPT_WRITEFUNCTION,
                        debuginfod_write_callback);
       curl_easy_setopt(data[i].handle, CURLOPT_WRITEDATA, (void*)&data[i]);
-      curl_easy_setopt(data[i].handle, CURLOPT_TIMEOUT, (long) server_timeout);
+      if (connect_timeout >= 0)
+        curl_easy_setopt(data[i].handle, CURLOPT_CONNECTTIMEOUT, connect_timeout);
+      if (transfer_timeout >= 0)
+        curl_easy_setopt(data[i].handle, CURLOPT_TIMEOUT, transfer_timeout);
       curl_easy_setopt(data[i].handle, CURLOPT_FILETIME, (long) 1);
       curl_easy_setopt(data[i].handle, CURLOPT_FOLLOWLOCATION, (long) 1);
       curl_easy_setopt(data[i].handle, CURLOPT_FAILONERROR, (long) 1);
@@ -511,11 +525,32 @@ debuginfod_query_server (debuginfod_client *c,
   long loops = 0;
   do
     {
+      /* Wait 1 second, the minimum DEBUGINFOD_TIMEOUT.  */
+      curl_multi_wait(curlm, NULL, 0, 1000, NULL);
+
+      /* If the target file has been found, abort the other queries.  */
+      if (target_handle != NULL)
+        for (int i = 0; i < num_urls; i++)
+          if (data[i].handle != target_handle)
+            curl_multi_remove_handle(curlm, data[i].handle);
+
+      CURLMcode curlm_res = curl_multi_perform(curlm, &still_running);
+      if (curlm_res != CURLM_OK)
+        {
+          switch (curlm_res)
+            {
+            case CURLM_CALL_MULTI_PERFORM: continue;
+            case CURLM_OUT_OF_MEMORY: rc = -ENOMEM; break;
+            default: rc = -ENETUNREACH; break;
+            }
+          goto out1;
+        }
+
       if (c->progressfn) /* inform/check progress callback */
         {
           loops ++;
           long pa = loops; /* default params for progress callback */
-          long pb = 0;
+          long pb = 0; /* transfer_timeout tempting, but loops != elapsed-time */
           if (target_handle) /* we've committed to a server; report its download progress */
             {
               CURLcode curl_res;
@@ -535,6 +570,8 @@ debuginfod_query_server (debuginfod_client *c,
                 pa = (dl > LONG_MAX ? LONG_MAX : (long)dl);
 #endif
 
+              /* NB: If going through deflate-compressing proxies, this
+                 number is likely to be unavailable, so -1 may show. */
 #ifdef CURLINFO_CURLINFO_CONTENT_LENGTH_DOWNLOAD_T
               curl_off_t cl;
               curl_res = curl_easy_getinfo(target_handle,
@@ -555,27 +592,6 @@ debuginfod_query_server (debuginfod_client *c,
           if ((*c->progressfn) (c, pa, pb))
             break;
         }
-
-      /* Wait 1 second, the minimum DEBUGINFOD_TIMEOUT.  */
-      curl_multi_wait(curlm, NULL, 0, 1000, NULL);
-
-      /* If the target file has been found, abort the other queries.  */
-      if (target_handle != NULL)
-        for (int i = 0; i < num_urls; i++)
-          if (data[i].handle != target_handle)
-            curl_multi_remove_handle(curlm, data[i].handle);
-
-      CURLMcode curlm_res = curl_multi_perform(curlm, &still_running);
-      if (curlm_res != CURLM_OK)
-        {
-          switch (curlm_res)
-            {
-            case CURLM_CALL_MULTI_PERFORM: continue;
-            case CURLM_OUT_OF_MEMORY: rc = -ENOMEM; break;
-            default: rc = -ENETUNREACH; break;
-            }
-          goto out1;
-        }
     } while (still_running);
 
   /* Check whether a query was successful. If so, assign its handle
@@ -674,9 +690,9 @@ debuginfod_query_server (debuginfod_client *c,
 
   curl_multi_cleanup(curlm);
   unlink (target_cache_tmppath);
+  close (fd); /* before the rmdir, otherwise it'll fail */
   (void) rmdir (target_cache_dir); /* nop if not empty */
   free(data);
-  close (fd);
 
  out0:
   free (server_urls);
@@ -685,6 +701,22 @@ debuginfod_query_server (debuginfod_client *c,
   return rc;
 }
 
+
+/* Activate a basic form of progress tracing */
+static int
+default_progressfn (debuginfod_client *c, long a, long b)
+{
+  (void) c;
+
+  dprintf(STDERR_FILENO,
+          "Downloading from debuginfod %ld/%ld%s", a, b,
+          ((a == b) ? "\n" : "\r"));
+  /* XXX: include URL - stateful */
+
+  return 0;
+}
+
+
 /* See debuginfod.h  */
 debuginfod_client  *
 debuginfod_begin (void)
@@ -693,7 +725,12 @@ debuginfod_begin (void)
   size_t size = sizeof (struct debuginfod_client);
   client = (debuginfod_client *) malloc (size);
   if (client != NULL)
-    client->progressfn = NULL;
+    {
+      if (getenv(DEBUGINFOD_PROGRESS_ENV_VAR))
+       client->progressfn = default_progressfn;
+      else
+       client->progressfn = NULL;
+    }
   return client;
 }
 
index 6b1b1cc3e8ae04e76f4595219eb56581951d26ff..33fae863d3703456d1a07c9a8485f04a92af3b04 100644 (file)
@@ -33,6 +33,7 @@
 #define DEBUGINFOD_URLS_ENV_VAR "DEBUGINFOD_URLS"
 #define DEBUGINFOD_CACHE_PATH_ENV_VAR "DEBUGINFOD_CACHE_PATH"
 #define DEBUGINFOD_TIMEOUT_ENV_VAR "DEBUGINFOD_TIMEOUT"
+#define DEBUGINFOD_PROGRESS_ENV_VAR "DEBUGINFOD_PROGRESS"
 
 /* Handle for debuginfod-client connection.  */
 typedef struct debuginfod_client debuginfod_client;
index 00a61ac3330f409ad45020d00b4810c8e3f8e064..9542161791e4954af1e3acb5ff5c12ea4eac7faa 100644 (file)
@@ -1,3 +1,9 @@
+2019-12-04  Frank Ch. Eigler  <fche@redhat.com>
+
+       * debuginfod-find.1: Bump default timeout to 30.
+       * debuginfod_find_debuginfo.3: Ditto.
+       Document $DEBUGINFOD_PROGRESS.
+
 2019-09-02  Mark Wielaard  <mark@klomp.org>
 
        * readelf.1 (symbols): Add optional section name.
index a759ecbaf2ab1020e77fc88d240ac6c4484847e7..023acbb3b722e908946ea1501379cdeda063bb4c 100644 (file)
@@ -119,9 +119,13 @@ debuginfod instances.  Alternate URL prefixes are separated by space.
 
 .TP 21
 .B DEBUGINFOD_TIMEOUT
-This environment variable governs the timeout for each debuginfod HTTP
-connection.  A server that fails to respond within this many seconds
-is skipped.  The default is 5.
+This environment variable governs the timeouts for each debuginfod
+HTTP connection.  One or two comma-separated numbers may be given.
+The first is the number of seconds for the connection establishment
+(CURLOPT_CONNECTTIMEOUT), and the default is 5.  The second is the
+number of seconds for the transfer completion (CURLOPT_TIMEOUT), and
+the default is no timeout.  (Zero or negative also means "no
+timeout".)
 
 .TP 21
 .B DEBUGINFOD_CACHE_PATH
index be8eed09eb88c96fdb213ce385e593611c3b9e73..ea8c61612924e8c2980a4ff8a7166f23843237f0 100644 (file)
@@ -163,9 +163,21 @@ debuginfod instances.  Alternate URL prefixes are separated by space.
 
 .TP 21
 .B DEBUGINFOD_TIMEOUT
-This environment variable governs the timeout for each debuginfod HTTP
-connection.  A server that fails to respond within this many seconds
-is skipped.  The default is 5.
+This environment variable governs the timeouts for each debuginfod
+HTTP connection.  One or two comma-separated numbers may be given.
+The first is the number of seconds for the connection establishment
+(CURLOPT_CONNECTTIMEOUT), and the default is 5.  The second is the
+number of seconds for the transfer completion (CURLOPT_TIMEOUT), and
+the default is no timeout.  (Zero or negative also means "no
+timeout".)
+
+.TP 21
+.B DEBUGINFOD_PROGRESS
+This environment variable governs the default progress function.  If
+set, and if a progressfn is not explicitly set, then the library will
+configure a default progressfn.  This function will append a simple
+progress message periodically to the given file.  Consider using
+"/dev/stderr" on platforms that support it.  The default is nothing.
 
 .TP 21
 .B DEBUGINFOD_CACHE_PATH
index 30bd82562e8a93df7f82716f0b7747e167a4bf40..7103cf5191affb1027fa4e69f251d1f8b5e8f92d 100644 (file)
@@ -1,3 +1,7 @@
+2019-12-04  Frank Ch. Eigler  <fche@redhat.com>
+
+       * run-debuinfod-find.sh: Test $DEBUGINFOD_PROGRESS.
+
 2019-12-11  Omar Sandoval  <osandov@fb.com>
 
        * dwfl-report-segment-coalesce.c: New test.
index 6533996a56d3d891f0345d26fdb13d0171476175..4cf61389a810623ff6020a4349d93ff4f634de36 100755 (executable)
@@ -89,7 +89,7 @@ wait_ready $PORT1 'ready' 1
 export DEBUGINFOD_URLS=http://127.0.0.1:$PORT1/   # or without trailing /
 
 # Be patient when run on a busy machine things might take a bit.
-export DEBUGINFOD_TIMEOUT=10
+export DEBUGINFOD_TIMEOUT=1,10
 
 # We use -t0 and -g0 here to turn off time-based scanning & grooming.
 # For testing purposes, we just sic SIGUSR1 / SIGUSR2 at the process.
@@ -153,8 +153,11 @@ cmp $filename F/prog2
 cat vlog
 grep -q Progress vlog
 tempfiles vlog
-filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find executable $BUILDID2`
+filename=`testrun env DEBUGINFOD_PROGRESS=1 ${abs_top_builddir}/debuginfod/debuginfod-find executable $BUILDID2 2>vlog2`
 cmp $filename F/prog2
+cat vlog2
+grep -q Downloading vlog2
+tempfiles vlog2
 filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find source $BUILDID2 ${PWD}/prog2.c`
 cmp $filename ${PWD}/prog2.c