Fix duplication of main disk in QEMU

This commit is contained in:
Ambrose Chua 2020-12-04 00:30:17 +08:00
parent f5d5e28012
commit 436ac8ef26
7 changed files with 124 additions and 26 deletions

View File

@ -554,11 +554,6 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) {
}
}
if c.DiskImage && len(c.AdditionalDiskSize) > 0 {
errs = packersdk.MultiErrorAppend(
errs, errors.New("disk_additional_size can only be used when disk_image is false"))
}
if c.SkipResizeDisk && !(c.DiskImage) {
errs = packersdk.MultiErrorAppend(
errs, errors.New("skip_resize_disk can only be used when disk_image is true"))

View File

@ -196,8 +196,8 @@ func TestBuilderPrepare_AdditionalDiskSize(t *testing.T) {
if len(warns) > 0 {
t.Fatalf("bad: %#v", warns)
}
if err == nil {
t.Fatalf("should have error")
if err != nil {
t.Fatalf("should not have error")
}
delete(config, "disk_image")

View File

@ -27,13 +27,18 @@ func (s *stepCopyDisk) Run(ctx context.Context, state multistep.StateBag) multis
ui := state.Get("ui").(packersdk.Ui)
path := filepath.Join(s.OutputDir, s.VMName)
// Only make a copy of the ISO as the 'main' or 'default' disk if is a disk
// image and not using a backing file. The create disk step (step_create_disk.go)
// would already have made the disk otherwise
if !s.DiskImage || s.UseBackingFile {
return multistep.ActionContinue
}
// isoPath extention is:
// In some cases, the file formats provided are equivalent by comparing the
// file extensions. Skip the conversion step
// This also serves as a workaround for a QEMU bug: https://bugs.launchpad.net/qemu/+bug/1776920
ext := filepath.Ext(isoPath)
if len(ext) >= 1 && ext[1:] == s.Format {
if len(ext) >= 1 && ext[1:] == s.Format && len(s.QemuImgArgs.Convert) == 0 {
ui.Message("File extension already matches desired output format. " +
"Skipping qemu-img convert step")
err := driver.Copy(isoPath, path)

View File

@ -28,11 +28,10 @@ func (s *stepCreateDisk) Run(ctx context.Context, state multistep.StateBag) mult
ui := state.Get("ui").(packersdk.Ui)
name := s.VMName
if s.DiskImage && !s.UseBackingFile {
return multistep.ActionContinue
if len(s.AdditionalDiskSize) > 0 || s.UseBackingFile {
ui.Say("Creating required virtual machine disks")
}
ui.Say("Creating required virtual machine disks")
// The 'main' or 'default' disk
diskFullPaths := []string{filepath.Join(s.OutputDir, name)}
diskSizes := []string{s.DiskSize}
@ -42,13 +41,17 @@ func (s *stepCreateDisk) Run(ctx context.Context, state multistep.StateBag) mult
for i, diskSize := range s.AdditionalDiskSize {
path := filepath.Join(s.OutputDir, fmt.Sprintf("%s-%d", name, i+1))
diskFullPaths = append(diskFullPaths, path)
size := diskSize
diskSizes = append(diskSizes, size)
diskSizes = append(diskSizes, diskSize)
}
}
// Create all required disks
for i, diskFullPath := range diskFullPaths {
if s.DiskImage && !s.UseBackingFile && i == 0 {
// Let the copy disk step (step_copy_disk.go) create the 'main' or
// 'default' disk.
continue
}
log.Printf("[INFO] Creating disk with Path: %s and Size: %s", diskFullPath, diskSizes[i])
command := s.buildCreateCommand(diskFullPath, diskSizes[i], i, state)
@ -70,7 +73,8 @@ func (s *stepCreateDisk) Run(ctx context.Context, state multistep.StateBag) mult
func (s *stepCreateDisk) buildCreateCommand(path string, size string, i int, state multistep.StateBag) []string {
command := []string{"create", "-f", s.Format}
if s.UseBackingFile && i == 0 {
if s.DiskImage && s.UseBackingFile && i == 0 {
// Use a backing file for the 'main' or 'default' disk
isoPath := state.Get("iso_path").(string)
command = append(command, "-b", isoPath)
}

View File

@ -1,6 +1,7 @@
package qemu
import (
"context"
"fmt"
"testing"
@ -27,17 +28,20 @@ func Test_buildCreateCommand(t *testing.T) {
},
{
&stepCreateDisk{
Format: "qcow2",
UseBackingFile: true,
Format: "qcow2",
DiskImage: true,
UseBackingFile: true,
AdditionalDiskSize: []string{"1M", "2M"},
},
0,
[]string{"create", "-f", "qcow2", "-b", "source.qcow2", "target.qcow2", "1234M"},
"Basic, happy path, backing store, no extra args",
"Basic, happy path, backing store, additional disks",
},
{
&stepCreateDisk{
Format: "qcow2",
UseBackingFile: true,
DiskImage: true,
},
1,
[]string{"create", "-f", "qcow2", "target.qcow2", "1234M"},
@ -47,6 +51,7 @@ func Test_buildCreateCommand(t *testing.T) {
&stepCreateDisk{
Format: "qcow2",
UseBackingFile: true,
DiskImage: true,
QemuImgArgs: QemuImgArgs{
Create: []string{"-foo", "bar"},
},
@ -78,3 +83,94 @@ func Test_buildCreateCommand(t *testing.T) {
fmt.Sprintf("%s. Expected %#v", tc.Reason, tc.Expected))
}
}
func Test_StepCreateCalled(t *testing.T) {
type testCase struct {
Step *stepCreateDisk
Expected []string
Reason string
}
testcases := []testCase{
{
&stepCreateDisk{
Format: "qcow2",
DiskImage: true,
DiskSize: "1M",
VMName: "target",
UseBackingFile: true,
},
[]string{
"create", "-f", "qcow2", "-b", "source.qcow2", "target", "1M",
},
"Basic, happy path, backing store, no additional disks",
},
{
&stepCreateDisk{
Format: "raw",
DiskImage: false,
DiskSize: "4M",
VMName: "target",
UseBackingFile: false,
},
[]string{
"create", "-f", "raw", "target", "4M",
},
"Basic, happy path, raw, no additional disks",
},
{
&stepCreateDisk{
Format: "qcow2",
DiskImage: true,
DiskSize: "4M",
VMName: "target",
UseBackingFile: false,
AdditionalDiskSize: []string{"3M", "8M"},
},
[]string{
"create", "-f", "qcow2", "target-1", "3M",
"create", "-f", "qcow2", "target-2", "8M",
},
"Skips disk creation when disk can be copied",
},
{
&stepCreateDisk{
Format: "qcow2",
DiskImage: true,
DiskSize: "4M",
VMName: "target",
UseBackingFile: false,
},
nil,
"Skips disk creation when disk can be copied",
},
{
&stepCreateDisk{
Format: "qcow2",
DiskImage: true,
DiskSize: "1M",
VMName: "target",
UseBackingFile: true,
AdditionalDiskSize: []string{"3M", "8M"},
},
[]string{
"create", "-f", "qcow2", "-b", "source.qcow2", "target", "1M",
"create", "-f", "qcow2", "target-1", "3M",
"create", "-f", "qcow2", "target-2", "8M",
},
"Basic, happy path, backing store, additional disks",
},
}
for _, tc := range testcases {
d := new(DriverMock)
state := copyTestState(t, d)
state.Put("iso_path", "source.qcow2")
action := tc.Step.Run(context.TODO(), state)
if action != multistep.ActionContinue {
t.Fatalf("Should have gotten an ActionContinue")
}
assert.Equal(t, d.QemuImgCalls, tc.Expected,
fmt.Sprintf("%s. Expected %#v", tc.Reason, tc.Expected))
}
}

View File

@ -209,12 +209,7 @@ func (s *stepRun) getDeviceAndDriveArgs(config *Config, state multistep.StateBag
// Configure virtual hard drives
if s.atLeastVersion2 {
// We have different things to attach based on whether we are booting
// from an iso or a boot image.
drivesToAttach := []string{}
if config.DiskImage {
drivesToAttach = append(drivesToAttach, imgPath)
}
if v, ok := state.GetOk("qemu_disk_paths"); ok {
diskFullPaths := v.([]string)

View File

@ -150,7 +150,8 @@ func Test_DriveAndDeviceArgs(t *testing.T) {
DetectZeroes: "off",
},
map[string]interface{}{
"cd_path": "fake_cd_path.iso",
"cd_path": "fake_cd_path.iso",
"qemu_disk_paths": []string{"path_to_output"},
},
&stepRun{
DiskImage: true,
@ -178,7 +179,8 @@ func Test_DriveAndDeviceArgs(t *testing.T) {
DetectZeroes: "on",
},
map[string]interface{}{
"cd_path": "fake_cd_path.iso",
"cd_path": "fake_cd_path.iso",
"qemu_disk_paths": []string{"path_to_output"},
},
&stepRun{
DiskImage: true,
@ -381,7 +383,8 @@ func Test_DriveAndDeviceArgs(t *testing.T) {
Format: "qcow2",
},
map[string]interface{}{
"cd_path": "fake_cd_path.iso",
"cd_path": "fake_cd_path.iso",
"qemu_disk_paths": []string{"path_to_output"},
},
&stepRun{
DiskImage: true,