From c384b910c0f16129a0374c1865ab9dc9124b1e5e Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Fri, 1 Nov 2024 22:27:27 +0200 Subject: [PATCH 1/8] Fix typos in comments Signed-off-by: Nir Soffer --- convert/convert.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/convert/convert.go b/convert/convert.go index bb5f6c5..8cfef34 100644 --- a/convert/convert.go +++ b/convert/convert.go @@ -11,7 +11,7 @@ import ( const BufferSize = 1024 * 1024 // Smaller value may increase the overhead of synchornizing multiple works. -// Larger value may be less efficient for smaller images. The defualt value +// Larger value may be less efficient for smaller images. The default value // gives good results for the lima default Ubuntu image. const SegmentSize = 32 * BufferSize @@ -80,7 +80,7 @@ type Converter struct { err error } -// New returns a new converter intialized from options. +// New returns a new converter initialized from options. func New(opts Options) (*Converter, error) { if err := opts.Validate(); err != nil { return nil, err @@ -113,7 +113,7 @@ func (c *Converter) nextSegment() (int64, int64, bool) { return start, c.offset, false } -// setError keeps the first error set. Setting the error signal other workes to +// setError keeps the first error set. Setting the error signal other workers to // abort the operation. func (c *Converter) setError(err error) { c.mutex.Lock() @@ -166,7 +166,7 @@ func (c *Converter) Convert(wa io.WriterAt, ra io.ReaderAt, size int64) error { } // EOF for the last read of the last segment is expected, but since we - // read exactly size bytes, we shoud never get a zero read. + // read exactly size bytes, we should never get a zero read. if nr == 0 { c.setError(errors.New("unexpected EOF")) return From 9e4c6b931faa61e67b5f5779ed0e78d7b57c7b3c Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sat, 26 Oct 2024 22:25:32 +0300 Subject: [PATCH 2/8] Fix errors formatting when index >= len(table) When validating table index, if the index was too long, we formatted the table using %d instead of the length. Testing show that this format the entire table which is way too big to include in an error message. Signed-off-by: Nir Soffer --- image/qcow2/qcow2.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/image/qcow2/qcow2.go b/image/qcow2/qcow2.go index 978cea1..e46c6e8 100644 --- a/image/qcow2/qcow2.go +++ b/image/qcow2/qcow2.go @@ -707,7 +707,7 @@ func (img *Qcow2) readAtAligned(p []byte, off int64) (int, error) { } l1Index := int((off / int64(img.clusterSize)) / int64(l2Entries)) if l1Index >= len(img.l1Table) { - return 0, fmt.Errorf("index %d exceeds the L1 table length %d", l1Index, img.l1Table) + return 0, fmt.Errorf("index %d exceeds the L1 table length %d", l1Index, len(img.l1Table)) } l1Entry := img.l1Table[l1Index] l2TableOffset := l1Entry.l2Offset() @@ -726,7 +726,7 @@ func (img *Qcow2) readAtAligned(p []byte, off int64) (int, error) { return 0, fmt.Errorf("failed to read extended L2 table for L1 entry %v (index %d): %w", l1Entry, l1Index, err) } if l2Index >= len(extL2Table) { - return 0, fmt.Errorf("index %d exceeds the extended L2 table length %d", l2Index, extL2Table) + return 0, fmt.Errorf("index %d exceeds the extended L2 table length %d", l2Index, len(extL2Table)) } extL2Entry = &extL2Table[l2Index] l2Entry = extL2Entry.L2TableEntry @@ -736,7 +736,7 @@ func (img *Qcow2) readAtAligned(p []byte, off int64) (int, error) { return 0, fmt.Errorf("failed to read L2 table for L1 entry %v (index %d): %w", l1Entry, l1Index, err) } if l2Index >= len(l2Table) { - return 0, fmt.Errorf("index %d exceeds the L2 table length %d", l2Index, l2Table) + return 0, fmt.Errorf("index %d exceeds the L2 table length %d", l2Index, len(l2Table)) } l2Entry = l2Table[l2Index] } From 5b8959abd15ed3c3ed5eae3de69c0da11461de86 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sat, 26 Oct 2024 17:08:21 +0300 Subject: [PATCH 3/8] Add qemuimg.Create() helper Extract qemuimg() helper for running qemu-img command and add a wrapper for the qemu-img crate sub command. Signed-off-by: Nir Soffer --- test/qemuimg/qemuimg.go | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/test/qemuimg/qemuimg.go b/test/qemuimg/qemuimg.go index b01b695..80fcdef 100644 --- a/test/qemuimg/qemuimg.go +++ b/test/qemuimg/qemuimg.go @@ -2,8 +2,9 @@ package qemuimg import ( "bytes" - "errors" + "fmt" "os/exec" + "strconv" ) type CompressionType string @@ -26,18 +27,30 @@ func Convert(src, dst string, dstFormat Format, compressionType CompressionType) args = append(args, "-c", "-o", "compression_type="+string(compressionType)) } args = append(args, src, dst) - cmd := exec.Command("qemu-img", args...) + _, err := qemuImg(args) + return err +} +func Create(path string, format Format, size int64, backingFile string, backingFormat Format) error { + args := []string{"create", "-f", string(format)} + if backingFile != "" { + args = append(args, "-b", backingFile, "-F", string(backingFormat)) + } + args = append(args, path, strconv.FormatInt(size, 10)) + _, err := qemuImg(args) + return err +} + +func qemuImg(args []string) ([]byte, error) { + cmd := exec.Command("qemu-img", args...) var stderr bytes.Buffer cmd.Stderr = &stderr - - if err := cmd.Run(); err != nil { - // Return qemu-img stderr instead of the unhelpful default error (exited - // with status 1). + out, err := cmd.Output() + if err != nil { if _, ok := err.(*exec.ExitError); ok { - return errors.New(stderr.String()) + return out, fmt.Errorf("%w: stderr=%q", err, stderr.String()) } - return err + return out, err } - return nil + return out, nil } From ca0cc664d2226e5aeafe3341d2483c4b8d260e87 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sat, 2 Nov 2024 18:04:33 +0200 Subject: [PATCH 4/8] Add qemuio test package To qcow2 test images, we used raw image and converted to qcow2 format. This does not work for creating qcow2 chain with some data in the backing file and some data in the top image. qemu-io allows writing data, zeros or discarding a byte range in an image of any format supported by qemu. This change add a tiny test package for running qemu-io. We provide 2 functions, wrapping usage of the write command with different flags: - Write() - write a pattern to specified range - Zero() - write zeros to specified range - Discard() - discard specified range With these functions we can test all kind of extents in both top layer or the backing file: - Allocated: false, Zero: true - Allocated: true, Zero: false - Allocated: true, Zero: true Signed-off-by: Nir Soffer --- test/qemuio/qemuio.go | 52 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 test/qemuio/qemuio.go diff --git a/test/qemuio/qemuio.go b/test/qemuio/qemuio.go new file mode 100644 index 0000000..f49ebe8 --- /dev/null +++ b/test/qemuio/qemuio.go @@ -0,0 +1,52 @@ +package qemuio + +import ( + "bytes" + "fmt" + "os/exec" + + "github.com/lima-vm/go-qcow2reader/test/qemuimg" +) + +// Write writes a number of bytes at a specified offset, allocating all clusters +// in specified range. +func Write(path string, format qemuimg.Format, off, len int64, pattern byte) error { + // qemu-io -f qcow2 -c 'write -P pattern off len' disk.qcow2 + command := fmt.Sprintf("write -P %d %d %d", pattern, off, len) + _, err := qemuIo([]string{"-f", string(format), "-c", command, path}) + return err +} + +// Zero writes zeros at a specified offset. The behavior depens on qcow2 +// version; In qcow2 v3, allocate zero clusters, marming entire cluster as zero. +// In qcow2 v2, if the cluster are unallocated and there is no backing file, do +// nothing. Otherwise allocate clusters and write actual zeros. +func Zero(path string, format qemuimg.Format, off, len int64) error { + // qemu-io -f qcow2 -c 'write -z off len' disk.qcow2 + command := fmt.Sprintf("write -z %d %d", off, len) + _, err := qemuIo([]string{"-f", string(format), "-c", command, path}) + return err +} + +// Discard unmap number of bytes at specified offset. Allocated cluster are +// deaallocated and replaced with zero clusters. +func Discard(path string, format qemuimg.Format, off, len int64, unmap bool) error { + // qemu-io -f qcow2 -c 'write -zu off len' disk.qcow2 + command := fmt.Sprintf("write -zu %d %d", off, len) + _, err := qemuIo([]string{"-f", string(format), "-c", command, path}) + return err +} + +func qemuIo(args []string) ([]byte, error) { + cmd := exec.Command("qemu-io", args...) + var stderr bytes.Buffer + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil { + if _, ok := err.(*exec.ExitError); ok { + return out, fmt.Errorf("%w: stderr=%q", err, stderr.String()) + } + return out, err + } + return out, nil +} From 77ff9696339fec38f8434fd6b33188eb89ab122b Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sat, 26 Oct 2024 22:09:46 +0300 Subject: [PATCH 5/8] Make qemu-img and qemu-io available for unit tests We want to use qemu-img and qemu-io to create and manipulate test images. Signed-off-by: Nir Soffer --- .github/workflows/test.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 575d673..76adb52 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,14 +20,14 @@ jobs: with: version: v1.52.2 args: --verbose - - name: Unit tests - run: go test -v ./... - - name: Install go-qcow2reader-example - run: cd ./cmd/go-qcow2reader-example && go install - name: Install qemu-img as a test dependency run: | sudo apt-get update sudo apt-get install -y qemu-utils + - name: Unit tests + run: go test -v ./... + - name: Install go-qcow2reader-example + run: cd ./cmd/go-qcow2reader-example && go install - name: Cache test-images-ro id: cache-test-images-ro uses: actions/cache@v4 From 2682794ad6f040a6d801aad3f10b97bceefc2cdc Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sun, 27 Oct 2024 00:46:48 +0300 Subject: [PATCH 6/8] Separate cluster metadata from reading Extract a helper for getting cluster metadata from readAtAligned. The helper will be reused for getting cluster status. Signed-off-by: Nir Soffer --- image/qcow2/qcow2.go | 101 +++++++++++++++++++++++++++++++------------ 1 file changed, 73 insertions(+), 28 deletions(-) diff --git a/image/qcow2/qcow2.go b/image/qcow2/qcow2.go index e46c6e8..545c4d0 100644 --- a/image/qcow2/qcow2.go +++ b/image/qcow2/qcow2.go @@ -699,56 +699,101 @@ func (img *Qcow2) getL2Table(l1Entry l1TableEntry) ([]l2TableEntry, error) { return l2Table, nil } -// readAtAligned requires that off and off+len(p)-1 belong to the same cluster. -func (img *Qcow2) readAtAligned(p []byte, off int64) (int, error) { - l2Entries := img.clusterSize / 8 +type clusterMeta struct { + L2Entries int + + // L1 info. + L1Index int + L1Entry l1TableEntry + + // L2 info. + L2Index int + L2Entry l2TableEntry + + // Extended L2 info. + ExtL2Entry extendedL2TableEntry + + // Cluster is not allocated in this image, but it may be allocated in the + // backing file. + Allocated bool + + // Cluster is present in this image and is compressed. + Compressed bool + + // Cluster is present in this image and is all zeros. + Zero bool +} + +func (img *Qcow2) getClusterMeta(off int64, cm *clusterMeta) error { + cm.L2Entries = img.clusterSize / 8 if img.extendedL2() { - l2Entries = img.clusterSize / 16 + cm.L2Entries = img.clusterSize / 16 } - l1Index := int((off / int64(img.clusterSize)) / int64(l2Entries)) - if l1Index >= len(img.l1Table) { - return 0, fmt.Errorf("index %d exceeds the L1 table length %d", l1Index, len(img.l1Table)) + cm.L1Index = int((off / int64(img.clusterSize)) / int64(cm.L2Entries)) + if cm.L1Index >= len(img.l1Table) { + return fmt.Errorf("index %d exceeds the L1 table length %d", cm.L1Index, len(img.l1Table)) } - l1Entry := img.l1Table[l1Index] - l2TableOffset := l1Entry.l2Offset() + + cm.L1Entry = img.l1Table[cm.L1Index] + l2TableOffset := cm.L1Entry.l2Offset() if l2TableOffset == 0 { - return img.readAtAlignedUnallocated(p, off) + return nil } - l2Index := int((off / int64(img.clusterSize)) % int64(l2Entries)) - var ( - extL2Entry *extendedL2TableEntry - l2Entry l2TableEntry - ) + + cm.L2Index = int((off / int64(img.clusterSize)) % int64(cm.L2Entries)) + if img.extendedL2() { // TODO extL2Table, err := readExtendedL2Table(img.ra, l2TableOffset, img.clusterSize) if err != nil { - return 0, fmt.Errorf("failed to read extended L2 table for L1 entry %v (index %d): %w", l1Entry, l1Index, err) + return fmt.Errorf("failed to read extended L2 table for L1 entry %v (index %d): %w", cm.L1Entry, cm.L1Index, err) } - if l2Index >= len(extL2Table) { - return 0, fmt.Errorf("index %d exceeds the extended L2 table length %d", l2Index, len(extL2Table)) + if cm.L2Index >= len(extL2Table) { + return fmt.Errorf("index %d exceeds the extended L2 table length %d", cm.L2Index, len(extL2Table)) } - extL2Entry = &extL2Table[l2Index] - l2Entry = extL2Entry.L2TableEntry + cm.ExtL2Entry = extL2Table[cm.L2Index] + cm.L2Entry = cm.ExtL2Entry.L2TableEntry } else { - l2Table, err := img.getL2Table(l1Entry) + l2Table, err := img.getL2Table(cm.L1Entry) if err != nil { - return 0, fmt.Errorf("failed to read L2 table for L1 entry %v (index %d): %w", l1Entry, l1Index, err) + return fmt.Errorf("failed to read L2 table for L1 entry %v (index %d): %w", cm.L1Entry, cm.L1Index, err) } - if l2Index >= len(l2Table) { - return 0, fmt.Errorf("index %d exceeds the L2 table length %d", l2Index, len(l2Table)) + if cm.L2Index >= len(l2Table) { + return fmt.Errorf("index %d exceeds the L2 table length %d", cm.L2Index, len(l2Table)) } - l2Entry = l2Table[l2Index] + cm.L2Entry = l2Table[cm.L2Index] } - desc := l2Entry.clusterDescriptor() + + desc := cm.L2Entry.clusterDescriptor() if desc == 0 && !img.extendedL2() { + return nil + } + + cm.Allocated = true + if cm.L2Entry.compressed() { + cm.Compressed = true + } else { + cm.Zero = standardClusterDescriptor(desc).allZero() + } + + return nil +} + +// readAtAligned requires that off and off+len(p)-1 belong to the same cluster. +func (img *Qcow2) readAtAligned(p []byte, off int64) (int, error) { + var cm clusterMeta + if err := img.getClusterMeta(off, &cm); err != nil { + return 0, err + } + if !cm.Allocated { return img.readAtAlignedUnallocated(p, off) } var ( n int err error ) - if l2Entry.compressed() { + desc := cm.L2Entry.clusterDescriptor() + if cm.Compressed { compressedDesc := compressedClusterDescriptor(desc) n, err = img.readAtAlignedCompressed(p, off, compressedDesc) if err != nil { @@ -757,7 +802,7 @@ func (img *Qcow2) readAtAligned(p []byte, off int64) (int, error) { } else { standardDesc := standardClusterDescriptor(desc) if img.extendedL2() { - n, err = img.readAtAlignedStandardExtendedL2(p, off, standardDesc, *extL2Entry) + n, err = img.readAtAlignedStandardExtendedL2(p, off, standardDesc, cm.ExtL2Entry) if err != nil { err = fmt.Errorf("failed to read standard cluster with Extended L2 (len=%d, off=%d, desc=0x%X): %w", len(p), off, desc, err) } From ab791cc8220787b743348abf0b89366adddad15d Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 28 Oct 2024 03:05:10 +0200 Subject: [PATCH 7/8] Cleanup getClusterMeta Computing number of L2 entires for every cluster does not feel right since this is a constant value per image. Computing it once after reading the header. Computing L1 index and L2 index use the cluster number. Make the computation more clear by introducing a variable. This change also speed up empty image processing by 10%, since we actually have a tight loop over all image clusters. Signed-off-by: Nir Soffer --- image/qcow2/qcow2.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/image/qcow2/qcow2.go b/image/qcow2/qcow2.go index 545c4d0..3625d1b 100644 --- a/image/qcow2/qcow2.go +++ b/image/qcow2/qcow2.go @@ -536,6 +536,7 @@ type Qcow2 struct { HeaderExtensions []HeaderExtension `json:"header_extensions"` errUnreadable error clusterSize int + l2Entries int l1Table []l1TableEntry l2TableCache *lru.Cache[l1TableEntry, []l2TableEntry] decompressor Decompressor @@ -585,6 +586,13 @@ func Open(ra io.ReaderAt, openWithType image.OpenWithType) (*Qcow2, error) { } } + // Used to get cluster metadata. + if img.extendedL2() { + img.l2Entries = img.clusterSize / 16 + } else { + img.l2Entries = img.clusterSize / 8 + } + // Load L1 table img.l1Table, err = readL1Table(ra, img.Header.L1TableOffset, img.Header.L1Size) if err != nil { @@ -700,8 +708,6 @@ func (img *Qcow2) getL2Table(l1Entry l1TableEntry) ([]l2TableEntry, error) { } type clusterMeta struct { - L2Entries int - // L1 info. L1Index int L1Entry l1TableEntry @@ -725,11 +731,8 @@ type clusterMeta struct { } func (img *Qcow2) getClusterMeta(off int64, cm *clusterMeta) error { - cm.L2Entries = img.clusterSize / 8 - if img.extendedL2() { - cm.L2Entries = img.clusterSize / 16 - } - cm.L1Index = int((off / int64(img.clusterSize)) / int64(cm.L2Entries)) + clusterNo := off / int64(img.clusterSize) + cm.L1Index = int(clusterNo / int64(img.l2Entries)) if cm.L1Index >= len(img.l1Table) { return fmt.Errorf("index %d exceeds the L1 table length %d", cm.L1Index, len(img.l1Table)) } @@ -740,7 +743,7 @@ func (img *Qcow2) getClusterMeta(off int64, cm *clusterMeta) error { return nil } - cm.L2Index = int((off / int64(img.clusterSize)) % int64(cm.L2Entries)) + cm.L2Index = int(clusterNo % int64(img.l2Entries)) if img.extendedL2() { // TODO From 11847eb8088713980758050464905aafff702a23 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sun, 3 Nov 2024 16:29:51 +0200 Subject: [PATCH 8/8] Require go 1.22 This allows using useful new packages such as slices package and other fixes. This also matches lima requirement. Updating also golanci-lint based on lima test workflow. Signed-off-by: Nir Soffer --- .github/workflows/test.yml | 4 ++-- cmd/go-qcow2reader-example/go.mod | 2 +- go.mod | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 76adb52..b8d5b30 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,14 +11,14 @@ jobs: steps: - uses: actions/setup-go@v5 with: - go-version: 1.20.x + go-version: 1.22.x - uses: actions/checkout@v4 with: fetch-depth: 1 - name: Run golangci-lint uses: golangci/golangci-lint-action@v6.1.1 with: - version: v1.52.2 + version: v1.60.1 args: --verbose - name: Install qemu-img as a test dependency run: | diff --git a/cmd/go-qcow2reader-example/go.mod b/cmd/go-qcow2reader-example/go.mod index 1eb43db..1f10b56 100644 --- a/cmd/go-qcow2reader-example/go.mod +++ b/cmd/go-qcow2reader-example/go.mod @@ -1,6 +1,6 @@ module github.com/lima-vm/go-qcow2reader/cmd/go-qcow2reader-example -go 1.20 +go 1.22 require ( github.com/klauspost/compress v1.16.5 diff --git a/go.mod b/go.mod index 1738377..44a1c49 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/lima-vm/go-qcow2reader -go 1.20 +go 1.22