guest: Drop self.installer and start_install wrapper

This changes all the callers to invoke start_install directly on the
Installer object. We still stash the installer instance inside the
guest object in create.py, just for simplicity
This commit is contained in:
Cole Robinson 2018-09-03 15:21:11 -04:00
parent c154bbacd4
commit 2ac54ac001
8 changed files with 65 additions and 79 deletions

View File

@ -7,6 +7,7 @@ import io
import os import os
import unittest import unittest
from virtinst import Installer
from virtconv import VirtConverter from virtconv import VirtConverter
from tests import utils from tests import utils
@ -34,7 +35,8 @@ class TestVirtConv(unittest.TestCase):
converter.convert_disks(disk_format, dry=True) converter.convert_disks(disk_format, dry=True)
guest = converter.get_guest() guest = converter.get_guest()
ignore, out_xml = guest.start_install(return_xml=True) installer = Installer(guest.conn)
ignore, out_xml = installer.start_install(guest, return_xml=True)
out_expect = out_xml out_expect = out_xml
if outbuf.getvalue(): if outbuf.getvalue():
out_expect += ("\n\n" + outbuf.getvalue().replace(base_dir, "")) out_expect += ("\n\n" + outbuf.getvalue().replace(base_dir, ""))

View File

@ -14,14 +14,9 @@ from virtcli import CLIConfig
from tests import utils from tests import utils
def _make_guest(installer=None, conn=None, os_variant=None): def _make_guest(conn=None, os_variant=None):
if not conn: if not conn:
if installer: conn = utils.URIs.open_testdriver_cached()
conn = installer.conn
else:
conn = utils.URIs.open_testdriver_cached()
if not installer:
installer = _make_installer(conn=conn)
g = conn.caps.lookup_virtinst_guest() g = conn.caps.lookup_virtinst_guest()
g.type = "kvm" g.type = "kvm"
@ -36,7 +31,6 @@ def _make_guest(installer=None, conn=None, os_variant=None):
g.features.pae = False g.features.pae = False
g.vcpus = 5 g.vcpus = 5
g.installer = installer
g.emulator = "/usr/lib/xen/bin/qemu-dm" g.emulator = "/usr/lib/xen/bin/qemu-dm"
g.os.arch = "i686" g.os.arch = "i686"
g.os.os_type = "hvm" g.os.os_type = "hvm"
@ -107,7 +101,9 @@ class TestXMLMisc(unittest.TestCase):
def _compare(self, guest, filebase, do_install): def _compare(self, guest, filebase, do_install):
filename = os.path.join("tests/xmlconfig-xml", filebase + ".xml") filename = os.path.join("tests/xmlconfig-xml", filebase + ".xml")
inst_xml, boot_xml = guest.start_install(return_xml=True, dry=True) installer = _make_installer(conn=guest.conn)
inst_xml, boot_xml = installer.start_install(
guest, return_xml=True, dry=True)
if do_install: if do_install:
actualXML = inst_xml actualXML = inst_xml
else: else:
@ -158,13 +154,13 @@ class TestXMLMisc(unittest.TestCase):
# does much more exhaustive testing but it's only run occasionally # does much more exhaustive testing but it's only run occasionally
i = _make_installer( i = _make_installer(
location="tests/cli-test-xml/fakefedoratree") location="tests/cli-test-xml/fakefedoratree")
g = _make_guest(i) g = _make_guest()
v = i.detect_distro(g) v = i.detect_distro(g)
self.assertEqual(v, "fedora17") self.assertEqual(v, "fedora17")
i = _make_installer( i = _make_installer(
location="tests/cli-test-xml/fakerhel6tree") location="tests/cli-test-xml/fakerhel6tree")
g = _make_guest(i) g = _make_guest()
v = i.detect_distro(g) v = i.detect_distro(g)
self.assertEqual(v, "rhel6.0") self.assertEqual(v, "rhel6.0")

View File

