Adding configurable download retries

This commit adds two new options:

-r, --max-retries: maximum number of retries for download failures
-d, --retry-delay: initial delay between download retries

With these two is possible to increase the number of retries in case the
user is in a non reliable network, or to turn off retries intirely (by
setting --max-retries to 0). The --retry-delay will also allow more
flexibility for unreliable connections, and will make testing easier
since the --retry-delat can also be set to 0.

Closes #322

Signed-off-by: Castulo Martinez <castulo.martinez@intel.com>

src/curl.c

@@ -567,7 +567,7 @@ static int retry_download_loop(const char *url, char *filename, struct curl_file
 {
 
 	int current_retry = 0;
-	int sleep_time = RETRY_DELAY;
+	int sleep_time = retry_delay;
 	int strategy;
 	int ret;
 
@@ -591,17 +591,34 @@ static int retry_download_loop(const char *url, char *filename, struct curl_file
 		case RETRY_NOW:
 			/* if we have reached the retry limit just return the failure,
 			 * if not try again immediately */
-			if (current_retry <= MAX_TRIES) {
-				continue;
+			if (max_retries) {
+				if (current_retry <= max_retries) {
+					fprintf(stderr, "Retry #%d downloading from %s\n", current_retry, url);
+					continue;
+				} else {
+					fprintf(stderr, "Maximum number of retries reached\n");
+				}
+			} else {
+				fprintf(stderr, "Download retries is disabled\n");
 			}
 			return ret;
 		case RETRY_WITH_DELAY:
 			/* if we have reached the retry limit just return the failure,
 			 * if not, wait for the delay and try again */
-			if (current_retry <= MAX_TRIES) {
-				sleep(sleep_time);
-				sleep_time *= DELAY_MULTIPLIER;
-				continue;
+			if (max_retries) {
+				if (current_retry <= max_retries) {
+					if (sleep_time) {
+						fprintf(stderr, "Waiting %d seconds before retrying the download\n", sleep_time);
+					}
+					fprintf(stderr, "Retry #%d downloading from %s\n", current_retry, url);
+					sleep(sleep_time);
+					sleep_time = (sleep_time * DELAY_MULTIPLIER) > MAX_DELAY ? MAX_DELAY : (sleep_time * DELAY_MULTIPLIER);
+					continue;
+				} else {
+					fprintf(stderr, "Maximum number of retries reached\n");
+				}
+			} else {
+				fprintf(stderr, "Download retries is disabled\n");
 			}
 			return ret;
 		default:

src/curl_async.c

@@ -191,7 +191,7 @@ void *swupd_curl_parallel_download_start(size_t max_xfer)
 
 	h->max_xfer = max_xfer;
 	h->curl_hashmap = hashmap_new(SWUPD_CURL_HASH_BUCKETS, file_hash_cmp, file_hash_value);
-	h->retry_delay = RETRY_DELAY;
+	h->retry_delay = retry_delay;
 
 	return h;
 error:
@@ -265,7 +265,7 @@ static int perform_curl_io_and_complete(struct swupd_curl_parallel_handle *h, in
 			//Check if user can handle errors
 			if (!h->error_cb || h->error_cb(file->status, file->data)) {
 				// Don't retry download if error was handled
-				file->retries = MAX_TRIES;
+				file->retries = max_retries;
 				file->cb_retval = true;
 				h->failed = list_prepend_data(h->failed, file);
 			} else {
@@ -533,7 +533,7 @@ int swupd_curl_parallel_download_end(void *handle, int *num_downloads)
 		for (l = h->failed; l;) {
 			struct multi_curl_file *file = l->data;
 
-			if (file->retries < MAX_TRIES &&
+			if (file->retries < max_retries &&
 			    file->status != DOWNLOAD_STATUS_WRITE_ERROR) {
 				struct list *next;
 
@@ -559,7 +559,7 @@ int swupd_curl_parallel_download_end(void *handle, int *num_downloads)
 		}
 		if (retry) {
 			sleep(h->retry_delay);
-			h->retry_delay *= DELAY_MULTIPLIER;
+			h->retry_delay = (h->retry_delay * DELAY_MULTIPLIER) > MAX_DELAY ? MAX_DELAY : (h->retry_delay * DELAY_MULTIPLIER);
 		}
 	}
 

src/globals.c

@@ -58,6 +58,8 @@ char *state_dir = NULL;
 int skip_diskspace_check = 0;
 bool keepcache = false;
 timelist *global_times = NULL;
+int max_retries = 3;
+int retry_delay = 10;
 
 /* NOTE: Today the content and version server urls are the same in
  * all cases.  It is highly likely these will eventually differ, eg:
@@ -565,6 +567,8 @@ static const struct option global_opts[] = {
 	{ "debug", no_argument, &log_level, LOG_DEBUG },
 	//TODO: -D option is deprecated. Remove that on a Major release
 	{ "", required_argument, 0, 'D' },
+	{ "max-retries", required_argument, 0, 'r' },
+	{ "retry-delay", required_argument, 0, 'd' },
 	{ 0, 0, 0, 0 }
 };
 
@@ -636,6 +640,20 @@ static bool global_parse_opt(int opt, char *optarg)
 			return false;
 		}
 		return true;
+	case 'r':
+		err = strtoi_err(optarg, &max_retries);
+		if (err < 0 || max_retries < 0) {
+			fprintf(stderr, "Invalid --max-retries argument: %s\n\n", optarg);
+			return false;
+		}
+		return true;
+	case 'd':
+		err = strtoi_err(optarg, &retry_delay);
+		if (err < 0 || retry_delay < 0 || retry_delay > 60) {
+			fprintf(stderr, "Invalid --retry-delay argument: %s (should be between 0 - %d seconds)\n\n", optarg, MAX_DELAY);
+			return false;
+		}
+		return true;
 	default:
 		return false;
 	}
@@ -681,7 +699,7 @@ void global_print_help(void)
 {
 	fprintf(stderr, "Global Options:\n");
 	fprintf(stderr, "   -h, --help              Show help options\n");
-	fprintf(stderr, "   -p, --path=[PATH...]    Use [PATH...] as the path to verify (eg: a chroot or btrfs subvol\n");
+	fprintf(stderr, "   -p, --path=[PATH...]    Use [PATH...] as the path to verify (eg: a chroot or btrfs subvol)\n");
 	fprintf(stderr, "   -u, --url=[URL]         RFC-3986 encoded url for version string and content file downloads\n");
 	fprintf(stderr, "   -P, --port=[port #]     Port number to connect to at the url for version string and content file downloads\n");
 	fprintf(stderr, "   -c, --contenturl=[URL]  RFC-3986 encoded url for content file downloads\n");
@@ -696,6 +714,8 @@ void global_print_help(void)
 	fprintf(stderr, "   -N, --no-scripts        Do not run the post-update scripts and boot update tool\n");
 	fprintf(stderr, "   -b, --no-boot-update    Do not install boot files to the boot partition (containers)\n");
 	fprintf(stderr, "   -W, --max-parallel-downloads=[n] Set the maximum number of parallel downloads\n");
+	fprintf(stderr, "   -r, --max-retries       Maximum number of retries for download failures\n");
+	fprintf(stderr, "   -d, --retry-delay       Initial delay between download retries, this will be doubled for each retry\n");
 	fprintf(stderr, "\n");
 }
 

src/swupd.h

@@ -34,10 +34,9 @@ extern "C" {
 
 #define UNUSED_PARAM __attribute__((__unused__))
 
-/* values used for configuring downloads */
-#define MAX_TRIES 3
-#define RETRY_DELAY 10
+/* value used for configuring downloads */
 #define DELAY_MULTIPLIER 2
+#define MAX_DELAY 60
 
 #define SWUPD_HASH_DIRNAME "DIRECTORY"
 #define SWUPD_DEFAULTS "/usr/share/defaults/swupd/"
@@ -187,6 +186,8 @@ extern char *bundle_to_add;
 extern char *state_dir;
 extern int skip_diskspace_check;
 extern timelist *global_times;
+extern int max_retries;
+extern int retry_delay;
 
 extern char *version_url;
 extern char *content_url;

src/swupd_curl.h

@@ -81,7 +81,7 @@ int swupd_curl_get_file_memory(const char *url, struct curl_file_data *file_data
  * Parameters:
  *  - max_xfer: The maximum number of simultaneos downloads.
  *
- * Parallel download handler will retry MAX_TRIES times to download each file,
+ * Parallel download handler will retry max_retries times to download each file,
  * ading a timeout between each try.
  *
  * Note: This function is non-blocking.