From 1182224c138b555a8474d01c046019f7017cdd9c Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sun, 3 Nov 2024 19:54:06 +0200 Subject: [PATCH] Do not return io.EOF when n == len(p) io.ReaderAt docs says[1]: > If the n = len(p) bytes returned by ReadAt are at the end of the input > source, ReadAt may return either err == EOF or err == nil. We chose to return EOF, and this confuses io.CopyBuffer, since it expects to get io.EOF, not an error wrapping io.EOF[2]: if er != nil { if er != EOF { err = er } break } We can fix this by never wrapping io.EOF, or by not returning io.EOF when we can avoid it. I chose the second way since it is much simpler. Fixed by returning EOF only if n < len(p) when reading zeros. This means that reading the last buffer will return len(p), nil, and trying to read the next buffer will return 0, EOF. To reproduce we need to write zero cluster at the end of the image: % qemu-img create -f qcow2 test.qcow2 1m Formatting 'test.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=1048576 lazy_refcounts=off refcount_bits=16 % qemu-io -f qcow2 -c 'write -z 960k 64k' test.qcow2 wrote 65536/65536 bytes at offset 983040 64 KiB, 1 ops; 00.00 sec (97.962 MiB/sec and 1567.3981 ops/sec) % qemu-img map --output json test.qcow2 [{ "start": 0, "length": 983040, "depth": 0, "present": false, "zero": true, "data": false, "compressed": false}, { "start": 983040, "length": 65536, "depth": 0, "present": true, "zero": true, "data": false, "compressed": false}] Without the fix we fail when reading the last 32k at 1015808: % ./go-qcow2reader-example read test.qcow2 >/dev/null ERROR: failed to read standard cluster (len=32768, off=1015808, desc=0x1): EOF I don't understand why we try to read with a buffer size of 32k, when we use a 64k buffer in the read example. This may be another bug. [1] https://pkg.go.dev/io#ReaderAt [2] https://cs.opensource.google/go/go/+/refs/tags/go1.23.2:src/io/io.go;l=449 Fixes: #40 Signed-off-by: Nir Soffer --- image/qcow2/qcow2.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/image/qcow2/qcow2.go b/image/qcow2/qcow2.go index 3625d1b..8ee3370 100644 --- a/image/qcow2/qcow2.go +++ b/image/qcow2/qcow2.go @@ -944,7 +944,11 @@ func (img *Qcow2) readZero(p []byte, off int64) (int, error) { func readZero(p []byte, off int64, sz uint64) (int, error) { var err error l := len(p) - if uint64(off+int64(l)) >= sz { + // If the n = len(p) bytes returned by ReadAt are at the end of the input + // source, ReadAt may return either err == EOF or err == nil. Returning io.EOF + // seems to confuse io.SectionReader so we return EOF only for out of bound + // request. + if uint64(off+int64(l)) > sz { l = int(sz - uint64(off)) if l < 0 { l = 0