dm-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
* [dm-devel] [PATCH v2 00/14] multipath: fixes for sysfs accessors
@ 2022-07-13  8:20 mwilck
  2022-07-13  8:20 ` [dm-devel] [PATCH v2 08/14] libmultipath: sysfs_attr_set_value(): don't return 0 on partial write mwilck
  0 siblings, 1 reply; 3+ messages in thread
From: mwilck @ 2022-07-13  8:20 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck

From: Martin Wilck <mwilck@suse.com>

This set fixes some strangeness in our sysfs accessors which I
found while looking at
https://github.com/opensvc/multipath-tools/issues/35#issuecomment-1175901745.
(The patches don't fix this issue, which seems to be related to
Debian's initramfs setup).

Most importantly, sysfs_attr_get_value() and sysfs_attr_set_value()
would return 0 if the number of bytes read/written was different from
the expected value, which is non-standard and unexpected. This
series changes the return value semantics of these functions:

 - in sysfs_attr_get_value(), if a read buffer is too small to hold
   the string read plus a terminating 0 byte, the return value
   equals the buffer size.
 - in sysfs_bin_attr_get_value(), no 0 bytes are appended. It's not
   an error if the read buffer is completely filled, and no warning
   is printed in this case.
 - sysfs_attr_set_value() always returns the number of bytes written
   unless an error occured in open() or write().

Tests for the new semantics are added. Moreover, the sysfs.c code
is slightly refactored to avoid code duplication.

Changes v1 -> v2:
   08/14: fixes suggested by Benjamin Marzinski

I am not resending the entire series; see
https://github.com/openSUSE/multipath-tools/commits/tip
 
Martin Wilck (14):
  libmultipath: alua: remove get_sysfs_pg83()
  libmultipath: remove sysfs_get_binary()
  libmultipath: sysfs_bin_attr_get_value(): no error if buffer is filled
  libmultipath: common code path for sysfs_attr_get_value()
  libmultipath: sanitize error checking in sysfs accessors
  libmultipath: get rid of PATH_SIZE
  libmultipath: sysfs_attr_get_value(): don't return 0 if buffer too
    small
  libmultipath: sysfs_attr_set_value(): don't return 0 on partial write
  libmultipath: sysfs: cleanup file descriptors on pthread_cancel()
  libmultipath, multipathd: log failure setting sysfs attributes
  multipath tests: expect_condlog: skip depending on verbosity
  multipath tests: __wrap_dlog: print log message
  multipath tests: add sysfs test
  libmultipath.version: bump version for sysfs accessors

 libmultipath/configure.c              |  30 +-
 libmultipath/discovery.c              | 124 +++----
 libmultipath/libmultipath.version     |   8 +-
 libmultipath/prioritizers/alua_rtpg.c |  57 +--
 libmultipath/propsel.c                |   6 +-
 libmultipath/structs.h                |   3 -
 libmultipath/sysfs.c                  | 191 ++++------
 libmultipath/sysfs.h                  |  23 ++
 libmultipath/util.c                   |   8 +-
 multipathd/cli_handlers.c             |   2 +-
 multipathd/fpin_handlers.c            |  11 +-
 multipathd/main.c                     |  40 ++-
 tests/Makefile                        |   5 +-
 tests/sysfs.c                         | 494 ++++++++++++++++++++++++++
 tests/test-lib.c                      |   1 -
 tests/test-log.c                      |   5 +
 16 files changed, 753 insertions(+), 255 deletions(-)
 create mode 100644 tests/sysfs.c

-- 
2.36.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [dm-devel] [PATCH v2 08/14] libmultipath: sysfs_attr_set_value(): don't return 0 on partial write
  2022-07-13  8:20 [dm-devel] [PATCH v2 00/14] multipath: fixes for sysfs accessors mwilck
@ 2022-07-13  8:20 ` mwilck
  2022-07-15 15:17   ` Benjamin Marzinski
  0 siblings, 1 reply; 3+ messages in thread
From: mwilck @ 2022-07-13  8:20 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck

From: Martin Wilck <mwilck@suse.com>

sysfs_attr_set_value() returned 0 if not all requested bytes were written.
Change this to return the number of bytes written. Error checking is now
somewhat more involved; provide a helper macro for it.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/configure.c | 10 ++++--
 libmultipath/discovery.c | 74 +++++++++++++++++++++++++---------------
 libmultipath/sysfs.c     |  6 ++--
 libmultipath/sysfs.h     | 10 ++++++
 4 files changed, 66 insertions(+), 34 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 467bbaa..0607dba 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -568,6 +568,7 @@ sysfs_set_max_sectors_kb(struct multipath *mpp, int is_reload)
 	struct pathgroup * pgp;
 	struct path *pp;
 	char buff[11];
+	ssize_t len;
 	int i, j, ret, err = 0;
 	struct udev_device *udd;
 	int max_sectors_kb;
@@ -600,14 +601,17 @@ sysfs_set_max_sectors_kb(struct multipath *mpp, int is_reload)
 		}
 	}
 	snprintf(buff, 11, "%d", max_sectors_kb);
+	len = strlen(buff);
 
 	vector_foreach_slot (mpp->pg, pgp, i) {
 		vector_foreach_slot(pgp->paths, pp, j) {
 			ret = sysfs_attr_set_value(pp->udev,
 						   "queue/max_sectors_kb",
-						   buff, strlen(buff));
-			if (ret < 0) {
-				condlog(1, "failed setting max_sectors_kb on %s : %s", pp->dev, strerror(-ret));
+						   buff, len);
+			if (ret != len) {
+				log_sysfs_attr_set_value(1, ret,
+					"failed setting max_sectors_kb on %s",
+					pp->dev);
 				err = 1;
 			}
 		}
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 54b1caf..ee29009 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -598,13 +598,15 @@ sysfs_set_eh_deadline(struct path *pp)
 		len = sprintf(value, "%d", pp->eh_deadline);
 
 	ret = sysfs_attr_set_value(hostdev, "eh_deadline",
-				   value, len + 1);
+				   value, len);
 	/*
 	 * not all scsi drivers support setting eh_deadline, so failing
 	 * is totally reasonable
 	 */
-	if (ret <= 0)
-		condlog(3, "%s: failed to set eh_deadline to %s, error %d", udev_device_get_sysname(hostdev), value, -ret);
+	if (ret != len)
+		log_sysfs_attr_set_value(3, ret,
+			"%s: failed to set eh_deadline to %s",
+			udev_device_get_sysname(hostdev), value);
 
 	udev_device_unref(hostdev);
 	return (ret <= 0);
@@ -667,19 +669,22 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 	    pp->fast_io_fail != MP_FAST_IO_FAIL_OFF) {
 		/* Check if we need to temporarily increase dev_loss_tmo */
 		if ((unsigned int)pp->fast_io_fail >= tmo) {
+			ssize_t len;
+
 			/* Increase dev_loss_tmo temporarily */
 			snprintf(value, sizeof(value), "%u",
 				 (unsigned int)pp->fast_io_fail + 1);
+			len = strlen(value);
 			ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
-						   value, strlen(value));
-			if (ret <= 0) {
+						   value, len);
+			if (ret != len) {
 				if (ret == -EBUSY)
 					condlog(3, "%s: rport blocked",
 						rport_id);
 				else
-					condlog(0, "%s: failed to set "
-						"dev_loss_tmo to %s, error %d",
-						rport_id, value, -ret);
+					log_sysfs_attr_set_value(0, ret,
+						"%s: failed to set dev_loss_tmo to %s",
+						rport_id, value);
 				goto out;
 			}
 		}
@@ -691,32 +696,39 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 		pp->dev_loss = DEFAULT_DEV_LOSS_TMO;
 	}
 	if (pp->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
+		ssize_t len;
+
 		if (pp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
 			sprintf(value, "off");
 		else if (pp->fast_io_fail == MP_FAST_IO_FAIL_ZERO)
 			sprintf(value, "0");
 		else
 			snprintf(value, 16, "%u", pp->fast_io_fail);
+		len = strlen(value);
 		ret = sysfs_attr_set_value(rport_dev, "fast_io_fail_tmo",
-					   value, strlen(value));
-		if (ret <= 0) {
+					   value, len);
+		if (ret != len) {
 			if (ret == -EBUSY)
 				condlog(3, "%s: rport blocked", rport_id);
 			else
-				condlog(0, "%s: failed to set fast_io_fail_tmo to %s, error %d",
-					rport_id, value, -ret);
+				log_sysfs_attr_set_value(0, ret,
+					"%s: failed to set fast_io_fail_tmo to %s",
+					rport_id, value);
 		}
 	}
 	if (pp->dev_loss != DEV_LOSS_TMO_UNSET) {
+		ssize_t len;
+
 		snprintf(value, 16, "%u", pp->dev_loss);
-		ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
-					   value, strlen(value));
-		if (ret <= 0) {
+		len = strlen(value);
+		ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, len);
+		if (ret != len) {
 			if (ret == -EBUSY)
 				condlog(3, "%s: rport blocked", rport_id);
 			else
-				condlog(0, "%s: failed to set dev_loss_tmo to %s, error %d",
-					rport_id, value, -ret);
+				log_sysfs_attr_set_value(0, ret,
+					"%s: failed to set dev_loss_tmo to %s",
+					rport_id, value);
 		}
 	}
 out:
@@ -754,12 +766,16 @@ sysfs_set_session_tmo(struct path *pp)
 			condlog(3, "%s: can't set fast_io_fail_tmo to '0'"
 				"on iSCSI", pp->dev);
 		} else {
+			ssize_t len, ret;
+
 			snprintf(value, 11, "%u", pp->fast_io_fail);
-			if (sysfs_attr_set_value(session_dev, "recovery_tmo",
-						 value, strlen(value)) <= 0) {
-				condlog(3, "%s: Failed to set recovery_tmo, "
-					" error %d", pp->dev, errno);
-			}
+			len = strlen(value);
+			ret = sysfs_attr_set_value(session_dev, "recovery_tmo",
+						   value, len);
+			if (ret != len)
+				log_sysfs_attr_set_value(3, ret,
+					"%s: Failed to set recovery_tmo to %s",
+							 pp->dev, value);
 		}
 	}
 	udev_device_unref(session_dev);
@@ -802,12 +818,16 @@ sysfs_set_nexus_loss_tmo(struct path *pp)
 		pp->sg_id.channel, pp->sg_id.scsi_id, end_dev_id);
 
 	if (pp->dev_loss != DEV_LOSS_TMO_UNSET) {
+		ssize_t len, ret;
+
 		snprintf(value, 11, "%u", pp->dev_loss);
-		if (sysfs_attr_set_value(sas_dev, "I_T_nexus_loss_timeout",
-					 value, strlen(value)) <= 0)
-			condlog(3, "%s: failed to update "
-				"I_T Nexus loss timeout, error %d",
-				pp->dev, errno);
+		len = strlen(value);
+		ret = sysfs_attr_set_value(sas_dev, "I_T_nexus_loss_timeout",
+					   value, len);
+		if (ret != len)
+			log_sysfs_attr_set_value(3, ret,
+				"%s: failed to update I_T Nexus loss timeout",
+				pp->dev);
 	}
 	udev_device_unref(sas_dev);
 	return;
diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c
index 125f1c2..9c84af7 100644
--- a/libmultipath/sysfs.c
+++ b/libmultipath/sysfs.c
@@ -134,7 +134,7 @@ ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
 	/* write attribute value */
 	fd = open(devpath, O_WRONLY);
 	if (fd < 0) {
-		condlog(2, "%s: attribute '%s' can not be opened: %s",
+		condlog(3, "%s: attribute '%s' can not be opened: %s",
 			__func__, devpath, strerror(errno));
 		return -errno;
 	}
@@ -144,11 +144,9 @@ ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
 		size = -errno;
 		condlog(3, "%s: write to %s failed: %s", __func__, 
 			devpath, strerror(errno));
-	} else if (size < (ssize_t)value_len) {
+	} else if (size < (ssize_t)value_len)
 		condlog(3, "%s: underflow writing %zu bytes to %s. Wrote %zd bytes",
 			__func__, value_len, devpath, size);
-		size = 0;
-	}
 
 	close(fd);
 	return size;
diff --git a/libmultipath/sysfs.h b/libmultipath/sysfs.h
index cdc84e4..799f68e 100644
--- a/libmultipath/sysfs.h
+++ b/libmultipath/sysfs.h
@@ -5,6 +5,7 @@
 #ifndef _LIBMULTIPATH_SYSFS_H
 #define _LIBMULTIPATH_SYSFS_H
 #include <stdbool.h>
+#include "strbuf.h"
 
 ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
 			     const char * value, size_t value_len);
@@ -25,6 +26,15 @@ ssize_t sysfs_bin_attr_get_value(struct udev_device *dev, const char *attr_name,
 		sysfs_attr_value_ok(__rc, __l); \
 	})
 
+#define log_sysfs_attr_set_value(prio, rc, fmt, __args...)		\
+do {									\
+	STRBUF_ON_STACK(__buf);						\
+	if (print_strbuf(&__buf, fmt, ##__args) >= 0 &&			\
+	    print_strbuf(&__buf, ": %s", rc < 0 ? strerror(-rc) :	\
+					"write underflow") >= 0)	\
+		condlog(prio, "%s", get_strbuf_str(&__buf));		\
+} while(0)
+
 int sysfs_get_size (struct path *pp, unsigned long long * size);
 int sysfs_check_holders(char * check_devt, char * new_devt);
 bool sysfs_is_multipathed(struct path *pp, bool set_wwid);
-- 
2.36.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [dm-devel] [PATCH v2 08/14] libmultipath: sysfs_attr_set_value(): don't return 0 on partial write
  2022-07-13  8:20 ` [dm-devel] [PATCH v2 08/14] libmultipath: sysfs_attr_set_value(): don't return 0 on partial write mwilck
