kvs: check that fence parameters are identical

src/common/libkvs/kvs.h

@@ -45,8 +45,11 @@ flux_future_t *flux_kvs_namespace_remove (flux_t *h, const char *name_space);
  * - if never set, the value from the environment variable
  *   FLUX_KVS_NAMESPACE is used.
  * - if FLUX_KVS_NAMESPACE is not set, KVS_PRIMARY_NAMESPACE is assumed.
+ *
+ *   NOTE: Take care to avoid conflicting with C++'s keyword "namespace"
+ *   in the external interfaces.
  */
-int flux_kvs_set_namespace (flux_t *h, const char *namespace);
+int flux_kvs_set_namespace (flux_t *h, const char *name_space);
 const char *flux_kvs_get_namespace (flux_t *h);
 
 /* Synchronization:

src/modules/kvs/fence.c

@@ -99,14 +99,14 @@ bool fence_count_reached (fence_t *f)
     return (f->count == f->nprocs);
 }
 
-int fence_get_flags (fence_t *f)
+int fence_get_nprocs (fence_t *f)
 {
-    return f->flags;
+    return f->nprocs;
 }
 
-void fence_set_flags (fence_t *f, int flags)
+int fence_get_flags (fence_t *f)
 {
-    f->flags = flags;
+    return f->flags;
 }
 
 json_t *fence_get_json_ops (fence_t *f)

src/modules/kvs/fence.h

@@ -15,8 +15,8 @@ void fence_destroy (fence_t *f);
 /* if number of calls to fence_add_request_data() is == nprocs */
 bool fence_count_reached (fence_t *f);
 
+int fence_get_nprocs (fence_t *f);
 int fence_get_flags (fence_t *f);
-void fence_set_flags (fence_t *f, int flags);
 
 json_t *fence_get_json_ops (fence_t *f);
 

src/modules/kvs/kvs.c

@@ -1726,8 +1726,12 @@ static void relayfence_request_cb (flux_t *h, flux_msg_handler_t *mh,
             goto error;
         }
     }
-    else
-        fence_set_flags (f, fence_get_flags (f) | flags);
+
+    if (fence_get_flags (f) != flags
+        || fence_get_nprocs (f) != nprocs) {
+        errno = EINVAL;
+        goto error;
+    }
 
     if (fence_add_request_data (f, ops) < 0) {
         flux_log_error (h, "%s: fence_add_request_data", __FUNCTION__);
@@ -1794,8 +1798,12 @@ static void fence_request_cb (flux_t *h, flux_msg_handler_t *mh,
             goto error;
         }
     }
-    else
-        fence_set_flags (f, fence_get_flags (f) | flags);
+
+    if (fence_get_flags (f) != flags
+        || fence_get_nprocs (f) != nprocs) {
+        errno = EINVAL;
+        goto error;
+    }
 
     if (fence_add_request_copy (f, msg) < 0)
         goto error;

src/modules/kvs/test/fence.c

@@ -41,13 +41,11 @@ void basic_api_tests (void)
     ok (fence_count_reached (f) == false,
         "initial fence_count_reached() is false");
 
-    ok (fence_get_flags (f) == 3,
-        "initial fence_get_flags works");
-
-    fence_set_flags (f, 5),
+    ok (fence_get_nprocs (f) == 1,
+        "fence_get_nprocs works");
 
-    ok (fence_get_flags (f) == 5,
-        "changed flags, fence_get_flags works");
+    ok (fence_get_flags (f) == 3,
+        "fence_get_flags works");
 
     ok ((o = fence_get_json_names (f)) != NULL,
         "initial fence_get_json_names works");

t/Makefile.am

@@ -186,6 +186,7 @@ check_PROGRAMS = \
 	kvs/commit \
 	kvs/commitmerge \
 	kvs/fence_namespace_remove \
+	kvs/fence_invalid \
 	module/basic \
 	request/treq \
 	barrier/tbarrier
@@ -313,6 +314,11 @@ kvs_fence_namespace_remove_CPPFLAGS = $(test_cppflags)
 kvs_fence_namespace_remove_LDADD = \
 	$(test_ldadd) $(LIBDL) $(LIBUTIL)
 
+kvs_fence_invalid_SOURCES = kvs/fence_invalid.c
+kvs_fence_invalid_CPPFLAGS = $(test_cppflags)
+kvs_fence_invalid_LDADD = \
+	$(test_ldadd) $(LIBDL) $(LIBUTIL)
+
 kvs_watch_disconnect_SOURCES = kvs/watch_disconnect.c
 kvs_watch_disconnect_CPPFLAGS = $(test_cppflags)
 kvs_watch_disconnect_LDADD = \

t/kvs/fence_invalid.c

