From 0aed63f0e25e65fe99614a75e67ff7ac59c6ae32 Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Fri, 2 Apr 2021 06:26:12 -0400 Subject: [PATCH] ut/blob: avoid modifying blob state in simultaneous sync test This patch addresses couple issues: 1) Before issuing the md syncs the previous steps in test left blob state in dirty state already. The resize never had a chance to apply. This patch adds a proper md sync and polls for completion. 2) Changing blob state is something that should be done via API. In order for dirty state to apply immidietly set_xattr is now used instead. 3) Verify test state in callbacks to make sure not only the number of completions is correct, but their order. This patch is introduced because of the test originally worked only because of the extent pages always writing out its pages. The second sync always was delayed because of this. Meanwhile that should not be the case, since no MD or EP modification was done. Later in the series Extent Pages are fixed, but this test remained incorrect. Signed-off-by: Tomasz Zawadzki Change-Id: Iac17c27f6ff83f2b79835aa6e48472d5293c44d0 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/7233 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Ben Walker Community-CI: Mellanox Build Bot --- test/unit/lib/blob/blob.c/blob_ut.c | 64 +++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 17 deletions(-) diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index 19eec2106..0ab825779 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c +++ b/test/unit/lib/blob/blob.c/blob_ut.c @@ -6382,6 +6382,42 @@ blob_io_unit_compatiblity(void) g_blobid = 0; } +static void +first_sync_complete(void *cb_arg, int bserrno) +{ + struct spdk_blob *blob = cb_arg; + int rc; + + CU_ASSERT(bserrno == 0); + rc = spdk_blob_set_xattr(blob, "sync", "second", strlen("second") + 1); + CU_ASSERT(rc == 0); + CU_ASSERT(g_bserrno == -1); + + /* Keep g_bserrno at -1, only the + * second sync completion should set it at 0. */ +} + +static void +second_sync_complete(void *cb_arg, int bserrno) +{ + struct spdk_blob *blob = cb_arg; + const void *value; + size_t value_len; + int rc; + + CU_ASSERT(bserrno == 0); + + /* Verify that the first sync completion had a chance to execute */ + rc = spdk_blob_get_xattr_value(blob, "sync", &value, &value_len); + CU_ASSERT(rc == 0); + SPDK_CU_ASSERT_FATAL(value != NULL); + CU_ASSERT(value_len == strlen("second") + 1); + CU_ASSERT_NSTRING_EQUAL_FATAL(value, "second", value_len); + + CU_ASSERT(g_bserrno == -1); + g_bserrno = bserrno; +} + static void blob_simultaneous_operations(void) { @@ -6390,6 +6426,7 @@ blob_simultaneous_operations(void) struct spdk_blob *blob, *snapshot; spdk_blob_id blobid, snapshotid; struct spdk_io_channel *channel; + int rc; channel = spdk_bs_alloc_io_channel(bs); SPDK_CU_ASSERT_FATAL(channel != NULL); @@ -6465,6 +6502,8 @@ blob_simultaneous_operations(void) poll_threads(); CU_ASSERT(blob->locked_operation_in_progress == false); /* Blob resized successfully */ + spdk_blob_sync_md(blob, blob_op_complete, NULL); + poll_threads(); CU_ASSERT(g_bserrno == 0); /* Issue two consecutive blob syncs, neither should fail. @@ -6473,25 +6512,16 @@ blob_simultaneous_operations(void) * since disk I/O is required to complete it. */ g_bserrno = -1; - blob->state = SPDK_BLOB_STATE_DIRTY; - spdk_blob_sync_md(blob, blob_op_complete, NULL); - SPDK_CU_ASSERT_FATAL(g_bserrno == -1); + rc = spdk_blob_set_xattr(blob, "sync", "first", strlen("first") + 1); + CU_ASSERT(rc == 0); + spdk_blob_sync_md(blob, first_sync_complete, blob); + CU_ASSERT(g_bserrno == -1); - blob->state = SPDK_BLOB_STATE_DIRTY; - spdk_blob_sync_md(blob, blob_op_complete, NULL); - SPDK_CU_ASSERT_FATAL(g_bserrno == -1); + spdk_blob_sync_md(blob, second_sync_complete, blob); + CU_ASSERT(g_bserrno == -1); - uint32_t completions = 0; - while (completions < 2) { - SPDK_CU_ASSERT_FATAL(poll_thread_times(0, 1)); - if (g_bserrno == 0) { - g_bserrno = -1; - completions++; - } - /* Never should the g_bserrno be other than -1. - * It would mean that either of syncs failed. */ - SPDK_CU_ASSERT_FATAL(g_bserrno == -1); - } + poll_threads(); + CU_ASSERT(g_bserrno == 0); spdk_bs_free_io_channel(channel); poll_threads();