virtinst: guest: drop 'continue_install' concept

continue_install is intended to facilitate windows XP style 3 stage
installs:

  stage 1: initial dos style disk setup, reboot
  stage 2: actual full installer, reboot
  stage 3: OS is functional, virt-install is done

The code assumed that we needed to keep the cdrom as the primary
boot device for the second stage, so virt-install/virt-manager needed
to hang around through the second stage run, wait until the VM shutdown,
then encode the final XML to boot of the disk.

Windows is and always has been smart enough to handle that case though...
after the initial boot, if we set the hd as the primary boot device
for stage 2, the disk bits that windows already installed will make
use of the cdrom as necessary. So the entire premise of continue_install
is irrelevant. Maybe back when it was added, when xen didn't even have
working ACPI support, this served a purpose, but I'm pretty sure we
can safely drop it nowadays.

7 files changed, 44 insertions, 331 deletions

diff --git a/tests/cli-test-xml/compare/virt-install-kvm-win2k3-cdrom.xml b/tests/cli-test-xml/compare/virt-install-kvm-win2k3-cdrom.xml
index 8423178d..869634f4 100644
--- a/tests/cli-test-xml/compare/virt-install-kvm-win2k3-cdrom.xml
+++ b/tests/cli-test-xml/compare/virt-install-kvm-win2k3-cdrom.xml
@@ -99,76 +99,6 @@
     <timer name="hypervclock" present="yes"/>
   </clock>
   <on_poweroff>destroy</on_poweroff>
-  <on_reboot>destroy</on_reboot>
-  <on_crash>destroy</on_crash>
-  <pm>
-    <suspend-to-mem enabled="no"/>
-    <suspend-to-disk enabled="no"/>
-  </pm>
-  <devices>
-    <emulator>/usr/bin/qemu-kvm</emulator>
-    <disk type="file" device="disk">
-      <driver name="qemu" type="qcow2"/>
-      <source file="/dev/default-pool/testvol1.img"/>
-      <target dev="hda" bus="ide"/>
-    </disk>
-    <disk type="file" device="cdrom">
-      <driver name="qemu" type="qcow2"/>
-      <source file="/dev/default-pool/testvol2.img"/>
-      <target dev="hdb" bus="ide"/>
-      <readonly/>
-    </disk>
-    <controller type="usb" index="0"/>
-    <interface type="bridge">
-      <source bridge="eth0"/>
-      <mac address="00:11:22:33:44:55"/>
-    </interface>
-    <input type="tablet" bus="usb"/>
-    <graphics type="spice" port="-1" tlsPort="-1" autoport="yes">
-      <image compression="off"/>
-    </graphics>
-    <console type="pty"/>
-    <channel type="spicevmc">
-      <target type="virtio" name="com.redhat.spice.0"/>
-    </channel>
-    <sound model="ich6"/>
-    <video>
-      <model type="qxl"/>
-    </video>
-    <redirdev bus="usb" type="spicevmc"/>
-    <redirdev bus="usb" type="spicevmc"/>
-  </devices>
-</domain>
-<domain type="kvm">
-  <name>foobar</name>
-  <uuid>00000000-1111-2222-3333-444444444444</uuid>
-  <memory>65536</memory>
-  <currentMemory>65536</currentMemory>
-  <vcpu>1</vcpu>
-  <os>
-    <type arch="x86_64">hvm</type>
-    <boot dev="hd"/>
-  </os>
-  <features>
-    <acpi/>
-    <apic/>
-    <vmport state="off"/>
-    <hyperv>
-      <relaxed state="on"/>
-      <vapic state="on"/>
-      <spinlocks state="on" retries="8191"/>
-    </hyperv>
-  </features>
-  <cpu mode="custom" match="exact">
-    <model>Opteron_G4</model>
-  </cpu>
-  <clock offset="localtime">
-    <timer name="rtc" tickpolicy="catchup"/>
-    <timer name="pit" tickpolicy="delay"/>
-    <timer name="hpet" present="no"/>
-    <timer name="hypervclock" present="yes"/>
-  </clock>
-  <on_poweroff>destroy</on_poweroff>
   <on_reboot>restart</on_reboot>
   <on_crash>restart</on_crash>
   <pm>
