# HG changeset patch
# User john.levon@sun.com
# Date 1215470371 25200
# Node ID f0ddebbf39a3926f55045ade4fbf850706ed6c3e
# Parent c75d57437b7b219443580f6e9d6736d27d76ebcf
Cleanup on failure

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

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

diff --git a/virt-convert b/virt-convert
--- a/virt-convert
+++ b/virt-convert
@@ -31,6 +31,7 @@ import virtconv.vmconfig as vmconfig
import virtconv.vmconfig as vmconfig

def parse_args():
+ """Parse and verify command line."""
opts = OptionParser()
opts.set_usage("%prog [options] inputdir|input.vmx "
"[outputdir|output.xml]")
@@ -93,6 +94,36 @@ def parse_args():

return options

+def rmrf(path):
+ """Remove a directory and all its contents."""
+
+ assert path is not None
+
+ for dirpath, _, files in os.walk(path):
+ for filename in files:
+ os.remove(os.path.join(dirpath, filename))
+ for dirpath, subdirs, _ in os.walk(path, topdown=False):
+ for dirname in subdirs:
+ os.rmdir(os.path.join(dirpath, dirname))
+ os.rmdir(path)
+
+def cleanup(msg, options, created_dir):
+ """
+ After failure, clean up anything we created. Take a conservative
+ approach: only if we created the output directory do we delete
+ anything.
+ """
+ logging.error(msg)
+
+ if created_dir:
+ try:
+ rmrf(options.output_dir)
+ 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)
@@ -131,12 +162,15 @@ def main():

vmdef.arch = options.arch

+ created_dir = False
unixname = vmdef.name.replace(" ", "-")
+
if not options.output_dir:
options.output_dir = unixname
try:
logging.debug("Creating directory %s" % options.output_dir)
os.mkdir(options.output_dir)
+ created_dir = True
except OSError, e:
if (e.errno != errno.EEXIST):
logging.error("Could not create directory %s: %s" %
@@ -156,19 +190,19 @@ def main():
for d in vmdef.disks:
d.convert(options.input_dir, options.output_dir,
vmconfig.DISK_TYPE_RAW)
- except Exception, e:
- logging.error(e)
- sys.exit(1)
+ except OSError, e:
+ cleanup("Couldn't convert disks: %s" % e.strerror, options, created_dir)
+ except RuntimeError, e:
+ cleanup("Couldn't convert disks: %s" % e.message, options, created_dir)

try:
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, created_dir)

print "

Conversion completed and placed in: %s" % options.output_dir

-
if __name__ == "__main__":
try:
main()
diff --git a/virtconv/vmconfig.py b/virtconv/vmconfig.py
--- a/virtconv/vmconfig.py
+++ b/virtconv/vmconfig.py
@@ -77,7 +77,9 @@ class disk(object):
(infile, qemu_formats[output_type],
os.path.join(output_dir, outfile)))

- os.system(convert_cmd)
+ ret = os.system(convert_cmd)
+ if ret != 0:
+ raise RuntimeError("qemu-img failed with exit status %d" % ret)

# Note: this is the *relative* path still
self.path = outfile

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

On Mon, Jul 07, 2008 at 03:51:32PM -0700, john.levon@sun.com wrote:
> # HG changeset patch
> # User john.levon@sun.com
> # Date 1215470371 25200
> # Node ID f0ddebbf39a3926f55045ade4fbf850706ed6c3e
> # Parent c75d57437b7b219443580f6e9d6736d27d76ebcf
> Cleanup on failure
>
> If we can't convert the disks or export the file, perform some cleanup.

ACK

Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

john.levon@sun.com wrote:

Hi John,

Comments inline:

> # HG changeset patch
> # User john.levon@sun.com
> # Date 1215470371 25200
> # Node ID f0ddebbf39a3926f55045ade4fbf850706ed6c3e
> # Parent c75d57437b7b219443580f6e9d6736d27d76ebcf
> Cleanup on failure
>
> If we can't convert the disks or export the file, perform some cleanup.
>
> Signed-off-by: John Levon <john.levon@sun.com>
>
> diff --git a/virt-convert b/virt-convert
> --- a/virt-convert
> +++ b/virt-convert
> @@ -31,6 +31,7 @@ import virtconv.vmconfig as vmconfig
> import virtconv.vmconfig as vmconfig
>
> def parse_args():
> + """Parse and verify command line."""
> opts = OptionParser()
> opts.set_usage("%prog [options] inputdir|input.vmx "
> "[outputdir|output.xml]")
> @@ -93,6 +94,36 @@ def parse_args():
>
> return options
>
> +def rmrf(path):
> + """Remove a directory and all its contents."""
> +
> + assert path is not None
> +
> + for dirpath, _, files in os.walk(path):

You'll want to use a different variable other than '_' since this will
overwrite the gettext function in the local scope. Doesn't cause issues
in the current code, but could with future changes.

