From 908906951c9d4d4d5fa781cdc95161d3a4ad0539 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 2 Nov 2021 16:42:00 -0700 Subject: [PATCH 1/7] flux-kvs: add missing cmdline option to usage --- src/cmd/flux-kvs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/flux-kvs.c b/src/cmd/flux-kvs.c index af1bc31b2d0d..590199b0a788 100644 --- a/src/cmd/flux-kvs.c +++ b/src/cmd/flux-kvs.c @@ -550,7 +550,7 @@ static struct optparse_option namespace_create_opts[] = { static struct optparse_subcommand namespace_subcommands[] = { { "create", - "name [name...]", + "[-o owner] name [name...]", "Create a KVS namespace", cmd_namespace_create, 0, From fbae5599ce15274be0a6055063238ffab6bca0e5 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Thu, 4 Nov 2021 15:05:42 -0700 Subject: [PATCH 2/7] flux-kvs: support --blobref in getroot Problem: It is inconvenient to parse a getroot treeobj if you are only interested in the blobref. Solution: Add a --blobref option to flux kvs getroot to get just the blobref. --- doc/man1/flux-kvs.rst | 4 ++-- src/cmd/flux-kvs.c | 12 +++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/doc/man1/flux-kvs.rst b/doc/man1/flux-kvs.rst index ff8f8a0484ed..5f68d048d2fe 100644 --- a/doc/man1/flux-kvs.rst +++ b/doc/man1/flux-kvs.rst @@ -171,11 +171,11 @@ COMMANDS reads version, sends version to node B. Node B waits for version, gets value. -**getroot** [-N ns] [-s \| -o] +**getroot** [-N ns] [-s \| -o \| -b] Retrieve the current KVS root, displaying it as an RFC 11 dirref object. Specify an alternate namespace to retrieve from via *-N*. If *-o* is specified, display the namespace owner. If *-s* is specified, display - the root sequence number. + the root sequence number. If *-b* is specified, display the root blobref. **eventlog get** [-N ns] [-W] [-w] [-c count] [-u] *key* Display the contents of an RFC 18 KVS eventlog referred to by *key*. diff --git a/src/cmd/flux-kvs.c b/src/cmd/flux-kvs.c index 590199b0a788..984dd92967dd 100644 --- a/src/cmd/flux-kvs.c +++ b/src/cmd/flux-kvs.c @@ -205,6 +205,9 @@ static struct optparse_option getroot_opts[] = { { .name = "owner", .key = 'o', .has_arg = 0, .usage = "Show owner", }, + { .name = "blobref", .key = 'b', .has_arg = 0, + .usage = "Show blobref", + }, OPTPARSE_TABLE_END }; @@ -355,7 +358,7 @@ static struct optparse_subcommand subcommands[] = { namespace_opt }, { "getroot", - "[-N ns] [-s|-o]", + "[-N ns] [-s|-o|-b]", "Get KVS root treeobj", cmd_getroot, 0, @@ -1741,6 +1744,13 @@ void getroot_continuation (flux_future_t *f, void *arg) log_err_exit ("flux_kvs_getroot_get_sequence"); printf ("%d\n", sequence); } + else if (optparse_hasopt (p, "blobref")) { + const char *blobref; + + if (flux_kvs_getroot_get_blobref (f, &blobref) < 0) + log_err_exit ("flux_kvs_getroot_get_blobref"); + printf ("%s\n", blobref); + } else { const char *treeobj; From 0ebdfc45e0ebf081aaffcd2d3838be5e7e71866a Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Thu, 4 Nov 2021 15:09:09 -0700 Subject: [PATCH 3/7] testsuite: add flux kvs getroot --blobref test --- t/t1000-kvs.t | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/t/t1000-kvs.t b/t/t1000-kvs.t index 158fa917ece6..3ed875c90dbd 100755 --- a/t/t1000-kvs.t +++ b/t/t1000-kvs.t @@ -1195,6 +1195,13 @@ test_expect_success 'flux kvs getroot --owner returns instance owner' ' test $OWNER -eq $(id -u) ' +test_expect_success 'flux kvs getroot --blobref returns changing blobrefs' ' + BLOBREF1=$(flux kvs getroot --blobref) && + flux kvs put test.c=barf && + BLOBREF2=$(flux kvs getroot --blobref) && + test $BLOBREF1 != $BLOBREF2 +' + test_expect_success 'flux kvs getroot works on alt namespace' ' flux kvs namespace create testns1 && SEQ=$(flux kvs getroot --namespace=testns1 --sequence) && From 7314bca7744fb7ad4db9a85c6d3cc7c50bf8fb5d Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Fri, 5 Nov 2021 11:14:02 -0700 Subject: [PATCH 4/7] kvs: support rootref in namespace create RPC Problem: In the future we would like to checkpoint KVS guest namespaces and restart them. However, there is no way to "re-initialize" a KVS namespace to a specific root reference. Solution: Refactor the KVS namespace create RPC to take a root reference. Callers must supply the initial root reference. Update flux_kvs_namespace_create() to supply the root reference of an empty directory. --- src/common/libkvs/kvs.c | 41 ++++++++++++++++++++++++++++++++++++----- src/modules/kvs/kvs.c | 33 +++++++-------------------------- 2 files changed, 43 insertions(+), 31 deletions(-) diff --git a/src/common/libkvs/kvs.c b/src/common/libkvs/kvs.c index d6d535ce7cfa..14ac86ded92f 100644 --- a/src/common/libkvs/kvs.c +++ b/src/common/libkvs/kvs.c @@ -16,22 +16,53 @@ #include #include +#include "src/common/libutil/blobref.h" + +#include "treeobj.h" #include "kvs_util_private.h" flux_future_t *flux_kvs_namespace_create (flux_t *h, const char *ns, uint32_t owner, int flags) { + flux_future_t *rv = NULL; + const char *hash_name; + json_t *rootdir = NULL; + char rootref[BLOBREF_MAX_STRING_SIZE]; + void *data = NULL; + int len; + if (!ns || flags) { errno = EINVAL; return NULL; } + if (!(hash_name = flux_attr_get (h, "content.hash"))) + goto cleanup; + + if (!(rootdir = treeobj_create_dir ())) + goto cleanup; + + if (!(data = treeobj_encode (rootdir))) + goto cleanup; + len = strlen (data); + + /* N.B. blobref of empty treeobj dir guaranteed to be in content store + * b/c that is how the primary KVS is initialized. + */ + if (blobref_hash (hash_name, data, len, rootref, sizeof (rootref)) < 0) + goto cleanup; + /* N.B. owner cast to int */ - return flux_rpc_pack (h, "kvs.namespace-create", 0, 0, - "{ s:s s:i s:i }", - "namespace", ns, - "owner", owner, - "flags", flags); + rv = flux_rpc_pack (h, "kvs.namespace-create", 0, 0, + "{ s:s s:s s:i s:i }", + "namespace", ns, + "rootref", rootref, + "owner", owner, + "flags", flags); +cleanup: + json_decref (rootdir); + free (data); + return rv; } flux_future_t *flux_kvs_namespace_remove (flux_t *h, const char *ns) diff --git a/src/modules/kvs/kvs.c b/src/modules/kvs/kvs.c index 65d840f34f3a..e79536c6eae5 100644 --- a/src/modules/kvs/kvs.c +++ b/src/modules/kvs/kvs.c @@ -2230,15 +2230,12 @@ static void stats_clear_request_cb (flux_t *h, flux_msg_handler_t *mh, } static int namespace_create (struct kvs_ctx *ctx, const char *ns, - uint32_t owner, int flags, const char **errmsg) + const char *rootref, uint32_t owner, + int flags, const char **errmsg) { struct kvsroot *root; - json_t *rootdir = NULL; - char ref[BLOBREF_MAX_STRING_SIZE]; - void *data = NULL; flux_msg_t *msg = NULL; char *topic = NULL; - int len; int rv = -1; /* If namespace already exists, return EEXIST. Doesn't matter if @@ -2262,23 +2259,7 @@ static int namespace_create (struct kvs_ctx *ctx, const char *ns, return -1; } - if (!(rootdir = treeobj_create_dir ())) { - flux_log_error (ctx->h, "%s: treeobj_create_dir", __FUNCTION__); - goto cleanup; - } - - if (!(data = treeobj_encode (rootdir))) { - flux_log_error (ctx->h, "%s: treeobj_encode", __FUNCTION__); - goto cleanup; - } - len = strlen (data); - - if (blobref_hash (ctx->hash_name, data, len, ref, sizeof (ref)) < 0) { - flux_log_error (ctx->h, "%s: blobref_hash", __FUNCTION__); - goto cleanup; - } - - setroot (ctx, root, ref, 0); + setroot (ctx, root, rootref, 0); if (event_subscribe (ctx, ns) < 0) { flux_log_error (ctx->h, "%s: event_subscribe", __FUNCTION__); @@ -2312,8 +2293,6 @@ static int namespace_create (struct kvs_ctx *ctx, const char *ns, cleanup: if (rv < 0) kvsroot_mgr_remove_root (ctx->krm, ns); - free (data); - json_decref (rootdir); free (topic); flux_msg_destroy (msg); return rv; @@ -2325,14 +2304,16 @@ static void namespace_create_request_cb (flux_t *h, flux_msg_handler_t *mh, struct kvs_ctx *ctx = arg; const char *errmsg = NULL; const char *ns; + const char *rootref; uint32_t owner; int flags; assert (ctx->rank == 0); /* N.B. owner read into uint32_t */ - if (flux_request_unpack (msg, NULL, "{ s:s s:i s:i }", + if (flux_request_unpack (msg, NULL, "{ s:s s:s s:i s:i }", "namespace", &ns, + "rootref", &rootref, "owner", &owner, "flags", &flags) < 0) { flux_log_error (h, "%s: flux_request_unpack", __FUNCTION__); @@ -2342,7 +2323,7 @@ static void namespace_create_request_cb (flux_t *h, flux_msg_handler_t *mh, if (owner == FLUX_USERID_UNKNOWN) owner = getuid (); - if (namespace_create (ctx, ns, owner, flags, &errmsg) < 0) + if (namespace_create (ctx, ns, rootref, owner, flags, &errmsg) < 0) goto error; if (flux_respond (h, msg, NULL) < 0) From f5604ffba13accaaed0e0eab52f3936ee8b06ff4 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Fri, 5 Nov 2021 11:53:24 -0700 Subject: [PATCH 5/7] libkvs: Support flux_kvs_namespace_create_with() Problem: libkvs does not support a way to initialize a namespace to a specific rootref. Solution: Add a new function flux_kvs_namespace_create_with(). --- doc/man3/flux_kvs_namespace_create.rst | 13 +++++++++++++ src/common/libkvs/kvs.c | 18 ++++++++++++++++++ src/common/libkvs/kvs.h | 3 +++ src/common/libkvs/test/kvs.c | 5 +++++ 4 files changed, 39 insertions(+) diff --git a/doc/man3/flux_kvs_namespace_create.rst b/doc/man3/flux_kvs_namespace_create.rst index 9f048f33151b..e59b9be3cbea 100644 --- a/doc/man3/flux_kvs_namespace_create.rst +++ b/doc/man3/flux_kvs_namespace_create.rst @@ -17,6 +17,14 @@ SYNOPSIS uint32_t owner, int flags); +:: + + flux_future_t *flux_kvs_namespace_create_with (flux_t *h, + const char *namespace, + const char *rootref, + uint32_t owner, + int flags); + :: flux_future_t *flux_kvs_namespace_remove (flux_t *h, @@ -32,6 +40,11 @@ other KVS namespaces. An owner of the namespace other than the instance owner can be chosen by setting *owner*. Otherwise, *owner* can be set to FLUX_USERID_UNKNOWN. +``flux_kvs_namespace_create_with()`` is identical to +``flux_kvs_namespace_create()`` but will initialize the namespace to +the specified *rootref*. This may be useful in several circumstances, +such as initializing a namespace to an earlier checkpoint. + ``flux_kvs_namespace_remove()`` removes a KVS namespace. diff --git a/src/common/libkvs/kvs.c b/src/common/libkvs/kvs.c index 14ac86ded92f..050a1cb756a7 100644 --- a/src/common/libkvs/kvs.c +++ b/src/common/libkvs/kvs.c @@ -65,6 +65,24 @@ flux_future_t *flux_kvs_namespace_create (flux_t *h, const char *ns, return rv; } +flux_future_t *flux_kvs_namespace_create_with (flux_t *h, const char *ns, + const char *rootref, + uint32_t owner, int flags) +{ + if (!ns || !rootref || flags) { + errno = EINVAL; + return NULL; + } + + /* N.B. owner cast to int */ + return flux_rpc_pack (h, "kvs.namespace-create", 0, 0, + "{ s:s s:s s:i s:i }", + "namespace", ns, + "rootref", rootref, + "owner", owner, + "flags", flags); +} + flux_future_t *flux_kvs_namespace_remove (flux_t *h, const char *ns) { if (!ns) { diff --git a/src/common/libkvs/kvs.h b/src/common/libkvs/kvs.h index 423d1a7dc53b..a253242d4b1f 100644 --- a/src/common/libkvs/kvs.h +++ b/src/common/libkvs/kvs.h @@ -49,6 +49,9 @@ enum kvs_op { */ flux_future_t *flux_kvs_namespace_create (flux_t *h, const char *ns, uint32_t owner, int flags); +flux_future_t *flux_kvs_namespace_create_with (flux_t *h, const char *ns, + const char *rootref, + uint32_t owner, int flags); flux_future_t *flux_kvs_namespace_remove (flux_t *h, const char *ns); /* Synchronization: diff --git a/src/common/libkvs/test/kvs.c b/src/common/libkvs/test/kvs.c index a89d75722caa..7b4dea7661ec 100644 --- a/src/common/libkvs/test/kvs.c +++ b/src/common/libkvs/test/kvs.c @@ -27,6 +27,11 @@ void errors (void) ok (flux_kvs_namespace_create (NULL, NULL, 0, 5) == NULL && errno == EINVAL, "flux_kvs_namespace_create fails on bad input"); + errno = 0; + ok (flux_kvs_namespace_create_with (NULL, NULL, NULL, 0, 5) == NULL + && errno == EINVAL, + "flux_kvs_namespace_create_with fails on bad input"); + errno = 0; ok (flux_kvs_namespace_remove (NULL, NULL) == NULL && errno == EINVAL, "flux_kvs_namespace_remove fails on bad input"); From b317a54e5cd13111eb00ca85af06c880d36eae7c Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 2 Nov 2021 16:41:15 -0700 Subject: [PATCH 6/7] flux-kvs: support rootref on namespace create --- doc/man1/flux-kvs.rst | 4 +++- src/cmd/flux-kvs.c | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/doc/man1/flux-kvs.rst b/doc/man1/flux-kvs.rst index 5f68d048d2fe..a90031d7ecfa 100644 --- a/doc/man1/flux-kvs.rst +++ b/doc/man1/flux-kvs.rst @@ -48,10 +48,12 @@ arguments are described below. COMMANDS ======== -**namespace create** [-o owner] *name* [*name* ...] +**namespace create** [-o owner] [-r rootref] *name* [*name* ...] Create a new kvs namespace. User may specify an alternate userid of a user that owns the namespace via *-o*. Specifying an alternate owner would allow a non-instance owner to read/write to a namespace. + User may specify an initial root reference for the namespace via + *-r*. **namespace remove** *name* [*name...*] Remove a kvs namespace. diff --git a/src/cmd/flux-kvs.c b/src/cmd/flux-kvs.c index 984dd92967dd..5f1cb47f0fcf 100644 --- a/src/cmd/flux-kvs.c +++ b/src/cmd/flux-kvs.c @@ -439,6 +439,7 @@ int cmd_namespace_create (optparse_t *p, int argc, char **argv) int optindex, i; uint32_t owner = FLUX_USERID_UNKNOWN; const char *str; + const char *rootref; optindex = optparse_option_index (p); if ((optindex - argc) == 0) { @@ -453,13 +454,19 @@ int cmd_namespace_create (optparse_t *p, int argc, char **argv) log_msg_exit ("--owner requires an unsigned integer argument"); } + rootref = optparse_get_str (p, "rootref", NULL); + if (!(h = flux_open (NULL, 0))) log_err_exit ("flux_open"); for (i = optindex; i < argc; i++) { const char *name = argv[i]; int flags = 0; - if (!(f = flux_kvs_namespace_create (h, name, owner, flags))) + if (rootref) + f = flux_kvs_namespace_create_with (h, name, rootref, owner, flags); + else + f = flux_kvs_namespace_create (h, name, owner, flags); + if (!f) log_err_exit ("%s", name); if (flux_future_get (f, NULL) < 0) log_msg_exit ("%s: %s", name, future_strerror (f, errno)); @@ -548,12 +555,15 @@ static struct optparse_option namespace_create_opts[] = { { .name = "owner", .key = 'o', .has_arg = 1, .usage = "Specify alternate namespace owner via userid", }, + { .name = "rootref", .key = 'r', .has_arg = 1, + .usage = "Initialize namespace with specific root reference", + }, OPTPARSE_TABLE_END }; static struct optparse_subcommand namespace_subcommands[] = { { "create", - "[-o owner] name [name...]", + "[-o owner] [-r rootref] name [name...]", "Create a KVS namespace", cmd_namespace_create, 0, From 21acfef8176962dbd1fada8ee6ea77d07416c66a Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Thu, 4 Nov 2021 15:04:14 -0700 Subject: [PATCH 7/7] testsuite: add namespace create with rootref tests --- t/t1004-kvs-namespace.t | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/t/t1004-kvs-namespace.t b/t/t1004-kvs-namespace.t index 2636dfcce6bc..b958fcbc6802 100755 --- a/t/t1004-kvs-namespace.t +++ b/t/t1004-kvs-namespace.t @@ -27,6 +27,7 @@ NAMESPACETEST=namespacetest NAMESPACETMP=namespacetmp NAMESPACERANK1=namespacerank1 NAMESPACEORDER=namespaceorder +NAMESPACEROOTREF=namespacerootref namespace_create_loop() { i=0 @@ -339,6 +340,39 @@ test_expect_success 'kvs: put - namespace specified in command line overrides en test_kvs_key_namespace $NAMESPACEORDER-2 $DIR.puttest 5 ' +# +# Namespace rootref initialization +# + +test_expect_success HAVE_JQ 'kvs: namespace rootref setup' ' + flux kvs namespace create $NAMESPACEROOTREF-1 && + flux kvs put --namespace=$NAMESPACEROOTREF-1 $DIR.rootreftest=foobar && + test_kvs_key_namespace $NAMESPACEROOTREF-1 $DIR.rootreftest foobar && + flux kvs getroot --blobref --namespace=$NAMESPACEROOTREF-1 > rootref1 +' + +test_expect_success HAVE_JQ 'kvs: namespace create with init rootref' ' + flux kvs namespace create --rootref=$(cat rootref1) $NAMESPACEROOTREF-2 && + test_kvs_key_namespace $NAMESPACEROOTREF-1 $DIR.rootreftest foobar +' + +test_expect_success HAVE_JQ 'kvs: namespaces dont clobber each other' ' + flux kvs put --namespace=$NAMESPACEROOTREF-1 $DIR.val=42 && + flux kvs put --namespace=$NAMESPACEROOTREF-2 $DIR.val=43 && + test_kvs_key_namespace $NAMESPACEROOTREF-1 $DIR.val 42 && + test_kvs_key_namespace $NAMESPACEROOTREF-2 $DIR.val 43 +' + +BADROOTREF="sha1-0123456789abcdef0123456789abcdef01234567" +test_expect_success HAVE_JQ 'kvs: namespace create can take bad blobref' ' + flux kvs namespace create --rootref=$BADROOTREF $NAMESPACEROOTREF-3 && + flux kvs get --namespace=$NAMESPACEROOTREF-3 --treeobj . +' + +test_expect_success HAVE_JQ 'kvs: namespace with bad rootref fails otherwise' ' + test_must_fail flux kvs ls --namespace=$NAMESPACEROOTREF-3 . +' + # # Namespace corner case tests #