A little patch fix dev_read to make it works with large file

[chenlinxuan]

 From 83e965dc6ec45e5c9811a27023c9cc213d50816b Mon Sep 17 00:00:00 2001
From: Chen Linxuan <chenlinxuan at uniontech.com>
Date: Thu, 20 Oct 2022 17:53:03 +0800
Subject: [PATCH] erofs-utils: erofs-utils: lib: fix dev_read

When using `fsck.erofs` to extract some image have a very large file
inside, for example 2G, my fsck.erofs report some thing like this:

<E> erofs_io: Failed to read data from device - erofs.image:[4096,
2147483648].
<E> erofs: failed to read data of m_pa 4096, m_plen 2147483648 @ nid 40: -17
<E> erofs: Failed to extract filesystem

You can use this script to reproduce this issue.

mkdir tmp extract
dd if=/dev/urandom of=tmp/big_file bs=1M count=2048

mkfs.erofs erofs.image tmp
fsck.erofs erofs.image --extract=extract

I found that dev_open will failed if we can not get all data we want
with one pread call.

I write this little patch try to fix this issue.

Signed-off-by: Chen Linxuan <chenlinxuan at uniontech.com>
---
  lib/io.c | 28 +++++++++++++++++++++-------
  1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/lib/io.c b/lib/io.c
index 524cfb4..5cb2b3a 100644
--- a/lib/io.c
+++ b/lib/io.c
@@ -256,7 +256,7 @@ int dev_resize(unsigned int blocks)

  int dev_read(int device_id, void *buf, u64 offset, size_t len)
  {
-    int ret, fd;
+    int read_count, fd;

      if (cfg.c_dry_run)
          return 0;
@@ -278,15 +278,29 @@ int dev_read(int device_id, void *buf, u64 offset, 
size_t len)
          fd = erofs_blobfd[device_id - 1];
      }

+    while (len > 0) {
  #ifdef HAVE_PREAD64
-    ret = pread64(fd, buf, len, (off64_t)offset);
+        read_count = pread64(fd, buf, len, (off64_t)offset);
  #else
-    ret = pread(fd, buf, len, (off_t)offset);
+        read_count = pread(fd, buf, len, (off_t)offset);
  #endif
-    if (ret != (int)len) {
-        erofs_err("Failed to read data from device - %s:[%" PRIu64 ", 
%zd].",
-              erofs_devname, offset, len);
-        return -errno;
+        if (read_count == -1 || read_count == 0) {
+            if (errno) {
+                erofs_err("Failed to read data from device - "
+                      "%s:[%" PRIu64 ", %zd].",
+                      erofs_devname, offset, len);
+                return -errno;
+            } else {
+                erofs_err("Reach EOF of device - "
+                      "%s:[%" PRIu64 ", %zd].",
+                      erofs_devname, offset, len);
+                return -EINVAL;
+            }
+        }
+
+        offset += read_count;
+        len -= read_count;
+        buf += read_count;
      }
      return 0;
  }
-- 
2.37.3

On 2022/10/20 17:24, Gao Xiang wrote:
> Hi LinXuan,
> 
> On Thu, Oct 20, 2022 at 04:12:57PM +0800, chenlinxuan wrote:
>> When using `fsck.erofs` to extract some image have a very large file (3G)
>> inside, my fsck.erofs report some thing like:
>>
>> <E> erofs_io: Failed to read data from device - erofs.image:[4096,
>> 2147483648].
>> <E> erofs: failed to read data of m_pa 4096, m_plen 2147483648 @ nid 40: -17
>> <E> erofs: Failed to extract filesystem
>>
>> You can use this script to reproduce this issue.
>>
>> #!/bin/env sh
>>
>> mkdir tmp extract
>> dd if=/dev/urandom of=tmp/big_file bs=1M count=2048
>>
>> mkfs.erofs erofs.image tmp
>> fsck.erofs erofs.image --extract=extract
>>
>> I found that dev_open will failed if we can not get all data we want with
>> one pread call.
>>
>> I write this little patch try to fix this issue.
>>
>> This is my first patch send via email, sorry if anything goes wrong.
>>
> 
> Thanks for your report, and the issue looks true.
> 
> In order to merge this patch, could you apply the message above into
> the commit message?
> 
> BTW, the commit message can be updated with "git commit --amend".
> 
>>  From 156af5b173c1f9e2a91e4d2126214b96966babd1 Mon Sep 17 00:00:00 2001
>> From: chenlinxuan <chenlinxuan at uniontech.com>
> 
> It would be better to update your real name into
> "Linxuan Chen" Or "Chen Linxuan" (whichever you prefer).
> 
>> Date: Thu, 20 Oct 2022 14:39:17 +0800
>> Subject: [PATCH] erofs-utils: lib: fix dev_read
> 
> Subject title:
> "erofs-utils: lib: fix dev_read to make it works with large file"
> 
>>
>> We need to keep calling pread until we get all data we want
>> ---
>>   lib/io.c | 28 +++++++++++++++++++++-------
>>   1 file changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/io.c b/lib/io.c
>> index 524cfb4..bd3d790 100644
>> --- a/lib/io.c
>> +++ b/lib/io.c
>> @@ -256,7 +256,7 @@ int dev_resize(unsigned int blocks)
>>
>>   int dev_read(int device_id, void *buf, u64 offset, size_t len)
>>   {
>> -   int ret, fd;
>> +   int read_count, fd;
>>
>>      if (cfg.c_dry_run)
>>          return 0;
>> @@ -278,15 +278,29 @@ int dev_read(int device_id, void *buf, u64 offset,
>> size_t len)
>>          fd = erofs_blobfd[device_id - 1];
>>      }
>>
>> +   while (len > 0) {
>>   #ifdef HAVE_PREAD64
>> -   ret = pread64(fd, buf, len, (off64_t)offset);
>> +       read_count = pread64(fd, buf, len, (off64_t)offset);
>>   #else
>> -   ret = pread(fd, buf, len, (off_t)offset);
>> +       read_count = pread(fd, buf, len, (off_t)offset);
>>   #endif
>> -   if (ret != (int)len) {
>> -       erofs_err("Failed to read data from device - %s:[%" PRIu64 ",
>> %zd].",
>> -             erofs_devname, offset, len);
>> -       return -errno;
>> +       if (read_count == -1 || read_count ==  0) {
> 
>                                                  ^ extra space here.
> 
> Thanks,
> Gao Xiang
>