[PATCH] prepare for a user-configurable RHEV-image-import timeout

Jim Meyering jim at meyering.net
Mon Oct 3 10:15:57 UTC 2011


Jim Meyering wrote:
> Jim Meyering wrote:
>> Here's a little patch (untested, but minimal semantic change),
>> in preparation for a configurable timeout interval:
>>
>> Subject: [PATCH] prepare for a user-configurable RHEV-image-import timeout
>>
>> The current timeout of 10 minutes is too short for some users,
>> and what they need (60 minutes) seems like it would be far too
>> long for most others to wait for notification of failure.
>> * dc-rhev-image.c (IMPORT_SLEEP_SECONDS): Define to 1, not 3.
>> (IMPORT_SLEEP_MULTIPLIER): Define to 2: double the length of the
>> sleep interval after each iteration.
>> (import_tpl): Use the new IMPORT_SLEEP_MULTIPLIER.
>
> Here's a draft of what the above was preparing for.
> The only change I'm still debating about is where/how to publicize
> the default RHEVM import timeout value.  As the FIXME comment below
> suggests, I'll probably make it a global and print it via iwhd's --help.
>
> Oh, and --help (or registrations.md) will be the place to actually
> document that import-timeout is the maximum number of seconds we'll
> sleep while waiting for an import to complete.

Here's a complete change:

>From aa6a92256ec284247901046f6fd672931f291483 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Sun, 2 Oct 2011 09:51:25 +0200
Subject: [PATCH] the RHEV-M import timeout interval is now configurable

* iwh.h (RHEVM_DEFAULT_IMPORT_TIMEOUT_SECONDS): Define.
* NEWS (New features): Mention it.
* doc/registrations.md: Update an example and mention the new
timeout parameter.
* backend.c (fs_rhevm_register): Use import-timeout, if specified.
Otherwise, use the default when constructing the temporary config file
we'll hand off to ...
* dc-rhev-image.c (import_tpl): Extract new (required) "timeout"
parameter from the config file, and use it instead of the old
hard-coded default.
* rest.c (usage): Print the default import timeout value here.
---
 NEWS                 |    4 ++++
 backend.c            |   27 ++++++++++++++++++++++++---
 dc-rhev-image.c      |   39 +++++++++++++++++++++++++++++----------
 doc/registrations.md |    5 +++++
 iwh.h                |    6 ++++++
 rest.c               |    5 +++++
 6 files changed, 73 insertions(+), 13 deletions(-)

diff --git a/NEWS b/NEWS
index c7467a9..f5f6715 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,10 @@ iwhd NEWS                                                   -*- outline -*-
   object deletion now succeeds for an object whose FS-backed file has been
   removed behind iwhd's back.

+** New features
+
+  The RHEVM import timeout interval is now configurable.
+

 * Noteworthy changes in release 0.98 (2011-08-09) [stable]

diff --git a/backend.c b/backend.c
index 5a38cca..7816b5a 100644
--- a/backend.c
+++ b/backend.c
@@ -36,6 +36,7 @@

 #define GLOBALS_IMPL
 #include "iwh.h"
+#include "intprops.h"
 #include "logging.h"
 #include "meta.h"
 #include "setup.h"
@@ -44,6 +45,7 @@
 #include "backend.h"
 #include "state_defs.h"
 #include "ignore-value.h"
+#include "xstrtol.h"

 #define ARRAY_CARDINALITY(Array) (sizeof (Array) / sizeof *(Array))

@@ -1396,7 +1398,7 @@ static char *
 fs_rhevm_genconf(const char *bucket, const char *key, const char *api_url,
 	const char *api_user, const char *api_secret,
 	const char *nfs_host, const char *nfs_path, const char *nfs_dir,
-	const char *cluster)
+	time_t import_timeout_seconds, const char *cluster)
 {
 	int rc;
 	char *b, *k, *url, *user, *pass, *nhost, *npath, *ndir;
@@ -1422,9 +1424,11 @@ fs_rhevm_genconf(const char *bucket, const char *key, const char *api_url,
 		"  \"nfshost\" : \"%s\",\n"
 		"  \"nfspath\" : \"%s\",\n"
 		"  \"nfsdir\"  : \"%s\",\n"
+		"  \"timeout\" : %lu,\n"
 		"  \"cluster\" : \"%s\"\n"
 		"}\n",
-		b, k, url, user, pass, nhost, npath, ndir, cluster);
+		b, k, url, user, pass, nhost, npath, ndir,
+		import_timeout_seconds, cluster);
 	if (rc < 0)
 		ret = NULL;	/* for undefined behaviour in asprintf() */
  err:
