Re: [PATCH v7 4/5] scsi_transport_fc: Added store fucntionality to set the rport port_state using sysfs

["Ewan D. Milne" <emilne@redhat.com>]

On Wed, 2020-11-11 at 10:28 +0530, Muneendra wrote:
> Added a store functionality to set rport port_state using sysfs
> under  fc_remote_ports/rport-*/port_state
> 
> With this functionality the user can move the port_state from
> Marginal -> Online and Online->Marginal.
> 
> On Marginal :This interface will set SCMD_NORETRIES_ABORT bit in
> scmd->state for all the pending io's on the scsi device associated
> with target port.
> 
> On Online :This interface will clear SCMD_NORETRIES_ABORT bit in
> scmd->state for all the pending io's on the scsi device associated
> with target port.
> 
> Below is the interface provided to set the port state to Marginal
> and Online.
> 
> echo "Marginal" >> /sys/class/fc_remote_ports/rport-X\:Y-Z/port_state
> echo "Online" >> /sys/class/fc_remote_ports/rport-X\:Y-Z/port_state
> 
> Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
> 
> ---
> v7:
> No change
> 
> v6:
> No change
> 
> v5:
> No change
> 
> v4:
> Addressed the error reported by kernel test robot
> Removed the code needed to traverse all the devices under rport
> to set/clear SCMD_NORETRIES_ABORT
> Removed unncessary comments.
> Return the error values on failure while setting the port_state
> 
> v3:
> Removed the port_state from starget attributes.
> Enabled the store functionality for port_state under remote port.
> used the starget_for_each_device to traverse around all the devices
> under rport
> 
> v2:
> Changed from a noretries_abort attribute under fc_transport/target*/
> to
> port_state for changing the port_state to a marginal state
> ---
>  drivers/scsi/scsi_transport_fc.c | 56
> ++++++++++++++++++++++++++++++--
>  1 file changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_transport_fc.c
> b/drivers/scsi/scsi_transport_fc.c
> index ffd25195ae62..d378ca4a60fe 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -1238,7 +1238,59 @@ show_fc_rport_roles (struct device *dev,
> struct device_attribute *attr,
>  static FC_DEVICE_ATTR(rport, roles, S_IRUGO,
>  		show_fc_rport_roles, NULL);
>  
> -fc_private_rport_rd_enum_attr(port_state, FC_PORTSTATE_MAX_NAMELEN);
> +static ssize_t fc_rport_set_marginal_state(struct device *dev,
> +						struct device_attribute
> *attr,
> +						const char *buf, size_t
> count)
> +{
> +	struct fc_rport *rport = transport_class_to_rport(dev);
> +	enum fc_port_state port_state;
> +	int ret = 0;
> +
> +	ret = get_fc_port_state_match(buf, &port_state);
> +	if (ret)
> +		return -EINVAL;
> +	if (port_state == FC_PORTSTATE_MARGINAL) {
> +		/*
> +		 * Change the state to marginal only if the
> +		 * current rport state is Online
> +		 * Allow only Online->marginal
> +		 */
> +		if (rport->port_state == FC_PORTSTATE_ONLINE)
> +			rport->port_state = port_state;
> +		else
> +			return -EINVAL;
> +	} else if (port_state == FC_PORTSTATE_ONLINE) {
> +		/*
> +		 * Change the state to Online only if the
> +		 * current rport state is Marginal
> +		 * Allow only  MArginal->Online
> +		 */
> +		if (rport->port_state == FC_PORTSTATE_MARGINAL)
> +			rport->port_state = port_state;
> +		else
> +			return -EINVAL;
> +	} else
> +		return -EINVAL;
> +	return count;
> +}
> +
> +static ssize_t
> +show_fc_rport_port_state(struct device *dev,
> +				struct device_attribute *attr, char
> *buf)
> +{
> +	const char *name;
> +	struct fc_rport *rport = transport_class_to_rport(dev);
> +
> +	name = get_fc_port_state_name(rport->port_state);
> +	if (!name)
> +		return -EINVAL;
> +
> +	return snprintf(buf, 20, "%s\n", name);
> +}
> +
> +static FC_DEVICE_ATTR(rport, port_state, 0444 | 0200,
> +			show_fc_rport_port_state,
> fc_rport_set_marginal_state);
> +
>  fc_private_rport_rd_attr(scsi_target_id, "%d\n", 20);
>  
>  /*
> @@ -2681,7 +2733,7 @@ fc_attach_transport(struct fc_function_template
> *ft)
>  	SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(port_name);
>  	SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(port_id);
>  	SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(roles);
> -	SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(port_state);
> +	SETUP_PRIVATE_RPORT_ATTRIBUTE_RW(port_state);
>  	SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(scsi_target_id);
>  	SETUP_PRIVATE_RPORT_ATTRIBUTE_RW(fast_io_fail_tmo);
>  

Reviewed-by: Ewan D. Milne <emilne@redhat.com>