DAOS-10250 control: Get enabled and disabled ranks with dmg pool query (

#14436) Allow enabled and disabled ranks option to be used simultaneously (DAOS-10250). Update and add cmocka unit tests of engine management related functions (DAOS-10253). Fix memory leaks of ranks string in function ds_mgmt_drpc_pool_query(). Signed-off-by: Cedric Koch-Hofer <[email protected]>
daos-stack · Jun 11, 2024 · 5c8868f · 5c8868f
1 parent 3ca434b
commit 5c8868f
Show file tree

Hide file tree

Showing 15 changed files with 506 additions and 281 deletions.
diff --git a/src/common/tests_dmg_helpers.c b/src/common/tests_dmg_helpers.c
@@ -1118,9 +1118,9 @@ dmg_pool_extend(const char *dmg_config_file, const uuid_t uuid,
 	rank_list.rl_ranks = ranks;
 	rank_list.rl_nr = rank_nr;
 
-	rank_str = d_rank_list_to_str(&rank_list);
-	if (rank_str == NULL)
-		D_GOTO(out, rc = -DER_NOMEM);
+	rc = d_rank_list_to_str(&rank_list, &rank_str);
+	if (rc != 0)
+		D_GOTO(out, rc);
 
 	uuid_unparse_lower(uuid, uuid_str);
 	args = cmd_push_arg(args, &argcount, "%s ", uuid_str);

diff --git a/src/control/cmd/dmg/pool.go b/src/control/cmd/dmg/pool.go
@@ -646,10 +646,6 @@ func (cmd *PoolQueryCmd) Execute(args []string) error {
 	if cmd.HealthOnly {
 		req.QueryMask = daos.HealthOnlyPoolQueryMask
 	}
-	// TODO (DAOS-10250) The two options should not be incompatible (i.e. engine limitation)
-	if cmd.ShowEnabledRanks && cmd.ShowDisabledRanks {
-		return errIncompatFlags("show-enabled-ranks", "show-disabled-ranks")
-	}
 	if cmd.ShowEnabledRanks {
 		req.QueryMask.SetOptions(daos.PoolQueryOptionEnabledEngines)
 	}

diff --git a/src/control/cmd/dmg/pool_test.go b/src/control/cmd/dmg/pool_test.go
@@ -1012,6 +1012,32 @@ func TestPoolCommands(t *testing.T) {
 			}, " "),
 			nil,
 		},
+		{
+			"Query pool with UUID, enabled ranks and disabled ranks",
+			"pool query --show-disabled --show-enabled 12345678-1234-1234-1234-1234567890ab",
+			strings.Join([]string{
+				printRequest(t, &control.PoolQueryReq{
+					ID: "12345678-1234-1234-1234-1234567890ab",
+					QueryMask: setQueryMask(func(qm *daos.PoolQueryMask) {
+						qm.SetOptions(daos.PoolQueryOptionEnabledEngines, daos.PoolQueryOptionDisabledEngines)
+					}),
+				}),
+			}, " "),
+			nil,
+		},
+		{
+			"Query pool with UUID, enabled ranks and disabled ranks",
+			"pool query -b -e 12345678-1234-1234-1234-1234567890ab",
+			strings.Join([]string{
+				printRequest(t, &control.PoolQueryReq{
+					ID: "12345678-1234-1234-1234-1234567890ab",
+					QueryMask: setQueryMask(func(qm *daos.PoolQueryMask) {
+						qm.SetOptions(daos.PoolQueryOptionEnabledEngines, daos.PoolQueryOptionDisabledEngines)
+					}),
+				}),
+			}, " "),
+			nil,
+		},
 		{
 			"Query pool for health only",
 			"pool query --health-only 12345678-1234-1234-1234-1234567890ab",
@@ -1056,12 +1082,6 @@ func TestPoolCommands(t *testing.T) {
 			"",
 			fmt.Errorf("Unknown command"),
 		},
-		{
-			"Query pool with incompatible arguments",
-			"pool query --show-disabled --show-enabled 12345678-1234-1234-1234-1234567890ab",
-			"",
-			errors.New("may not be mixed"),
-		},
 	})
 }
 

diff --git a/src/gurt/misc.c b/src/gurt/misc.c
@@ -744,24 +744,39 @@ d_rank_list_dump(d_rank_list_t *rank_list, d_string_t name, int name_len)
  * Create a ranged string representation of a rank list.
  *
  * \param[in]  rank_list	the rank list to represent
+ * \param[out] ranks_str	Returned ranged string (caller must free)
  *
- * \return			a ranged string (caller must free)
+ * \return			0 on success, a negative value on error
  */