@ -13,6 +13,7 @@
import sys import sys
from virtinst import cli from virtinst import cli
from virtinst import Installer
from virtinst.cli import fail, print_stderr, print_stdout from virtinst.cli import fail, print_stderr, print_stdout
from virtconv import VirtConverter from virtconv import VirtConverter
@ -99,17 +100,18 @@ def main(conn=None):
destdir=options.destination, dry=options.dry) destdir=options.destination, dry=options.dry)
guest = converter.get_guest() guest = converter.get_guest()
installer = Installer(guest.conn)
conscb = None conscb = None
if options.autoconsole: if options.autoconsole:
conscb = cli.get_console_cb(guest) or None conscb = cli.get_console_cb(guest) or None
if options.xmlonly: if options.xmlonly:
print_stdout(guest.start_install(return_xml=True)[1], print_stdout(installer.start_install(guest, return_xml=True)[1],
do_force=True) do_force=True)
elif not options.dry: elif not options.dry:
print_stdout(_("Creating guest '%s'.") % guest.name) print_stdout(_("Creating guest '%s'.") % guest.name)
domain = guest.start_install() domain = installer.start_install(guest)
cli.connect_console(guest, domain, conscb, True) cli.connect_console(guest, domain, conscb, True)
except Exception: except Exception:
converter.cleanup() converter.cleanup()

View File

