Thanks for the patch Wendy! Some comments below:
On 11/07/2017 06:39 PM, wenxi...@linux.vnet.ibm.com wrote:
> From: Wen Xiong <wenxi...@linux.vnet.ibm.com>
>
> iprconfig unable to donwload microcode on all applicable devices
> in both CMD and GUI interfaces. The patch fixes the issues.
minor: s/donwload/download/
>
> Signed-off-by: Wen Xiong <wenxi...@linux.vnet.ibm.com>
> ---
> iprconfig.c | 27 +++++++++++++++++++++++++++
> 1 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/iprconfig.c b/iprconfig.c
> index 11233cf..a1afb77 100644
> --- a/iprconfig.c
> +++ b/iprconfig.c
> @@ -11842,6 +11842,27 @@ int download_all_ucode(i_container *i_con)
> if (!ioa->ioa.scsi_dev_data || ioa->ioa_dead)
> continue;
>
> + dev = find_dev(ioa->ioa.gen_name);
> + lfw = get_latest_fw_image(dev);
> +
> + if (!lfw || lfw->version > get_fw_version(dev)) {
I got confused by this condition. So, you're basically checking if lfw
is NULL _or_ its version is higher than a current version of the device.
This is strange, because it'll enter the if block if it's NULL.
Suppose:
lfw = NULL;
if (!lfw || anything)
printf("here");
So, "here" is always printed, since !NULL resolves to true.
Hence, I guess this conditions is erroneous, perhaps you need to check
if it's not NULL *AND* some other condition about the version of the FW.
> + sprintf(line, "%-6s %-10X %-10s %s\n",
> + basename(dev->gen_name), lfw->version,
> + lfw->date, lfw->file);
> +
> + body = add_string_to_body(body, line, "",
> &header_lines);
> +
> + /* Add device to update list */
> + elem = malloc(sizeof(struct download_ucode_elem));
> + elem->dev = dev;
> + elem->lfw = lfw;
> + elem->next = update_list;
> + update_list = elem;
> + }
> +
> + if (!lfw || lfw->version <= get_fw_version(dev))
> + free(lfw);
Could use an "else", right?
> +
> for_each_dev (ioa, dev) {
> if (ipr_is_volume_set(dev))
> continue;
> @@ -15754,6 +15775,12 @@ static int update_all_ucodes(char **args, int
> num_args)
> for_each_ioa(ioa) {
> if (!ioa->ioa.scsi_dev_data)
> continue;
> +
> + dev = find_dev(ioa->ioa.gen_name);
> + lfw = get_latest_fw_image(dev);
> + if (!lfw || lfw->version > get_fw_version(dev))
Same comment about the condition as above.
> + rc = update_ioa_ucode(dev->ioa, lfw->file);
> +
> for_each_dev(ioa, dev) {
> if (ipr_is_volume_set(dev))
> continue;
>
Cheers,
Guilherme
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Iprdd-devel mailing list
Iprdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iprdd-devel
