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

Rework of ledctl help and manual #206

Merged
merged 5 commits into from
Feb 27, 2024

Conversation

mtkaczyk
Copy link
Contributor

@mtkaczyk mtkaczyk commented Feb 8, 2024

This is a newer version of #192

  • adding help for ledctl modes,
  • add --ibpi mode and make it default,
  • Do not print all log level options just --log-level,
  • add help test,
  • improve manual, print modes and modes options separatelly.

Fixes: intel/ledmon/#123

This is how updated section of ledctl man looks like on my setup:

MODES AND MODE SPECIFIC OPTIONS
       This chapter describes modes and options that can be used with for particular mode. If mode is not specified then it fallbacks to IBPI mode.

   -I or --ibpi
       Sets pattern on given devices. Allows ledctl to determine best controller automatically. Must be followed by IBPI option.

       PATTERN={device1 device2} or PATTERN=device1,device2
           IBPI option with special syntax. It accepts a list of devices in two formats. The first format is a list with comma separated elements. The second format is a list in curly braces and
           elements are separated by space. Reffer to "Pattern names" for supported states list.  See "EXAMPLES" for examples of usage.

       -x or --listed-only
           By default ledctl checks all devices and may update states on different ones than requested if state change is determined (generally it happens for devices in raid array). With this
           option ledctl will change state only on devices listed in CLI. It is optional.

   -L or --list-controllers
       Prints information (system path and type) of all controllers detected by ledctl and exits.

   -P or --list-slots
       Prints all slots for the controller type. Must be followed by --controller-type.

   -G or --get-slot
       Displays particular slot details. Must be followed by --controller-type and slot identificatior --device or --slot. --print can be used to control output.

       -r=print or --print=print>
           It is optional parameter for --get-slot. It is used to retrieve only one specific property from output per every slot. Supported values are: slot, device and state.

   -S or --set-slot
       Changes led state for slot. Must be followed by --controller-type, slot identificatior --device or --slot and --state.

       -p=state or --state=state
           It is mandatory parmeter for --set-slot It provides state to be set for the slot. Reffer to "Pattern names" for supported states list.

   SLOT MANAGEMENT OPTIONS
       This section describes options necessary by varius slot management modes.

       -n=controller_type or --controller-type=controller_type
           Controller type chosen for the request. Supported types are: VMD, NPEM and SCSI.

       -d=device or --device=device
           Block device node for device attached to the slot. It can be used instead of --slot.

       -p=slot or --slot=slot
           Unique slot identifier It can be used instead of --device. Use --list-slots to determine slot identifiers. Slot definition depends on the controller and is unique across all controllers
           of the same type. Slot identifier is always available, even if device is not connected.

GENERAL OPTIONS
       This section describes options aviable for every mode.

       -l-path or --log=path
           Sets a path to log file. If this option is not specified then default log file /var/log/ledctl.log is used.

       -h or --help
           Prints help text out and exits. Use --MODE --help to get help for requested mode or --help to get general help.

       -v or --version
           Displays version of ledctl and information about the license and exits.

       --log_level=log_level
           Sets log level for application. Following levels can be used, starting from the most verbose: ALL, DEBUG, INFO, WARNING, ERROR, QUIET. By default WARNING is set.

@mtkaczyk mtkaczyk requested review from bkucman and ktanska February 8, 2024 15:47
@mtkaczyk mtkaczyk force-pushed the mtkaczyk_ledctl_help branch from a047869 to c0ad3da Compare February 9, 2024 14:34
- adding help for ledctl modes,
- add --ibpi mode and make it default,
- Do not print all log level options just --log-level,
- add help test,
- improve manual, print modes and modes options separatelly.

Fixes: intel/ledmon/intel#123

Signed-off-by: Blazej Kucman <blazej.kucman@intel.com>
Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
@mtkaczyk mtkaczyk force-pushed the mtkaczyk_ledctl_help branch from c0ad3da to e75a7be Compare February 9, 2024 14:35
Copy link
Contributor

@mku514k mku514k left a comment

Choose a reason for hiding this comment

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

Small notes, overall LGTM.

src/ledctl/ledctl.c Outdated Show resolved Hide resolved
tests/ledctl/parameters_test.py Outdated Show resolved Hide resolved
tests/ledctl/parameters_test.py Outdated Show resolved Hide resolved
Few fixes suggested by Mateusz.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
@mtkaczyk mtkaczyk force-pushed the mtkaczyk_ledctl_help branch from a109c4e to cc30b17 Compare February 13, 2024 11:23
src/ledctl/help.c Show resolved Hide resolved
src/ledctl/help.c Outdated Show resolved Hide resolved
src/ledctl/help.c Show resolved Hide resolved
src/ledctl/help.c Outdated Show resolved Hide resolved
src/ledctl/help.c Outdated Show resolved Hide resolved
doc/ledctl.pod Outdated Show resolved Hide resolved
doc/ledctl.pod Outdated Show resolved Hide resolved
doc/ledctl.pod Outdated Show resolved Hide resolved
doc/ledctl.pod Outdated Show resolved Hide resolved
doc/ledctl.pod Show resolved Hide resolved
@mtkaczyk
Copy link
Contributor Author

