On 15.02.2023 14:36, Verma, Vishal L wrote:
On Wed, 2023-02-15 at 11:49 -0500, Alexander Motin wrote:
There is no NDN_MSFT_CMD_SMART command.  There are 3 relevant ones,
reporting different aspects of the module health.  Define those and
use NDN_MSFT_CMD_NHEALTH, while making the code more universal to
allow use of others later.

Signed-off-by:  Alexander Motin <[email protected]>
---
  ndctl/lib/msft.c | 41 +++++++++++++++++++++++++++++++++--------
  ndctl/lib/msft.h |  8 ++++----
  2 files changed, 37 insertions(+), 12 deletions(-)

Just one question below, the rest looks good. Thanks for the re-spin.
<..>

+
  static int msft_smart_valid(struct ndctl_cmd *cmd)
  {
         if (cmd->type != ND_CMD_CALL ||
-           cmd->size != sizeof(*cmd) + sizeof(struct ndn_pkg_msft) ||

Why is this size check dropped?

Primarily because intel_smart_valid(), which I tried to mimic with this commit, does not check the size. Different commands have different data sizes. With addition of size arguments to alloc_msft_cmd() this check looks more strict, but while no other commands are used now, either way could probably make sense. Any way this check is redundant, and should have been made an assertion to begin with.

             CMD_MSFT(cmd)->gen.nd_family != NVDIMM_FAMILY_MSFT ||
-           CMD_MSFT(cmd)->gen.nd_command != NDN_MSFT_CMD_SMART ||
+           CMD_MSFT(cmd)->gen.nd_command != NDN_MSFT_CMD_NHEALTH ||
             cmd->status != 0)
                 return cmd->status < 0 ? cmd->status : -EINVAL;
         return 0;
@@ -170,6 +194,7 @@ static int msft_cmd_xlat_firmware_status(struct ndctl_cmd 
*cmd)
  }
 struct ndctl_dimm_ops * const msft_dimm_ops = &(struct ndctl_dimm_ops) {
+       .cmd_desc = msft_cmd_desc,
         .new_smart = msft_dimm_cmd_new_smart,
         .smart_get_flags = msft_cmd_smart_get_flags,
         .smart_get_health = msft_cmd_smart_get_health,
diff --git a/ndctl/lib/msft.h b/ndctl/lib/msft.h
index c462612..8d246a5 100644
--- a/ndctl/lib/msft.h
+++ b/ndctl/lib/msft.h
@@ -2,14 +2,14 @@
  /* Copyright (C) 2016-2017 Dell, Inc. */
  /* Copyright (C) 2016 Hewlett Packard Enterprise Development LP */
  /* Copyright (C) 2014-2020, Intel Corporation. */
+/* Copyright (C) 2022 iXsystems, Inc. */
  #ifndef __NDCTL_MSFT_H__
  #define __NDCTL_MSFT_H__
 enum {
-       NDN_MSFT_CMD_QUERY = 0,
-
-       /* non-root commands */
-       NDN_MSFT_CMD_SMART = 11,
+       NDN_MSFT_CMD_CHEALTH = 10,
+       NDN_MSFT_CMD_NHEALTH = 11,
+       NDN_MSFT_CMD_EHEALTH = 12,
  };
 /*


--
Alexander Motin

Reply via email to