Finish reordering stuff and cleaning up

Finishes shuffling bits of code around to make the options better
grouped in the code so it's easier to read and understand what's what at
a glance.

tests/data/cli/compare/virt-install-boot-container.xml

@@ -6,15 +6,15 @@
   <vcpu>1</vcpu>
   <os>
     <type arch="i686">hvm</type>
-    <initdir>/my/custom/cwd</initdir>
-    <inituser>tester</inituser>
-    <initgroup>1000</initgroup>
-    <boot dev="hd"/>
     <init>/bin/systemd</init>
     <initarg>--unit</initarg>
     <initarg>emergency.service</initarg>
     <initenv name="MYENV">some value</initenv>
     <initenv name="FOO">bar</initenv>
+    <initdir>/my/custom/cwd</initdir>
+    <inituser>tester</inituser>
+    <initgroup>1000</initgroup>
+    <boot dev="hd"/>
   </os>
   <features>
     <pae/>

tests/data/cli/compare/virt-install-boot-guest-loader-bios.xml

@@ -6,7 +6,7 @@
   <vcpu>1</vcpu>
   <os firmware="bios">
     <type arch="i686">hvm</type>
-    <loader readonly="yes" type="rom" secure="no">/path/to/loader</loader>
+    <loader readonly="yes" secure="no" type="rom">/path/to/loader</loader>
     <boot dev="hd"/>
     <bootmenu enable="yes" timeout="5000"/>
     <bios rebootTimeout="5000"/>

tests/data/cli/compare/virt-install-boot-guest-loader-efi.xml

@@ -6,7 +6,7 @@
   <vcpu>1</vcpu>
   <os firmware="efi">
     <type arch="i686">hvm</type>
-    <loader readonly="yes" type="pflash" secure="yes">/path/to/loader</loader>
+    <loader readonly="yes" secure="yes" type="pflash">/path/to/loader</loader>
     <nvram>/path/to/nvram</nvram>
     <boot dev="hd"/>
     <bios useserial="yes"/>

tests/data/cli/compare/virt-install-boot-uefi.xml

@@ -6,7 +6,7 @@
   <vcpu>1</vcpu>
   <os>
     <type arch="x86_64" machine="q35">hvm</type>
-    <loader readonly="yes" type="pflash" secure="yes">/usr/share/ovmf/OVMF_CODE.secboot.fd</loader>
+    <loader readonly="yes" secure="yes" type="pflash">/usr/share/ovmf/OVMF_CODE.secboot.fd</loader>
     <boot dev="hd"/>
   </os>
   <features>

tests/data/cli/compare/virt-install-singleton-config-1.xml

@@ -23,11 +23,11 @@
   <os>
     <type arch="x86_64" machine="q35">hvm</type>
     <loader readonly="yes" type="pflash">/usr/share/OVMF/OVMF_CODE.fd</loader>
+    <initarg>foo</initarg>
+    <initarg>bar=baz</initarg>
     <boot dev="network"/>
     <boot dev="hd"/>
     <smbios mode="host"/>
-    <initarg>foo</initarg>
-    <initarg>bar=baz</initarg>
   </os>
   <features>
     <acpi/>

tests/data/cli/compare/virt-install-singleton-config-2.xml

@@ -93,20 +93,20 @@
   <bootloader>/new/bootld</bootloader>
   <os firmware="efi">
     <type arch="x86_64" machine="q35">hvm</type>
-    <loader>/foo/bar</loader>
-    <initdir>/my/custom/cwd</initdir>
-    <inituser>tester</inituser>
-    <initgroup>1000</initgroup>
-    <boot dev="network"/>
-    <smbios mode="sysinfo"/>
-    <bootmenu enable="no"/>
-    <bios rebootTimeout="3"/>
     <firmware>
       <feature enabled="yes" name="secure-boot"/>
       <feature enabled="no" name="enrolled-keys"/>
     </firmware>
+    <loader>/foo/bar</loader>
     <initarg>foo=bar</initarg>
     <initarg>baz=woo</initarg>