mtkaczyk commented Feb 22, 2024

Here updated examples:

EXAMPLES
   The following example illustrates how to set locate on a single block device. Note that all remaining LEDs, might be changed.
       "ledctl --ibpi locate=/dev/sda"

   The following example illustrates how to set locate_off on a single block device.
       "ledctl --ibpi --listed-only locate_off=/dev/sda"

   The following example illustrates how to set off on the given devices. It uses second format of device list.
       "ledctl --ibpi --listed-only off={ /dev/sda /dev/sdb }"

   The following example illustrates how to set locate and rebuild on different devices at the same time. It uses the second format of device list.
       "ledctl --ibpi --listed-only locate={ /dev/sdb } rebuild={ /sys/block/sdc }"

   The following example illustrates how to locate on three block devices. It uses the first format of device list.
       "ledctl --ibpi --listed-only locate=/dev/sda,/dev/sdb,/dev/sdc"

   The following example illustrates how to set locate and rebuild on different devices at the same time. It uses the first format of device list.
       "ledctl --ibpi --listed-only locate=/dev/sdb rebuild=/sys/block/sdc"

   The following example illustrates how to set locate and rebuild on different devices at the same time. It uses the both formats of device list.
       "ledctl --ibpi --listed-only locate={ /dev/sdb } rebuild=/sys/block/sdc"

   The following example illustrates how to get all detected controllers.
       "ledctl --list-controllers"

   The following example illustrates how to get all slots for controller type NPEM.
       "ledctl --list-slots --controller-type=npem"

   The following example illustrates how to get particular slot with device specified, for controller type SCSI. Print state only.
       "ledctl --get-slot --controller-type=scsi --device=/dev/sda --print=state"

   The following example illustrates how to get particular slot with slot specified, for controller type NPEM. Print device only.
       "ledctl --get-slot --controller-type=npem --slot=10000:02:04.0 --print=device"

   The following example illustrates how to set locate for slot with device specified, for controller type VMD.
       "ledctl --set-slot --controller-type=vmd --device=/dev/nvme0n1 --state=locate"

   The following example illustrates how to set failure for slot with slot specified for controller type VMD.
       "ledctl --set-slot --controller-type=vmd --slot=1 --state=failure"

@mtkaczyk
Copy link
Contributor Author

mtkaczyk commented Feb 22, 2024

and options:

MODES AND MODE SPECIFIC OPTIONS
       This chapter describes modes and options that can be used with for particular mode. If mode is not specified then, it fallbacks to IBPI mode.

   -I or --ibpi
       Sets pattern on given devices. Allows ledctl to determine the best controller automatically. Must be followed by IBPI option. By default, it checks all devices and may update states on
       different drives than requested if state change is determined (generally it happens for devices in raid array).

       PATTERN={ device1 device2 } or PATTERN=device1,device2
           IBPI option with special syntax. It accepts a list of devices in two formats. The first format is a list with comma separated elements. The second format is a list in curly braces and
           elements are separated by space. Refer to "Pattern names" for supported states list.  See "EXAMPLES" for examples of usage.

       -x or --listed-only
           Suppresses default behavior, changes state only on devices listed in CLI. It is optional.

   -L or --list-controllers
       Prints information (system path and type) of all controllers detected.

   -P or --list-slots
       Prints all slots for the controller type. Must be followed by --controller-type.

   -G or --get-slot
       Displays particular slot details. Must be followed by --controller-type and slot identifier --device or --slot. --print can be used to control output.

       -r print or --print=print
           Optional parameter for --get-slot. It is used to retrieve only one specific property from output. Supported values are: slot, device and state.

   -S or --set-slot
       Changes led state for slot. Must be followed by --controller-type, slot identifier --device or --slot and --state.

       -p state or --state=state
           It is mandatory for --set-slot. It provides state to be set for the slot. Refer to "Pattern names" for supported states list.

   SLOT MANAGEMENT OPTIONS
       This section describes options necessary by various slot management modes.

       -n controller_type or --controller-type=controller_type
           Controller type chosen for the request. It agregates multiple controllers of the same type. Supported types are: VMD, NPEM and SCSI.

       -d device or --device=device
           Block device node for device attached to the slot. It can be used instead of --slot.

       -p slot or --slot=slot
           Unique slot identifier. It can be used instead of --device. Use --list-slots to determine slot identifiers. Slot definition depends on the controller and is unique across all controllers
           of the same type. Slot identifier is always available, even if a device is not connected.

GENERAL OPTIONS
       This section describes options available for every mode.

       -l path or --log=path
           Sets a path to log file. If this option is not specified then default log file /var/log/ledctl.log is used.

       -h or --help
           Prints help text out. Use --MODE --help to get help for requested mode or --help to get general help.

       -v or --version
           Displays version of ledctl and information about the license.

       --log_level=log_level
           Sets log level for application. Following levels can be used, starting from the most verbose: ALL, DEBUG, INFO, WARNING, ERROR, QUIET. By default, WARNING is set.

