Hi -

Pushed the following additional patch to fche/debuginfod-progress-env.
Hand-tested on a ginormous download with various length timeouts; hard
to test reliably within the elfutils testsuite (esp after removal of
that induced debuginfod-side wait).

commit beb09f4aa0634e89cf3ed513c66abec6bc65009f
Author: Frank Ch. Eigler <f...@redhat.com>
Date:   Fri Dec 13 11:51:37 2019 -0500

    debuginfod client: progress notification v2

diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 7a0a17519f42..55aab7b5b9a1 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -99,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 = 30;
+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.  */
@@ -399,8 +399,18 @@ debuginfod_query_server (debuginfod_client *c,
       goto out;
     }
 
-  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);
@@ -492,7 +502,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);
@@ -538,7 +551,7 @@ debuginfod_query_server (debuginfod_client *c,
         {
           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 */
             {
 #ifdef CURLINFO_SIZE_DOWNLOAD_T
@@ -557,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,
@@ -693,25 +708,12 @@ default_progressfn (debuginfod_client *c, long a, long b)
 {
   (void) c;
 
-  int fd = open(getenv(DEBUGINFOD_PROGRESS_ENV_VAR),
-               O_APPEND|O_WRONLY|O_CREAT, 0666);
-  if (fd < 0)
-    goto out;
+  dprintf(STDERR_FILENO,
+          "Downloading from debuginfod %ld/%ld%s", a, b,
+          ((a == b) ? "\n" : "\r"));
+  /* XXX: include URL - stateful */
 
-  char *msg = NULL;
-  int rc = asprintf(&msg,
-                   "Downloading from debuginfod %ld/%ld%s", a, b,
-                   ((a == b) ? "\n" : "\r")); /* XXX: include URL - stateful */
-  if (rc < 0)
-    goto out1;
-
-  (void) write_retry(fd, msg, rc);
-  free (msg);
-
-  out1:
-    close (fd);
-  out:
-    return 0;
+  return 0;
 }
 
 
diff --git a/doc/debuginfod-find.1 b/doc/debuginfod-find.1
index 7f14e8c1411b..023acbb3b722 100644
--- a/doc/debuginfod-find.1
+++ b/doc/debuginfod-find.1
@@ -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 30.
+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
diff --git a/doc/debuginfod_find_debuginfo.3 b/doc/debuginfod_find_debuginfo.3
index 63766b33718d..ea8c61612924 100644
--- a/doc/debuginfod_find_debuginfo.3
+++ b/doc/debuginfod_find_debuginfo.3
@@ -163,9 +163,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 30.
+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
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index be6d56575a3c..6ef30256749a 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -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,7 +153,7 @@ cmp $filename F/prog2
 cat vlog
 grep -q Progress vlog
 tempfiles vlog
-filename=`testrun env DEBUGINFOD_PROGRESS=vlog2 
${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

Reply via email to