From f66f710f2c67b1cfb0cf0b701a8b0b36d4259063 Mon Sep 17 00:00:00 2001 From: wokron Date: Tue, 8 Sep 2026 17:55:42 +0800 Subject: [PATCH 1/3] add retry logic in get_dev_info() --- include/ublk/detail/control.hpp | 41 +++++++++++++++++++++++---------- include/ublk/detail/retry.hpp | 37 +++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 12 deletions(-) create mode 100644 include/ublk/detail/retry.hpp diff --git a/include/ublk/detail/control.hpp b/include/ublk/detail/control.hpp index 6975e3e..f9206c3 100644 --- a/include/ublk/detail/control.hpp +++ b/include/ublk/detail/control.hpp @@ -6,9 +6,12 @@ #pragma once #include "ublk/detail/path.hpp" +#include "ublk/detail/retry.hpp" #include "ublk/detail/task.hpp" #include "ublk/raw.hpp" +#include #include +#include namespace ublk { namespace detail { @@ -20,19 +23,33 @@ struct control_get_dev_info_t { ex::task> invoke(int fd, uint32_t dev_id, ublksrv_ctrl_dev_info *info) { DevPathBuf buf(dev_id); - auto s = - raw::get_dev_info2(fd, dev_id, &buf, sizeof(buf), DEV_PATH_LEN) | - ex::then([&](int32_t r) noexcept { - *info = buf.payload; - return r; - }) | - ex::let_error([&](std::error_code ec) { - if (ec.value() != EOPNOTSUPP) { - throw std::system_error(ec, "get_dev_info2"); + bool ok = co_await retry( + [&]() -> ex::task> { + int32_t r = + co_await (raw::get_dev_info2(fd, dev_id, &buf, sizeof(buf), + DEV_PATH_LEN) | + ex::upon_error([](std::error_code ec) noexcept { + return -ec.value(); + })); + if (r >= 0) { + *info = buf.payload; + co_return true; } - return raw::get_dev_info(fd, dev_id, info); - }); - co_await std::move(s); + if (r == -EACCES) { + co_return false; + } + if (r == -EOPNOTSUPP) { + co_await raw::get_dev_info(fd, dev_id, info); + co_return true; + } + throw std::system_error(-r, std::generic_category(), + "get_dev_info2"); + }, + 20, 100); + if (!ok) { + throw std::system_error(EACCES, std::generic_category(), + "get_dev_info2"); + } } }; diff --git a/include/ublk/detail/retry.hpp b/include/ublk/detail/retry.hpp new file mode 100644 index 0000000..dfc6576 --- /dev/null +++ b/include/ublk/detail/retry.hpp @@ -0,0 +1,37 @@ +#pragma once + +#include "ublk/detail/task.hpp" +#include +#include +#include +#include + +namespace ublk { +namespace detail { + +namespace ex = condy::detail::ex; + +template +inline ex::task> retry(Fn fn, size_t max_retry, + int64_t sleep_ms) { + if (co_await fn()) { + co_return true; + } + __kernel_timespec ts = {}; + ts.tv_sec = sleep_ms / 1000; + ts.tv_nsec = (sleep_ms % 1000) * 1'000'000; + bool ok = false; + for (size_t i = 0; i < max_retry; i++) { + co_await (condy::async_timeout(&ts, 0, 0) | + ex::upon_error( + [](std::error_code ec) noexcept { return -ec.value(); })); + ok = co_await fn(); + if (ok) { + break; + } + } + co_return ok; +} + +} // namespace detail +} // namespace ublk \ No newline at end of file From e05d2915a345f44ea9c8aa387da7c29a7b77b10a Mon Sep 17 00:00:00 2001 From: wokron Date: Tue, 8 Sep 2026 18:02:53 +0800 Subject: [PATCH 2/3] remove sleep for unprivileged in tests --- tests/test_control.cpp | 7 ------- tests/test_daemon.cpp | 3 --- tests/test_io.cpp | 7 ------- tests/test_query.cpp | 3 --- 4 files changed, 20 deletions(-) diff --git a/tests/test_control.cpp b/tests/test_control.cpp index 2fce883..5ae0b92 100644 --- a/tests/test_control.cpp +++ b/tests/test_control.cpp @@ -33,9 +33,6 @@ TEST_CASE("test control") { unprivileged ? UBLK_F_UNPRIVILEGED_DEV : 0); ex::sync_wait(ex::starts_on(sched, ublk::add_dev(control_fd, &info))); REQUIRE(info.dev_id == dev_id); - if (unprivileged) { - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - } ex::sync_wait( ex::starts_on(sched, ublk::del_dev(control_fd, info.dev_id))); } @@ -62,10 +59,6 @@ TEST_CASE("test control") { [](const std::exception_ptr &) noexcept {}))); }); - if (unprivileged) { - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - } - SUBCASE("del_dev_async") { // TODO: Kernel bug // https://lore.kernel.org/linux-block/20260901142438.237586-1-yi1tang.yang@gmail.com/ diff --git a/tests/test_daemon.cpp b/tests/test_daemon.cpp index 4274ce8..02ba6bf 100644 --- a/tests/test_daemon.cpp +++ b/tests/test_daemon.cpp @@ -47,9 +47,6 @@ TEST_CASE("test daemon") { auto d2 = ublk::detail::defer([&]() noexcept { ex::sync_wait(ex::starts_on(sched, ublk::del_dev(control_fd, dev_id))); }); - if (unprivileged) { - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - } std::tie(applyed) = // NOLINTNEXTLINE(bugprone-unchecked-optional-access) diff --git a/tests/test_io.cpp b/tests/test_io.cpp index f75fd76..eabc639 100644 --- a/tests/test_io.cpp +++ b/tests/test_io.cpp @@ -80,10 +80,6 @@ TEST_CASE("test io") { ex::sync_wait( ex::starts_on(sched, ublk::del_dev(control_fd, dev_id))); }); - if (unprivileged) { - // Wait for udev to chown /dev/ublkcN. - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - } ublk_params params = {}; fill_params(params); @@ -236,9 +232,6 @@ TEST_CASE("test io") { ex::upon_error( [](const std::exception_ptr &) noexcept {}))); }); - if (unprivileged) { - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - } ublk_params params = {}; fill_params(params); diff --git a/tests/test_query.cpp b/tests/test_query.cpp index c0b67c8..f4aa286 100644 --- a/tests/test_query.cpp +++ b/tests/test_query.cpp @@ -74,9 +74,6 @@ TEST_CASE("test query - cached_get_dev_info cache miss") { auto d2 = ublk::detail::defer([&]() noexcept { ex::sync_wait(ex::starts_on(sched, ublk::del_dev(control_fd, dev_id))); }); - if (unprivileged) { - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - } // No cached info in the env -> miss -> fetch via syscall. ublksrv_ctrl_dev_info out{}; From 1f972f839bda1fe9eed5725b839616d272d08027 Mon Sep 17 00:00:00 2001 From: wokron Date: Tue, 8 Sep 2026 19:37:03 +0800 Subject: [PATCH 3/3] update doc statement --- docs/guide.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide.md b/docs/guide.md index 357cc24..16726b6 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -231,7 +231,7 @@ ex::sync_wait(ex::starts_on(sched, s)); ### Unprivileged Mode -The control commands provided by ublk-cpp support unprivileged mode natively, and this is transparent to upper-layer users. You only need one set of code to deal with both modes. (Of course, you may still need udev configuration logic similar to what [ublksrv describes](https://github.com/ublk-org/ublksrv#unprivileged-mode), and use sleep and retry to handle the race conditions caused by permission changes. But there is only so much ublk-cpp can do.) +The control commands provided by ublk-cpp support unprivileged mode natively, and this is transparent to upper-layer users. You only need one set of code to deal with both modes. (Of course, you may still need udev configuration logic similar to what [ublksrv describes](https://github.com/ublk-org/ublksrv#unprivileged-mode).) ### Raw APIs