@@ -1548,11 +1552,28 @@ fs_rhevm_register (my_state *ms, const provider_t *prov, const char *next,
 	if (!cluster)
 		cluster = "_none_";

+	char *import_timeout_str = kv_hash_lookup (args, "import-timeout");
+	time_t import_timeout_seconds = 0;
+	if (import_timeout_str) {
+		unsigned long timeout_tmp;
+		if (xstrtoul (import_timeout_str, NULL, 10,
+			      &timeout_tmp, NULL) != LONGINT_OK
+		    || timeout_tmp == 0
+		    || TYPE_MAXIMUM (time_t) < timeout_tmp) {
+			log_msg(0, _("ignoring invalid import-timeout: %s"),
+				import_timeout_str);
+			import_timeout_seconds = timeout_tmp;
+		}
+	}
+	if (import_timeout_seconds == 0)
+		import_timeout_seconds = RHEVM_DEFAULT_IMPORT_TIMEOUT_SECONDS;
+
 	ret = MHD_HTTP_INTERNAL_SERVER_ERROR;

 	conf_text = fs_rhevm_genconf(ms->bucket, ms->key,
 			api_url, api_user, api_secret,
-			nfs_host, nfs_path, nfs_dir, cluster);
+			nfs_host, nfs_path, nfs_dir,
+			import_timeout_seconds, cluster);
 	if (!conf_text) {
 		log_msg(0, _("no core"));
 		goto cleanup;
diff --git a/dc-rhev-image.c b/dc-rhev-image.c
index fb45d7d..db24a94 100644
--- a/dc-rhev-image.c
+++ b/dc-rhev-image.c
@@ -32,6 +32,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdbool.h>
+#include <stdint.h>
 #include <unistd.h>
 #include <errno.h>
 #include <time.h>
@@ -45,13 +46,14 @@
 #include <uuid/uuid.h>

 #include "base64.h"
+#include "c-strcase.h"
 #include "close-stream.h"
 #include "closeout.h"
 #include "copy-file.h"
-#include "progname.h"
+#include "intprops.h"
 #include "dirname.h"
+#include "progname.h"
 #include "xalloc.h"
-#include "c-strcase.h"

 /*
  * Note that we almost never prefix with TAG due to compatibility with EC2.
@@ -76,10 +78,6 @@ enum {
      and then multiply the current number of seconds to sleep by
      this integer factor.  */
   IMPORT_SLEEP_MULTIPLIER = 2,
-
-  /* An import times out after this many seconds.  60 is too small.
-     600 appears to be adequate in some real-life situations.  */
-  IMPORT_MAXIMUM_DURATION_SECONDS = 600
 };

 struct config {
@@ -90,6 +88,7 @@ struct config {
 	char *nfshost;
 	char *nfspath;
 	char *nfsdir;
+	time_t import_timeout;
 	char *cluster;
 };

@@ -305,6 +304,26 @@ static const char *image_name(const char *path)
 }

 static void
