Re: [PATCH] libnvdimm/bus: return the outvar 'cmd_rc' error code in __nd_ioctl()

On Tue, Feb 18, 2020 at 1:03 PM Dan Williams [off-list ref] wrote:

On Tue, Feb 18, 2020 at 1:00 PM Jeff Moyer [off-list ref] wrote:

quoted

Vaibhav Jain [off-list ref] writes:

quoted

Presently the error code returned via out variable 'cmd_rc' from the
nvdimm-bus controller function is ignored when called from
__nd_ioctl() and never communicated back to user-space code that called
an ioctl on dimm/bus.

This minor patch updates __nd_ioctl() to propagate the value of out
variable 'cmd_rc' back to user-space in case it reports an error.

Signed-off-by: Vaibhav Jain <redacted>
---
 drivers/nvdimm/bus.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index a8b515968569..5b687a27fdf2 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c

@@ -1153,6 +1153,11 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
      if (rc < 0)
              goto out_unlock;

+     if (cmd_rc < 0) {
+             rc = cmd_rc;
+             goto out_unlock;
+     }
+
      if (!nvdimm && cmd == ND_CMD_CLEAR_ERROR && cmd_rc >= 0) {
              struct nd_cmd_clear_error *clear_err = buf;

Looks good to me.

Reviewed-by: Jeff Moyer <redacted>

Applied.

Unapplied. This breaks the NVDIMM unit test, and now that I look
closer you are likely overlooking the fact that cmd_rc is a
translation of the firmware status, while the ioctl rc is whether the
command was successfully submitted. If you want the equivalent of
cmd_rc in userspace you need to translate the firmware status. See
ndctl_cmd_submit_xlat() in libndctl as an example of how the
equivalent of cmd_rc is generated from the firmware status.