diff --git a/builder/qemu/builder.go b/builder/qemu/builder.go index b80afe275..0265fcad5 100644 --- a/builder/qemu/builder.go +++ b/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{ diff --git a/builder/qemu/config.go b/builder/qemu/config.go index 5c64a3f64..e095a2bcd 100644 --- a/builder/qemu/config.go +++ b/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 diff --git a/builder/qemu/driver.go b/builder/qemu/driver.go index 000c0b915..d4b600c11 100644 --- a/builder/qemu/driver.go +++ b/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() diff --git a/builder/qemu/driver_mock.go b/builder/qemu/driver_mock.go index a01ff7d5a..382892866 100644 --- a/builder/qemu/driver_mock.go +++ b/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 - QemuImgErrs []error + QemuImgCalled bool + 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) { diff --git a/builder/qemu/step_copy_disk.go b/builder/qemu/step_copy_disk.go index dc59ddb7d..5d461104d 100644 --- a/builder/qemu/step_copy_disk.go +++ b/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) diff --git a/builder/qemu/step_copy_disk_test.go b/builder/qemu/step_copy_disk_test.go new file mode 100644 index 000000000..efe30f4ea --- /dev/null +++ b/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") + } +} diff --git a/website/pages/docs/builders/qemu.mdx b/website/pages/docs/builders/qemu.mdx index e96f74068..d81fb3e50 100644 --- a/website/pages/docs/builders/qemu.mdx +++ b/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. \ No newline at end of file diff --git a/website/pages/partials/builder/qemu/Config-not-required.mdx b/website/pages/partials/builder/qemu/Config-not-required.mdx index ef4d2f935..e43002c27 100644 --- a/website/pages/partials/builder/qemu/Config-not-required.mdx +++ b/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