Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DAOS-16038 pool: Enable pool list for clients #14575

Closed
wants to merge 11 commits into from
2 changes: 1 addition & 1 deletion src/client/api/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const struct daos_task_api dc_funcs[] = {
/** Management */
{dc_deprecated, 0},
{dc_deprecated, 0},
{dc_deprecated, 0},
{dc_mgmt_pool_list, sizeof(daos_mgmt_pool_list_t)},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you really need to add a user task API for this function though? I'm not really sure this is required and you can probably not worry about changes in the task stuff.
otherwise changes here are not enough and you would need to add a few more things.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. I was following the pattern set by @kccain when he added the original implementation of pool list. I haven't really dug into it beyond that.

Given that I don't feel strongly either way, I'm happy to follow your guidance on the best way to implement this feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mchaarawi: Ping... Please advise on the recommended approach here. I'm getting ready to rebase all of this work on master, and would prefer to push that PR in as close to a final state as possible rather than iterating on it.

Also, I don't see much in the way of client API tests. Not particularly keen to invent all of that myself, but happy to work with someone who knows more about this side of things. I am planning to add ftests for this feature -- would that be acceptable in lieu of new client API unit tests?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry i didn't know you were still looking for my review on this. as i mentioned, you should just avoid adding this change with the task api, otherwise you still need to update other structures to support that.
for client api tests, im not sure what you mean. i believe you should add some tests there:
https://github.com/daos-stack/daos/blob/master/src/tests/suite/daos_mgmt.c
but if you want to use ftest, that is fine too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry i didn't know you were still looking for my review on this. as i mentioned, you should just avoid adding this change with the task api, otherwise you still need to update other structures to support that.

OK, is the task API deprecated? I have a superficial understanding of the client API's workings, so as I said I was just following the existing patterns. If I didn't use the task API, would I basically just rework daos_mgmt_list_pools() to just call dc_mgmt_pool_list() directly? Is there any downside to doing it this way?

for client api tests, im not sure what you mean. i believe you should add some tests there: https://github.com/daos-stack/daos/blob/master/src/tests/suite/daos_mgmt.c but if you want to use ftest, that is fine too.

Ah, OK. I was thinking of unit tests under src/client/api/tests. I'll just rework the suite test to use the re-added client API instead of the dmg helper.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, is the task API deprecated? I have a superficial understanding of the client API's workings, so as I said I was just following the existing patterns.

no the task api is not deprecated. but to add the task API entry for this function, you still need to update:
https://github.com/daos-stack/daos/blob/mjmac/DAOS-15982/src/include/daos/task.h
to be safe.
otherwise you can just remove the task api entry for it, and do as you suggested:

If I didn't use the task API, would I basically just rework daos_mgmt_list_pools() to just call dc_mgmt_pool_list() directly? Is there any downside to doing it this way?

either way is fine for me.

{dc_debug_set_params, sizeof(daos_set_params_t)},
{dc_mgmt_get_bs_state, sizeof(daos_mgmt_get_bs_state_t)},

Expand Down
26 changes: 26 additions & 0 deletions src/client/api/mgmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,29 @@ daos_mgmt_put_sys_info(struct daos_sys_info *info)
{
dc_mgmt_put_sys_info(info);
}

int
daos_mgmt_list_pools(const char *group, daos_size_t *npools, daos_mgmt_pool_info_t *pools,
daos_event_t *ev)
{
daos_mgmt_pool_list_t *args;
tse_task_t *task;
int rc;

DAOS_API_ARG_ASSERT(*args, MGMT_LIST_POOLS);

if (npools == NULL) {
D_ERROR("npools must be non-NULL\n");
return -DER_INVAL;
}

rc = dc_task_create(dc_mgmt_pool_list, NULL, ev, &task);
if (rc)
return rc;
args = dc_task_get_args(task);
args->grp = group;
args->pools = pools;
args->npools = npools;

return dc_task_schedule(task, true);
}
211 changes: 169 additions & 42 deletions src/control/cmd/daos/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
"github.com/daos-stack/daos/src/control/cmd/daos/pretty"
"github.com/daos-stack/daos/src/control/common"
"github.com/daos-stack/daos/src/control/lib/daos"
"github.com/daos-stack/daos/src/control/lib/ranklist"
"github.com/daos-stack/daos/src/control/lib/ui"
"github.com/daos-stack/daos/src/control/logging"
)

