qemu: fix default machine for argv -> xml convertor

Historically the argv -> xml convertor wanted the same default machine
as we'd set when parsing xml. The latter has now changed, however, to
use a default defined by libvirt. The former needs fixing to again
honour the default QEMU machine.

This exposed a bug in handling for the aarch64 target, as QEMU does not
define any default machine. Thus we should not having been accepting
argv without a -machine provided.
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

src/qemu/qemu_capabilities.c

@@ -2069,6 +2069,18 @@ const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr qemuCaps,
     return name;
 }
 
+const char *
+virQEMUCapsGetDefaultMachine(virQEMUCapsPtr qemuCaps)
+{
+    size_t i;
+
+    for (i = 0; i < qemuCaps->nmachineTypes; i++) {
+        if (qemuCaps->machineTypes[i].qemuDefault)
+            return qemuCaps->machineTypes[i].name;
+    }
+
+    return NULL;
+}
 
 int virQEMUCapsGetMachineMaxCpus(virQEMUCapsPtr qemuCaps,
                                  const char *name)

src/qemu/qemu_capabilities.h

@@ -559,6 +559,7 @@ bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
                                    virCPUMode mode);
 const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr qemuCaps,
                                            const char *name);
+const char *virQEMUCapsGetDefaultMachine(virQEMUCapsPtr qemuCaps);
 int virQEMUCapsGetMachineMaxCpus(virQEMUCapsPtr qemuCaps,
                                  const char *name);
 bool virQEMUCapsGetMachineHotplugCpus(virQEMUCapsPtr qemuCaps,

src/qemu/qemu_driver.c

@@ -7103,7 +7103,8 @@ static char *qemuConnectDomainXMLFromNative(virConnectPtr conn,
     if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
         goto cleanup;
 
-    def = qemuParseCommandLineString(caps, driver->xmlopt, config,
+    def = qemuParseCommandLineString(driver->qemuCapsCache,
+                                     caps, driver->xmlopt, config,
                                      NULL, NULL, NULL);
     if (!def)
         goto cleanup;
@@ -16606,7 +16607,8 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn,
     if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
         goto cleanup;
 
-    if (!(def = qemuParseCommandLinePid(caps, driver->xmlopt, pid,
+    if (!(def = qemuParseCommandLinePid(driver->qemuCapsCache,
+                                        caps, driver->xmlopt, pid,
                                         &pidfile, &monConfig, &monJSON)))
         goto cleanup;
 

src/qemu/qemu_parse_command.c

@@ -1829,7 +1829,8 @@ qemuParseCommandLineBootDevs(virDomainDefPtr def, const char *str)
  * as is practical. This is not an exact science....
  */
 static virDomainDefPtr
-qemuParseCommandLine(virCapsPtr caps,
+qemuParseCommandLine(virFileCachePtr capsCache,
+                     virCapsPtr caps,
                      virDomainXMLOptionPtr xmlopt,
                      char **progenv,
                      char **progargv,
@@ -1851,6 +1852,7 @@ qemuParseCommandLine(virCapsPtr caps,
     virDomainDiskDefPtr disk = NULL;
     const char *ceph_args = qemuFindEnv(progenv, "CEPH_ARGS");
     bool have_sdl = false;
+    virQEMUCapsPtr qemuCaps;
 
     if (pidfile)
         *pidfile = NULL;
@@ -1865,6 +1867,9 @@ qemuParseCommandLine(virCapsPtr caps,
         return NULL;
     }
 
+    if (!(qemuCaps = virQEMUCapsCacheLookup(capsCache, progargv[0])))
+        goto error;
+
     if (!(def = virDomainDefNew()))
         goto error;
 
@@ -2017,17 +2022,18 @@ qemuParseCommandLine(virCapsPtr caps,
     /* If no machine type has been found among the arguments, then figure
      * out a reasonable value by using capabilities */
     if (!def->os.machine) {
-        virCapsDomainDataPtr capsdata;
+        const char *mach = virQEMUCapsGetDefaultMachine(qemuCaps);
 
-        if (!(capsdata = virCapabilitiesDomainDataLookup(caps, def->os.type,
-                def->os.arch, def->virtType, NULL, NULL)))
+        if (!mach) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Binary '%s' does not have a default machine type "
+                             "and no '-machine' arg is present"),
+                           progargv[0]);
             goto error;
+        }
 
-        if (VIR_STRDUP(def->os.machine, capsdata->machinetype) < 0) {
-            VIR_FREE(capsdata);
+        if (VIR_STRDUP(def->os.machine, mach) < 0)
             goto error;
-        }
-        VIR_FREE(capsdata);
     }
 
     /* Now the real processing loop */
@@ -2718,6 +2724,7 @@ qemuParseCommandLine(virCapsPtr caps,
     else
         qemuDomainCmdlineDefFree(cmd);
 
+    virObjectUnref(qemuCaps);
     return def;
 
  error:
@@ -2732,11 +2739,13 @@ qemuParseCommandLine(virCapsPtr caps,
     }
     if (pidfile)
         VIR_FREE(*pidfile);
+    virObjectUnref(qemuCaps);
     return NULL;
 }
 
 
-virDomainDefPtr qemuParseCommandLineString(virCapsPtr caps,
+virDomainDefPtr qemuParseCommandLineString(virFileCachePtr capsCache,
+                                           virCapsPtr caps,
                                            virDomainXMLOptionPtr xmlopt,
                                            const char *args,
                                            char **pidfile,
@@ -2750,7 +2759,7 @@ virDomainDefPtr qemuParseCommandLineString(virCapsPtr caps,
     if (qemuStringToArgvEnv(args, &progenv, &progargv) < 0)
         goto cleanup;
 
-    def = qemuParseCommandLine(caps, xmlopt, progenv, progargv,
+    def = qemuParseCommandLine(capsCache, caps, xmlopt, progenv, progargv,
                                pidfile, monConfig, monJSON);
 
  cleanup:
@@ -2808,7 +2817,8 @@ static int qemuParseProcFileStrings(int pid_value,
     return ret;
 }
 
-virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps,
+virDomainDefPtr qemuParseCommandLinePid(virFileCachePtr capsCache,
+                                        virCapsPtr caps,
                                         virDomainXMLOptionPtr xmlopt,
                                         pid_t pid,
                                         char **pidfile,
@@ -2828,7 +2838,7 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps,
         qemuParseProcFileStrings(pid, "environ", &progenv) < 0)
         goto cleanup;
 
-    if (!(def = qemuParseCommandLine(caps, xmlopt, progenv, progargv,
+    if (!(def = qemuParseCommandLine(capsCache, caps, xmlopt, progenv, progargv,
                                      pidfile, monConfig, monJSON)))
         goto cleanup;
 

src/qemu/qemu_parse_command.h

@@ -24,19 +24,23 @@
 #ifndef __QEMU_PARSE_COMMAND_H__
 # define __QEMU_PARSE_COMMAND_H__
 
+# include "virfilecache.h"
+
 # define QEMU_QXL_VGAMEM_DEFAULT 16 * 1024
 
 /*
  * NB: def->name can be NULL upon return and the caller
  * *must* decide how to fill in a name in this case
  */
-virDomainDefPtr qemuParseCommandLineString(virCapsPtr caps,
+virDomainDefPtr qemuParseCommandLineString(virFileCachePtr capsCache,
+                                           virCapsPtr caps,
                                            virDomainXMLOptionPtr xmlopt,
                                            const char *args,
                                            char **pidfile,
                                            virDomainChrSourceDefPtr *monConfig,
                                            bool *monJSON);
-virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps,
+virDomainDefPtr qemuParseCommandLinePid(virFileCachePtr capsCache,
+                                        virCapsPtr caps,
                                         virDomainXMLOptionPtr xmlopt,
                                         pid_t pid,
                                         char **pidfile,

tests/qemuargv2xmldata/nomachine-aarch64.args

-LC_ALL=C \
-PATH=/bin \
-HOME=/home/test \
-USER=test \
-LOGNAME=test \
-QEMU_AUDIO_DRV=none \
-/usr/bin/qemu-system-aarch64 \
--name QEMUGuest1 \
--m 512 \
--hda /dev/HostVG/QEMUGuest1 \
--cdrom /root/boot.iso

tests/qemuargv2xmldata/nomachine-aarch64.xml

-<domain type='qemu'>
-  <name>QEMUGuest1</name>
-  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
-  <memory unit='KiB'>524288</memory>
-  <currentMemory unit='KiB'>524288</currentMemory>
-  <vcpu placement='static'>1</vcpu>
-  <os>
-    <type arch='aarch64' machine='virt'>hvm</type>
-    <boot dev='hd'/>
-  </os>
-  <features>
-    <gic version='2'/>
-  </features>
-  <clock offset='utc'/>
-  <on_poweroff>destroy</on_poweroff>
-  <on_reboot>restart</on_reboot>
-  <on_crash>destroy</on_crash>
-  <devices>
-    <emulator>/usr/bin/qemu-system-aarch64</emulator>
-    <disk type='block' device='disk'>
-      <driver name='qemu' type='raw'/>
-      <source dev='/dev/HostVG/QEMUGuest1'/>
-      <target dev='hda' bus='ide'/>
-      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
-    </disk>
-    <disk type='file' device='cdrom'>
-      <driver name='qemu' type='raw'/>
-      <source file='/root/boot.iso'/>
-      <target dev='hdc' bus='ide'/>
-      <readonly/>
-      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
-    </disk>
-    <controller type='ide' index='0'/>
-    <graphics type='sdl'/>
-    <video>
-      <model type='cirrus' vram='16384' heads='1' primary='yes'/>
-    </video>
-    <memballoon model='none'/>
-  </devices>
-</domain>

tests/qemuargv2xmldata/nomachine-ppc64.xml

@@ -5,7 +5,7 @@
   <currentMemory unit='KiB'>524288</currentMemory>
   <vcpu placement='static'>1</vcpu>
   <os>
-    <type arch='ppc64' machine='pseries'>hvm</type>
+    <type arch='ppc64' machine='pseries-2.12'>hvm</type>
     <boot dev='hd'/>
   </os>
   <clock offset='utc'/>
@@ -27,7 +27,7 @@
       <readonly/>
       <address type='drive' controller='0' bus='0' target='0' unit='2'/>
     </disk>
-    <controller type='usb' index='0'>
+    <controller type='usb' index='0' model='pci-ohci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'>

tests/qemuargv2xmldata/nomachine-x86_64.xml

@@ -5,7 +5,7 @@
   <currentMemory unit='KiB'>524288</currentMemory>
   <vcpu placement='static'>1</vcpu>
   <os>
-    <type arch='x86_64' machine='pc-0.11'>hvm</type>
+    <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type>
     <boot dev='hd'/>
   </os>
   <features>
@@ -30,7 +30,7 @@
       <readonly/>
       <address type='drive' controller='0' bus='1' target='0' unit='0'/>
     </disk>
-    <controller type='usb' index='0'>
+    <controller type='usb' index='0' model='piix3-uhci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>

tests/qemuargv2xmldata/pseries-disk.xml

@@ -5,7 +5,7 @@
   <currentMemory unit='KiB'>524288</currentMemory>
   <vcpu placement='static'>1</vcpu>
   <os>
-    <type arch='ppc64' machine='pseries'>hvm</type>
+    <type arch='ppc64' machine='pseries-2.12'>hvm</type>
     <boot dev='hd'/>
   </os>
   <clock offset='utc'/>
@@ -27,7 +27,7 @@
       <readonly/>
       <address type='drive' controller='0' bus='0' target='0' unit='2'/>
     </disk>
-    <controller type='usb' index='0'>
+    <controller type='usb' index='0' model='pci-ohci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'>

tests/qemuargv2xmldata/pseries-nvram.xml

@@ -5,7 +5,7 @@
   <currentMemory unit='KiB'>524288</currentMemory>
   <vcpu placement='static'>1</vcpu>
   <os>
-    <type arch='ppc64' machine='pseries'>hvm</type>
+    <type arch='ppc64' machine='pseries-2.12'>hvm</type>
     <boot dev='hd'/>
   </os>
   <clock offset='utc'/>
@@ -14,7 +14,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-ppc64</emulator>
-    <controller type='usb' index='0'>
+    <controller type='usb' index='0' model='pci-ohci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'>

tests/qemuargv2xmltest.c

@@ -65,7 +65,8 @@ static int testCompareXMLToArgvFiles(const char *xmlfile,
     if (virTestLoadFile(cmdfile, &cmd) < 0)
         goto fail;
 
-    if (!(vmdef = qemuParseCommandLineString(driver.caps, driver.xmlopt,
+    if (!(vmdef = qemuParseCommandLineString(driver.qemuCapsCache,
+                                             driver.caps, driver.xmlopt,
                                              cmd, NULL, NULL, NULL)))
         goto fail;
 
@@ -154,6 +155,20 @@ mymain(void)
     if (qemuTestDriverInit(&driver) < 0)
         return EXIT_FAILURE;
 
+# define LOAD_CAPS(arch) \
+    do { \
+        virQEMUCapsPtr qemuCaps; \
+        qemuCaps = qemuTestParseCapabilitiesArch(VIR_ARCH_X86_64, \
+                                                 "qemucapabilitiesdata/caps_2.12.0." arch ".xml"); \
+        if (virFileCacheInsertData(driver.qemuCapsCache, \
+                                   "/usr/bin/qemu-system-" arch, \
+                                   qemuCaps) < 0) \
+            return EXIT_FAILURE; \
+    } while (0)
+
+    LOAD_CAPS("x86_64");
+    LOAD_CAPS("aarch64");
+    LOAD_CAPS("ppc64");
 
 # define DO_TEST_FULL(name, flags) \
     do { \
@@ -290,7 +305,6 @@ mymain(void)
     DO_TEST("machine-keywrap-none-argv");
 
     DO_TEST("nomachine-x86_64");
-    DO_TEST("nomachine-aarch64");
     DO_TEST("nomachine-ppc64");
 
     qemuTestDriverFree(&driver);