if no machine is set, use default types depending on the arch (#1014)

* Improve 'machine' type selection

If no machine is set, use default types depending on the arch.
The selection is taken from https://github.com/virt-manager/virt-manager
file virtinst/guest.py function get_recommended_machine().

Signed-off-by: Egbert Eich <eich@suse.com>

* Test for getMachineTypeForArch()

Signed-off-by: Egbert Eich <eich@suse.com>

---------

Signed-off-by: Egbert Eich <eich@suse.com>

libvirt/domain_def.go

@@ -139,8 +139,11 @@ func newDomainDefForConnection(virConn *libvirt.Libvirt, rd *schema.ResourceData
 
 	if machine, ok := rd.GetOk("machine"); ok {
 		d.OS.Type.Machine = machine.(string)
-	} else if len(guest.Arch.Machines) > 0 {
-		d.OS.Type.Machine = guest.Arch.Machines[0].Name
+	} else {
+		d.OS.Type.Machine, err = getMachineTypeForArch(caps, d.OS.Type.Arch, d.OS.Type.Type)
+		if err != nil {
+			return d, err
+		}
 	}
 
 	canonicalmachine, err := getCanonicalMachineName(caps, d.OS.Type.Arch, d.OS.Type.Type, d.OS.Type.Machine)

libvirt/resource_libvirt_domain.go

@@ -516,7 +516,6 @@ func resourceLibvirtDomainCreate(ctx context.Context, d *schema.ResourceData, me
 	domainDef.OS.Initrd = d.Get("initrd").(string)
 	domainDef.OS.Type.Arch = d.Get("arch").(string)
 
-	domainDef.OS.Type.Machine = d.Get("machine").(string)
 	domainDef.Devices.Emulator = d.Get("emulator").(string)
 
 	if v := os.Getenv("TERRAFORM_LIBVIRT_TEST_DOMAIN_TYPE"); v != "" {

libvirt/utils_domain_def.go

@@ -71,6 +71,53 @@ func getOriginalMachineName(caps libvirtxml.Caps, arch string, virttype string,
 	return targetmachine, nil // There wasn't a canonical mapping to this
 }
 
+func isMachineSupported(guest libvirtxml.CapsGuest, name string) bool {
+	for _, m := range guest.Arch.Machines {
+		if m.Name == name {
+			return true
+		}
+	}
+	for _, domain := range guest.Arch.Domains {
+		for _, m := range domain.Machines {
+			if m.Name == name {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// For backward compatibility Libvirt may pick the machine which has been
+// implemented initially for an architecture as default machine type.
+// This may not provide the hardware features expected and machine creation
+// fails. To work around this, we pick a default type unless the user has
+// specified one.
+func getMachineTypeForArch(caps libvirtxml.Caps, arch string, virttype string) (string, error) {
+	guest, err := getGuestForArchType(caps, arch, virttype)
+	var machines []string
+	if err != nil {
+		return "", err
+	}
+	switch arch {
+	case "i686", "x86_64":
+		machines = []string{"pc"} // "q35"?
+	case "aarch64", "armv7l":
+		machines = []string{"virt", "vexpress-a15"}
+	case "s390x":
+		machines = []string{"s390-ccw-virtio"}
+	case "ppc64le", "ppc64":
+		machines = []string{"pseries"}
+	default:
+		return "", fmt.Errorf("[DEBUG] Could not get machine type for arch %s", arch)
+	}
+	for _, machine := range machines {
+		if isMachineSupported(guest, machine) {
+			return machine, nil
+		}
+	}
+	return "", fmt.Errorf("[DEBUG] Machine type for arch %s not available", arch)
+}
+
 // as kernal args allow duplicate keys, we use a list of maps
 // we jump to a next map as soon as we find a duplicate
 // key.

libvirt/utils_domain_def_test.go

@@ -102,6 +102,29 @@ func TestGetOriginalMachineName(t *testing.T) {
 	t.Logf("Reverse canonical lookup for %s is %s which matches %s", canonname, reversename, machine)
 }
 
+func TestGetMachineTypeForArch(t *testing.T) {
+	skipIfAccDisabled(t)
+	conn := testAccProvider.Meta().(*Client).libvirt
+	caps, err := getHostCapabilities(conn)
+	if err != nil {
+	       t.Error(err)
+	}
+
+	arch := "x86_64"
+	targettype := "hvm"
+	machine := "pc"
+
+	m, err := getMachineTypeForArch(caps, arch, targettype)
+	if err != nil {
+		t.Error(err)
+	}
+	if m != machine {
+		t.Errorf("Machine found for arch %s, targettype %s is %s expected %s", arch, targettype, m, machine)
+	}
+
+	t.Logf("Machine for arch %s, targettype %s is %s which matches %s", arch, targettype, m, machine)
+}
+
 func TestGetHostCapabilties(t *testing.T) {
 	skipIfAccDisabled(t)
 	conn := testAccProvider.Meta().(*Client).libvirt