DAOS-16829 client: Fix daos pool query regression

The patch landed for DAOS-16477 introduced a regression in the pool query functionality of the daos tool. This patch fixes the regression and introduces a v2 of the pool query API that will get both enabled and disabled ranks in one call. Features: pool Required-githooks: true Signed-off-by: Michael MacDonald <[email protected]>
daos-stack · Dec 13, 2024 · 33bc637 · 33bc637
1 parent b4e29b5
commit 33bc637
Show file tree

Hide file tree

Showing 5 changed files with 141 additions and 168 deletions.
diff --git a/src/client/api/pool.c b/src/client/api/pool.c
@@ -1,5 +1,5 @@
 /**
- * (C) Copyright 2015-2023 Intel Corporation.
+ * (C) Copyright 2015-2024 Intel Corporation.
  *
  * SPDX-License-Identifier: BSD-2-Clause-Patent
  */
@@ -95,11 +95,49 @@ daos_pool_query(daos_handle_t poh, d_rank_list_t **ranks, daos_pool_info_t *info
 	if (rc)
 		return rc;
 
+	if (info->pi_bits & ((DPI_ENGINES_ENABLED | DPI_ENGINES_DISABLED) ==
+			     (DPI_ENGINES_ENABLED | DPI_ENGINES_DISABLED))) {
+		D_ERROR("enabled and disabled not supported in v1 query\n");
+		return -DER_NOTSUPPORTED;
+	}
+
 	args = dc_task_get_args(task);
-	args->poh	= poh;
-	args->ranks	= ranks;
+	args->poh       = poh;
 	args->info	= info;
 	args->prop	= pool_prop;
+	if (info == NULL || (info->pi_bits & DPI_ENGINES_DISABLED) != 0)
+		args->disabled_ranks = ranks;
+	else if (info != NULL && (info->pi_bits & DPI_ENGINES_ENABLED) != 0)
+		args->enabled_ranks = ranks;
+
+	return dc_task_schedule(task, true);
+}
+
+int
+daos_pool_query_v2(daos_handle_t poh, d_rank_list_t **enabled_ranks, d_rank_list_t **disabled_ranks,
+		   daos_pool_info_t *info, daos_prop_t *pool_prop, daos_event_t *ev)
+{
+	daos_pool_query_t *args;
+	tse_task_t        *task;
+	int                rc;
+
+	DAOS_API_ARG_ASSERT(*args, POOL_QUERY);
+
+	if (pool_prop != NULL && !daos_prop_valid(pool_prop, true, false)) {
+		D_ERROR("invalid pool_prop parameter.\n");
+		return -DER_INVAL;
+	}
+
+	rc = dc_task_create(dc_pool_query, NULL, ev, &task);
+	if (rc)
+		return rc;
+
+	args                 = dc_task_get_args(task);
+	args->poh            = poh;
+	args->enabled_ranks  = enabled_ranks;
+	args->disabled_ranks = disabled_ranks;
+	args->info           = info;
+	args->prop           = pool_prop;
 
 	return dc_task_schedule(task, true);
 }

diff --git a/src/control/cmd/daos/pool.go b/src/control/cmd/daos/pool.go
@@ -295,21 +295,21 @@ func convertPoolInfo(pinfo *C.daos_pool_info_t) (*daos.PoolInfo, error) {
 	return poolInfo, nil
 }
 
