Merge ~paelzer/uvtool:lp-1869185-host-model into uvtool:master

[Christian Ehrhardt ]

[Christian Ehrhardt  (paelzer)]

This is RFC for now as it is yet untested and it might take a while to get e.g. a KVM capable arm board again. But it is enough as RFC for a pre-review ahead of spending too much effort down that path.

[Christian Ehrhardt  (paelzer)]

I pushed a fix for emulation targets based on the testing

[Christian Ehrhardt  (paelzer)]

I exercised some tests this should be good now:
Test details:
https://bugs.launchpad.net/ubuntu/+source/uvtool/+bug/1869185/comments/6
https://bugs.launchpad.net/ubuntu/+source/uvtool/+bug/1869185/comments/7
https://bugs.launchpad.net/ubuntu/+source/uvtool/+bug/1869185/comments/8
https://bugs.launchpad.net/ubuntu/+source/uvtool/+bug/1869185/comments/9

I pushed the updated code (had one more fix for arm64) to the branch here.
Ready for review!

[Robie Basak (racb)]

This looks good. Thanks! I have no architectural objections.

Some copyediting requests inline, and one question about the name of the new option.

review: Needs Fixing

[Christian Ehrhardt  (paelzer)]

I've done all the wording changes.
Except the one where you commented on a removed line - I'm not bringing it back to fix the word and then remove it :-P

--cpu-model is fine with me, changing that ...

[Christian Ehrhardt  (paelzer)]

pushed the fixes, ok to merge now?

[Robie Basak (racb)]

review: Approve

[Christian Ehrhardt  (paelzer)]

thanks - pushed to master

diff --git a/man/uvt-kvm.1 b/man/uvt-kvm.1
index 4e25ca7..689f0ee 100644
--- a/man/uvt-kvm.1
+++ b/man/uvt-kvm.1
@@ -384,9 +384,23 @@ the sclp console used.
 
 .TP
 .B --host-passthrough
-Instead of the default cpu model - which mostly is a compatibility focused
-lowest denominator of cpu features - use host-passthrough which will try to
-make all of the hosts cpu features available in the guest.
+Instead of the default CPU model - which is host-model this will use
+host-passthrough which will try to make all of the host's CPU features
+(not migration safe) available in the guest.
+This option is deprecated and will later be removed. Please use --model instead.
+
+.TP
+.B --cpu-model
+Set the CPU model that shall be used.
+The default generally is \fBhost-model\fR, except on arm64 where
+\fBhost-model\fR won't work - there the default is \fBhost-passthrough\fR.
+If set to \fBhost-model\fR or \fBhost-passthrough\fR this will set the CPU mode.
+If set to any other string it will be handled as a \fBnamed CPU\fR passed to
+libvirt.
+Allowed values are all CPUs listed on your host as usable=yes in
+`virsh domcapabilities`.
+This option has no effect if the target is emulated, in this case the safe
+defaults of libvirt will be used.
 
 .TP
 .BI --machine-type\  type
@@ -489,7 +503,7 @@ Default:
 .BI --guest-arch\  architecture
 Specify the architecture of the guest template file that will be selected.
 If an explicit \fB--template\fR is given then \fB--guest-arch\fR has no effect.
-If neither \fB--template\fR nor \fB--guest-arch\fR are set the hosts
+If neither \fB--template\fR nor \fB--guest-arch\fR are set the host's
 architecture will be used to determine the default xml template.
 
 If you set an architecture that requires emulation on the current host
diff --git a/template-aarch64.xml b/template-aarch64.xml
index 4a8296c..515fe81 100644
--- a/template-aarch64.xml
+++ b/template-aarch64.xml
@@ -10,9 +10,6 @@
     <apic/>
     <pae/>
   </features>
-  <cpu mode='custom' match='exact'>
-    <model fallback='allow'>host</model>
-  </cpu>
   <devices>
     <interface type='network'>
       <source network='default'/>
