Hi Gabriel. Thanks for the patch! A couple of comments below.
On 06/10/2015 02:34 PM, Gabriel Krisman Bertazi wrote:
> show-ucode-levels prints all IPR adapters and devices, along with their
> current firmware levels as well as the latest available version that could
> be found on the filesystem.
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
> ---
> iprconfig.8 | 5 +++
> iprconfig.c | 108
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 113 insertions(+)
>
> diff --git a/iprconfig.8 b/iprconfig.8
> index 2282fac..65cf1a1 100644
> --- a/iprconfig.8
> +++ b/iprconfig.8
> @@ -337,6 +337,11 @@ Show the microcode level that is currently loaded on the
> specified device.
> Note: The device specified may be the sg device associated with an IOA,
> in which case the IOA's microcode level will be shown.
> .TP
> +.B show-ucode-levels
> +.br
> +Show the microcode level that is currently loaded for every device and
> +adapter in the system
> +.TP
> .B query-format-timeout [device]
> .br
> Show the current format timeout to be used when formatting the specified
> disk.
> diff --git a/iprconfig.c b/iprconfig.c
> index 9e034ee..c8e0326 100644
> --- a/iprconfig.c
> +++ b/iprconfig.c
> @@ -15019,6 +15019,113 @@ static int update_ucode_cmd(char **args, int
> num_args)
> }
>
> /**
> + * ucode_level_str - format version string of microcode of a device
> + *
> + * @dev: Device
> + * @buff: Output buffer
> + * @buffsiz: Buffer size
> + *
> + * Returns:
> + * 0 if success / non-zero on failure
> + **/
> +int ucode_level_str(struct ipr_dev *dev, char *buff, size_t bufsiz)
> +{
> + u32 level, level_sw;
> + char *asc;
> +
> + if (!dev)
> + return -EINVAL;
> +
> + if (&dev->ioa->ioa == dev)
> + snprintf(buff, bufsiz, "%08X",
> + get_ioa_fw_version(dev->ioa));
> + else {
> + level = get_dev_fw_version(dev);
> + level_sw = htonl(level);
> + asc = (char *)&level_sw;
> + if (isprint(asc[0]) && isprint(asc[1]) &&
> + isprint(asc[2]) && isprint(asc[3]))
> + snprintf(buff, bufsiz, "%.8X (%c%c%c%c)",
> + level, asc[0], asc[1], asc[2], asc[3]);
> + else
> + snprintf(buff, bufsiz, "%.8X", level);
> + }
> + return 0;
> +}
> +
> +/**
> + * search_latest_ucode_str - format version string of the latest
> + * microcode imagem found in the filesystem.
> + *
> + * @dev: Device
> + * @buff: Output buffer
> + * @buffsiz: Buffer size
> + *
> + * Returns:
> + * 0 if success / non-zero on failure
> + **/
> +static int search_latest_ucode_str(struct ipr_dev *dev, char *buff,
> + size_t bufsiz)
> +{
> + struct ipr_fw_images *fw = NULL;
> + u32 version = 0;
> + buff[0] = '\0';
> +
> + if (!dev)
> + return -ENODEV;
> +
> + if (dev->scsi_dev_data->type == IPR_TYPE_ADAPTER)
> + get_ioa_firmware_image_list(dev->ioa, &fw);
> + else if (ipr_is_ses(dev))
> + get_ses_firmware_image_list(dev, &fw);
> + else if (ipr_is_gscsi(dev) || ipr_is_af_dasd_device(dev))
> + get_dasd_firmware_image_list(dev, &fw);
> +
> + if (!fw)
> + return -EINVAL;
> +
> + snprintf(buff, bufsiz, "%X (%s)", fw->version, fw->file);
> +
> + free(fw);
> + return 0;
> +}
> +
> +/**
> + * show_ucode_levels - List microcode level of every device and adapter
> + * @args: argument vector
> + * @num_args: number of arguments
> + *
> + * Returns:
> + * 0 if success / non-zero on failure
> + **/
> +static int show_ucode_levels(char **args, int num_args)
> +{
> + struct ipr_ioa *ioa;
> + struct ipr_dev *dev;
> + size_t buflen = 32;
> + char fwb[buflen];
> + char lfwb[PATH_MAX];
> +
> + printf("Name Microcode Level Latest Microcode Available\n"
> + "------ ----------------
> ------------------------------------\n");
I'd like to see a little more device identification data for each device
listed. We
probably don't need to list the full filename of the microcode image. It would
also
be nice if we had a flag to indicate whether the firmware was up to date or not.
How about something like this:
Name PCI/SCSI Location Vendor Product ID Current Available
------ ------------------------- -------- ---------------- -------- ---------
sg0 0007:01:00.0/2: IBM 57B4001SISIOA 12511700 12511701*
sg1 0007:01:00.0/2:0:0:0 IBM HUC106030CSS600 41333236 41333236
sg2 0007:01:00.0/2:0:1:0 IBM HUSMM1640ASS201 42303730 42303730
sdb 0007:01:00.0/2:0:2:0 IBM HUC101860CS420 4E373230
> +
> + for_each_ioa(ioa) {
> + for_each_dev(ioa, dev) {
> + if (ipr_is_volume_set(dev))
> + continue;
> +
> + ucode_level_str(dev, fwb, buflen);
> + search_latest_ucode_str(dev, lfwb, PATH_MAX);
> +
> + printf("%-6s %-16s %s\n", basename(dev->gen_name),
> + fwb, lfwb);
I'd prefer to use dev->dev_name if its a non zero string, as that is more
useful for JBOD devices.
> + }
> + }
> +
> + return 0;
> +}
> +
> +/**
> * disrupt_device_cmd -
> * @args: argument vector
> * @num_args: number of arguments
> @@ -18395,6 +18502,7 @@ static const struct {
> { "raid-rebuild", 1, 0, 1, raid_rebuild_cmd,
> "sg6" },
> { "disrupt-device", 1, 0, 1, disrupt_device_cmd,
> "sg6" },
> { "update-ucode", 2, 0, 2, update_ucode_cmd, "sg5
> /root/ucode.bin" },
> + { "show-ucode-levels", 0, 0, 0, show_ucode_levels, ""
> },
> { "set-format-timeout", 2, 0, 2, set_format_timeout,
> "sg6 4" },
> { "set-qdepth", 2, 0, 2, set_qdepth, "sda 16" },
> { "set-tcq-enable", 2, 0, 2, set_tcq_enable, "sda
> 0" },
>
--
Brian King
Power Linux I/O
IBM Linux Technology Center
------------------------------------------------------------------------------
_______________________________________________
Iprdd-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/iprdd-devel