Hi Denis,
On 2026-07-16T22:33:54, None <[email protected]> wrote:
> tests: add blk_dflush() coverage
>
> Add minimal test invoking new blk_dflush() API.
>
> Signed-off-by: Denis Mukhin <[email protected]>
>
> arch/sandbox/cpu/os.c | 5 +++++
> drivers/block/sandbox.c | 9 +++++++++
> include/os.h | 8 ++++++++
> test/dm/host.c | 2 ++
> test/dm/mmc.c | 2 ++
> 5 files changed, 26 insertions(+)
> tests: add blk_dflush() coverage
The subject says tests, but this patch also adds os_fsync() and
implements flush support in the sandbox host block driver. Please can you
split the sandbox bits into its own sandbox: patch, before the tests?
> diff --git a/drivers/block/sandbox.c b/drivers/block/sandbox.c
> @@ -52,9 +52,18 @@ static unsigned long host_block_write(struct udevice *dev,
> +unsigned long host_block_flush(struct udevice *dev)
> +{
This should be static, to match host_block_read() and
host_block_write() - it is only referenced via the ops table.
> diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
> @@ -139,6 +139,11 @@ int os_close(int fd)
> +int os_fsync(int fd)
> +{
> + return fsync(fd);
> +}
The function comment in os.h says this returns a negative error code,
but fsync() returns -1 with errno set. Since blk_flush() propagates
this value to callers, please can you return -errno on failure so the
caller gets a meaningful error?
> diff --git a/test/dm/host.c b/test/dm/host.c
> @@ -62,6 +62,8 @@ static int dm_test_host(struct unit_test_state *uts)
> + ut_asserteq(0, blk_dflush(desc));
> +
This covers the direct blk_dflush() path, but nothing exercises the
partition-level disk_blk_flush() path added in patch 2, where flushing
a partition flushes the whole device. Would a test for that be
worthwhile?
Regards,
Simon