+    <initdir>/my/custom/cwd</initdir>
+    <inituser>tester</inituser>
+    <initgroup>1000</initgroup>
+    <boot dev="network"/>
+    <bootmenu enable="no"/>
+    <bios rebootTimeout="3"/>
+    <smbios mode="sysinfo"/>
   </os>
   <idmap>
     <uid start="0" target="1000" count="10"/>
@@ -345,7 +345,13 @@
   <bootloader>/new/bootld</bootloader>
   <os firmware="efi">
     <type arch="x86_64" machine="q35">hvm</type>
+    <firmware>
+      <feature enabled="yes" name="secure-boot"/>
+      <feature enabled="no" name="enrolled-keys"/>
+    </firmware>
     <loader>/foo/bar</loader>
+    <initarg>foo=bar</initarg>
+    <initarg>baz=woo</initarg>
     <initdir>/my/custom/cwd</initdir>
     <inituser>tester</inituser>
     <initgroup>1000</initgroup>
@@ -353,15 +359,9 @@
     <boot dev="fd"/>
     <boot dev="hd"/>
     <boot dev="network"/>
-    <smbios mode="sysinfo"/>
     <bootmenu enable="no"/>
     <bios rebootTimeout="3"/>
-    <firmware>
-      <feature enabled="yes" name="secure-boot"/>
-      <feature enabled="no" name="enrolled-keys"/>
-    </firmware>
-    <initarg>foo=bar</initarg>
-    <initarg>baz=woo</initarg>
+    <smbios mode="sysinfo"/>
   </os>
   <idmap>
     <uid start="0" target="1000" count="10"/>

tests/data/cli/compare/virt-install-singleton-config-3.xml

@@ -46,9 +46,9 @@
     <loader readonly="yes" type="rom">/tmp/foo</loader>
     <cmdline>root=/foo</cmdline>
     <boot dev="hd"/>
-    <smbios mode="sysinfo"/>
     <bootmenu enable="yes" timeout="5000"/>
     <bios useserial="no" rebootTimeout="60"/>
+    <smbios mode="sysinfo"/>
   </os>
   <features>
     <acpi/>

tests/data/cli/compare/virt-xml-edit-simple-boot.xml

@@ -4,8 +4,8 @@
 -    <loader type="rom">/usr/lib/xen/boot/hvmloader</loader>
 +    <loader type="rom" readonly="yes" secure="no">foo.bar</loader>
 +    <nvram>/test/nvram.img</nvram>
-+    <bios useserial="yes"/>
 +    <init>/bin/bash</init>
++    <bios useserial="yes"/>
    </os>
    <idmap>
      <uid start="0" target="1000" count="10"/>

tests/data/xmlparse/change-guest-out.xml

@@ -11,13 +11,13 @@
   <os>
     <type machine="pc-0.11">xen</type>
     <loader>/foo/loader</loader>
-    <boot dev="fd"/>
     <init>/sbin/init</init>
-    <bootmenu enable="no" timeout="30000"/>
-    <bios useserial="yes" rebootTimeout="-1"/>
     <initarg>foo</initarg>
     <initarg>bar baz</initarg>
     <initarg>frib</initarg>
+    <boot dev="fd"/>
+    <bootmenu enable="no" timeout="30000"/>
+    <bios useserial="yes" rebootTimeout="-1"/>
   </os>
   <features>
     <apic eoi="on"/>

virtinst/cli.py

@@ -2659,7 +2659,9 @@ class ParserBoot(VirtCLIParser):
         cls.add_arg("network", None, lookup_cb=None, cb=cls.noset_cb)
 
         # UEFI depends on these bits, so set them first
+        cls.add_arg("os_type", "os_type")
         cls.add_arg("arch", "arch")
+        cls.add_arg("machine", "machine")
         cls.add_arg("bootloader", None, lookup_cb=None,
                 cb=cls.set_bootloader_cb)
         cls.add_arg("bootloader_args", None, lookup_cb=None,
@@ -2670,9 +2672,30 @@ class ParserBoot(VirtCLIParser):
                 cb=cls.set_emulator_cb)
         cls.add_arg("uefi", None, lookup_cb=None,
                 cb=cls.set_uefi_cb)
-        cls.add_arg("os_type", "os_type")
-        cls.add_arg("machine", "machine")
 
