[et-mgmt-tools] [PATCH 4 of 5] Cleanup on failure

john.levon at sun.com john.levon at sun.com
Wed Jul 9 18:29:38 UTC 2008


# HG changeset patch
# User john.levon at sun.com
# Date 1215627983 25200
# Node ID a36c666ef5d381e3961545d0359949e82d6d064b
# Parent  c96c084d9b8d39a6df238cd9798011a8c4a34dd0
Cleanup on failure

If we can't convert the disks or export the file, perform some cleanup.

Signed-off-by: John Levon <john.levon at sun.com>

diff --git a/virt-convert b/virt-convert
--- a/virt-convert
+++ b/virt-convert
@@ -29,9 +29,12 @@
 import virtinst.cli as cli
 import virtinst.util as util
 import virtconv
-import virtconv.vmconfig as vmconfig
+import virtconv.formats as formats
+import virtconv.vmcfg as vmcfg
+import virtconv.diskcfg as diskcfg
 
 def parse_args():
+    """Parse and verify command line."""
     opts = OptionParser()
     opts.set_usage("%prog [options] inputdir|input.vmx "
         "[outputdir|output.xml]")
@@ -98,19 +101,41 @@
     if not options.quiet:
         print msg
 
+def cleanup(msg, options, vmdef, paths):
+    """
+    After failure, clean up anything we created.
+    """
+    logging.error(msg)
+
+    try:
+        paths.reverse()
+        for path in paths:
+            if os.path.isdir(path):
+                os.rmdir(path)
+            elif os.path.isfile(path):
+                os.remove(path)
+
+        for disk in vmdef.disks:
+            disk.cleanup()
+    except OSError, e:
+        logging.error("Couldn't clean up output directory \"%s\": %s" %
+            (options.output_dir, e.strerror))
+
+    sys.exit(1)
+
 def main():
     options = parse_args()
     cli.setupLogging("virt-convert", options.debug)
 
     try:
-        inp = virtconv.vmconfig.find_parser_by_name(options.input_format)
+        inp = formats.find_parser_by_name(options.input_format)
     except:
         logging.error("No parser of format \"%s\" was found." %
             options.input_format)
         sys.exit(1)
  
     try:
-        outp = virtconv.vmconfig.find_parser_by_name(options.output_format)
+        outp = formats.find_parser_by_name(options.output_format)
     except:
         logging.error("No parser of format \"%s\" was found." %
             options.output_format)
@@ -130,17 +155,21 @@
         sys.exit(1)
 
     if options.paravirt:
-        vmdef.type = vmconfig.VM_TYPE_PV
+        vmdef.type = vmcfg.VM_TYPE_PV
     else:
-        vmdef.type = vmconfig.VM_TYPE_HVM
+        vmdef.type = vmcfg.VM_TYPE_HVM
 
     vmdef.arch = options.arch
 
+    clean = []
+
     unixname = vmdef.name.replace(" ", "-")
+
     if not options.output_dir:
         options.output_dir = unixname
     try:
         logging.debug("Creating directory %s" % options.output_dir)
+        clean += [ options.output_dir ]
         os.mkdir(options.output_dir)
     except OSError, e:
         if (e.errno != errno.EEXIST):
@@ -163,18 +192,20 @@
     try:
         for d in vmdef.disks:
             verbose(options, "Converting disk \"%s\" to type %s..." %
-                (d.path, vmconfig.qemu_formats[vmconfig.DISK_TYPE_RAW]))
-            d.convert(options.input_dir, options.output_dir,
-                vmconfig.DISK_TYPE_RAW)
-    except Exception, e:
-        logging.error(e)
-        sys.exit(1)
- 
+                (d.path, "raw"))
+            d.convert(options.input_dir, options.output_dir, "raw")
+    except OSError, e:
+        cleanup("Couldn't convert disks: %s" % e.strerror,
+            options, vmdef, clean)
+    except RuntimeError, e:
+        cleanup("Couldn't convert disks: %s" % e.message, options, vmdef, clean)
+
     try:
+        clean += [ options.output_file ]
         outp.export_file(vmdef, options.output_file)
-    except Exception, e:
-        logging.error(e)
-        sys.exit(1)
+    except ValueError, e:
+        cleanup("Couldn't export to file \"%s\": %s" %
+            (options.output_file, e.message), options, vmdef, clean)
 
     verbose(options, "Done.")
 
diff --git a/virtconv/diskcfg.py b/virtconv/diskcfg.py
new file mode 100644
--- /dev/null
+++ b/virtconv/diskcfg.py
@@ -0,0 +1,181 @@
+#
+# Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
+# Use is subject to license terms.
+#
+# 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 2 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, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA 02110-1301 USA.
+#
+
+import shutil
+import errno
+import os
+
+DISK_FORMAT_NONE = 0
+DISK_FORMAT_RAW = 1
+DISK_FORMAT_VMDK = 2
+DISK_FORMAT_VDISK = 3
+
+DISK_TYPE_DISK = 0
+DISK_TYPE_CDROM = 1
+DISK_TYPE_ISO = 2
+
+disk_suffixes = {
+    DISK_FORMAT_RAW: ".img",
+    DISK_FORMAT_VMDK: ".vmdk",
+    DISK_FORMAT_VDISK: ".vdisk.xml",
+}
+
+qemu_formats = {
+    DISK_FORMAT_RAW: "raw",
+    DISK_FORMAT_VMDK: "vmdk",
+    DISK_FORMAT_VDISK: "vdisk",
+}
+
+disk_format_names = {
+    "none": DISK_FORMAT_NONE,
+    "raw": DISK_FORMAT_RAW,
+    "vmdk": DISK_FORMAT_VMDK,
+    "vdisk": DISK_FORMAT_VDISK,
+}
+
+def ensuredirs(path):
+    """
+    Make sure that all the containing directories of the given file
+    path exist.
+    """
+    try:
+        os.makedirs(os.path.dirname(path))
+    except OSError, e: 
+        if e.errno != errno.EEXIST:
+            raise
+
+class disk(object):
+    """Definition of an individual disk instance."""
+
+    def __init__(self, path = None, number = 0, format = None, bus = "ide",
+        type = DISK_TYPE_DISK):
+        self.path = path
+        self.format = format
+        self.number = number
+        self.bus = bus
+        self.type = type
+        self.clean = []
+
+    def cleanup(self):
+        """
+        Remove any generated output.
+        """
+
+        for path in self.clean:
+            if os.path.isfile(path):
+                os.remove(path)
+
+        self.clean = []
+
+    def copy_file(self, infile, outfile):
+        """Copy an individual file."""
+        self.clean += [ outfile ]
+        ensuredirs(outfile)
+        shutil.copy(infile, outfile)
+
+    def copy(self, indir, outdir, out_format):
+        """
+        Copy the underlying disk files to a destination, if necessary.
+        Return True if we need a further conversion step.
+        """
+
+        if os.path.isabs(self.path):
+            return False
+
+        need_copy = False
+        need_convert = False
+
+        if self.format == out_format:
+            need_convert = False
+            need_copy = (indir != outdir)
+        else:
+            if out_format == DISK_FORMAT_NONE:
+                need_copy = (indir != outdir)
+                need_convert = False
+            else:
+                need_copy = (indir != outdir and out_format == DISK_FORMAT_VDISK)
+                need_convert = True
+
+        if need_copy:
+            if out_format == DISK_FORMAT_VDISK:
+                stdin, stdout = os.popen2(["/usr/bin/vdiskadm", "import", "-n",
+                    "-f", "-t", qemu_formats[self.format],
+                    "\"%s\"" % os.path.join(indir, self.path)])
+                paths = stdout.readlines()
+                stdin.close()
+                stdout.close()
+                for path in paths:
+                    self.copy_file(os.path.join(indir, path),
+                        os.path.join(outdir, path))
+                return need_convert
+
+            # this is not correct for all VMDK files, but it will have
+            # to do for now
+            self.copy_file(os.path.join(indir, self.path),
+                os.path.join(outdir, self.path))
+
+        return need_convert
+
+    def convert(self, indir, outdir, output_format):
+        """
+        Convert a disk into the requested format if possible, in the
+        given output directory.  Raises RuntimeError or other
+        failures.
+        """
+
+        out_format = disk_format_names[output_format]
+        indir = os.path.normpath(os.path.abspath(indir))
+        outdir = os.path.normpath(os.path.abspath(outdir))
+
+        need_convert = self.copy(indir, outdir, out_format)
+        if not need_convert:
+            return
+
+        relin = self.path
+        relout = self.path.replace(disk_suffixes[self.format],
+            disk_suffixes[out_format])
+        absin = os.path.join(indir, relin)
+        absout = os.path.join(outdir, relout)
+
+        ensuredirs(absout)
+
+        if out_format == DISK_FORMAT_VDISK:
+            convert_cmd = ("""/usr/bin/vdiskadm import -t %s "%s" "%s" """ %
+                (qemu_formats[out_format], absin, absout)) 
+        elif out_format == DISK_FORMAT_RAW:
+            convert_cmd = ("""qemu-img convert "%s" -O %s "%s" """ %
+                (absin, qemu_formats[out_format], absout))
+        else:
+            raise NotImplementedError("Cannot convert to disk format %s" %
+                output_format)
+
+        ret = os.system(convert_cmd)
+        if ret != 0:
+            raise RuntimeError("Disk conversion failed with exit status %d"
+                % ret)
+
+        self.path = relout
+        self.format = out_format
+
+def disk_formats():
+    """
+    Return a list of supported disk formats.
+    """
+    return disk_format_names.keys()
diff --git a/virtconv/formats.py b/virtconv/formats.py
new file mode 100644
--- /dev/null
+++ b/virtconv/formats.py
@@ -0,0 +1,82 @@
+#
+# Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
+# Use is subject to license terms.
+#
+# 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 2 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, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA 02110-1301 USA.
+#
+
+_parsers = [ ]
+
+class parser(object):
+    """
+    Base class for particular config file format definitions of
+    a VM instance.
+
+    Warning: this interface is not (yet) considered stable and may
+    change at will.
+    """
+
+    @staticmethod
+    def identify_file(input_file):
+        """
+        Return True if the given file is of this format.
+        """
+        raise NotImplementedError
+
+    @staticmethod
+    def import_file(input_file):
+        """
+        Import a configuration file.  Raises if the file couldn't be
+        opened, or parsing otherwise failed.
+        """
+        raise NotImplementedError
+
+    @staticmethod
+    def export_file(vm, output_file):
+        """
+        Export a configuration file.
+        @vm vm configuration instance
+        @output_file Output file
+
+        Raises ValueError if configuration is not suitable, or another
+        exception on failure to write the output file.
+        """
+        raise NotImplementedError
+            
+
+def register_parser(parser):
+    """
+    Register a particular config format parser.  This should be called by each
+    config plugin on import.
+    """
+
+    global _parsers
+    _parsers += [ parser ]
+
+def find_parser_by_name(name):
+    """
+    Return the parser of the given name
+    """
+    return [p for p in _parsers if p.name == name][0] or None
+
+def find_parser_by_file(input_file):
+    """
+    Return the parser that is capable of comprehending the given file.
+    """
+    for p in _parsers:
+        if p.identify_file(input_file):
+            return p
+    return None
diff --git a/virtconv/parsers/virtimage.py b/virtconv/parsers/virtimage.py
--- a/virtconv/parsers/virtimage.py
+++ b/virtconv/parsers/virtimage.py
@@ -19,7 +19,9 @@
 #
 
 from string import ascii_letters
-import virtconv.vmconfig as vmconfig
+import virtconv.formats as formats
+import virtconv.vmcfg as vmcfg
+import virtconv.diskcfg as diskcfg
 
 pv_boot_template = """
   <boot type="xen">
@@ -70,7 +72,7 @@
 </image>
 """
 
-class virtimage_parser(vmconfig.parser):
+class virtimage_parser(formats.parser):
     """
     Support for virt-install's image format (see virt-image man page).
     """
@@ -122,9 +124,10 @@
             hvm_disks.append("""<drive disk="%s" target="hd%s" />\n""" %
                 (path, ascii_letters[number % 26]))
             storage_disks.append(
-                """<disk file="%s" use="system" format="raw"/>\n""" % path)
+                """<disk file="%s" use="system" format="%s"/>\n"""
+                    % (path, diskcfg.qemu_formats[disk.format]))
 
-        if vm.type == vmconfig.VM_TYPE_PV:
+        if vm.type == vmcfg.VM_TYPE_PV:
             boot_template = pv_boot_template
         else:
             boot_template = hvm_boot_template
@@ -149,4 +152,4 @@
         outfile.writelines(out)
         outfile.close()
 
-vmconfig.register_parser(virtimage_parser)
+formats.register_parser(virtimage_parser)
diff --git a/virtconv/parsers/vmx.py b/virtconv/parsers/vmx.py
--- a/virtconv/parsers/vmx.py
+++ b/virtconv/parsers/vmx.py
@@ -18,9 +18,11 @@
 # MA 02110-1301 USA.
 #
 
-import virtconv.vmconfig as vmconfig
+import virtconv.formats as formats
+import virtconv.vmcfg as vmcfg
+import virtconv.diskcfg as diskcfg
 
-class vmx_parser(vmconfig.parser):
+class vmx_parser(formats.parser):
     """
     Support for VMWare .vmx files.  Note that documentation is
     particularly sparse on this format, with pretty much the best
@@ -44,7 +46,7 @@
         opened, or parsing otherwise failed.
         """
 
-        vm = vmconfig.vm()
+        vm = vmcfg.vm()
 
         infile = open(input_file, "r")
         contents = infile.readlines()
