Skip to content

Commit

Permalink
Do not return io.EOF when n == len(p)
Browse files Browse the repository at this point in the history
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 <nirsof@gmail.com>
  • Loading branch information
nirs committed Nov 3, 2024
1 parent 8255b8a commit 1182224
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion image/qcow2/qcow2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1182224

Please sign in to comment.