| Message ID | 20220706143822.30182-1-mwilck@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | multipath: fixes for sysfs accessors | expand |
On Wed, Jul 06, 2022 at 04:38:08PM +0200, mwilck@suse.com wrote: > 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. For all except 8/14: Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com> > > 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 | 120 +++---- > 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, 751 insertions(+), 253 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
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. 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 | 120 +++---- 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, 751 insertions(+), 253 deletions(-) create mode 100644 tests/sysfs.c