Skip to content

Commit

Permalink
Ledctl: add command to retrieve default controller (#248)
Browse files Browse the repository at this point in the history
Add new command to ledctl, which enables to print the default,
preferred available controller for given device. Output from this
can be helpful when user must specify controller in other
ledctl commands.
Add parameter tests for new command: --default-controller.
Use new method to choose default controller for device. It is especially
needed when VMD and NPEM are available on one platform. Skip IBPI test
when drive is connected to different default controller, than used to
test.
Test multipath drives only with default controller connected.

---------

Signed-off-by: Kinga Stefaniuk <kinga.stefaniuk@intel.com>
  • Loading branch information
ktanska authored Oct 16, 2024
1 parent 4c63ac8 commit a8b320d
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 21 deletions.
10 changes: 10 additions & 0 deletions doc/ledctl.pod
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,16 @@ Suppresses default behavior, changes state only on devices listed in CLI. It is

Prints information (system path and type) of all controllers detected.

=head2 B<-B> or B<--default-controller>

There is a possibility that device is supported by more than one controller (e.g. NPEM and VMD).
In that case, this command can be used to query ledctl to return preferred controller type.
The returned controller type is used by subset of routines which do not take controller type
directly (e.g. --get-slot, --list-slots, --set-slot).

Prints preferred controller type which should be used to control LED for this device.
Must be followed by B<--device>, for which controller will be printed.

=head2 B<-P> or B<--list-slots>

Prints all slots for the controller type. Must be followed by B<--controller-type>.
Expand Down
11 changes: 11 additions & 0 deletions src/ledctl/help.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ static struct help_option LIST_CTRL_HELP_OPTS[] = {
HELP_OPTION_LOG_LEVEL,
};

static struct help_option DEFAULT_CTRL_HELP_OPTS[] = {
HELP_OPTION_DEVICE,
HELP_OPTION_HELP,
HELP_OPTION_LOG_LEVEL,
};

struct help_mode {
enum opt option_id;
struct option *opt;
Expand Down Expand Up @@ -121,6 +127,9 @@ static struct help_mode modes[] = {
HELP_MODE(SET_SLOT,
"Set given state for given slot or device under the given controller.\n"
"Options \"--slot\" and \"--device\" cannot be used simultaneously."),

HELP_MODE(DEFAULT_CTRL,
"Print the controller with the highest priority for given device."),
};

/**
Expand Down Expand Up @@ -285,6 +294,8 @@ static struct help_option GENERAL_HELP_OPTS[] = {
{NULL, "Print slot details for device/slot.", &longopt_all[OPT_GET_SLOT]},
{NULL, "Indicate IBPI mode, it is used as default.", &longopt_all[OPT_IBPI]},
{NULL, "Display list of controllers recognizable by ledctl.", &longopt_all[OPT_LIST_CTRL]},
{NULL, "Print the preferred supported controller for device.",
&longopt_all[OPT_DEFAULT_CTRL]},
{NULL, "Print all slots for a controller requested.", &longopt_all[OPT_LIST_SLOTS]},
{NULL, "Set state for slot/device by controller requested.", &longopt_all[OPT_SET_SLOT]}
};
Expand Down
35 changes: 35 additions & 0 deletions src/ledctl/ledctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,19 @@ static int possible_params_modes[] = {
OPT_SET_SLOT,
OPT_LIST_SLOTS,
OPT_LIST_CTRL,
OPT_DEFAULT_CTRL,
OPT_IBPI
};

static int possible_params_list_ctrl[] = {
COMMON_GETOPT_ARGS
};

static int possible_params_default_ctrl[] = {
OPT_DEVICE,
COMMON_GETOPT_ARGS
};

static int possible_params_set_slot[] = {
OPT_CNTRL_TYPE,
OPT_DEVICE,
Expand Down Expand Up @@ -541,6 +547,9 @@ void _cmdline_parse_modes(int argc, char *argv[], struct request *req)
case 'L':
req->chosen_opt = OPT_LIST_CTRL;
break;
case 'B':
req->chosen_opt = OPT_DEFAULT_CTRL;
break;
case 'I':
req->chosen_opt = OPT_IBPI;
break;
Expand Down Expand Up @@ -581,6 +590,10 @@ bool _setup_mode_options(struct request * const req, char **shortopts, struct op
setup_options(longopts, shortopts, possible_params_list_ctrl,
ARRAY_SIZE(possible_params_list_ctrl));
break;
case OPT_DEFAULT_CTRL:
setup_options(longopts, shortopts, possible_params_default_ctrl,
ARRAY_SIZE(possible_params_default_ctrl));
break;
case OPT_IBPI:
setup_options(longopts, shortopts, possible_params_ibpi,
ARRAY_SIZE(possible_params_ibpi));
Expand Down Expand Up @@ -756,6 +769,13 @@ static led_status_t verify_request(struct led_ctx *ctx, struct request *req)
{
if (req->chosen_opt == OPT_LIST_CTRL)
return LED_STATUS_SUCCESS;
if (req->chosen_opt == OPT_DEFAULT_CTRL) {
if (!req->device[0]) {
log_error("Device is missing, aborting.");
return LED_STATUS_CMDLINE_ERROR;
} else
return LED_STATUS_SUCCESS;
}
if (req->cntrl == LED_CNTRL_TYPE_UNKNOWN) {
log_error("Invalid controller in the request.");
return LED_STATUS_INVALID_CONTROLLER;
Expand Down Expand Up @@ -858,6 +878,21 @@ led_status_t execute_request(struct led_ctx *ctx, struct request *req)

if (req->chosen_opt == OPT_LIST_SLOTS)
return list_slots(req->cntrl);
if (req->chosen_opt == OPT_DEFAULT_CTRL) {
enum led_cntrl_type cntrl_type;
char device_path[PATH_MAX];

led_device_name_lookup(ctx, req->device, device_path);
cntrl_type = led_is_management_supported(ctx, device_path);

if (cntrl_type == LED_CNTRL_TYPE_UNKNOWN)
lib_log(ctx, LED_LOG_LEVEL_ERROR,
"Unable to determine controller for device\n");
else
printf("%s\n", led_cntrl_type_to_string(cntrl_type));

return STATUS_SUCCESS;
}

if (req->chosen_opt == OPT_LIST_CTRL) {
struct led_cntrl_list_entry *cntrl = NULL;
Expand Down
1 change: 1 addition & 0 deletions src/lib/utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,7 @@ struct option longopt_all[] = {
[OPT_SLOT] = {"slot", required_argument, NULL, 'p'},
[OPT_STATE] = {"state", required_argument, NULL, 's'},
[OPT_PRINT_PARAM] = {"print", required_argument, NULL, 'r'},
[OPT_DEFAULT_CTRL] = {"default-controller", no_argument, NULL, 'B'},
[OPT_TEST] = {"test", no_argument, NULL, 'T'},
[OPT_IBPI] = {"ibpi", no_argument, NULL, 'I' },
[OPT_NULL_ELEMENT] = {NULL, no_argument, NULL, '\0'}
Expand Down
1 change: 1 addition & 0 deletions src/lib/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ enum opt {
OPT_STATE,
OPT_PRINT_PARAM,
OPT_IBPI,
OPT_DEFAULT_CTRL,
OPT_TEST,
OPT_NULL_ELEMENT
};
Expand Down
16 changes: 16 additions & 0 deletions tests/ledctl/ledctl_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,22 @@ def get_slot_by_device(self, slot: Slot):
]).stdout
return self.parse_slot_line(slot.cntrl_type, out)

def get_slot_by_device_cntrl(self, dev_node, cntrl):
# While using this method controllers may be not filtered out.
# Do not return slot for controller removed from test.
if cntrl not in self.slot_ctrls:
return None

out = self.run_ledctl_cmd_valid(
["--get-slot", "--controller-type", cntrl, "--device",
dev_node]).stdout
return self.parse_slot_line(cntrl, out)

def default_controller_by_device(self, dev_node):
result = self.run_ledctl_cmd_valid(
["--default-controller", "--device", dev_node]).stdout
return result.rstrip()

def list_slots(self, controller_type):
rc = []
out = self.run_ledctl_cmd_valid(
Expand Down
24 changes: 19 additions & 5 deletions tests/ledctl/parameters_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ def test_parameters_are_valid_long_test_flag(ledctl_binary):
"-G -n vmd -p 1 -r state -T",
"--get-slot --controller-type=vmd --slot=1 --print=state -T",
"-S -n vmd -p 1 -s normal -T",
"--set-slot --controller-type=vmd --slot=1 --state=normal -T"
"--set-slot --controller-type=vmd --slot=1 --state=normal -T",
"-B -d /dev/nvme0n1 -T",
"--default-controller --device /dev/nvme0n1 -T"
],
)
def test_parameters_are_valid_short_test_flag(ledctl_binary,
Expand Down Expand Up @@ -149,10 +151,22 @@ def parse_help(lines):
@pytest.mark.parametrize(
"help_cmd",
[
"--help", "-h", "--help --badflag", "-hd", "-h -s",
"--set-slot --help", "-S -h --badflag", "-Sh --badflag",
"--get-slot --help", "-Ghb", "--list-controllers --help",
"--ibpi --help", "--list-slots --help", "-L -h"
"--help",
"-h",
"--help --badflag",
"-hd",
"-h -s",
"--set-slot --help",
"-S -h --badflag",
"-Sh --badflag",
"--get-slot --help",
"-Ghb",
"--list-controllers --help",
"--ibpi --help",
"--list-slots --help",
"-L -h",
"--default-controller --help",
"-Bh",
],
)
# Check formatting, header and footer.
Expand Down
39 changes: 23 additions & 16 deletions tests/ledctl/slot_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ def get_slots_with_device_or_skip(cmd: LedctlCmd, cntrl):
return slots_with_device_node


def filter_by_default_controller(cmd: LedctlCmd, slots_to_test):
filtered_slots = []
for slot in slots_to_test:
default_cntrl = cmd.default_controller_by_device(slot.device_node)
if default_cntrl == slot.cntrl_type:
filtered_slots.append(slot)
return filtered_slots


def verify_state(slot, current, expected, msg):
if slot.cntrl_type == "SCSI" and expected == "rebuild":
# No good way to validate this one as read value won't match what we sent down.
Expand All @@ -43,9 +52,15 @@ def test_ibpi(ledctl_binary, slot_filters, controller_filters, cntrl):
"""

cmd = LedctlCmd(ledctl_binary, slot_filters, controller_filters)
slots_with_device_node = get_slots_with_device_or_skip(cmd, cntrl)
slots_to_test = get_slots_with_device_or_skip(cmd, cntrl)
slots_to_test = filter_by_default_controller(cmd, slots_to_test)

for slot in slots_with_device_node:
if not slots_to_test:
pytest.skip(
"Devices detected but this is not primary controller for any drive, skipping"
)

for slot in slots_to_test:
for state in LedctlCmd.base_states:
cmd.set_ibpi(slot.device_node, state)
cur = cmd.get_slot(slot)
Expand Down Expand Up @@ -105,9 +120,8 @@ def test_set_slot_by_device(ledctl_binary, slot_filters, controller_filters,
slot_set_and_get_by_device_all(cmd, slot)


@pytest.mark.parametrize("cntrl", ["VMD", "NPEM"])
def test_nvme_multipath_drives(ledctl_binary, slot_filters, controller_filters,
cntrl):
def test_nvme_multipath_drives(ledctl_binary, slot_filters,
controller_filters):
"""
Special test for multipath drives using both set methods and get via device. We need to check
if ledctl provides nvme multipath minimal support.
Expand All @@ -118,15 +132,11 @@ def test_nvme_multipath_drives(ledctl_binary, slot_filters, controller_filters,
if len(mp_drives) == 0:
pytest.skip("No nvme multipath drives found")

slots_with_device_node = get_slots_with_device_or_skip(cmd, cntrl)
any_found = False

for slot in slots_with_device_node:
if slot.device_node not in mp_drives:
for mp_drive in mp_drives:
mp_cntrl = cmd.default_controller_by_device(mp_drive)
slot = cmd.get_slot_by_device_cntrl(mp_drive, mp_cntrl)
if slot is None:
continue
any_found = True

LOGGER.debug(f"Found nvme multipath drive {slot}")

for state in cmd.base_states:
cmd.set_ibpi(slot.device_node, state)
Expand All @@ -137,6 +147,3 @@ def test_nvme_multipath_drives(ledctl_binary, slot_filters, controller_filters,
)

slot_set_and_get_by_device_all(cmd, slot)

if not any_found:
pytest.skip("Multipath drives are not connected to tested controller")

0 comments on commit a8b320d

Please sign in to comment.