Re: [dm-devel] [PATCH 00/14] multipath: fixes for sysfs accessors

[Benjamin Marzinski <bmarzins@redhat.com>]

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