From 00d41be5c57c40a7a4f0b8d4613d9c2063de3afb Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Tue, 16 Jul 2013 18:12:13 -0400 Subject: [PATCH] DistroInstaller: Bunch of misc cleanup --- virt-install | 4 +- virtinst/DistroInstaller.py | 285 +++++++++++++++++------------------- virtinst/Installer.py | 116 ++++++++------- 3 files changed, 200 insertions(+), 205 deletions(-) diff --git a/virt-install b/virt-install index 5573b9068..70a9fcc91 100755 --- a/virt-install +++ b/virt-install @@ -362,9 +362,7 @@ def validate_install_media(guest, location, cdpath, cdinstall=False): guest.installer.cdrom = True if location or cdpath: guest.installer.location = (location or cdpath) - - if hasattr(guest.installer, "check_location"): - guest.installer.check_location() + guest.installer.check_location() ############################# diff --git a/virtinst/DistroInstaller.py b/virtinst/DistroInstaller.py index 545f282fe..85efa73b9 100644 --- a/virtinst/DistroInstaller.py +++ b/virtinst/DistroInstaller.py @@ -154,6 +154,90 @@ def _upload_file(conn, meter, destpool, src): return vol +def _perform_initrd_injections(initrd, injections, scratchdir): + """ + Insert files into the root directory of the initial ram disk + """ + if not injections: + return + + tempdir = tempfile.mkdtemp(dir=scratchdir) + os.chmod(tempdir, 0775) + + for filename in injections: + logging.debug("Copying %s to the initrd.", filename) + shutil.copy(filename, tempdir) + + logging.debug("Appending to the initrd.") + find_proc = subprocess.Popen(['find', '.', '-print0'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + cwd=tempdir) + cpio_proc = subprocess.Popen(['cpio', '-o', '--null', '-Hnewc', '--quiet'], + stdin=find_proc.stdout, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + cwd=tempdir) + f = open(initrd, 'ab') + gzip_proc = subprocess.Popen(['gzip'], stdin=cpio_proc.stdout, + stdout=f, stderr=subprocess.PIPE) + cpio_proc.wait() + find_proc.wait() + gzip_proc.wait() + f.close() + shutil.rmtree(tempdir) + + finderr = find_proc.stderr.read() + cpioerr = cpio_proc.stderr.read() + gziperr = gzip_proc.stderr.read() + if finderr: + logging.debug("find stderr=%s", finderr) + if cpioerr: + logging.debug("cpio stderr=%s", cpioerr) + if gziperr: + logging.debug("gzip stderr=%s", gziperr) + + +def _support_remote_url_install(conn): + if hasattr(conn, "_virtinst__fake_conn"): + return False + return conn.check_stream_support(conn.SUPPORT_STREAM_UPLOAD) + + +def _upload_media(conn, scratchdir, system_scratchdir, + meter, kernel, initrd): + """ + Upload kernel/initrd media to remote connection if necessary + """ + tmpvols = [] + + if (not conn.is_remote() and + (conn.is_session_uri() or scratchdir == system_scratchdir)): + # We have access to system scratchdir, don't jump through hoops + logging.debug("Have access to preferred scratchdir so" + " nothing to upload") + return kernel, initrd, tmpvols + + if not _support_remote_url_install(conn): + logging.debug("Media upload not supported") + return kernel, initrd, tmpvols + + # Build pool + logging.debug("Uploading kernel/initrd media") + pool = _build_pool(conn, meter, system_scratchdir) + + kvol = _upload_file(conn, meter, pool, kernel) + newkernel = kvol.path() + tmpvols.append(kvol) + + ivol = _upload_file(conn, meter, pool, initrd) + newinitrd = ivol.path() + tmpvols.append(ivol) + + return newkernel, newinitrd, tmpvols + + + class DistroInstaller(Installer.Installer): def __init__(self, *args, **kwargs): Installer.Installer.__init__(self, *args, **kwargs) @@ -162,64 +246,10 @@ class DistroInstaller(Installer.Installer): self._location_is_path = True - def get_location(self): - return self._location - def set_location(self, val): - """ - Valid values for location: + ####################### + # Install prepartions # + ####################### - 1) it can be a local file (ex. boot.iso), directory (ex. distro - tree) or physical device (ex. cdrom media) - - 2) tuple of the form (poolname, volname) pointing to a file or - device which will set location as that path - - 3) http, ftp, or nfs path for an install tree - """ - validated = True - self._location_is_path = True - is_local = not self.conn.is_remote() - - if _is_url(val, is_local): - val = _sanitize_url(val) - self._location_is_path = False - logging.debug("DistroInstaller location is a network source.") - - elif os.path.exists(os.path.abspath(val)) and is_local: - val = os.path.abspath(val) - logging.debug("DistroInstaller location is a local " - "file/path: %s", val) - - else: - # Didn't determine anything about the location - validated = False - - if (self._location_is_path or - (not validated and - self.conn.check_conn_support(self.conn.SUPPORT_CONN_STORAGE))): - # If user passed a storage tuple, OR - # We couldn't determine the location type and a storage capable - # connection was passed: - # Pass the parameters off to VirtualDisk to validate, and pull - # out the path - try: - d = self._make_cdrom_dev(val) - val = d.path - except: - logging.debug("Error validating install location", - exc_info=True) - raise ValueError(_("Checking installer location failed: " - "Could not find media '%s'." % str(val))) - elif not validated: - raise ValueError(_("Install media location must be an NFS, HTTP " - "or FTP network install source, or an existing " - "file/device")) - - self._location = val - location = property(get_location, set_location) - - - # Private helper methods def _prepare_cdrom(self, guest, meter): transient = not self.livecd if not self._location_is_path: @@ -240,81 +270,6 @@ class DistroInstaller(Installer.Installer): disk.transient = transient self.install_devices.append(disk) - def _perform_initrd_injections(self, initrd): - """ - Insert files into the root directory of the initial ram disk - """ - tempdir = tempfile.mkdtemp(dir=self.scratchdir) - os.chmod(tempdir, 0775) - - for filename in self._initrd_injections: - logging.debug("Copying %s to the initrd.", filename) - shutil.copy(filename, tempdir) - - logging.debug("Appending to the initrd.") - find_proc = subprocess.Popen(['find', '.', '-print0'], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - cwd=tempdir) - cpio_proc = subprocess.Popen(['cpio', '-o', '--null', '-Hnewc', '--quiet'], - stdin=find_proc.stdout, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - cwd=tempdir) - f = open(initrd, 'ab') - gzip_proc = subprocess.Popen(['gzip'], stdin=cpio_proc.stdout, - stdout=f, stderr=subprocess.PIPE) - cpio_proc.wait() - find_proc.wait() - gzip_proc.wait() - f.close() - shutil.rmtree(tempdir) - - finderr = find_proc.stderr.read() - cpioerr = cpio_proc.stderr.read() - gziperr = gzip_proc.stderr.read() - if finderr: - logging.debug("find stderr=%s", finderr) - if cpioerr: - logging.debug("cpio stderr=%s", cpioerr) - if gziperr: - logging.debug("gzip stderr=%s", gziperr) - - def support_remote_url_install(self): - if hasattr(self.conn, "_virtinst__fake_conn"): - return False - return self.conn.check_stream_support(self.conn.SUPPORT_STREAM_UPLOAD) - - def _upload_media(self, guest, meter, kernel, initrd): - conn = guest.conn - system_scratchdir = self._get_system_scratchdir() - - if (not self.conn.is_remote() and - (self.conn.is_session_uri() or - self.scratchdir == system_scratchdir)): - # We have access to system scratchdir, don't jump through hoops - logging.debug("Have access to preferred scratchdir so" - " nothing to upload") - return kernel, initrd - - if not self.support_remote_url_install(): - logging.debug("Media upload not supported") - return kernel, initrd - - # Build pool - logging.debug("Uploading kernel/initrd media") - pool = _build_pool(conn, meter, system_scratchdir) - - kvol = _upload_file(conn, meter, pool, kernel) - newkernel = kvol.path() - self._tmpvols.append(kvol) - - ivol = _upload_file(conn, meter, pool, initrd) - newinitrd = ivol.path() - self._tmpvols.append(ivol) - - return newkernel, newinitrd - def _prepare_kernel_and_initrd(self, guest, meter): disk = None @@ -351,12 +306,14 @@ class DistroInstaller(Installer.Installer): if initrdfn: self._tmpfiles.append(initrdfn) - if self._initrd_injections: - self._perform_initrd_injections(initrdfn) + _perform_initrd_injections(initrdfn, + self._initrd_injections, + self.scratchdir) - # If required, upload media to an accessible guest location - kernelfn, initrdfn = self._upload_media(guest, meter, - kernelfn, initrdfn) + kernelfn, initrdfn, tmpvols = _upload_media( + guest.conn, self.scratchdir, self._get_system_scratchdir(), + meter, kernelfn, initrdfn) + self._tmpvols += tmpvols self._install_bootconfig.kernel = kernelfn self._install_bootconfig.initrd = initrdfn @@ -364,17 +321,53 @@ class DistroInstaller(Installer.Installer): return disk - def _persistent_cd(self): - return (self._location_is_path and self.cdrom and self.livecd) + + ########################### + # Private installer impls # + ########################### def _get_bootdev(self, isinstall, guest): - if isinstall or self._persistent_cd(): + persistent_cd = (self._location_is_path and + self.cdrom and + self.livecd) + + if isinstall or persistent_cd: bootdev = self.bootconfig.BOOT_DEVICE_CDROM else: bootdev = self.bootconfig.BOOT_DEVICE_HARDDISK return bootdev - # General Installer methods + def _validate_location(self, val): + """ + Valid values for location: + + 1) it can be a local file (ex. boot.iso), directory (ex. distro + tree) or physical device (ex. cdrom media) + + 2) http, ftp, or nfs path for an install tree + """ + is_local = not self.conn.is_remote() + if _is_url(val, is_local): + self._location_is_path = False + self._location = _sanitize_url(val) + logging.debug("DistroInstaller location is a network source.") + return val + + try: + d = self._make_cdrom_dev(val) + val = d.path + except: + logging.debug("Error validating install location", exc_info=True) + raise ValueError(_("Checking installer location failed: " + "Could not find media '%s'." % str(val))) + + self._location_is_path = True + return val + + + ########################## + # Public installer impls # + ########################## def scratchdir_required(self): if not self.location: @@ -385,9 +378,7 @@ class DistroInstaller(Installer.Installer): return bool(is_url or mount_dvd) - def prepare(self, guest, meter): - self.cleanup() - + def _prepare(self, guest, meter): dev = None if self.cdrom: if self.location: @@ -405,9 +396,9 @@ class DistroInstaller(Installer.Installer): if self._location_is_path: # We already mostly validated this return True - else: - # This will throw an error for us - OSDistro.detectMediaDistro(location=self.location, arch=self.arch) + + # This will throw an error for us + OSDistro.detectMediaDistro(location=self.location, arch=self.arch) return True def detect_distro(self): diff --git a/virtinst/Installer.py b/virtinst/Installer.py index b66eef0ea..b3037fef3 100644 --- a/virtinst/Installer.py +++ b/virtinst/Installer.py @@ -91,6 +91,10 @@ class Installer(XMLBuilder): self._os_type = "xen" + ##################### + # XML related props # + ##################### + def _get_bootconfig(self): return self._bootconfig bootconfig = property(_get_bootconfig) @@ -147,6 +151,11 @@ class Installer(XMLBuilder): init = XMLProperty(_get_init, _set_init, xpath="./os/init") + + ###################### + # Non-XML properties # + ###################### + def get_scratchdir(self): if not self.scratchdir_required(): return None @@ -168,7 +177,7 @@ class Installer(XMLBuilder): def get_location(self): return self._location def set_location(self, val): - self._location = val + self._location = self._validate_location(val) location = property(get_location, set_location) def get_initrd_injections(self): @@ -177,7 +186,6 @@ class Installer(XMLBuilder): self._initrd_injections = val initrd_injections = property(get_initrd_injections, set_initrd_injections) - # extra arguments to pass to the guest installer def get_extra_args(self): return self._install_bootconfig.kernel_args def set_extra_args(self, val): @@ -185,23 +193,10 @@ class Installer(XMLBuilder): extraargs = property(get_extra_args, set_extra_args) - # Public helper methods - def scratchdir_required(self): - """ - Returns true if scratchdir is needed for the passed install parameters. - Apps can use this to determine if they should attempt to ensure - scratchdir permissions are adequate - """ - return False + ################### + # Private helpers # + ################### - def is_hvm(self): - return self.os_type == "hvm" - def is_xenpv(self): - return self.os_type in ["xen", "linux"] - def is_container(self): - return self.os_type == "exe" - - # Private methods def _get_system_scratchdir(self): if platform.system() == "SunOS": return "/var/tmp" @@ -227,9 +222,6 @@ class Installer(XMLBuilder): return scratch - def _get_bootdev(self, isinstall, guest): - raise NotImplementedError("Must be implemented in subclass") - def _build_boot_order(self, isinstall, guest): bootorder = [self._get_bootdev(isinstall, guest)] @@ -311,9 +303,6 @@ class Installer(XMLBuilder): return osblob - - # Method definitions - def _get_xml_config(self, guest, isinstall): """ Generate the portion of the guest xml that determines boot devices @@ -343,6 +332,41 @@ class Installer(XMLBuilder): return self._get_osblob_helper(guest, isinstall, bootconfig) + + ########################## + # Internal API overrides # + ########################## + + def _get_bootdev(self, isinstall, guest): + raise NotImplementedError("Must be implemented in subclass") + + def _validate_location(self, val): + return val + + def _prepare(self, guest, meter): + ignore = guest + ignore = meter + + + ############## + # Public API # + ############## + + def scratchdir_required(self): + """ + Returns true if scratchdir is needed for the passed install parameters. + Apps can use this to determine if they should attempt to ensure + scratchdir permissions are adequate + """ + return False + + def is_hvm(self): + return self.os_type == "hvm" + def is_xenpv(self): + return self.os_type in ["xen", "linux"] + def is_container(self): + return self.os_type == "exe" + def has_install_phase(self): """ Return True if the requested setup is actually installing an OS @@ -368,14 +392,15 @@ class Installer(XMLBuilder): self.install_devices = [] def prepare(self, guest, meter): + self.cleanup() + self._prepare(guest, meter) + + def check_location(self): """ - Fetch any files needed for installation. - @param guest: guest instance being installed - @type guest: L{Guest} - @param meter: progress meter - @type meter: Urlgrabber ProgressMeter + Validate self.location seems to work. This will might hit the + network so we don't want to do it on demand. """ - pass + return True def detect_distro(self): """ @@ -439,33 +464,14 @@ class PXEInstaller(Installer): class LiveCDInstaller(Installer): _has_install_phase = False + cdrom = True def _validate_location(self, val): - if not val: - return None - return self._make_cdrom_dev(val) - def _get_location(self): - return self._location - def _set_location(self, val): - self._validate_location(val) - self._location = val - self.cdrom = True - location = property(_get_location, _set_location) - - - # General Installer methods - def prepare(self, guest, meter): - self.cleanup() - - disk = self._validate_location(self.location) - - if not disk: - raise ValueError(_("CDROM media must be specified for the live " - "CD installer.")) - - self.install_devices.append(disk) - - # Internal methods + return self._make_cdrom_dev(val).path + def _prepare(self, guest, meter): + ignore = guest + ignore = meter + self.install_devices.append(self._make_cdrom_dev(self.location)) def _get_bootdev(self, isinstall, guest): return self.bootconfig.BOOT_DEVICE_CDROM