bdev: add callback to free part_base

This puts responsibility on the caller to free the buffer
and do any other cleanup associated with the partition base
(for example, GPT buffer).

While here, also clean up a bunch of places where on
various failures during initialization, it would just
free the buffer instead of calling spdk_bdev_part_base_free().
The latter is required to make sure the bdev descriptor
gets closed as well.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Ic000339459eca4a4a1d103da2e1f3feffe7e764f
Reviewed-on: https://review.gerrithub.io/378653
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>

include/spdk_internal/bdev.h

@@ -433,11 +433,16 @@ spdk_bdev_io_from_ctx(void *ctx)
 	       ((uintptr_t)ctx - offsetof(struct spdk_bdev_io, driver_ctx));
 }
 
+struct spdk_bdev_part_base;
+
+typedef void (*spdk_bdev_part_base_free_fn)(struct spdk_bdev_part_base *base);
+
 struct spdk_bdev_part_base {
 	struct spdk_bdev		*bdev;
 	struct spdk_bdev_desc		*desc;
 	uint32_t			ref;
 	uint32_t			channel_size;
+	spdk_bdev_part_base_free_fn	base_free_fn;
 	bool				claimed;
 	struct spdk_bdev_module_if	*module;
 	struct spdk_bdev_fn_table	*fn_table;
@@ -469,6 +474,7 @@ int spdk_bdev_part_base_construct(struct spdk_bdev_part_base *base, struct spdk_
 				  struct spdk_bdev_module_if *module,
 				  struct spdk_bdev_fn_table *fn_table,
 				  struct bdev_part_tailq *tailq,
+				  spdk_bdev_part_base_free_fn free_fn,
 				  uint32_t channel_size,
 				  spdk_io_channel_create_cb ch_create_cb,
 				  spdk_io_channel_destroy_cb ch_destroy_cb);

lib/bdev/bdev.c

@@ -1880,10 +1880,11 @@ spdk_bdev_module_list_add(struct spdk_bdev_module_if *bdev_module)
 void
 spdk_bdev_part_base_free(struct spdk_bdev_part_base *base)
 {
-	assert(base->bdev);
+	if (base->desc) {
-	assert(base->desc);
+		spdk_bdev_close(base->desc);
-	spdk_bdev_close(base->desc);
+		base->desc = NULL;
-	free(base);
+	}
+	base->base_free_fn(base);
 }
 
 void
@@ -2043,6 +2044,7 @@ int
 spdk_bdev_part_base_construct(struct spdk_bdev_part_base *base, struct spdk_bdev *bdev,
 			      spdk_bdev_remove_cb_t remove_cb, struct spdk_bdev_module_if *module,
 			      struct spdk_bdev_fn_table *fn_table, struct bdev_part_tailq *tailq,
+			      spdk_bdev_part_base_free_fn free_fn,
 			      uint32_t channel_size, spdk_io_channel_create_cb ch_create_cb,
 			      spdk_io_channel_destroy_cb ch_destroy_cb)
 {
@@ -2052,6 +2054,7 @@ spdk_bdev_part_base_construct(struct spdk_bdev_part_base *base, struct spdk_bdev
 	fn_table->io_type_supported = spdk_bdev_part_io_type_supported;
 
 	base->bdev = bdev;
+	base->desc = NULL;
 	base->ref = 0;
 	base->module = module;
 	base->fn_table = fn_table;
@@ -2060,9 +2063,11 @@ spdk_bdev_part_base_construct(struct spdk_bdev_part_base *base, struct spdk_bdev
 	base->channel_size = channel_size;
 	base->ch_create_cb = ch_create_cb;
 	base->ch_destroy_cb = ch_destroy_cb;
+	base->base_free_fn = free_fn;
 
 	rc = spdk_bdev_open(bdev, false, remove_cb, bdev, &base->desc);
 	if (rc) {
+		spdk_bdev_part_base_free(base);
 		SPDK_ERRLOG("could not open bdev %s\n", spdk_bdev_get_name(bdev));
 		return -1;
 	}

lib/bdev/error/vbdev_error.c

@@ -70,6 +70,12 @@ struct error_channel {
 static pthread_mutex_t g_vbdev_error_mutex = PTHREAD_MUTEX_INITIALIZER;
 static SPDK_BDEV_PART_TAILQ g_error_disks = TAILQ_HEAD_INITIALIZER(g_error_disks);
 
+static void
+spdk_error_free_base(struct spdk_bdev_part_base *base)
+{
+	free(base);
+}
+
 int
 spdk_vbdev_inject_error(char *name, uint32_t io_type, uint32_t error_type, uint32_t error_num)
 {
@@ -223,25 +229,25 @@ spdk_vbdev_error_create(struct spdk_bdev *base_bdev)
 
 	rc = spdk_bdev_part_base_construct(base, base_bdev, NULL,
 					   SPDK_GET_BDEV_MODULE(error), &vbdev_error_fn_table,
-					   &g_error_disks, sizeof(struct error_channel), NULL, NULL);
+					   &g_error_disks, spdk_error_free_base,
+					   sizeof(struct error_channel), NULL, NULL);
 	if (rc) {
 		SPDK_ERRLOG("could not construct part base for bdev %s\n", spdk_bdev_get_name(base_bdev));
-		free(base);
 		return -1;
 	}
 
 	disk = calloc(1, sizeof(*disk));
 	if (!disk) {
 		SPDK_ERRLOG("Memory allocation failure\n");
-		free(base);
+		spdk_error_free_base(base);
 		return -1;
 	}
 
 	name = spdk_sprintf_alloc("EE_%s", spdk_bdev_get_name(base_bdev));
 	if (!name) {
 		SPDK_ERRLOG("name allocation failure\n");
+		spdk_error_free_base(base);
 		free(disk);
-		free(base);
 		return -1;
 	}
 
@@ -250,8 +256,8 @@ spdk_vbdev_error_create(struct spdk_bdev *base_bdev)
 	if (rc) {
 		SPDK_ERRLOG("could not construct part for bdev %s\n", spdk_bdev_get_name(base_bdev));
 		/* spdk_bdev_part_construct will free name on failure */
+		spdk_error_free_base(base);
 		free(disk);
-		free(base);
 		return -1;
 	}
 

lib/bdev/gpt/vbdev_gpt.c

@@ -75,10 +75,12 @@ static SPDK_BDEV_PART_TAILQ g_gpt_disks = TAILQ_HEAD_INITIALIZER(g_gpt_disks);
 static bool g_gpt_disabled;
 
 static void
-spdk_gpt_base_free(struct gpt_base *gpt_base)
+spdk_gpt_base_free(struct spdk_bdev_part_base *base)
 {
+	struct gpt_base *gpt_base = SPDK_CONTAINEROF(base, struct gpt_base, part_base);
+
 	spdk_dma_free(gpt_base->gpt.buf);
-	spdk_bdev_part_base_free(&gpt_base->part_base);
+	free(gpt_base);
 }
 
 static void
@@ -113,11 +115,10 @@ spdk_gpt_base_bdev_init(struct spdk_bdev *bdev)
 	rc = spdk_bdev_part_base_construct(&gpt_base->part_base, bdev,
 					   spdk_gpt_base_bdev_hotremove_cb,
 					   SPDK_GET_BDEV_MODULE(gpt), &vbdev_gpt_fn_table,
-					   &g_gpt_disks, sizeof(struct gpt_channel),
+					   &g_gpt_disks, spdk_gpt_base_free,
-					   NULL, NULL);
+					   sizeof(struct gpt_channel), NULL, NULL);
 	if (rc) {
 		SPDK_ERRLOG("cannot construct gpt_base");
-		free(gpt_base);
 		return NULL;
 	}
 
@@ -125,7 +126,7 @@ spdk_gpt_base_bdev_init(struct spdk_bdev *bdev)
 	gpt->buf = spdk_dma_zmalloc(SPDK_GPT_BUFFER_SIZE, 0x1000, NULL);
 	if (!gpt->buf) {
 		SPDK_ERRLOG("Cannot alloc buf\n");
-		free(gpt_base);
+		spdk_bdev_part_base_free(&gpt_base->part_base);
 		return NULL;
 	}
 
@@ -309,7 +310,7 @@ end:
 
 	if (gpt_base->part_base.ref == 0) {
 		/* If no gpt_disk instances were created, free the base context */
-		spdk_gpt_base_free(gpt_base);
+		spdk_bdev_part_base_free(&gpt_base->part_base);
 	}
 }
 
@@ -331,7 +332,7 @@ vbdev_gpt_read_gpt(struct spdk_bdev *bdev)
 			    SPDK_GPT_BUFFER_SIZE, spdk_gpt_bdev_complete, gpt_base);
 	if (rc < 0) {
 		spdk_put_io_channel(gpt_base->ch);
-		spdk_gpt_base_free(gpt_base);
+		spdk_bdev_part_base_free(&gpt_base->part_base);
 		SPDK_ERRLOG("Failed to send bdev_io command\n");
 		return -1;
 	}

lib/bdev/split/vbdev_split.c

@@ -56,6 +56,12 @@ struct vbdev_split_channel {
 	struct spdk_bdev_part_channel	part_ch;
 };
 
+static void
+vbdev_split_base_free(struct spdk_bdev_part_base *base)
+{
+	free(base);
+}
+
 static int
 vbdev_split_destruct(void *ctx)
 {
@@ -151,11 +157,10 @@ vbdev_split_create(struct spdk_bdev *base_bdev, uint64_t split_count, uint64_t s
 	rc = spdk_bdev_part_base_construct(split_base, base_bdev,
 					   vbdev_split_base_bdev_hotremove_cb,
 					   SPDK_GET_BDEV_MODULE(split), &vbdev_split_fn_table,
-					   &g_split_disks, sizeof(struct vbdev_split_channel),
+					   &g_split_disks, vbdev_split_base_free,
-					   NULL, NULL);
+					   sizeof(struct vbdev_split_channel), NULL, NULL);
 	if (rc) {
 		SPDK_ERRLOG("Cannot construct bdev part base\n");
-		free(split_base);
 		return -1;
 	}