Merge pull request #9949 from hashicorp/do_6963

Skip qemu-img convert on OSX

builder/qemu/builder.go

@@ -69,7 +69,13 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			Label: b.config.CDConfig.CDLabel,
 		},
 		new(stepCreateDisk),
-		new(stepCopyDisk),
+		&stepCopyDisk{
+			DiskImage:      b.config.DiskImage,
+			Format:         b.config.Format,
+			OutputDir:      b.config.OutputDir,
+			UseBackingFile: b.config.UseBackingFile,
+			VMName:         b.config.VMName,
+		},
 		new(stepResizeDisk),
 		new(stepHTTPIPDiscover),
 		&common.StepHTTPServer{

builder/qemu/config.go

@@ -143,7 +143,12 @@ type Config struct {
 	// using qemu-img convert. Defaults to false.
 	DiskCompression bool `mapstructure:"disk_compression" required:"false"`
 	// Either `qcow2` or `raw`, this specifies the output format of the virtual
-	// machine image. This defaults to `qcow2`.
+	// machine image. This defaults to `qcow2`. Due to a long-standing bug with
+	// `qemu-img convert` on OSX, sometimes the qemu-img convert call will
+	// create a corrupted image. If this is an issue for you, make sure that the
+	// the output format matches the input file's format, and Packer will
+	// perform a simple copy operation instead. See
+	// https://bugs.launchpad.net/qemu/+bug/1776920 for more details.
 	Format string `mapstructure:"format" required:"false"`
 	// Packer defaults to building QEMU virtual machines by
 	// launching a GUI that shows the console of the machine being built. When this

builder/qemu/driver.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"os"
 	"os/exec"
 	"regexp"
 	"strings"
@@ -22,6 +23,10 @@ type DriverCancelCallback func(state multistep.StateBag) bool
 // A driver is able to talk to qemu-system-x86_64 and perform certain
 // operations with it.
 type Driver interface {
+	// Copy bypasses qemu-img convert and directly copies an image
+	// that doesn't need converting.
+	Copy(string, string) error
+
 	// Stop stops a running machine, forcefully.
 	Stop() error
 
@@ -65,6 +70,33 @@ func (d *QemuDriver) Stop() error {
 	return nil
 }
 
+func (d *QemuDriver) Copy(sourceName, targetName string) error {
+	source, err := os.Open(sourceName)
+	if err != nil {
+		err = fmt.Errorf("Error opening iso for copy: %s", err)
+		return err
+	}
+	defer source.Close()
+
+	// Create will truncate an existing file
+	target, err := os.Create(targetName)
+	if err != nil {
+		err = fmt.Errorf("Error creating hard drive in output dir: %s", err)
+		return err
+	}
+	defer target.Close()
+
+	log.Printf("Copying %s to %s", source.Name(), target.Name())
+	bytes, err := io.Copy(target, source)
+	if err != nil {
+		err = fmt.Errorf("Error copying iso to output dir: %s", err)
+		return err
+	}
+	log.Printf(fmt.Sprintf("Copied %d bytes", bytes))
+
+	return nil
+}
+
 func (d *QemuDriver) Qemu(qemuArgs ...string) error {
 	d.lock.Lock()
 	defer d.lock.Unlock()

builder/qemu/driver_mock.go

@@ -5,6 +5,9 @@ import "sync"
 type DriverMock struct {
 	sync.Mutex
 
+	CopyCalled bool
+	CopyErr    error
+
 	StopCalled bool
 	StopErr    error
 
@@ -14,8 +17,9 @@ type DriverMock struct {
 	WaitForShutdownCalled bool
 	WaitForShutdownState  bool
 
-	QemuImgCalls [][]string
+	QemuImgCalled bool
-	QemuImgErrs  []error
+	QemuImgCalls  [][]string
+	QemuImgErrs   []error
 
 	VerifyCalled bool
 	VerifyErr    error
@@ -25,6 +29,11 @@ type DriverMock struct {
 	VersionErr    error
 }
 
+func (d *DriverMock) Copy(source, dst string) error {
+	d.CopyCalled = true
+	return d.CopyErr
+}
+
 func (d *DriverMock) Stop() error {
 	d.StopCalled = true
 	return d.StopErr
@@ -45,6 +54,7 @@ func (d *DriverMock) WaitForShutdown(cancelCh <-chan struct{}) bool {
 }
 
 func (d *DriverMock) QemuImg(args ...string) error {
+	d.QemuImgCalled = true
 	d.QemuImgCalls = append(d.QemuImgCalls, args)
 
 	if len(d.QemuImgErrs) >= len(d.QemuImgCalls) {

builder/qemu/step_copy_disk.go

@@ -11,26 +11,45 @@ import (
 
 // This step copies the virtual disk that will be used as the
 // hard drive for the virtual machine.
-type stepCopyDisk struct{}
+type stepCopyDisk struct {
+	DiskImage      bool
+	Format         string
+	OutputDir      string
+	UseBackingFile bool
+	VMName         string
+}
 
 func (s *stepCopyDisk) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
-	config := state.Get("config").(*Config)
 	driver := state.Get("driver").(Driver)
 	isoPath := state.Get("iso_path").(string)
 	ui := state.Get("ui").(packer.Ui)
-	path := filepath.Join(config.OutputDir, config.VMName)
+	path := filepath.Join(s.OutputDir, s.VMName)
+
+	if !s.DiskImage || s.UseBackingFile {
+		return multistep.ActionContinue
+	}
+
+	// isoPath extention is:
+	ext := filepath.Ext(isoPath)
+	if ext[1:] == s.Format {
+		ui.Message("File extension already matches desired output format. " +
+			"Skipping qemu-img convert step")
+		err := driver.Copy(isoPath, path)
+		if err != nil {
+			state.Put("error", err)
+			ui.Error(err.Error())
+			return multistep.ActionHalt
+		}
+		return multistep.ActionContinue
+	}
 
 	command := []string{
 		"convert",
-		"-O", config.Format,
+		"-O", s.Format,
 		isoPath,
 		path,
 	}
 
-	if !config.DiskImage || config.UseBackingFile {
-		return multistep.ActionContinue
-	}
-
 	ui.Say("Copying hard drive...")
 	if err := driver.QemuImg(command...); err != nil {
 		err := fmt.Errorf("Error creating hard drive: %s", err)

builder/qemu/step_copy_disk_test.go

@@ -0,0 +1,91 @@
+package qemu
+
+import (
+	"context"
+	"testing"
+
+	"github.com/hashicorp/packer/helper/multistep"
+	"github.com/hashicorp/packer/packer"
+)
+
+func copyTestState(t *testing.T, d *DriverMock) multistep.StateBag {
+	state := new(multistep.BasicStateBag)
+	state.Put("ui", packer.TestUi(t))
+	state.Put("driver", d)
+	state.Put("iso_path", "example_source.qcow2")
+
+	return state
+}
+
+func Test_StepCopySkip(t *testing.T) {
+	testcases := []stepCopyDisk{
+		stepCopyDisk{
+			DiskImage:      false,
+			UseBackingFile: false,
+		},
+		stepCopyDisk{
+			DiskImage:      true,
+			UseBackingFile: true,
+		},
+		stepCopyDisk{
+			DiskImage:      false,
+			UseBackingFile: true,
+		},
+	}
+
+	for _, tc := range testcases {
+		d := new(DriverMock)
+		state := copyTestState(t, d)
+		action := tc.Run(context.TODO(), state)
+		if action != multistep.ActionContinue {
+			t.Fatalf("Should have gotten an ActionContinue")
+		}
+
+		if d.CopyCalled || d.QemuImgCalled {
+			t.Fatalf("Should have skipped step since DiskImage and UseBackingFile are not set")
+		}
+	}
+}
+
+func Test_StepCopyCalled(t *testing.T) {
+	step := stepCopyDisk{
+		DiskImage: true,
+		Format:    "qcow2",
+		VMName:    "output.qcow2",
+	}
+
+	d := new(DriverMock)
+	state := copyTestState(t, d)
+	action := step.Run(context.TODO(), state)
+	if action != multistep.ActionContinue {
+		t.Fatalf("Should have gotten an ActionContinue")
+	}
+
+	if !d.CopyCalled {
+		t.Fatalf("Should have copied since all extensions are qcow2")
+	}
+	if d.QemuImgCalled {
+		t.Fatalf("Should not have called qemu-img when formats match")
+	}
+}
+
+func Test_StepQemuImgCalled(t *testing.T) {
+	step := stepCopyDisk{
+		DiskImage: true,
+		Format:    "raw",
+		VMName:    "output.qcow2",
+	}
+
+	d := new(DriverMock)
+	state := copyTestState(t, d)
+	action := step.Run(context.TODO(), state)
+	if action != multistep.ActionContinue {
+		t.Fatalf("Should have gotten an ActionContinue")
+	}
+	if d.CopyCalled {
+		t.Fatalf("Should not have copied since extensions don't match")
+	}
+	if !d.QemuImgCalled {
+		t.Fatalf("Should have called qemu-img since extensions don't match")
+	}
+}

website/pages/docs/builders/qemu.mdx

@@ -196,6 +196,8 @@ necessary for this build to succeed and can be found further down the page.
 
 ### Troubleshooting
 
+#### Invalid Keymaps
+
 Some users have experienced errors complaining about invalid keymaps. This
 seems to be related to having a `common` directory or file in the directory
 they've run Packer in, like the packer source directory. This appears to be an
@@ -205,3 +207,14 @@ file/directory or run in another directory.
 Some users have reported issues with incorrect keymaps using qemu version 2.11.
 This is a bug with qemu, and the solution is to upgrade, or downgrade to 2.10.1
 or earlier.
+
+#### Corrupted image after Packer calls qemu-img convert on OSX
+
+Due to an upstream bug with `qemu-img convert` on OSX, sometimes the qemu-img
+convert call will create a corrupted image. If this is an issue for you, make
+sure that the the output format (provided using the option `format`) matches
+the input file's format and file extension, and Packer will
+perform a simple copy operation instead. You will also want to set
+`"skip_compaction": true,` and `"disk_compression": false` to skip a final
+image conversion at the end of the build. See
+https://bugs.launchpad.net/qemu/+bug/1776920 for more details.

website/pages/partials/builder/qemu/Config-not-required.mdx

@@ -77,7 +77,12 @@
   using qemu-img convert. Defaults to false.
 
 - `format` (string) - Either `qcow2` or `raw`, this specifies the output format of the virtual
-  machine image. This defaults to `qcow2`.
+  machine image. This defaults to `qcow2`. Due to a long-standing bug with
+  `qemu-img convert` on OSX, sometimes the qemu-img convert call will
+  create a corrupted image. If this is an issue for you, make sure that the
+  the output format matches the input file's format, and Packer will
+  perform a simple copy operation instead. See
+  https://bugs.launchpad.net/qemu/+bug/1776920 for more details.
 
 - `headless` (bool) - Packer defaults to building QEMU virtual machines by
   launching a GUI that shows the console of the machine being built. When this