@ -354,31 +354,31 @@ def get_guest(conn, options):
# Install media setup/validation # # Install media setup/validation #
################################## ##################################
def set_distro_variant(options, guest): def set_distro_variant(options, guest, installer):
try: try:
if options.distro_variant not in ["auto", "none"]: if options.distro_variant not in ["auto", "none"]:
guest.os_variant = options.distro_variant guest.os_variant = options.distro_variant
guest.installer.check_location(guest) installer.check_location(guest)
if options.distro_variant == "auto": if options.distro_variant == "auto":
guest.os_variant = guest.installer.detect_distro(guest) guest.os_variant = installer.detect_distro(guest)
except ValueError as e: except ValueError as e:
fail(_("Error validating install location: %s") % str(e)) fail(_("Error validating install location: %s") % str(e))
def do_test_media_detection(conn, url): def do_test_media_detection(conn, url):
guest = conn.caps.lookup_virtinst_guest() guest = conn.caps.lookup_virtinst_guest()
guest.installer = virtinst.DistroInstaller(conn) installer = virtinst.DistroInstaller(conn)
guest.installer.location = url installer.location = url
print_stdout(guest.installer.detect_distro(guest), do_force=True) print_stdout(installer.detect_distro(guest), do_force=True)
############################# #############################
# General option validation # # General option validation #
############################# #############################
def validate_required_options(options, guest): def validate_required_options(options, guest, installer):
# Required config. Don't error right away if nothing is specified, # Required config. Don't error right away if nothing is specified,
# aggregate the errors to help first time users get it right # aggregate the errors to help first time users get it right
msg = "" msg = ""
@ -394,7 +394,7 @@ def validate_required_options(options, guest):
msg += "\n" + ( msg += "\n" + (
_("--disk storage must be specified (override with --disk none)")) _("--disk storage must be specified (override with --disk none)"))
if not guest.installer: if not installer:
msg += "\n" + ( msg += "\n" + (
_("An install method must be specified\n(%(methods)s)") % _("An install method must be specified\n(%(methods)s)") %
{"methods": install_methods}) {"methods": install_methods})
@ -407,7 +407,7 @@ _cdrom_location_man_page = _("See the man page for examples of "
"using --location with CDROM media") "using --location with CDROM media")
def check_option_collisions(options, guest): def check_option_collisions(options, guest, installer):
if options.noreboot and options.transient: if options.noreboot and options.transient:
fail(_("--noreboot and --transient can not be specified together")) fail(_("--noreboot and --transient can not be specified together"))
@ -433,7 +433,7 @@ def check_option_collisions(options, guest):
fail(_("Libvirt version does not support remote --location installs")) fail(_("Libvirt version does not support remote --location installs"))
cdrom_err = "" cdrom_err = ""
if guest.installer.cdrom: if installer.cdrom:
cdrom_err = " " + _cdrom_location_man_page cdrom_err = " " + _cdrom_location_man_page
if not options.location and options.extra_args: if not options.location and options.extra_args:
fail(_("--extra-args only work if specified with --location.") + fail(_("--extra-args only work if specified with --location.") +
@ -443,13 +443,13 @@ def check_option_collisions(options, guest):
cdrom_err) cdrom_err)
def _show_nographics_warnings(options, guest): def _show_nographics_warnings(options, guest, installer):
if guest.devices.graphics: if guest.devices.graphics:
return return
if not options.autoconsole: if not options.autoconsole:
return return
if guest.installer.cdrom: if installer.cdrom:
logging.warning(_("CDROM media does not print to the text console " logging.warning(_("CDROM media does not print to the text console "
"by default, so you likely will not see text install output. " "by default, so you likely will not see text install output. "
"You might want to use --location.") + " " + "You might want to use --location.") + " " +
@ -490,7 +490,7 @@ def _show_nographics_warnings(options, guest):
"guest."), {"console_string": console_type}) "guest."), {"console_string": console_type})
def show_warnings(options, guest): def show_warnings(options, guest, installer):
if options.pxe and not supports_pxe(guest): if options.pxe and not supports_pxe(guest):
logging.warning(_("The guest's network configuration does not support " logging.warning(_("The guest's network configuration does not support "
"PXE")) "PXE"))
@ -500,7 +500,7 @@ def show_warnings(options, guest):
logging.warning(_("No operating system detected, VM performance may " logging.warning(_("No operating system detected, VM performance may "
"suffer. Specify an OS with --os-variant for optimal results.")) "suffer. Specify an OS with --os-variant for optimal results."))
_show_nographics_warnings(options, guest) _show_nographics_warnings(options, guest, installer)
########################## ##########################
@ -582,21 +582,21 @@ def build_guest_instance(conn, options):
logging.warning("Couldn't configure UEFI: %s", e) logging.warning("Couldn't configure UEFI: %s", e)
logging.warning("Your VM may not boot successfully.") logging.warning("Your VM may not boot successfully.")
guest.installer = build_installer(options, guest) installer = build_installer(options, guest)
validate_required_options(options, guest) validate_required_options(options, guest, installer)
set_distro_variant(options, guest) set_distro_variant(options, guest, installer)
check_option_collisions(options, guest) check_option_collisions(options, guest, installer)
show_warnings(options, guest) show_warnings(options, guest, installer)
return guest return guest, installer
########################### ###########################
# Install process helpers # # Install process helpers #
########################### ###########################
def start_install(guest, options): def start_install(guest, installer, options):
if options.wait is not None: if options.wait is not None:
wait_on_install = True wait_on_install = True
wait_time = options.wait * 60 wait_time = options.wait * 60
@ -633,7 +633,7 @@ def start_install(guest, options):
meter = cli.get_meter() meter = cli.get_meter()
logging.debug("Guest.has_install_phase: %s", logging.debug("Guest.has_install_phase: %s",
guest.installer.has_install_phase()) installer.has_install_phase())
# we've got everything -- try to start the install # we've got everything -- try to start the install
print_stdout(_("\nStarting install...")) print_stdout(_("\nStarting install..."))
@ -642,18 +642,17 @@ def start_install(guest, options):
try: try:
start_time = time.time() start_time = time.time()
# Do first install phase domain = installer.start_install(guest, meter=meter,
domain = guest.start_install(meter=meter,
doboot=not options.noreboot, doboot=not options.noreboot,
transient=options.transient, transient=options.transient,
autostart=options.autostart) autostart=options.autostart)
cli.connect_console(guest, domain, conscb, wait_on_console) cli.connect_console(guest, domain, conscb, wait_on_console)
check_domain(guest, domain, conscb, options.transient, check_domain(installer, domain, conscb, options.transient,
wait_on_install, wait_time, start_time) wait_on_install, wait_time, start_time)
print_stdout(_("Domain creation completed.")) print_stdout(_("Domain creation completed."))
if not options.transient and not domain.isActive(): if not options.transient and not domain.isActive():
if options.noreboot or not guest.installer.has_install_phase(): if options.noreboot or not installer.has_install_phase():
print_stdout( print_stdout(
_("You can restart your domain by running:\n %s") % _("You can restart your domain by running:\n %s") %
cli.virsh_start_cmd(guest)) cli.virsh_start_cmd(guest))
@ -669,11 +668,11 @@ def start_install(guest, options):
except Exception as e: except Exception as e:
fail(e, do_exit=False) fail(e, do_exit=False)
if domain is None: if domain is None:
guest.cleanup_created_disks(meter) installer.cleanup_created_disks(guest, meter)
cli.install_fail(guest) cli.install_fail(guest)
def check_domain(guest, domain, conscb, transient, def check_domain(installer, domain, conscb, transient,
wait_for_install, wait_time, start_time): wait_for_install, wait_time, start_time):
""" """
Make sure domain ends up in expected state, and wait if for install Make sure domain ends up in expected state, and wait if for install
@ -713,7 +712,7 @@ def check_domain(guest, domain, conscb, transient,
# used --noautoconsole # used --noautoconsole
# used --wait 0 # used --wait 0
# killed console and guest is still running # killed console and guest is still running
if not guest.installer.has_install_phase(): if not installer.has_install_phase():
return return
print_stdout( print_stdout(
@ -749,8 +748,9 @@ def check_domain(guest, domain, conscb, transient,
# XML printing helpers # # XML printing helpers #
######################## ########################
def xml_to_print(guest, xmlonly, dry): def xml_to_print(guest, installer, xmlonly, dry):
start_xml, final_xml = guest.start_install(dry=dry, return_xml=True) start_xml, final_xml = installer.start_install(
guest, dry=dry, return_xml=True)
if not start_xml: if not start_xml:
start_xml = final_xml start_xml = final_xml
final_xml = None final_xml = None
@ -963,13 +963,13 @@ def main(conn=None):
do_test_media_detection(conn, options.test_media_detection) do_test_media_detection(conn, options.test_media_detection)
return 0 return 0
guest = build_guest_instance(conn, options) guest, installer = build_guest_instance(conn, options)
if options.xmlonly or options.dry: if options.xmlonly or options.dry:
xml = xml_to_print(guest, options.xmlonly, options.dry) xml = xml_to_print(guest, installer, options.xmlonly, options.dry)
if xml: if xml:
print_stdout(xml, do_force=True) print_stdout(xml, do_force=True)
else: else:
start_install(guest, options) start_install(guest, installer, options)
return 0 return 0

View File

@ -1141,11 +1141,13 @@ class vmmCreate(vmmGObjectUI):
we should auto cleanup we should auto cleanup
""" """
if (self._failed_guest and if (self._failed_guest and
self._failed_guest.get_created_disks()): self._failed_guest.installer_instance.get_created_disks(
self._failed_guest)):
def _cleanup_disks(asyncjob): def _cleanup_disks(asyncjob):
meter = asyncjob.get_meter() meter = asyncjob.get_meter()
self._failed_guest.cleanup_created_disks(meter) self._failed_guest.installer_instance.cleanup_created_disks(
self._failed_guest, meter)
def _cleanup_disks_finished(error, details): def _cleanup_disks_finished(error, details):
if error: if error:
@ -1754,7 +1756,6 @@ class vmmCreate(vmmGObjectUI):
self._guest = self._build_guest(variant) self._guest = self._build_guest(variant)
if not self._guest: if not self._guest:
return False return False
self._guest.installer = installer
except Exception as e: except Exception as e:
return self.err.val_err( return self.err.val_err(
_("Error setting installer parameters."), e) _("Error setting installer parameters."), e)
@ -1762,12 +1763,12 @@ class vmmCreate(vmmGObjectUI):
# Validate media location # Validate media location
try: try:
if location is not None: if location is not None:
self._guest.installer.location = location installer.location = location
if cdrom: if cdrom:
self._guest.installer.cdrom = True installer.cdrom = True
if extra: if extra:
self._guest.installer.extraargs = [extra] installer.extraargs = [extra]
if init: if init:
self._guest.os.init = init self._guest.os.init = init
@ -1827,10 +1828,10 @@ class vmmCreate(vmmGObjectUI):
if not self._validate_storage_page(): if not self._validate_storage_page():
return False return False
if self._guest.installer.scratchdir_required(): if installer.scratchdir_required():
path = util.make_scratchdir(self._guest.conn, self._guest.type) path = util.make_scratchdir(self._guest.conn, self._guest.type)
elif instmethod == INSTALL_PAGE_ISO: elif instmethod == INSTALL_PAGE_ISO:
path = self._guest.installer.location path = installer.location
else: else:
path = None path = None
@ -1859,6 +1860,10 @@ class vmmCreate(vmmGObjectUI):
storage_size = int(res["storage"]) // (1024 ** 3) storage_size = int(res["storage"]) // (1024 ** 3)
self._addstorage.widget("storage-size").set_value(storage_size) self._addstorage.widget("storage-size").set_value(storage_size)
# Stash the installer in the _guest instance so we don't need
# to cache both objects individually
self._guest.installer_instance = installer
# Validation passed, store the install path (if there is one) in # Validation passed, store the install path (if there is one) in
# gsettings # gsettings
self._get_config_oscontainer_source_url(store_media=True) self._get_config_oscontainer_source_url(store_media=True)
@ -1929,7 +1934,7 @@ class vmmCreate(vmmGObjectUI):
if self._get_config_install_page() == INSTALL_PAGE_ISO: if self._get_config_install_page() == INSTALL_PAGE_ISO:
# CD/ISO install and no disks implies LiveCD # CD/ISO install and no disks implies LiveCD
self._guest.installer.livecd = not storage_enabled self._guest.installer_instance.livecd = not storage_enabled
if disk and self._addstorage.validate_disk_object(disk) is False: if disk and self._addstorage.validate_disk_object(disk) is False:
return False return False
@ -2136,7 +2141,7 @@ class vmmCreate(vmmGObjectUI):
# This encodes all the virtinst defaults up front, so the customize # This encodes all the virtinst defaults up front, so the customize
# dialog actually shows disk buses, cache values, default devices, # dialog actually shows disk buses, cache values, default devices,
# etc. Not required for straight start_install but doesn't hurt. # etc. Not required for straight start_install but doesn't hurt.
self._guest.installer.set_install_defaults(self._guest) self._guest.installer_instance.set_install_defaults(self._guest)
if not self.widget("summary-customize").get_active(): if not self.widget("summary-customize").get_active():
self._start_install(self._guest) self._start_install(self._guest)
@ -2263,7 +2268,7 @@ class vmmCreate(vmmGObjectUI):
refresh_pools.append(poolname) refresh_pools.append(poolname)
logging.debug("Starting background install process") logging.debug("Starting background install process")
guest.start_install(meter=meter) guest.installer_instance.start_install(guest, meter=meter)
logging.debug("Install completed") logging.debug("Install completed")
# Wait for VM to show up # Wait for VM to show up
@ -2289,7 +2294,7 @@ class vmmCreate(vmmGObjectUI):
# Probably means guest had no 'install' phase, as in # Probably means guest had no 'install' phase, as in
# for live cds. Try to restart the domain. # for live cds. Try to restart the domain.
vm.startup() vm.startup()
elif guest.installer.has_install_phase(): elif guest.installer_instance.has_install_phase():
# Register a status listener, which will restart the # Register a status listener, which will restart the
# guest after the install has finished # guest after the install has finished
def cb(): def cb():

View File

@ -234,8 +234,6 @@ def _import_file(conn, input_file):
# Generate the Guest # Generate the Guest
guest = conn.caps.lookup_virtinst_guest() guest = conn.caps.lookup_virtinst_guest()
guest.installer = virtinst.Installer(conn)
if not name: if not name:
name = os.path.basename(input_file) name = os.path.basename(input_file)

View File

@ -285,8 +285,6 @@ class vmx_parser(parser_class):
disk.path = None disk.path = None
guest = conn.caps.lookup_virtinst_guest() guest = conn.caps.lookup_virtinst_guest()
guest.installer = virtinst.Installer(conn)
guest.name = name.replace(" ", "_") guest.name = name.replace(" ", "_")
guest.description = desc or None guest.description = desc or None
if vcpus: if vcpus:

View File

@ -15,7 +15,6 @@ from virtcli import CLIConfig
from . import util from . import util
from .devices import * # pylint: disable=wildcard-import from .devices import * # pylint: disable=wildcard-import
from .distroinstaller import DistroInstaller
from .domain import * # pylint: disable=wildcard-import from .domain import * # pylint: disable=wildcard-import
from .domcapabilities import DomainCapabilities from .domcapabilities import DomainCapabilities
from .osdict import OSDB from .osdict import OSDB
@ -138,8 +137,6 @@ class Guest(XMLBuilder):
# This is set via Capabilities.build_virtinst_guest # This is set via Capabilities.build_virtinst_guest
self.capsinfo = None self.capsinfo = None
self.installer = DistroInstaller(self.conn)
###################### ######################
# Property accessors # # Property accessors #
@ -274,18 +271,6 @@ class Guest(XMLBuilder):
devices = XMLChildProperty(_DomainDevices, is_single=True) devices = XMLChildProperty(_DomainDevices, is_single=True)
#################################
# Install API transition compat #
#################################
def start_install(self, *args, **kwargs):
return self.installer.start_install(self, *args, **kwargs)
def get_created_disks(self):
return self.installer.get_created_disks(self)
def cleanup_created_disks(self, meter):
return self.installer.cleanup_created_disks(self, meter)
########################### ###########################
# XML convenience helpers # # XML convenience helpers #
########################### ###########################