[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