[PATCH] dynamic providers

Jim Meyering jim at meyering.net
Fri Dec 17 19:51:08 UTC 2010


I've included below a 7-cset series that allows providers to
be added and removed dynamically.  This shows you what the interface
will look like.

To illustrate, here's part of the t/basic test:

  # Add a provider:
  p1_url=http://localhost:$port/_providers/PROVIDER-1
  curl -d type=s3 -dhost=localhost -dport=80 -dkey=u -dsecret=p \
    $p1_url || fail=1

  # Ensure it was added
  curl http://localhost:$port/_providers > p || fail=1
  grep PROVIDER-1 p || fail=1

  # Add another provider:
  p2_url=http://localhost:$port/_providers/PROVIDER-2
  curl -dtype=http -dhost=localhost -dport=9091 $p2_url || fail=1
  # Ensure it was added.
  curl http://localhost:$port/_providers > p || fail=1
  grep PROVIDER-2 p || fail=1

  # Add provider using a "name" parameter (not permitted):
  p3_url=http://localhost:$port/_providers/PROVIDER-3
  curl -dtype=http -dname=X -dhost=localhost -dport=9091 $p3_url || fail=1
  # Ensure it was not added.
  curl http://localhost:$port/_providers > p || fail=1
  grep PROVIDER-3 p && { warn_ add-provider-w/name-param not rejected; fail=1; }

  # Delete a provider.
  curl -f -X DELETE $p2_url 2> p || fail=1
  # Ensure it was deleted.
  curl http://localhost:$port/_providers > p || fail=1
  grep PROVIDER-2 p && { warn_ $ME_: provider deletion failed; fail=1; }

  # Delete a non-existent provider.
  curl -f -X DELETE http://localhost:$port/_providers/no-such 2> p
  # ensure it fails; expect exit-22 and http: 404
  test $? = 22 || fail=1
  grep ' 404$' p || fail=1

  # Get the name of the current primary provider.
  curl http://localhost:$port/_providers/_primary > p || fail=1
  test "$(cat p)" = primary || fail=1

  # Trying to GET with anything other than "_primary" returns the empty string.
  curl http://localhost:$port/_providers/anything > p 2>/dev/null || fail=1
  test -s p && fail=1

  # Try to add a provider with the reserved name.  It must fail.
  p_reserved_url=http://localhost:$port/_providers/_primary
  curl -dtype=http -dhost=localhost -dport=9091 $p_reserved_url || fail=1
  # Ensure it was not added.
  curl http://localhost:$port/_primary > p || fail=1
  grep _primary p && { warn_ add-provider/reserved-name not rejected; fail=1; }

  # Move the "primary" attribute to a different provider.
  curl -X PUT $p1_url/_set_primary > p || fail=1
  test -s p && fail=1
  new_primary=$(curl http://localhost:$port/_providers/_primary) || fail=1
  test $new_primary = PROVIDER-1 || fail=1

  # Restore the primary attribute to the original.
  # FIXME: if I don't restore, the following headless test makes iwhd segfault.
  # Investigate that.
  p1_url=http://localhost:$port/_providers/primary
  curl -X PUT $p1_url/_set_primary > p || fail=1
========================================================

There are caveats: to be done properly, this will
require reference-counting each provider and actually freeing one
only when we're sure no other thread is using it.
Likewise, the provider hash table will need protection
against corruption via concurrent modification.

At first, I hoped to remove json support altogether,
but if we do that, the only way to specify the providers
required for initial operation would be via the command line
use of curl.  Obviously, we know we don't want to expose provider
credentials that way.  Of course, if there's a safe json-free way to
send provider-defining information to iwhd, then I'd love to
hear it and will gladly remove json support altogether.

This series must be applied on top of the gnulib patches.


>From 5fed65a802c3364210a0b9df84a3a33b9fe451ca Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Mon, 15 Nov 2010 17:50:53 +0100
Subject: [PATCH 1/7] allow dynamic addition/deletion of providers

* setup.h (struct _provider) [deleted]: New member.
* setup.c (validate_provider): New function.
(json_validate_server): Renamed from validate_server.
(convert_provider): Initialize new "deleted" member.
(add_provider, find_provider): New functions.
Declare new functions.
* rest.c (prov_list_generator): Don't list a provider that
is marked as deleted.
(url_to_provider_name): New function.
(proxy_delete_prov, proxy_add_prov): New functions.
(my_rules): Add corresponding entries in this table.
* t/basic: Add minimal tests of new functionality.
---
 rest.c  |  138 ++++++++++++++++++++++++++++++++++++++++++++++++-
 setup.c |  177 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 setup.h |   10 +++-
 t/basic |   39 ++++++++++++--
 4 files changed, 352 insertions(+), 12 deletions(-)

diff --git a/rest.c b/rest.c
index ea8b289..f156862 100644
--- a/rest.c
+++ b/rest.c
@@ -34,6 +34,7 @@
 #include <curl/curl.h>
 #include <glib.h>

+#include "dirname.h"
 #include "iwh.h"
 #include "closeout.h"
 #include "progname.h"
@@ -57,7 +58,7 @@

 typedef enum {
 	URL_ROOT=0, URL_BUCKET, URL_OBJECT, URL_ATTR, URL_INVAL,
-	URL_QUERY, URL_PROVLIST
+	URL_QUERY, URL_PROVLIST, URL_PROVIDER
 } url_type;

 typedef struct {
@@ -931,6 +932,7 @@ proxy_delete (void *cctx, struct MHD_Connection *conn, const char *url,
 		free_ms(ms);
 		return MHD_NO;
 	}
+	error (0, 0, "DELETE BUCKET: rc=%d", rc);
 	MHD_queue_response(conn,rc,resp);
 	MHD_destroy_response(resp);

@@ -1458,7 +1460,7 @@ proxy_object_post (void *cctx, struct MHD_Connection *conn, const char *url,
 	(void)method;
 	(void)version;

-	DPRINTF("PROXY POST (%s, %zu)\n",url,*data_size);
+	DPRINTF("PROXY POST obj (%s, %zu)\n",url,*data_size);

 	if (ms->state == MS_NEW) {
 		ms->state = MS_NORMAL;
@@ -1563,6 +1565,8 @@ prov_list_generator (void *ctx, uint64_t pos, char *buf, size_t max)
 	}

 	if (g_hash_table_iter_next(&ms->prov_iter,&key,(gpointer *)&prov)) {
+		if (prov->deleted)
+			return 0;
 		len = tmpl_prov_entry(ms->gen_ctx,prov->name,prov->type,
 			prov->host, prov->port, prov->username, prov->password);
 		if (!len) {
@@ -1691,6 +1695,126 @@ proxy_update_prov (void *cctx, struct MHD_Connection *conn, const char *url,
 	return MHD_YES;
 }

+static char *
+url_to_provider_name (const char *url)
+{
+  char *prov_name = base_name (url);
+  if (prov_name)
+    strip_trailing_slashes (prov_name);
+  /* Ensure we handle trailing slashes (i.e., remove them).  */
+  assert (prov_name == NULL
+	  || (*prov_name && prov_name[strlen(prov_name)-1] != '/'));
+  return prov_name;
+}
+
+static int
+proxy_delete_prov (void *cctx, struct MHD_Connection *conn, const char *url,
+		   const char *method, const char *version, const char *data,
+		   size_t *data_size, void **rctx)
+{
+	DPRINTF("PROXY DELETE PROVIDER %s\n",url);
+	(void)cctx;
+	(void)method;
+	(void)version;
+	(void)data;
+	(void)data_size;
+
+	my_state *ms = *rctx;
+	struct MHD_Response *resp
+	  = MHD_create_response_from_data(0,NULL,MHD_NO,MHD_NO);
+	if (!resp) {
+		free_ms(ms);
+		return MHD_NO;
+	}
+
+	char *prov_name = url_to_provider_name (url);
+	DPRINTF("PROXY DELETE PROVIDER prov=%s\n", prov_name);
+	provider_t *prov = find_provider (prov_name);
+
+	// don't allow removal of current main_prov.
+	if (prov == main_prov)
+		prov = NULL;
+
+	int rc = prov ? MHD_HTTP_OK : MHD_HTTP_NOT_FOUND;
+	if (prov)
+		prov->deleted = 1;
+
+	error (0, 0, "DELETE PROV: rc=%d", rc);
+
+	free (prov_name);
+	MHD_queue_response(conn,rc,resp);
+	MHD_destroy_response(resp);
+
+	free_ms(ms);
+	return MHD_YES;
+}
+
+static int
+proxy_add_prov (void *cctx, struct MHD_Connection *conn, const char *url,
+		   const char *method, const char *version, const char *data,
+		   size_t *data_size, void **rctx)
+{
+	DPRINTF("PROXY ADD PROVIDER %s\n",url);
+	struct MHD_Response	*resp;
+	my_state		*ms	= *rctx;
+
+	(void)cctx;
+	(void)method;
+	(void)version;
+
+	if (ms->state == MS_NEW) {
+		ms->state = MS_NORMAL;
+		ms->url = (char *)url;
+		ms->dict = g_hash_table_new_full(
+			g_str_hash,g_str_equal,free,free);
+		ms->cleanup |= CLEANUP_DICT;
+		ms->post = MHD_create_post_processor(conn,4096,
+			prov_iterator,ms->dict);
+		ms->cleanup |= CLEANUP_POST;
+	}
+	else if (*data_size) {
+		MHD_post_process(ms->post,data,*data_size);
+		*data_size = 0;
+	}
+	else {
+		int rc = MHD_HTTP_BAD_REQUEST;
+		char *prov_name = url_to_provider_name (url);
+		/* We're about to insert "name -> $prov_name".
+		   Ensure there is no "name" key already there.  */
+		const char *name = g_hash_table_lookup (ms->dict, "name");
+		if (name) {
+			fprintf(stderr,
+				"add_provider: do not specify name: name=%s\n",
+				name);
+			return MHD_NO;
+			// FIXME: be careful that this does not leak "ms"
+		}
+		// FIXME: unchecked strdup
+		g_hash_table_insert(ms->dict,strdup("name"),prov_name);
+
+		if (validate_provider (ms->dict)) {
+			if (!add_provider (ms->dict)) {
+			      DPRINTF("add provider failed\n");
+			} else {
+			      rc = MHD_HTTP_OK;
+			}
+		}
+		else {
+			DPRINTF("invalid provider\n");
+		}
+		resp = MHD_create_response_from_data(0,NULL,MHD_NO,MHD_NO);
+		if (!resp) {
+			fprintf(stderr,"MHD_crfd failed\n");
+			return MHD_NO;
+		}
+		MHD_queue_response(conn,rc,resp);
+		MHD_destroy_response(resp);
+		free_ms(ms);
+	}
+
+	return MHD_YES;
+}
+
 static int
 proxy_create_bucket (void *cctx, struct MHD_Connection *conn, const char *url,
 		     const char *method, const char *version, const char *data,
@@ -1753,6 +1877,10 @@ static const rule my_rules[] = {
 	  "GET",	URL_PROVLIST,	proxy_list_provs	},
 	{ /* update a provider */
 	  "POST",	URL_PROVLIST,	proxy_update_prov	},
+	{ /* create a provider */
+	  "POST",	URL_PROVIDER,	proxy_add_prov		},
+	{ /* delete a provider */
+	  "DELETE",	URL_PROVIDER,	proxy_delete_prov	},
 	{ NULL, 0, NULL }
 };

@@ -1806,6 +1934,12 @@ parse_url (const char *url, my_state *ms)
 		}
 	}

+	if (eindex == URL_OBJECT
+	    && !strcmp (parts[URL_BUCKET], "_providers"))
+	  eindex = URL_PROVIDER;
+
+	DPRINTF("parse_url: %d: %s %s %s", eindex, parts[URL_BUCKET],
+		parts[URL_OBJECT], parts[URL_ATTR]);
 	return eindex;
 }

diff --git a/setup.c b/setup.c
index 2a03f40..86061d1 100644
--- a/setup.c
+++ b/setup.c
@@ -27,6 +27,7 @@
 #include <string.h>
 #include <strings.h>
 #include <unistd.h>
+#include <assert.h>

 #include <jansson.h>

@@ -71,8 +72,74 @@ static GHashTable	*prov_hash	= NULL;
 provider_t	*main_prov	= NULL;
 provider_t	*master_prov	= NULL;

+int
+validate_provider (GHashTable *h)
+{
+    const char *name = g_hash_table_lookup (h, "name");
+    assert (name);
+    const char *type = g_hash_table_lookup (h, "type");
+    if (type == NULL) {
+	error (0, 0, "provider %s has no type", name);
+	return 0;
+    }
+
+    unsigned int needs;
+    if (!strcasecmp(type,"s3") || !strcasecmp(type,"cf")) {
+	needs = NEED_SERVER | NEED_CREDS;
+    } else if (!strcasecmp(type,"http")) {
+	needs = NEED_SERVER;
+    } else if (!strcasecmp(type,"fs")) {
+	needs = NEED_PATH;
+    } else {
+	error (0, 0, "provider %s has invalid type: %s", name, type);
+	return 0;
+    }
+
+    int ok = 1;
+    if (needs & NEED_SERVER) {
+	const char *host = g_hash_table_lookup (h, "host");
+	if (!host) {
+	    error (0, 0, "%s: %s-provider requires a host", name, type);
+	    ok = 0;
+	}
+	const char *port = g_hash_table_lookup (h, "port");
+	if (!port) {
+	    error (0, 0, "%s: %s-provider requires a port", name, type);
+	    ok = 0;
+	}
+	// ensure port is a positive integer with 5 or fewer digits
+	if (5 < strlen (port) || strcspn (port, "0123456789")) {
+	    error (0, 0, "%s: %s-provider: invalid port: %s", name, type, port);
+	    ok = 0;
+	}
+    }
+
+    if (needs & NEED_CREDS) {
+	const char *key = g_hash_table_lookup (h, "key");
+	if (!key) {
+	    error (0, 0, "%s: %s-provider requires a key", name, type);
+	    ok = 0;
+	}
+	const char *secret = g_hash_table_lookup (h, "secret");
+	if (!secret) {
+	    error (0, 0, "%s: %s-provider requires a secret", name, type);
+	    ok = 0;
+	}
+    }
+
+    if (needs & NEED_PATH) {
+	const char *path = g_hash_table_lookup (h, "path");
+	if (!path) {
+	    error (0, 0, "%s: %s-provider requires a path", name, type);
+	    ok = 0;
+	}
+    }
+
+    return ok;
+}
+
 static int
-validate_server (unsigned int i)
+json_validate_server (unsigned int i)
 {
 	json_t		*server;
 	json_t		*elem;
@@ -209,6 +276,7 @@ convert_provider (int i, provider_t *out)
 	out->username = dup_json_string(server,"key");
 	out->password = dup_json_string(server,"secret");
 	out->path = dup_json_string(server,"path");
+	/* FIXME: detect failed "dup_*" calls.  */

 	/* TBD: do this a cleaner way. */
 	if (!strcasecmp(out->type,"s3")) {
@@ -231,6 +299,7 @@ convert_provider (int i, provider_t *out)
 	iter = json_object_iter(server);
 	while (iter) {
 		key = json_object_iter_key(iter);
+		error(0,0,"convert-provider: ITER key: %s",key);
 		if (!is_reserved_attr(key)) {
 			value = json_string_value(json_object_iter_value(iter));
 			if (value) {
@@ -249,10 +318,100 @@ convert_provider (int i, provider_t *out)
 	}

 	out->token = NULL;
+	out->deleted = 0;

 	return 1;
 }

+int
+add_provider (GHashTable *h)
+{
+    char *name = g_hash_table_lookup (h, "name");
+    assert (name);
+
+    provider_t *prov = calloc (1, sizeof *prov);
+    if (prov == NULL)
+        return 0;
+
+    prov->name = strdup (name);
+    if (prov->name == NULL)
+        goto fail;
+
+    prov->type = g_hash_table_lookup(h,"type");
+    if (prov->type == NULL)
+        goto fail;
+    prov->type = strdup (prov->type);
+    if (prov->type == NULL)
+        goto fail;
+
+    prov->host = g_hash_table_lookup(h,"host");
+    prov->port = atoi(g_hash_table_lookup(h,"port"));
+    /* TBD: change key/secret field names to username/password */
+    prov->username = g_hash_table_lookup(h,"key");
+    prov->password = g_hash_table_lookup(h,"secret");
+    prov->path = g_hash_table_lookup(h,"path");
+
+    if (prov->host)
+        prov->host = strdup (prov->host);
+    if (prov->username)
+        prov->username = strdup (prov->username);
+    if (prov->password)
+        prov->password = strdup (prov->password);
+    if (prov->path)
+        prov->path = strdup (prov->path);
+
+    /* TBD: do this a cleaner way. */
+    if (!strcasecmp(prov->type,"s3"))
+        prov->func_tbl = &s3_func_tbl;
+    else if (!strcasecmp(prov->type,"http"))
+        prov->func_tbl = &curl_func_tbl;
+    else if (!strcasecmp(prov->type,"cf"))
+        prov->func_tbl = &cf_func_tbl;
+    else if (!strcasecmp(prov->type,"fs"))
+        prov->func_tbl = &fs_func_tbl;
+    else
+        prov->func_tbl = &bad_func_tbl;
+
+    prov->attrs = g_hash_table_new_full(g_str_hash,g_str_equal,free,free);
+    // FIXME: can't the above fail?
+
+    GHashTableIter iter;
+    g_hash_table_iter_init (&iter, h);
+    while (1) {
+        gpointer key;
+        gpointer val;
+        if (!g_hash_table_iter_next (&iter, &key, &val))
+            break;
+
+        if (!is_reserved_attr(key)) {
+            if (val) {
+                error(0,0,"no value for %s", (char *)key);
+                continue;
+            }
+            DPRINTF("%p.%s = %s\n",prov, (char *)key, (char *)val);
+            g_hash_table_insert(prov->attrs, strdup(key), strdup(val));
+            // FIXME: check strdup for failure
+        }
+    }
+
+    /* Note that we must strdup "name", since here it's a key, but above it's a
+       value.  Not using // strdup here would lead to a use-after-free bug.  */
+    // FIXME: check strdup for failure
+    g_hash_table_insert(prov_hash,strdup(name),prov);
+
+    return 1;
+
+   fail:
+    free ((char *) prov->name);
+    free ((char *) prov->type);
+    free ((char *) prov->host);
+    free ((char *) prov->username);
+    free ((char *) prov->password);
+    free ((char *) prov->path);
+    free (prov);
+    return 0;
+}
+
 static const char *
 parse_config_inner (void)
 {
@@ -274,7 +433,7 @@ parse_config_inner (void)
 	}

 	for (i = 0; i < nservers; ++i) {
-		if (!validate_server(i)) {
+		if (!json_validate_server(i)) {
 			goto err;
 		}
 	}
@@ -402,7 +561,7 @@ auto_config(void)
 	return primary;
 }

-const provider_t *
+provider_t *
 get_provider (const char *name)
 {
 	if (!prov_hash || !name || (*name == '\0')) {
@@ -411,6 +570,18 @@ get_provider (const char *name)
 	return g_hash_table_lookup(prov_hash,name);
 }

+provider_t *
+find_provider (const char *name)
+{
+	if (!prov_hash || !name || (*name == '\0')) {
+		return NULL;
+	}
+
+	provider_t *p = g_hash_table_lookup(prov_hash, name);
+
+	return p;
+}
+
 const char *
 get_provider_value (const provider_t *prov, const char *fname)
 {
diff --git a/setup.h b/setup.h
index e70cffa..11d2207 100644
--- a/setup.h
+++ b/setup.h
@@ -26,6 +26,7 @@ typedef struct _provider {
 	const char		*type;
 	const char		*host;
 	int			 port;
+	int			deleted;
 	const char		*username;
 	const char		*password;
 	const char		*path;
@@ -38,14 +39,17 @@ provider_t	*main_prov;
 provider_t	*master_prov;

 const char	 *parse_config		(char *);
-const provider_t *get_provider		(const char *name);
-void	 	  update_provider	(const char *provname,
+provider_t	 *get_provider		(const char *name);
+void		  update_provider	(const char *provname,
 					 const char *username,
 					 const char *password);
 const char	 *get_provider_value	(const provider_t *prov,
 					 const char *fname);
 void		  init_prov_iter	(GHashTableIter *iter);

-const char 	 *auto_config		(void);
+const char	 *auto_config		(void);
+int validate_provider (GHashTable *h);
+provider_t *find_provider (const char *name);
+int add_provider (GHashTable *h);

 #endif
diff --git a/t/basic b/t/basic
index 6a30fcc..fc42ac4 100644
--- a/t/basic
+++ b/t/basic
@@ -311,14 +311,45 @@ echo bye | curl -f -T - $bucket/trunc_test || fail=1
 cat FS/b1/trunc_test
 test "$(cat FS/b1/trunc_test)" = bye || fail=1

+# TBD: add attribute-delete tests when that functionality is implemented
+
+# TBD: add white-box tests for attributes in mongo
+
+# Add a provider:
+p1_url=http://localhost:$port/_providers/PROVIDER-1
+curl -d type=s3 -dhost=localhost -dport=80 -dkey=u -dsecret=p \
+  $p1_url || fail=1
+
+# Ensure it was added
+curl http://localhost:$port/_providers > p || fail=1
+grep PROVIDER-1 p || fail=1
+
+# Add another provider:
+p2_url=http://localhost:$port/_providers/PROVIDER-2
+curl -dtype=http -dhost=localhost -dport=9091 $p2_url || fail=1
+# Ensure it was added.
+curl http://localhost:$port/_providers > p || fail=1
+grep PROVIDER-2 p || fail=1
+
+# Delete a provider.
+curl -f -X DELETE $p2_url 2> p || fail=1
+# Ensure it was deleted.
+curl http://localhost:$port/_providers > p || fail=1
+grep PROVIDER-2 p && { warn_ $ME_: provider deletion failed; fail=1; }
+
+# Delete a non-existent provider.
+curl -f -X DELETE http://localhost:$port/_providers/no-such
+test $? = 22 || fail=1
+# FIXME ensure it fails
+cat p
+
+curl http://localhost:$port/_providers > p || fail=1
+cat p
+
 # Test "headless" operation (no access to metadata DB).
 kill -9 $mongo_pid
 cleanup_() { kill $iwhd_pid; }
 curl $bucket/attr_put > f3copy
 test "$(cat f3copy)" = "nothing" || fail=1

-# TBD: add attribute-delete tests when that functionality is implemented
-
-# TBD: add white-box tests for attributes in mongo
-
 Exit $fail
--
1.7.3.4


>From 58a4226b234adba05d382e52a935ded572ebb5b3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Fri, 10 Dec 2010 21:11:27 +0100
Subject: [PATCH 2/7] add provider ref-counting; FIXME: partial impl. (i.e., no incr)

delete_provider: New function.
---
 rest.c  |   14 ++++++++++----
 setup.c |   14 ++++++++++++++
 setup.h |    4 +++-
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/rest.c b/rest.c
index f156862..4797438 100644
--- a/rest.c
+++ b/rest.c
@@ -1728,7 +1728,6 @@ proxy_delete_prov (void *cctx, struct MHD_Connection *conn, const char *url,
 	}

 	char *prov_name = url_to_provider_name (url);
-	DPRINTF("PROXY DELETE PROVIDER prov=%s\n", prov_name);
 	provider_t *prov = find_provider (prov_name);

 	// don't allow removal of current main_prov.
@@ -1736,10 +1735,17 @@ proxy_delete_prov (void *cctx, struct MHD_Connection *conn, const char *url,
 		prov = NULL;

 	int rc = prov ? MHD_HTTP_OK : MHD_HTTP_NOT_FOUND;
-	if (prov)
-		prov->deleted = 1;

-	error (0, 0, "DELETE PROV: rc=%d", rc);
+	DPRINTF("PROXY DELETE PROVIDER prov=%s rc=%d\n", prov_name, rc);
+
+	if (prov) {
+		/* Delete for real if no one is using it.
+		   Otherwise, just mark it as deleted.  */
+		if (g_atomic_int_get (&prov->refcnt) == 0)
+			delete_provider (prov);
+		else
+			prov->deleted = 1;
+	}

 	free (prov_name);
 	MHD_queue_response(conn,rc,resp);
diff --git a/setup.c b/setup.c
index 86061d1..dc494af 100644
--- a/setup.c
+++ b/setup.c
@@ -319,6 +319,7 @@ convert_provider (int i, provider_t *out)

 	out->token = NULL;
 	out->deleted = 0;
+	out->refcnt = 0;

 	return 1;
 }
@@ -582,6 +583,19 @@ find_provider (const char *name)
 	return p;
 }

+void
+delete_provider (provider_t *prov)
+{
+  // Remove from prov_hash, then free
+
+  g_hash_table_destroy(prov->attrs);
+  // FIXME: free other members, too?
+
+  // FIXME: unchecked strdup
+  char *name = strdup (prov->name);
+  g_hash_table_remove(prov_hash, prov->name);
+}
+
 const char *
 get_provider_value (const provider_t *prov, const char *fname)
 {
diff --git a/setup.h b/setup.h
index 11d2207..dc89a74 100644
--- a/setup.h
+++ b/setup.h
@@ -26,7 +26,8 @@ typedef struct _provider {
 	const char		*type;
 	const char		*host;
 	int			 port;
-	int			deleted;
+	int			 deleted;
+	gint			 refcnt;
 	const char		*username;
 	const char		*password;
 	const char		*path;
@@ -51,5 +52,6 @@ const char	 *auto_config		(void);
 int validate_provider (GHashTable *h);
 provider_t *find_provider (const char *name);
 int add_provider (GHashTable *h);
+void delete_provider (provider_t *prov);

 #endif
--
1.7.3.4


>From f59d136ee30fa0088cc64ad45a39dc94a8a98536 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Sat, 11 Dec 2010 15:58:18 +0100
Subject: [PATCH 3/7] reject an attempt to add a provider with "name" parameter

The "name" is specified as part of the URL, not via a parameter.
* rest.c (proxy_add_prov): Handle undesired "name" parameter properly.
* t/basic: Exercise the above.
---
 rest.c  |    5 +++--
 t/basic |    7 +++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/rest.c b/rest.c
index 4797438..f7ece46 100644
--- a/rest.c
+++ b/rest.c
@@ -1792,8 +1792,7 @@ proxy_add_prov (void *cctx, struct MHD_Connection *conn, const char *url,
 			fprintf(stderr,
 				"add_provider: do not specify name: name=%s\n",
 				name);
-			return MHD_NO;
-			// FIXME: be careful that this does not leak "ms"
+			goto add_fail;
 		}
 		// FIXME: unchecked strdup
 		g_hash_table_insert(ms->dict,strdup("name"),prov_name);
@@ -1808,6 +1807,8 @@ proxy_add_prov (void *cctx, struct MHD_Connection *conn, const char *url,
 		else {
 			DPRINTF("invalid provider\n");
 		}
+
+	add_fail:
 		resp = MHD_create_response_from_data(0,NULL,MHD_NO,MHD_NO);
 		if (!resp) {
 			fprintf(stderr,"MHD_crfd failed\n");
diff --git a/t/basic b/t/basic
index fc42ac4..0d394c7 100644
--- a/t/basic
+++ b/t/basic
@@ -331,6 +331,13 @@ curl -dtype=http -dhost=localhost -dport=9091 $p2_url || fail=1
 curl http://localhost:$port/_providers > p || fail=1
 grep PROVIDER-2 p || fail=1

+# Add provider using a "name" parameter (not permitted):
+p3_url=http://localhost:$port/_providers/PROVIDER-3
+curl -dtype=http -dname=X -dhost=localhost -dport=9091 $p3_url || fail=1
+# Ensure it was not added.
+curl http://localhost:$port/_providers > p || fail=1
+grep PROVIDER-3 p && { warn_ add-provider-w/name-param not rejected; fail=1; }
+
 # Delete a provider.
 curl -f -X DELETE $p2_url 2> p || fail=1
 # Ensure it was deleted.
--
1.7.3.4


>From 41c74ff4b9a3883b25aca8cced49bb9b40f4932c Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 15 Dec 2010 12:45:28 +0100
Subject: [PATCH 4/7] use new function, get_main_provider, rather than global "main_prov"

* setup.c (get_main_provider): New function.
(main_prov): Declare static.
* setup.h (main_prov): Remove global decl.
(get_main_provider): Declare.
* rest.c, replica.c: Update all uses of "main_prov".
---
 replica.c |    2 +-
 rest.c    |    9 +++++++--
 setup.c   |    8 +++++++-
 setup.h   |    2 +-
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/replica.c b/replica.c
index f0b4b51..301c8d3 100644
--- a/replica.c
+++ b/replica.c
@@ -73,7 +73,7 @@ proxy_repl_prod (void *ctx)
 	void			*result;

 	thunk.parent = item->ms;
-	thunk.prov = main_prov;
+	thunk.prov = get_main_provider();

 	result = thunk.prov->func_tbl->get_child_func(&thunk);
 	return result;
diff --git a/rest.c b/rest.c
index f7ece46..a56f2d3 100644
--- a/rest.c
+++ b/rest.c
@@ -338,6 +338,7 @@ proxy_get_data (void *cctx, struct MHD_Connection *conn, const char *url,
 	if (!pp) {
 		return MHD_NO;
 	}
+	provider_t *main_prov = get_main_provider();
 	ms->thunk.parent = ms;
 	ms->thunk.prov = ms->from_master ? master_prov : main_prov;
 	pthread_create(&ms->backend_th,NULL,
@@ -468,6 +469,7 @@ proxy_put_data (void *cctx, struct MHD_Connection *conn, const char *url,
 		if (!pp) {
 			return MHD_NO;
 		}
+		provider_t *main_prov = get_main_provider();
 		pp->prov = main_prov;
 		ms->be_flags = BACKEND_GET_SIZE;
 		pthread_create(&ms->backend_th,NULL,
@@ -913,6 +915,7 @@ proxy_delete (void *cctx, struct MHD_Connection *conn, const char *url,

 	DPRINTF("PROXY DELETE %s\n",url);

+	provider_t *main_prov = get_main_provider();
 	ms->thunk.parent = ms;
 	ms->thunk.prov = main_prov;
 	rc = ms->thunk.prov->func_tbl->delete_func(main_prov,
@@ -1145,6 +1148,7 @@ create_bucket (char *name, my_state *ms)
 		return MHD_HTTP_BAD_REQUEST;
 	}

+	provider_t *main_prov = get_main_provider();
 	rc = main_prov->func_tbl->bcreate_func(main_prov,name);
 	if (rc == MHD_HTTP_OK) {
 		if (meta_set_value(name,"_default", "_policy","0") != 0) {
@@ -1730,8 +1734,8 @@ proxy_delete_prov (void *cctx, struct MHD_Connection *conn, const char *url,
 	char *prov_name = url_to_provider_name (url);
 	provider_t *prov = find_provider (prov_name);

-	// don't allow removal of current main_prov.
-	if (prov == main_prov)
+	// don't allow removal of current main provider.
+	if (prov == get_main_provider())
 		prov = NULL;

 	int rc = prov ? MHD_HTTP_OK : MHD_HTTP_NOT_FOUND;
@@ -2156,6 +2160,7 @@ args_done:
 	sem_init(&the_sem,0,0);

 	if (verbose) {
+		provider_t *main_prov = get_main_provider();
 		printf("primary store type is %s\n",main_prov->type);
 		if (master_host) {
 			printf("operating as slave to %s:%u\n",
diff --git a/setup.c b/setup.c
index dc494af..f613f24 100644
--- a/setup.c
+++ b/setup.c
@@ -69,9 +69,15 @@ extern backend_func_tbl	fs_func_tbl;
 static json_t		*config		= NULL;
 static GHashTable	*prov_hash	= NULL;

-provider_t	*main_prov	= NULL;
+static provider_t	*main_prov	= NULL;
 provider_t	*master_prov	= NULL;

+provider_t *
+get_main_provider (void)
+{
+  return main_prov;
+}
+
 int
 validate_provider (GHashTable *h)
 {
diff --git a/setup.h b/setup.h
index dc89a74..ecdef5f 100644
--- a/setup.h
+++ b/setup.h
@@ -36,7 +36,6 @@ typedef struct _provider {
 	char			*token;
 } provider_t;

-provider_t	*main_prov;
 provider_t	*master_prov;

 const char	 *parse_config		(char *);
@@ -53,5 +52,6 @@ int validate_provider (GHashTable *h);
 provider_t *find_provider (const char *name);
 int add_provider (GHashTable *h);
 void delete_provider (provider_t *prov);
+provider_t *get_main_provider (void);

 #endif
--
1.7.3.4


>From 957d83176e4051eaa13cf18c245ab6fc715459da Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Thu, 16 Dec 2010 18:27:10 +0100
Subject: [PATCH 5/7] tests: clean up provider-deletion test

---
 t/basic |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/t/basic b/t/basic
index 0d394c7..19eda9b 100644
--- a/t/basic
+++ b/t/basic
@@ -345,13 +345,11 @@ curl http://localhost:$port/_providers > p || fail=1
 grep PROVIDER-2 p && { warn_ $ME_: provider deletion failed; fail=1; }

 # Delete a non-existent provider.
-curl -f -X DELETE http://localhost:$port/_providers/no-such
+curl -f -X DELETE http://localhost:$port/_providers/no-such 2> p
+# ensure it fails; expect exit-22 and http: 404
 test $? = 22 || fail=1
-# FIXME ensure it fails
-cat p
+grep ' 404$' p || fail=1

-curl http://localhost:$port/_providers > p || fail=1
-cat p

 # Test "headless" operation (no access to metadata DB).
 kill -9 $mongo_pid
--
1.7.3.4


>From e355e63972d3655fbe8ae9815079e93e398b388a Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Fri, 17 Dec 2010 15:37:14 +0100
Subject: [PATCH 6/7] get primary provider name via http://host:$port/_providers/_primary

Get a URL of the form http://.../_providers/_primary
to obtain the name of the primary provider.
* rest.c (proxy_primary_prov): Implement it.
(proxy_add_prov): Prohibit addition of a provider
with the reserved name, "_primary".
* t/basic: Exercise the new functionality.
---
 rest.c  |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 t/basic |   16 ++++++++++++++++
 2 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/rest.c b/rest.c
index a56f2d3..cc0a99b 100644
--- a/rest.c
+++ b/rest.c
@@ -1712,6 +1712,40 @@ url_to_provider_name (const char *url)
 }

 static int
+proxy_primary_prov (void *cctx, struct MHD_Connection *conn, const char *url,
+		    const char *method, const char *version, const char *data,
+		    size_t *data_size, void **rctx)
+{
+	(void)cctx;
+	(void)method;
+	(void)version;
+	(void)data;
+
+	DPRINTF("PROXY GET PRIMARY PROVIDER (%s)\n", url);
+
+	my_state *ms = *rctx;
+
+	// "/_providers/_primary" is the only one we accept for now.
+	bool valid = strcmp (url, "/_providers/_primary") == 0;
+	unsigned int rc = (valid ? MHD_HTTP_OK : MHD_HTTP_BAD_REQUEST);
+	if (!valid)
+		error (0, 0, "invalid request: %s", url);
+
+	const char *name = get_main_provider()->name;
+	struct MHD_Response *resp;
+	resp = MHD_create_response_from_data(valid ? strlen (name) : 0,
+					     valid ? (void *) name : NULL,
+					     MHD_NO, MHD_NO);
+	if (!resp) {
+		return MHD_NO;
+	}
+	MHD_queue_response(conn,rc,resp);
+	MHD_destroy_response(resp);
+
+	return MHD_YES;
+}
+
+static int
 proxy_delete_prov (void *cctx, struct MHD_Connection *conn, const char *url,
 		   const char *method, const char *version, const char *data,
 		   size_t *data_size, void **rctx)
@@ -1798,6 +1832,16 @@ proxy_add_prov (void *cctx, struct MHD_Connection *conn, const char *url,
 				name);
 			goto add_fail;
 		}
+
+		// another reserved word: provider name
+		// FIXME: don't hard-code it here
+		if (strcmp (prov_name, "_primary") == 0) {
+			fprintf(stderr,
+				"add_provider: %s is a reserved name\n",
+				prov_name);
+			goto add_fail;
+		}
+
 		// FIXME: unchecked strdup
 		g_hash_table_insert(ms->dict,strdup("name"),prov_name);

@@ -1888,6 +1932,8 @@ static const rule my_rules[] = {
 	  "GET",	URL_PROVLIST,	proxy_list_provs	},
 	{ /* update a provider */
 	  "POST",	URL_PROVLIST,	proxy_update_prov	},
+	{ /* get the primary provider */
+	  "GET",	URL_PROVIDER,	proxy_primary_prov	},
 	{ /* create a provider */
 	  "POST",	URL_PROVIDER,	proxy_add_prov		},
 	{ /* delete a provider */
diff --git a/t/basic b/t/basic
index 19eda9b..59e2608 100644
--- a/t/basic
+++ b/t/basic
@@ -350,6 +350,22 @@ curl -f -X DELETE http://localhost:$port/_providers/no-such 2> p
 test $? = 22 || fail=1
 grep ' 404$' p || fail=1

+# Get the name of the current primary provider.
+curl http://localhost:$port/_providers/_primary > p || fail=1
+test "$(cat p)" = primary || fail=1
+
+# Trying to GET with anything other than "_primary" returns the empty string.
+curl http://localhost:$port/_providers/anything > p 2>/dev/null || fail=1
+test -s p && fail=1
+
+# Try to add a provider with the reserved name.  It must fail.
+p_reserved_url=http://localhost:$port/_providers/_primary
+curl -dtype=http -dhost=localhost -dport=9091 $p_reserved_url || fail=1
+# Ensure it was not added.
+curl http://localhost:$port/_primary > p || fail=1
+grep _primary p && { warn_ add-provider/reserved-name not rejected; fail=1; }
+
+

 # Test "headless" operation (no access to metadata DB).
 kill -9 $mongo_pid
--
1.7.3.4


>From ae2735c029b4e79f7d382f6f3421a1a035da8a72 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Fri, 17 Dec 2010 17:07:47 +0100
Subject: [PATCH 7/7] new interface: curl -X PUT http://_providers/PROVIDER/_set_primary

* rest.c: Include <errno.h>.
(proxy_set_primary): New function.
(parse_url): Handle new type: URL_PROVIDER_SET_PRIMARY.
* setup.c (set_main_provider): New function.
* setup.h (set_main_provider): Declare it.
* t/basic: Exercise the new functionality.
---
 rest.c  |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 setup.c |    6 +++++
 setup.h |    1 +
 t/basic |   12 +++++++++++
 4 files changed, 83 insertions(+), 1 deletions(-)

diff --git a/rest.c b/rest.c
index cc0a99b..be4a888 100644
--- a/rest.c
+++ b/rest.c
@@ -28,6 +28,7 @@
 #include <unistd.h>
 #include <sys/stat.h>
 #include <assert.h>
+#include <errno.h>

 #include <microhttpd.h>
 #include <hstor.h>	/* only for ARRAY_SIZE at this point */
@@ -58,7 +59,7 @@

 typedef enum {
 	URL_ROOT=0, URL_BUCKET, URL_OBJECT, URL_ATTR, URL_INVAL,
-	URL_QUERY, URL_PROVLIST, URL_PROVIDER
+	URL_QUERY, URL_PROVLIST, URL_PROVIDER, URL_PROVIDER_SET_PRIMARY
 } url_type;

 typedef struct {
@@ -1746,6 +1747,62 @@ proxy_primary_prov (void *cctx, struct MHD_Connection *conn, const char *url,
 }

 static int
+proxy_set_primary (void *cctx, struct MHD_Connection *conn, const char *url,
+		   const char *method, const char *version, const char *data,
+		   size_t *data_size, void **rctx)
+{
+	(void)cctx;
+	(void)method;
+	(void)version;
+	(void)data;
+
+	DPRINTF("PROXY SET PRIMARY PROVIDER (%s)\n", url);
+
+	my_state *ms = *rctx;
+	char *name = NULL;
+	unsigned int rc = MHD_HTTP_BAD_REQUEST;
+
+	/* URL is guaranteed to be of the form "/_providers/NAME/_set_primary"
+	   Extract NAME:  */
+	bool valid = memcmp (url, "/_providers/", strlen("/_providers/")) == 0;
+	if (!valid) {
+		error (0, 0, "invalid request: %s", url);
+		goto bad_set;
+	}
+	const char *start = url + strlen("/_providers/");
+	const char *slash = strchr (start, '/');
+	if (slash == NULL) {
+		error (0, 0, "invalid request: %s", url);
+		goto bad_set;
+	}
+	name = strndup (start, slash - start);
+	if (name == NULL) {
+		error (0, errno, "failed to extract provider name: %s", url);
+		goto bad_set;
+	}
+
+	/* If it's not a provider name, you lose.  */
+	provider_t *prov = find_provider (name);
+	if (prov) {
+		rc = MHD_HTTP_OK;
+		set_main_provider (prov);
+	}
+
+ bad_set:
+	free (name);
+
+	struct MHD_Response *resp;
+	resp = MHD_create_response_from_data(0, NULL, MHD_NO, MHD_NO);
+	if (!resp) {
+		return MHD_NO;
+	}
+	MHD_queue_response(conn,rc,resp);
+	MHD_destroy_response(resp);
+
+	return MHD_YES;
+}
+
+static int
 proxy_delete_prov (void *cctx, struct MHD_Connection *conn, const char *url,
 		   const char *method, const char *version, const char *data,
 		   size_t *data_size, void **rctx)
@@ -1938,6 +1995,8 @@ static const rule my_rules[] = {
 	  "POST",	URL_PROVIDER,	proxy_add_prov		},
 	{ /* delete a provider */
 	  "DELETE",	URL_PROVIDER,	proxy_delete_prov	},
+	{ /* set the primary provider */
+	  "PUT",	URL_PROVIDER_SET_PRIMARY, proxy_set_primary },
 	{ NULL, 0, NULL }
 };

@@ -1994,6 +2053,10 @@ parse_url (const char *url, my_state *ms)
 	if (eindex == URL_OBJECT
 	    && !strcmp (parts[URL_BUCKET], "_providers"))
 	  eindex = URL_PROVIDER;
+	else if (eindex == URL_ATTR
+		 && !strcmp (parts[URL_BUCKET], "_providers")
+		 && !strcmp (parts[URL_ATTR], "_set_primary"))
+	  eindex = URL_PROVIDER_SET_PRIMARY;

 	DPRINTF("parse_url: %d: %s %s %s", eindex, parts[URL_BUCKET],
 		parts[URL_OBJECT], parts[URL_ATTR]);
diff --git a/setup.c b/setup.c
index f613f24..6ae5f01 100644
--- a/setup.c
+++ b/setup.c
@@ -78,6 +78,12 @@ get_main_provider (void)
   return main_prov;
 }

+void
+set_main_provider (provider_t *prov)
+{
+  main_prov = prov;
+}
+
 int
 validate_provider (GHashTable *h)
 {
diff --git a/setup.h b/setup.h
index ecdef5f..0bfbb81 100644
--- a/setup.h
+++ b/setup.h
@@ -53,5 +53,6 @@ provider_t *find_provider (const char *name);
 int add_provider (GHashTable *h);
 void delete_provider (provider_t *prov);
 provider_t *get_main_provider (void);
+void set_main_provider (provider_t *prov);

 #endif
diff --git a/t/basic b/t/basic
index 59e2608..436f123 100644
--- a/t/basic
+++ b/t/basic
@@ -365,6 +365,18 @@ curl -dtype=http -dhost=localhost -dport=9091 $p_reserved_url || fail=1
 curl http://localhost:$port/_primary > p || fail=1
 grep _primary p && { warn_ add-provider/reserved-name not rejected; fail=1; }

+# Move the "primary" attribute to a different provider.
+curl -X PUT $p1_url/_set_primary > p || fail=1
+test -s p && fail=1
+new_primary=$(curl http://localhost:$port/_providers/_primary) || fail=1
+test $new_primary = PROVIDER-1 || fail=1
+
+# Restore the primary attribute to the original.
+# FIXME: if I don't restore, the following headless test makes iwhd segfault.
+# Investigate that.
+p1_url=http://localhost:$port/_providers/primary
+curl -X PUT $p1_url/_set_primary > p || fail=1
+


 # Test "headless" operation (no access to metadata DB).
--
1.7.3.4


More information about the iwhd-devel mailing list