Updates after review from Blazej.

Remove license information from --version for both ledctl and ledmon to
keep consistency. It is unnecessary there.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
@mtkaczyk mtkaczyk force-pushed the mtkaczyk_ledctl_help branch from 35824ea to acbc8a6 Compare February 23, 2024 08:37
@mtkaczyk
Copy link
Contributor Author

@tasleson mind look? Especially wording as we are not native speakers.

Thanks,
Mariusz

Copy link
Contributor

@bkucman bkucman left a comment

Choose a reason for hiding this comment

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

A few small corrections.
Please update date in "# Copyright (C) 2009-2023 Intel Corporation" in files where you made changes.

doc/ledctl.pod Outdated Show resolved Hide resolved
tests/ledctl/parameters_test.py Outdated Show resolved Hide resolved
src/ledctl/help.c Outdated Show resolved Hide resolved
doc/ledctl.pod Show resolved Hide resolved
@tasleson
Copy link
Contributor

@tasleson mind look? Especially wording as we are not native speakers.

Thanks, Mariusz

It looks pretty good. The following are some suggestions that you may want to incorporate.

diff --git a/doc/ledctl.pod b/doc/ledctl.pod
index ac3896d..4c5989e 100644
--- a/doc/ledctl.pod
+++ b/doc/ledctl.pod
@@ -25,10 +25,11 @@ B<ledctl> [I<MODE>] [I<OPTIONS>] ...
 
 =head1 DESCRIPTION
 
-The ledctl is an user space application designed to control LEDs associated with each
-slot in an enclosure or a drive bay. User must have root privileges to use this application.
+The ledctl is an user-space application designed to control LEDs associated with each
+slot in an enclosure or a drive bay. Root privileges are required for its operation.
 
-The ledctl application supports LED management of the SAS/SATA and PCIe storages.
+
+The ledctl application facilitates LED management for SAS/SATA and PCIe storage devices.
 
 =head4 Supported protocols/methods for LED management are:
 
@@ -289,8 +290,8 @@ I<failure> or I<disk_failed> is translated to I<ses_fault>
 
 =head1 MODES AND MODE SPECIFIC OPTIONS
 
-This chapter describes modes and options that can be used with for particular mode. If mode is not
-specified then, it fallbacks to IBPI mode.
+This chapter outlines the modes and options applicable to specific modes. If no mode is
+specified, it defaults to IBPI mode.
 
 =head2 B<-I> or B<--ibpi>
 
@@ -303,10 +304,9 @@ array).
 
 =item B<PATTERN={ device1 device2 }> or B<PATTERN=device1,device2>
 
-IBPI option with special syntax. It accepts a list of devices in two formats. The first format is
-a list with comma separated elements. The second format is a list in curly braces and elements are
-separated by space. Refer to "Pattern names" for supported states list.
-See "EXAMPLES" for examples of usage.
+The IBPI option accepts device lists in two formats: comma-separated elements and elements
+enclosed in curly braces with spaces between them. Refer to "Pattern names" for supported states.
+For usage examples, see "EXAMPLES".
 
 =item B<-x> or B<--listed-only>
 
@@ -324,15 +324,15 @@ Prints all slots for the controller type. Must be followed by B<--controller-typ
 
 =head2 B<-G> or B<--get-slot>
 
-Displays particular slot details. Must be followed by B<--controller-type> and slot identifier
+Displays specific slot details. Must be followed by B<--controller-type> and slot identifier
 B<--device> or B<--slot>. B<--print> can be used to control output.
 
 =over 4
 
 =item B<-r I<print>> or B<--print>=I<print>
 
-Optional parameter for B<--get-slot>. It is used to retrieve only one specific property
-from output. Supported values are: slot, device and state.
+Optional parameter for B<--get-slot>. It is utilized to retrieve a specific property
+from the output. Supported values include: slot, device, and state.
 
 =back
 
@@ -369,7 +369,7 @@ Block device node for device attached to the slot. It can be used instead of B<-
 
 Unique slot identifier. It can be used instead of B<--device>. Use B<--list-slots> to determine slot
 identifiers. Slot definition depends on the controller and is unique across all controllers of the
-same type. Slot identifier is always available, even if a device is not connected.
+same type. The slot identifier is always available, even if a device is not connected.
 
 =back
 
@@ -395,8 +395,8 @@ Displays version of ledctl and information about the license.
 
 =item B<--log_level=I<log_level>>
 
-Sets log level for application. Following levels can be used, starting from the most verbose:
-ALL, DEBUG, INFO, WARNING, ERROR, QUIET. By default, WARNING is set.
+Set the application's logging level. Available levels, listed from most verbose to least verbose,
+include: ALL, DEBUG, INFO, WARNING, ERROR, and QUIET. The default setting is WARNING.
 
 =back
 

Wording fixes.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
Update copyright to 2024.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
@mtkaczyk mtkaczyk force-pushed the mtkaczyk_ledctl_help branch from 05c0b2c to 9849270 Compare February 27, 2024 10:21
@mtkaczyk mtkaczyk merged commit c81fa32 into intel:master Feb 27, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants