From 7a63bcc874ecfee5e7664906ba27baff7ff7435a Mon Sep 17 00:00:00 2001 From: Vangelis Koukis Date: Sat, 28 Mar 2015 12:42:59 +0200 Subject: [PATCH 1/7] Change "entrance point" to "entry point" --- image_creator/dialog_main.py | 4 ++-- image_creator/main.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/image_creator/dialog_main.py b/image_creator/dialog_main.py index 84204ed..0390987 100644 --- a/image_creator/dialog_main.py +++ b/image_creator/dialog_main.py @@ -16,7 +16,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""This module is the entrance point for the dialog-based version of the +"""This module is the entry point for the dialog-based version of the snf-image-creator program. The main function will create a dialog where the user is asked if he wants to use the program in expert or wizard mode. """ @@ -244,7 +244,7 @@ def dialog_main(media, **kwargs): def main(): - """Entrance Point""" + """Entry Point""" if os.geteuid() != 0: sys.stderr.write("Error: You must run %s as root\n" % PROGNAME) sys.exit(2) diff --git a/image_creator/main.py b/image_creator/main.py index 030c2d8..1c91b26 100644 --- a/image_creator/main.py +++ b/image_creator/main.py @@ -16,7 +16,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""This module is the entrance point for the non-interactive version of the +"""This module is the entry point for the non-interactive version of the snf-image-creator program. """ From 630074306fd66431dc70f7ce2bca2312d5cbabaf Mon Sep 17 00:00:00 2001 From: Vangelis Koukis Date: Sat, 28 Mar 2015 12:50:05 +0200 Subject: [PATCH 2/7] Factor out checking for root privileges Factor out checking for root privileges into a separate utility function. Also ensure "snf-image-creator --help" runs successfully without root privileges. --- image_creator/dialog_main.py | 10 ++++------ image_creator/main.py | 8 ++++---- image_creator/util.py | 7 +++++++ 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/image_creator/dialog_main.py b/image_creator/dialog_main.py index 0390987..846a1c1 100644 --- a/image_creator/dialog_main.py +++ b/image_creator/dialog_main.py @@ -32,7 +32,7 @@ import tempfile from image_creator import __version__ as version -from image_creator.util import FatalError +from image_creator.util import FatalError, ensure_root from image_creator.output.cli import SimpleOutput from image_creator.output.dialog import GaugeOutput from image_creator.output.composite import CompositeOutput @@ -245,10 +245,6 @@ def dialog_main(media, **kwargs): def main(): """Entry Point""" - if os.geteuid() != 0: - sys.stderr.write("Error: You must run %s as root\n" % PROGNAME) - sys.exit(2) - usage = "Usage: %prog [options] []" parser = optparse.OptionParser(version=version, usage=usage) parser.add_option("-l", "--logfile", type="string", dest="logfile", @@ -266,8 +262,10 @@ def main(): opts, args = parser.parse_args(sys.argv[1:]) + ensure_root(PROGNAME) + if len(args) > 1: - parser.error("Wrong number of arguments") + parser.error("Too many arguments. Please see output of `--help'.") media = args[0] if len(args) == 1 else None diff --git a/image_creator/main.py b/image_creator/main.py index 1c91b26..b740b71 100644 --- a/image_creator/main.py +++ b/image_creator/main.py @@ -22,7 +22,7 @@ from image_creator import __version__ as version from image_creator.disk import Disk -from image_creator.util import FatalError +from image_creator.util import FatalError, ensure_root from image_creator.output.cli import SilentOutput, SimpleOutput, \ OutputWthProgress from image_creator.output.composite import CompositeOutput @@ -39,6 +39,8 @@ import subprocess import time +PROGNAME = os.path.basename(sys.argv[0]) + def check_writable_dir(option, opt_str, value, parser): """Check if a directory is writable""" @@ -222,9 +224,7 @@ def parse_options(input_args): def image_creator(options, out): """snf-mkimage main function""" - if os.geteuid() != 0: - raise FatalError("You must run %s as root" - % os.path.basename(sys.argv[0])) + ensure_root(PROGNAME) # Check if the authentication info is valid. The earlier the better if options.token is not None and options.url is not None: diff --git a/image_creator/util.py b/image_creator/util.py index 3229994..4d100d9 100644 --- a/image_creator/util.py +++ b/image_creator/util.py @@ -23,6 +23,7 @@ import time import os import re +import sys import json import tempfile @@ -133,6 +134,12 @@ def virtio_versions(virtio_state): return ret +def ensure_root(progname): + if os.geteuid() != 0: + sys.stderr.write("Error: %s requires root privileges\n" % progname) + sys.exit(2) + + class QemuNBD(object): """Wrapper class for the qemu-nbd tool""" From 41a054cfc5a7c2180ef03352e81592a5c6dd5825 Mon Sep 17 00:00:00 2001 From: Vangelis Koukis Date: Sat, 28 Mar 2015 13:52:24 +0200 Subject: [PATCH 3/7] Port dialog_main.py to argparse, improve help text Port dialog_main.py to use argparse instead of optparse. Add explicit positional argument, improve help text for every argument, add description and epilog blocks to output of --help. --- image_creator/dialog_main.py | 71 +++++++++++++++++++++--------------- setup.py | 3 +- 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/image_creator/dialog_main.py b/image_creator/dialog_main.py index 846a1c1..e560e77 100644 --- a/image_creator/dialog_main.py +++ b/image_creator/dialog_main.py @@ -25,7 +25,7 @@ import sys import os import signal -import optparse +import argparse import types import termios import traceback @@ -245,46 +245,59 @@ def dialog_main(media, **kwargs): def main(): """Entry Point""" - usage = "Usage: %prog [options] []" - parser = optparse.OptionParser(version=version, usage=usage) - parser.add_option("-l", "--logfile", type="string", dest="logfile", - default=None, help="log all messages to FILE", - metavar="FILE") - parser.add_option("--no-snapshot", dest="snapshot", default=True, - help="don't snapshot the input media. (THIS IS " - "DANGEROUS AS IT WILL ALTER THE ORIGINAL MEDIA!!!)", - action="store_false") - parser.add_option("--syslog", dest="syslog", default=False, - help="log to syslog", action="store_true") - parser.add_option("--tmpdir", type="string", dest="tmp", default=None, - help="create large temporary image files under DIR", - metavar="DIR") - - opts, args = parser.parse_args(sys.argv[1:]) + d = ("Create a cloud Image from the specified INPUT_MEDIA." + " INPUT_MEDIA must be the hard disk of an existing OS deployment" + " to be used as the template for Image creation. Supported formats" + " include raw block devices, all disk image file formats supported" + " by QEMU (e.g., QCOW2, VMDK, VDI, VHD), or the filesystem of the" + " host itself. The resulting Image is meant to be used with Synnefo" + " and other IaaS cloud platforms. Note this program works on a" + " snapshot of INPUT_MEDIA, and will not modify its contents.") + e = ("%(prog)s requires root privileges.") + + parser = argparse.ArgumentParser(version=version, description=d, epilog=e) + + parser.add_argument("-l", "--logfile", metavar="FILE", type=str, + dest="logfile", default=None, + help="Log all messages to FILE") + parser.add_argument("--syslog", dest="syslog", default=False, + help="Also log to syslog", action="store_true") + parser.add_argument("--tmpdir", metavar="DIR", type=str, dest="tmpdir", + default=None, + help="Create large temporary image files under DIR") + parser.add_argument("--no-snapshot", dest="snapshot", default=True, + action="store_false", + help=("Do not work on a snapshot, but modify the input" + " media directly instead. DO NOT USE THIS OPTION" + " UNLESS YOU REALLY KNOW WHAT YOU ARE DOING." + " THIS WILL ALTER THE ORIGINAL MEDIA!")) + parser.add_argument(metavar="INPUT_MEDIA", + nargs='?', dest="media", type=str, default=None, + help=("Use INPUT_MEDIA as the template for" + " Image creation, e.g., /dev/sdc, /disk0.vmdk." + " Specify a single slash character (/) to bundle" + " the filesystem of the host itself.")) + + args = parser.parse_args() ensure_root(PROGNAME) - if len(args) > 1: - parser.error("Too many arguments. Please see output of `--help'.") - - media = args[0] if len(args) == 1 else None - - if opts.tmp is not None and not os.path.isdir(opts.tmp): - parser.error("Directory: `%s' specified with --tmpdir is not valid" - % opts.tmp) + if args.tmpdir is not None and not os.path.isdir(args.tmpdir): + parser.error("Argument `%s' to --tmpdir must be a directory" + % args.tmpdir) try: - logfile = open(opts.logfile, 'w') if opts.logfile is not None else None + logfile = open(args.logfile, 'w') if args.logfile is not None else None except IOError as error: parser.error("Unable to open logfile `%s' for writing. Reason: %s" % - (opts.logfile, error.strerror)) + (args.logfile, error.strerror)) try: # Save the terminal attributes attr = termios.tcgetattr(sys.stdin.fileno()) try: - ret = dialog_main(media, logfile=logfile, tmpdir=opts.tmp, - snapshot=opts.snapshot, syslog=opts.syslog) + ret = dialog_main(args.media, logfile=logfile, tmpdir=args.tmpdir, + snapshot=args.snapshot, syslog=args.syslog) finally: # Restore the terminal attributes. If an error occurs make sure # that the terminal turns back to normal. diff --git a/setup.py b/setup.py index 9bba333..64c667f 100755 --- a/setup.py +++ b/setup.py @@ -34,7 +34,8 @@ license='GNU GPLv3', packages=find_packages(), include_package_data=True, - install_requires=['sh', 'ansicolors', 'progress>=1.0.2', 'kamaki>=0.9'], + install_requires=['sh', 'ansicolors', 'progress>=1.0.2', 'kamaki>=0.9', + 'argparse'], # Unresolvable dependencies: # pysendfile|py-sendfile, hivex, guestfs, parted, rsync, entry_points={ From d412b4dbf7ec02d54912f99cab15a050d72ff2da Mon Sep 17 00:00:00 2001 From: Vangelis Koukis Date: Sat, 28 Mar 2015 14:20:34 +0200 Subject: [PATCH 4/7] Replace all instances of `media' with `medium' Media is the plural of medium. This program only supports a single medium at a time, a raw hard disk, or a disk image file, so `medium' is appropriate. --- image_creator/dialog_main.py | 44 +++++++++++------------ image_creator/dialog_menu.py | 4 +-- image_creator/dialog_wizard.py | 2 +- image_creator/disk.py | 26 +++++++------- image_creator/image.py | 6 ++-- image_creator/main.py | 36 +++++++++---------- image_creator/os_type/__init__.py | 10 +++--- image_creator/os_type/freebsd.py | 2 +- image_creator/os_type/linux.py | 6 ++-- image_creator/os_type/slackware.py | 2 +- image_creator/os_type/unsupported.py | 4 +-- image_creator/os_type/windows/__init__.py | 28 +++++++-------- 12 files changed, 85 insertions(+), 85 deletions(-) diff --git a/image_creator/dialog_main.py b/image_creator/dialog_main.py index e560e77..ae921a8 100644 --- a/image_creator/dialog_main.py +++ b/image_creator/dialog_main.py @@ -46,13 +46,13 @@ PROGNAME = os.path.basename(sys.argv[0]) -def create_image(d, media, out, tmp, snapshot): - """Create an image out of `media'""" +def create_image(d, medium, out, tmp, snapshot): + """Create an image out of `medium'""" d.setBackgroundTitle('snf-image-creator') gauge = GaugeOutput(d, "Initialization", "Initializing...") out.append(gauge) - disk = Disk(media, out, tmp) + disk = Disk(medium, out, tmp) def signal_handler(signum, frame): gauge.cleanup() @@ -83,7 +83,7 @@ def dummy(self): session['excluded_tasks'] = [-1] session['task_metadata'] = ["EXCLUDE_ALL_TASKS"] - msg = "The system on the input media is not supported." \ + msg = "The system on the input medium is not supported." \ "\n\nReason: %s\n\n" \ "We highly recommend not to create an image out of this, " \ "since the image won't be cleaned up and you will not be " \ @@ -97,7 +97,7 @@ def dummy(self): d.infobox("Thank you for using snf-image-creator. Bye", width=53) return 0 - msg = "snf-image-creator detected a %s system on the input media. " \ + msg = "snf-image-creator detected a %s system on the input medium. " \ "Would you like to run a wizard to assist you through the " \ "image creation process?\n\nChoose to run the wizard," \ " to run snf-image-creator in expert mode or press " \ @@ -161,7 +161,7 @@ def _dialog_form(self, text, height=20, width=60, form_height=15, fields=[], return (code, output.splitlines()) -def dialog_main(media, **kwargs): +def dialog_main(medium, **kwargs): """Main function for the dialog-based version of the program""" tmpdir = kwargs['tmpdir'] if 'tmpdir' in kwargs else None @@ -197,13 +197,13 @@ def dialog_main(media, **kwargs): d.setBackgroundTitle('snf-image-creator') - # Pick input media + # Pick input medium while True: - media = select_file(d, init=media, ftype="br", bundle_host=True, - title="Please select an input media.") - if media is None: + medium = select_file(d, init=medium, ftype="br", bundle_host=True, + title="Please select an input medium.") + if medium is None: if confirm_exit( - d, "You canceled the media selection dialog box."): + d, "You canceled the medium selection dialog box."): return 0 continue break @@ -222,7 +222,7 @@ def dialog_main(media, **kwargs): try: out = CompositeOutput(logs) out.info("Starting %s v%s ..." % (PROGNAME, version)) - ret = create_image(d, media, out, tmpdir, snapshot) + ret = create_image(d, medium, out, tmpdir, snapshot) break except Reset: for log in logs: @@ -245,14 +245,14 @@ def dialog_main(media, **kwargs): def main(): """Entry Point""" - d = ("Create a cloud Image from the specified INPUT_MEDIA." - " INPUT_MEDIA must be the hard disk of an existing OS deployment" + d = ("Create a cloud Image from the specified INPUT_MEDIUM." + " INPUT_MEDIUM must be the hard disk of an existing OS deployment" " to be used as the template for Image creation. Supported formats" " include raw block devices, all disk image file formats supported" " by QEMU (e.g., QCOW2, VMDK, VDI, VHD), or the filesystem of the" " host itself. The resulting Image is meant to be used with Synnefo" " and other IaaS cloud platforms. Note this program works on a" - " snapshot of INPUT_MEDIA, and will not modify its contents.") + " snapshot of INPUT_MEDIUM, and will not modify its contents.") e = ("%(prog)s requires root privileges.") parser = argparse.ArgumentParser(version=version, description=d, epilog=e) @@ -268,12 +268,12 @@ def main(): parser.add_argument("--no-snapshot", dest="snapshot", default=True, action="store_false", help=("Do not work on a snapshot, but modify the input" - " media directly instead. DO NOT USE THIS OPTION" - " UNLESS YOU REALLY KNOW WHAT YOU ARE DOING." - " THIS WILL ALTER THE ORIGINAL MEDIA!")) - parser.add_argument(metavar="INPUT_MEDIA", - nargs='?', dest="media", type=str, default=None, - help=("Use INPUT_MEDIA as the template for" + " medium directly instead. DO NOT USE THIS" + " OPTION UNLESS YOU REALLY KNOW WHAT YOU ARE" + " DOING. THIS WILL ALTER THE ORIGINAL MEDIUM!")) + parser.add_argument(metavar="INPUT_MEDIUM", + nargs='?', dest="medium", type=str, default=None, + help=("Use INPUT_MEDIUM as the template for" " Image creation, e.g., /dev/sdc, /disk0.vmdk." " Specify a single slash character (/) to bundle" " the filesystem of the host itself.")) @@ -296,7 +296,7 @@ def main(): # Save the terminal attributes attr = termios.tcgetattr(sys.stdin.fileno()) try: - ret = dialog_main(args.media, logfile=logfile, tmpdir=args.tmpdir, + ret = dialog_main(args.medium, logfile=logfile, tmpdir=args.tmpdir, snapshot=args.snapshot, syslog=args.syslog) finally: # Restore the terminal attributes. If an error occurs make sure diff --git a/image_creator/dialog_menu.py b/image_creator/dialog_menu.py index 4429198..4980757 100644 --- a/image_creator/dialog_menu.py +++ b/image_creator/dialog_menu.py @@ -742,7 +742,7 @@ def sysprep_params(session): def virtio(session): - """Display the state of the VirtIO drivers in the media""" + """Display the state of the VirtIO drivers in the medium""" d = session['dialog'] image = session['image'] @@ -758,7 +758,7 @@ def virtio(session): (code, choice) = d.menu( "In this menu you can see details about the installed VirtIO " - "drivers on the input media. Press to see more information " + "drivers on the input medium. Press to see more info " "about a specific installed driver or to install one or " "more new drivers.", height=16, width=WIDTH, choices=choices, ok_label="Info", menu_height=len(choices), cancel="Back", diff --git a/image_creator/dialog_wizard.py b/image_creator/dialog_wizard.py index 62ba2b1..d1023e8 100644 --- a/image_creator/dialog_wizard.py +++ b/image_creator/dialog_wizard.py @@ -376,7 +376,7 @@ def validate_virtio(_): dialog = session['dialog'] title = "VirtIO driver missing" msg = "Image creation cannot proceed unless a VirtIO %s driver " \ - "is installed on the media!" + "is installed on the medium." if not (viostor or new_viostor): dialog.msgbox(msg % "Block Device", width=PAGE_WIDTH, height=PAGE_HEIGHT, title=title) diff --git a/image_creator/disk.py b/image_creator/disk.py index 7cae1d3..e3f8977 100644 --- a/image_creator/disk.py +++ b/image_creator/disk.py @@ -59,14 +59,14 @@ def get_tmp_dir(default=None): class Disk(object): """This class represents a hard disk hosting an Operating System - A Disk instance never alters the source media it is created from. + A Disk instance never alters the source medium it is created from. Any change is done on a snapshot created by the device-mapper of the Linux kernel. """ def __init__(self, source, output, tmp=None): - """Create a new Disk instance out of a source media. The source - media can be an image file, a block device or a directory. + """Create a new Disk instance out of a source medium. The source + medium can be an image file, a block device or a directory. """ self._cleanup_jobs = [] self._images = [] @@ -106,7 +106,7 @@ def check_unlink(path): self._add_cleanup(check_unlink, image) bundle.create_image(image) return image - raise FatalError("Using a directory as media source is supported") + raise FatalError("Using a directory as medium source is supported") def cleanup(self): """Cleanup internal data. This needs to be called before the @@ -125,12 +125,12 @@ def cleanup(self): @property def file(self): - """Convert the source media into a file.""" + """Convert the source medium into a file.""" if self._file is not None: return self._file - self.out.info("Examining source media `%s' ..." % self.source, False) + self.out.info("Examining source medium `%s' ..." % self.source, False) mode = os.stat(self.source).st_mode if stat.S_ISDIR(mode): self.out.success('looks like a directory') @@ -139,7 +139,7 @@ def file(self): self.out.success('looks like an image file') self._file = self.source elif not stat.S_ISBLK(mode): - raise FatalError("Invalid media source. Only block devices, " + raise FatalError("Invalid medium source. Only block devices, " "regular files and directories are supported.") else: self.out.success('looks like a block device') @@ -148,7 +148,7 @@ def file(self): return self._file def snapshot(self): - """Creates a snapshot of the original source media of the Disk + """Creates a snapshot of the original source medium of the Disk instance. """ @@ -156,10 +156,10 @@ def snapshot(self): self.out.warn("Snapshotting ignored for host bundling mode.") return self.file - # Examine media file + # Examine medium file info = image_info(self.file) - self.out.info("Snapshotting media source ...", False) + self.out.info("Snapshotting medium source ...", False) # Create a qcow2 snapshot for image files that are not raw if info['format'] != 'raw': @@ -196,10 +196,10 @@ def snapshot(self): self.out.success('done') return "/dev/mapper/%s" % snapshot - def get_image(self, media, **kwargs): + def get_image(self, medium, **kwargs): """Returns a newly created Image instance.""" - info = image_info(media) - image = Image(media, self.out, format=info['format'], **kwargs) + info = image_info(medium) + image = Image(medium, self.out, format=info['format'], **kwargs) self._images.append(image) image.enable() return image diff --git a/image_creator/image.py b/image_creator/image.py index a57ce19..1f72b20 100644 --- a/image_creator/image.py +++ b/image_creator/image.py @@ -100,9 +100,9 @@ def enable(self): self.size = self.g.blockdev_getsize64(self.guestfs_device) if len(roots) > 1: - reason = "Multiple operating systems found on the media." + reason = "Multiple operating systems found on the medium." else: - reason = "Unable to detect any operating system on the media." + reason = "Unable to detect any operating system on the medium." self.set_unsupported(reason) return @@ -126,7 +126,7 @@ def set_unsupported(self, reason): self._unsupported = reason self.meta['UNSUPPORTED'] = reason - self.out.warn('Media is not supported. Reason: %s' % reason) + self.out.warn('Medium is not supported. Reason: %s' % reason) def is_unsupported(self): """Returns if this image is unsupported""" diff --git a/image_creator/main.py b/image_creator/main.py index b740b71..4174b8e 100644 --- a/image_creator/main.py +++ b/image_creator/main.py @@ -57,7 +57,7 @@ def check_writable_dir(option, opt_str, value, parser): def parse_options(input_args): """Parse input parameters""" - usage = "Usage: %prog [options] " + usage = "Usage: %prog [options] " parser = optparse.OptionParser(version=version, usage=usage) parser.add_option("-a", "--authentication-url", dest="url", type="string", @@ -65,7 +65,7 @@ def parse_options(input_args): "uploading/registering images") parser.add_option("--allow-unsupported", dest="allow_unsupported", - help="proceed with the image creation even if the media " + help="proceed with image creation even if the medium " "is not supported", default=False, action="store_true") parser.add_option("-c", "--cloud", dest="cloud", type="string", @@ -75,11 +75,11 @@ def parse_options(input_args): parser.add_option("--disable-sysprep", dest="disabled_syspreps", help="prevent SYSPREP operation from running on the " - "input media", default=[], action="append", + "input medium", default=[], action="append", metavar="SYSPREP") parser.add_option("--enable-sysprep", dest="enabled_syspreps", default=[], - help="run SYSPREP operation on the input media", + help="run SYSPREP operation on the input medium", action="append", metavar="SYSPREP") parser.add_option("-f", "--force", dest="force", default=False, @@ -87,8 +87,8 @@ def parse_options(input_args): help="overwrite output files if they exist") parser.add_option("--host-run", dest="host_run", default=[], - help="mount the media in the host and run a script " - "against the guest media. This option may be defined " + help="mount the medium in the host and run a script " + "against the guest medium. This option may be defined " "multiple times. The script's working directory will be " "the guest's root directory. BE CAREFUL! DO NOT USE " "ABSOLUTE PATHS INSIDE THE SCRIPT! YOU MAY HARM YOUR " @@ -103,8 +103,8 @@ def parse_options(input_args): action="append", metavar="KEY=VALUE") parser.add_option("--no-snapshot", dest="snapshot", default=True, - help="don't snapshot the input media. (THIS IS " - "DANGEROUS AS IT WILL ALTER THE ORIGINAL MEDIA!!!)", + help="don't snapshot the input medium. (THIS IS " + "DANGEROUS AS IT WILL ALTER THE ORIGINAL MEDIUM!!!)", action="store_false") parser.add_option("--no-sysprep", dest="sysprep", default=True, @@ -121,12 +121,12 @@ def parse_options(input_args): parser.add_option("--print-syspreps", dest="print_syspreps", default=False, help="print the enabled and disabled system preparation " - "operations for this input media", action="store_true") + "operations for this input medium", action="store_true") parser.add_option("--print-sysprep-params", dest="print_sysprep_params", default=False, action="store_true", help="print the defined system preparation parameters " - "for this input media") + "for this input medium") parser.add_option("--public", dest="public", default=False, help="register image with the cloud as public", @@ -165,7 +165,7 @@ def parse_options(input_args): options.source = args[0] if not os.path.exists(options.source): - parser.error("Input media `%s' is not accessible" % options.source) + parser.error("Input medium `%s' is not accessible" % options.source) if options.register and not options.upload: parser.error("You also need to set -u when -r option is set") @@ -275,15 +275,15 @@ def signal_handler(signum, frame): signal.signal(signal.SIGINT, signal_handler) signal.signal(signal.SIGTERM, signal_handler) try: - # There is no need to snapshot the media if it was created by the Disk + # There is no need to snapshot the medium if it was created by the Disk # instance as a temporary object. device = disk.file if not options.snapshot else disk.snapshot() image = disk.get_image(device, sysprep_params=options.sysprep_params) if image.is_unsupported() and not options.allow_unsupported: raise FatalError( - "The media seems to be unsupported.\n\n" + - textwrap.fill("To create an image from an unsupported media, " + "The medium seems to be unsupported.\n\n" + + textwrap.fill("To create an image from an unsupported medium, " "you'll need to use the`--allow-unsupported' " "command line option. Using this is highly " "discouraged, since the resulting image will " @@ -291,7 +291,7 @@ def signal_handler(signum, frame): "not get customized during the deployment.")) if len(options.host_run) != 0 and not image.mount_local_support: - raise FatalError("Running scripts against the guest media is not " + raise FatalError("Running scripts against the guest medium is not " "supported for this build of libguestfs.") if len(options.host_run) != 0: @@ -337,12 +337,12 @@ def signal_handler(signum, frame): image.os.install_virtio_drivers() if len(options.host_run) != 0: - out.info("Running scripts on the input media:") + out.info("Running scripts on the input medium:") mpoint = tempfile.mkdtemp() try: image.mount(mpoint) if not image.is_mounted(): - raise FatalError("Mounting the media on the host failed.") + raise FatalError("Mounting the medium on the host failed.") try: size = len(options.host_run) cnt = 1 @@ -357,7 +357,7 @@ def signal_handler(signum, frame): cnt += 1 finally: while not image.umount(): - out.warn("Unable to umount the media. Retrying ...") + out.warn("Unable to umount the medium. Retrying ...") time.sleep(1) out.info() finally: diff --git a/image_creator/os_type/__init__.py b/image_creator/os_type/__init__.py index c8ff0ab..8878417 100644 --- a/image_creator/os_type/__init__.py +++ b/image_creator/os_type/__init__.py @@ -307,7 +307,7 @@ def _cleanup(self, namespace): del self._cleanup_jobs[namespace] def inspect(self): - """Inspect the media to check if it is supported""" + """Inspect the medium to check if it is supported""" if self.image.is_unsupported(): return @@ -454,7 +454,7 @@ def do_sysprep(self): if self.image.is_unsupported(): self.out.warn( - "System preparation is disabled for unsupported media") + "System preparation is disabled for unsupported medium") return enabled = [s for s in self.list_syspreps() if self.sysprep_enabled(s)] @@ -499,7 +499,7 @@ class Mount: """The Mount context manager""" def __enter__(self): mount_type = 'read-only' if readonly else 'read-write' - output("Mounting the media %s ..." % mount_type, False) + output("Mounting the medium %s ..." % mount_type, False) parent._mount_error = "" del parent._mount_warnings[:] @@ -511,7 +511,7 @@ def __enter__(self): raise if not parent.ismounted: - msg = "Unable to mount the media %s. Reason: %s" % \ + msg = "Unable to mount the medium %s. Reason: %s" % \ (mount_type, parent._mount_error) if fatal: raise FatalError(msg) @@ -525,7 +525,7 @@ def __enter__(self): success('done') def __exit__(self, exc_type, exc_value, traceback): - output("Umounting the media ...", False) + output("Umounting the medium ...", False) parent.image.g.umount_all() parent._mounted = False success('done') diff --git a/image_creator/os_type/freebsd.py b/image_creator/os_type/freebsd.py index 0723724..bceec26 100644 --- a/image_creator/os_type/freebsd.py +++ b/image_creator/os_type/freebsd.py @@ -50,7 +50,7 @@ def _check_enabled_sshd(self): return sshd_enabled def _do_inspect(self): - """Run various diagnostics to check if media is supported""" + """Run various diagnostics to check if medium is supported""" self.out.info('Checking partition table type...', False) ptype = self.image.g.part_get_parttype(self.image.guestfs_device) diff --git a/image_creator/os_type/linux.py b/image_creator/os_type/linux.py index 5e588a9..5356d7f 100644 --- a/image_creator/os_type/linux.py +++ b/image_creator/os_type/linux.py @@ -440,16 +440,16 @@ def _convert_fstab_line(self, line, devices): return orig, dev, mpoint def _do_inspect(self): - """Run various diagnostics to check if media is supported""" + """Run various diagnostics to check if medium is supported""" self.out.info( - 'Checking if the media contains logical volumes (LVM)...', False) + 'Checking if the medium contains logical volumes (LVM)...', False) has_lvm = True if len(self.image.g.lvs()) else False if has_lvm: self.out.info() - self.image.set_unsupported('The media contains logical volumes') + self.image.set_unsupported('The medium contains logical volumes') else: self.out.success('no') diff --git a/image_creator/os_type/slackware.py b/image_creator/os_type/slackware.py index bf28476..5908771 100644 --- a/image_creator/os_type/slackware.py +++ b/image_creator/os_type/slackware.py @@ -41,7 +41,7 @@ def is_enabled(self, service): if self.image.g.is_file(name): return self.image.g.stat(name)['mode'] & 0400 - self.out.warn('Service %s not found on the media' % service) + self.out.warn('Service %s not found on the medium' % service) return False # vim: set sta sts=4 shiftwidth=4 sw=4 et ai : diff --git a/image_creator/os_type/unsupported.py b/image_creator/os_type/unsupported.py index 41ef24e..448710d 100644 --- a/image_creator/os_type/unsupported.py +++ b/image_creator/os_type/unsupported.py @@ -27,11 +27,11 @@ def __init__(self, image, **kwargs): def collect_metadata(self): """Collect metadata about the OS""" - self.out.warn("Unable to collect metadata for unsupported media") + self.out.warn("Unable to collect metadata for unsupported medium") def _do_mount(self, readonly): """Mount partitions in correct order""" - self._mount_error = "not supported for this media" + self._mount_error = "not supported for this medium" return False # vim: set sta sts=4 shiftwidth=4 sw=4 et ai : diff --git a/image_creator/os_type/windows/__init__.py b/image_creator/os_type/windows/__init__.py index 76fb573..acb857f 100644 --- a/image_creator/os_type/windows/__init__.py +++ b/image_creator/os_type/windows/__init__.py @@ -271,7 +271,7 @@ def __init__(self, image, **kwargs): self.registry = Registry(self.image) with self.mount(readonly=True, silent=True): - self.out.info("Checking media state ...", False) + self.out.info("Checking medium state ...", False) # Enumerate the windows users (self.usernames, @@ -429,7 +429,7 @@ def _shrink(self): querymax = int(querymax) if querymax == 0: - self.out.warn("Unable to reclaim any space. The media is full.") + self.out.warn("Unable to reclaim any space. The medium is full.") return # Not sure if we should use 1000 or 1024 here @@ -469,7 +469,7 @@ def _shrink(self): if rc != 0: raise FatalError( - "Shrinking failed. Please make sure the media is defragged.") + "Shrinking failed. Please make sure the medium is defragged.") for line in stdout.splitlines(): if line.find("%d" % querymax) >= 0: @@ -490,23 +490,23 @@ def do_sysprep(self): if self.sysprepped: raise FatalError( - "Microsoft's System Preparation Tool has ran on the media. " + "Microsoft's System Preparation Tool has ran on the medium. " "Further image customization is not possible.") if len(self.virtio_state['viostor']) == 0: raise FatalError( - "The media has no VirtIO SCSI controller driver installed. " + "The medium has no VirtIO SCSI controller driver installed. " "Further image customization is not possible.") if len(self.virtio_state['netkvm']) == 0: raise FatalError( - "The media has no VirtIO Ethernet Adapter driver installed. " + "The medium has no VirtIO Ethernet Adapter driver installed. " "Further image customization is not possible.") timeout = self.sysprep_params['boot_timeout'].value shutdown_timeout = self.sysprep_params['shutdown_timeout'].value - self.out.info("Preparing media for boot ...", False) + self.out.info("Preparing medium for boot ...", False) with self.mount(readonly=False, silent=True): @@ -601,7 +601,7 @@ def if_not_sysprepped(task, *args): finally: self.image.enable_guestfs() - self.out.info("Reverting media boot preparations ...", False) + self.out.info("Reverting medium boot preparations ...", False) with self.mount(readonly=False, silent=True, fatal=False): if not self.ismounted: @@ -780,7 +780,7 @@ def _check_connectivity(self): def compute_virtio_state(self, directory=None): """Returns information about the VirtIO drivers found either in a - directory or the media itself if the directory is None. + directory or the medium itself if the directory is None. """ state = {} for driver in VIRTIO: @@ -817,7 +817,7 @@ def local_files(): def _fetch_virtio_drivers(self, dirname): """Examines a directory for VirtIO drivers and returns only the drivers - that are suitable for this media. + that are suitable for this medium. """ collection = self.compute_virtio_state(dirname) @@ -829,7 +829,7 @@ def _fetch_virtio_drivers(self, dirname): for inf, content in drvs.items(): valid = True found_match = False - # Check if the driver is suitable for the input media + # Check if the driver is suitable for the input medium for target in content['TargetOSVersions']: if len(target) > len(self.windows_version): match = target.startswith(self.windows_version) @@ -863,8 +863,8 @@ def _fetch_virtio_drivers(self, dirname): return collection def install_virtio_drivers(self, upgrade=True): - """Install new VirtIO drivers on the input media. If upgrade is True, - then the old drivers found in the media will be removed. + """Install new VirtIO drivers on the input medium. If upgrade is True, + then the old drivers found in the medium will be removed. """ dirname = self.sysprep_params['virtio'].value @@ -1048,7 +1048,7 @@ def remove_tmp(): self._boot_virtio_vm() def _boot_virtio_vm(self): - """Boot the media and install the VirtIO drivers""" + """Boot the medium and install the VirtIO drivers""" old_windows = self.check_version(6, 1) <= 0 self.image.disable_guestfs() From 592f73a5f1c91be822a5cc2175ec1bc6e737cc4d Mon Sep 17 00:00:00 2001 From: Vangelis Koukis Date: Sat, 28 Mar 2015 20:59:21 +0200 Subject: [PATCH 5/7] Ensure early exceptions appear in the terminal Make the process of resetting the terminal and outputting the text of an unexpected exception to stderr more robust. It seems resetting the terminal after using dialog races with printing the text of the exception, and overwrites it with the dialog background. Sleep for a tiny amount of time to ensure the exception is visible. This is an ugly workaround and should be replaced. --- image_creator/dialog_main.py | 39 ++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/image_creator/dialog_main.py b/image_creator/dialog_main.py index ae921a8..f89962c 100644 --- a/image_creator/dialog_main.py +++ b/image_creator/dialog_main.py @@ -2,6 +2,7 @@ # -*- coding: utf-8 -*- # # Copyright (C) 2011-2015 GRNET S.A. +# Copyright (C) 2015 Vangelis Koukis # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -27,6 +28,7 @@ import signal import argparse import types +import time import termios import traceback import tempfile @@ -292,28 +294,57 @@ def main(): parser.error("Unable to open logfile `%s' for writing. Reason: %s" % (args.logfile, error.strerror)) + # Ensure we run on a terminal, so we can use termios calls liberally + if not (os.isatty(sys.stdin.fileno()) and os.isatty(sys.stdout.fileno())): + sys.stderr.write(("Error: This program is interactive and requires a" + "terminal for standard input and output.")) + sys.exit(2) + + # Save the terminal attributes + attr = termios.tcgetattr(sys.stdin.fileno()) + try: - # Save the terminal attributes - attr = termios.tcgetattr(sys.stdin.fileno()) try: ret = dialog_main(args.medium, logfile=logfile, tmpdir=args.tmpdir, snapshot=args.snapshot, syslog=args.syslog) finally: # Restore the terminal attributes. If an error occurs make sure # that the terminal turns back to normal. - termios.tcsetattr(sys.stdin.fileno(), termios.TCSADRAIN, attr) + + # This is an ugly hack: + # + # It seems resetting the terminal after using dialog + # races with printing the text of the exception, + # and overwrites it with the dialog background. + # + # Sleep for a tiny amount of time to ensure + # the exception is visible. + # + # This code path is ugly and must be replaced: + # + # Logging should be mandatory, since we have a temporary directory + # anyway, and all logging output should go there. + time.sleep(0.5) + termios.tcflush(sys.stdin.fileno(), termios.TCIOFLUSH) + termios.tcsetattr(sys.stdin.fileno(), termios.TCSANOW, attr) except: - # Clear the screen + # Clear the screen and output the traceback + sys.stdout.flush() sys.stdout.write('\033[2J') # Erase Screen sys.stdout.write('\033[H') # Cursor Home sys.stdout.flush() exception = traceback.format_exc() + sys.stderr.write("An unexpected exception has occured. Please" + " include the following text in any bug report:\n\n") sys.stderr.write(exception) + sys.stderr.flush() + if logfile is not None: logfile.write(exception) sys.exit(3) + finally: if logfile is not None: logfile.close() From d94e7ccd2fe3a703b33325a6a677fd13816e5ee6 Mon Sep 17 00:00:00 2001 From: Vangelis Koukis Date: Sun, 29 Mar 2015 18:12:39 +0300 Subject: [PATCH 6/7] Re-write tmpdir selection, disable it temporarily Re-write the procedure of selecting a directory to hold temporary files, so it does not use an arbitrary list of directories including /mnt and the user's home directory (which is presumably /root), but starts with a list of all mountpoints and /tmp, /var/tmp as candidates. Also fix bug where the creator would choose a directory under a read-only mount for storing temporary files, and fail. Still, disable this code path temporarily, and only choose between /tmp and /var/tmp, if the user has made no explicit choice. It is very un-intuitive to create temporary files in unrelated directories without the user's explicit consent. Perhaps this code path can be enabled again when snf-image-creator has the ability to propose these alternate locations to the user, and have them choose explicitly. --- image_creator/disk.py | 77 +++++++++++++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 17 deletions(-) diff --git a/image_creator/disk.py b/image_creator/disk.py index e3f8977..85855e9 100644 --- a/image_creator/disk.py +++ b/image_creator/disk.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # # Copyright (C) 2011-2014 GRNET S.A. +# Copyright (C) 2015 Vangelis Koukis # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -28,32 +29,74 @@ import uuid import shutil +import logging +log = logging.getLogger(__name__) + dd = get_command('dd') dmsetup = get_command('dmsetup') losetup = get_command('losetup') blockdev = get_command('blockdev') -def get_tmp_dir(default=None): - """Check tmp directory candidates and return the one with the most - available space. - """ - if default is not None: - return default - - TMP_CANDIDATES = ['/var/tmp', os.path.expanduser('~'), '/mnt'] +def get_tmp_dir(tmpdir=None): + """Try to guess a suitable directory for holding temporary files. - space = [free_space(t) for t in TMP_CANDIDATES] + Try to guess a suitable directory for holding (rather big) temporary + files, including the snapshot file used to protect the source medium. - max_idx = 0 - max_val = space[0] - for i, val in zip(range(len(space)), space): - if val > max_val: - max_val = val - max_idx = i + """ - # Return the candidate path with more available space - return TMP_CANDIDATES[max_idx] + # Don't try to outsmart the user, if they have already made a choice + if tmpdir is not None: + log.debug("Using user-specified value `%s' as tmp directory", tmpdir) + return tmpdir + + # If the TMPDIR environment has been set, use it + if "TMPDIR" in os.environ: + tmpdir = os.environ["TMPDIR"] + log.debug(("Using value of TMPDIR environment variable as tmp" + "directory"), tmpdir) + return tmpdir + + # Otherwise, make a list of candidate locations (all mountpoints) and pick + # the directory with the most available space, provided it is mounted + # read-write. The standard /var/tmp, /tmp are added to the end of the list, + # to ensure they are preferred over the mountpoint of the filesystem they + # belong to. + # + # FIXME: Enumerating mount points using /etc/mtab is Linux-specific. + # FIXME: Perhaps omit remote directories, e.g., NFS/SMB mounts? + # Must use stafs(2) for this, statvfs(2) does not return f_type + + with open("/etc/mtab", "r") as mtab: + mounts = [l.strip() for l in mtab.readlines()] + points = [m.split(" ")[1] for m in mounts] + ["/tmp", "/var/tmp"] + + # FIXME: + # Disable the above algorithm for the time being, and reduce the + # list of candidate directories to the standard /var/tmp, /tmp directories. + # + # It is un-intuitive and completely unexpected by the user to end up + # using a random directory under /home, or under /mnt to hold temporary + # files. Perhaps re-enable it when we actually have the ability to + # propose these alternate locations to the user, and have them make + # choose explicitly. + # + points = ["/var/tmp", "/tmp"] + log.debug("Trying to guess a suitable tmpdir, candidates are: %s", + ", ".join(points)) + + stats = [os.statvfs(p) for p in points] + rwzip = [z for z in zip(points, stats) if + z[1].f_flag & os.ST_RDONLY == 0] + # See http://plug.org/pipermail/plug/2010-August/023606.html + # on why calculation of free space is based on f_frsize + sortedzip = sorted(rwzip, key=lambda z: z[1].f_bavail * z[1].f_frsize, + reverse=True) + tmpdir = sortedzip[0][0] + + log.debug("Using directory `%s' as tmp directory", tmpdir) + return tmpdir class Disk(object): From 51b1552c23af802bdbd0e8d3c66865ae22b30f7d Mon Sep 17 00:00:00 2001 From: Vangelis Koukis Date: Mon, 30 Mar 2015 00:31:42 +0300 Subject: [PATCH 7/7] Use Python logging, separate logging from Output Separate the process of logging from the process of interacting with the user ("Output" instances) to ensure emitting debugging information is possible from anywhere inside the code. Use the standard Python logging module for logging, instead of a custom approach. This brings lots of advantages, including proper log formatting and timestamping. Use logging.handlers.SysLogHandler for logging to syslog, instead of calling the syslog module directly. Also use a standard default logfile /var/log/snf-image-creator for persistence, instead of a temporary logfile, and remove redundant functions show_log(), copy_file(). This simplifies the error handling path significantly. The general idea of this commit is that the instances of "Output" should be independent frontends for interacting with the user, and can be extended to support Input as well as Output in the future. This is a Work in Progress. The previous code assumed that all messages to the user via Output instances would also make it to the log file. For now, only what is explicitly logged reaches the logfile. A workaround is to have the base Output class log all messages to the user, and will be implemented in a future commit. --- image_creator/constants.py | 22 +++++++ image_creator/dialog_main.py | 88 ++++++++++++++-------------- image_creator/dialog_menu.py | 30 +--------- image_creator/dialog_util.py | 15 ----- image_creator/log.py | 110 +++++++++++++++++++++++++++++++++++ 5 files changed, 179 insertions(+), 86 deletions(-) create mode 100644 image_creator/constants.py create mode 100644 image_creator/log.py diff --git a/image_creator/constants.py b/image_creator/constants.py new file mode 100644 index 0000000..b6d05a7 --- /dev/null +++ b/image_creator/constants.py @@ -0,0 +1,22 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2015 Vangelis Koukis +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +"""Various constants used throughout snf-image-creator""" + +DEFAULT_LOGFILE = "/var/log/snf-image-creator.log" + +# vim: set sta sts=4 shiftwidth=4 sw=4 et ai : diff --git a/image_creator/dialog_main.py b/image_creator/dialog_main.py index f89962c..db9f9fc 100644 --- a/image_creator/dialog_main.py +++ b/image_creator/dialog_main.py @@ -32,8 +32,11 @@ import termios import traceback import tempfile +import logging from image_creator import __version__ as version +from image_creator import constants +from image_creator.log import SetupLogging from image_creator.util import FatalError, ensure_root from image_creator.output.cli import SimpleOutput from image_creator.output.dialog import GaugeOutput @@ -47,6 +50,8 @@ PROGNAME = os.path.basename(sys.argv[0]) +log = logging.getLogger(__name__) + def create_image(d, medium, out, tmp, snapshot): """Create an image out of `medium'""" @@ -168,7 +173,6 @@ def dialog_main(medium, **kwargs): tmpdir = kwargs['tmpdir'] if 'tmpdir' in kwargs else None snapshot = kwargs['snapshot'] if 'snapshot' in kwargs else True - logfile = kwargs['logfile'] if 'logfile' in kwargs else None syslog = kwargs['syslog'] if 'syslog' in kwargs else False # In openSUSE dialog is buggy under xterm @@ -210,37 +214,29 @@ def dialog_main(medium, **kwargs): continue break - tmplog = None if logfile else tempfile.NamedTemporaryFile(prefix='fatal-', - delete=False) + # FIXME: It does not make sense to pass both the dialog instance + # explicitly, and Output instances separately. The called function + # shouldn't have to know that it is using a dialog instance, or call + # pythondialog-specific methods, but use the Output instance via a + # defined interface. + # This is an ugly workaround, until the separation of logging and Output + # frontends is complete: Just make logging through Output a no-op for now, + # by passing an empty list. logs = [] try: - stream = logfile if logfile else tmplog - logs.append(SimpleOutput(colored=False, stderr=stream, stdout=stream)) - if syslog: - logs.append(SyslogOutput()) - while 1: try: out = CompositeOutput(logs) - out.info("Starting %s v%s ..." % (PROGNAME, version)) ret = create_image(d, medium, out, tmpdir, snapshot) break except Reset: - for log in logs: - log.info("Resetting everything ...") + log.info("Resetting everything ...") except FatalError as error: - for log in logs: - log.error(str(error)) - msg = 'A fatal error occured. See %s for a full log.' % log.stderr.name + log.error("Fatal: " + str(error)) + msg = "A fatal error has occured: " + str(error) d.infobox(msg, width=WIDTH, title="Fatal Error") return 1 - else: - if tmplog: - os.unlink(tmplog.name) - finally: - if tmplog: - tmplog.close() return ret @@ -257,22 +253,30 @@ def main(): " snapshot of INPUT_MEDIUM, and will not modify its contents.") e = ("%(prog)s requires root privileges.") - parser = argparse.ArgumentParser(version=version, description=d, epilog=e) + parser = argparse.ArgumentParser(description=d, epilog=e) - parser.add_argument("-l", "--logfile", metavar="FILE", type=str, - dest="logfile", default=None, - help="Log all messages to FILE") - parser.add_argument("--syslog", dest="syslog", default=False, - help="Also log to syslog", action="store_true") - parser.add_argument("--tmpdir", metavar="DIR", type=str, dest="tmpdir", + parser.add_argument("--tmpdir", metavar="TMPDIR", type=str, dest="tmpdir", default=None, - help="Create large temporary image files under DIR") + help=("Create large temporary files under TMPDIR." + " Default is to use a randomly-named temporary" + " directory under /var/tmp or /tmp.")) + parser.add_argument("-l", "--logfile", metavar="LOGFILE", type=str, + dest="logfile", default=constants.DEFAULT_LOGFILE, + help=("Log all messages to LOGFILE." + " Default: %(default)s")) + parser.add_argument("--syslog", dest="syslog", default=False, + action="store_true", help="Also log to syslog") + parser.add_argument("-v", "--verbose", dest="verbose", default=False, + action="store_true", + help="Be verbose, log everything to ease debugging") parser.add_argument("--no-snapshot", dest="snapshot", default=True, action="store_false", help=("Do not work on a snapshot, but modify the input" " medium directly instead. DO NOT USE THIS" " OPTION UNLESS YOU REALLY KNOW WHAT YOU ARE" " DOING. THIS WILL ALTER THE ORIGINAL MEDIUM!")) + parser.add_argument("-V", "--version", action="version", + version=version) parser.add_argument(metavar="INPUT_MEDIUM", nargs='?', dest="medium", type=str, default=None, help=("Use INPUT_MEDIUM as the template for" @@ -288,11 +292,12 @@ def main(): parser.error("Argument `%s' to --tmpdir must be a directory" % args.tmpdir) - try: - logfile = open(args.logfile, 'w') if args.logfile is not None else None - except IOError as error: - parser.error("Unable to open logfile `%s' for writing. Reason: %s" % - (args.logfile, error.strerror)) + # Setup logging and get a logger as early as possible. + # FIXME: Must turn on redirect_stderr, but need to verify that only + # errors/diagnostics and not user-visible output goes to stderr + SetupLogging(PROGNAME, logfile=args.logfile, debug=args.verbose, + use_syslog=args.syslog, redirect_stderr_fd=False) + log.info("%s v%s starting..." % (PROGNAME, version)) # Ensure we run on a terminal, so we can use termios calls liberally if not (os.isatty(sys.stdin.fileno()) and os.isatty(sys.stdout.fileno())): @@ -302,10 +307,9 @@ def main(): # Save the terminal attributes attr = termios.tcgetattr(sys.stdin.fileno()) - try: try: - ret = dialog_main(args.medium, logfile=logfile, tmpdir=args.tmpdir, + ret = dialog_main(args.medium, tmpdir=args.tmpdir, snapshot=args.snapshot, syslog=args.syslog) finally: # Restore the terminal attributes. If an error occurs make sure @@ -328,7 +332,10 @@ def main(): termios.tcflush(sys.stdin.fileno(), termios.TCIOFLUSH) termios.tcsetattr(sys.stdin.fileno(), termios.TCSANOW, attr) except: - # Clear the screen and output the traceback + # An unexpected exception has occured. + # Ensure the exception is logged, + # then clear the screen and output the traceback. + log.exception("Internal error. Unexpected Exception:") sys.stdout.flush() sys.stdout.write('\033[2J') # Erase Screen sys.stdout.write('\033[H') # Cursor Home @@ -338,17 +345,12 @@ def main(): sys.stderr.write("An unexpected exception has occured. Please" " include the following text in any bug report:\n\n") sys.stderr.write(exception) + sys.stderr.write(("\nLogfile `%s' may contain more information about" + " the cause of this error.\n\n" % args.logfile)) sys.stderr.flush() - if logfile is not None: - logfile.write(exception) - sys.exit(3) - finally: - if logfile is not None: - logfile.close() - sys.exit(ret) # vim: set sta sts=4 shiftwidth=4 sw=4 et ai : diff --git a/image_creator/dialog_menu.py b/image_creator/dialog_menu.py index 4980757..2c24e44 100644 --- a/image_creator/dialog_menu.py +++ b/image_creator/dialog_menu.py @@ -34,8 +34,7 @@ from image_creator.help import get_help_file from image_creator.dialog_util import SMALL_WIDTH, WIDTH, \ update_background_title, confirm_reset, confirm_exit, Reset, \ - extract_image, add_cloud, edit_cloud, update_sysprep_param, select_file, \ - copy_file + extract_image, add_cloud, edit_cloud, update_sysprep_param, select_file CONFIGURATION_TASKS = [ ("Partition table manipulation", ["FixPartitionTable"], lambda x: True), @@ -979,31 +978,6 @@ def mount(session): os.rmdir(mpoint) -def show_log(session): - """Show the current execution log""" - - d = session['dialog'] - log = session['image'].out[0].stderr - - log.file.flush() - - while 1: - code = d.textbox(log.name, title="Log", width=70, height=40, - extra_button=1, extra_label="Save", ok_label="Close") - if code == d.DIALOG_EXTRA: - while 1: - path = select_file(d, title="Save log as...") - if path is None: - break - if os.path.isdir(path): - continue - - if copy_file(d, log.name, path): - break - else: - return - - def customization_menu(session): """Show image customization menu""" d = session['dialog'] @@ -1057,7 +1031,7 @@ def main_menu(session): default_item = "Customize" actions = {"Customize": customization_menu, "Register": kamaki_menu, - "Extract": extract_image, "Log": show_log} + "Extract": extract_image} title = "Image Creator for Synnefo (snf-image-creator v%s)" % version while 1: (code, choice) = d.menu( diff --git a/image_creator/dialog_util.py b/image_creator/dialog_util.py index 129ef58..1709bfe 100644 --- a/image_creator/dialog_util.py +++ b/image_creator/dialog_util.py @@ -452,19 +452,4 @@ def update_sysprep_param(session, name, title=None): return True - -def copy_file(d, src, dest): - """Copy src file to dest""" - - assert os.path.exists(src), "File: `%s' does not exist" % src - - if os.path.exists(dest): - if d.yesno("File: `%s' exists! Are you sure you want to overwrite it?", - defaultno=1, width=WIDTH): - return False - - shutil.copyfile(src, dest) - d.msgbox("File: `%s' was successfully written!") - return True - # vim: set sta sts=4 shiftwidth=4 sw=4 et ai : diff --git a/image_creator/log.py b/image_creator/log.py new file mode 100644 index 0000000..129ce91 --- /dev/null +++ b/image_creator/log.py @@ -0,0 +1,110 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2015 Vangelis Koukis +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +"""Setup logging: Formatters, handlers, and loggers""" + +import logging +import logging.handlers + + +def _get_log_format(progname, debug=False, use_syslog=False): + """Return an appropriate log format + + Different formats ensure entries are uniformly formatted and do not + contain redundant information -- syslogd will log timestamps on its own. + + """ + if use_syslog: + fmt = progname + "[%(process)d]: " + else: + fmt = "%(asctime)-15s " + progname + " pid=%(process)-6d " + + if debug: + fmt += "%(module)s:%(lineno)s " + + fmt += "[%(levelname)s] %(message)s" + + return fmt + + +def SetupLogging(progname, debug=False, logfile=None, redirect_stderr_fd=False, + use_syslog=False, syslog_facility=None): + """Configure the logging module. + + If debug is False (default) log messages of level INFO and higher, + otherwise log all messages. + + If logfile is specified, use it to log messages of the appropriate level, + according to the setting of debug. + + If redirect_stderr_fd is True, also redirect stderr (fd 2) to logfile. + This ensures the log captures the stderr of child processes and the Python + interpreter itself. + + Finally, if use_syslog is True, also log to syslog using the facility + specified via syslog_facility, or LOG_USER if left unspecified. + + Note more logging points may be added in the future by using + root_handler.add_handler() repeatedly. + + """ + + # File and syslog formatters + file_formatter = logging.Formatter(_get_log_format(progname, debug, + False)) + syslog_formatter = logging.Formatter(_get_log_format(progname, debug, + True)) + + # Root logger + root_logger = logging.getLogger("") + root_logger.setLevel(logging.NOTSET) # Process all log messages by default + + level = logging.NOTSET if debug else logging.INFO + + for handler in root_logger.handlers: + handler.close() + root_logger.removeHandler(handler) + + # Syslog handler + if use_syslog: + facility = (syslog_facility if syslog_facility is not None + else logging.handlers.SysLogHandler.LOG_USER) + # We hardcode address `/dev/log' for now, i.e., deliver to local + # syslogd. + # + # The local administrator can impose whatevery policy they wish + # by configuring the local syslogd, e.g., to forward logs + # to a remote syslog server. + syslog_handler = logging.handlers.SysLogHandler(address="/dev/log", + facility=facility) + syslog_handler.setFormatter(syslog_formatter) + syslog_handler.setLevel(level) + root_logger.addHandler(syslog_handler) + + # File handler + if logfile is not None: + logfile_handler = logging.FileHandler(logfile, "a") + logfile_handler.setFormatter(file_formatter) + logfile_handler.setLevel(level) + root_logger.addHandler(logfile_handler) + + # Redirect standard error at the OS level, to catch all error output, + # including that of child processes and the Python interpreter itself + if redirect_stderr_fd: + os.dup2(logfile_handler.stream.fileno(), 1) + +# vim: set sta sts=4 shiftwidth=4 sw=4 et ai :