diff --git a/uvtool/libvirt/kvm.py b/uvtool/libvirt/kvm.py
index 71c6d68..9139ee2 100755
--- a/uvtool/libvirt/kvm.py
+++ b/uvtool/libvirt/kvm.py
@@ -427,7 +427,7 @@ def get_boot_item(backing_volume_name, hostname, item, conn=None):
 
 
 def compose_domain_xml(name, volumes, template_path, cpu=1, memory=512,
-        unsafe_caching=False, log_console_output=False, host_passthrough=False,
+        unsafe_caching=False, log_console_output=False, model=False,
         machine_type=None, bridge=None, mac=None, ssh_known_hosts=None,
         kernel=None, initrd=None):
     tree = etree.parse(template_path)
@@ -511,12 +511,19 @@ def compose_domain_xml(name, volumes, template_path, cpu=1, memory=512,
             etree.strip_elements(devices, 'serial')
             devices.append(E.serial(E.target(port='0'), type='stdio'))
 
-    if host_passthrough:
+    if model:
+        if model == "host-passthrough" or model == "host-model":
+            etree.SubElement(domain, 'cpu', mode=model)
+        elif model != "unmodified-emulation":
+            cpu = etree.SubElement(domain, 'cpu', match='exact')
+            etree.SubElement(cpu, 'model', fallback='allow').text = model
+    else:
         if ARCH == 'aarch64':
-            print("Info: on aarch64 a host type cpu is the default",
-                  file=sys.stderr)
-        else:
+            # arm64 can't use host-model, there the default is passthrough
             etree.SubElement(domain, 'cpu', mode='host-passthrough')
+        else:
+            # the default without any argument will be host-model
+            etree.SubElement(domain, 'cpu', mode='host-model')
 
     if machine_type:
         domos = domain.find('os')
@@ -524,12 +531,12 @@ def compose_domain_xml(name, volumes, template_path, cpu=1, memory=512,
         domtype.set('machine', machine_type)
 
     if initrd:
-       domos = domain.find('os')
-       etree.SubElement(domos, 'initrd').text = initrd
+        domos = domain.find('os')
+        etree.SubElement(domos, 'initrd').text = initrd
 
     if kernel:
-       domos = domain.find('os')
-       etree.SubElement(domos, 'kernel').text = kernel
+        domos = domain.find('os')
+        etree.SubElement(domos, 'kernel').text = kernel
 
     if ssh_known_hosts:
         metadata = domain.find('metadata')
@@ -568,7 +575,7 @@ def create(
         disk=2,
         unsafe_caching=False,
         log_console_output=False,
-        host_passthrough=False,
+        model=False,
         bridge=None,
         mac=None,
         backing_image_file=None,
@@ -622,6 +629,8 @@ def create(
         if target_arch and uvtool.libvirt.is_emulation_target(target_arch):
             kernel = get_boot_item(base_volume_name, hostname, "vmlinuz")
             initrd = get_boot_item(base_volume_name, hostname, "initrd.img")
+            # emulation always uses the default
+            model = "unmodified-emulation"
         else:
             kernel = None
             initrd = None
@@ -632,7 +641,7 @@ def create(
             mac=mac,
             cpu=cpu,
             log_console_output=log_console_output,
-            host_passthrough=host_passthrough,
+            model=model,
             memory=memory,
             template_path=template_path,
             unsafe_caching=unsafe_caching,
@@ -881,6 +890,9 @@ def main_create(parser, args):
     if args.template:
         template = args.template
 
+    if args.host_passthrough and not args.cpu_model:
+        args.cpu_model = 'host-passthrough'
+
     if args.backing_image_file:
         abs_image_backing_file = os.path.abspath(args.backing_image_file)
     else:
@@ -897,7 +909,7 @@ def main_create(parser, args):
         cpu=args.cpu,
         disk=args.disk,
         log_console_output=args.log_console_output,
-        host_passthrough=args.host_passthrough,
+        model=args.cpu_model,
         memory=args.memory,
         template_path=template,
         unsafe_caching=args.unsafe_caching,
@@ -1055,6 +1067,7 @@ def main(args):
         help='guest arch to select template, default is the host architecture')
     create_subparser.add_argument('--log-console-output', action='store_true')
     create_subparser.add_argument('--host-passthrough', action='store_true')
+    create_subparser.add_argument('--cpu-model')
     create_subparser.add_argument('--machine-type')
     create_subparser.add_argument('--backing-image-file')
     create_subparser.add_argument('--run-script-once', action='append')