diff --git a/src/modules/kvs/kvs.c b/src/modules/kvs/kvs.c index 1dbaec0fc08d..7396ad996c4f 100644 --- a/src/modules/kvs/kvs.c +++ b/src/modules/kvs/kvs.c @@ -82,6 +82,12 @@ typedef struct { const char *hash_name; } kvs_ctx_t; +struct kvs_cb_data { + kvs_ctx_t *ctx; + wait_t *wait; + int errnum; +}; + static int setroot_event_send (kvs_ctx_t *ctx, json_t *names); static int error_event_send (kvs_ctx_t *ctx, json_t *names, int errnum); static void commit_prep_cb (flux_reactor_t *r, flux_watcher_t *w, @@ -444,15 +450,9 @@ static void setroot (kvs_ctx_t *ctx, const char *rootdir, int rootseq) } } -struct commit_cb_data { - kvs_ctx_t *ctx; - wait_t *wait; - int errnum; -}; - static int commit_load_cb (commit_t *c, const char *ref, void *data) { - struct commit_cb_data *cbd = data; + struct kvs_cb_data *cbd = data; bool stall; /* is_raw flag is always false on commit loads, we will never @@ -474,7 +474,7 @@ static int commit_load_cb (commit_t *c, const char *ref, void *data) */ static int commit_cache_cb (commit_t *c, struct cache_entry *hp, void *data) { - struct commit_cb_data *cbd = data; + struct kvs_cb_data *cbd = data; void *storedata; int storedatalen = 0; bool is_raw; @@ -529,7 +529,7 @@ static void commit_apply (commit_t *c) } if (ret == COMMIT_PROCESS_LOAD_MISSING_REFS) { - struct commit_cb_data cbd; + struct kvs_cb_data cbd; if (!(wait = wait_create ((wait_cb_f)commit_apply, c))) { errnum = errno; @@ -556,7 +556,7 @@ static void commit_apply (commit_t *c) goto stall; } else if (ret == COMMIT_PROCESS_DIRTY_CACHE_ENTRIES) { - struct commit_cb_data cbd; + struct kvs_cb_data cbd; if (!(wait = wait_create ((wait_cb_f)commit_apply, c))) { errnum = errno; @@ -714,6 +714,22 @@ static void heartbeat_cb (flux_t *h, flux_msg_handler_t *w, flux_log_error (ctx->h, "%s: cache_expire_entries", __FUNCTION__); } +static int lookup_load_cb (lookup_t *lh, const char *ref, + bool raw_data, void *data) +{ + struct kvs_cb_data *cbd = data; + bool stall; + + if (load (cbd->ctx, ref, raw_data, cbd->wait, &stall) < 0) { + cbd->errnum = errno; + flux_log_error (cbd->ctx->h, "%s: load", __FUNCTION__); + return -1; + } + /* if not stalling, logic issue within code */ + assert (stall); + return 0; +} + static void get_request_cb (flux_t *h, flux_msg_handler_t *w, const flux_msg_t *msg, void *arg) { @@ -769,8 +785,16 @@ static void get_request_cb (flux_t *h, flux_msg_handler_t *w, assert (ret == 0); } else { + int err; + lh = arg; + /* error in prior load(), waited for in flight rpcs to complete */ + if ((err = lookup_get_aux_errnum (lh))) { + errno = err; + goto done; + } + ctx = lookup_get_aux_data (lh); assert (ctx); @@ -779,20 +803,28 @@ static void get_request_cb (flux_t *h, flux_msg_handler_t *w, } if (!lookup (lh)) { - const char *missing_ref; - bool ref_raw, stall; - - missing_ref = lookup_get_missing_ref (lh, &ref_raw); - assert (missing_ref); + struct kvs_cb_data cbd; if (!(wait = wait_create_msg_handler (h, w, msg, get_request_cb, lh))) goto done; - if (load (ctx, missing_ref, ref_raw, wait, &stall) < 0) { - flux_log_error (h, "%s: load", __FUNCTION__); + + cbd.ctx = ctx; + cbd.wait = wait; + cbd.errnum = 0; + + if (lookup_iter_missing_refs (lh, lookup_load_cb, &cbd) < 0) { + errno = cbd.errnum; + + /* rpcs already in flight, stall for them to complete */ + if (wait_get_usecount (wait) > 0) { + lookup_set_aux_errnum (lh, cbd.errnum); + goto stall; + } + goto done; } - /* if not stalling, logic issue within code */ - assert (stall); + + assert (wait_get_usecount (wait) > 0); goto stall; } if (lookup_get_errnum (lh) != 0) { @@ -875,8 +907,16 @@ static void watch_request_cb (flux_t *h, flux_msg_handler_t *w, assert (ret == 0); } else { + int err; + lh = arg; + /* error in prior load(), waited for in flight rpcs to complete */ + if ((err = lookup_get_aux_errnum (lh))) { + errno = err; + goto done; + } + ctx = lookup_get_aux_data (lh); assert (ctx); @@ -887,20 +927,28 @@ static void watch_request_cb (flux_t *h, flux_msg_handler_t *w, } if (!lookup (lh)) { - const char *missing_ref; - bool ref_raw, stall; - - missing_ref = lookup_get_missing_ref (lh, &ref_raw); - assert (missing_ref); + struct kvs_cb_data cbd; if (!(wait = wait_create_msg_handler (h, w, msg, watch_request_cb, lh))) goto done; - if (load (ctx, missing_ref, ref_raw, wait, &stall) < 0) { - flux_log_error (h, "%s: load", __FUNCTION__); + + cbd.ctx = ctx; + cbd.wait = wait; + cbd.errnum = 0; + + if (lookup_iter_missing_refs (lh, lookup_load_cb, &cbd) < 0) { + errno = cbd.errnum; + + /* rpcs already in flight, stall for them to complete */ + if (wait_get_usecount (wait) > 0) { + lookup_set_aux_errnum (lh, cbd.errnum); + goto stall; + } + goto done; } - /* if not stalling, logic issue within code */ - assert (stall); + + assert (wait_get_usecount (wait) > 0); goto stall; } if (lookup_get_errnum (lh) != 0) { diff --git a/src/modules/kvs/lookup.c b/src/modules/kvs/lookup.c index 647db873a163..cb1df58f79af 100644 --- a/src/modules/kvs/lookup.c +++ b/src/modules/kvs/lookup.c @@ -78,9 +78,15 @@ struct lookup { /* potential return values from lookup */ json_t *val; /* value of lookup */ - const char *missing_ref; /* on stall, missing ref to load */ - bool missing_ref_raw; /* if true, missing ref points to raw data */ + + /* if valref_missing_refs is true, iterate on refs, else + * return missing_ref string. + */ + json_t *valref_missing_refs; + const char *missing_ref; + int errnum; /* errnum if error */ + int aux_errnum; /* API internal */ json_t *root_dirent; @@ -248,7 +254,6 @@ static bool walk (lookup_t *lh) if (!(hp = cache_lookup (lh->cache, refstr, lh->current_epoch)) || !cache_entry_get_valid (hp)) { lh->missing_ref = refstr; - lh->missing_ref_raw = false; goto stall; } if (!(dir = cache_entry_get_json (hp))) { @@ -425,8 +430,8 @@ lookup_t *lookup_create (struct cache *cache, lh->aux = NULL; lh->val = NULL; + lh->valref_missing_refs = NULL; lh->missing_ref = NULL; - lh->missing_ref_raw = false; lh->errnum = 0; if (!(lh->root_dirent = treeobj_create_dirref (lh->root_ref))) { @@ -490,6 +495,22 @@ int lookup_get_errnum (lookup_t *lh) return EINVAL; } +int lookup_get_aux_errnum (lookup_t *lh) +{ + if (lh && lh->magic == LOOKUP_MAGIC) + return lh->aux_errnum; + return EINVAL; +} + +int lookup_set_aux_errnum (lookup_t *lh, int errnum) +{ + if (lh && lh->magic == LOOKUP_MAGIC) { + lh->aux_errnum = errnum; + return lh->aux_errnum; + } + return EINVAL; +} + json_t *lookup_get_value (lookup_t *lh) { if (lh @@ -500,18 +521,49 @@ json_t *lookup_get_value (lookup_t *lh) return NULL; } -const char *lookup_get_missing_ref (lookup_t *lh, bool *ref_raw) +int lookup_iter_missing_refs (lookup_t *lh, lookup_ref_f cb, void *data) { if (lh && lh->magic == LOOKUP_MAGIC && (lh->state == LOOKUP_STATE_CHECK_ROOT || lh->state == LOOKUP_STATE_WALK || lh->state == LOOKUP_STATE_VALUE)) { - if (ref_raw) - (*ref_raw) = lh->missing_ref_raw; - return lh->missing_ref; + if (lh->valref_missing_refs) { + int refcount, i; + + if (!treeobj_is_valref (lh->valref_missing_refs)) { + errno = EPERM; + return -1; + } + + refcount = treeobj_get_count (lh->valref_missing_refs); + assert (refcount > 0); + + for (i = 0; i < refcount; i++) { + struct cache_entry *hp; + const char *ref; + + if (!(ref = treeobj_get_blobref (lh->valref_missing_refs, i))) + return -1; + + if (!(hp = cache_lookup (lh->cache, ref, lh->current_epoch)) + || !cache_entry_get_valid (hp)) { + + /* valref points to raw data, raw_data flag is always + * true */ + if (cb (lh, ref, true, data) < 0) + return -1; + } + } + } + else { + if (cb (lh, lh->missing_ref, false, data) < 0) + return -1; + } + return 0; } - return NULL; + errno = EINVAL; + return -1; } struct cache *lookup_get_cache (lookup_t *lh) @@ -581,6 +633,150 @@ int lookup_set_aux_data (lookup_t *lh, void *data) return -1; } +/* return 0 on success, -1 on failure. On success, stall should be + * checked */ +static int get_single_blobref_valref_value (lookup_t *lh, bool *stall) +{ + struct cache_entry *hp; + const char *reftmp; + void *valdata; + int len; + + if (!(reftmp = treeobj_get_blobref (lh->wdirent, 0))) { + lh->errnum = errno; + return -1; + } + if (!(hp = cache_lookup (lh->cache, reftmp, lh->current_epoch)) + || !cache_entry_get_valid (hp)) { + lh->valref_missing_refs = lh->wdirent; + (*stall) = true; + return 0; + } + if (cache_entry_get_raw (hp, &valdata, &len) < 0) { + flux_log (lh->h, LOG_ERR, "valref points to non-raw data"); + lh->errnum = EPERM; + return -1; + } + if (!(lh->val = treeobj_create_val (valdata, len))) { + lh->errnum = errno; + return -1; + } + (*stall) = false; + return 0; +} + +static int get_multi_blobref_valref_length (lookup_t *lh, int refcount, + int *total_len, bool *stall) +{ + struct cache_entry *hp; + const char *reftmp; + int total = 0; + int len; + int i; + + for (i = 0; i < refcount; i++) { + if (!(reftmp = treeobj_get_blobref (lh->wdirent, i))) { + lh->errnum = errno; + return -1; + } + if (!(hp = cache_lookup (lh->cache, reftmp, lh->current_epoch)) + || !cache_entry_get_valid (hp)) { + lh->valref_missing_refs = lh->wdirent; + (*stall) = true; + return 0; + } + + if (cache_entry_get_raw (hp, NULL, &len) < 0) { + flux_log (lh->h, LOG_ERR, "valref points to non-raw data"); + lh->errnum = EPERM; + return -1; + } + + /* cache ensures all lens >= 0 */ + if (len > (INT_MAX - total)) { + lh->errnum = EOVERFLOW; + return -1; + } + total += len; + } + + (*total_len) = total; + (*stall) = false; + return 0; +} + +static char *get_multi_blobref_valref_data (lookup_t *lh, int refcount, + int total_len) +{ + struct cache_entry *hp; + const char *reftmp; + void *valdata; + int len; + char *valbuf = NULL; + int pos = 0; + int i; + + if (!(valbuf = malloc (total_len))) { + lh->errnum = errno; + return NULL; + } + + for (i = 0; i < refcount; i++) { + int ret; + + /* this function should only be called if all cache entries + * known to be valid & raw, thus assert checks below */ + + reftmp = treeobj_get_blobref (lh->wdirent, i); + assert (reftmp); + + hp = cache_lookup (lh->cache, reftmp, lh->current_epoch); + assert (hp); + assert (cache_entry_get_valid (hp)); + + ret = cache_entry_get_raw (hp, &valdata, &len); + assert (ret == 0); + + memcpy (valbuf + pos, valdata, len); + pos += len; + assert (pos <= total_len); + } + + return valbuf; +} + +/* return 0 on success, -1 on failure. On success, stall should be + * check */ +static int get_multi_blobref_valref_value (lookup_t *lh, int refcount, + bool *stall) +{ + char *valbuf = NULL; + int total_len = 0; + int rc = -1; + + if (get_multi_blobref_valref_length (lh, refcount, &total_len, stall) < 0) + goto done; + + if ((*stall) == true) { + rc = 0; + goto done; + } + + if (!(valbuf = get_multi_blobref_valref_data (lh, refcount, total_len))) + goto done; + + if (!(lh->val = treeobj_create_val (valbuf, total_len))) { + lh->errnum = errno; + goto done; + } + + (*stall) = false; + rc = 0; +done: + free (valbuf); + return rc; +} + bool lookup (lookup_t *lh) { json_t *valtmp = NULL; @@ -614,7 +810,6 @@ bool lookup (lookup_t *lh) || !cache_entry_get_valid (hp)) { lh->state = LOOKUP_STATE_CHECK_ROOT; lh->missing_ref = lh->root_ref; - lh->missing_ref_raw = false; goto stall; } if (!(valtmp = cache_entry_get_json (hp))) { @@ -678,7 +873,6 @@ bool lookup (lookup_t *lh) if (!(hp = cache_lookup (lh->cache, reftmp, lh->current_epoch)) || !cache_entry_get_valid (hp)) { lh->missing_ref = reftmp; - lh->missing_ref_raw = false; goto stall; } if (!(valtmp = cache_entry_get_json (hp))) { @@ -693,8 +887,7 @@ bool lookup (lookup_t *lh) } lh->val = json_incref (valtmp); } else if (treeobj_is_valref (lh->wdirent)) { - void *valdata; - int len; + bool stall; if ((lh->flags & FLUX_KVS_READLINK)) { lh->errnum = EINVAL; @@ -708,30 +901,25 @@ bool lookup (lookup_t *lh) lh->errnum = errno; goto done; } - if (refcount != 1) { + if (!refcount) { flux_log (lh->h, LOG_ERR, "invalid valref count: %d", refcount); lh->errnum = EPERM; goto done; } - if (!(reftmp = treeobj_get_blobref (lh->wdirent, 0))) { - lh->errnum = errno; - goto done; - } - if (!(hp = cache_lookup (lh->cache, reftmp, lh->current_epoch)) - || !cache_entry_get_valid (hp)) { - lh->missing_ref = reftmp; - lh->missing_ref_raw = true; - goto stall; - } - if (cache_entry_get_raw (hp, &valdata, &len) < 0) { - flux_log (lh->h, LOG_ERR, "valref points to non-raw data"); - lh->errnum = EPERM; - goto done; + if (refcount == 1) { + if (get_single_blobref_valref_value (lh, &stall) < 0) + goto done; + if (stall) + goto stall; } - if (!(lh->val = treeobj_create_val (valdata, len))) { - lh->errnum = errno; - goto done; + else { + if (get_multi_blobref_valref_value (lh, + refcount, + &stall) < 0) + goto done; + if (stall) + goto stall; } } else if (treeobj_is_dir (lh->wdirent)) { if ((lh->flags & FLUX_KVS_READLINK)) { diff --git a/src/modules/kvs/lookup.h b/src/modules/kvs/lookup.h index ad18713708a2..34b49c018063 100644 --- a/src/modules/kvs/lookup.h +++ b/src/modules/kvs/lookup.h @@ -6,6 +6,14 @@ typedef struct lookup lookup_t; +/* ref - missing reference + * raw_data - true if reference points to raw data + */ +typedef int (*lookup_ref_f)(lookup_t *c, + const char *ref, + bool raw_data, + void *data); + /* Initialize a lookup handle * - If root_ref is same as root_dir, can be set to NULL. * - flux_t is optional, if NULL logging will go to stderr @@ -28,18 +36,23 @@ bool lookup_validate (lookup_t *lh); * an error occurred or not */ int lookup_get_errnum (lookup_t *lh); +/* if user wishes to stall, but needs future knowledge to fail and + * what error caused the failure. + */ +int lookup_get_aux_errnum (lookup_t *lh); +int lookup_set_aux_errnum (lookup_t *lh, int errnum); + /* Get resulting value of lookup() after lookup() returns true. The * json object returned gives a reference to the caller and must be * json_decref()'ed to free memory. */ json_t *lookup_get_value (lookup_t *lh); -/* Get missing ref after a lookup stall, missing reference can then be - * used to load reference into the KVS cache +/* On lookup stall, get missing reference that should be loaded into + * the KVS cache via callback function. * - * If the missing references points to raw data, 'ref_raw' will be set - * to true, otherwise false. + * return -1 in callback to break iteration */ -const char *lookup_get_missing_ref (lookup_t *lh, bool *ref_raw); +int lookup_iter_missing_refs (lookup_t *lh, lookup_ref_f cb, void *data); /* Convenience function to get cache from earlier instantiation. * Convenient if replaying RPC and don't have it presently. diff --git a/src/modules/kvs/test/lookup.c b/src/modules/kvs/test/lookup.c index 25ad6ce6c6c0..0bedac915497 100644 --- a/src/modules/kvs/test/lookup.c +++ b/src/modules/kvs/test/lookup.c @@ -2,6 +2,7 @@ #include "config.h" #endif #include +#include #include #include "src/common/libtap/tap.h" @@ -12,6 +13,37 @@ #include "src/modules/kvs/types.h" #include "src/common/libutil/blobref.h" +struct lookup_ref_data +{ + const char *ref; + bool raw_data; + int count; +}; + +int lookup_ref (lookup_t *c, + const char *ref, + bool raw_data, + void *data) +{ + struct lookup_ref_data *ld = data; + if (ld) { + ld->ref = ref; + ld->raw_data = raw_data; + ld->count++; + } + return 0; +} + +int lookup_ref_error (lookup_t *c, + const char *ref, + bool raw_data, + void *data) +{ + /* pick a weird errno */ + errno = EMLINK; + return -1; +} + void basic_api (void) { struct cache *cache; @@ -59,6 +91,12 @@ void basic_api (void) "lookup_set_aux_data works"); ok (lookup_get_aux_data (lh) == lh, "lookup_get_aux_data returns works"); + ok (lookup_get_aux_errnum (lh) == 0, + "lookup_get_aux_errnum returns no error"); + ok (lookup_set_aux_errnum (lh, EINVAL) == EINVAL, + "lookup_set_aux_errnum works"); + ok (lookup_get_aux_errnum (lh) == EINVAL, + "lookup_get_aux_errnum gets EINVAL"); lookup_destroy (lh); @@ -116,8 +154,8 @@ void basic_api_errors (void) "lookup_get_errnum returns EINVAL b/c lookup not yet started"); ok (lookup_get_value (lh) == NULL, "lookup_get_value fails b/c lookup not yet started"); - ok (lookup_get_missing_ref (lh, NULL) == NULL, - "lookup_get_missing_ref fails b/c lookup not yet started"); + ok (lookup_iter_missing_refs (lh, lookup_ref, NULL) < 0, + "lookup_iter_missing_refs fails b/c lookup not yet started"); ok (lookup_validate (NULL) == false, "lookup_validate fails on NULL pointer"); @@ -127,8 +165,8 @@ void basic_api_errors (void) "lookup_get_errnum returns EINVAL on NULL pointer"); ok (lookup_get_value (NULL) == NULL, "lookup_get_value fails on NULL pointer"); - ok (lookup_get_missing_ref (NULL, NULL) == NULL, - "lookup_get_missing_ref fails on NULL pointer"); + ok (lookup_iter_missing_refs (NULL, lookup_ref, NULL) < 0, + "lookup_iter_missing_refs fails on NULL pointer"); ok (lookup_get_cache (NULL) == NULL, "lookup_get_cache fails on NULL pointer"); ok (lookup_get_current_epoch (NULL) < 0, @@ -162,8 +200,8 @@ void basic_api_errors (void) "lookup_get_errnum returns EINVAL on bad pointer"); ok (lookup_get_value (lh) == NULL, "lookup_get_value fails on bad pointer"); - ok (lookup_get_missing_ref (lh, NULL) == NULL, - "lookup_get_missing_ref fails on bad pointer"); + ok (lookup_iter_missing_refs (lh, lookup_ref, NULL) < 0, + "lookup_iter_missing_refs fails on bad pointer"); ok (lookup_get_cache (lh) == NULL, "lookup_get_cache fails on bad pointer"); ok (lookup_get_current_epoch (lh) < 0, @@ -192,12 +230,14 @@ void check_common (lookup_t *lh, bool lookup_result, int get_errnum_result, json_t *get_value_result, + int missing_ref_count, const char *missing_ref_result, bool missing_ref_raw, const char *msg, bool destroy_lookup) { json_t *val; + struct lookup_ref_data ld = { .ref = NULL, .raw_data = false, .count = 0 }; ok (lookup (lh) == lookup_result, "%s: lookup matched result", msg); @@ -220,29 +260,42 @@ void check_common (lookup_t *lh, "%s: lookup_get_value returns NULL as expected", msg); json_decref (val); /* just in case error */ } - if (missing_ref_result) { - const char *missing_ref; - bool ref_raw; - - ok ((missing_ref = lookup_get_missing_ref (lh, &ref_raw)) != NULL, - "%s: lookup_get_missing_ref returns expected non-NULL result", msg); - - if (missing_ref) { - ok (strcmp (missing_ref_result, missing_ref) == 0, - "%s: missing ref returned matched expectation", msg); + if (missing_ref_count == 1) { + if (missing_ref_result) { + ok (lookup_iter_missing_refs (lh, lookup_ref, &ld) == 0, + "%s: lookup_iter_missing_refs success", msg); + + ok (ld.count == missing_ref_count, + "%s: missing ref returned one missing refs", msg); + + if (ld.ref) { + ok (strcmp (ld.ref, missing_ref_result) == 0, + "%s: missing ref returned matched expectation", msg); + } + else { + ok (false, "%s: missing ref returned matched expectation", msg); + } + + ok (ld.raw_data == missing_ref_raw, + "%s: missing ref raw_data bool returned expected value", msg); } else { - ok (false, "%s: missing ref returned matched expectation", msg); + ok (lookup_iter_missing_refs (lh, lookup_ref, &ld) < 0, + "%s: lookup_iter_missing_refs fails as expected", msg); } - - ok (ref_raw == missing_ref_raw, - "%s: lookup_get_missing_ref returned expected ref_raw", msg); } else { - ok (lookup_get_missing_ref (lh, NULL) == NULL, - "%s: lookup_get_missing_ref returns NULL as expected", msg); + /* if missing_ref is > 1, we only care about number of missing refs, + * as order isn't important. + */ + ok (lookup_iter_missing_refs (lh, lookup_ref, &ld) == 0, + "%s: lookup_iter_missing_refs fails as expected", msg); + + ok (ld.count == missing_ref_count, + "%s: missing ref returned number of expected missing refs", msg); } + if (destroy_lookup) lookup_destroy (lh); } @@ -256,6 +309,7 @@ void check (lookup_t *lh, true, get_errnum_result, get_value_result, + 1, NULL, false, /* doesn't matter */ msg, @@ -264,6 +318,7 @@ void check (lookup_t *lh, void check_stall (lookup_t *lh, int get_errnum_result, + int missing_ref_count, const char *missing_ref_result, bool missing_ref_raw, const char *msg) @@ -272,6 +327,7 @@ void check_stall (lookup_t *lh, false, get_errnum_result, NULL, + missing_ref_count, missing_ref_result, missing_ref_raw, msg, @@ -360,10 +416,12 @@ void lookup_basic (void) { json_t *root; json_t *dirref; json_t *dir; + json_t *valref_multi; json_t *test; struct cache *cache; lookup_t *lh; href_t valref_ref; + href_t valref2_ref; href_t dirref_ref; href_t root_ref; @@ -375,8 +433,12 @@ void lookup_basic (void) { * valref_ref * "abcd" * + * valref2_ref + * "efgh" + * * dirref_ref * "valref" : valref to valref_ref + * "valref_multi" : valref to [ valref_ref, valref2_ref ] * "val" : val to "foo" * "dir" : dir w/ "val" : val to "bar" * "symlink" : symlink to "baz" @@ -388,6 +450,9 @@ void lookup_basic (void) { blobref_hash ("sha1", "abcd", 4, valref_ref, sizeof (href_t)); cache_insert (cache, valref_ref, cache_entry_create_raw (strdup ("abcd"), 4)); + blobref_hash ("sha1", "efgh", 4, valref2_ref, sizeof (href_t)); + cache_insert (cache, valref2_ref, cache_entry_create_raw (strdup ("efgh"), 4)); + dir = treeobj_create_dir (); treeobj_insert_entry (dir, "val", treeobj_create_val ("bar", 3)); @@ -397,6 +462,11 @@ void lookup_basic (void) { treeobj_insert_entry (dirref, "dir", dir); treeobj_insert_entry (dirref, "symlink", treeobj_create_symlink ("baz")); + valref_multi = treeobj_create_valref (valref_ref); + treeobj_append_blobref (valref_multi, valref2_ref); + + treeobj_insert_entry (dirref, "valref_multi", valref_multi); + kvs_util_json_hash ("sha1", dirref, dirref_ref); cache_insert (cache, dirref_ref, cache_entry_create_json (dirref)); @@ -430,6 +500,19 @@ void lookup_basic (void) { check (lh, 0, test, "lookup dirref.valref"); json_decref (test); + /* Lookup value via valref with multiple blobrefs */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "dirref.valref_multi", + NULL, + 0)) != NULL, + "lookup_create on valref_multi"); + test = treeobj_create_val ("abcdefgh", 8); + check (lh, 0, test, "lookup valref_multi"); + json_decref (test); + /* lookup value via val */ ok ((lh = lookup_create (cache, 1, @@ -538,12 +621,14 @@ void lookup_errors (void) { json_t *root; json_t *dirref; json_t *dir; - json_t *valref_multi; json_t *dirref_multi; + json_t *valref_multi_bad; + json_t *valref_multi_overflow; struct cache *cache; lookup_t *lh; href_t dirref_ref; href_t valref_ref; + href_t valref_long_ref; href_t root_ref; ok ((cache = cache_create ()) != NULL, @@ -554,6 +639,9 @@ void lookup_errors (void) { * valref_ref * "abcd" * + * valref_long_ref + * "long", but invalid length on buffer + * * dirref_ref * "val" : val to "bar" * @@ -567,13 +655,23 @@ void lookup_errors (void) { * "dir" : dir w/ "val" : val to "baz" * "dirref_bad" : dirref to valref_ref * "valref_bad" : valref to dirref_ref + * "valref_multi_bad" : [ valref_ref, dirref_ref ] + * "valref_multi_overflow" : [ valref_long_ref, valref_long_ref ] * "dirref_multi" : dirref to [ dirref_ref, dirref_ref ] - * "valref_multi" : valref to [ valref_ref, valref_ref ] */ blobref_hash ("sha1", "abcd", 4, valref_ref, sizeof (href_t)); cache_insert (cache, valref_ref, cache_entry_create_raw (strdup ("abcd"), 4)); + /* achu: Note that I am abusing internal knowledge that cache + * entries blindly store pointers and lengths. Obviously the + * "real" length of the buffer below is length 4, but I'm storing + * a huge number. + */ + blobref_hash ("sha1", "long", 4, valref_long_ref, sizeof (href_t)); + cache_insert (cache, valref_long_ref, + cache_entry_create_raw (strdup ("long"), INT_MAX - 32)); + dirref = treeobj_create_dir (); treeobj_insert_entry (dirref, "val", treeobj_create_val ("bar", 3)); kvs_util_json_hash ("sha1", dirref, dirref_ref); @@ -593,14 +691,18 @@ void lookup_errors (void) { treeobj_insert_entry (root, "dirref_bad", treeobj_create_dirref (valref_ref)); treeobj_insert_entry (root, "valref_bad", treeobj_create_valref (dirref_ref)); - valref_multi = treeobj_create_valref (valref_ref); - treeobj_append_blobref (valref_multi, valref_ref); + valref_multi_bad = treeobj_create_valref (valref_ref); + treeobj_append_blobref (valref_multi_bad, dirref_ref); + treeobj_insert_entry (root, "valref_multi_bad", valref_multi_bad); + + valref_multi_overflow = treeobj_create_valref (valref_long_ref); + treeobj_append_blobref (valref_multi_overflow, valref_long_ref); + treeobj_insert_entry (root, "valref_multi_overflow", valref_multi_overflow); dirref_multi = treeobj_create_dirref (dirref_ref); treeobj_append_blobref (dirref_multi, dirref_ref); treeobj_insert_entry (root, "dirref_multi", dirref_multi); - treeobj_insert_entry (root, "valref_multi", valref_multi); kvs_util_json_hash ("sha1", root, root_ref); cache_insert (cache, root_ref, cache_entry_create_json (root)); @@ -795,6 +897,31 @@ void lookup_errors (void) { "lookup_create on valref_bad"); check (lh, EPERM, NULL, "lookup valref_bad"); + /* Lookup a valref multiple blobref that doesn't point to raw + * data, should get EPERM */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "valref_multi_bad", + NULL, + 0)) != NULL, + "lookup_create on valref_multi_bad"); + check (lh, EPERM, NULL, "lookup valref_multi_bad"); + + /* Lookup a valref multiple blobref that points to buffers that will + * over int, should get EOVERFLOW. + */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "valref_multi_overflow", + NULL, + 0)) != NULL, + "lookup_create on valref_multi_overflow"); + check (lh, EOVERFLOW, NULL, "lookup valref_multi_overflow"); + /* Lookup with an invalid root_ref, should get EINVAL */ ok ((lh = lookup_create (cache, 1, @@ -829,30 +956,6 @@ void lookup_errors (void) { "lookup_create on dirref_multi, part of path"); check (lh, EPERM, NULL, "lookup dirref_multi, part of path"); - /* Lookup valref with multiple blobrefs, should get EPERM */ - ok ((lh = lookup_create (cache, - 1, - root_ref, - root_ref, - "valref_multi", - NULL, - 0)) != NULL, - "lookup_create on valref_multi"); - check (lh, EPERM, NULL, "lookup valref_multi"); - - /* Lookup path w/ valref w/ multiple blobrefs in middle, Not - * ENOENT - caller of lookup decides what to do with entry not - * found */ - ok ((lh = lookup_create (cache, - 1, - root_ref, - root_ref, - "valref_multi.foo", - NULL, - 0)) != NULL, - "lookup_create on valref_multi, part of path"); - check (lh, 0, NULL, "lookup valref_multi, part of path"); - cache_destroy (cache); } @@ -1178,7 +1281,7 @@ void lookup_stall_root (void) { NULL, FLUX_KVS_READDIR)) != NULL, "lookup_create stalltest \".\""); - check_stall (lh, EAGAIN, root_ref, false, "root \".\" stall"); + check_stall (lh, EAGAIN, 1, root_ref, false, "root \".\" stall"); cache_insert (cache, root_ref, cache_entry_create_json (root)); @@ -1202,12 +1305,18 @@ void lookup_stall_root (void) { /* lookup stall tests */ void lookup_stall (void) { json_t *root; + json_t *valref_tmp; json_t *dirref1; json_t *dirref2; json_t *test; struct cache *cache; lookup_t *lh; - href_t valref_ref; + href_t valref1_ref; + href_t valref2_ref; + href_t valref3_ref; + href_t valref4_ref; + href_t valrefmisc1_ref; + href_t valrefmisc2_ref; href_t dirref1_ref; href_t dirref2_ref; href_t root_ref; @@ -1217,12 +1326,29 @@ void lookup_stall (void) { /* This cache is * - * valref_ref + * valref1_ref * "abcd" * + * valref2_ref + * "efgh" + * + * valref3_ref + * "ijkl" + * + * valref4_ref + * "mnop" + * + * valrefmisc1_ref + * "foobar" + * + * valrefmisc2_ref + * "foobaz" + * * dirref1_ref * "val" : val to "foo" * "valref" : valref to valref_ref + * "valrefmisc" : valref to valrefmisc1_ref + * "valrefmisc_multi" : valref to [ valrefmisc1_ref, valrefmisc2_ref ] * * dirref2_ref * "val" : val to "bar" @@ -1234,11 +1360,27 @@ void lookup_stall (void) { * */ - blobref_hash ("sha1", "abcd", 4, valref_ref, sizeof (href_t)); + blobref_hash ("sha1", "abcd", 4, valref1_ref, sizeof (href_t)); + blobref_hash ("sha1", "efgh", 4, valref2_ref, sizeof (href_t)); + blobref_hash ("sha1", "ijkl", 4, valref3_ref, sizeof (href_t)); + blobref_hash ("sha1", "mnop", 4, valref4_ref, sizeof (href_t)); + blobref_hash ("sha1", "foobar", 4, valrefmisc1_ref, sizeof (href_t)); + blobref_hash ("sha1", "foobaz", 4, valrefmisc2_ref, sizeof (href_t)); dirref1 = treeobj_create_dir (); treeobj_insert_entry (dirref1, "val", treeobj_create_val ("foo", 3)); - treeobj_insert_entry (dirref1, "valref", treeobj_create_valref (valref_ref)); + treeobj_insert_entry (dirref1, "valref", treeobj_create_valref (valref1_ref)); + valref_tmp = treeobj_create_valref (valref1_ref); + treeobj_append_blobref (valref_tmp, valref2_ref); + treeobj_insert_entry (dirref1, "valref_multi", valref_tmp); + valref_tmp = treeobj_create_valref (valref3_ref); + treeobj_append_blobref (valref_tmp, valref4_ref); + treeobj_insert_entry (dirref1, "valref_multi2", valref_tmp); + treeobj_insert_entry (dirref1, "valrefmisc", treeobj_create_valref (valrefmisc1_ref)); + valref_tmp = treeobj_create_valref (valrefmisc1_ref); + treeobj_append_blobref (valref_tmp, valrefmisc2_ref); + treeobj_insert_entry (dirref1, "valrefmisc_multi", valref_tmp); + kvs_util_json_hash ("sha1", dirref1, dirref1_ref); dirref2 = treeobj_create_dir (); @@ -1262,12 +1404,12 @@ void lookup_stall (void) { NULL, 0)) != NULL, "lookup_create stalltest dirref1.val"); - check_stall (lh, EAGAIN, root_ref, false, "dirref1.val stall #1"); + check_stall (lh, EAGAIN, 1, root_ref, false, "dirref1.val stall #1"); cache_insert (cache, root_ref, cache_entry_create_json (root)); /* next call to lookup, should stall */ - check_stall (lh, EAGAIN, dirref1_ref, false, "dirref1.val stall #2"); + check_stall (lh, EAGAIN, 1, dirref1_ref, false, "dirref1.val stall #2"); cache_insert (cache, dirref1_ref, cache_entry_create_json (dirref1)); @@ -1298,7 +1440,7 @@ void lookup_stall (void) { NULL, 0)) != NULL, "lookup_create stalltest symlink.val"); - check_stall (lh, EAGAIN, dirref2_ref, false, "symlink.val stall"); + check_stall (lh, EAGAIN, 1, dirref2_ref, false, "symlink.val stall"); cache_insert (cache, dirref2_ref, cache_entry_create_json (dirref2)); @@ -1329,9 +1471,9 @@ void lookup_stall (void) { NULL, 0)) != NULL, "lookup_create stalltest dirref1.valref"); - check_stall (lh, EAGAIN, valref_ref, true, "dirref1.valref stall"); + check_stall (lh, EAGAIN, 1, valref1_ref, true, "dirref1.valref stall"); - cache_insert (cache, valref_ref, cache_entry_create_raw (strdup ("abcd"), 4)); + cache_insert (cache, valref1_ref, cache_entry_create_raw (strdup ("abcd"), 4)); /* lookup dirref1.valref, should succeed */ test = treeobj_create_val ("abcd", 4); @@ -1351,6 +1493,110 @@ void lookup_stall (void) { check (lh, 0, test, "dirref1.valref #2"); json_decref (test); + /* lookup dirref1.valref_multi, should stall */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "dirref1.valref_multi", + NULL, + 0)) != NULL, + "lookup_create stalltest dirref1.valref_multi"); + /* should only be one missing ref, as we loaded one of the refs in + * the 'valref' above */ + check_stall (lh, EAGAIN, 1, valref2_ref, true, "dirref1.valref_multi stall"); + + cache_insert (cache, valref2_ref, cache_entry_create_raw (strdup ("efgh"), 4)); + + /* lookup dirref1.valref_multi, should succeed */ + test = treeobj_create_val ("abcdefgh", 8); + check (lh, 0, test, "dirref1.valref_multi #1"); + json_decref (test); + + /* lookup dirref1.valref_multi, now fully cached, should succeed */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "dirref1.valref_multi", + NULL, + 0)) != NULL, + "lookup_create stalltest dirref1.valref"); + test = treeobj_create_val ("abcdefgh", 8); + check (lh, 0, test, "dirref1.valref_multi #2"); + json_decref (test); + + /* lookup dirref1.valref_multi2, should stall */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "dirref1.valref_multi2", + NULL, + 0)) != NULL, + "lookup_create stalltest dirref1.valref_multi2"); + /* should two missing refs, as we have not loaded either here */ + check_stall (lh, EAGAIN, 2, NULL, true, "dirref1.valref_multi2 stall"); + + cache_insert (cache, valref3_ref, cache_entry_create_raw (strdup ("ijkl"), 4)); + cache_insert (cache, valref4_ref, cache_entry_create_raw (strdup ("mnop"), 4)); + + /* lookup dirref1.valref_multi2, should succeed */ + test = treeobj_create_val ("ijklmnop", 8); + check (lh, 0, test, "dirref1.valref_multi2 #1"); + json_decref (test); + + /* lookup dirref1.valref_multi2, now fully cached, should succeed */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "dirref1.valref_multi2", + NULL, + 0)) != NULL, + "lookup_create stalltest dirref1.valref"); + test = treeobj_create_val ("ijklmnop", 8); + check (lh, 0, test, "dirref1.valref_multi2 #2"); + json_decref (test); + + /* lookup dirref1.valrefmisc, should stall */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "dirref1.valrefmisc", + NULL, + 0)) != NULL, + "lookup_create stalltest dirref1.valrefmisc"); + /* don't call check_stall, this is primarily to test if callback + * functions returning errors are caught */ + ok (lookup (lh) == false, + "dirref1.valrefmisc: lookup stalled"); + errno = 0; + ok (lookup_iter_missing_refs (lh, lookup_ref_error, NULL) < 0 + && errno == EMLINK, + "dirref1.valrefmisc: error & errno properly returned from callback error"); + lookup_destroy (lh); + + /* lookup dirref1.valrefmisc_multi, should stall */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "dirref1.valrefmisc_multi", + NULL, + 0)) != NULL, + "lookup_create stalltest dirref1.valrefmisc_multi"); + /* don't call check_stall, this is primarily to test if callback + * functions returning errors are caught */ + ok (lookup (lh) == false, + "dirref1.valrefmisc_multi: lookup stalled"); + errno = 0; + ok (lookup_iter_missing_refs (lh, lookup_ref_error, NULL) < 0 + && errno == EMLINK, + "dirref1.valrefmisc_multi: error & errno properly returned from callback error"); + lookup_destroy (lh); + cache_destroy (cache); } diff --git a/t/kvs/basic.c b/t/kvs/basic.c index 18f85b750a4a..d71de1e69765 100644 --- a/t/kvs/basic.c +++ b/t/kvs/basic.c @@ -255,7 +255,7 @@ void cmd_copy_tokvs (flux_t *h, int argc, char **argv) { char *file, *key; int fd, len; - uint8_t *buf; + uint8_t *buf = NULL; flux_kvs_txn_t *txn; flux_future_t *f; @@ -287,7 +287,7 @@ void cmd_copy_fromkvs (flux_t *h, int argc, char **argv) { char *file, *key; int fd, len; - const uint8_t *buf; + const uint8_t *buf = NULL; flux_future_t *f; if (argc != 2) diff --git a/t/t1002-kvs-extra.t b/t/t1002-kvs-extra.t index e7cdfd972aae..d53edbb7a1c9 100755 --- a/t/t1002-kvs-extra.t +++ b/t/t1002-kvs-extra.t @@ -304,6 +304,62 @@ test_expect_success 'kvs: valref that points to zero size content store data can test $(${KVSBASIC} copy-fromkvs $TEST.empty -|wc -c) -eq 0 ' +test_expect_success 'kvs: valref that doesnt point to raw data fails' ' + flux kvs unlink -Rf $TEST && + flux kvs mkdir $TEST.a.b.c && + dirhash=`${KVSBASIC} get-treeobj $TEST.a.b.c | grep -P "sha1-[A-Za-z0-9]+" -o` && + ${KVSBASIC} put-treeobj $TEST.value="{\"data\":[\"${dirhash}\"],\"type\":\"valref\",\"ver\":1}" && + test_must_fail ${KVSBASIC} copy-fromkvs $TEST.value - +' + +# multi-blobref valrefs + +test_expect_success 'kvs: multi blob-ref valref can be read' ' + flux kvs unlink -Rf $TEST && + hashval1=`echo -n "abcd" | flux content store` && + hashval2=`echo -n "efgh" | flux content store` && + ${KVSBASIC} put-treeobj $TEST.multival="{\"data\":[\"${hashval1}\", \"${hashval2}\"],\"type\":\"valref\",\"ver\":1}" && + ${KVSBASIC} copy-fromkvs $TEST.multival - | grep "abcdefgh" && + test $(${KVSBASIC} copy-fromkvs $TEST.multival -|wc -c) -eq 8 +' + +test_expect_success 'kvs: multi blob-ref valref with an empty blobref on left, can be read' ' + flux kvs unlink -Rf $TEST && + hashval1=`flux content store < /dev/null` && + hashval2=`echo -n "abcd" | flux content store` && + ${KVSBASIC} put-treeobj $TEST.multival="{\"data\":[\"${hashval1}\", \"${hashval2}\"],\"type\":\"valref\",\"ver\":1}" && + ${KVSBASIC} copy-fromkvs $TEST.multival - | grep "abcd" && + test $(${KVSBASIC} copy-fromkvs $TEST.multival -|wc -c) -eq 4 +' + +test_expect_success 'kvs: multi blob-ref valref with an empty blobref on right, can be read' ' + flux kvs unlink -Rf $TEST && + hashval1=`echo -n "abcd" | flux content store` && + hashval2=`flux content store < /dev/null` && + ${KVSBASIC} put-treeobj $TEST.multival="{\"data\":[\"${hashval1}\", \"${hashval2}\"],\"type\":\"valref\",\"ver\":1}" && + ${KVSBASIC} copy-fromkvs $TEST.multival - | grep "abcd" && + test $(${KVSBASIC} copy-fromkvs $TEST.multival -|wc -c) -eq 4 +' + +test_expect_success 'kvs: multi blob-ref valref with an empty blobref in middle, can be read' ' + flux kvs unlink -Rf $TEST && + hashval1=`echo -n "abcd" | flux content store` && + hashval2=`flux content store < /dev/null` && + hashval3=`echo -n "efgh" | flux content store` && + ${KVSBASIC} put-treeobj $TEST.multival="{\"data\":[\"${hashval1}\", \"${hashval2}\", \"${hashval3}\"],\"type\":\"valref\",\"ver\":1}" && + ${KVSBASIC} copy-fromkvs $TEST.multival - | grep "abcdefgh" && + test $(${KVSBASIC} copy-fromkvs $TEST.multival -|wc -c) -eq 8 +' + +test_expect_success 'kvs: multi blob-ref valref with a blobref that doesnt point to raw data fails' ' + flux kvs unlink -Rf $TEST && + hashval1=`echo -n "abcd" | flux content store` && + flux kvs mkdir $TEST.a.b.c && + dirhash=`${KVSBASIC} get-treeobj $TEST.a.b.c | grep -P "sha1-[A-Za-z0-9]+" -o` && + ${KVSBASIC} put-treeobj $TEST.multival="{\"data\":[\"${hashval1}\", \"${dirhash}\"],\"type\":\"valref\",\"ver\":1}" && + test_must_fail ${KVSBASIC} copy-fromkvs $TEST.multival - +' + # dtree tests test_expect_success 'kvs: store 16x3 directory tree' '