nvmet: Change error code of discard command if not supported
Subsystems (2)
PATCH 1/1 nvmet: Change error code of discard command if not supported
Execute discard command on block device that doesn't support it should return not supported error.
Returning internal error while using multi-path fails the path.
--- drivers/nvme/target/io-cmd.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/nvme/target/io-cmd.c +7 -2
@@ -105,10 +105,15 @@ static void nvmet_execute_flush(struct nvmet_req *req)static u16 nvmet_discard_range (struct nvmet_ns *ns,struct nvme_dsm_range *range,struct bio **bio) { -if (__blkdev_issue_discard (ns->bdev, +int ret; + + ret =__blkdev_issue_discard (ns->bdev,le64_to_cpu (range->slba) << (ns->blksize_shift -9 ),le32_to_cpu (range->nlb) << (ns->blksize_shift -9 ), -GFP_KERNEL ,0 , bio)) +GFP_KERNEL ,0 , bio); +if (ret == -EOPNOTSUPP ) +return NVME_SC_INVALID_OPCODE |NVME_SC_DNR ; +else if (ret)return NVME_SC_INTERNAL |NVME_SC_DNR ;return 0 ; }
Signature · 1 line
1.8.3.1
Sagi Grimberg
maintainerPATCH 1/1 nvmet: Change error code of discard command if not supported
Quoted 19 lines
Execute discard command on block device that doesn't support it should return not supported error.
Returning internal error while using multi-path fails the path.Reviewed-by Max Gurtovoy <maxg at mellanox.com>Signed-off-by Israel Rukshin <israelr at mellanox.com>--- drivers/nvme/target/io-cmd.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)drivers/nvme/target/io-cmd.c +0
-0@@ -105,10 +105,15 @@ static void nvmet_execute_flush(struct nvmet_req *req)static u16 nvmet_discard_range (struct nvmet_ns *ns,struct nvme_dsm_range *range,struct bio **bio) {
drivers/nvme/target/io-cmd.c static void nvmet_execute_flush(struct nvmet_req *req) · line 115
Quoted 10 lines
drivers/nvme/target/io-cmd.c +6
-2-if (__blkdev_issue_discard (ns->bdev, +int ret; + + ret =__blkdev_issue_discard (ns->bdev,le64_to_cpu (range->slba) << (ns->blksize_shift -9 ),le32_to_cpu (range->nlb) << (ns->blksize_shift -9 ), -GFP_KERNEL ,0 , bio)) +GFP_KERNEL ,0 , bio); +if (ret == -EOPNOTSUPP ) +return NVME_SC_INVALID_OPCODE |NVME_SC_DNR ;
Actually I don't think we are allowed to fail with this status. We claim we support dsm regardless of the underlying backend device (because we can expose different devices in the same subsystem), so we can't fail a valid dsm command on invalid opcode. Either we keep the existing behavior or we simply return success as dsm is a hint anyways...
Quoted 5 lines
drivers/nvme/target/io-cmd.c +1
-0+else if (ret)return NVME_SC_INTERNAL |NVME_SC_DNR ;return 0 ; }
PATCH 1/1 nvmet: Change error code of discard command if not supported
Quoted 28 lines · Sagi Grimberg
Quoted 28 lines
Execute discard command on block device that doesn't support it should return not supported error.
Returning internal error while using multi-path fails the path.Reviewed-by Max Gurtovoy <maxg at mellanox.com>Signed-off-by Israel Rukshin <israelr at mellanox.com>---
?drivers/nvme/target/io-cmd.c| 9 +++++++--
? 1 file changed, 7 insertions(+), 2 deletions(-)drivers/nvme/target/io-cmd.c +4
-2@@ -105,10 +105,15 @@ static void nvmet_execute_flush(struct nvmet_req *req) ? static u16 nvmet_discard_range(struct nvmet_ns *ns, ????????? struct nvme_dsm_range *range, struct bio **bio) ? { -??? if (__blkdev_issue_discard(ns->bdev, +??? int ret; + +??? ret = __blkdev_issue_discard(ns->bdev, ????????????? le64_to_cpu(range->slba) << (ns->blksize_shift - 9), ????????????? le32_to_cpu(range->nlb) << (ns->blksize_shift - 9), -??????????? GFP_KERNEL, 0, bio)) +??????????? GFP_KERNEL, 0, bio);
Quoted 8 lines
drivers/nvme/target/io-cmd.c static void nvmet_execute_flush(struct nvmet_req · line 116
Quoted 2 lines
drivers/nvme/target/io-cmd.c +2
-0+??? if (ret == -EOPNOTSUPP) +??????? return NVME_SC_INVALID_OPCODE | NVME_SC_DNR;Actually I don't think we are allowed to fail with this status. We claim we support dsm regardless of the underlying backend device (because we can expose different devices in the same subsystem), so we can't fail a valid dsm command on invalid opcode. Either we keep the existing behavior or we simply return success as dsm is a hint anyways...
we can't fail a path in case the backend device doesn't support dsm. The avarage user that will try to create a fs using mkfs.ext4, for example, on top of a multipath device, without the appropriate flags will fail. I'm fine with returning successful value, we'll test it and send a V2.
Quoted 6 lines
Quoted 6 lines
drivers/nvme/target/io-cmd.c +1
-0+??? else if (ret) ????????? return NVME_SC_INTERNAL | NVME_SC_DNR; ????? return 0;? }
Christoph Hellwig
maintainerPATCH 1/1 nvmet: Change error code of discard command if not supported
In reply to Sagi GrimbergQuoted 5 lines · Sagi Grimberg
Actually I don't think we are allowed to fail with this status. We claim we support dsm regardless of the underlying backend device (because we can expose different devices in the same subsystem), so we can't fail a valid dsm command on invalid opcode. Either we keep the existing behavior or we simply return success as dsm is a hint anyways...
Yes, we should return success here.