-char *
-d_rank_list_to_str(d_rank_list_t *rank_list)
+int
+d_rank_list_to_str(d_rank_list_t *ranks, char **ranks_str)
 {
-	char			*str;
-	bool			 truncated = false;
-	d_rank_range_list_t	*range_list;
+	d_rank_range_list_t *range_list = NULL;
+	char                *range_list_str;
+	int                  rc;
+
+	D_ASSERT(ranks_str != NULL);
 
-	range_list = d_rank_range_list_create_from_ranks(rank_list);
+	if (ranks == NULL) {
+		range_list_str = NULL;
+		D_GOTO(out, rc = -DER_SUCCESS);
+	}
+
+	range_list = d_rank_range_list_create_from_ranks(ranks);
 	if (range_list == NULL)
-		return NULL;
-	str = d_rank_range_list_str(range_list, &truncated);
+		D_GOTO(error, rc = -DER_NOMEM);
 
+	rc = d_rank_range_list_str(range_list, &range_list_str);
+	if (rc != 0)
+		D_GOTO(error, rc);
+
+out:
+	*ranks_str = range_list_str;
+
+error:
 	d_rank_range_list_free(range_list);
 
-	return str;
+	return rc;
 }
 
 d_rank_list_t *
@@ -844,7 +859,6 @@ d_rank_range_list_realloc(d_rank_range_list_t *range_list, uint32_t size)
 	return range_list;
 }
 
-/* TODO (DAOS-10253) Add unit tests for this function */
 d_rank_range_list_t *
 d_rank_range_list_create_from_ranks(d_rank_list_t *rank_list)
 {
@@ -890,56 +904,51 @@ d_rank_range_list_create_from_ranks(d_rank_list_t *rank_list)
 	return range_list;
 }
 
-/* TODO (DAOS-10253) Add unit tests for this function */
-char *
-d_rank_range_list_str(d_rank_range_list_t *list, bool *truncated)
+int
+d_rank_range_list_str(d_rank_range_list_t *list, char **ranks_str)
 {
-	const size_t	MAXBYTES = 512;
-	char	       *line;
-	char	       *linepos;
-	int		ret = 0;
-	size_t		remaining = MAXBYTES - 2u;
-	int		i;
-	int		err = 0;
+	const size_t MAXBYTES  = 512u;
+	size_t       remaining = MAXBYTES - 2u;
+	char        *line;
+	char        *linepos;
+	int          i;
+	int          len;
+	int          rc;
+
+	D_ASSERT(list != NULL);
 
-	*truncated = false;
 	D_ALLOC(line, MAXBYTES);
 	if (line == NULL)
-		return NULL;
+		D_GOTO(error, rc = -DER_NOMEM);
 
 	*line = '[';
 	linepos = line + 1;
 	for (i = 0; i < list->rrl_nr; i++) {
-		uint32_t	lo = list->rrl_ranges[i].lo;
-		uint32_t	hi = list->rrl_ranges[i].hi;
-		bool		lastrange = (i == (list->rrl_nr - 1));
+		uint32_t lo        = list->rrl_ranges[i].lo;
+		uint32_t hi        = list->rrl_ranges[i].hi;
+		bool     lastrange = (i == (list->rrl_nr - 1));
 
 		if (lo == hi)
-			ret = snprintf(linepos, remaining, "%u%s", lo, lastrange ? "" : ",");
+			len = snprintf(linepos, remaining, "%u%s", lo, lastrange ? "" : ",");
 		else
-			ret = snprintf(linepos, remaining, "%u-%u%s", lo, hi, lastrange ? "" : ",");
-
-		if (ret < 0) {
-			err = errno;
-			D_ERROR("rank set could not be serialized: %s (%d)\n", strerror(err), err);
-			break;
-		}
-
-		if (ret >= remaining) {
-			err = EOVERFLOW;
-			D_WARN("rank set has been partially serialized\n");
-			break;
-		}
-
-		remaining -= ret;
-		linepos += ret;
+			len = snprintf(linepos, remaining, "%u-%u%s", lo, hi, lastrange ? "" : ",");
+		if (len < 0)
+			D_GOTO(error, rc = -DER_INVAL);
+		if (len >= remaining)
+			D_GOTO(error, rc = -DER_TRUNC);
+
+		remaining -= len;
+		linepos += len;
 	}
 	memcpy(linepos, "]", 2u);
 
-	if (err != 0)
-		*truncated = true;
+	*ranks_str = line;
+	D_GOTO(out, rc = -DER_SUCCESS);
 
-	return line;
+error:
+	D_FREE(line);
+out:
+	return rc;
 }
 
 void