+cfg_veripick_time(time_t *cfgval, const char *cfgname, json_t *jcfg,
+		  const char *cfgtag)
+{
+	json_t *elem = json_object_get(jcfg, cfgtag);
+	if (!json_is_integer(elem)) {
+		fprintf(stderr,
+		    "ERROR configuration %s: `%s' value is not an integer\n",
+		    cfgname, cfgtag);
+		exit(EXIT_FAILURE);
+	}
+	intmax_t val = json_integer_value(elem);
+	if (val < 0 || TYPE_MAXIMUM(time_t) < val) {
+		fprintf(stderr,
+		    "ERROR configuration %s: invalid `%s'\n", cfgname, cfgtag);
+		exit(EXIT_FAILURE);
+	}
+	*cfgval = val;
+}
+
+static void
 cfg_veripick(char **cfgval, const char *cfgname, json_t *jcfg,
 	     const char *cfgtag)
 {
@@ -2271,8 +2290,7 @@ static void import_tpl(struct config *cfg, struct api_conn *connection,
 		}
 		if (c_strcasecmp(status, "PENDING") == 0 ||
 		    c_strcasecmp(status, "IN_PROGRESS") == 0) {
-			if (total_slept_seconds
-			    >= IMPORT_MAXIMUM_DURATION_SECONDS) {
+			if (total_slept_seconds >= cfg->import_timeout) {
 				fprintf(stderr,
 				    "ERROR import times out, status `%s'\n",
 				    status);
@@ -2289,8 +2307,7 @@ static void import_tpl(struct config *cfg, struct api_conn *connection,
 		nanosleep(&tm, NULL);
 		total_slept_seconds += n_sec;
 		n_sec = MIN (n_sec * IMPORT_SLEEP_MULTIPLIER,
-			     IMPORT_MAXIMUM_DURATION_SECONDS
-			     - total_slept_seconds);
+			     cfg->import_timeout - total_slept_seconds);

 		free(status);
 	}
@@ -2454,6 +2471,8 @@ int main(int argc, char **argv, char **envp)
 		exit(EXIT_FAILURE);
 	}

+	cfg_veripick_time(&cfg.import_timeout, cfgname, jcfg, "timeout");
+
 	cfg_veripick(&cfg.cluster, cfgname, jcfg, "cluster");

 	json_decref(jcfg);
diff --git a/doc/registrations.md b/doc/registrations.md
index 13c77a4..c2f007c 100644
--- a/doc/registrations.md
+++ b/doc/registrations.md
@@ -118,6 +118,7 @@ Registration call:
   -d nfs-host=fish.usersys.redhat.com \
   -d nfs-path=/home/vdsm/v1 \
   -d nfs-dir=/mnt/iwhd-fish \
+  -d import-timeout=3600 \
   -d cluster=_any_ \
   http://localhost:9090/buk1/dummy_img

@@ -127,6 +128,10 @@ domain, but does not import the template. Do not use "Default" here.
 It is a dummy cluster that RHEV-M comes with, just like the template
 called "Blank".

+Note how we have specified an import-timeout value of 3600 seconds (1 hour).
+The import-timeout parameter is optional.  If you don't specify that,
+you'll use the default, which can be found in the output of iwhd --help.
+
 The name of the resulting template is going to be the object's name
 ("dummy_img" in the above example). Note that if you let iwhd import
 (by specifying "-d cluster=xxxxx"), template names have to be unique
diff --git a/iwh.h b/iwh.h
index ed2e01d..29b8ee0 100644
--- a/iwh.h
+++ b/iwh.h
@@ -43,6 +43,12 @@ GLOBAL(char *,          tmpfile_template, NULL);
 # define ATTRIBUTE_NORETURN __attribute__ ((__noreturn__))
 #endif

+/* When importing a RHEV-M image, we check periodically to see if the
+   operation has completed.  Each time we learn that it is not yet complete
+   we sleep for some small (yet increasing) interval.  This is the maximum
+   total amount of time we will sleep before declaring a timeout.  */
+enum { RHEVM_DEFAULT_IMPORT_TIMEOUT_SECONDS = 30 * 60 }; /* 30 minutes */
+
 /*
  * Common parts of autostart
  *
diff --git a/rest.c b/rest.c
index 92f36cb..c8d2f8f 100644
--- a/rest.c
+++ b/rest.c
@@ -2230,6 +2230,11 @@ A configuration file must be specified.\n\
 "), stdout);
       printf (_("\
 \n\
+The default RHEV-M import timeout is %d seconds.\n\
+"),
+	      RHEVM_DEFAULT_IMPORT_TIMEOUT_SECONDS);
+      printf (_("\
+\n\
 Report %s bugs to %s.\n\
 "),
 	      program_name, PACKAGE_BUGREPORT);
--
1.7.7.rc0.362.g5a14


More information about the iwhd-devel mailing list