diff --git a/tests/cli-test-xml/compare/virt-install-w2k3-cdrom.xml b/tests/cli-test-xml/compare/virt-install-w2k3-cdrom.xml
index 3660aa31..ac281c59 100644
--- a/tests/cli-test-xml/compare/virt-install-w2k3-cdrom.xml
+++ b/tests/cli-test-xml/compare/virt-install-w2k3-cdrom.xml
@@ -70,54 +70,6 @@
   </cpu>
   <clock offset="localtime"/>
   <on_poweroff>destroy</on_poweroff>
-  <on_reboot>destroy</on_reboot>
-  <on_crash>destroy</on_crash>
-  <pm>
-    <suspend-to-mem enabled="no"/>
-    <suspend-to-disk enabled="no"/>
-  </pm>
-  <devices>
-    <emulator>/usr/bin/test-hv</emulator>
-    <disk type="file" device="disk">
-      <source file="/dev/default-pool/testvol1.img"/>
-      <target dev="hda" bus="ide"/>
-    </disk>
-    <disk type="file" device="cdrom">
-      <source file="/dev/default-pool/testvol2.img"/>
-      <target dev="hdb" bus="ide"/>
-      <readonly/>
-    </disk>
-    <controller type="usb" index="0" model="none"/>
-    <interface type="user">
-      <mac address="00:11:22:33:44:55"/>
-    </interface>
-    <input type="mouse" bus="ps2"/>
-    <console type="pty"/>
-  </devices>
-</domain>
-<domain type="test">
-  <name>foobar</name>
-  <uuid>00000000-1111-2222-3333-444444444444</uuid>
-  <memory>65536</memory>
-  <currentMemory>65536</currentMemory>
-  <vcpu>4</vcpu>
-  <os>
-    <type arch="i686">hvm</type>
-    <boot dev="hd"/>
-  </os>
-  <features>
-    <pae/>
-    <hyperv>
-      <relaxed state="on"/>
-      <vapic state="on"/>
-      <spinlocks state="on" retries="8191"/>
-    </hyperv>
-  </features>
-  <cpu>
-    <topology sockets="1" cores="4" threads="1"/>
-  </cpu>
-  <clock offset="localtime"/>
-  <on_poweroff>destroy</on_poweroff>
   <on_reboot>restart</on_reboot>
   <on_crash>restart</on_crash>
   <pm>
diff --git a/tests/cli-test-xml/compare/virt-install-win7-uefi.xml b/tests/cli-test-xml/compare/virt-install-win7-uefi.xml
index 4c3b9b62..8d98056a 100644
--- a/tests/cli-test-xml/compare/virt-install-win7-uefi.xml
+++ b/tests/cli-test-xml/compare/virt-install-win7-uefi.xml
@@ -100,81 +100,6 @@
     <timer name="hpet" present="no"/>
   </clock>
   <on_poweroff>destroy</on_poweroff>