+        # Common/Shared boot options
+        cls.add_arg("loader", "loader")
+        cls.add_arg("loader.readonly", "loader_ro", is_onoff=True)
+        cls.add_arg("loader.type", "loader_type")
+        cls.add_arg("loader.secure", "loader_secure", is_onoff=True)
+
+        # Guest-Based bootloader options
+        cls.add_arg("firmware", "firmware")
+        cls.add_arg("firmware.feature[0-9]*.enabled", "enabled",
+                    find_inst_cb=cls.feature_find_inst_cb, is_onoff=True)
+        cls.add_arg("firmware.feature[0-9]*.name", "name",
+                    find_inst_cb=cls.feature_find_inst_cb)
+        cls.add_arg("nvram", "nvram")
+        cls.add_arg("nvram.template", "nvram_template")
+        cls.add_arg("boot[0-9]*.dev", "dev",
+                    find_inst_cb=cls.boot_find_inst_cb)
+        cls.add_arg("bootmenu.enable", "bootmenu_enable", is_onoff=True)
+        cls.add_arg("bootmenu.timeout", "bootmenu_timeout")
+        cls.add_arg("bios.useserial", "bios_useserial", is_onoff=True)
+        cls.add_arg("bios.rebootTimeout", "bios_rebootTimeout")
+        cls.add_arg("smbios.mode", "smbios_mode")
+
+        # Direct kernel boot options
         cls.add_arg("kernel", "kernel")
         cls.add_arg("initrd", "initrd")
         cls.add_arg("cmdline", "kernel_args", can_comma=True)
@@ -2680,17 +2703,7 @@ class ParserBoot(VirtCLIParser):
         cls.add_arg("acpi.table", "acpi_tb")
         cls.add_arg("acpi.table.type", "acpi_tb_type")
 
