2023-02-23 08:37:53

by zhanchengbin

[permalink] [raw]
Subject: [PATCH v2] lib/ext2fs: add some msg for io error

Add msgs to show whether there is eio in fsck process, when write and
fsync methods fail.

Signed-off-by: zhanchengbin <[email protected]>
---
v2->v1:
- Delete return 0.

lib/ext2fs/unix_io.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
index 3171c736..a6c85874 100644
--- a/lib/ext2fs/unix_io.c
+++ b/lib/ext2fs/unix_io.c
@@ -1265,12 +1265,16 @@ static errcode_t unix_write_byte(io_channel channel, unsigned long offset,
return errno;

actual = write(data->dev, buf, size);
+
if (actual < 0)
- return errno;
+ retval = errno;
if (actual != size)
- return EXT2_ET_SHORT_WRITE;
+ retval = EXT2_ET_SHORT_WRITE;

- return 0;
+ if (retval)
+ fprintf(stderr, "%s unix_write_byte error, error %d\n",
+ channel->name, errno);
+ return retval;
}

/*
@@ -1289,8 +1293,11 @@ static errcode_t unix_flush(io_channel channel)
retval = flush_cached_blocks(channel, data, 0);
#endif
#ifdef HAVE_FSYNC
- if (!retval && fsync(data->dev) != 0)
+ if (!retval && fsync(data->dev) != 0) {
+ fprintf(stderr, "%s flush error, error %d\n",
+ channel->name, errno);
return errno;
+ }
#endif
return retval;
}
--
2.31.1



2023-02-24 03:47:29

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH v2] lib/ext2fs: add some msg for io error

On Thu, Feb 23, 2023 at 05:01:11PM +0800, zhanchengbin wrote:
> Add msgs to show whether there is eio in fsck process, when write and
> fsync methods fail.
>
> Signed-off-by: zhanchengbin <[email protected]>

The libext2fs is a *library*. As such, well designed libraries do not
randomly write to stderr. Consider what might happen if there was a
curses based program that was calling libext2fs --- for example, like
the ext2ed program. Writing random errors to stderr is just *rude*. :-)

If what you're worried about errors from e2fsck, it's also not
necessary, since that's what the error handler callback is for.

Cheers,

- Ted

2023-02-24 07:58:51

by zhanchengbin

[permalink] [raw]
Subject: Re: [PATCH v2] lib/ext2fs: add some msg for io error

The e2fsck_handle_write_error is called only unix_write_blk64 and
unix_read_blk64 failed, but there is no failure message in unix_write_byte
and unix_flush.

Thanks,
- bin.

On 2023/2/24 11:47, Theodore Ts'o wrote:
> On Thu, Feb 23, 2023 at 05:01:11PM +0800, zhanchengbin wrote:
>> Add msgs to show whether there is eio in fsck process, when write and
>> fsync methods fail.
>>
>> Signed-off-by: zhanchengbin <[email protected]>
>
> The libext2fs is a *library*. As such, well designed libraries do not
> randomly write to stderr. Consider what might happen if there was a
> curses based program that was calling libext2fs --- for example, like
> the ext2ed program. Writing random errors to stderr is just *rude*. :-)
>
> If what you're worried about errors from e2fsck, it's also not
> necessary, since that's what the error handler callback is for.
>
> Cheers,
>
> - Ted
> .
>