diff --git a/builder/qemu/config.go b/builder/qemu/config.go index 7b56ff0e4..a5793aa43 100644 --- a/builder/qemu/config.go +++ b/builder/qemu/config.go @@ -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")) diff --git a/builder/qemu/config_test.go b/builder/qemu/config_test.go index 9b10e9cdf..23b0309f3 100644 --- a/builder/qemu/config_test.go +++ b/builder/qemu/config_test.go @@ -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") diff --git a/builder/qemu/step_copy_disk.go b/builder/qemu/step_copy_disk.go index b8ff9e06a..e6fd64849 100644 --- a/builder/qemu/step_copy_disk.go +++ b/builder/qemu/step_copy_disk.go @@ -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) diff --git a/builder/qemu/step_create_disk.go b/builder/qemu/step_create_disk.go index c8bc29940..8f35ad640 100644 --- a/builder/qemu/step_create_disk.go +++ b/builder/qemu/step_create_disk.go @@ -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) } diff --git a/builder/qemu/step_create_disk_test.go b/builder/qemu/step_create_disk_test.go index 6756d7e58..c0d178fac 100644 --- a/builder/qemu/step_create_disk_test.go +++ b/builder/qemu/step_create_disk_test.go @@ -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)) + } +} diff --git a/builder/qemu/step_run.go b/builder/qemu/step_run.go index 16a7470a1..d66f4061f 100644 --- a/builder/qemu/step_run.go +++ b/builder/qemu/step_run.go @@ -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) diff --git a/builder/qemu/step_run_test.go b/builder/qemu/step_run_test.go index d275fb401..96665eeb8 100644 --- a/builder/qemu/step_run_test.go +++ b/builder/qemu/step_run_test.go @@ -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,