-  <on_reboot>destroy</on_reboot>
-  <on_crash>destroy</on_crash>
-  <pm>
-    <suspend-to-mem enabled="no"/>
-    <suspend-to-disk enabled="no"/>
-  </pm>
-  <devices>
-    <emulator>/usr/bin/qemu-kvm</emulator>
-    <disk type="file" device="disk">
-      <driver name="qemu" type="qcow2"/>
-      <source file="/dev/default-pool/testvol1.img"/>
-      <target dev="hda" bus="ide"/>
-    </disk>
-    <disk type="file" device="cdrom">
-      <driver name="qemu" type="qcow2"/>
-      <source file="/dev/default-pool/testvol2.img"/>
-      <target dev="hdb" bus="ide"/>
-      <readonly/>
-    </disk>
-    <controller type="usb" index="0" model="ich9-ehci1"/>
-    <controller type="usb" index="0" model="ich9-uhci1">
-      <master startport="0"/>
-    </controller>
-    <controller type="usb" index="0" model="ich9-uhci2">
-      <master startport="2"/>
-    </controller>
-    <controller type="usb" index="0" model="ich9-uhci3">
-      <master startport="4"/>
-    </controller>
-    <interface type="bridge">
-      <source bridge="eth0"/>
-      <mac address="00:11:22:33:44:55"/>
-    </interface>
-    <input type="tablet" bus="usb"/>
-    <graphics type="spice" port="-1" tlsPort="-1" autoport="yes">
-      <image compression="off"/>
-    </graphics>
-    <console type="pty"/>
-    <channel type="spicevmc">
-      <target type="virtio" name="com.redhat.spice.0"/>
-    </channel>
-    <sound model="ich6"/>
-    <video>
-      <model type="qxl"/>
-    </video>
-    <redirdev bus="usb" type="spicevmc"/>
-    <redirdev bus="usb" type="spicevmc"/>
-  </devices>
-</domain>
-<domain type="kvm">
-  <name>foobar</name>
-  <uuid>00000000-1111-2222-3333-444444444444</uuid>
-  <memory>65536</memory>
-  <currentMemory>65536</currentMemory>
-  <vcpu>1</vcpu>
-  <os>
-    <type arch="x86_64">hvm</type>
-    <loader type="pflash">CODE.fd</loader>
-    <nvram template="VARS.fd"/>
-    <boot dev="hd"/>
-  </os>
-  <features>
-    <acpi/>
-    <apic/>
-    <vmport state="off"/>
-  </features>
-  <cpu mode="custom" match="exact">
-    <model>Opteron_G4</model>
-  </cpu>
-  <clock offset="localtime">
-    <timer name="rtc" tickpolicy="catchup"/>
-    <timer name="pit" tickpolicy="delay"/>
-    <timer name="hpet" present="no"/>
-  </clock>
-  <on_poweroff>destroy</on_poweroff>
   <on_reboot>restart</on_reboot>
   <on_crash>restart</on_crash>
   <pm>
diff --git a/tests/clitest.py b/tests/clitest.py
index 91ff94e7..43373e8f 100644
--- a/tests/clitest.py
+++ b/tests/clitest.py
@@ -646,7 +646,7 @@ c.add_valid("--paravirt --location %(TREEDIR)s")  # Paravirt location
 c.add_valid("--paravirt --location %(TREEDIR)s --os-variant none")  # Paravirt location with --os-variant none
 c.add_valid("--location %(TREEDIR)s --os-variant fedora12")  # URL install with manual os-variant
 c.add_valid("--cdrom %(EXISTIMG2)s --os-variant win2k3 --wait 0")  # HVM windows install with disk
-c.add_valid("--cdrom %(EXISTIMG2)s --os-variant win2k3 --wait 0 --print-step 3")  # HVM windows install, print 3rd stage XML
+c.add_valid("--cdrom %(EXISTIMG2)s --os-variant win2k3 --wait 0 --print-step 2")  # HVM windows install, print 3rd stage XML
 c.add_valid("--pxe --autostart")  # --autostart flag
 c.add_compare("--pxe --print-step all", "simple-pxe")  # Diskless PXE install
 c.add_invalid("--pxe --virt-type bogus")  # Bogus virt-type
diff --git a/virt-install b/virt-install
index 0bfe2e52..6d81fc53 100755
--- a/virt-install
+++ b/virt-install
@@ -683,23 +683,13 @@ def domain_is_active(domain):
         return False
 
 
-def start_install(guest, continue_inst, options):
-    # There are two main cases we care about:
-    #
-    # Scripts: these should specify --wait always, maintaining the
-    # semantics of virt-install exit implying the domain has finished
-    # installing.
-    #
-    # Interactive: If this is a continue_inst domain, we default to
-    # waiting.  Otherwise, we can exit before the domain has finished
-    # installing. Passing --wait will give the above semantics.
-    #
-    if options.wait is None:
-        wait_on_install = continue_inst
-        wait_time = -1
-    else:
+def start_install(guest, options):
+    if options.wait:
         wait_on_install = True
         wait_time = options.wait * 60