@ 2022-07-15 15:17   ` Benjamin Marzinski
  0 siblings, 0 replies; 3+ messages in thread
From: Benjamin Marzinski @ 2022-07-15 15:17 UTC (permalink / raw)
  To: mwilck; +Cc: Christophe Varoqui, dm-devel

On Wed, Jul 13, 2022 at 10:20:43AM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> sysfs_attr_set_value() returned 0 if not all requested bytes were written.
> Change this to return the number of bytes written. Error checking is now
> somewhat more involved; provide a helper macro for it.
> 

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/configure.c | 10 ++++--
>  libmultipath/discovery.c | 74 +++++++++++++++++++++++++---------------
>  libmultipath/sysfs.c     |  6 ++--
>  libmultipath/sysfs.h     | 10 ++++++
>  4 files changed, 66 insertions(+), 34 deletions(-)
> 
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index 467bbaa..0607dba 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -568,6 +568,7 @@ sysfs_set_max_sectors_kb(struct multipath *mpp, int is_reload)
>  	struct pathgroup * pgp;
>  	struct path *pp;
>  	char buff[11];
> +	ssize_t len;
>  	int i, j, ret, err = 0;
>  	struct udev_device *udd;
>  	int max_sectors_kb;
> @@ -600,14 +601,17 @@ sysfs_set_max_sectors_kb(struct multipath *mpp, int is_reload)
>  		}
>  	}
>  	snprintf(buff, 11, "%d", max_sectors_kb);
> +	len = strlen(buff);
>  
>  	vector_foreach_slot (mpp->pg, pgp, i) {
>  		vector_foreach_slot(pgp->paths, pp, j) {
>  			ret = sysfs_attr_set_value(pp->udev,
>  						   "queue/max_sectors_kb",
> -						   buff, strlen(buff));
> -			if (ret < 0) {
> -				condlog(1, "failed setting max_sectors_kb on %s : %s", pp->dev, strerror(-ret));
> +						   buff, len);
> +			if (ret != len) {
> +				log_sysfs_attr_set_value(1, ret,
> +					"failed setting max_sectors_kb on %s",
> +					pp->dev);
>  				err = 1;
>  			}
>  		}
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 54b1caf..ee29009 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -598,13 +598,15 @@ sysfs_set_eh_deadline(struct path *pp)
>  		len = sprintf(value, "%d", pp->eh_deadline);
>  
>  	ret = sysfs_attr_set_value(hostdev, "eh_deadline",
> -				   value, len + 1);
> +				   value, len);
>  	/*
>  	 * not all scsi drivers support setting eh_deadline, so failing
>  	 * is totally reasonable
>  	 */
> -	if (ret <= 0)
> -		condlog(3, "%s: failed to set eh_deadline to %s, error %d", udev_device_get_sysname(hostdev), value, -ret);
> +	if (ret != len)
> +		log_sysfs_attr_set_value(3, ret,
> +			"%s: failed to set eh_deadline to %s",
> +			udev_device_get_sysname(hostdev), value);
>  
>  	udev_device_unref(hostdev);
>  	return (ret <= 0);
> @@ -667,19 +669,22 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
>  	    pp->fast_io_fail != MP_FAST_IO_FAIL_OFF) {
>  		/* Check if we need to temporarily increase dev_loss_tmo */
>  		if ((unsigned int)pp->fast_io_fail >= tmo) {
> +			ssize_t len;
> +
>  			/* Increase dev_loss_tmo temporarily */
>  			snprintf(value, sizeof(value), "%u",
>  				 (unsigned int)pp->fast_io_fail + 1);
> +			len = strlen(value);
>  			ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
> -						   value, strlen(value));
> -			if (ret <= 0) {
> +						   value, len);
> +			if (ret != len) {
>  				if (ret == -EBUSY)
>  					condlog(3, "%s: rport blocked",
>  						rport_id);
>  				else
> -					condlog(0, "%s: failed to set "
> -						"dev_loss_tmo to %s, error %d",
> -						rport_id, value, -ret);
> +					log_sysfs_attr_set_value(0, ret,
> +						"%s: failed to set dev_loss_tmo to %s",
> +						rport_id, value);
>  				goto out;
>  			}
>  		}
> @@ -691,32 +696,39 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
>  		pp->dev_loss = DEFAULT_DEV_LOSS_TMO;
>  	}
>  	if (pp->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
> +		ssize_t len;
> +
>  		if (pp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
>  			sprintf(value, "off");
>  		else if (pp->fast_io_fail == MP_FAST_IO_FAIL_ZERO)
>  			sprintf(value, "0");
>  		else
>  			snprintf(value, 16, "%u", pp->fast_io_fail);
> +		len = strlen(value);
>  		ret = sysfs_attr_set_value(rport_dev, "fast_io_fail_tmo",
> -					   value, strlen(value));
> -		if (ret <= 0) {
> +					   value, len);
> +		if (ret != len) {
>  			if (ret == -EBUSY)
>  				condlog(3, "%s: rport blocked", rport_id);
>  			else
> -				condlog(0, "%s: failed to set fast_io_fail_tmo to %s, error %d",
> -					rport_id, value, -ret);
> +				log_sysfs_attr_set_value(0, ret,
> +					"%s: failed to set fast_io_fail_tmo to %s",
> +					rport_id, value);
>  		}
>  	}
>  	if (pp->dev_loss != DEV_LOSS_TMO_UNSET) {
> +		ssize_t len;
> +
>  		snprintf(value, 16, "%u", pp->dev_loss);
> -		ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
> -					   value, strlen(value));
> -		if (ret <= 0) {
> +		len = strlen(value);
> +		ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, len);
> +		if (ret != len) {
>  			if (ret == -EBUSY)
>  				condlog(3, "%s: rport blocked", rport_id);
>  			else
> -				condlog(0, "%s: failed to set dev_loss_tmo to %s, error %d",
> -					rport_id, value, -ret);
> +				log_sysfs_attr_set_value(0, ret,
> +					"%s: failed to set dev_loss_tmo to %s",
> +					rport_id, value);
>  		}
>  	}
>  out:
> @@ -754,12 +766,16 @@ sysfs_set_session_tmo(struct path *pp)
>  			condlog(3, "%s: can't set fast_io_fail_tmo to '0'"
>  				"on iSCSI", pp->dev);
>  		} else {
> +			ssize_t len, ret;
> +
>  			snprintf(value, 11, "%u", pp->fast_io_fail);
> -			if (sysfs_attr_set_value(session_dev, "recovery_tmo",
> -						 value, strlen(value)) <= 0) {
> -				condlog(3, "%s: Failed to set recovery_tmo, "
> -					" error %d", pp->dev, errno);
> -			}
> +			len = strlen(value);
> +			ret = sysfs_attr_set_value(session_dev, "recovery_tmo",
> +						   value, len);
> +			if (ret != len)
> +				log_sysfs_attr_set_value(3, ret,
> +					"%s: Failed to set recovery_tmo to %s",
> +							 pp->dev, value);
>  		}
>  	}
>  	udev_device_unref(session_dev);
> @@ -802,12 +818,16 @@ sysfs_set_nexus_loss_tmo(struct path *pp)
>  		pp->sg_id.channel, pp->sg_id.scsi_id, end_dev_id);
>  
>  	if (pp->dev_loss != DEV_LOSS_TMO_UNSET) {
> +		ssize_t len, ret;
> +
>  		snprintf(value, 11, "%u", pp->dev_loss);
> -		if (sysfs_attr_set_value(sas_dev, "I_T_nexus_loss_timeout",
> -					 value, strlen(value)) <= 0)
> -			condlog(3, "%s: failed to update "
> -				"I_T Nexus loss timeout, error %d",
> -				pp->dev, errno);
> +		len = strlen(value);
> +		ret = sysfs_attr_set_value(sas_dev, "I_T_nexus_loss_timeout",
> +					   value, len);
> +		if (ret != len)
> +			log_sysfs_attr_set_value(3, ret,
> +				"%s: failed to update I_T Nexus loss timeout",
> +				pp->dev);
>  	}
>  	udev_device_unref(sas_dev);
>  	return;
> diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c
> index 125f1c2..9c84af7 100644
> --- a/libmultipath/sysfs.c
> +++ b/libmultipath/sysfs.c
> @@ -134,7 +134,7 @@ ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
>  	/* write attribute value */
>  	fd = open(devpath, O_WRONLY);
>  	if (fd < 0) {
> -		condlog(2, "%s: attribute '%s' can not be opened: %s",
> +		condlog(3, "%s: attribute '%s' can not be opened: %s",
>  			__func__, devpath, strerror(errno));
>  		return -errno;
>  	}
> @@ -144,11 +144,9 @@ ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
>  		size = -errno;
>  		condlog(3, "%s: write to %s failed: %s", __func__, 
>  			devpath, strerror(errno));
> -	} else if (size < (ssize_t)value_len) {
> +	} else if (size < (ssize_t)value_len)
>  		condlog(3, "%s: underflow writing %zu bytes to %s. Wrote %zd bytes",
>  			__func__, value_len, devpath, size);
> -		size = 0;
> -	}
>  
>  	close(fd);
>  	return size;
> diff --git a/libmultipath/sysfs.h b/libmultipath/sysfs.h
> index cdc84e4..799f68e 100644
> --- a/libmultipath/sysfs.h
> +++ b/libmultipath/sysfs.h
> @@ -5,6 +5,7 @@
>  #ifndef _LIBMULTIPATH_SYSFS_H
>  #define _LIBMULTIPATH_SYSFS_H
>  #include <stdbool.h>
> +#include "strbuf.h"
>  
>  ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
>  			     const char * value, size_t value_len);
> @@ -25,6 +26,15 @@ ssize_t sysfs_bin_attr_get_value(struct udev_device *dev, const char *attr_name,
>  		sysfs_attr_value_ok(__rc, __l); \
>  	})
>  
> +#define log_sysfs_attr_set_value(prio, rc, fmt, __args...)		\
> +do {									\
> +	STRBUF_ON_STACK(__buf);						\
> +	if (print_strbuf(&__buf, fmt, ##__args) >= 0 &&			\
> +	    print_strbuf(&__buf, ": %s", rc < 0 ? strerror(-rc) :	\
> +					"write underflow") >= 0)	\
> +		condlog(prio, "%s", get_strbuf_str(&__buf));		\
> +} while(0)
> +
>  int sysfs_get_size (struct path *pp, unsigned long long * size);
>  int sysfs_check_holders(char * check_devt, char * new_devt);
>  bool sysfs_is_multipathed(struct path *pp, bool set_wwid);
> -- 
> 2.36.1
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-07-15 15:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-13  8:20 [dm-devel] [PATCH v2 00/14] multipath: fixes for sysfs accessors mwilck
2022-07-13  8:20 ` [dm-devel] [PATCH v2 08/14] libmultipath: sysfs_attr_set_value(): don't return 0 on partial write mwilck
2022-07-15 15:17   ` Benjamin Marzinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).