> + for filename in files:
> + os.remove(os.path.join(dirpath, filename))
> + for dirpath, subdirs, _ in os.walk(path, topdown=False):
> + for dirname in subdirs:
> + os.rmdir(os.path.join(dirpath, dirname))
> + os.rmdir(path)
> +
> +def cleanup(msg, options, created_dir):
> + """
> + After failure, clean up anything we created. Take a conservative
> + approach: only if we created the output directory do we delete
> + anything.
> + """
> + logging.error(msg)
> +
> + if created_dir:
> + try:
> + rmrf(options.output_dir)
> + except OSError, e:
> + logging.error("Couldn't clean up output directory "%s": %s" %
> + (options.output_dir, e.strerror))
> +
> + sys.exit(1)
> +

Hmm, did you see the comment I made in my response to this original
patch?

"Hmm, this scares the hell out of me, even with the created_dir
check. If someone in the future ever messed up this code we could
accidently start deleting someones homedir.

I think a whitelist approach would be better: have convert_disks clean
up its own files, and keep all other file/directory creation in the
main() function (like it currently is) so it can handle the rest
of the cleanup on a per file basis. At the end, if the output_dir
is empty and we created it, we can delete it. Maybe it's overly
paranoid, but better to be safe than sorry."

Any comment on this?


> def main():
> options = parse_args()
> cli.setupLogging("virt-convert", options.debug)
> @@ -131,12 +162,15 @@ def main():
>
> vmdef.arch = options.arch
>
> + created_dir = False
> unixname = vmdef.name.replace(" ", "-")
> +
> if not options.output_dir:
> options.output_dir = unixname
> try:
> logging.debug("Creating directory %s" % options.output_dir)
> os.mkdir(options.output_dir)
> + created_dir = True
> except OSError, e:
> if (e.errno != errno.EEXIST):
> logging.error("Could not create directory %s: %s" %
> @@ -156,19 +190,19 @@ def main():
> for d in vmdef.disks:
> d.convert(options.input_dir, options.output_dir,
> vmconfig.DISK_TYPE_RAW)
> - except Exception, e:
> - logging.error(e)
> - sys.exit(1)
> + except OSError, e:
> + cleanup("Couldn't convert disks: %s" % e.strerror, options, created_dir)
> + except RuntimeError, e:
> + cleanup("Couldn't convert disks: %s" % e.message, options, created_dir)
>
> try:
> 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, created_dir)
>
> print "

Conversion completed and placed in: %s" % options.output_dir
>
> -
> if __name__ == "__main__":
> try:
> main()
> diff --git a/virtconv/vmconfig.py b/virtconv/vmconfig.py
> --- a/virtconv/vmconfig.py
> +++ b/virtconv/vmconfig.py
> @@ -77,7 +77,9 @@ class disk(object):
> (infile, qemu_formats[output_type],
> os.path.join(output_dir, outfile)))
>
> - os.system(convert_cmd)
> + ret = os.system(convert_cmd)
> + if ret != 0:
> + raise RuntimeError("qemu-img failed with exit status %d" % ret)
>
> # Note: this is the *relative* path still
> self.path = outfile
>

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

On Tue, Jul 08, 2008 at 10:44:42AM -0400, Cole Robinson wrote:

> > + for dirpath, _, files in os.walk(path):
>
> You'll want to use a different variable other than '_' since this will
> overwrite the gettext function in the local scope. Doesn't cause issues
> in the current code, but could with future changes.

Hmm, OK. _ is pretty standard for "don't care", what would you prefer?

> > +def cleanup(msg, options, created_dir):
> > + """
> > + After failure, clean up anything we created. Take a conservative
> > + approach: only if we created the output directory do we delete
> > + anything.
> > + """
> > + logging.error(msg)
> > +
> > + if created_dir:
> > + try:
> > + rmrf(options.output_dir)
> > + except OSError, e:
> > + logging.error("Couldn't clean up output directory "%s": %s" %
> > + (options.output_dir, e.strerror))
> > +
> > + sys.exit(1)
> > +
>
> Hmm, did you see the comment I made in my response to this original
> patch?

No, I don't think so.

> I think a whitelist approach would be better: have convert_disks clean
> up its own files, and keep all other file/directory creation in the
> main() function (like it currently is) so it can handle the rest
> of the cleanup on a per file basis. At the end, if the output_dir
> is empty and we created it, we can delete it. Maybe it's overly
> paranoid, but better to be safe than sorry."
>
> Any comment on this?

Seems sensible, yes.

regards,
john

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

John Levon wrote:
> On Tue, Jul 08, 2008 at 10:44:42AM -0400, Cole Robinson wrote:
>
>>> + for dirpath, _, files in os.walk(path):
>> You'll want to use a different variable other than '_' since this will
>> overwrite the gettext function in the local scope. Doesn't cause issues
>> in the current code, but could with future changes.
>
> Hmm, OK. _ is pretty standard for "don't care", what would you prefer?

No preference: 'ignore', 'unused', anything reasonable and consistent.

- Cole

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

# HG changeset patch
# User john.levon@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@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.inpu t_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.outp ut_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" />
""" %
(path, ascii_letters[number % 26]))
storage_disks.append(
- """<disk file="%s" use="system" format="raw"/>
""" % path)
+ """<disk file="%s" use="system" format="%s"/>
"""
+ % (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

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools