ivampiresp/Spdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

bdev: remove vbdevs and base_bdevs arrays

Browse Source 
The intents of these arrays was to keep track in the
bdev layer of all base<->virtual bdev relationships -
i.e. which member disk bdevs make up a RAID bdev,
which logical volume bdevs are associated with a
bdev that contains an lvolstore, etc.

Currently none of this is used however.  And trying
to keep track in the bdev layer instead of asking
the bdev modules for the relationships has a number
of complications.  Early one, we tried to do this
with TAILQs - but that doesn't work since this can't
be done with a single TAILQ_ENTRY in the bdev
structures.  So we moved to arrays - that works a bit
better, but then the pointer arrays have to be
realloc'd which isn't ideal.

The biggest problem though with these arrays is that
they held bdev pointers - not bdev descriptor pointers.
It's not really valid to access bdevs without a
descriptor - the descriptors are what make sure active
references are accounted for when a bdev is hotplugged.
Of course the bdev layer knows when a bdev is getting
removed and could go and do the updates to these
arrays separately - but that just seems very convoluted.

So for now just remove these arrays completely.  If
there is a future need for the bdev layer to
understand relationships between bdevs, we can add
module APIs so that the generic layer can ask
the modules about the relationships.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I99ef1068240bff1262f64f234260cf2fb44df51d
Reviewed-on: https://review.gerrithub.io/420932
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: GangCao <gang.cao@intel.com>
This commit is contained in:
Jim Harris 2018-07-31 13:20:38 -07:00
parent 43c8d0eb1e
commit 671b77e5cd
4 changed files with 0 additions and 191 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
include/spdk/bdev_module.h
View File