-func queryPoolRankLists(poolHdl C.daos_handle_t, queryMask daos.PoolQueryMask) (*daos.PoolInfo, error) {
-	var rlPtr **C.d_rank_list_t = nil
-	var rl *C.d_rank_list_t = nil
+func queryPool(poolHdl C.daos_handle_t, queryMask daos.PoolQueryMask) (*daos.PoolInfo, error) {
+	poolInfo := &daos.PoolInfo{}
 
-	if queryMask.HasOption(daos.PoolQueryOptionEnabledEngines) || queryMask.HasOption(daos.PoolQueryOptionDisabledEngines) ||
-		queryMask.HasOption(daos.PoolQueryOptionDeadEngines) {
-		rlPtr = &rl
-	}
+	var enabledRanks *C.d_rank_list_t
+	var disabledRanks *C.d_rank_list_t
 
 	cPoolInfo := C.daos_pool_info_t{
 		pi_bits: C.uint64_t(queryMask),
 	}
 
-	rc := C.daos_pool_query(poolHdl, rlPtr, &cPoolInfo, nil, nil)
-	defer C.d_rank_list_free(rl)
+	rc := C.daos_pool_query_v2(poolHdl, &enabledRanks, &disabledRanks, &cPoolInfo, nil, nil)
+	defer func() {
+		C.d_rank_list_free(enabledRanks)
+		C.d_rank_list_free(disabledRanks)
+	}()
 	if err := daosError(rc); err != nil {
 		return nil, err
 	}
@@ -319,79 +319,19 @@ func queryPoolRankLists(poolHdl C.daos_handle_t, queryMask daos.PoolQueryMask) (
 		return nil, err
 	}
 
-	if rlPtr != nil {
-		rs, err := rankSetFromC(rl)
+	if enabledRanks != nil {
+		rs, err := rankSetFromC(enabledRanks)
 		if err != nil {
 			return nil, err
 		}
-		if queryMask.HasOption(daos.PoolQueryOptionEnabledEngines) {
-			poolInfo.EnabledRanks = rs
-		}
-		if queryMask.HasOption(daos.PoolQueryOptionDisabledEngines) {
-			poolInfo.DisabledRanks = rs
-		}
-		if queryMask.HasOption(daos.PoolQueryOptionDeadEngines) {
-			poolInfo.DeadRanks = rs
-		}
+		poolInfo.EnabledRanks = rs
 	}
-
-	return poolInfo, nil
-}
-func queryPool(poolHdl C.daos_handle_t, queryMask daos.PoolQueryMask) (*daos.PoolInfo, error) {
-	poolInfo := &daos.PoolInfo{}
-	originalMask := queryMask // Save the original queryMask
-
-	// Function to handle the query and return a single RankList
-	queryAndUpdate := func(option string) error {
-		// Clear previous options and set new option
-		queryMask.ClearAll()
-		queryMask.SetOptions(option)
-
-		poolInfo1, err := queryPoolRankLists(poolHdl, queryMask)
+	if disabledRanks != nil {
+		rs, err := rankSetFromC(disabledRanks)
 		if err != nil {
-			return err
-		}
-
-		switch option {
-		case daos.PoolQueryOptionEnabledEngines:
-			poolInfo.EnabledRanks = poolInfo1.EnabledRanks
-		case daos.PoolQueryOptionDisabledEngines:
-			poolInfo.DisabledRanks = poolInfo1.DisabledRanks
-		case daos.PoolQueryOptionDeadEngines:
-			poolInfo.DeadRanks = poolInfo1.DeadRanks
-		}
-		return nil
-	}
-
-	// Preprocess queryMask, select one option for the first query
-	var firstOption string
-	if originalMask.HasOption(daos.PoolQueryOptionEnabledEngines) {
-		firstOption = daos.PoolQueryOptionEnabledEngines
-	} else if originalMask.HasOption(daos.PoolQueryOptionDisabledEngines) {
-		firstOption = daos.PoolQueryOptionDisabledEngines
-	} else if originalMask.HasOption(daos.PoolQueryOptionDeadEngines) {
-		firstOption = daos.PoolQueryOptionDeadEngines
-	}
-
-	// Perform the first query to get basic information
-	if err := queryAndUpdate(firstOption); err != nil {
-		return nil, err
-	}
-
-	// Check the original query mask and update fields as needed
-	queryOptions := []string{
-		daos.PoolQueryOptionEnabledEngines,
-		daos.PoolQueryOptionDisabledEngines,
-		daos.PoolQueryOptionDeadEngines,
-	}
-
-	// Process each option sequentially
-	for _, opt := range queryOptions {
-		if originalMask.HasOption(opt) && opt != firstOption {
-			if err := queryAndUpdate(opt); err != nil {
-				return nil, err
-			}
+			return nil, err
 		}
+		poolInfo.DisabledRanks = rs
 	}
 
 	return poolInfo, nil

diff --git a/src/include/daos_pool.h b/src/include/daos_pool.h
@@ -341,6 +341,52 @@ int
 daos_pool_query(daos_handle_t poh, d_rank_list_t **ranks, daos_pool_info_t *info,
 		daos_prop_t *pool_prop, daos_event_t *ev);
 
+/**
+ * Query pool information, including multiple rank lists (enabled, disabled, dead). User
+ * should provide at least one of \a info or the \a *disabled_ranks list as output buffer.
+ *
+ * \param[in]	poh	Pool connection handle.
+ * \param[out]	enabled_ranks
+ * 			Optional, returned enabled pool storage engine ranks. This list will
+ * 			be populated if #info is not NULL and #pi_bits has DPI_ENGINES_ENABLED set.
+ *			The caller is responsible for freeing the list with d_rank_list_free().
+ * \param[out]	disabled_ranks
+ * 			Optional, returned disabled pool storage engine ranks.
+ *			If #info is NULL, this list will be populated with the ranks of
+ * 			all engines with any targets disabled. If #info is not NULL, this
+ * 			list will be populated if #pi_bits has DPI_ENGINES_DISABLED set.
+ *			The caller is responsible for freeing the list with d_rank_list_free().
+ * \param[in,out]
+ *		info	Optional, returned pool information,
+ *			see daos_pool_info_bit.
+ * \param[out]	pool_prop
+ *			Optional, returned pool properties.
+ *			If it is NULL, then needs not query the properties.
+ *			If pool_prop is non-NULL but its dpp_entries is NULL,
+ *			will query all pool properties, DAOS internally
+ *			allocates the needed buffers and assign pointer to
+ *			dpp_entries.
+ *			If pool_prop's dpp_nr > 0 and dpp_entries is non-NULL,
+ *			will query the properties for specific dpe_type(s), DAOS
+ *			internally allocates the needed buffer for dpe_str or
+ *			dpe_val_ptr, if the dpe_type with immediate value then
+ *			will directly assign it to dpe_val.
+ *			User can free the associated buffer by calling
+ *			daos_prop_free().
+ * \param[in]	ev	Completion event, it is optional and can be NULL.
+ *			The function will run in blocking mode if \a ev is NULL.
+ *
+ * \return		These values will be returned by \a ev::ev_error in
+ *			non-blocking mode:
+ *			0		Success
+ *			-DER_INVAL	Invalid parameter
+ *			-DER_UNREACH	Network is unreachable
+ *			-DER_NO_HDL	Invalid pool handle
+ */
+int
+daos_pool_query_v2(daos_handle_t poh, d_rank_list_t **enabled_ranks, d_rank_list_t **disabled_ranks,
+		   daos_pool_info_t *info, daos_prop_t *pool_prop, daos_event_t *ev);
+
 /**
  * Query information of storage targets within a DAOS pool.
  *

diff --git a/src/include/daos_task.h b/src/include/daos_task.h
@@ -206,8 +206,10 @@ typedef struct {
 typedef struct {
 	/** Pool open handle. */
 	daos_handle_t		poh;
-	/** Optional, returned storage ranks in this pool. */
-	d_rank_list_t	      **ranks;
+	/** Optional, returned enabled storage ranks in this pool. */
+	d_rank_list_t         **enabled_ranks;
+	/** Optional, returned disabled storage ranks in this pool. */
+	d_rank_list_t         **disabled_ranks;
 	/** Optional, returned pool information. */
 	daos_pool_info_t       *info;
 	/** Optional, returned pool properties. */