* [PATCH v2 01/14] libmultipath: reduce log level of directio messages
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-26 17:41 ` [PATCH v2 02/14] libmultipath: directio: don't reset ct->running after io_cancel() mwilck
` (12 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck
From: Martin Wilck <mwilck@suse.com>
The directio checker logs too much at verbosity 3.
Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/checkers/directio.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libmultipath/checkers/directio.c b/libmultipath/checkers/directio.c
index 2f3ece0..25b2383 100644
--- a/libmultipath/checkers/directio.c
+++ b/libmultipath/checkers/directio.c
@@ -266,7 +266,7 @@ get_events(struct aio_group *aio_grp, struct timespec *timeout)
for (i = 0; i < nr; i++) {
struct async_req *req = container_of(events[i].obj, struct async_req, io);
- LOG(3, "io finished %lu/%lu", events[i].res,
+ LOG(4, "io finished %lu/%lu", events[i].res,
events[i].res2);
/* got an orphaned request */
@@ -283,7 +283,7 @@ get_events(struct aio_group *aio_grp, struct timespec *timeout)
} while (nr == 128); /* assume there are more events and try again */
if (nr < 0)
- LOG(3, "async io getevents returned %i (errno=%s)",
+ LOG(4, "async io getevents returned %i (errno=%s)",
nr, strerror(errno));
return got_events;
@@ -315,7 +315,7 @@ check_state(int fd, struct directio_context *ct, int sync, int timeout_secs)
} else {
struct iocb *ios[1] = { &ct->req->io };
- LOG(3, "starting new request");
+ LOG(4, "starting new request");
memset(&ct->req->io, 0, sizeof(struct iocb));
io_prep_pread(&ct->req->io, fd, ct->req->buf,
ct->req->blksize, 0);
@@ -360,7 +360,7 @@ check_state(int fd, struct directio_context *ct, int sync, int timeout_secs)
ct->running = 0;
rc = PATH_DOWN;
} else {
- LOG(3, "async io pending");
+ LOG(4, "async io pending");
rc = PATH_PENDING;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 02/14] libmultipath: directio: don't reset ct->running after io_cancel()
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
2023-10-26 17:41 ` [PATCH v2 01/14] libmultipath: reduce log level of directio messages mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-27 18:54 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 03/14] libmultipath: directio: fix error handling mwilck
` (11 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Martin Wilck, Hannes Reinecke
From: Martin Wilck <mwilck@suse.com>
io_cancel() never succeeds, and even if it did, io_getevents() must
still be called to reclaim the IO resources from the kernel.
Don't pretend the opposite by resetting ct->running, or freeing the
memory, before io_getevents() has indicated that the request is finished.
In the test code, don't bother about the return value of __wrap_io_cancel().
Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
libmultipath/checkers/directio.c | 14 ++++----------
tests/directio.c | 27 ++-------------------------
2 files changed, 6 insertions(+), 35 deletions(-)
diff --git a/libmultipath/checkers/directio.c b/libmultipath/checkers/directio.c
index 25b2383..83ab29f 100644
--- a/libmultipath/checkers/directio.c
+++ b/libmultipath/checkers/directio.c
@@ -232,15 +232,15 @@ void libcheck_free (struct checker * c)
}
}
- if (ct->running &&
- (ct->req->state != PATH_PENDING ||
- io_cancel(ct->aio_grp->ioctx, &ct->req->io, &event) == 0))
+ if (ct->running && ct->req->state != PATH_PENDING)
ct->running = 0;
if (!ct->running) {
free(ct->req->buf);
free(ct->req);
ct->aio_grp->holders--;
} else {
+ /* Currently a no-op */
+ io_cancel(ct->aio_grp->ioctx, &ct->req->io, &event);
ct->req->state = PATH_REMOVED;
list_add(&ct->req->node, &ct->aio_grp->orphans);
check_orphaned_group(ct->aio_grp);
@@ -351,13 +351,7 @@ check_state(int fd, struct directio_context *ct, int sync, int timeout_secs)
LOG(3, "abort check on timeout");
- r = io_cancel(ct->aio_grp->ioctx, &ct->req->io, &event);
- /*
- * Only reset ct->running if we really
- * could abort the pending I/O
- */
- if (!r)
- ct->running = 0;
+ io_cancel(ct->aio_grp->ioctx, &ct->req->io, &event);
rc = PATH_DOWN;
} else {
LOG(4, "async io pending");
diff --git a/tests/directio.c b/tests/directio.c
index db9643e..5201d21 100644
--- a/tests/directio.c
+++ b/tests/directio.c
@@ -141,10 +141,9 @@ int __real_io_cancel(io_context_t ctx, struct iocb *iocb, struct io_event *evt);
int __wrap_io_cancel(io_context_t ctx, struct iocb *iocb, struct io_event *evt)
{
#ifdef DIO_TEST_DEV
- mock_type(int);
return __real_io_cancel(ctx, iocb, evt);
#else
- return mock_type(int);
+ return 0;
#endif
}
@@ -439,14 +438,8 @@ static void test_check_state_timeout(void **state)
do_libcheck_init(&c, 4096, NULL);
aio_grp = get_aio_grp(&c);
return_io_getevents_none();
- will_return(__wrap_io_cancel, 0);
do_check_state(&c, 1, 30, PATH_DOWN);
check_aio_grp(aio_grp, 1, 0);
-#ifdef DIO_TEST_DEV
- /* io_cancel will return negative value on timeout, so it happens again
- * when freeing the checker */
- will_return(__wrap_io_cancel, 0);
-#endif
libcheck_free(&c);
do_libcheck_reset(1);
}
@@ -468,12 +461,8 @@ static void test_check_state_async_timeout(void **state)
return_io_getevents_none();
do_check_state(&c, 0, 3, PATH_PENDING);
return_io_getevents_none();
- will_return(__wrap_io_cancel, 0);
do_check_state(&c, 0, 3, PATH_DOWN);
check_aio_grp(aio_grp, 1, 0);
-#ifdef DIO_TEST_DEV
- will_return(__wrap_io_cancel, 0);
-#endif
libcheck_free(&c);
do_libcheck_reset(1);
}
@@ -501,13 +490,8 @@ static void test_free_with_pending(void **state)
check_aio_grp(aio_grp, 2, 0);
libcheck_free(&c[0]);
check_aio_grp(aio_grp, 1, 0);
- will_return(__wrap_io_cancel, 0);
libcheck_free(&c[1]);
-#ifdef DIO_TEST_DEV
- check_aio_grp(aio_grp, 1, 1); /* real cancel doesn't remove request */
-#else
- check_aio_grp(aio_grp, 0, 0);
-#endif
+ check_aio_grp(aio_grp, 1, 1); /* cancel doesn't remove request */
do_libcheck_reset(1);
}
@@ -533,7 +517,6 @@ static void test_orphaned_aio_group(void **state)
assert_int_equal(i, 1);
for (i = 0; i < AIO_GROUP_SIZE; i++) {
assert_true(is_checker_running(&c[i]));
- will_return(__wrap_io_cancel, -1);
if (i == AIO_GROUP_SIZE - 1) {
/* remove the orphaned group and create a new one */
will_return(__wrap_io_destroy, 0);
@@ -559,12 +542,10 @@ static void test_timeout_cancel_failed(void **state)
do_libcheck_init(&c[i], 4096, &reqs[i]);
aio_grp = get_aio_grp(c);
return_io_getevents_none();
- will_return(__wrap_io_cancel, -1);
do_check_state(&c[0], 1, 30, PATH_DOWN);
assert_true(is_checker_running(&c[0]));
check_aio_grp(aio_grp, 2, 0);
return_io_getevents_none();
- will_return(__wrap_io_cancel, -1);
do_check_state(&c[0], 1, 30, PATH_DOWN);
assert_true(is_checker_running(&c[0]));
return_io_getevents_nr(NULL, 1, &reqs[0], &res[0]);
@@ -600,7 +581,6 @@ static void test_async_timeout_cancel_failed(void **state)
return_io_getevents_none();
do_check_state(&c[1], 0, 2, PATH_PENDING);
return_io_getevents_none();
- will_return(__wrap_io_cancel, -1);
do_check_state(&c[0], 0, 2, PATH_DOWN);
#ifndef DIO_TEST_DEV
/* can't pick which even gets returned on real devices */
@@ -608,7 +588,6 @@ static void test_async_timeout_cancel_failed(void **state)
do_check_state(&c[1], 0, 2, PATH_UP);
#endif
return_io_getevents_none();
- will_return(__wrap_io_cancel, -1);
do_check_state(&c[0], 0, 2, PATH_DOWN);
assert_true(is_checker_running(&c[0]));
return_io_getevents_nr(NULL, 2, reqs, res);
@@ -637,7 +616,6 @@ static void test_orphan_checker_cleanup(void **state)
aio_grp = get_aio_grp(c);
return_io_getevents_none();
do_check_state(&c[0], 0, 30, PATH_PENDING);
- will_return(__wrap_io_cancel, -1);
check_aio_grp(aio_grp, 2, 0);
libcheck_free(&c[0]);
check_aio_grp(aio_grp, 2, 1);
@@ -662,7 +640,6 @@ static void test_orphan_reset_cleanup(void **state)
orphan_aio_grp = get_aio_grp(&c);
return_io_getevents_none();
do_check_state(&c, 0, 30, PATH_PENDING);
- will_return(__wrap_io_cancel, -1);
check_aio_grp(orphan_aio_grp, 1, 0);
libcheck_free(&c);
check_aio_grp(orphan_aio_grp, 1, 1);
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 02/14] libmultipath: directio: don't reset ct->running after io_cancel()
2023-10-26 17:41 ` [PATCH v2 02/14] libmultipath: directio: don't reset ct->running after io_cancel() mwilck
@ 2023-10-27 18:54 ` Benjamin Marzinski
0 siblings, 0 replies; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 18:54 UTC (permalink / raw)
To: mwilck; +Cc: Christophe Varoqui, dm-devel, Hannes Reinecke
On Thu, Oct 26, 2023 at 07:41:41PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> io_cancel() never succeeds, and even if it did, io_getevents() must
> still be called to reclaim the IO resources from the kernel.
> Don't pretend the opposite by resetting ct->running, or freeing the
> memory, before io_getevents() has indicated that the request is finished.
>
> In the test code, don't bother about the return value of __wrap_io_cancel().
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
> libmultipath/checkers/directio.c | 14 ++++----------
> tests/directio.c | 27 ++-------------------------
> 2 files changed, 6 insertions(+), 35 deletions(-)
>
> diff --git a/libmultipath/checkers/directio.c b/libmultipath/checkers/directio.c
> index 25b2383..83ab29f 100644
> --- a/libmultipath/checkers/directio.c
> +++ b/libmultipath/checkers/directio.c
> @@ -232,15 +232,15 @@ void libcheck_free (struct checker * c)
> }
> }
>
> - if (ct->running &&
> - (ct->req->state != PATH_PENDING ||
> - io_cancel(ct->aio_grp->ioctx, &ct->req->io, &event) == 0))
> + if (ct->running && ct->req->state != PATH_PENDING)
> ct->running = 0;
> if (!ct->running) {
> free(ct->req->buf);
> free(ct->req);
> ct->aio_grp->holders--;
> } else {
> + /* Currently a no-op */
> + io_cancel(ct->aio_grp->ioctx, &ct->req->io, &event);
> ct->req->state = PATH_REMOVED;
> list_add(&ct->req->node, &ct->aio_grp->orphans);
> check_orphaned_group(ct->aio_grp);
> @@ -351,13 +351,7 @@ check_state(int fd, struct directio_context *ct, int sync, int timeout_secs)
>
> LOG(3, "abort check on timeout");
>
> - r = io_cancel(ct->aio_grp->ioctx, &ct->req->io, &event);
> - /*
> - * Only reset ct->running if we really
> - * could abort the pending I/O
> - */
> - if (!r)
> - ct->running = 0;
> + io_cancel(ct->aio_grp->ioctx, &ct->req->io, &event);
> rc = PATH_DOWN;
> } else {
> LOG(4, "async io pending");
> diff --git a/tests/directio.c b/tests/directio.c
> index db9643e..5201d21 100644
> --- a/tests/directio.c
> +++ b/tests/directio.c
> @@ -141,10 +141,9 @@ int __real_io_cancel(io_context_t ctx, struct iocb *iocb, struct io_event *evt);
> int __wrap_io_cancel(io_context_t ctx, struct iocb *iocb, struct io_event *evt)
> {
> #ifdef DIO_TEST_DEV
> - mock_type(int);
> return __real_io_cancel(ctx, iocb, evt);
> #else
> - return mock_type(int);
> + return 0;
> #endif
> }
>
> @@ -439,14 +438,8 @@ static void test_check_state_timeout(void **state)
> do_libcheck_init(&c, 4096, NULL);
> aio_grp = get_aio_grp(&c);
> return_io_getevents_none();
> - will_return(__wrap_io_cancel, 0);
> do_check_state(&c, 1, 30, PATH_DOWN);
> check_aio_grp(aio_grp, 1, 0);
> -#ifdef DIO_TEST_DEV
> - /* io_cancel will return negative value on timeout, so it happens again
> - * when freeing the checker */
> - will_return(__wrap_io_cancel, 0);
> -#endif
> libcheck_free(&c);
> do_libcheck_reset(1);
> }
> @@ -468,12 +461,8 @@ static void test_check_state_async_timeout(void **state)
> return_io_getevents_none();
> do_check_state(&c, 0, 3, PATH_PENDING);
> return_io_getevents_none();
> - will_return(__wrap_io_cancel, 0);
> do_check_state(&c, 0, 3, PATH_DOWN);
> check_aio_grp(aio_grp, 1, 0);
> -#ifdef DIO_TEST_DEV
> - will_return(__wrap_io_cancel, 0);
> -#endif
> libcheck_free(&c);
> do_libcheck_reset(1);
> }
> @@ -501,13 +490,8 @@ static void test_free_with_pending(void **state)
> check_aio_grp(aio_grp, 2, 0);
> libcheck_free(&c[0]);
> check_aio_grp(aio_grp, 1, 0);
> - will_return(__wrap_io_cancel, 0);
> libcheck_free(&c[1]);
> -#ifdef DIO_TEST_DEV
> - check_aio_grp(aio_grp, 1, 1); /* real cancel doesn't remove request */
> -#else
> - check_aio_grp(aio_grp, 0, 0);
> -#endif
> + check_aio_grp(aio_grp, 1, 1); /* cancel doesn't remove request */
> do_libcheck_reset(1);
> }
>
> @@ -533,7 +517,6 @@ static void test_orphaned_aio_group(void **state)
> assert_int_equal(i, 1);
> for (i = 0; i < AIO_GROUP_SIZE; i++) {
> assert_true(is_checker_running(&c[i]));
> - will_return(__wrap_io_cancel, -1);
> if (i == AIO_GROUP_SIZE - 1) {
> /* remove the orphaned group and create a new one */
> will_return(__wrap_io_destroy, 0);
> @@ -559,12 +542,10 @@ static void test_timeout_cancel_failed(void **state)
> do_libcheck_init(&c[i], 4096, &reqs[i]);
> aio_grp = get_aio_grp(c);
> return_io_getevents_none();
> - will_return(__wrap_io_cancel, -1);
> do_check_state(&c[0], 1, 30, PATH_DOWN);
> assert_true(is_checker_running(&c[0]));
> check_aio_grp(aio_grp, 2, 0);
> return_io_getevents_none();
> - will_return(__wrap_io_cancel, -1);
> do_check_state(&c[0], 1, 30, PATH_DOWN);
> assert_true(is_checker_running(&c[0]));
> return_io_getevents_nr(NULL, 1, &reqs[0], &res[0]);
> @@ -600,7 +581,6 @@ static void test_async_timeout_cancel_failed(void **state)
> return_io_getevents_none();
> do_check_state(&c[1], 0, 2, PATH_PENDING);
> return_io_getevents_none();
> - will_return(__wrap_io_cancel, -1);
> do_check_state(&c[0], 0, 2, PATH_DOWN);
> #ifndef DIO_TEST_DEV
> /* can't pick which even gets returned on real devices */
> @@ -608,7 +588,6 @@ static void test_async_timeout_cancel_failed(void **state)
> do_check_state(&c[1], 0, 2, PATH_UP);
> #endif
> return_io_getevents_none();
> - will_return(__wrap_io_cancel, -1);
> do_check_state(&c[0], 0, 2, PATH_DOWN);
> assert_true(is_checker_running(&c[0]));
> return_io_getevents_nr(NULL, 2, reqs, res);
> @@ -637,7 +616,6 @@ static void test_orphan_checker_cleanup(void **state)
> aio_grp = get_aio_grp(c);
> return_io_getevents_none();
> do_check_state(&c[0], 0, 30, PATH_PENDING);
> - will_return(__wrap_io_cancel, -1);
> check_aio_grp(aio_grp, 2, 0);
> libcheck_free(&c[0]);
> check_aio_grp(aio_grp, 2, 1);
> @@ -662,7 +640,6 @@ static void test_orphan_reset_cleanup(void **state)
> orphan_aio_grp = get_aio_grp(&c);
> return_io_getevents_none();
> do_check_state(&c, 0, 30, PATH_PENDING);
> - will_return(__wrap_io_cancel, -1);
> check_aio_grp(orphan_aio_grp, 1, 0);
> libcheck_free(&c);
> check_aio_grp(orphan_aio_grp, 1, 1);
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 03/14] libmultipath: directio: fix error handling
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
2023-10-26 17:41 ` [PATCH v2 01/14] libmultipath: reduce log level of directio messages mwilck
2023-10-26 17:41 ` [PATCH v2 02/14] libmultipath: directio: don't reset ct->running after io_cancel() mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-27 18:54 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 04/14] libmultipath: io_err_stat: don't free aio memory before completion mwilck
` (10 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck
From: Martin Wilck <mwilck@suse.com>
libaio uses a different error return convention than glibc. The error code is
not returned in errno, but as the negated return value of the function.
Adapt the directio checker code.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/checkers/directio.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/libmultipath/checkers/directio.c b/libmultipath/checkers/directio.c
index 83ab29f..12b8be4 100644
--- a/libmultipath/checkers/directio.c
+++ b/libmultipath/checkers/directio.c
@@ -70,6 +70,7 @@ static struct aio_group *
add_aio_group(void)
{
struct aio_group *aio_grp;
+ int rc;
aio_grp = malloc(sizeof(struct aio_group));
if (!aio_grp)
@@ -77,9 +78,9 @@ add_aio_group(void)
memset(aio_grp, 0, sizeof(struct aio_group));
INIT_LIST_HEAD(&aio_grp->orphans);
- if (io_setup(AIO_GROUP_SIZE, &aio_grp->ioctx) != 0) {
+ if ((rc = io_setup(AIO_GROUP_SIZE, &aio_grp->ioctx)) != 0) {
LOG(1, "io_setup failed");
- if (errno == EAGAIN)
+ if (rc == -EAGAIN)
LOG(1, "global number of io events too small. Increase fs.aio-max-nr with sysctl");
free(aio_grp);
return NULL;
@@ -259,7 +260,6 @@ get_events(struct aio_group *aio_grp, struct timespec *timeout)
struct timespec *timep = timeout;
do {
- errno = 0;
nr = io_getevents(aio_grp->ioctx, 1, 128, events, timep);
got_events |= (nr > 0);
@@ -283,8 +283,7 @@ get_events(struct aio_group *aio_grp, struct timespec *timeout)
} while (nr == 128); /* assume there are more events and try again */
if (nr < 0)
- LOG(4, "async io getevents returned %i (errno=%s)",
- nr, strerror(errno));
+ LOG(4, "async io getevents returned %s", strerror(-nr));
return got_events;
}
@@ -320,8 +319,8 @@ check_state(int fd, struct directio_context *ct, int sync, int timeout_secs)
io_prep_pread(&ct->req->io, fd, ct->req->buf,
ct->req->blksize, 0);
ct->req->state = PATH_PENDING;
- if (io_submit(ct->aio_grp->ioctx, 1, ios) != 1) {
- LOG(3, "io_submit error %i", errno);
+ if ((rc = io_submit(ct->aio_grp->ioctx, 1, ios)) != 1) {
+ LOG(3, "io_submit error %i", -rc);
return PATH_UNCHECKED;
}
}
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 03/14] libmultipath: directio: fix error handling
2023-10-26 17:41 ` [PATCH v2 03/14] libmultipath: directio: fix error handling mwilck
@ 2023-10-27 18:54 ` Benjamin Marzinski
0 siblings, 0 replies; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 18:54 UTC (permalink / raw)
To: mwilck; +Cc: Christophe Varoqui, dm-devel
On Thu, Oct 26, 2023 at 07:41:42PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> libaio uses a different error return convention than glibc. The error code is
> not returned in errno, but as the negated return value of the function.
> Adapt the directio checker code.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
> libmultipath/checkers/directio.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/libmultipath/checkers/directio.c b/libmultipath/checkers/directio.c
> index 83ab29f..12b8be4 100644
> --- a/libmultipath/checkers/directio.c
> +++ b/libmultipath/checkers/directio.c
> @@ -70,6 +70,7 @@ static struct aio_group *
> add_aio_group(void)
> {
> struct aio_group *aio_grp;
> + int rc;
>
> aio_grp = malloc(sizeof(struct aio_group));
> if (!aio_grp)
> @@ -77,9 +78,9 @@ add_aio_group(void)
> memset(aio_grp, 0, sizeof(struct aio_group));
> INIT_LIST_HEAD(&aio_grp->orphans);
>
> - if (io_setup(AIO_GROUP_SIZE, &aio_grp->ioctx) != 0) {
> + if ((rc = io_setup(AIO_GROUP_SIZE, &aio_grp->ioctx)) != 0) {
> LOG(1, "io_setup failed");
> - if (errno == EAGAIN)
> + if (rc == -EAGAIN)
> LOG(1, "global number of io events too small. Increase fs.aio-max-nr with sysctl");
> free(aio_grp);
> return NULL;
> @@ -259,7 +260,6 @@ get_events(struct aio_group *aio_grp, struct timespec *timeout)
> struct timespec *timep = timeout;
>
> do {
> - errno = 0;
> nr = io_getevents(aio_grp->ioctx, 1, 128, events, timep);
> got_events |= (nr > 0);
>
> @@ -283,8 +283,7 @@ get_events(struct aio_group *aio_grp, struct timespec *timeout)
> } while (nr == 128); /* assume there are more events and try again */
>
> if (nr < 0)
> - LOG(4, "async io getevents returned %i (errno=%s)",
> - nr, strerror(errno));
> + LOG(4, "async io getevents returned %s", strerror(-nr));
>
> return got_events;
> }
> @@ -320,8 +319,8 @@ check_state(int fd, struct directio_context *ct, int sync, int timeout_secs)
> io_prep_pread(&ct->req->io, fd, ct->req->buf,
> ct->req->blksize, 0);
> ct->req->state = PATH_PENDING;
> - if (io_submit(ct->aio_grp->ioctx, 1, ios) != 1) {
> - LOG(3, "io_submit error %i", errno);
> + if ((rc = io_submit(ct->aio_grp->ioctx, 1, ios)) != 1) {
> + LOG(3, "io_submit error %i", -rc);
> return PATH_UNCHECKED;
> }
> }
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 04/14] libmultipath: io_err_stat: don't free aio memory before completion
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
` (2 preceding siblings ...)
2023-10-26 17:41 ` [PATCH v2 03/14] libmultipath: directio: fix error handling mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-27 18:55 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 05/14] libmultipath: io_err_stat: call io_destroy() inside free_io_err_pathvec() mwilck
` (9 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Martin Wilck, Li Xiao Keng, Miao Guanqin, Guan Junxiong
From: Martin Wilck <mwilck@suse.com>
It is wrong to assume that aio data structures can be reused or freed
after io_cancel(). io_cancel() will almost always return -EINPROGRESS,
anyway. Use the io_starttime field to indicate whether an io event
has been completed by the kernel. Make sure no in-flight buffers are freed.
Fixes https://github.com/opensvc/multipath-tools/issues/73.
Signed-off-by: Martin Wilck <mwilck@suse.com>
Cc: Li Xiao Keng <lixiaokeng@huawei.com>
Cc: Miao Guanqin <miaoguanqin@huawei.com>
Cc: Guan Junxiong <guanjunxiong@huawei.com>
---
libmultipath/io_err_stat.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
index dc1c252..c474c34 100644
--- a/libmultipath/io_err_stat.c
+++ b/libmultipath/io_err_stat.c
@@ -111,10 +111,14 @@ static int init_each_dio_ctx(struct dio_ctx *ct, int blksize,
return 0;
}
-static void deinit_each_dio_ctx(struct dio_ctx *ct)
+static int deinit_each_dio_ctx(struct dio_ctx *ct)
{
- if (ct->buf)
- free(ct->buf);
+ if (!ct->buf)
+ return 0;
+ if (ct->io_starttime.tv_sec != 0 || ct->io_starttime.tv_nsec != 0)
+ return 1;
+ free(ct->buf);
+ return 0;
}
static int setup_directio_ctx(struct io_err_stat_path *p)
@@ -164,6 +168,7 @@ fail_close:
static void free_io_err_stat_path(struct io_err_stat_path *p)
{
int i;
+ int inflight = 0;
if (!p)
return;
@@ -173,8 +178,13 @@ static void free_io_err_stat_path(struct io_err_stat_path *p)
cancel_inflight_io(p);
for (i = 0; i < CONCUR_NR_EVENT; i++)
- deinit_each_dio_ctx(p->dio_ctx_array + i);
- free(p->dio_ctx_array);
+ inflight += deinit_each_dio_ctx(p->dio_ctx_array + i);
+
+ if (!inflight)
+ free(p->dio_ctx_array);
+ else
+ io_err_stat_log(2, "%s: can't free aio space of %s, %d IOs in flight",
+ __func__, p->devname, inflight);
if (p->fd > 0)
close(p->fd);
@@ -503,7 +513,7 @@ static int try_to_cancel_timeout_io(struct dio_ctx *ct, struct timespec *t,
int rc = PATH_UNCHECKED;
int r;
- if (ct->io_starttime.tv_sec == 0)
+ if (ct->io_starttime.tv_sec == 0 && ct->io_starttime.tv_nsec == 0)
return rc;
timespecsub(t, &ct->io_starttime, &difftime);
if (difftime.tv_sec > IOTIMEOUT_SEC) {
@@ -514,8 +524,6 @@ static int try_to_cancel_timeout_io(struct dio_ctx *ct, struct timespec *t,
if (r)
io_err_stat_log(5, "%s: io_cancel error %i",
dev, errno);
- ct->io_starttime.tv_sec = 0;
- ct->io_starttime.tv_nsec = 0;
rc = PATH_TIMEOUT;
} else {
rc = PATH_PENDING;
@@ -559,8 +567,6 @@ static void cancel_inflight_io(struct io_err_stat_path *pp)
if (r)
io_err_stat_log(5, "%s: io_cancel error %d, %i",
pp->devname, r, errno);
- ct->io_starttime.tv_sec = 0;
- ct->io_starttime.tv_nsec = 0;
}
}
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 04/14] libmultipath: io_err_stat: don't free aio memory before completion
2023-10-26 17:41 ` [PATCH v2 04/14] libmultipath: io_err_stat: don't free aio memory before completion mwilck
@ 2023-10-27 18:55 ` Benjamin Marzinski
0 siblings, 0 replies; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 18:55 UTC (permalink / raw)
To: mwilck
Cc: Christophe Varoqui, dm-devel, Li Xiao Keng, Miao Guanqin,
Guan Junxiong
On Thu, Oct 26, 2023 at 07:41:43PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> It is wrong to assume that aio data structures can be reused or freed
> after io_cancel(). io_cancel() will almost always return -EINPROGRESS,
> anyway. Use the io_starttime field to indicate whether an io event
> has been completed by the kernel. Make sure no in-flight buffers are freed.
>
> Fixes https://github.com/opensvc/multipath-tools/issues/73.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Cc: Li Xiao Keng <lixiaokeng@huawei.com>
> Cc: Miao Guanqin <miaoguanqin@huawei.com>
> Cc: Guan Junxiong <guanjunxiong@huawei.com>
> ---
> libmultipath/io_err_stat.c | 26 ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
> index dc1c252..c474c34 100644
> --- a/libmultipath/io_err_stat.c
> +++ b/libmultipath/io_err_stat.c
> @@ -111,10 +111,14 @@ static int init_each_dio_ctx(struct dio_ctx *ct, int blksize,
> return 0;
> }
>
> -static void deinit_each_dio_ctx(struct dio_ctx *ct)
> +static int deinit_each_dio_ctx(struct dio_ctx *ct)
> {
> - if (ct->buf)
> - free(ct->buf);
> + if (!ct->buf)
> + return 0;
> + if (ct->io_starttime.tv_sec != 0 || ct->io_starttime.tv_nsec != 0)
> + return 1;
> + free(ct->buf);
> + return 0;
> }
>
> static int setup_directio_ctx(struct io_err_stat_path *p)
> @@ -164,6 +168,7 @@ fail_close:
> static void free_io_err_stat_path(struct io_err_stat_path *p)
> {
> int i;
> + int inflight = 0;
>
> if (!p)
> return;
> @@ -173,8 +178,13 @@ static void free_io_err_stat_path(struct io_err_stat_path *p)
> cancel_inflight_io(p);
>
> for (i = 0; i < CONCUR_NR_EVENT; i++)
> - deinit_each_dio_ctx(p->dio_ctx_array + i);
> - free(p->dio_ctx_array);
> + inflight += deinit_each_dio_ctx(p->dio_ctx_array + i);
> +
> + if (!inflight)
> + free(p->dio_ctx_array);
> + else
> + io_err_stat_log(2, "%s: can't free aio space of %s, %d IOs in flight",
> + __func__, p->devname, inflight);
>
> if (p->fd > 0)
> close(p->fd);
> @@ -503,7 +513,7 @@ static int try_to_cancel_timeout_io(struct dio_ctx *ct, struct timespec *t,
> int rc = PATH_UNCHECKED;
> int r;
>
> - if (ct->io_starttime.tv_sec == 0)
> + if (ct->io_starttime.tv_sec == 0 && ct->io_starttime.tv_nsec == 0)
> return rc;
> timespecsub(t, &ct->io_starttime, &difftime);
> if (difftime.tv_sec > IOTIMEOUT_SEC) {
> @@ -514,8 +524,6 @@ static int try_to_cancel_timeout_io(struct dio_ctx *ct, struct timespec *t,
> if (r)
> io_err_stat_log(5, "%s: io_cancel error %i",
> dev, errno);
> - ct->io_starttime.tv_sec = 0;
> - ct->io_starttime.tv_nsec = 0;
> rc = PATH_TIMEOUT;
> } else {
> rc = PATH_PENDING;
> @@ -559,8 +567,6 @@ static void cancel_inflight_io(struct io_err_stat_path *pp)
> if (r)
> io_err_stat_log(5, "%s: io_cancel error %d, %i",
> pp->devname, r, errno);
> - ct->io_starttime.tv_sec = 0;
> - ct->io_starttime.tv_nsec = 0;
> }
> }
>
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 05/14] libmultipath: io_err_stat: call io_destroy() inside free_io_err_pathvec()
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
` (3 preceding siblings ...)
2023-10-26 17:41 ` [PATCH v2 04/14] libmultipath: io_err_stat: don't free aio memory before completion mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-27 18:55 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 06/14] libmultipath: io_err_stat: use higher number of aio slots mwilck
` (8 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Martin Wilck, Li Xiao Keng, Miao Guanqin, Guan Junxiong
From: Martin Wilck <mwilck@suse.com>
Memory used by aio iocbs must not be freed before either all iocbs
have finished, or io_destroy() has been called (which will wait until
all iocbs have finished).
Don't call cancel_inflight_io() from free_io_err_stat_path(). Rather,
cancel directly from free_io_err_pathvec(). This way we make sure that
we can't call io_cancel() with an already destroyed ioctx, and that
we don't free memory of in-flight requests.
The other callers of free_io_err_stat_path() also don't need to call
cancel_inflight_io(). In service_paths(), where free_io_err_stat_path()
it is called for paths on which all IO has either finished or timed out,
hanging requests will already have been cancelled in the
try_to_cancel_timeout_io() code path (note that total_time is at least
2 * IO_TIMEOUT_SEC). In the failure case of enqueue_io_err_stat_by_path(), no
IO has been submitted yet.
Signed-off-by: Martin Wilck <mwilck@suse.com>
Cc: Li Xiao Keng <lixiaokeng@huawei.com>
Cc: Miao Guanqin <miaoguanqin@huawei.com>
Cc: Guan Junxiong <guanjunxiong@huawei.com>
---
libmultipath/io_err_stat.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
index c474c34..3f32e32 100644
--- a/libmultipath/io_err_stat.c
+++ b/libmultipath/io_err_stat.c
@@ -175,8 +175,6 @@ static void free_io_err_stat_path(struct io_err_stat_path *p)
if (!p->dio_ctx_array)
goto free_path;
- cancel_inflight_io(p);
-
for (i = 0; i < CONCUR_NR_EVENT; i++)
inflight += deinit_each_dio_ctx(p->dio_ctx_array + i);
@@ -221,6 +219,15 @@ static void free_io_err_pathvec(void)
pthread_cleanup_push(cleanup_mutex, &io_err_pathvec_lock);
if (!io_err_pathvec)
goto out;
+
+ /* io_cancel() is a noop, but maybe in the future it won't be */
+ vector_foreach_slot(io_err_pathvec, path, i) {
+ if (path && path->dio_ctx_array)
+ cancel_inflight_io(path);
+ }
+
+ /* This blocks until all I/O is finished */
+ io_destroy(ioctx);
vector_foreach_slot(io_err_pathvec, path, i)
free_io_err_stat_path(path);
vector_free(io_err_pathvec);
@@ -752,5 +759,4 @@ void stop_io_err_stat_thread(void)
pthread_join(io_err_stat_thr, NULL);
free_io_err_pathvec();
- io_destroy(ioctx);
}
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 05/14] libmultipath: io_err_stat: call io_destroy() inside free_io_err_pathvec()
2023-10-26 17:41 ` [PATCH v2 05/14] libmultipath: io_err_stat: call io_destroy() inside free_io_err_pathvec() mwilck
@ 2023-10-27 18:55 ` Benjamin Marzinski
0 siblings, 0 replies; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 18:55 UTC (permalink / raw)
To: mwilck
Cc: Christophe Varoqui, dm-devel, Li Xiao Keng, Miao Guanqin,
Guan Junxiong
On Thu, Oct 26, 2023 at 07:41:44PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> Memory used by aio iocbs must not be freed before either all iocbs
> have finished, or io_destroy() has been called (which will wait until
> all iocbs have finished).
>
> Don't call cancel_inflight_io() from free_io_err_stat_path(). Rather,
> cancel directly from free_io_err_pathvec(). This way we make sure that
> we can't call io_cancel() with an already destroyed ioctx, and that
> we don't free memory of in-flight requests.
>
> The other callers of free_io_err_stat_path() also don't need to call
> cancel_inflight_io(). In service_paths(), where free_io_err_stat_path()
> it is called for paths on which all IO has either finished or timed out,
> hanging requests will already have been cancelled in the
> try_to_cancel_timeout_io() code path (note that total_time is at least
> 2 * IO_TIMEOUT_SEC). In the failure case of enqueue_io_err_stat_by_path(), no
> IO has been submitted yet.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Cc: Li Xiao Keng <lixiaokeng@huawei.com>
> Cc: Miao Guanqin <miaoguanqin@huawei.com>
> Cc: Guan Junxiong <guanjunxiong@huawei.com>
> ---
> libmultipath/io_err_stat.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
> index c474c34..3f32e32 100644
> --- a/libmultipath/io_err_stat.c
> +++ b/libmultipath/io_err_stat.c
> @@ -175,8 +175,6 @@ static void free_io_err_stat_path(struct io_err_stat_path *p)
> if (!p->dio_ctx_array)
> goto free_path;
>
> - cancel_inflight_io(p);
> -
> for (i = 0; i < CONCUR_NR_EVENT; i++)
> inflight += deinit_each_dio_ctx(p->dio_ctx_array + i);
>
> @@ -221,6 +219,15 @@ static void free_io_err_pathvec(void)
> pthread_cleanup_push(cleanup_mutex, &io_err_pathvec_lock);
> if (!io_err_pathvec)
> goto out;
> +
> + /* io_cancel() is a noop, but maybe in the future it won't be */
> + vector_foreach_slot(io_err_pathvec, path, i) {
> + if (path && path->dio_ctx_array)
> + cancel_inflight_io(path);
> + }
> +
> + /* This blocks until all I/O is finished */
> + io_destroy(ioctx);
> vector_foreach_slot(io_err_pathvec, path, i)
> free_io_err_stat_path(path);
> vector_free(io_err_pathvec);
> @@ -752,5 +759,4 @@ void stop_io_err_stat_thread(void)
>
> pthread_join(io_err_stat_thr, NULL);
> free_io_err_pathvec();
> - io_destroy(ioctx);
> }
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 06/14] libmultipath: io_err_stat: use higher number of aio slots
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
` (4 preceding siblings ...)
2023-10-26 17:41 ` [PATCH v2 05/14] libmultipath: io_err_stat: call io_destroy() inside free_io_err_pathvec() mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-27 18:56 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 07/14] libmultipath: io_err_stat: fix error handling mwilck
` (7 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Martin Wilck, Li Xiao Keng, Miao Guanqin, Guan Junxiong
From: Martin Wilck <mwilck@suse.com>
Currently the number of iocbs per path to test is the same as the
total number of iocbs in the ioctx. This can easily cause iocb
starvation, in particular if some IOs are hanging. In that case
io_submit() will fail, and some paths under test will use much
less IOs as intended, or in the worst case, none at all.
The total number of iocbs reserved in the kernel should be higher.
With this patch, we will be able to run the marginal path test for
at least NR_IOSTAT_PATHS=32 paths at the same time. This is not an upper
limit, because kernel IOCBs can be reused between paths.
Increase the log levels of io_setup and io_submit to make it sure
we catch problems with this approach.
Signed-off-by: Martin Wilck <mwilck@suse.com>
Cc: Li Xiao Keng <lixiaokeng@huawei.com>
Cc: Miao Guanqin <miaoguanqin@huawei.com>
Cc: Guan Junxiong <guanjunxiong@huawei.com>
---
libmultipath/io_err_stat.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
index 3f32e32..5749003 100644
--- a/libmultipath/io_err_stat.c
+++ b/libmultipath/io_err_stat.c
@@ -38,6 +38,7 @@
#define TIMEOUT_NO_IO_NSEC 10000000 /*10ms = 10000000ns*/
#define FLAKY_PATHFAIL_THRESHOLD 2
#define CONCUR_NR_EVENT 32
+#define NR_IOSTAT_PATHS 32
#define PATH_IO_ERR_IN_CHECKING -1
#define PATH_IO_ERR_WAITING_TO_CHECK -2
@@ -477,7 +478,7 @@ static int send_each_async_io(struct dio_ctx *ct, int fd, char *dev)
get_monotonic_time(&ct->io_starttime);
io_prep_pread(&ct->io, fd, ct->buf, ct->blksize, 0);
if (io_submit(ioctx, 1, ios) != 1) {
- io_err_stat_log(5, "%s: io_submit error %i",
+ io_err_stat_log(2, "%s: io_submit error %i",
dev, errno);
return rc;
}
@@ -703,8 +704,8 @@ int start_io_err_stat_thread(void *data)
if (uatomic_read(&io_err_thread_running) == 1)
return 0;
- if (io_setup(CONCUR_NR_EVENT, &ioctx) != 0) {
- io_err_stat_log(4, "io_setup failed");
+ if (io_setup(CONCUR_NR_EVENT * NR_IOSTAT_PATHS, &ioctx) != 0) {
+ io_err_stat_log(1, "io_setup failed - increase /proc/sys/fs/aio-nr ?");
return 1;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 06/14] libmultipath: io_err_stat: use higher number of aio slots
2023-10-26 17:41 ` [PATCH v2 06/14] libmultipath: io_err_stat: use higher number of aio slots mwilck
@ 2023-10-27 18:56 ` Benjamin Marzinski
0 siblings, 0 replies; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 18:56 UTC (permalink / raw)
To: mwilck
Cc: Christophe Varoqui, dm-devel, Li Xiao Keng, Miao Guanqin,
Guan Junxiong
On Thu, Oct 26, 2023 at 07:41:45PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> Currently the number of iocbs per path to test is the same as the
> total number of iocbs in the ioctx. This can easily cause iocb
> starvation, in particular if some IOs are hanging. In that case
> io_submit() will fail, and some paths under test will use much
> less IOs as intended, or in the worst case, none at all.
>
> The total number of iocbs reserved in the kernel should be higher.
> With this patch, we will be able to run the marginal path test for
> at least NR_IOSTAT_PATHS=32 paths at the same time. This is not an upper
> limit, because kernel IOCBs can be reused between paths.
>
> Increase the log levels of io_setup and io_submit to make it sure
> we catch problems with this approach.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Cc: Li Xiao Keng <lixiaokeng@huawei.com>
> Cc: Miao Guanqin <miaoguanqin@huawei.com>
> Cc: Guan Junxiong <guanjunxiong@huawei.com>
> ---
> libmultipath/io_err_stat.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
> index 3f32e32..5749003 100644
> --- a/libmultipath/io_err_stat.c
> +++ b/libmultipath/io_err_stat.c
> @@ -38,6 +38,7 @@
> #define TIMEOUT_NO_IO_NSEC 10000000 /*10ms = 10000000ns*/
> #define FLAKY_PATHFAIL_THRESHOLD 2
> #define CONCUR_NR_EVENT 32
> +#define NR_IOSTAT_PATHS 32
>
> #define PATH_IO_ERR_IN_CHECKING -1
> #define PATH_IO_ERR_WAITING_TO_CHECK -2
> @@ -477,7 +478,7 @@ static int send_each_async_io(struct dio_ctx *ct, int fd, char *dev)
> get_monotonic_time(&ct->io_starttime);
> io_prep_pread(&ct->io, fd, ct->buf, ct->blksize, 0);
> if (io_submit(ioctx, 1, ios) != 1) {
> - io_err_stat_log(5, "%s: io_submit error %i",
> + io_err_stat_log(2, "%s: io_submit error %i",
> dev, errno);
> return rc;
> }
> @@ -703,8 +704,8 @@ int start_io_err_stat_thread(void *data)
> if (uatomic_read(&io_err_thread_running) == 1)
> return 0;
>
> - if (io_setup(CONCUR_NR_EVENT, &ioctx) != 0) {
> - io_err_stat_log(4, "io_setup failed");
> + if (io_setup(CONCUR_NR_EVENT * NR_IOSTAT_PATHS, &ioctx) != 0) {
> + io_err_stat_log(1, "io_setup failed - increase /proc/sys/fs/aio-nr ?");
> return 1;
> }
>
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 07/14] libmultipath: io_err_stat: fix error handling
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
` (5 preceding siblings ...)
2023-10-26 17:41 ` [PATCH v2 06/14] libmultipath: io_err_stat: use higher number of aio slots mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-27 19:09 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 08/14] multipathd.service: require modprobe@dm_multipath.service if available mwilck
` (6 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck
From: Martin Wilck <mwilck@suse.com>
libaio uses a different error return convention than glibc. The error code is
not returned in errno, but as the negated return value of the function.
Adapt the error handling code in io_err_stat.c. Don't print an error message
for failure of io_cancel(), which always returns -EINPRPOGRESS.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/io_err_stat.c | 36 ++++++++++++++++--------------------
1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
index 5749003..1c59445 100644
--- a/libmultipath/io_err_stat.c
+++ b/libmultipath/io_err_stat.c
@@ -19,7 +19,6 @@
#include <sys/ioctl.h>
#include <linux/fs.h>
#include <libaio.h>
-#include <errno.h>
#include <sys/mman.h>
#include <sys/select.h>
@@ -469,7 +468,7 @@ static void end_io_err_stat(struct io_err_stat_path *pp)
static int send_each_async_io(struct dio_ctx *ct, int fd, char *dev)
{
- int rc = -1;
+ int rc;
if (ct->io_starttime.tv_nsec == 0 &&
ct->io_starttime.tv_sec == 0) {
@@ -477,15 +476,15 @@ static int send_each_async_io(struct dio_ctx *ct, int fd, char *dev)
get_monotonic_time(&ct->io_starttime);
io_prep_pread(&ct->io, fd, ct->buf, ct->blksize, 0);
- if (io_submit(ioctx, 1, ios) != 1) {
- io_err_stat_log(2, "%s: io_submit error %i",
- dev, errno);
- return rc;
+ if ((rc = io_submit(ioctx, 1, ios)) != 1) {
+ io_err_stat_log(2, "%s: io_submit error %s",
+ dev, strerror(-rc));
+ return -1;
}
- rc = 0;
+ return 0;
}
- return rc;
+ return -1;
}
static void send_batch_async_ios(struct io_err_stat_path *pp)
@@ -530,8 +529,8 @@ static int try_to_cancel_timeout_io(struct dio_ctx *ct, struct timespec *t,
io_err_stat_log(5, "%s: abort check on timeout", dev);
r = io_cancel(ioctx, ios[0], &event);
if (r)
- io_err_stat_log(5, "%s: io_cancel error %i",
- dev, errno);
+ io_err_stat_log(5, "%s: io_cancel error %s",
+ dev, strerror(-r));
rc = PATH_TIMEOUT;
} else {
rc = PATH_PENDING;
@@ -560,7 +559,7 @@ static void poll_async_io_timeout(void)
static void cancel_inflight_io(struct io_err_stat_path *pp)
{
struct io_event event;
- int i, r;
+ int i;
for (i = 0; i < CONCUR_NR_EVENT; i++) {
struct dio_ctx *ct = pp->dio_ctx_array + i;
@@ -571,10 +570,7 @@ static void cancel_inflight_io(struct io_err_stat_path *pp)
continue;
io_err_stat_log(5, "%s: abort infligh io",
pp->devname);
- r = io_cancel(ioctx, ios[0], &event);
- if (r)
- io_err_stat_log(5, "%s: io_cancel error %d, %i",
- pp->devname, r, errno);
+ io_cancel(ioctx, ios[0], &event);
}
}
@@ -610,12 +606,11 @@ static void process_async_ios_event(int timeout_nsecs, char *dev)
int i, n;
struct timespec timeout = { .tv_nsec = timeout_nsecs };
- errno = 0;
pthread_testcancel();
n = io_getevents(ioctx, 1L, CONCUR_NR_EVENT, events, &timeout);
if (n < 0) {
- io_err_stat_log(3, "%s: async io events returned %d (errno=%s)",
- dev, n, strerror(errno));
+ io_err_stat_log(3, "%s: io_getevents returned %s",
+ dev, strerror(-n));
} else {
for (i = 0; i < n; i++)
handle_async_io_done_event(&events[i]);
@@ -704,8 +699,9 @@ int start_io_err_stat_thread(void *data)
if (uatomic_read(&io_err_thread_running) == 1)
return 0;
- if (io_setup(CONCUR_NR_EVENT * NR_IOSTAT_PATHS, &ioctx) != 0) {
- io_err_stat_log(1, "io_setup failed - increase /proc/sys/fs/aio-nr ?");
+ if ((ret = io_setup(NR_IOSTAT_PATHS * CONCUR_NR_EVENT, &ioctx)) != 0) {
+ io_err_stat_log(1, "io_setup failed: %s, increase /proc/sys/fs/aio-nr ?",
+ strerror(-ret));
return 1;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 07/14] libmultipath: io_err_stat: fix error handling
2023-10-26 17:41 ` [PATCH v2 07/14] libmultipath: io_err_stat: fix error handling mwilck
@ 2023-10-27 19:09 ` Benjamin Marzinski
0 siblings, 0 replies; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 19:09 UTC (permalink / raw)
To: mwilck; +Cc: Christophe Varoqui, dm-devel
On Thu, Oct 26, 2023 at 07:41:46PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> libaio uses a different error return convention than glibc. The error code is
> not returned in errno, but as the negated return value of the function.
> Adapt the error handling code in io_err_stat.c. Don't print an error message
> for failure of io_cancel(), which always returns -EINPRPOGRESS.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
> libmultipath/io_err_stat.c | 36 ++++++++++++++++--------------------
> 1 file changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
> index 5749003..1c59445 100644
> --- a/libmultipath/io_err_stat.c
> +++ b/libmultipath/io_err_stat.c
> @@ -19,7 +19,6 @@
> #include <sys/ioctl.h>
> #include <linux/fs.h>
> #include <libaio.h>
> -#include <errno.h>
> #include <sys/mman.h>
> #include <sys/select.h>
>
> @@ -469,7 +468,7 @@ static void end_io_err_stat(struct io_err_stat_path *pp)
>
> static int send_each_async_io(struct dio_ctx *ct, int fd, char *dev)
> {
> - int rc = -1;
> + int rc;
>
> if (ct->io_starttime.tv_nsec == 0 &&
> ct->io_starttime.tv_sec == 0) {
> @@ -477,15 +476,15 @@ static int send_each_async_io(struct dio_ctx *ct, int fd, char *dev)
>
> get_monotonic_time(&ct->io_starttime);
> io_prep_pread(&ct->io, fd, ct->buf, ct->blksize, 0);
> - if (io_submit(ioctx, 1, ios) != 1) {
> - io_err_stat_log(2, "%s: io_submit error %i",
> - dev, errno);
> - return rc;
> + if ((rc = io_submit(ioctx, 1, ios)) != 1) {
> + io_err_stat_log(2, "%s: io_submit error %s",
> + dev, strerror(-rc));
> + return -1;
> }
> - rc = 0;
> + return 0;
> }
>
> - return rc;
> + return -1;
> }
>
> static void send_batch_async_ios(struct io_err_stat_path *pp)
> @@ -530,8 +529,8 @@ static int try_to_cancel_timeout_io(struct dio_ctx *ct, struct timespec *t,
> io_err_stat_log(5, "%s: abort check on timeout", dev);
> r = io_cancel(ioctx, ios[0], &event);
> if (r)
> - io_err_stat_log(5, "%s: io_cancel error %i",
> - dev, errno);
> + io_err_stat_log(5, "%s: io_cancel error %s",
> + dev, strerror(-r));
> rc = PATH_TIMEOUT;
> } else {
> rc = PATH_PENDING;
> @@ -560,7 +559,7 @@ static void poll_async_io_timeout(void)
> static void cancel_inflight_io(struct io_err_stat_path *pp)
> {
> struct io_event event;
> - int i, r;
> + int i;
>
> for (i = 0; i < CONCUR_NR_EVENT; i++) {
> struct dio_ctx *ct = pp->dio_ctx_array + i;
> @@ -571,10 +570,7 @@ static void cancel_inflight_io(struct io_err_stat_path *pp)
> continue;
> io_err_stat_log(5, "%s: abort infligh io",
> pp->devname);
> - r = io_cancel(ioctx, ios[0], &event);
> - if (r)
> - io_err_stat_log(5, "%s: io_cancel error %d, %i",
> - pp->devname, r, errno);
> + io_cancel(ioctx, ios[0], &event);
> }
> }
>
> @@ -610,12 +606,11 @@ static void process_async_ios_event(int timeout_nsecs, char *dev)
> int i, n;
> struct timespec timeout = { .tv_nsec = timeout_nsecs };
>
> - errno = 0;
> pthread_testcancel();
> n = io_getevents(ioctx, 1L, CONCUR_NR_EVENT, events, &timeout);
> if (n < 0) {
> - io_err_stat_log(3, "%s: async io events returned %d (errno=%s)",
> - dev, n, strerror(errno));
> + io_err_stat_log(3, "%s: io_getevents returned %s",
> + dev, strerror(-n));
> } else {
> for (i = 0; i < n; i++)
> handle_async_io_done_event(&events[i]);
> @@ -704,8 +699,9 @@ int start_io_err_stat_thread(void *data)
> if (uatomic_read(&io_err_thread_running) == 1)
> return 0;
>
> - if (io_setup(CONCUR_NR_EVENT * NR_IOSTAT_PATHS, &ioctx) != 0) {
> - io_err_stat_log(1, "io_setup failed - increase /proc/sys/fs/aio-nr ?");
> + if ((ret = io_setup(NR_IOSTAT_PATHS * CONCUR_NR_EVENT, &ioctx)) != 0) {
> + io_err_stat_log(1, "io_setup failed: %s, increase /proc/sys/fs/aio-nr ?",
> + strerror(-ret));
> return 1;
> }
>
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 08/14] multipathd.service: require modprobe@dm_multipath.service if available
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
` (6 preceding siblings ...)
2023-10-26 17:41 ` [PATCH v2 07/14] libmultipath: io_err_stat: fix error handling mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-27 19:09 ` Benjamin Marzinski
2023-10-27 20:05 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 09/14] libmpathutil: remove systemd_service_enabled() mwilck
` (5 subsequent siblings)
13 siblings, 2 replies; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck
From: Martin Wilck <mwilck@suse.com>
systemd v245 and newer provides the modprobe@.service unit, which
can be used to pull in modules via systemd. This is cleaner than
loading dm-multipath unconditionally via modules-load.d. Use it
if the detected systemd version support it.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
Makefile.inc | 4 +++-
multipath/Makefile | 6 ++++--
multipathd/Makefile | 4 ++--
multipathd/{multipathd.service => multipathd.service.in} | 4 ++--
4 files changed, 11 insertions(+), 7 deletions(-)
rename multipathd/{multipathd.service => multipathd.service.in} (85%)
diff --git a/Makefile.inc b/Makefile.inc
index 04bfa56..8655cba 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -90,6 +90,8 @@ ORIG_LDFLAGS := $(LDFLAGS)
SYSTEMD_CPPFLAGS := $(if $(SYSTEMD),-DUSE_SYSTEMD=$(SYSTEMD))
SYSTEMD_LIBDEPS := $(if $(SYSTEMD),$(if $(shell test $(SYSTEMD) -gt 209 && echo 1),-lsystemd,-lsystemd-daemon))
+MODPROBE_UNIT := $(shell test "0$(SYSTEMD)" -lt 245 2>/dev/null || \
+ echo "modprobe@dm_multipath.service")
OPTFLAGS := -O2 -g $(STACKPROT) --param=ssp-buffer-size=4
WARNFLAGS := -Werror -Wall -Wextra -Wformat=2 $(WFORMATOVERFLOW) -Werror=implicit-int \
@@ -144,4 +146,4 @@ NV_VERSION_SCRIPT = $(DEVLIB:%.so=%-nv.version)
%: %.in
@echo creating $@
- $(Q)sed 's:@CONFIGFILE@:'$(configfile)':g;s:@CONFIGDIR@:'$(configdir)':g;s:@STATE_DIR@:'$(statedir)':g;s:@RUNTIME_DIR@:'$(runtimedir)':g' $< >$@
+ $(Q)sed 's:@CONFIGFILE@:'$(configfile)':g;s:@CONFIGDIR@:'$(configdir)':g;s:@STATE_DIR@:'$(statedir)':g;s:@RUNTIME_DIR@:'$(runtimedir)':g;s/@MODPROBE_UNIT@/'$(MODPROBE_UNIT)'/g' $< >$@
diff --git a/multipath/Makefile b/multipath/Makefile
index 68cb5ce..0efb9b2 100644
--- a/multipath/Makefile
+++ b/multipath/Makefile
@@ -27,14 +27,16 @@ install:
$(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(udevrulesdir)
$(Q)$(INSTALL_PROGRAM) -m 644 11-dm-mpath.rules $(DESTDIR)$(udevrulesdir)
$(Q)$(INSTALL_PROGRAM) -m 644 multipath.rules $(DESTDIR)$(udevrulesdir)/56-multipath.rules
- $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(modulesloaddir)
- $(Q)$(INSTALL_PROGRAM) -m 644 modules-load.conf $(DESTDIR)$(modulesloaddir)/multipath.conf
$(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(tmpfilesdir)
$(Q)$(INSTALL_PROGRAM) -m 644 tmpfiles.conf $(DESTDIR)$(tmpfilesdir)/multipath.conf
$(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(mandir)/man8
$(Q)$(INSTALL_PROGRAM) -m 644 $(EXEC).8 $(DESTDIR)$(mandir)/man8
$(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(mandir)/man5
$(Q)$(INSTALL_PROGRAM) -m 644 $(EXEC).conf.5 $(DESTDIR)$(mandir)/man5
+ $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(modulesloaddir)
+ifeq ($(MODPROBE_UNIT),)
+ $(Q)$(INSTALL_PROGRAM) -m 644 modules-load.conf $(DESTDIR)$(modulesloaddir)/multipath.conf
+endif
ifneq ($(SCSI_DH_MODULES_PRELOAD),)
$(Q)$(INSTALL_PROGRAM) -m 644 scsi_dh.conf $(DESTDIR)$(modulesloaddir)/scsi_dh.conf
$(Q)for _x in $(SCSI_DH_MODULES_PRELOAD); do echo "$$_x"; done \
diff --git a/multipathd/Makefile b/multipathd/Makefile
index 0ba6ecb..997b40c 100644
--- a/multipathd/Makefile
+++ b/multipathd/Makefile
@@ -41,7 +41,7 @@ ifeq ($(FPIN_SUPPORT),1)
OBJS += fpin_handlers.o
endif
-all : $(EXEC) $(CLI) $(MANPAGES)
+all : $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service
$(EXEC): $(OBJS) $(multipathdir)/libmultipath.so $(mpathcmddir)/libmpathcmd.so
@echo building $@ because of $?
@@ -78,7 +78,7 @@ uninstall:
$(Q)$(RM) $(DESTDIR)$(unitdir)/$(EXEC).socket
clean: dep_clean
- $(Q)$(RM) core *.o $(EXEC) $(CLI) $(MANPAGES)
+ $(Q)$(RM) core *.o $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service
include $(wildcard $(OBJS:.o=.d) $(CLI_OBJS:.o=.d))
diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service.in
similarity index 85%
rename from multipathd/multipathd.service
rename to multipathd/multipathd.service.in
index 5a9cde1..6d03ff7 100644
--- a/multipathd/multipathd.service
+++ b/multipathd/multipathd.service.in
@@ -2,8 +2,8 @@
Description=Device-Mapper Multipath Device Controller
Before=lvm2-activation-early.service
Before=local-fs-pre.target blk-availability.service shutdown.target
-Wants=systemd-udevd-kernel.socket
-After=systemd-udevd-kernel.socket
+Wants=systemd-udevd-kernel.socket @MODPROBE_UNIT@
+After=systemd-udevd-kernel.socket @MODPROBE_UNIT@
After=multipathd.socket systemd-remount-fs.service
Before=initrd-cleanup.service
DefaultDependencies=no
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 08/14] multipathd.service: require modprobe@dm_multipath.service if available
2023-10-26 17:41 ` [PATCH v2 08/14] multipathd.service: require modprobe@dm_multipath.service if available mwilck
@ 2023-10-27 19:09 ` Benjamin Marzinski
2023-10-27 20:05 ` Benjamin Marzinski
1 sibling, 0 replies; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 19:09 UTC (permalink / raw)
To: mwilck; +Cc: Christophe Varoqui, dm-devel
On Thu, Oct 26, 2023 at 07:41:47PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> systemd v245 and newer provides the modprobe@.service unit, which
> can be used to pull in modules via systemd. This is cleaner than
> loading dm-multipath unconditionally via modules-load.d. Use it
> if the detected systemd version support it.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
> Makefile.inc | 4 +++-
> multipath/Makefile | 6 ++++--
> multipathd/Makefile | 4 ++--
> multipathd/{multipathd.service => multipathd.service.in} | 4 ++--
> 4 files changed, 11 insertions(+), 7 deletions(-)
> rename multipathd/{multipathd.service => multipathd.service.in} (85%)
>
> diff --git a/Makefile.inc b/Makefile.inc
> index 04bfa56..8655cba 100644
> --- a/Makefile.inc
> +++ b/Makefile.inc
> @@ -90,6 +90,8 @@ ORIG_LDFLAGS := $(LDFLAGS)
>
> SYSTEMD_CPPFLAGS := $(if $(SYSTEMD),-DUSE_SYSTEMD=$(SYSTEMD))
> SYSTEMD_LIBDEPS := $(if $(SYSTEMD),$(if $(shell test $(SYSTEMD) -gt 209 && echo 1),-lsystemd,-lsystemd-daemon))
> +MODPROBE_UNIT := $(shell test "0$(SYSTEMD)" -lt 245 2>/dev/null || \
> + echo "modprobe@dm_multipath.service")
>
> OPTFLAGS := -O2 -g $(STACKPROT) --param=ssp-buffer-size=4
> WARNFLAGS := -Werror -Wall -Wextra -Wformat=2 $(WFORMATOVERFLOW) -Werror=implicit-int \
> @@ -144,4 +146,4 @@ NV_VERSION_SCRIPT = $(DEVLIB:%.so=%-nv.version)
>
> %: %.in
> @echo creating $@
> - $(Q)sed 's:@CONFIGFILE@:'$(configfile)':g;s:@CONFIGDIR@:'$(configdir)':g;s:@STATE_DIR@:'$(statedir)':g;s:@RUNTIME_DIR@:'$(runtimedir)':g' $< >$@
> + $(Q)sed 's:@CONFIGFILE@:'$(configfile)':g;s:@CONFIGDIR@:'$(configdir)':g;s:@STATE_DIR@:'$(statedir)':g;s:@RUNTIME_DIR@:'$(runtimedir)':g;s/@MODPROBE_UNIT@/'$(MODPROBE_UNIT)'/g' $< >$@
> diff --git a/multipath/Makefile b/multipath/Makefile
> index 68cb5ce..0efb9b2 100644
> --- a/multipath/Makefile
> +++ b/multipath/Makefile
> @@ -27,14 +27,16 @@ install:
> $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(udevrulesdir)
> $(Q)$(INSTALL_PROGRAM) -m 644 11-dm-mpath.rules $(DESTDIR)$(udevrulesdir)
> $(Q)$(INSTALL_PROGRAM) -m 644 multipath.rules $(DESTDIR)$(udevrulesdir)/56-multipath.rules
> - $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(modulesloaddir)
> - $(Q)$(INSTALL_PROGRAM) -m 644 modules-load.conf $(DESTDIR)$(modulesloaddir)/multipath.conf
> $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(tmpfilesdir)
> $(Q)$(INSTALL_PROGRAM) -m 644 tmpfiles.conf $(DESTDIR)$(tmpfilesdir)/multipath.conf
> $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(mandir)/man8
> $(Q)$(INSTALL_PROGRAM) -m 644 $(EXEC).8 $(DESTDIR)$(mandir)/man8
> $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(mandir)/man5
> $(Q)$(INSTALL_PROGRAM) -m 644 $(EXEC).conf.5 $(DESTDIR)$(mandir)/man5
> + $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(modulesloaddir)
> +ifeq ($(MODPROBE_UNIT),)
> + $(Q)$(INSTALL_PROGRAM) -m 644 modules-load.conf $(DESTDIR)$(modulesloaddir)/multipath.conf
> +endif
> ifneq ($(SCSI_DH_MODULES_PRELOAD),)
> $(Q)$(INSTALL_PROGRAM) -m 644 scsi_dh.conf $(DESTDIR)$(modulesloaddir)/scsi_dh.conf
> $(Q)for _x in $(SCSI_DH_MODULES_PRELOAD); do echo "$$_x"; done \
> diff --git a/multipathd/Makefile b/multipathd/Makefile
> index 0ba6ecb..997b40c 100644
> --- a/multipathd/Makefile
> +++ b/multipathd/Makefile
> @@ -41,7 +41,7 @@ ifeq ($(FPIN_SUPPORT),1)
> OBJS += fpin_handlers.o
> endif
>
> -all : $(EXEC) $(CLI) $(MANPAGES)
> +all : $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service
>
> $(EXEC): $(OBJS) $(multipathdir)/libmultipath.so $(mpathcmddir)/libmpathcmd.so
> @echo building $@ because of $?
> @@ -78,7 +78,7 @@ uninstall:
> $(Q)$(RM) $(DESTDIR)$(unitdir)/$(EXEC).socket
>
> clean: dep_clean
> - $(Q)$(RM) core *.o $(EXEC) $(CLI) $(MANPAGES)
> + $(Q)$(RM) core *.o $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service
>
> include $(wildcard $(OBJS:.o=.d) $(CLI_OBJS:.o=.d))
>
> diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service.in
> similarity index 85%
> rename from multipathd/multipathd.service
> rename to multipathd/multipathd.service.in
> index 5a9cde1..6d03ff7 100644
> --- a/multipathd/multipathd.service
> +++ b/multipathd/multipathd.service.in
> @@ -2,8 +2,8 @@
> Description=Device-Mapper Multipath Device Controller
> Before=lvm2-activation-early.service
> Before=local-fs-pre.target blk-availability.service shutdown.target
> -Wants=systemd-udevd-kernel.socket
> -After=systemd-udevd-kernel.socket
> +Wants=systemd-udevd-kernel.socket @MODPROBE_UNIT@
> +After=systemd-udevd-kernel.socket @MODPROBE_UNIT@
> After=multipathd.socket systemd-remount-fs.service
> Before=initrd-cleanup.service
> DefaultDependencies=no
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 08/14] multipathd.service: require modprobe@dm_multipath.service if available
2023-10-26 17:41 ` [PATCH v2 08/14] multipathd.service: require modprobe@dm_multipath.service if available mwilck
2023-10-27 19:09 ` Benjamin Marzinski
@ 2023-10-27 20:05 ` Benjamin Marzinski
2023-10-30 11:45 ` Martin Wilck
1 sibling, 1 reply; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 20:05 UTC (permalink / raw)
To: mwilck; +Cc: Christophe Varoqui, dm-devel
On Thu, Oct 26, 2023 at 07:41:47PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> systemd v245 and newer provides the modprobe@.service unit, which
> can be used to pull in modules via systemd. This is cleaner than
> loading dm-multipath unconditionally via modules-load.d. Use it
> if the detected systemd version support it.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
I just realized that this should also add multipathd.service to
.gitignore
-Ben
> ---
> Makefile.inc | 4 +++-
> multipath/Makefile | 6 ++++--
> multipathd/Makefile | 4 ++--
> multipathd/{multipathd.service => multipathd.service.in} | 4 ++--
> 4 files changed, 11 insertions(+), 7 deletions(-)
> rename multipathd/{multipathd.service => multipathd.service.in} (85%)
>
> diff --git a/Makefile.inc b/Makefile.inc
> index 04bfa56..8655cba 100644
> --- a/Makefile.inc
> +++ b/Makefile.inc
> @@ -90,6 +90,8 @@ ORIG_LDFLAGS := $(LDFLAGS)
>
> SYSTEMD_CPPFLAGS := $(if $(SYSTEMD),-DUSE_SYSTEMD=$(SYSTEMD))
> SYSTEMD_LIBDEPS := $(if $(SYSTEMD),$(if $(shell test $(SYSTEMD) -gt 209 && echo 1),-lsystemd,-lsystemd-daemon))
> +MODPROBE_UNIT := $(shell test "0$(SYSTEMD)" -lt 245 2>/dev/null || \
> + echo "modprobe@dm_multipath.service")
>
> OPTFLAGS := -O2 -g $(STACKPROT) --param=ssp-buffer-size=4
> WARNFLAGS := -Werror -Wall -Wextra -Wformat=2 $(WFORMATOVERFLOW) -Werror=implicit-int \
> @@ -144,4 +146,4 @@ NV_VERSION_SCRIPT = $(DEVLIB:%.so=%-nv.version)
>
> %: %.in
> @echo creating $@
> - $(Q)sed 's:@CONFIGFILE@:'$(configfile)':g;s:@CONFIGDIR@:'$(configdir)':g;s:@STATE_DIR@:'$(statedir)':g;s:@RUNTIME_DIR@:'$(runtimedir)':g' $< >$@
> + $(Q)sed 's:@CONFIGFILE@:'$(configfile)':g;s:@CONFIGDIR@:'$(configdir)':g;s:@STATE_DIR@:'$(statedir)':g;s:@RUNTIME_DIR@:'$(runtimedir)':g;s/@MODPROBE_UNIT@/'$(MODPROBE_UNIT)'/g' $< >$@
> diff --git a/multipath/Makefile b/multipath/Makefile
> index 68cb5ce..0efb9b2 100644
> --- a/multipath/Makefile
> +++ b/multipath/Makefile
> @@ -27,14 +27,16 @@ install:
> $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(udevrulesdir)
> $(Q)$(INSTALL_PROGRAM) -m 644 11-dm-mpath.rules $(DESTDIR)$(udevrulesdir)
> $(Q)$(INSTALL_PROGRAM) -m 644 multipath.rules $(DESTDIR)$(udevrulesdir)/56-multipath.rules
> - $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(modulesloaddir)
> - $(Q)$(INSTALL_PROGRAM) -m 644 modules-load.conf $(DESTDIR)$(modulesloaddir)/multipath.conf
> $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(tmpfilesdir)
> $(Q)$(INSTALL_PROGRAM) -m 644 tmpfiles.conf $(DESTDIR)$(tmpfilesdir)/multipath.conf
> $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(mandir)/man8
> $(Q)$(INSTALL_PROGRAM) -m 644 $(EXEC).8 $(DESTDIR)$(mandir)/man8
> $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(mandir)/man5
> $(Q)$(INSTALL_PROGRAM) -m 644 $(EXEC).conf.5 $(DESTDIR)$(mandir)/man5
> + $(Q)$(INSTALL_PROGRAM) -d $(DESTDIR)$(modulesloaddir)
> +ifeq ($(MODPROBE_UNIT),)
> + $(Q)$(INSTALL_PROGRAM) -m 644 modules-load.conf $(DESTDIR)$(modulesloaddir)/multipath.conf
> +endif
> ifneq ($(SCSI_DH_MODULES_PRELOAD),)
> $(Q)$(INSTALL_PROGRAM) -m 644 scsi_dh.conf $(DESTDIR)$(modulesloaddir)/scsi_dh.conf
> $(Q)for _x in $(SCSI_DH_MODULES_PRELOAD); do echo "$$_x"; done \
> diff --git a/multipathd/Makefile b/multipathd/Makefile
> index 0ba6ecb..997b40c 100644
> --- a/multipathd/Makefile
> +++ b/multipathd/Makefile
> @@ -41,7 +41,7 @@ ifeq ($(FPIN_SUPPORT),1)
> OBJS += fpin_handlers.o
> endif
>
> -all : $(EXEC) $(CLI) $(MANPAGES)
> +all : $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service
>
> $(EXEC): $(OBJS) $(multipathdir)/libmultipath.so $(mpathcmddir)/libmpathcmd.so
> @echo building $@ because of $?
> @@ -78,7 +78,7 @@ uninstall:
> $(Q)$(RM) $(DESTDIR)$(unitdir)/$(EXEC).socket
>
> clean: dep_clean
> - $(Q)$(RM) core *.o $(EXEC) $(CLI) $(MANPAGES)
> + $(Q)$(RM) core *.o $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service
>
> include $(wildcard $(OBJS:.o=.d) $(CLI_OBJS:.o=.d))
>
> diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service.in
> similarity index 85%
> rename from multipathd/multipathd.service
> rename to multipathd/multipathd.service.in
> index 5a9cde1..6d03ff7 100644
> --- a/multipathd/multipathd.service
> +++ b/multipathd/multipathd.service.in
> @@ -2,8 +2,8 @@
> Description=Device-Mapper Multipath Device Controller
> Before=lvm2-activation-early.service
> Before=local-fs-pre.target blk-availability.service shutdown.target
> -Wants=systemd-udevd-kernel.socket
> -After=systemd-udevd-kernel.socket
> +Wants=systemd-udevd-kernel.socket @MODPROBE_UNIT@
> +After=systemd-udevd-kernel.socket @MODPROBE_UNIT@
> After=multipathd.socket systemd-remount-fs.service
> Before=initrd-cleanup.service
> DefaultDependencies=no
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 08/14] multipathd.service: require modprobe@dm_multipath.service if available
2023-10-27 20:05 ` Benjamin Marzinski
@ 2023-10-30 11:45 ` Martin Wilck
0 siblings, 0 replies; 30+ messages in thread
From: Martin Wilck @ 2023-10-30 11:45 UTC (permalink / raw)
To: Benjamin Marzinski; +Cc: Christophe Varoqui, dm-devel
On Fri, 2023-10-27 at 16:05 -0400, Benjamin Marzinski wrote:
> On Thu, Oct 26, 2023 at 07:41:47PM +0200, mwilck@suse.com wrote:
> > From: Martin Wilck <mwilck@suse.com>
> >
> > systemd v245 and newer provides the modprobe@.service unit, which
> > can be used to pull in modules via systemd. This is cleaner than
> > loading dm-multipath unconditionally via modules-load.d. Use it
> > if the detected systemd version support it.
> >
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
>
> I just realized that this should also add multipathd.service to
> .gitignore
I'll fix this up and keep your Reviewed-by.
Martin
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 09/14] libmpathutil: remove systemd_service_enabled()
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
` (7 preceding siblings ...)
2023-10-26 17:41 ` [PATCH v2 08/14] multipathd.service: require modprobe@dm_multipath.service if available mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-26 17:41 ` [PATCH v2 10/14] multipath.conf.5: fix typo mwilck
` (4 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck
From: Martin Wilck <mwilck@suse.com>
Since c203929 ("multipathd.service: add dependency on
systemd-udevd-kernel.socket"), multipathd will start early during boot.
Moreover, we recommend using socket activation for multipathd,
and if multipathd.socket is enabled, the __mpath_connect()
check will succeed anyway.
The systemd_service_enabled() test was just "good enough" for
standard situations but never robust; it checked for multipathd.wants in
"some" directory, which might or might not be the current target,
and it would return true even of multipathd.service was masked.
Remove this test.
Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmpathutil/libmpathutil.version | 17 +++------
libmpathutil/util.c | 58 -------------------------------
libmpathutil/util.h | 1 -
libmultipath/valid.c | 16 ++-------
tests/valid.c | 24 ++-----------
5 files changed, 9 insertions(+), 107 deletions(-)
diff --git a/libmpathutil/libmpathutil.version b/libmpathutil/libmpathutil.version
index 6ebb718..15ff467 100644
--- a/libmpathutil/libmpathutil.version
+++ b/libmpathutil/libmpathutil.version
@@ -93,12 +93,15 @@ local:
};
/* symbols referenced internally by libmultipath */
-LIBMPATHUTIL_1.0 {
+LIBMPATHUTIL_2.0 {
alloc_bitfield;
__append_strbuf_str;
append_strbuf_quoted;
basenamecpy;
+ cleanup_fd_ptr;
cleanup_free_ptr;
+ cleanup_vector_free;
+ cleanup_fclose;
filepresent;
find_keyword;
free_keywords;
@@ -120,21 +123,9 @@ LIBMPATHUTIL_1.0 {
snprint_keyword;
steal_strbuf_str;
strlcat;
- systemd_service_enabled;
validate_config_strvec;
vector_find_or_add_slot;
vector_insert_slot;
vector_move_up;
vector_sort;
};
-
-LIBMPATHUTIL_1.1 {
-global:
- cleanup_fd_ptr;
-} LIBMPATHUTIL_1.0;
-
-LIBMPATHUTIL_1.2 {
-global:
- cleanup_vector_free;
- cleanup_fclose;
-} LIBMPATHUTIL_1.0;
diff --git a/libmpathutil/util.c b/libmpathutil/util.c
index 92f25a5..9d147fc 100644
--- a/libmpathutil/util.c
+++ b/libmpathutil/util.c
@@ -213,64 +213,6 @@ setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached)
}
}
-int systemd_service_enabled_in(const char *dev, const char *prefix)
-{
- static const char service[] = "multipathd.service";
- char path[PATH_MAX], file[PATH_MAX];
- DIR *dirfd;
- struct dirent *d;
- int found = 0;
-
- if (safe_sprintf(path, "%s/systemd/system", prefix))
- return 0;
-
- condlog(3, "%s: checking for %s in %s", dev, service, path);
-
- dirfd = opendir(path);
- if (dirfd == NULL)
- return 0;
-
- while ((d = readdir(dirfd)) != NULL) {
- char *p;
- struct stat stbuf;
-
- if ((strcmp(d->d_name,".") == 0) ||
- (strcmp(d->d_name,"..") == 0))
- continue;
-
- if (strlen(d->d_name) < 6)
- continue;
-
- p = d->d_name + strlen(d->d_name) - 6;
- if (strcmp(p, ".wants"))
- continue;
- if (!safe_sprintf(file, "%s/%s/%s",
- path, d->d_name, service)
- && stat(file, &stbuf) == 0) {
- condlog(3, "%s: found %s", dev, file);
- found++;
- break;
- }
- }
- closedir(dirfd);
-
- return found;
-}
-
-int systemd_service_enabled(const char *dev)
-{
- int found = 0;
-
- found = systemd_service_enabled_in(dev, "/etc");
- if (!found)
- found = systemd_service_enabled_in(dev, "/usr/lib");
- if (!found)
- found = systemd_service_enabled_in(dev, "/lib");
- if (!found)
- found = systemd_service_enabled_in(dev, "/run");
- return found;
-}
-
static int _linux_version_code;
static pthread_once_t _lvc_initialized = PTHREAD_ONCE_INIT;
diff --git a/libmpathutil/util.h b/libmpathutil/util.h
index 99a471d..de9fcfd 100644
--- a/libmpathutil/util.h
+++ b/libmpathutil/util.h
@@ -21,7 +21,6 @@ size_t strlcat(char * restrict dst, const char * restrict src, size_t size);
dev_t parse_devt(const char *dev_t);
char *convert_dev(char *dev, int is_path_device);
void setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached);
-int systemd_service_enabled(const char *dev);
int get_linux_version_code(void);
int safe_write(int fd, const void *buf, size_t count);
void set_max_fds(rlim_t max_fds);
diff --git a/libmultipath/valid.c b/libmultipath/valid.c
index d4dae3e..f223778 100644
--- a/libmultipath/valid.c
+++ b/libmultipath/valid.c
@@ -314,23 +314,11 @@ is_path_valid(const char *name, struct config *conf, struct path *pp,
return PATH_IS_VALID_NO_CHECK;
}
- /*
- * "multipath -u" may be run before the daemon is started. In this
- * case, systemd might own the socket but might delay multipathd
- * startup until some other unit (udev settle!) has finished
- * starting. With many LUNs, the listen backlog may be exceeded, which
- * would cause connect() to block. This causes udev workers calling
- * "multipath -u" to hang, and thus creates a deadlock, until "udev
- * settle" times out. To avoid this, call connect() in non-blocking
- * mode here, and take EAGAIN as indication for a filled-up systemd
- * backlog.
- */
-
if (check_multipathd) {
fd = __mpath_connect(1);
if (fd < 0) {
- if (errno != EAGAIN && !systemd_service_enabled(name)) {
- condlog(3, "multipathd not running or enabled");
+ if (errno != EAGAIN) {
+ condlog(3, "multipathd not running");
return PATH_IS_NOT_VALID;
}
} else
diff --git a/tests/valid.c b/tests/valid.c
index 7032932..18a5a7b 100644
--- a/tests/valid.c
+++ b/tests/valid.c
@@ -62,11 +62,6 @@ int __wrap___mpath_connect(int nonblocking)
return -1;
}
-int __wrap_systemd_service_enabled(const char *dev)
-{
- return (int)mock_type(bool);
-}
-
/* There's no point in checking the return value here */
int __wrap_mpath_disconnect(int fd)
{
@@ -216,7 +211,6 @@ enum {
enum {
CHECK_MPATHD_RUNNING,
CHECK_MPATHD_EAGAIN,
- CHECK_MPATHD_ENABLED,
CHECK_MPATHD_SKIP,
};
@@ -232,11 +226,8 @@ static void setup_passing(char *name, char *wwid, unsigned int check_multipathd,
else if (check_multipathd == CHECK_MPATHD_EAGAIN) {
will_return(__wrap___mpath_connect, false);
will_return(__wrap___mpath_connect, EAGAIN);
- } else if (check_multipathd == CHECK_MPATHD_ENABLED) {
- will_return(__wrap___mpath_connect, false);
- will_return(__wrap___mpath_connect, ECONNREFUSED);
- will_return(__wrap_systemd_service_enabled, true);
}
+
/* nothing for CHECK_MPATHD_SKIP */
if (stage == STAGE_CHECK_MULTIPATHD)
return;
@@ -342,19 +333,10 @@ static void test_check_multipathd(void **state)
will_return(__wrap_sysfs_is_multipathed, false);
will_return(__wrap___mpath_connect, false);
will_return(__wrap___mpath_connect, ECONNREFUSED);
- will_return(__wrap_systemd_service_enabled, false);
+
assert_int_equal(is_path_valid(name, &conf, &pp, true),
PATH_IS_NOT_VALID);
assert_string_equal(pp.dev, name);
- /* test pass because service is enabled. fail getting udev */
- memset(&pp, 0, sizeof(pp));
- setup_passing(name, NULL, CHECK_MPATHD_ENABLED, STAGE_CHECK_MULTIPATHD);
- will_return(__wrap_udev_device_new_from_subsystem_sysname, false);
- will_return(__wrap_udev_device_new_from_subsystem_sysname,
- name);
- assert_int_equal(is_path_valid(name, &conf, &pp, true),
- PATH_IS_ERROR);
- assert_string_equal(pp.dev, name);
/* test pass because connect returned EAGAIN. fail getting udev */
setup_passing(name, NULL, CHECK_MPATHD_EAGAIN, STAGE_CHECK_MULTIPATHD);
will_return(__wrap_udev_device_new_from_subsystem_sysname, false);
@@ -533,7 +515,7 @@ static void test_check_uuid_present(void **state)
memset(&pp, 0, sizeof(pp));
conf.find_multipaths = FIND_MULTIPATHS_STRICT;
- setup_passing(name, wwid, CHECK_MPATHD_ENABLED, STAGE_CHECK_WWIDS);
+ setup_passing(name, wwid, CHECK_MPATHD_RUNNING, STAGE_CHECK_WWIDS);
will_return(__wrap_dm_map_present_by_uuid, 1);
will_return(__wrap_dm_map_present_by_uuid, wwid);
assert_int_equal(is_path_valid(name, &conf, &pp, true),
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 10/14] multipath.conf.5: fix typo
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
` (8 preceding siblings ...)
2023-10-26 17:41 ` [PATCH v2 09/14] libmpathutil: remove systemd_service_enabled() mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-27 19:10 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 11/14] Makefile.inc, README.md: fix docs for prefix in split-usr case mwilck
` (3 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck
From: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/multipath.conf.5.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/multipath/multipath.conf.5.in b/multipath/multipath.conf.5.in
index 226d001..2b70b6d 100644
--- a/multipath/multipath.conf.5.in
+++ b/multipath/multipath.conf.5.in
@@ -782,7 +782,7 @@ Specify the maximum number of seconds the SCSI layer will spend doing error
handling when scsi devices fail. After this timeout the scsi layer will perform
a full HBA reset. Setting this may be necessary in cases where the rport is
never lost, so \fIfast_io_fail_tmo\fR and \fIdev_loss_tmo\fR will never
-trigger, but (frequently do to load) scsi commands still hang. \fBNote:\fR when
+trigger, but (frequently due to load) scsi commands still hang. \fBNote:\fR when
the scsi error handler performs the HBA reset, all target paths on that HBA
will be affected. eh_deadline should only be set in cases where all targets on
the affected HBAs are multipathed.
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 10/14] multipath.conf.5: fix typo
2023-10-26 17:41 ` [PATCH v2 10/14] multipath.conf.5: fix typo mwilck
@ 2023-10-27 19:10 ` Benjamin Marzinski
0 siblings, 0 replies; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 19:10 UTC (permalink / raw)
To: mwilck; +Cc: Christophe Varoqui, dm-devel
On Thu, Oct 26, 2023 at 07:41:49PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
> multipath/multipath.conf.5.in | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/multipath/multipath.conf.5.in b/multipath/multipath.conf.5.in
> index 226d001..2b70b6d 100644
> --- a/multipath/multipath.conf.5.in
> +++ b/multipath/multipath.conf.5.in
> @@ -782,7 +782,7 @@ Specify the maximum number of seconds the SCSI layer will spend doing error
> handling when scsi devices fail. After this timeout the scsi layer will perform
> a full HBA reset. Setting this may be necessary in cases where the rport is
> never lost, so \fIfast_io_fail_tmo\fR and \fIdev_loss_tmo\fR will never
> -trigger, but (frequently do to load) scsi commands still hang. \fBNote:\fR when
> +trigger, but (frequently due to load) scsi commands still hang. \fBNote:\fR when
> the scsi error handler performs the HBA reset, all target paths on that HBA
> will be affected. eh_deadline should only be set in cases where all targets on
> the affected HBAs are multipathed.
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 11/14] Makefile.inc, README.md: fix docs for prefix in split-usr case
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
` (9 preceding siblings ...)
2023-10-26 17:41 ` [PATCH v2 10/14] multipath.conf.5: fix typo mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-27 19:27 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 12/14] README.md: update mailing list and contributing information mwilck
` (2 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Martin Wilck, Xose Vazquez Perez
From: Martin Wilck <mwilck@suse.com>
systemd with "split-usr=true" uses rootprefixdir as prefix for
units and for udev libraries and rules ("udevlibexecdir" in
systemd). Fix the documentation for this case. Also, slightly
improve the paragraph about SCSI module loading.
Signed-off-by: Martin Wilck <mwilck@suse.com>
Cc: Xose Vazquez Perez <xose.vazquez@gmail.com>
---
Makefile.inc | 6 ++++--
README.md | 25 ++++++++++++++++++-------
2 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/Makefile.inc b/Makefile.inc
index 8655cba..a20e2ce 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -40,8 +40,10 @@ usr_prefix := $(prefix)
# Prefix for configfuration files (multipath.conf)
etc_prefix := $(prefix)
# Where to install systemd-related files. systemd is usually installed under /usr
-# Note: some systemd installations use separate "prefix" and "rootprefix".
-# In this case, override only unitdir to use systemd's "rootprefix" instead of $(systemd_prefix)
+# Note: systemd installations with "split-usr=true" use separate "prefixdir" and
+# "rootprefixdir". Our systemd_prefix corresponds to "prefixdir".
+# In this case, override only unitdir and libudevdir below to use
+# systemd's "rootprefixdir" instead of $(systemd_prefix)
systemd_prefix := /usr
# Make sure all prefix variables end in "/"
diff --git a/README.md b/README.md
index 524c9fb..25ce963 100644
--- a/README.md
+++ b/README.md
@@ -111,6 +111,13 @@ The following variables can be passed to the `make` command line:
polling API. For use with pre-5.0 kernels that don't support dmevent polling
(but even if you don't use this option, multipath-tools will work with
these kernels).
+ * `SYSTEMD`: The version number of systemd (e.g. "244") to compile the code for.
+ The default is autodetected, assuming that the systemd version in the build
+ environment is the same as on the target system. Override the value to
+ build for a different systemd version, or set it to `""` to build for a
+ system without systemd.
+ **Caution:** multipathd without systemd has been largely untested by the
+ upstream maintainers since at least 2020.
* `SCSI_DH_MODULES_PRELOAD="(list)"`: specify a space-separated list of SCSI
device handler kernel modules to load early during boot. Some
multipath-tools functionality depends on these modules being loaded
@@ -122,7 +129,9 @@ The following variables can be passed to the `make` command line:
It's especially useful if `scsi_mod` is builtin but `scsi_dh_alua` and
other device handler modules are built as modules. If `scsi_mod` itself is compiled
as a module, it might make more sense to use a module softdep for the same
- purpose.
+ purpose by creating a `modprobe.d` file like this:
+
+ softdep scsi_mod post: scsi_dh_alua scsi_dh_rdac
### Installation Paths
@@ -133,9 +142,9 @@ The following variables can be passed to the `make` command line:
for booting. Non-usr-merged distributions[^systemd] may want to set this to
`/usr`. The default is `$(prefix)`.
* `systemd_prefix`: Prefix for systemd-related files[^systemd]. The default is `/usr`.
- * `etc_prefix`: The prefix for configuration files. "Usr-merged"
+ * `etc_prefix`: The prefix for configuration files. "usr-merged"
distributions with immutable `/usr`[^systemd] may want to set this to
- `/etc`. The default is `$(prefix)`.
+ `""`. The default is `$(prefix)`.
* `LIB`: the subdirectory under `prefix` where shared libraries will be
installed. By default, the makefile uses `/lib64` if this directory is
found on the build system, and `/lib` otherwise.
@@ -144,10 +153,12 @@ The options `configdir`, `plugindir`, `configfile`, and `statedir` above can
be used for setting individual paths where the `prefix` variables don't provide
sufficient control. See `Makefile.inc` for even more fine-grained control.
-[^systemd]: Some systemd installations use separate `prefix` and `rootprefix`.
- On such a distribution, set `prefix`, and override `unitdir` to use systemd's
- `rootprefix`. Recent systemd releases generally require everything to be
- installed under `/usr` (so-called "usr-merged" distribution). On "usr-
+[^systemd]: systemd installations up to v254 which have been built with
+ `split-usr=true` may use separate `prefixdir` and `rootprefixdir`
+ directories, where `prefixdir` is a subdirectory of `rootprefixdir`.
+ multipath-tools' `systemd_prefix` corresponds to systemd's `prefixdir`.
+ On such distributions, override `unitdir` and `libudevdir` to use systemd's
+ `rootprefix`: `make libudevdir=/lib/udev unitdir=/lib/systemd/system`
### Compiler Options
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 11/14] Makefile.inc, README.md: fix docs for prefix in split-usr case
2023-10-26 17:41 ` [PATCH v2 11/14] Makefile.inc, README.md: fix docs for prefix in split-usr case mwilck
@ 2023-10-27 19:27 ` Benjamin Marzinski
0 siblings, 0 replies; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 19:27 UTC (permalink / raw)
To: mwilck; +Cc: Christophe Varoqui, dm-devel, Xose Vazquez Perez
On Thu, Oct 26, 2023 at 07:41:50PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> systemd with "split-usr=true" uses rootprefixdir as prefix for
> units and for udev libraries and rules ("udevlibexecdir" in
> systemd). Fix the documentation for this case. Also, slightly
> improve the paragraph about SCSI module loading.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Cc: Xose Vazquez Perez <xose.vazquez@gmail.com>
> ---
> Makefile.inc | 6 ++++--
> README.md | 25 ++++++++++++++++++-------
> 2 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/Makefile.inc b/Makefile.inc
> index 8655cba..a20e2ce 100644
> --- a/Makefile.inc
> +++ b/Makefile.inc
> @@ -40,8 +40,10 @@ usr_prefix := $(prefix)
> # Prefix for configfuration files (multipath.conf)
> etc_prefix := $(prefix)
> # Where to install systemd-related files. systemd is usually installed under /usr
> -# Note: some systemd installations use separate "prefix" and "rootprefix".
> -# In this case, override only unitdir to use systemd's "rootprefix" instead of $(systemd_prefix)
> +# Note: systemd installations with "split-usr=true" use separate "prefixdir" and
> +# "rootprefixdir". Our systemd_prefix corresponds to "prefixdir".
> +# In this case, override only unitdir and libudevdir below to use
> +# systemd's "rootprefixdir" instead of $(systemd_prefix)
> systemd_prefix := /usr
>
> # Make sure all prefix variables end in "/"
> diff --git a/README.md b/README.md
> index 524c9fb..25ce963 100644
> --- a/README.md
> +++ b/README.md
> @@ -111,6 +111,13 @@ The following variables can be passed to the `make` command line:
> polling API. For use with pre-5.0 kernels that don't support dmevent polling
> (but even if you don't use this option, multipath-tools will work with
> these kernels).
> + * `SYSTEMD`: The version number of systemd (e.g. "244") to compile the code for.
> + The default is autodetected, assuming that the systemd version in the build
> + environment is the same as on the target system. Override the value to
> + build for a different systemd version, or set it to `""` to build for a
> + system without systemd.
> + **Caution:** multipathd without systemd has been largely untested by the
> + upstream maintainers since at least 2020.
> * `SCSI_DH_MODULES_PRELOAD="(list)"`: specify a space-separated list of SCSI
> device handler kernel modules to load early during boot. Some
> multipath-tools functionality depends on these modules being loaded
> @@ -122,7 +129,9 @@ The following variables can be passed to the `make` command line:
> It's especially useful if `scsi_mod` is builtin but `scsi_dh_alua` and
> other device handler modules are built as modules. If `scsi_mod` itself is compiled
> as a module, it might make more sense to use a module softdep for the same
> - purpose.
> + purpose by creating a `modprobe.d` file like this:
> +
> + softdep scsi_mod post: scsi_dh_alua scsi_dh_rdac
>
> ### Installation Paths
>
> @@ -133,9 +142,9 @@ The following variables can be passed to the `make` command line:
> for booting. Non-usr-merged distributions[^systemd] may want to set this to
> `/usr`. The default is `$(prefix)`.
> * `systemd_prefix`: Prefix for systemd-related files[^systemd]. The default is `/usr`.
> - * `etc_prefix`: The prefix for configuration files. "Usr-merged"
> + * `etc_prefix`: The prefix for configuration files. "usr-merged"
> distributions with immutable `/usr`[^systemd] may want to set this to
> - `/etc`. The default is `$(prefix)`.
> + `""`. The default is `$(prefix)`.
> * `LIB`: the subdirectory under `prefix` where shared libraries will be
> installed. By default, the makefile uses `/lib64` if this directory is
> found on the build system, and `/lib` otherwise.
> @@ -144,10 +153,12 @@ The options `configdir`, `plugindir`, `configfile`, and `statedir` above can
> be used for setting individual paths where the `prefix` variables don't provide
> sufficient control. See `Makefile.inc` for even more fine-grained control.
>
> -[^systemd]: Some systemd installations use separate `prefix` and `rootprefix`.
> - On such a distribution, set `prefix`, and override `unitdir` to use systemd's
> - `rootprefix`. Recent systemd releases generally require everything to be
> - installed under `/usr` (so-called "usr-merged" distribution). On "usr-
> +[^systemd]: systemd installations up to v254 which have been built with
> + `split-usr=true` may use separate `prefixdir` and `rootprefixdir`
> + directories, where `prefixdir` is a subdirectory of `rootprefixdir`.
> + multipath-tools' `systemd_prefix` corresponds to systemd's `prefixdir`.
> + On such distributions, override `unitdir` and `libudevdir` to use systemd's
> + `rootprefix`: `make libudevdir=/lib/udev unitdir=/lib/systemd/system`
>
> ### Compiler Options
>
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 12/14] README.md: update mailing list and contributing information
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
` (10 preceding siblings ...)
2023-10-26 17:41 ` [PATCH v2 11/14] Makefile.inc, README.md: fix docs for prefix in split-usr case mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-27 19:29 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 13/14] README.md: Extend the section about NVMe mwilck
2023-10-26 17:41 ` [PATCH v2 14/14] README.md: fix formatting of Changelog section mwilck
13 siblings, 1 reply; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Martin Wilck, Xose Vazquez Perez
From: Martin Wilck <mwilck@suse.com>
Create a "Contributing" section. Move the the current sections
related to contributing into that section.
Signed-off-by: Martin Wilck <mwilck@suse.com>
Cc: Xose Vazquez Perez <xose.vazquez@gmail.com>
---
README.md | 56 ++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 37 insertions(+), 19 deletions(-)
diff --git a/README.md b/README.md
index 25ce963..b2ed1f2 100644
--- a/README.md
+++ b/README.md
@@ -42,14 +42,6 @@ Go to: https://github.com/opensvc/multipath-tools/tags
Select a release-tag and then click on "zip" or "tar.gz".
-Devel code
-==========
-
-To get latest devel code:
-
- git clone -b queue https://github.com/openSUSE/multipath-tools
-
-
Building multipath-tools
========================
@@ -188,19 +180,45 @@ The following targets are intended for developers only.
* `make compile-commands.json` to create input for [clangd](https://clangd.llvm.org/).
-Add storage devices
-===================
-
-Follow the instructions in the `libmultipath/hwtable.c` header.
-
-
-Mailing list
+Contributing
============
-(subscribers-only)
-To subscribe and archives: https://listman.redhat.com/mailman/listinfo/dm-devel
-Searchable: https://marc.info/?l=dm-devel
+Please send patches or contributions for general discussion about
+multipath tools to the mailing list (see below). You can also create
+issues or pull requests on
+[GitHub](https://github.com/opensvc/multipath-tools).
+You will be asked to send your patches to the mailing list
+unless your patch is trivial.
+Mailing list
+------------
+
+The mailing list for multipath-tools is `dm-devel@lists.linux.dev`.
+To subscribe, send an email to `dm-devel+subscribe@lists.linux.dev`.
+Mailing list archives are available on
+[lore.kernel.org](https://lore.kernel.org/dm-devel/) and
+[marc.info](https://marc.info/?l=dm-devel). See also the
+[lists.linux.dev home page](https://subspace.kernel.org/lists.linux.dev.html).
+
+When sending patches to the mailing list, lease add a `Signed-off-by:`
+tag, and add Benjamin Marzinski <bmarzins@redhat.com> and
+Martin Wilck <mwilck@suse.com> to the Cc list.
+
+Staging area
+------------
+
+Between releases, the latest reviewed code can be obtained from
+[the queue branch](https://github.com/openSUSE/multipath-tools/tree/queue)
+in the openSUSE/multipath-tools repository on GitHub. From there,
+pull requests for new releases in the master repository are
+created roughly every 3 months.
+
+Adding new storage devices
+--------------------------
+
+If you want to add special settings for a storage device which is
+new on the market, follow the instructions at the top of the
+file `libmultipath/hwtable.c`.
Changelog
=========
@@ -213,7 +231,7 @@ Maintainer
==========
Christophe Varoqui <christophe.varoqui@opensvc.com>
-Device-mapper development mailing list <dm-devel@redhat.com>
+Device-mapper development mailing list <dm-devel@lists.linux.dev>
Licence
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 12/14] README.md: update mailing list and contributing information
2023-10-26 17:41 ` [PATCH v2 12/14] README.md: update mailing list and contributing information mwilck
@ 2023-10-27 19:29 ` Benjamin Marzinski
2023-10-30 11:41 ` Martin Wilck
0 siblings, 1 reply; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 19:29 UTC (permalink / raw)
To: mwilck; +Cc: Christophe Varoqui, dm-devel, Xose Vazquez Perez
On Thu, Oct 26, 2023 at 07:41:51PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> Create a "Contributing" section. Move the the current sections
> related to contributing into that section.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> Cc: Xose Vazquez Perez <xose.vazquez@gmail.com>
> ---
> README.md | 56 ++++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 37 insertions(+), 19 deletions(-)
>
> diff --git a/README.md b/README.md
> index 25ce963..b2ed1f2 100644
> --- a/README.md
> +++ b/README.md
> @@ -42,14 +42,6 @@ Go to: https://github.com/opensvc/multipath-tools/tags
> Select a release-tag and then click on "zip" or "tar.gz".
>
>
> -Devel code
> -==========
> -
> -To get latest devel code:
> -
> - git clone -b queue https://github.com/openSUSE/multipath-tools
> -
> -
> Building multipath-tools
> ========================
>
> @@ -188,19 +180,45 @@ The following targets are intended for developers only.
> * `make compile-commands.json` to create input for [clangd](https://clangd.llvm.org/).
>
>
> -Add storage devices
> -===================
> -
> -Follow the instructions in the `libmultipath/hwtable.c` header.
> -
> -
> -Mailing list
> +Contributing
> ============
>
> -(subscribers-only)
> -To subscribe and archives: https://listman.redhat.com/mailman/listinfo/dm-devel
> -Searchable: https://marc.info/?l=dm-devel
> +Please send patches or contributions for general discussion about
> +multipath tools to the mailing list (see below). You can also create
> +issues or pull requests on
> +[GitHub](https://github.com/opensvc/multipath-tools).
> +You will be asked to send your patches to the mailing list
> +unless your patch is trivial.
>
> +Mailing list
> +------------
> +
> +The mailing list for multipath-tools is `dm-devel@lists.linux.dev`.
> +To subscribe, send an email to `dm-devel+subscribe@lists.linux.dev`.
> +Mailing list archives are available on
> +[lore.kernel.org](https://lore.kernel.org/dm-devel/) and
> +[marc.info](https://marc.info/?l=dm-devel). See also the
> +[lists.linux.dev home page](https://subspace.kernel.org/lists.linux.dev.html).
> +
> +When sending patches to the mailing list, lease add a `Signed-off-by:`
Missed the 'p' in please. Otherwise this looks fine.
-Ben
> +tag, and add Benjamin Marzinski <bmarzins@redhat.com> and
> +Martin Wilck <mwilck@suse.com> to the Cc list.
> +
> +Staging area
> +------------
> +
> +Between releases, the latest reviewed code can be obtained from
> +[the queue branch](https://github.com/openSUSE/multipath-tools/tree/queue)
> +in the openSUSE/multipath-tools repository on GitHub. From there,
> +pull requests for new releases in the master repository are
> +created roughly every 3 months.
> +
> +Adding new storage devices
> +--------------------------
> +
> +If you want to add special settings for a storage device which is
> +new on the market, follow the instructions at the top of the
> +file `libmultipath/hwtable.c`.
>
> Changelog
> =========
> @@ -213,7 +231,7 @@ Maintainer
> ==========
>
> Christophe Varoqui <christophe.varoqui@opensvc.com>
> -Device-mapper development mailing list <dm-devel@redhat.com>
> +Device-mapper development mailing list <dm-devel@lists.linux.dev>
>
>
> Licence
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 12/14] README.md: update mailing list and contributing information
2023-10-27 19:29 ` Benjamin Marzinski
@ 2023-10-30 11:41 ` Martin Wilck
0 siblings, 0 replies; 30+ messages in thread
From: Martin Wilck @ 2023-10-30 11:41 UTC (permalink / raw)
To: Benjamin Marzinski; +Cc: Christophe Varoqui, dm-devel, Xose Vazquez Perez
On Fri, 2023-10-27 at 15:29 -0400, Benjamin Marzinski wrote:
> On Thu, Oct 26, 2023 at 07:41:51PM +0200, mwilck@suse.com wrote:
> >
> > +Mailing list
> > +------------
> > +
> > +The mailing list for multipath-tools is
> > `dm-devel@lists.linux.dev`.
> > +To subscribe, send an email to
> > `dm-devel+subscribe@lists.linux.dev`.
> > +Mailing list archives are available on
> > +[lore.kernel.org](https://lore.kernel.org/dm-devel/) and
> > +[marc.info](https://marc.info/?l=dm-devel). See also the
> > +[lists.linux.dev home
> > page](https://subspace.kernel.org/lists.linux.dev.html).
> > +
> > +When sending patches to the mailing list, lease add a `Signed-off-
> > by:`
>
> Missed the 'p' in please. Otherwise this looks fine.
I'll take the liberty to add the "p" and add your Acked-by.
Let me know if you disagree.
Martin
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 13/14] README.md: Extend the section about NVMe
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
` (11 preceding siblings ...)
2023-10-26 17:41 ` [PATCH v2 12/14] README.md: update mailing list and contributing information mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-27 19:34 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 14/14] README.md: fix formatting of Changelog section mwilck
13 siblings, 1 reply; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Martin Wilck, Xose Vazquez Perez
From: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Cc: Xose Vazquez Perez <xose.vazquez@gmail.com>
---
README.md | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/README.md b/README.md
index b2ed1f2..2fb423b 100644
--- a/README.md
+++ b/README.md
@@ -273,15 +273,25 @@ To enable ALUA, the following options should be changed:
NVMe
====
-To use Device Mapper/multipath-tools with NVMe devices,
-if the Native NVMe Multipath subsystem is enabled
-( "Y" in `/sys/module/nvme_core/parameters/multipath` ),
-it has to be disabled:
-`echo "options nvme_core multipath=N" > /etc/modprobe.d/01-nvme_core-mp.conf`,
-regenerate the initramfs (`dracut -f` or `update-initramfs`) and reboot.
+Using dm-multipath with NVMe
+----------------------------
-Check that it is disabled(N) with:
-`cat /sys/module/nvme_core/parameters/multipath`
-or
-`systool -m nvme_core -A multipath`
+NVMe multipath is natively supported by the Linux kernel. If for some reason
+you prefer using device mapper multipath with NVMe devices,
+you need to disable native multipathing first:
+
+ echo "options nvme_core multipath=N" > /etc/modprobe.d/01-nvme_core-mp.conf
+
+Afterwards, regenerate the initramfs (`dracut -f` or `update-initramfs`) and reboot.
+
+Using multipath-tools with native NVMe multipath
+------------------------------------------------
+
+If native NVMe multipathing is enabled, you can still use multipath-tools
+for displaying the topology and some other information about native NVMe
+multipath setups. This feature is disabled by default. To enable it, set
+`enable_foreign nvme` in the `defaults` section of `multipath.conf`.
+Commands like `multipath -ll` will then display information about NVMe
+native multipath. This support is read-only; modifying the native multipath
+configuration is not supported.
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 13/14] README.md: Extend the section about NVMe
2023-10-26 17:41 ` [PATCH v2 13/14] README.md: Extend the section about NVMe mwilck
@ 2023-10-27 19:34 ` Benjamin Marzinski
0 siblings, 0 replies; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 19:34 UTC (permalink / raw)
To: mwilck; +Cc: Christophe Varoqui, dm-devel, Xose Vazquez Perez
On Thu, Oct 26, 2023 at 07:41:52PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Cc: Xose Vazquez Perez <xose.vazquez@gmail.com>
> ---
> README.md | 30 ++++++++++++++++++++----------
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/README.md b/README.md
> index b2ed1f2..2fb423b 100644
> --- a/README.md
> +++ b/README.md
> @@ -273,15 +273,25 @@ To enable ALUA, the following options should be changed:
>
> NVMe
> ====
> -To use Device Mapper/multipath-tools with NVMe devices,
> -if the Native NVMe Multipath subsystem is enabled
> -( "Y" in `/sys/module/nvme_core/parameters/multipath` ),
> -it has to be disabled:
>
> -`echo "options nvme_core multipath=N" > /etc/modprobe.d/01-nvme_core-mp.conf`,
> -regenerate the initramfs (`dracut -f` or `update-initramfs`) and reboot.
> +Using dm-multipath with NVMe
> +----------------------------
>
> -Check that it is disabled(N) with:
> -`cat /sys/module/nvme_core/parameters/multipath`
> -or
> -`systool -m nvme_core -A multipath`
> +NVMe multipath is natively supported by the Linux kernel. If for some reason
> +you prefer using device mapper multipath with NVMe devices,
> +you need to disable native multipathing first:
> +
> + echo "options nvme_core multipath=N" > /etc/modprobe.d/01-nvme_core-mp.conf
> +
> +Afterwards, regenerate the initramfs (`dracut -f` or `update-initramfs`) and reboot.
> +
> +Using multipath-tools with native NVMe multipath
> +------------------------------------------------
> +
> +If native NVMe multipathing is enabled, you can still use multipath-tools
> +for displaying the topology and some other information about native NVMe
> +multipath setups. This feature is disabled by default. To enable it, set
> +`enable_foreign nvme` in the `defaults` section of `multipath.conf`.
> +Commands like `multipath -ll` will then display information about NVMe
> +native multipath. This support is read-only; modifying the native multipath
> +configuration is not supported.
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 14/14] README.md: fix formatting of Changelog section
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
` (12 preceding siblings ...)
2023-10-26 17:41 ` [PATCH v2 13/14] README.md: Extend the section about NVMe mwilck
@ 2023-10-26 17:41 ` mwilck
2023-10-27 19:35 ` Benjamin Marzinski
13 siblings, 1 reply; 30+ messages in thread
From: mwilck @ 2023-10-26 17:41 UTC (permalink / raw)
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Martin Wilck, Xose Vazquez Perez
From: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Cc: Xose Vazquez Perez <xose.vazquez@gmail.com>
---
README.md | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/README.md b/README.md
index 2fb423b..0c9f51e 100644
--- a/README.md
+++ b/README.md
@@ -223,8 +223,8 @@ file `libmultipath/hwtable.c`.
Changelog
=========
-pre-0.4.5: https://web.archive.org/web/20070309224034/http://christophe.varoqui.free.fr/wiki/wakka.php?wiki=ChangeLog
-post-0.4.5: https://github.com/opensvc/multipath-tools/commits/master
+* pre-0.4.5: https://web.archive.org/web/20070309224034/http://christophe.varoqui.free.fr/wiki/wakka.php?wiki=ChangeLog
+* post-0.4.5: https://github.com/opensvc/multipath-tools/commits/master
Maintainer
--
2.42.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 14/14] README.md: fix formatting of Changelog section
2023-10-26 17:41 ` [PATCH v2 14/14] README.md: fix formatting of Changelog section mwilck
@ 2023-10-27 19:35 ` Benjamin Marzinski
0 siblings, 0 replies; 30+ messages in thread
From: Benjamin Marzinski @ 2023-10-27 19:35 UTC (permalink / raw)
To: mwilck; +Cc: Christophe Varoqui, dm-devel, Xose Vazquez Perez
On Thu, Oct 26, 2023 at 07:41:53PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Cc: Xose Vazquez Perez <xose.vazquez@gmail.com>
> ---
> README.md | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/README.md b/README.md
> index 2fb423b..0c9f51e 100644
> --- a/README.md
> +++ b/README.md
> @@ -223,8 +223,8 @@ file `libmultipath/hwtable.c`.
> Changelog
> =========
>
> -pre-0.4.5: https://web.archive.org/web/20070309224034/http://christophe.varoqui.free.fr/wiki/wakka.php?wiki=ChangeLog
> -post-0.4.5: https://github.com/opensvc/multipath-tools/commits/master
> +* pre-0.4.5: https://web.archive.org/web/20070309224034/http://christophe.varoqui.free.fr/wiki/wakka.php?wiki=ChangeLog
> +* post-0.4.5: https://github.com/opensvc/multipath-tools/commits/master
>
>
> Maintainer
> --
> 2.42.0
^ permalink raw reply [flat|nested] 30+ messages in thread