@@ -0,0 +1,137 @@
+/*****************************************************************************\
+ *  Copyright (c) 2014 Lawrence Livermore National Security, LLC.  Produced at
+ *  the Lawrence Livermore National Laboratory (cf, AUTHORS, DISCLAIMER.LLNS).
+ *  LLNL-CODE-658032 All rights reserved.
+ *
+ *  This file is part of the Flux resource manager framework.
+ *  For details, see https://github.com/flux-framework.
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License as published by the Free
+ *  Software Foundation; either version 2 of the license, or (at your option)
+ *  any later version.
+ *
+ *  Flux is distributed in the hope that it will be useful, but WITHOUT
+ *  ANY WARRANTY; without even the IMPLIED WARRANTY OF MERCHANTABILITY or
+ *  FITNESS FOR A PARTICULAR PURPOSE.  See the terms and conditions of the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *  See also:  http://www.gnu.org/licenses/
+\*****************************************************************************/
+
+#if HAVE_CONFIG_H
+#include "config.h"
+#endif
+#include <assert.h>
+#include <libgen.h>
+#include <pthread.h>
+#include <getopt.h>
+#include <inttypes.h>
+#include <czmq.h>
+#include <jansson.h>
+#include <flux/core.h>
+
+#include "src/common/libutil/oom.h"
+#include "src/common/libutil/log.h"
+#include "src/common/libutil/xzmalloc.h"
+#include "src/common/libutil/monotime.h"
+#include "src/common/libutil/tstat.h"
+
+static char *prefix = NULL;
+
+static void usage (void)
+{
+    fprintf (stderr, "Usage: fence_invalid prefix\n");
+    exit (1);
+}
+
+int main (int argc, char *argv[])
+{
+    flux_t *h = NULL;
+    char *fence_name = NULL;
+    uint32_t rank;
+    char *key1 = NULL;
+    char *key2 = NULL;
+    flux_future_t *f1 = NULL;
+    flux_future_t *f2 = NULL;
+    flux_kvs_txn_t *txn1 = NULL;
+    flux_kvs_txn_t *txn2 = NULL;
+
+    log_init (basename (argv[0]));
+
+    if (argc != 2)
+        usage ();
+
+    prefix = argv[1];
+
+    if (!(h = flux_open (NULL, 0))) {
+        log_err_exit ("flux_open");
+        goto done;
+    }
+
+    if (flux_get_rank (h, &rank) < 0) {
+        log_err ("flux_get_rank");
+        goto done;
+    }
+
+    if (!(txn1 = flux_kvs_txn_create ())) {
+        log_err ("flux_kvs_txn_create");
+        goto done;
+    }
+
+    if (!(txn2 = flux_kvs_txn_create ())) {
+        log_err ("flux_kvs_txn_create");
+        goto done;
+    }
+
+    fence_name = xasprintf ("%s-%d", prefix, rank);
+    key1 = xasprintf ("%s.1.%d", prefix, rank);
+    key2 = xasprintf ("%s.2.%d", prefix, rank);
+
+    if (flux_kvs_txn_pack (txn1, 0, key1, "i", 42) < 0) {
+        log_err ("%s: flux_kvs_txn_pack", key1);
+        goto done;
+    }
+
+    if (flux_kvs_txn_pack (txn2, 0, key2, "i", 42) < 0) {
+        log_err ("%s: flux_kvs_txn_pack", key2);
+        goto done;
+    }
+
+    /* alter flags to generate an error on second fence */
+
+    if (!(f1 = flux_kvs_fence (h, 0x1, fence_name, 2, txn1))) {
+        log_err ("flux_kvs_fence");
+        goto done;
+    }
+
+    if (!(f2 = flux_kvs_fence (h, 0x2, fence_name, 2, txn2))) {
+        log_err ("flux_kvs_fence");
+        goto done;
+    }
+
+    if (flux_future_get (f2, NULL) < 0) {
+        printf ("flux_future_get: %s\n", flux_strerror (errno));
+        goto done;
+    }
+
+done:
+    flux_future_destroy (f1);
+    flux_future_destroy (f2);
+    free (key1);
+    free (key2);
+    free (fence_name);
+    flux_kvs_txn_destroy (txn1);
+    flux_kvs_txn_destroy (txn2);
+    flux_close (h);
+    log_fini ();
+
+    return 0;
+}
+
+/*
+ * vi:tabstop=4 shiftwidth=4 expandtab
+ */

t/t1001-kvs-internals.t

@@ -297,4 +297,18 @@ test_expect_success 'kvs: clear stats globally' '
         flux exec sh -c "flux module stats kvs | grep no-op | grep -q 0"
 '
 
+#
+# test invalid fence arguments
+#
+
+test_expect_success 'kvs: test invalid fence arguments on rank 0' '
+        ${FLUX_BUILD_DIR}/t/kvs/fence_invalid invalidtest1 > invalid_output &&
+        grep "flux_future_get: Invalid argument" invalid_output
+'
+
+test_expect_success 'kvs: test invalid fence arguments on rank 1' '
+        flux exec -r 1 sh -c "${FLUX_BUILD_DIR}/t/kvs/fence_invalid invalidtest2" > invalid_output &&
+        grep "flux_future_get: Invalid argument" invalid_output
+'
+
 test_done