/*
Expand Down Expand Up @@ -180,6 +180,7 @@ func (cmd *poolBaseCmd) getAttr(name string) (*attribute, error) {
}

type poolCmd struct {
List poolListCmd `command:"list" description:"list pools to which this user has access"`
Query poolQueryCmd `command:"query" description:"query pool info"`
QueryTargets poolQueryTargetsCmd `command:"query-targets" description:"query pool target info"`
ListConts containerListCmd `command:"list-containers" alias:"list-cont" description:"list all containers in pool"`
Expand Down Expand Up @@ -263,21 +264,43 @@ func convertPoolInfo(pinfo *C.daos_pool_info_t) (*daos.PoolInfo, error) {
return poolInfo, nil
}

func generateRankSet(ranklist *C.d_rank_list_t) string {
if ranklist.rl_nr == 0 {
return ""
func queryPool(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

if queryMask.HasOption(daos.PoolQueryOptionEnabledEngines) || queryMask.HasOption(daos.PoolQueryOptionDisabledEngines) {
rlPtr = &rl
}

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)
if err := daosError(rc); err != nil {
return nil, err
}

poolInfo, err := convertPoolInfo(&cPoolInfo)
if err != nil {
return nil, err
}
ranks := uintptr(unsafe.Pointer(ranklist.rl_ranks))
const size = unsafe.Sizeof(uint32(0))
rankset := "["
for i := 0; i < int(ranklist.rl_nr); i++ {
if i > 0 {
rankset += ","

if rlPtr != nil {
rs, err := rankSetFromC(rl)
if err != nil {
return nil, err
}
if queryMask.HasOption(daos.PoolQueryOptionEnabledEngines) {
poolInfo.EnabledRanks = rs
}
if queryMask.HasOption(daos.PoolQueryOptionDisabledEngines) {
poolInfo.DisabledRanks = rs
}
rankset += fmt.Sprint(*(*uint32)(unsafe.Pointer(ranks + uintptr(i)*size)))
}
rankset += "]"
return rankset

return poolInfo, nil
}

func (cmd *poolQueryCmd) Execute(_ []string) error {
Expand All @@ -295,42 +318,15 @@ func (cmd *poolQueryCmd) Execute(_ []string) error {
queryMask.SetOptions(daos.PoolQueryOptionDisabledEngines)
}

var rlPtr **C.d_rank_list_t = nil
var rl *C.d_rank_list_t = nil

if cmd.ShowEnabledRanks || cmd.ShowDisabledRanks {
rlPtr = &rl
}

cleanup, err := cmd.resolveAndConnect(C.DAOS_PC_RO, nil)
if err != nil {
return err
}
defer cleanup()

cPoolInfo := C.daos_pool_info_t{
pi_bits: C.uint64_t(queryMask),
}

rc := C.daos_pool_query(cmd.cPoolHandle, rlPtr, &cPoolInfo, nil, nil)
defer C.d_rank_list_free(rl)
if err := daosError(rc); err != nil {
return errors.Wrapf(err,
"failed to query pool %s", cmd.poolUUID)
}

poolInfo, err := convertPoolInfo(&cPoolInfo)
poolInfo, err := queryPool(cmd.cPoolHandle, queryMask)
if err != nil {
return err
}

if rlPtr != nil {
if cmd.ShowEnabledRanks {
poolInfo.EnabledRanks = ranklist.MustCreateRankSet(generateRankSet(rl))
}
if cmd.ShowDisabledRanks {
poolInfo.DisabledRanks = ranklist.MustCreateRankSet(generateRankSet(rl))
}
return errors.Wrapf(err, "failed to query pool %q", cmd.PoolID())
}

if cmd.JSONOutputEnabled() {
Expand Down Expand Up @@ -601,3 +597,134 @@ func (cmd *poolAutoTestCmd) Execute(_ []string) error {

return nil
}

func getPoolList(log logging.Logger, sysName string, queryEnabled bool) ([]*daos.PoolInfo, error) {
var cSysName *C.char
if sysName != "" {
cSysName := C.CString(sysName)
defer freeString(cSysName)
}

var cPools []C.daos_mgmt_pool_info_t
for {
var rc C.int
var poolCount C.size_t

// First, fetch the total number of pools in the system.
// We may not have access to all of them, so this is an upper bound.
rc = C.daos_mgmt_list_pools(cSysName, &poolCount, nil, nil)
if err := daosError(rc); err != nil {
return nil, err
}
log.Debugf("pools in system: %d", poolCount)

if poolCount < 1 {
return nil, nil
}

// Now, we actually fetch the pools into the buffer that we've created.
cPools = make([]C.daos_mgmt_pool_info_t, poolCount)
rc = C.daos_mgmt_list_pools(cSysName, &poolCount, &cPools[0], nil)
err := daosError(rc)
if err == nil {
cPools = cPools[:poolCount] // adjust the slice to the number of pools retrieved
log.Debugf("fetched %d pools", len(cPools))
break
}
if err == daos.StructTooSmall {
log.Notice("server-side pool list changed; re-fetching")
continue
}
log.Errorf("failed to fetch pool list: %s", err)
return nil, err
}

pools := make([]*daos.PoolInfo, 0, len(cPools))
for i := 0; i < len(cPools); i++ {
cPool := &cPools[i]

svcRanks, err := rankSetFromC(cPool.mgpi_svc)
if err != nil {
return nil, err
}
poolUUID, err := uuidFromC(cPool.mgpi_uuid)
if err != nil {
return nil, err
}

poolLabel := C.GoString(cPool.mgpi_label)

var pool *daos.PoolInfo
if queryEnabled {
var poolInfo C.daos_pool_info_t
var poolHandle C.daos_handle_t
cPoolUUID := C.CString(poolUUID.String())
defer freeString(cPoolUUID)

if err := daosError(C.daos_pool_connect(cPoolUUID, cSysName, C.DAOS_PC_RO,
&poolHandle, &poolInfo, nil)); err != nil {
log.Errorf("failed to connect to pool %q: %s", poolLabel, err)
continue
}

var qErr error
pool, qErr = queryPool(poolHandle, daos.DefaultPoolQueryMask)
if qErr != nil {
log.Errorf("failed to query pool %q: %s", poolLabel, qErr)
}
if err := daosError(C.daos_pool_disconnect(poolHandle, nil)); err != nil {
log.Errorf("failed to disconnect from pool %q: %s", poolLabel, err)
}
if qErr != nil {
continue
}

// Add a few missing pieces that the query doesn't fill in.
pool.Label = poolLabel
pool.ServiceReplicas = svcRanks.Ranks()
} else {
// Just populate the basic info.
pool = &daos.PoolInfo{
UUID: poolUUID,
Label: poolLabel,
ServiceReplicas: svcRanks.Ranks(),
State: daos.PoolServiceStateReady,
}
}

pools = append(pools, pool)
}

log.Debugf("fetched %d/%d pools", len(pools), len(cPools))
return pools, nil
}

type poolListCmd struct {
daosCmd
SysName string `long:"sys-name" short:"G" description:"DAOS system name"`
Verbose bool `short:"v" long:"verbose" description:"Add pool UUIDs and service replica lists to display"`
NoQuery bool `short:"n" long:"no-query" description:"Disable query of listed pools"`
}

func (cmd *poolListCmd) Execute(_ []string) error {
pools, err := getPoolList(cmd.Logger, cmd.SysName, !cmd.NoQuery)
if err != nil {
return err
}

if cmd.JSONOutputEnabled() {
return cmd.OutputJSON(struct {
Pools []*daos.PoolInfo `json:"pools"` // compatibility with dmg
}{
Pools: pools,
}, nil)
}

var buf strings.Builder
if err := pretty.PrintPoolList(pools, &buf, cmd.Verbose); err != nil {
return err
}
cmd.Info(buf.String())

return nil
}
Loading
Loading