[PATCH] multipath: fix setting sysfs fc timeout parameters

[PATCH] multipath: fix setting sysfs fc timeout parameters

[Benjamin Marzinski]

Multipath was accidentally trying to write to the directory where
dev_loss_tmo and fast_io_fail_tmo were located instead to the files
themselves. Also, if dev_loss_tmo was unset, it was trying to set it
to 0. Finally, it wasn't correctly checking for errors in setting
the files.  This patch fixes these issues.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/discovery.c |    7 ++++---
 libmultipath/sysfs.c     |    7 ++++---
 2 files changed, 8 insertions(+), 6 deletions(-)

Index: multipath-tools-120727/libmultipath/discovery.c
===================================================================
--- multipath-tools-120727.orig/libmultipath/discovery.c
+++ multipath-tools-120727/libmultipath/discovery.c
@@ -281,7 +281,8 @@ sysfs_set_rport_tmo(struct multipath *mp
 		pp->sg_id.channel, pp->sg_id.scsi_id, rport_id);
 
 	snprintf(value, 11, "%u", mpp->dev_loss);
-	if (sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, 11) < 0) {
+	if (mpp->dev_loss &&
+	    sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, 11) <= 0) {
 		if ((!mpp->fast_io_fail ||
 		     mpp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
 		    && mpp->dev_loss > 600) {
@@ -289,7 +290,7 @@ sysfs_set_rport_tmo(struct multipath *mp
 				"fast_io_fail is not set", mpp->alias);
 			snprintf(value, 11, "%u", 600);
 			if (sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
-						 value, 11) < 0)
+						 value, 11) <= 0)
 				condlog(0, "%s failed to set dev_loss_tmo",
 					mpp->alias);
 			goto out;
@@ -303,7 +304,7 @@ sysfs_set_rport_tmo(struct multipath *mp
 		else
 			snprintf(value, 11, "%u", mpp->fast_io_fail);
 		if (sysfs_attr_set_value(rport_dev, "fast_io_fail_tmo",
-					 value, 11) < 0) {
+					 value, 11) <= 0) {
 			condlog(0, "%s failed to set fast_io_fail_tmo",
 				mpp->alias);
 		}
Index: multipath-tools-120727/libmultipath/sysfs.c
===================================================================
--- multipath-tools-120727.orig/libmultipath/sysfs.c
+++ multipath-tools-120727/libmultipath/sysfs.c
@@ -41,7 +41,7 @@
 ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
 			     char * value, size_t value_len)
 {
-	const char *devpath;
+	char devpath[PATH_SIZE];
 	struct stat statbuf;
 	int fd;
 	ssize_t size = -1;
@@ -49,8 +49,9 @@ ssize_t sysfs_attr_set_value(struct udev
 	if (!dev || !attr_name || !value)
 		return 0;
 
-	devpath = udev_device_get_syspath(dev);
-	condlog(4, "open '%s'/'%s'", devpath, attr_name);
+	snprintf(devpath, PATH_SIZE, "%s/%s", udev_device_get_syspath(dev),
+		 attr_name);
+	condlog(4, "open '%s'", devpath);
 	if (stat(devpath, &statbuf) != 0) {
 		condlog(4, "stat '%s' failed: %s", devpath, strerror(errno));
 		return 0;

Re: [PATCH] multipath: fix setting sysfs fc timeout parameters

[Christophe Varoqui]

On lun., 2012-08-20 at 17:26 -0500, Benjamin Marzinski wrote:
> Multipath was accidentally trying to write to the directory where
> dev_loss_tmo and fast_io_fail_tmo were located instead to the files
> themselves. Also, if dev_loss_tmo was unset, it was trying to set it
> to 0. Finally, it wasn't correctly checking for errors in setting
> the files.  This patch fixes these issues.
> 
Applied.

Thanks.

> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/discovery.c |    7 ++++---
>  libmultipath/sysfs.c     |    7 ++++---
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> Index: multipath-tools-120727/libmultipath/discovery.c
> ===================================================================
> --- multipath-tools-120727.orig/libmultipath/discovery.c
> +++ multipath-tools-120727/libmultipath/discovery.c
> @@ -281,7 +281,8 @@ sysfs_set_rport_tmo(struct multipath *mp
>  		pp->sg_id.channel, pp->sg_id.scsi_id, rport_id);
>  
>  	snprintf(value, 11, "%u", mpp->dev_loss);
> -	if (sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, 11) < 0) {
> +	if (mpp->dev_loss &&
> +	    sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, 11) <= 0) {
>  		if ((!mpp->fast_io_fail ||
>  		     mpp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
>  		    && mpp->dev_loss > 600) {
> @@ -289,7 +290,7 @@ sysfs_set_rport_tmo(struct multipath *mp
>  				"fast_io_fail is not set", mpp->alias);
>  			snprintf(value, 11, "%u", 600);
>  			if (sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
> -						 value, 11) < 0)
> +						 value, 11) <= 0)
>  				condlog(0, "%s failed to set dev_loss_tmo",
>  					mpp->alias);
>  			goto out;
> @@ -303,7 +304,7 @@ sysfs_set_rport_tmo(struct multipath *mp
>  		else
>  			snprintf(value, 11, "%u", mpp->fast_io_fail);
>  		if (sysfs_attr_set_value(rport_dev, "fast_io_fail_tmo",
> -					 value, 11) < 0) {
> +					 value, 11) <= 0) {
>  			condlog(0, "%s failed to set fast_io_fail_tmo",
>  				mpp->alias);
>  		}
> Index: multipath-tools-120727/libmultipath/sysfs.c
> ===================================================================
> --- multipath-tools-120727.orig/libmultipath/sysfs.c
> +++ multipath-tools-120727/libmultipath/sysfs.c
> @@ -41,7 +41,7 @@
>  ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
>  			     char * value, size_t value_len)
>  {
> -	const char *devpath;
> +	char devpath[PATH_SIZE];
>  	struct stat statbuf;
>  	int fd;
>  	ssize_t size = -1;
> @@ -49,8 +49,9 @@ ssize_t sysfs_attr_set_value(struct udev
>  	if (!dev || !attr_name || !value)
>  		return 0;
>  
> -	devpath = udev_device_get_syspath(dev);
> -	condlog(4, "open '%s'/'%s'", devpath, attr_name);
> +	snprintf(devpath, PATH_SIZE, "%s/%s", udev_device_get_syspath(dev),
> +		 attr_name);
> +	condlog(4, "open '%s'", devpath);
>  	if (stat(devpath, &statbuf) != 0) {
>  		condlog(4, "stat '%s' failed: %s", devpath, strerror(errno));
>  		return 0;