diff --git a/builder/qemu/builder.go b/builder/qemu/builder.go index 42d733792..f03329947 100644 --- a/builder/qemu/builder.go +++ b/builder/qemu/builder.go @@ -11,7 +11,9 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "runtime" + "strings" "time" "github.com/hashicorp/packer/common" @@ -354,6 +356,23 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { if b.config.DiskSize == "" || b.config.DiskSize == "0" { b.config.DiskSize = "40960M" + } else { + // Make sure supplied disk size is valid + // (digits, plus an optional valid unit character). e.g. 5000, 40G, 1t + re := regexp.MustCompile(`^[\d]+(b|k|m|g|t){0,1}$`) + matched := re.MatchString(strings.ToLower(b.config.DiskSize)) + if !matched { + errs = packer.MultiErrorAppend(errs, fmt.Errorf("Invalid disk size.")) + } else { + // Okay, it's valid -- if it doesn't alreay have a suffix, then + // append "M" as the default unit. + re = regexp.MustCompile(`^[\d]+$`) + matched = re.MatchString(strings.ToLower(b.config.DiskSize)) + if matched { + // Needs M added. + b.config.DiskSize = fmt.Sprintf("%sM", b.config.DiskSize) + } + } } if b.config.DiskCache == "" { @@ -457,7 +476,6 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { if b.config.ISOSkipCache { b.config.ISOChecksumType = "none" } - isoWarnings, isoErrs := b.config.ISOConfig.Prepare(&b.config.ctx) warnings = append(warnings, isoWarnings...) errs = packer.MultiErrorAppend(errs, isoErrs...) diff --git a/builder/qemu/builder_test.go b/builder/qemu/builder_test.go index 15f3db8b3..4616ed3e2 100644 --- a/builder/qemu/builder_test.go +++ b/builder/qemu/builder_test.go @@ -157,34 +157,38 @@ func TestBuilderPrepare_DiskCompaction(t *testing.T) { } func TestBuilderPrepare_DiskSize(t *testing.T) { - var b Builder - config := testConfig() - - delete(config, "disk_size") - warns, err := b.Prepare(config) - if len(warns) > 0 { - t.Fatalf("bad: %#v", warns) - } - if err != nil { - t.Fatalf("bad err: %s", err) + type testcase struct { + InputSize string + OutputSize string + ErrExpected bool } - if b.config.DiskSize != "40960M" { - t.Fatalf("bad size: %s", b.config.DiskSize) + testCases := []testcase{ + testcase{"", "40960M", false}, // not provided + testcase{"12345", "12345M", false}, // no unit given, defaults to M + testcase{"12345x", "12345x", true}, // invalid unit + testcase{"12345T", "12345T", false}, // terabytes + testcase{"12345b", "12345b", false}, // bytes get preserved when set. + testcase{"60000M", "60000M", false}, // Original test case } + for _, tc := range testCases { + // Set input disk size + var b Builder + config := testConfig() + delete(config, "disk_size") + config["disk_size"] = tc.InputSize - config["disk_size"] = "60000M" - b = Builder{} - warns, err = b.Prepare(config) - if len(warns) > 0 { - t.Fatalf("bad: %#v", warns) - } - if err != nil { - t.Fatalf("should not have error: %s", err) - } + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } + if (err == nil) == tc.ErrExpected { + t.Fatalf("bad: error when providing disk size %s; Err expected: %t; err recieved: %v", tc.InputSize, tc.ErrExpected, err) + } - if b.config.DiskSize != "60000M" { - t.Fatalf("bad size: %s", b.config.DiskSize) + if b.config.DiskSize != tc.OutputSize { + t.Fatalf("bad size: received: %s but expected %s", b.config.DiskSize, tc.OutputSize) + } } }