From b9252b1272ca955a404b7c2bd7dd36cd449b44f0 Mon Sep 17 00:00:00 2001 From: Piotr Pelplinski Date: Fri, 22 Dec 2017 09:18:43 +0100 Subject: [PATCH] blob: check if resize failed This patch provides logic for returning errors instead of assert when size is larger than blobstore size. Signed-off-by: Piotr Pelplinski Change-Id: I16d12338e2b682c39bd33d507d57ea126501a0d7 Reviewed-on: https://review.gerrithub.io/392749 Tested-by: SPDK Automated Test System Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- lib/blob/blobstore.c | 31 ++++++++++++++++++----------- test/unit/lib/blob/blob.c/blob_ut.c | 12 +++++++++++ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index a7ced7769..2b7a35aa2 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -997,6 +997,7 @@ _spdk_resize_blob(struct spdk_blob_data *blob, uint64_t sz) uint64_t i; uint64_t *tmp; uint64_t lfc; /* lowest free cluster */ + uint64_t num_clusters; struct spdk_blob_store *bs; bs = blob->bs; @@ -1013,42 +1014,42 @@ _spdk_resize_blob(struct spdk_blob_data *blob, uint64_t sz) * larger without syncing, then the cluster array already * contains spare assigned clusters we can use. */ - blob->active.num_clusters = spdk_min(blob->active.cluster_array_size, - sz); + num_clusters = spdk_min(blob->active.cluster_array_size, + sz); + } else { + num_clusters = blob->active.num_clusters; } - blob->state = SPDK_BLOB_STATE_DIRTY; - /* Do two passes - one to verify that we can obtain enough clusters * and another to actually claim them. */ lfc = 0; - for (i = blob->active.num_clusters; i < sz; i++) { + for (i = num_clusters; i < sz; i++) { lfc = spdk_bit_array_find_first_clear(bs->used_clusters, lfc); if (lfc >= bs->total_clusters) { /* No more free clusters. Cannot satisfy the request */ - assert(false); - return -1; + return -ENOSPC; } lfc++; } - if (sz > blob->active.num_clusters) { + if (sz > num_clusters) { /* Expand the cluster array if necessary. * We only shrink the array when persisting. */ tmp = realloc(blob->active.clusters, sizeof(uint64_t) * sz); if (sz > 0 && tmp == NULL) { - assert(false); - return -1; + return -ENOMEM; } blob->active.clusters = tmp; blob->active.cluster_array_size = sz; } + blob->state = SPDK_BLOB_STATE_DIRTY; + lfc = 0; - for (i = blob->active.num_clusters; i < sz; i++) { + for (i = num_clusters; i < sz; i++) { lfc = spdk_bit_array_find_first_clear(bs->used_clusters, lfc); SPDK_DEBUGLOG(SPDK_LOG_BLOB, "Claiming cluster %lu for blob %lu\n", lfc, blob->id); _spdk_bs_claim_cluster(bs, lfc); @@ -2792,7 +2793,13 @@ void spdk_bs_create_blob_ext(struct spdk_blob_store *bs, const struct spdk_blob_ if (opts->thin_provision) { _spdk_blob_set_thin_provision(blob); } - spdk_blob_resize(__data_to_blob(blob), opts->num_clusters); + + rc = spdk_blob_resize(__data_to_blob(blob), opts->num_clusters); + if (rc < 0) { + _spdk_blob_free(blob); + cb_fn(cb_arg, 0, rc); + return; + } cpl.type = SPDK_BS_CPL_TYPE_BLOBID; cpl.u.blobid.cb_fn = cb_fn; cpl.u.blobid.cb_arg = cb_arg; diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index 3c0f90bae..50ec3b9ce 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c +++ b/test/unit/lib/blob/blob.c/blob_ut.c @@ -338,6 +338,14 @@ blob_create(void) spdk_blob_close(blob, blob_op_complete, NULL); CU_ASSERT(g_bserrno == 0); + /* Try to create blob with size larger than blobstore */ + + spdk_blob_opts_init(&opts); + opts.num_clusters = bs->total_clusters + 1; + + spdk_bs_create_blob_ext(bs, &opts, blob_op_with_id_complete, NULL); + CU_ASSERT(g_bserrno == -ENOSPC); + spdk_bs_unload(g_bs, bs_op_complete, NULL); CU_ASSERT(g_bserrno == 0); g_bs = NULL; @@ -502,6 +510,10 @@ blob_resize(void) CU_ASSERT(rc == 0); CU_ASSERT((free_clusters - 10) == spdk_bs_free_cluster_count(bs)); + /* Try to resize the blob to size larger than blobstore. */ + rc = spdk_blob_resize(blob, bs->total_clusters + 1); + CU_ASSERT(rc == -ENOSPC); + spdk_blob_close(blob, blob_op_complete, NULL); CU_ASSERT(g_bserrno == 0);