+    else:
+        wait_on_install = False
+        wait_time = -1
 
     # If --wait specified, we don't want the default behavior of waiting
     # for virt-viewer to exit, since then we can't exit the app when time
@@ -742,12 +732,6 @@ def start_install(guest, continue_inst, options):
         dom = check_domain(guest, dom, conscb,
                            wait_on_install, wait_time, start_time)
 
-        if continue_inst:
-            dom = guest.continue_install(meter=meter)
-            cli.connect_console(guest, conscb, wait_on_console)
-            dom = check_domain(guest, dom, conscb,
-                               wait_on_install, wait_time, start_time)
-
         print_stdout(_("Domain creation completed."))
         if not domain_is_active(dom):
             if options.noreboot or not guest.installer.has_install_phase():
@@ -854,36 +838,29 @@ def check_domain(guest, dom, conscb, wait_for_install, wait_time, start_time):
 # XML printing helpers #
 ########################
 
-def xml_to_print(guest, continue_inst, xmlonly, dry):
+def xml_to_print(guest, xmlonly, dry):
     start_xml, final_xml = guest.start_install(dry=dry, return_xml=True)
-    second_xml = None
     if not start_xml:
         start_xml = final_xml
         final_xml = None
 
-    if continue_inst:
-        second_xml, final_xml = guest.continue_install(dry=dry,
-                                                       return_xml=True)
-
     if dry and not xmlonly:
         print_stdout(_("Dry run completed successfully"))
         return
 
+    if xmlonly not in [False, "1", "2", "all"]:
+        fail(_("Unknown XML step request '%s', must be 1, 2, or all") %
+             xmlonly)
+
     if xmlonly == "1":
         return start_xml
     if xmlonly == "2":
-        if not (second_xml or final_xml):
+        if not final_xml:
             fail(_("Requested installation does not have XML step 2"))
-        return second_xml or final_xml
-    if xmlonly == "3":
-        if not second_xml:
-            fail(_("Requested installation does not have XML step 3"))
         return final_xml
 
     # "all" case
     xml = start_xml
-    if second_xml:
-        xml += second_xml
     if final_xml:
         xml += final_xml
     return xml
@@ -1040,19 +1017,13 @@ def main(conn=None):
         do_test_media_detection(conn, options.test_media_detection)
         return 0
 
-    if options.xmlonly not in [False, "1", "2", "3", "all"]:
-        fail(_("--print-step must be 1, 2, 3, or all"))
-
     guest = build_guest_instance(conn, options)
-    continue_inst = guest.get_continue_inst()
-
     if options.xmlonly or options.dry:
-        xml = xml_to_print(guest, continue_inst,
-                           options.xmlonly, options.dry)
+        xml = xml_to_print(guest, options.xmlonly, options.dry)
         if xml:
             print_stdout(xml, do_force=True)
     else:
-        start_install(guest, continue_inst, options)
+        start_install(guest, options)
 
     return 0
 
diff --git a/virtManager/create.py b/virtManager/create.py
index 3ee74f8f..737c1faa 100644
--- a/virtManager/create.py
+++ b/virtManager/create.py
@@ -2316,7 +2316,7 @@ class vmmCreate(vmmGObjectUI):
             # guest after the install has finished
             def cb():
                 vm.connect_opt_out("state-changed",
-                                   self._check_install_status, guest)
+                                   self._check_install_status)
                 return False
             self.idle_add(cb)
 
@@ -2330,7 +2330,7 @@ class vmmCreate(vmmGObjectUI):
                     "VM creation.", poolname, exc_info=True)
 
 