@@ -86,7 +88,7 @@
         vm.nr_vcpus = config.get("numvcpus")
 
         for (number, path) in enumerate(disks):
-            vm.disks += [ vmconfig.disk(path, number, vmconfig.DISK_TYPE_VMDK) ]
+            vm.disks += [ diskcfg.disk(path, number, diskcfg.DISK_FORMAT_VMDK) ]
 
         vm.validate()
         return vm
@@ -104,4 +106,4 @@
 
         raise NotImplementedError
 
-vmconfig.register_parser(vmx_parser)
+formats.register_parser(vmx_parser)
diff --git a/virtconv/vmconfig.py b/virtconv/vmcfg.py
rename from virtconv/vmconfig.py
rename to virtconv/vmcfg.py
--- a/virtconv/vmconfig.py
+++ b/virtconv/vmcfg.py
@@ -18,71 +18,8 @@
 # MA 02110-1301 USA.
 #
 
-import os
-
-_parsers = [ ]
-
 VM_TYPE_PV = 0
 VM_TYPE_HVM = 1
-
-DISK_TYPE_RAW = 0
-DISK_TYPE_VMDK = 1
-
-disk_suffixes = {
-    DISK_TYPE_RAW: ".img",
-    DISK_TYPE_VMDK: ".vmdk",
-}
-
-qemu_formats = {
-    DISK_TYPE_RAW: "raw",
-    DISK_TYPE_VMDK: "vmdk",
-}
-
-class disk(object):
-    """Definition of an individual disk instance."""
-
-    def __init__(self, path = None, number = None, type = None):
-        self.path = path
-        self.number = number
-        self.type = type
-
-    def convert(self, input_dir, output_dir, output_type):
-        """
-        Convert a disk into the requested format if possible, in the
-        given output directory.  Raises NotImplementedError or other
-        failures.
-        """
-
-        if self.type == output_type:
-            return
-
-        if output_type != DISK_TYPE_RAW:
-            raise NotImplementedError("Cannot convert to disk type %d" %
-                output_type)
-
-        infile = self.path
-
-        if not os.path.isabs(infile):
-            infile = os.path.join(input_dir, infile)
-
-        outfile = self.path
-
-        if os.path.isabs(outfile):
-            outfile = os.path.basename(outfile)
-
-        outfile = outfile.replace(disk_suffixes[self.type],
-            disk_suffixes[output_type]).strip()
-
-        convert_cmd = ("qemu-img convert \"%s\" -O %s \"%s\"" %
-            (infile, qemu_formats[output_type],
-            os.path.join(output_dir, outfile)))
-
-        os.system(convert_cmd)
-
-        # Note: this is the *relative* path still
-        self.path = outfile
-        self.type = output_type
-
 
 class vm(object):
     """
@@ -111,7 +48,7 @@
         self.nr_vcpus = None
         self.disks = [ ]
         self.type = VM_TYPE_HVM
-        self.arch = "i686" # FIXME?
+        self.arch = "i686"
 
     def validate(self):
         """
@@ -129,65 +66,3 @@
             raise ValueError("VM type is not set")
         if not self.arch:
             raise ValueError("VM arch is not set")
-
-        
-class parser(object):
-    """
-    Base class for particular config file format definitions of
-    a VM instance.
-
-    Warning: this interface is not (yet) considered stable and may
-    change at will.
-    """
-
-    @staticmethod
-    def identify_file(input_file):
-        """
-        Return True if the given file is of this format.
-        """
-        raise NotImplementedError
-
-    @staticmethod
-    def import_file(input_file):
-        """
-        Import a configuration file.  Raises if the file couldn't be
-        opened, or parsing otherwise failed.
-        """
-        raise NotImplementedError
-
-    @staticmethod
-    def export_file(vm, output_file):
-        """
-        Export a configuration file.
-        @vm vm configuration instance
-        @output_file Output file
-
-        Raises ValueError if configuration is not suitable, or another
-        exception on failure to write the output file.
-        """
-        raise NotImplementedError
-            
-
-def register_parser(parser):
-    """
-    Register a particular config format parser.  This should be called by each
-    config plugin on import.
-    """
-
-    global _parsers
-    _parsers += [ parser ]
-
-def find_parser_by_name(name):
-    """
-    Return the parser of the given name
-    """
-    return [p for p in _parsers if p.name == name][0] or None
-
-def find_parser_by_file(input_file):
-    """
-    Return the parser that is capable of comprehending the given file.
-    """
-    for p in _parsers:
-        if p.identify_file(input_file):
-            return p
-    return None




More information about the et-mgmt-tools mailing list