From 5dcf922cdacf06a268c4b82a523b1ed2ab130b11 Mon Sep 17 00:00:00 2001 From: Ziye Yang Date: Tue, 13 Sep 2016 10:25:57 +0800 Subject: [PATCH] nvmf: fix tgt subsystem delete related operations. 1 In our nvmf tgt implemention, we use the async mode to delete the nvmf subsystem. However, when we parse nvmf subsystem, we need to use the sync function to delete the nvmf subsystem. Since if there is error, we will call spdk_app_stop, thus async functions will not be executed. It is approved in my local test. 2 Add debug info in spdk_nvmf_delete_subsystem Change-Id: Ia8ecd6eee1bbd25cb3e1ceeb0e2146f3f03be228 Signed-off-by: Ziye Yang --- app/nvmf_tgt/conf.c | 9 --------- app/nvmf_tgt/nvmf_tgt.c | 17 ++++++++++++++++- app/nvmf_tgt/nvmf_tgt.h | 3 --- lib/nvmf/subsystem.c | 8 +++++--- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/app/nvmf_tgt/conf.c b/app/nvmf_tgt/conf.c index 824d04050..f71119112 100644 --- a/app/nvmf_tgt/conf.c +++ b/app/nvmf_tgt/conf.c @@ -388,7 +388,6 @@ spdk_nvmf_parse_subsystem(struct spdk_conf_section *sp) mode = spdk_conf_section_get_val(sp, "Mode"); if (mode == NULL) { - nvmf_tgt_delete_subsystem(app_subsys); SPDK_ERRLOG("No Mode specified for Subsystem %d\n", sp->num); return -1; } @@ -398,7 +397,6 @@ spdk_nvmf_parse_subsystem(struct spdk_conf_section *sp) } else if (strcasecmp(mode, "Virtual") == 0) { subsystem->mode = NVMF_SUBSYSTEM_MODE_VIRTUAL; } else { - nvmf_tgt_delete_subsystem(app_subsys); SPDK_ERRLOG("Invalid Subsystem mode: %s\n", mode); return -1; } @@ -454,7 +452,6 @@ spdk_nvmf_parse_subsystem(struct spdk_conf_section *sp) bdf = spdk_conf_section_get_val(sp, "NVMe"); if (bdf == NULL) { SPDK_ERRLOG("Subsystem %d: missing NVMe directive\n", sp->num); - nvmf_tgt_delete_subsystem(app_subsys); return -1; } @@ -486,18 +483,15 @@ spdk_nvmf_parse_subsystem(struct spdk_conf_section *sp) sn = spdk_conf_section_get_val(sp, "SN"); if (sn == NULL) { SPDK_ERRLOG("Subsystem %d: missing serial number\n", sp->num); - nvmf_tgt_delete_subsystem(app_subsys); return -1; } if (spdk_nvmf_validate_sn(sn) != 0) { - nvmf_tgt_delete_subsystem(app_subsys); return -1; } namespace = spdk_conf_section_get_val(sp, "Namespace"); if (namespace == NULL) { SPDK_ERRLOG("Subsystem %d: missing Namespace directive\n", sp->num); - nvmf_tgt_delete_subsystem(app_subsys); return -1; } @@ -513,19 +507,16 @@ spdk_nvmf_parse_subsystem(struct spdk_conf_section *sp) namespace = spdk_conf_section_get_nmval(sp, "Namespace", i, 0); if (!namespace) { SPDK_ERRLOG("Namespace %d: missing block device\n", i); - nvmf_tgt_delete_subsystem(app_subsys); return -1; } bdev = spdk_bdev_get_by_name(namespace); if (!bdev) { SPDK_ERRLOG("bdev is NULL\n"); - nvmf_tgt_delete_subsystem(app_subsys); return -1; } if (spdk_nvmf_subsystem_add_ns(subsystem, bdev)) { - nvmf_tgt_delete_subsystem(app_subsys); return -1; } diff --git a/app/nvmf_tgt/nvmf_tgt.c b/app/nvmf_tgt/nvmf_tgt.c index 773aa3280..77d1edc34 100644 --- a/app/nvmf_tgt/nvmf_tgt.c +++ b/app/nvmf_tgt/nvmf_tgt.c @@ -93,7 +93,7 @@ subsystem_delete_event(struct spdk_event *event) } } -void +static void nvmf_tgt_delete_subsystem(struct nvmf_tgt_subsystem *app_subsys) { struct spdk_event *event; @@ -216,6 +216,20 @@ nvmf_tgt_create_subsystem(int num, const char *name, enum spdk_nvmf_subtype subt return app_subsys; } +/* This function can only be used before the pollers are started. */ +static void +nvmf_tgt_delete_subsystems(void) +{ + struct nvmf_tgt_subsystem *app_subsys, *tmp; + struct spdk_nvmf_subsystem *subsystem; + + TAILQ_FOREACH_SAFE(app_subsys, &g_subsystems, tailq, tmp) { + subsystem = app_subsys->subsystem; + spdk_nvmf_delete_subsystem(subsystem); + free(app_subsys); + } +} + static void usage(void) { @@ -251,6 +265,7 @@ spdk_nvmf_startup(spdk_event_t event) rc = spdk_nvmf_parse_conf(); if (rc < 0) { + nvmf_tgt_delete_subsystems(); SPDK_ERRLOG("spdk_nvmf_parse_conf() failed\n"); goto initialize_error; } diff --git a/app/nvmf_tgt/nvmf_tgt.h b/app/nvmf_tgt/nvmf_tgt.h index cc4f59960..d5ad86daa 100644 --- a/app/nvmf_tgt/nvmf_tgt.h +++ b/app/nvmf_tgt/nvmf_tgt.h @@ -60,7 +60,4 @@ struct nvmf_tgt_subsystem *nvmf_tgt_create_subsystem(int num, const char *name, enum spdk_nvmf_subtype subtype, uint32_t lcore); - -void nvmf_tgt_delete_subsystem(struct nvmf_tgt_subsystem *app_subsys); - #endif diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index 3dde02e9b..1b0299eff 100644 --- a/lib/nvmf/subsystem.c +++ b/lib/nvmf/subsystem.c @@ -155,9 +155,11 @@ spdk_nvmf_delete_subsystem(struct spdk_nvmf_subsystem *subsystem) struct spdk_nvmf_listen_addr *listen_addr, *listen_addr_tmp; struct spdk_nvmf_host *host, *host_tmp; - /* - * The poller has been unregistered, so now the memory can be freed. - */ + if (!subsystem) { + return; + } + + SPDK_TRACELOG(SPDK_TRACE_NVMF, "subsystem is %p\n", subsystem); TAILQ_FOREACH_SAFE(listen_addr, &subsystem->listen_addrs, link, listen_addr_tmp) { TAILQ_REMOVE(&subsystem->listen_addrs, listen_addr, link);