-    def _check_install_status(self, vm, virtinst_guest):
+    def _check_install_status(self, vm):
         """
         Watch the domain that we are installing, waiting for the state
         to change, so we can restart it as needed
@@ -2348,20 +2348,6 @@ class vmmCreate(vmmGObjectUI):
             return True
 
         try:
-            if virtinst_guest:
-                continue_inst = virtinst_guest.get_continue_inst()
-
-                if continue_inst:
-                    logging.debug("VM needs a 2 stage install, continuing.")
-                    # Continue the install, then reconnect this opt
-                    # out handler, removing the virtinst_guest which
-                    # will force one final restart.
-                    virtinst_guest.continue_install()
-
-                    vm.connect_opt_out("state-changed",
-                                       self._check_install_status, None)
-                    return True
-
             logging.debug("Install should be completed, starting VM.")
             vm.startup()
         except Exception, e:
diff --git a/virtinst/guest.py b/virtinst/guest.py
index 73566bae..22da8fe2 100644
--- a/virtinst/guest.py
+++ b/virtinst/guest.py
@@ -331,35 +331,29 @@ class Guest(XMLBuilder):
         finally:
             self._finish_get_xml(data)
 
-    def _do_get_install_xml(self, install=True, disk_boot=False):
+    def _do_get_install_xml(self, install):
         """
         Return the full Guest xml configuration.
 
-        @param install: Whether we want the 'OS install' configuration or
-                        the 'post-install' configuration. (Some installs,
-                        like an import or livecd may not have an 'install'
-                        config.)
-        @type install: C{bool}
-        @param disk_boot: Whether we should boot off the harddisk, regardless
-                          of our position in the install process (this is
-                          used for 2 stage installs, where the second stage
-                          boots off the disk. You probably don't need to touch
-                          this.)
-        @type disk_boot: C{bool}
+        @install: Whether we want the 'OS install' configuration or
+            the 'post-install' configuration. The difference is mostly
+            whether the install media is attached and set as the boot
+            device. Some installs, like an import or livecd, do not have
+            an 'install' config.
         """
-        osblob_install = install and not disk_boot
-        if osblob_install and not self.installer.has_install_phase():
+        if install and not self.installer.has_install_phase():
             return None
 
-        self.installer.alter_bootconfig(self, osblob_install)
+        self.installer.alter_bootconfig(self, install)
         if not install:
             self._remove_cdrom_install_media()
 
         if install:
             self.on_reboot = "destroy"
             self.on_crash = "destroy"
-        # on_crash=restart can cause reboot loops on s390x, so use preserve
         elif self.os.is_s390x():
+            # on_crash=restart can cause reboot loops on s390x,
+            # so use preserve
             self.on_crash = "preserve"
 
         self._set_osxml_defaults()
@@ -378,60 +372,46 @@ class Guest(XMLBuilder):
     # Private install helpers #
     ###########################
 
-    def _build_meter(self, meter, is_initial):
-        if is_initial:
-            meter_label = _("Creating domain...")
-        else:
-            meter_label = _("Starting domain...")
-
+    def _build_meter(self, meter):
+        meter_label = _("Creating domain...")
         meter = util.ensure_meter(meter)
         meter.start(size=None, text=meter_label)
 
         return meter
 
-    def _build_xml(self, is_initial):
-        log_label = is_initial and "install" or "continue"
-        disk_boot = not is_initial
-
-        start_xml = self._get_install_xml(install=True, disk_boot=disk_boot)
+    def _build_xml(self):
+        start_xml = self._get_install_xml(install=True)
         final_xml = self._get_install_xml(install=False)
 
-        logging.debug("Generated %s XML: %s",
-                      log_label,
+        logging.debug("Generated install XML: %s",
                       (start_xml and ("\n" + start_xml) or "None required"))
         logging.debug("Generated boot XML: \n%s", final_xml)
 
         return start_xml, final_xml
 
-    def _create_guest(self, meter,
-                      start_xml, final_xml, is_initial, noboot):
+    def _create_guest(self, meter, start_xml, final_xml, noboot):
         """
         Actually do the XML logging, guest defining/creating
 
