Re: [PATCH] nvmet: return Invalid Log Page status code for unsupported lid
From: Klaus Jensen <hidden>
Date: 2021-09-02 10:01:39
On Sep 2 11:53, amit.engel@dell.com wrote:
quoted hunk ↗ jump to hunk
From: Amit Engel <redacted> In case of an invalid or not supported log page return NVME_SC_INVALID_LOG_PAGE Signed-off-by: Amit Engel <redacted> --- drivers/nvme/target/admin-cmd.c | 2 +- drivers/nvme/target/discovery.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c index aa6d84d8848e..5dcda2c87fdd 100644 --- a/drivers/nvme/target/admin-cmd.c +++ b/drivers/nvme/target/admin-cmd.c@@ -339,7 +339,7 @@ static void nvmet_execute_get_log_page(struct nvmet_req *req) pr_debug("unhandled lid %d on qid %d\n", req->cmd->get_log_page.lid, req->sq->qid); req->error_loc = offsetof(struct nvme_get_log_page_command, lid); - nvmet_req_complete(req, NVME_SC_INVALID_FIELD | NVME_SC_DNR); + nvmet_req_complete(req, NVME_SC_INVALID_LOG_PAGE | NVME_SC_DNR); } static void nvmet_execute_identify_ctrl(struct nvmet_req *req)diff --git a/drivers/nvme/target/discovery.c b/drivers/nvme/target/discovery.c index 7aa62bc6ae84..fdd52dbdd00f 100644 --- a/drivers/nvme/target/discovery.c +++ b/drivers/nvme/target/discovery.c@@ -176,9 +176,11 @@ static void nvmet_execute_disc_get_log_page(struct nvmet_req *req) return; if (req->cmd->get_log_page.lid != NVME_LOG_DISC) { + pr_err("unhandled get log page cmd with lid %#x from ctrlid %d\n" + req->cmd->get_log_page.lid, ctrl->cntlid); req->error_loc = offsetof(struct nvme_get_log_page_command, lid); - status = NVME_SC_INVALID_FIELD | NVME_SC_DNR; + status = NVME_SC_INVALID_LOG_PAGE | NVME_SC_DNR; goto out; }
This should only be done if the target is reporting v1.4 or above. But honestly I'm not sure if there is any benefit to potentially breaking hosts here. It is perfectly ok for a v1.4 compliant controller to continue using Invalid Field in Command in this case. The spec actually says that the controller *should* abort the command with a status code of Invalid Field in Command. But I acknowledge that v1.4+ specs are a bit contradictory here.