@ -271,10 +271,6 @@ struct spdk_bdev {
/** function table for all LUN ops */
const struct spdk_bdev_fn_table *fn_table;
/** The array of virtual block devices built on top of this block device. */
struct spdk_bdev **vbdevs;
size_t vbdevs_cnt;
/** Fields that are used internally by the bdev subsystem. Bdev modules
* must not read or write to these fields.
*/
@ -291,10 +287,6 @@ struct spdk_bdev {
/** The bdev status */
enum spdk_bdev_status status;
/** The array of block devices that this block device is built on top of (if any). */
struct spdk_bdev **base_bdevs;
size_t base_bdevs_cnt;
/**
* Pointer to the module that has claimed this bdev for purposes of creating virtual
* bdevs on top of it. Set to NULL if the bdev has not been claimed.

102
lib/bdev/bdev.c
View File

@ -2926,94 +2926,6 @@ spdk_bdev_register(struct spdk_bdev *bdev)
return rc;
}
static void
spdk_vbdev_remove_base_bdevs(struct spdk_bdev *vbdev)
{
struct spdk_bdev **bdevs;
struct spdk_bdev *base;
size_t i, j, k;
bool found;
/* Iterate over base bdevs to remove vbdev from them. */
for (i = 0; i < vbdev->internal.base_bdevs_cnt; i++) {
found = false;
base = vbdev->internal.base_bdevs[i];
for (j = 0; j < base->vbdevs_cnt; j++) {
if (base->vbdevs[j] != vbdev) {
continue;
}
for (k = j; k + 1 < base->vbdevs_cnt; k++) {
base->vbdevs[k] = base->vbdevs[k + 1];
}
base->vbdevs_cnt--;
if (base->vbdevs_cnt > 0) {
bdevs = realloc(base->vbdevs, base->vbdevs_cnt * sizeof(bdevs[0]));
/* It would be odd if shrinking memory block fail. */
assert(bdevs);
base->vbdevs = bdevs;
} else {
free(base->vbdevs);
base->vbdevs = NULL;
}
found = true;
break;
}
if (!found) {
SPDK_WARNLOG("Bdev '%s' is not base bdev of '%s'.\n", base->name, vbdev->name);
}
}
free(vbdev->internal.base_bdevs);
vbdev->internal.base_bdevs = NULL;
vbdev->internal.base_bdevs_cnt = 0;
}
static int
spdk_vbdev_set_base_bdevs(struct spdk_bdev *vbdev, struct spdk_bdev **base_bdevs, size_t cnt)
{
struct spdk_bdev **vbdevs;
struct spdk_bdev *base;
size_t i;
/* Adding base bdevs isn't supported (yet?). */
assert(vbdev->internal.base_bdevs_cnt == 0);
vbdev->internal.base_bdevs = malloc(cnt * sizeof(vbdev->internal.base_bdevs[0]));
if (!vbdev->internal.base_bdevs) {
SPDK_ERRLOG("%s - realloc() failed\n", vbdev->name);
return -ENOMEM;
}
memcpy(vbdev->internal.base_bdevs, base_bdevs, cnt * sizeof(vbdev->internal.base_bdevs[0]));
vbdev->internal.base_bdevs_cnt = cnt;
/* Iterate over base bdevs to add this vbdev to them. */
for (i = 0; i < cnt; i++) {
base = vbdev->internal.base_bdevs[i];
assert(base != NULL);
assert(base->internal.claim_module != NULL);
vbdevs = realloc(base->vbdevs, (base->vbdevs_cnt + 1) * sizeof(vbdevs[0]));
if (!vbdevs) {
SPDK_ERRLOG("%s - realloc() failed\n", base->name);
spdk_vbdev_remove_base_bdevs(vbdev);
return -ENOMEM;
}
vbdevs[base->vbdevs_cnt] = vbdev;
base->vbdevs = vbdevs;
base->vbdevs_cnt++;
}
return 0;
}
int
spdk_vbdev_register(struct spdk_bdev *vbdev, struct spdk_bdev **base_bdevs, int base_bdev_count)
{
@ -3024,22 +2936,10 @@ spdk_vbdev_register(struct spdk_bdev *vbdev, struct spdk_bdev **base_bdevs, int
return rc;
}
if (base_bdev_count == 0) {
spdk_bdev_start(vbdev);
return 0;
}
rc = spdk_vbdev_set_base_bdevs(vbdev, base_bdevs, base_bdev_count);
if (rc) {
spdk_bdev_fini(vbdev);
return rc;
}
spdk_bdev_start(vbdev);
return 0;
}
void
spdk_bdev_destruct_done(struct spdk_bdev *bdev, int bdeverrno)
{
@ -3076,8 +2976,6 @@ spdk_bdev_unregister(struct spdk_bdev *bdev, spdk_bdev_unregister_cb cb_fn, void
pthread_mutex_lock(&bdev->internal.mutex);
spdk_vbdev_remove_base_bdevs(bdev);
bdev->internal.status = SPDK_BDEV_STATUS_REMOVING;
bdev->internal.unregister_cb = cb_fn;
bdev->internal.unregister_ctx = cb_arg;

75
test/unit/lib/bdev/bdev.c/bdev_ut.c
View File

@ -195,45 +195,6 @@ vbdev_ut_examine(struct spdk_bdev *bdev)
spdk_bdev_module_examine_done(&vbdev_ut_if);
}
static bool
is_vbdev(struct spdk_bdev *base, struct spdk_bdev *vbdev)
{
size_t i;
int found = 0;
for (i = 0; i < base->vbdevs_cnt; i++) {
found += base->vbdevs[i] == vbdev;
}
CU_ASSERT(found <= 1);
return !!found;
}
static bool
is_base_bdev(struct spdk_bdev *base, struct spdk_bdev *vbdev)
{
size_t i;
int found = 0;
for (i = 0; i < vbdev->internal.base_bdevs_cnt; i++) {
found += vbdev->internal.base_bdevs[i] == base;
}
CU_ASSERT(found <= 1);
return !!found;
}
static bool
check_base_and_vbdev(struct spdk_bdev *base, struct spdk_bdev *vbdev)
{
bool _is_vbdev = is_vbdev(base, vbdev);
bool _is_base = is_base_bdev(base, vbdev);
CU_ASSERT(_is_vbdev == _is_base);
return _is_base && _is_vbdev;
}
static struct spdk_bdev *
allocate_bdev(char *name)
{
@ -250,8 +211,6 @@ allocate_bdev(char *name)
rc = spdk_bdev_register(bdev);
CU_ASSERT(rc == 0);
CU_ASSERT(bdev->internal.base_bdevs_cnt == 0);
CU_ASSERT(bdev->vbdevs_cnt == 0);
return bdev;
}
@ -278,14 +237,6 @@ allocate_vbdev(char *name, struct spdk_bdev *base1, struct spdk_bdev *base2)
rc = spdk_vbdev_register(bdev, array, base2 == NULL ? 1 : 2);
CU_ASSERT(rc == 0);
CU_ASSERT(bdev->internal.base_bdevs_cnt > 0);
CU_ASSERT(bdev->vbdevs_cnt == 0);
CU_ASSERT(check_base_and_vbdev(base1, bdev) == true);
if (base2) {
CU_ASSERT(check_base_and_vbdev(base2, bdev) == true);
}
return bdev;
}
@ -301,7 +252,6 @@ free_bdev(struct spdk_bdev *bdev)
static void
free_vbdev(struct spdk_bdev *bdev)
{
CU_ASSERT(bdev->internal.base_bdevs_cnt != 0);
spdk_bdev_unregister(bdev, NULL, NULL);
memset(bdev, 0xFF, sizeof(*bdev));
free(bdev);
@ -404,31 +354,6 @@ open_write_test(void)
bdev[8] = allocate_vbdev("bdev8", bdev[4], bdev[5]);
/* Check tree */
CU_ASSERT(check_base_and_vbdev(bdev[0], bdev[4]) == true);
CU_ASSERT(check_base_and_vbdev(bdev[0], bdev[5]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[0], bdev[6]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[0], bdev[7]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[0], bdev[8]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[1], bdev[4]) == true);
CU_ASSERT(check_base_and_vbdev(bdev[1], bdev[5]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[1], bdev[6]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[1], bdev[7]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[1], bdev[8]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[2], bdev[4]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[2], bdev[5]) == true);
CU_ASSERT(check_base_and_vbdev(bdev[2], bdev[6]) == true);
CU_ASSERT(check_base_and_vbdev(bdev[2], bdev[7]) == true);
CU_ASSERT(check_base_and_vbdev(bdev[2], bdev[8]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[3], bdev[4]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[3], bdev[5]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[3], bdev[6]) == false);
CU_ASSERT(check_base_and_vbdev(bdev[3], bdev[7]) == true);
CU_ASSERT(check_base_and_vbdev(bdev[3], bdev[8]) == false);
/* Open bdev0 read-only. This should succeed. */
rc = spdk_bdev_open(bdev[0], false, NULL, NULL, &desc[0]);
CU_ASSERT(rc == 0);

6
test/unit/lib/bdev/part.c/part_ut.c
View File

@ -119,12 +119,6 @@ part_test(void)
spdk_bdev_part_base_hotremove(&bdev_base, &tailq);
/*
* The base device was removed - ensure that the partition vbdevs were
* removed from the base's vbdev list.
*/
CU_ASSERT(bdev_base.vbdevs_cnt == 0);
spdk_bdev_part_base_free(base);
spdk_bdev_unregister(&bdev_base, NULL, NULL);
}