-        @param is_initial: If running initial guest creation, else we
-                           are continuing the install
         @param noboot: Don't boot guest if no install phase
         """
-        meter = self._build_meter(meter, is_initial)
+        meter = self._build_meter(meter)
         doboot = not noboot or self.installer.has_install_phase()
 
-        if is_initial and doboot:
+        if doboot:
             dom = self.conn.createLinux(start_xml or final_xml, 0)
         else:
             dom = self.conn.defineXML(start_xml or final_xml)
-            if doboot:
-                dom.create()
 
         self.domain = dom
         meter.end(0)
 
         self.domain = self.conn.defineXML(final_xml)
-        if is_initial:
-            try:
-                logging.debug("XML fetched from libvirt object:\n%s",
-                              dom.XMLDesc(0))
-            except Exception, e:
-                logging.debug("Error fetching XML from libvirt object: %s", e)
+        try:
+            logging.debug("XML fetched from libvirt object:\n%s",
+                          dom.XMLDesc(0))
+        except Exception, e:
+            logging.debug("Error fetching XML from libvirt object: %s", e)
 
         return self.domain
 
@@ -458,18 +438,6 @@ class Guest(XMLBuilder):
     # Public API #
     ##############
 
-    def get_continue_inst(self):
-        """
-        Return True if this guest requires a call to 'continue_install',
-        which means the OS requires a 2 stage install (windows)
-        """
-        # If we are doing an 'import' or 'liveCD' install, there is
-        # no true install process, so continue install has no meaning
-        if not self.installer.has_install_phase():
-            return False
-
-        return self._os_object.is_windows()
-
     def start_install(self, meter=None,
                       dry=False, return_xml=False, noboot=False):
         """
@@ -479,7 +447,6 @@ class Guest(XMLBuilder):
         if self.domain is not None:
             raise RuntimeError(_("Domain has already been started!"))
 
-        is_initial = True
         self.set_install_defaults()
 
         self._prepare_install(meter, dry)
@@ -489,7 +456,7 @@ class Guest(XMLBuilder):
                 for dev in self.get_all_devices():
                     dev.setup(meter)
 
-            start_xml, final_xml = self._build_xml(is_initial)
+            start_xml, final_xml = self._build_xml()
             if return_xml:
                 return (start_xml, final_xml)
             if dry:
@@ -499,8 +466,7 @@ class Guest(XMLBuilder):
             self.check_vm_collision(self.conn, self.name,
                                     do_remove=self.replace)
 
-            self.domain = self._create_guest(meter,
-                                             start_xml, final_xml, is_initial,
+            self.domain = self._create_guest(meter, start_xml, final_xml,
                                              noboot)
             # Set domain autostart flag if requested
             self._flag_autostart()
@@ -509,23 +475,6 @@ class Guest(XMLBuilder):
         finally:
             self.installer.cleanup()
 
-    def continue_install(self, meter=None,
-                         dry=False, return_xml=False):
-        """
-        Continue with stage 2 of a guest install. Only required for
-        guests which have the 'continue' flag set (accessed via
-        get_continue_inst)
-        """
-        is_initial = False
-        start_xml, final_xml = self._build_xml(is_initial)
-        if return_xml:
-            return (start_xml, final_xml)
-        if dry:
-            return
-
-        return self._create_guest(meter,
-                                  start_xml, final_xml, is_initial, False)
-
     def get_created_disks(self):
         return [d for d in self.get_devices("disk") if d.storage_was_created]
 
@@ -711,11 +660,11 @@ class Guest(XMLBuilder):
     def _remove_cdrom_install_media(self):
         for dev in self.get_devices("disk"):
             # Keep the install cdrom device around, but with no media attached.
-            # But only if we are a distro that doesn't have a multi
-            # stage install (aka not Windows)
+            # But only if we are installing windows which has a multi stage
+            # install.
             if (dev.is_cdrom() and
                 getattr(dev, "installer_media", False) and
-                not self.get_continue_inst()):
+                not self._os_object.is_windows()):
                 dev.path = None
 
     def _set_defaults(self):