Re: [PATCH 2/2] ndctl: add "all" dimm-id option for update-firmware

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Wed, 2018-02-21 at 15:06 -0700, Dave Jiang wrote:
> Adding all option to allow ndctl to update all DIMMs at once.

I was going to ask that we should also update the documentation for
this command to include the new 'all' behavior, but it looks like
during the revisions of the original series, the documentation update
to list the options explicitly got dropped?

Anyway, can you re-add those, and also talk about --dimm=all there.

On a side note, should we make this more inline with other dimm
commands, such as xable-dimm or read/write-labels, where the dimm name
(or 'all') is a non-option argument? That would make it something like:

  ndctl update-firmware --firmware=<file> nmem0 (or 'all)

> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  ndctl/update.c |   25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/ndctl/update.c b/ndctl/update.c
> index fc26acf..1b771ff 100644
> --- a/ndctl/update.c
> +++ b/ndctl/update.c
> @@ -77,6 +77,7 @@ struct update_context {
>  	struct ndctl_dimm *dimm;
>  	struct fw_info dimm_fw;
>  	struct ndctl_cmd *start;
> +	bool all;
>  };
>  
>  /*
> @@ -471,16 +472,23 @@ static int get_ndctl_dimm(struct update_context
> *uctx, void *ctx)
>  {
>  	struct ndctl_dimm *dimm;
>  	struct ndctl_bus *bus;
> +	int rc = -ENODEV;
>  
>  	ndctl_bus_foreach(ctx, bus)
>  		ndctl_dimm_foreach(bus, dimm) {
> -			if (!util_dimm_filter(dimm, uctx->dimm_id))
> +			if (!uctx->all &&
> +				!util_dimm_filter(dimm, uctx-
> >dimm_id))
>  				continue;
>  			uctx->dimm = dimm;
> -			return 0;
> +			rc = update_firmware(uctx);
> +			if (rc < 0) {
> +				error("Update firmware for dimm %s
> failed\n",
> +						ndctl_dimm_get_devna
> me(dimm));
> +				continue;
> +			}
>  		}
>  
> -	return -ENODEV;
> +	return rc;
>  }
>  
>  static int verify_fw_file(struct update_context *uctx)
> @@ -573,18 +581,17 @@ int cmd_update_firmware(int argc, const char
> **argv, void *ctx)
>  		return rc;
>  	}
>  
> +	if (strcmp(uctx.dimm_id, "all") == 0)
> +		uctx.all = true;
> +	else
> +		uctx.all = false;
> +
>  	rc = get_ndctl_dimm(&uctx, ctx);
>  	if (rc < 0) {
>  		error("DIMM %s not found", uctx.dimm_id);
>  		return rc;
>  	}
>  
> -	rc = update_firmware(&uctx);
> -	if (rc < 0) {
> -		error("Update firmware failed");
> -		return rc;
> -	}
> -
>  	if (uctx.start)
>  		ndctl_cmd_unref(uctx.start);
>  
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm