Skip to content

Commit 5a01237

Browse files
cwize1cwize1
authored
Fix VHD parsing - Take 2. (#364)
Back in the Microsoft Virtual PC days, a VHD file's size was based on "Disk Geometry" instead of simple byte count. However, this method restricts the sizes that the disk can be. So, Hyper-V just ignores the "Disk Geometry" fields and uses the VHD's "Current Size" field instead. Unfortunately, `qemu-img` still by default outputs VHDs using "Disk Geometry". This generally means it has to round up the size. To make matters worse, the new disk size is almost never a multiple of 1 MiB, which can prevent this disk from being uploaded to Azure. When Image Customizer uses `qemu-img`, it knows to pass an option that tells `qemu-img` to use "Current Size" instead of "Disk Geometry". Specifically, `-o force_size=on`. But if a user manually calls `qemu-img`, they might not know they need to do this. This change adds logic to reject input VHDs that use "Disk Geometry" instead of "Current Size". This will make it easier to catch problematic disks earlier, instead of Image Customizer being blamed for the incorrect disk size.
1 parent 1ca3fbf commit 5a01237

File tree

8 files changed

+357
-24
lines changed

8 files changed

+357
-24
lines changed

‎docs/imagecustomizer/api/cli.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ But it can also be an Azure Linux image that has been customized.
3232

3333
Supported image file formats: vhd, vhdx, qcow2, and raw.
3434

35+
Note: Image Customizer will reject VHD files created by `qemu-img` unless the
36+
`-o force_size=on` option was passed. Without this option, `qemu-img` will
37+
likely change the size of the disk (to a non-multiple of 1 MiB), which can cause
38+
problems when trying to upload the disk to Azure.
39+
3540
If verity is enabled in the base image, then:
3641

3742
- If the partitions are recustomized using the

‎docs/imagecustomizer/api/configuration/inputImage.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ But it can also be an Azure Linux image that has been customized.
2727

2828
Supported image file formats: vhd, vhdx, qcow2, and raw.
2929

30+
Note: Image Customizer will reject VHD files created by `qemu-img` unless the
31+
`-o force_size=on` option was passed. Without this option, `qemu-img` will
32+
likely change the size of the disk (to a non-multiple of 1 MiB), which can cause
33+
problems when trying to upload the disk to Azure.
34+
3035
If verity is enabled in the base image, then:
3136

3237
- If the partitions are recustomized using the

‎toolkit/tools/internal/vhdutils/main_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
package vhdutils
5+
6+
import (
7+
"flag"
8+
"os"
9+
"path/filepath"
10+
"testing"
11+
12+
"github.com/microsoft/azurelinux/toolkit/tools/internal/logger"
13+
)
14+
15+
var (
16+
testsTempDir string
17+
)
18+
19+
func TestMain(m *testing.M) {
20+
var err error
21+
22+
logger.InitStderrLog()
23+
24+
flag.Parse()
25+
26+
workingDir, err := os.Getwd()
27+
if err != nil {
28+
logger.Log.Panicf("Failed to get working directory, error: %s", err)
29+
}
30+
31+
testsTempDir = filepath.Join(workingDir, "_tmp")
32+
33+
err = os.MkdirAll(testsTempDir, os.ModePerm)
34+
if err != nil {
35+
logger.Log.Panicf("Failed to create test temp directory, error: %s", err)
36+
}
37+
38+
retVal := m.Run()
39+
40+
err = os.RemoveAll(testsTempDir)
41+
if err != nil {
42+
logger.Log.Warnf("Failed to cleanup test temp dir (%s). Error: %s", testsTempDir, err)
43+
}
44+
45+
os.Exit(retVal)
46+
}

‎toolkit/tools/internal/vhdutils/vhdutils.go

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
package vhdutils
5+
6+
import (
7+
"bytes"
8+
"encoding/binary"
9+
"errors"
10+
"io"
11+
"os"
12+
)
13+
14+
const (
15+
VhdFileFooterSize = 512
16+
VhdFileSignature = "conectix"
17+
VhdFileVersion = 0x00010000
18+
)
19+
20+
type VhdFileFooter struct {
21+
Cookie [8]byte
22+
Features uint32
23+
FileFormatVersion uint32
24+
DataOffset uint64
25+
TimeStamp uint32
26+
CreatorApplication [4]byte
27+
CreatorVersion [4]byte
28+
CreatorHostOS [4]byte
29+
OriginalSize uint64
30+
CurrentSize uint64
31+
Cylinder uint16
32+
Heads uint8
33+
SectorsPerCylinder uint8
34+
DiskType uint32
35+
Checksum [4]byte
36+
UniqueId [16]byte
37+
SavedState uint8
38+
Reserved [427]byte
39+
}
40+
41+
var (
42+
ErrVhdFileTooSmall = errors.New("file is too small to be a VHD")
43+
ErrVhdWrongFileSignature = errors.New("footer does not have correct VHD file signature")
44+
ErrVhdWrongFileVersion = errors.New("VHD footer has unsupported file format version")
45+
)
46+
47+
type VhdFileSizeCalcType int
48+
49+
const (
50+
// File is not a VHD file.
51+
VhdFileSizeCalcTypeNone VhdFileSizeCalcType = iota
52+
// VHD uses "Current Size" field to calculate the disk size.
53+
VhdFileSizeCalcTypeCurrentSize
54+
// VHD uses "Disk Geometry" fields to calculate the disk size.
55+
VhdFileSizeCalcTypeDiskGeometry
56+
)
57+
58+
func GetVhdFileSizeCalcType(filename string) (VhdFileSizeCalcType, error) {
59+
footer, err := ParseVhdFileFooter(filename)
60+
if errors.Is(err, ErrVhdFileTooSmall) || errors.Is(err, ErrVhdWrongFileSignature) {
61+
// Not a VHD file.
62+
return VhdFileSizeCalcTypeNone, nil
63+
}
64+
if err != nil {
65+
return VhdFileSizeCalcTypeNone, err
66+
}
67+
68+
creatorApplication := string(footer.CreatorApplication[:])
69+
70+
// There are actually two different ways of calculating the disk size of a VHD file. The old method, which is
71+
// used by Microsoft Virtual PC, uses the VHD's footer's 'Disk Geometry' (cylinder, heads, and sectors per
72+
// track/cylinder) fields. Using 'Disk Geometry' limits what file sizes are possible. So, Hyper-V uses only uses the
73+
// the 'Current Size' field, which allows it to accept disks of any size.
74+
// Microsoft Virtual PC is pretty dead at this point. So, it is fairly safe to assume that almost all VHD files
75+
// use the Hyper-V format. Unfortunately, qemu-img still defaults to using 'Disk Geometry' when a user requests a
76+
// VHD (i.e. 'vpc') image. Image Customizer knows to pass the 'force_size=on' arg to qemu-img so that it uses
77+
// 'Current Size'. But users might not know that they need to do this when using qemu-img manually.
78+
// Fortunately, qemu-img is nice enough to use different values of the 'Creator Application' field depending on
79+
// the value of 'force_size'. Specifically, "qemu" for 'Disk Geometry' and "qem2 " for 'Current Size'. This can be
80+
// used to determine which type of VHD we are dealing with.
81+
// qemu-img uses the 'Creator Application' field internally to determine what type of VHD it is dealing with.
82+
// However, if it sees a 'Creator Application' value it doesn't recognize, it will assume it uses 'Disk Geometry'.
83+
// Whereas, nowadays it is more likely for a VHD to use 'Current Size'.
84+
switch creatorApplication {
85+
case "vpc ", "vs ", "qemu":
86+
return VhdFileSizeCalcTypeDiskGeometry, nil
87+
88+
default:
89+
return VhdFileSizeCalcTypeCurrentSize, nil
90+
}
91+
}
92+
93+
func ParseVhdFileFooter(filename string) (VhdFileFooter, error) {
94+
fd, err := os.Open(filename)
95+
if err != nil {
96+
return VhdFileFooter{}, err
97+
}
98+
defer fd.Close()
99+
100+
stat, err := fd.Stat()
101+
if err != nil {
102+
return VhdFileFooter{}, err
103+
}
104+
105+
if stat.Size() < VhdFileFooterSize {
106+
return VhdFileFooter{}, ErrVhdFileTooSmall
107+
}
108+
109+
_, err = fd.Seek(-VhdFileFooterSize, io.SeekEnd)
110+
if err != nil {
111+
return VhdFileFooter{}, err
112+
}
113+
114+
footerBytes := [VhdFileFooterSize]byte{}
115+
_, err = fd.Read([]byte(footerBytes[:]))
116+
if err != nil {
117+
return VhdFileFooter{}, err
118+
}
119+
120+
footerReader := bytes.NewReader(footerBytes[:])
121+
122+
var footer VhdFileFooter
123+
err = binary.Read(footerReader, binary.BigEndian, &footer)
124+
if err != nil {
125+
return VhdFileFooter{}, err
126+
}
127+
128+
if string(footer.Cookie[:]) != VhdFileSignature {
129+
return VhdFileFooter{}, ErrVhdWrongFileSignature
130+
}
131+
132+
if footer.FileFormatVersion != VhdFileVersion {
133+
return VhdFileFooter{}, ErrVhdWrongFileVersion
134+
}
135+
136+
return footer, nil
137+
}

‎toolkit/tools/internal/vhdutils/vhdutils_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
package vhdutils
5+
6+
import (
7+
"os"
8+
"path/filepath"
9+
"testing"
10+
11+
"github.com/microsoft/azurelinux/toolkit/tools/internal/file"
12+
"github.com/microsoft/azurelinux/toolkit/tools/internal/shell"
13+
"github.com/stretchr/testify/assert"
14+
)
15+
16+
func TestGetVhdFileSizeCalcTypeDiskGeometryDynamic(t *testing.T) {
17+
testGetVhdFileSizeCalcTypeHelper(t, "TestGetVhdFileSizeCalcTypeDiskGeometryDynamic", VhdFileSizeCalcTypeDiskGeometry,
18+
[]string{"-f", "vpc", "-o", "force_size=off,subformat=dynamic"})
19+
}
20+
21+
func TestGetVhdFileSizeCalcTypeDiskGeometryFixed(t *testing.T) {
22+
testGetVhdFileSizeCalcTypeHelper(t, "TestGetVhdFileSizeCalcTypeDiskGeometryFixed", VhdFileSizeCalcTypeDiskGeometry,
23+
[]string{"-f", "vpc", "-o", "force_size=off,subformat=fixed"})
24+
}
25+
26+
func TestGetVhdFileSizeCalcTypeCurrentSizeDynamic(t *testing.T) {
27+
testGetVhdFileSizeCalcTypeHelper(t, "TestGetVhdFileSizeCalcTypeCurrentSizeDynamic", VhdFileSizeCalcTypeCurrentSize,
28+
[]string{"-f", "vpc", "-o", "force_size=on,subformat=dynamic"})
29+
}
30+
31+
func TestGetVhdFileSizeCalcTypeCurrentSizeFixed(t *testing.T) {
32+
testGetVhdFileSizeCalcTypeHelper(t, "TestGetVhdFileSizeCalcTypeCurrentSizeFixed", VhdFileSizeCalcTypeCurrentSize,
33+
[]string{"-f", "vpc", "-o", "force_size=on,subformat=fixed"})
34+
}
35+
36+
func TestGetVhdFileSizeCalcTypeNoneVhdx(t *testing.T) {
37+
testGetVhdFileSizeCalcTypeHelper(t, "TestGetVhdFileSizeCalcTypeNoneVhdx", VhdFileSizeCalcTypeNone,
38+
[]string{"-f", "vhdx"})
39+
}
40+
41+
func TestGetVhdFileSizeCalcTypeNoneQcow2(t *testing.T) {
42+
testGetVhdFileSizeCalcTypeHelper(t, "TestGetVhdFileSizeCalcTypeNoneQcow2", VhdFileSizeCalcTypeNone,
43+
[]string{"-f", "qcow2"})
44+
}
45+
46+
func TestGetVhdFileSizeCalcTypeNoneRaw(t *testing.T) {
47+
testGetVhdFileSizeCalcTypeHelper(t, "TestGetVhdFileSizeCalcTypeNoneRaw", VhdFileSizeCalcTypeNone,
48+
[]string{"-f", "raw"})
49+
}
50+
51+
func testGetVhdFileSizeCalcTypeHelper(t *testing.T, testName string, expectedVhdFileSizeCalcType VhdFileSizeCalcType, qemuImgArgs []string) {
52+
qemuimgExists, err := file.CommandExists("qemu-img")
53+
assert.NoError(t, err)
54+
if !qemuimgExists {
55+
t.Skip("The 'qemu-img' command is not available")
56+
}
57+
58+
testTempDir := filepath.Join(testsTempDir, testName)
59+
testVhdFile := filepath.Join(testTempDir, "test.vhd")
60+
61+
err = os.MkdirAll(testTempDir, os.ModePerm)
62+
assert.NoError(t, err)
63+
64+
args := []string{"create", testVhdFile, "1M"}
65+
args = append(args, qemuImgArgs...)
66+
67+
err = shell.ExecuteLive(true, "qemu-img", args...)
68+
assert.NoError(t, err)
69+
70+
vhdType, err := GetVhdFileSizeCalcType(testVhdFile)
71+
assert.NoError(t, err)
72+
assert.Equal(t, expectedVhdFileSizeCalcType, vhdType)
73+
}

‎toolkit/tools/pkg/imagecustomizerlib/artifactsinputoutput.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,10 +292,9 @@ func InjectFiles(ctx context.Context, buildDir string, baseConfigPath string, in
292292
if err != nil {
293293
return err
294294
}
295-
inputImageFormat := strings.TrimLeft(filepath.Ext(inputImageFile), ".")
296295
rawImageFile := filepath.Join(buildDirAbs, BaseImageName)
297296

298-
detectedImageFormat, err := convertImageToRaw(inputImageFile, inputImageFormat, rawImageFile)
297+
detectedImageFormat, err := convertImageToRaw(inputImageFile, rawImageFile)
299298
if err != nil {
300299
return err
301300
}

‎toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/microsoft/azurelinux/toolkit/tools/internal/safeloopback"
2424
"github.com/microsoft/azurelinux/toolkit/tools/internal/safemount"
2525
"github.com/microsoft/azurelinux/toolkit/tools/internal/shell"
26+
"github.com/microsoft/azurelinux/toolkit/tools/internal/vhdutils"
2627
"github.com/sirupsen/logrus"
2728
"go.opentelemetry.io/otel"
2829
"go.opentelemetry.io/otel/attribute"
@@ -486,7 +487,7 @@ func convertInputImageToWriteableFormat(ctx context.Context, ic *ImageCustomizer
486487
} else {
487488
logger.Log.Infof("Creating raw base image: %s", ic.rawImageFile)
488489

489-
_, err := convertImageToRaw(ic.inputImageFile, ic.inputImageFormat, ic.rawImageFile)
490+
_, err := convertImageToRaw(ic.inputImageFile, ic.rawImageFile)
490491
if err != nil {
491492
return nil, err
492493
}
@@ -495,9 +496,7 @@ func convertInputImageToWriteableFormat(ctx context.Context, ic *ImageCustomizer
495496
}
496497
}
497498

498-
func convertImageToRaw(inputImageFile string, inputImageFormat string,
499-
rawImageFile string,
500-
) (imagecustomizerapi.ImageFormatType, error) {
499+
func convertImageToRaw(inputImageFile string, rawImageFile string) (imagecustomizerapi.ImageFormatType, error) {
501500
imageInfo, err := GetImageFileInfo(inputImageFile)
502501
if err != nil {
503502
return "", fmt.Errorf("%w (file='%s'):\n%w", ErrDetectImageFormat, inputImageFile, err)
@@ -506,24 +505,26 @@ func convertImageToRaw(inputImageFile string, inputImageFormat string,
506505
detectedImageFormat := imageInfo.Format
507506
sourceArg := fmt.Sprintf("file.filename=%s", qemuImgEscapeOptionValue(inputImageFile))
508507

509-
// The fixed-size VHD format is just a raw disk file with small metadata footer appended to the end. Unfortunatley,
510-
// that footer doesn't contain a file signature (i.e. "magic number"). So, qemu-img can't correctly detect this
511-
// format and instead reports fixed-size VHDs as raw images. So, use the filename extension as a hint.
512-
if inputImageFormat == "vhd" && detectedImageFormat == "raw" {
513-
// Force qemu-img to treat the file as a VHD.
514-
detectedImageFormat = "vpc"
515-
}
516-
517-
if detectedImageFormat == "vpc" {
518-
// There are actually two different ways of calculating the disk size of a VHD file. The old method, which is
519-
// used by Microsoft Virtual PC, uses the VHD's footer's "Disk Geometry" (cylinder, heads, and sectors per
520-
// track/cylinder) fields. Whereas, the new method, which is used by Hyper-V, simply uses the VHD's footer's
521-
// "Current Size" field. The qemu-img tool does try to correctly detect which one is being used by looking at
522-
// the footer's "Creator Application" field. But if the tool that created the VHD uses a name that qemu-img
523-
// doesn't recognize, then the heuristic can pick the wrong one. This seems to be the case for VHDs downloaded
524-
// from Azure. For the Image Customizer tool, it is pretty safe to assume all VHDs use the Hyper-V format.
525-
// So, force qemu-img to use that format.
526-
sourceArg += ",driver=vpc,force_size_calc=current_size"
508+
if detectedImageFormat == "raw" || detectedImageFormat == "vpc" {
509+
// The fixed-size VHD format is just a raw disk file with small metadata footer appended to the end.
510+
// Unfortunatley, qemu-img doesn't look at the VHD footer when detecting file formats. So, it reports
511+
// fixed-sized VHDs as raw disk images. So, manually detect if a raw image is a VHD.
512+
vhdFileType, err := vhdutils.GetVhdFileSizeCalcType(inputImageFile)
513+
if err != nil {
514+
return "", err
515+
}
516+
517+
switch vhdFileType {
518+
case vhdutils.VhdFileSizeCalcTypeDiskGeometry:
519+
return "", fmt.Errorf("rejecting VHD file that uses 'Disk Geometry' based size:\npass '-o force_size=on' to qemu-img when outputting as 'vpc' (i.e. VHD)")
520+
521+
case vhdutils.VhdFileSizeCalcTypeCurrentSize:
522+
sourceArg += ",driver=vpc,force_size_calc=current_size"
523+
detectedImageFormat = "vpc"
524+
525+
default:
526+
// Not a VHD file.
527+
}
527528
}
528529

529530
err = shell.ExecuteLiveWithErr(1, "qemu-img", "convert", "-O", "raw", "--image-opts", sourceArg, rawImageFile)

0 commit comments

Comments
 (0)