-        cls.add_arg("firmware", "firmware")
-        cls.add_arg("firmware.feature[0-9]*.enabled", "enabled",
-                    find_inst_cb=cls.feature_find_inst_cb, is_onoff=True)
-        cls.add_arg("firmware.feature[0-9]*.name", "name",
-                    find_inst_cb=cls.feature_find_inst_cb)
-        cls.add_arg("boot[0-9]*.dev", "dev",
-                    find_inst_cb=cls.boot_find_inst_cb)
-        cls.add_arg("bootmenu.enable", "bootmenu_enable", is_onoff=True)
-        cls.add_arg("bootmenu.timeout", "bootmenu_timeout")
-        cls.add_arg("bios.useserial", "bios_useserial", is_onoff=True)
-        cls.add_arg("bios.rebootTimeout", "bios_rebootTimeout")
+        # Container boot options
         cls.add_arg("init", "init")
         cls.add_arg("initargs", "initargs", cb=cls.set_initargs_cb)
         cls.add_arg("initarg[0-9]*", "val",
@@ -2702,13 +2715,6 @@ class ParserBoot(VirtCLIParser):
         cls.add_arg("initdir", "initdir")
         cls.add_arg("inituser", "inituser")
         cls.add_arg("initgroup", "initgroup")
-        cls.add_arg("loader", "loader")
-        cls.add_arg("loader.readonly", "loader_ro", is_onoff=True)
-        cls.add_arg("loader.type", "loader_type")
-        cls.add_arg("loader.secure", "loader_secure", is_onoff=True)
-        cls.add_arg("nvram", "nvram")
-        cls.add_arg("nvram.template", "nvram_template")
-        cls.add_arg("smbios.mode", "smbios_mode")
 
 
 ###################

virtinst/domain/os.py

@@ -78,13 +78,30 @@ class DomainOs(XMLBuilder):
     def is_riscv_virt(self):
         return self.is_riscv() and str(self.machine).startswith("virt")
 
-    XML_NAME = "os"
-    _XML_PROP_ORDER = ["arch", "os_type", "loader", "loader_ro", "loader_type",
-                       "nvram", "nvram_template", "kernel", "initrd",
-                       "initdir", "inituser", "initgroup",
-                       "kernel_args", "dtb", "bootdevs", "smbios_mode"]
+    ##################
+    # XML properties #
+    ##################
 
-    # BIOS bootloader
+    XML_NAME = "os"
+    _XML_PROP_ORDER = [
+            "firmware", "os_type", "arch", "machine", "firmware_features",
+            "loader", "loader_ro", "loader_secure", "loader_type",
+            "nvram", "nvram_template",
+            "init", "initargs", "initenvs", "initdir", "inituser", "initgroup",
+            "kernel", "initrd", "kernel_args", "dtb", "acpi_tb", "acpi_tb_type",
+            "bootdevs", "bootmenu_enable", "bootmenu_timeout",
+            "bios_useserial", "bios_rebootTimeout", "smbios_mode"]
+
+    # Shared/Generic boot options
+    os_type = XMLProperty("./type")
+    arch = XMLProperty("./type/@arch")
+    machine = XMLProperty("./type/@machine")
+    loader = XMLProperty("./loader")
+    loader_ro = XMLProperty("./loader/@readonly", is_yesno=True)
+    loader_type = XMLProperty("./loader/@type")
+    loader_secure = XMLProperty("./loader/@secure", is_yesno=True)
+
+    # BIOS bootloader options
     def _get_bootorder(self):
         return [dev.dev for dev in self.bootdevs]
     def _set_bootorder(self, newdevs):
@@ -96,18 +113,22 @@ class DomainOs(XMLBuilder):
             dev.dev = d
     bootorder = property(_get_bootorder, _set_bootorder)
     bootdevs = XMLChildProperty(_BootDevice)
-    smbios_mode = XMLProperty("./smbios/@mode")
+    firmware = XMLProperty("./@firmware")
+    firmware_features = XMLChildProperty(_FirmwareFeature, relative_xpath="./firmware")
+    nvram = XMLProperty("./nvram", do_abspath=True)
+    nvram_template = XMLProperty("./nvram/@template")
     bootmenu_enable = XMLProperty("./bootmenu/@enable", is_yesno=True)
     bootmenu_timeout = XMLProperty("./bootmenu/@timeout", is_int=True)
-    bios_rebootTimeout = XMLProperty("./bios/@rebootTimeout", is_int=True)
     bios_useserial = XMLProperty("./bios/@useserial", is_yesno=True)
+    bios_rebootTimeout = XMLProperty("./bios/@rebootTimeout", is_int=True)
+    smbios_mode = XMLProperty("./smbios/@mode")
 
-    # Host bootloader
+    # Host bootloader options
     # Since the elements for a host bootloader are actually directly under
     # <domain> rather than <domain><os>, they are handled via callbacks in
     # the CLI. This is just a placeholder to remind of that fact.
 
-    # Direct kernel boot
+    # Direct kernel boot options
     kernel = XMLProperty("./kernel", do_abspath=True)
     initrd = XMLProperty("./initrd", do_abspath=True)
     kernel_args = XMLProperty("./cmdline")
@@ -115,7 +136,7 @@ class DomainOs(XMLBuilder):
     acpi_tb = XMLProperty("./acpi/table", do_abspath=True)
     acpi_tb_type = XMLProperty("./acpi/table/@type")
 
-    # Container boot
+    # Container boot options
     init = XMLProperty("./init")
     initargs = XMLChildProperty(_InitArg)
     initenvs = XMLChildProperty(_InitEnv)
@@ -130,18 +151,6 @@ class DomainOs(XMLBuilder):
             obj = self.initargs.add_new()
             obj.val = val
 
-    loader = XMLProperty("./loader")
-    loader_ro = XMLProperty("./loader/@readonly", is_yesno=True)
-    loader_type = XMLProperty("./loader/@type")
-    loader_secure = XMLProperty("./loader/@secure", is_yesno=True)
-    nvram = XMLProperty("./nvram", do_abspath=True)
-    nvram_template = XMLProperty("./nvram/@template")
-    arch = XMLProperty("./type/@arch")
-    machine = XMLProperty("./type/@machine")
-    os_type = XMLProperty("./type")
-    firmware = XMLProperty("./@firmware")
-    firmware_features = XMLChildProperty(_FirmwareFeature, relative_xpath="./firmware")
-
 
     ##################
     # Default config #