[et-mgmt-tools] [PATCH] convert vmware machines to virt-image xml format
Joey Boggs
jboggs at redhat.com
Thu Jun 26 20:42:16 UTC 2008
I integrated the common cli options and all the other modifications, but
wasn't sure the best way to approach the para/full virt check removal
from the comments
"If you use the above --paravirt/--hvm option change, you won't need
this check." Both before and after code snips below.
I changed the if/else to evaluate the options for True on either
para/full, if there's something else you'd want to see please let me know
-------------------------------------------------------------------------------
parser.add_option("-v", "--hvm", action="store_true", dest="fullvirt",
help=("This guest should be a fully virtualized
guest"))
parser.add_option("-p", "--paravirt", action="store_true",
dest="paravirt",
help=("This guest should be a paravirtualized guest"))
if options.paravirt is True:
virt_boot_template = """<boot type="xen">
<guest>
<arch>%(vm_arch)s</arch>
<features>
<pae/>
</features>
</guest>
<os>
<loader>pygrub</loader>
</os>
%(vm_pv_disks)s
</boot>"""
elif options.fullvirt is True:
virt_boot_template = """<boot type="hvm">
<guest>
<arch>%(vm_arch)s</arch>
</guest>
<os>
<loader dev="hd"/>
</os>
%(vm_fv_disks)s
</boot>"""
else:
print "Invalid virtualization type specified"
sys.exit(1)
-------------------------------------------------------------------------------
> > + if options.type == "paravirt":
> > + virt_boot_template = """<boot type="xen">
> > + <guest>
> > + <arch>%(vm_arch)s</arch>
> > + <features>
> > + <pae/>
> > + </features>
> > + </guest>
> > + <os>
> > + <loader>pygrub</loader>
> > + </os>
> > + %(vm_pv_disks)s
> > + </boot>"""
> > + elif options.type == "fullvirt":
> > +
> > + virt_boot_template = """<boot type="hvm">
> > + <guest>
> > + <arch>%(vm_arch)s</arch>
> > + </guest>
> > + <os>
> > + <loader dev="hd"/>
> > + </os>
> > + %(vm_fv_disks)s
> > + </boot>"""
> > + else:
> > + print "Invalid virtualization type specified"
> > + sys.exit(1)
If you use the above --paravirt/--hvm option change, you won't need
this check.
Regards,
Joey
Cole Robinson wrote:
> Joey Boggs wrote:
>
>
>>> These patches provide the virt-unpack command which converts vmware
>>> format machines into virt-image xml format hvm/paravirt runnable
>>> instances. Next revision will contain libvirt format as well. As also
>>> with the virt-pack command all required vmware/xen/hvm kernel modules
>>> must be in place prior to conversion to boot machine.
>>> ------------------------------------------------------------------------
>>>
>>>
>
>
> Comments inline:
>
>
>> diff -Naur virtinst--devel.orig/virt-unpack virtinst--devel/virt-unpack
>> --- virtinst--devel.orig/virt-unpack 1969-12-31 19:00:00.000000000 -0500
>> +++ virtinst--devel/virt-unpack 2008-06-26 10:30:45.000000000 -0400
>> @@ -0,0 +1,231 @@
>> +#!/usr/bin/python
>> +#
>> +# Convert a VMware(tm) virtual machine into an XML image description
>> +#
>> +# Copyright 2008 Red Hat, Inc.
>> +# Joey Boggs <jboggs at redhat.com>
>> +#
>> +# 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 sys
>> +from string import ascii_letters
>> +import virtinst.cli as cli
>> +import os
>> +import random
>> +import pdb
>> +import shutil
>> +import logging
>> +from optparse import OptionParser, OptionValueError
>> +
>> +def parse_args():
>> + parser = OptionParser()
>> + parser.set_usage("%prog [options] image.vmx")
>> + parser.add_option("-a", "--arch", type="string", dest="arch",
>> + help=("Machine Architecture Type (i686/x86_64/ppc)"))
>> + parser.add_option("-t", "--type", type="string", dest="type",
>> + help=("Output virtualization type (hvm, paravirt"))
>>
>
> For type I would use the convention that virt-install uses: add seperate
> options for --paravirt and --hvm (optionally -p or -v). Also I'd recommend
> just defaulting to --hvm since that is the most common case at this point.
>
>
>> + parser.add_option("-d", "--debug", action="store_true", dest="debug",
>> + help=("Print debugging information"))
>> + parser.add_option("-o", "--outputdir", type="string", dest="outputdir",
>> + help=("Output directory in which files will be created"))
>> + (options,args) = parser.parse_args()
>> +
>> + if len(args) < 1:
>> + parser.error(("You need to provide an image XML descriptor"))
>> + options.file = args[0]
>> +
>> + if (options.arch is None) or (options.type is None):
>> + parser.error(("You need to provide all input options"
>> + "\n\nArchitecture: " + str(options.arch) +
>> + "\nOutput Virt Type: " + str(options.type)))
>> + sys.exit(1)
>>
>
> Won't need sys.exit here since parser.error will handle this for you.
>
>
>> + return options
>> +
>> +# Begin creation of xml template from parsed vmx config file
>> +def create_xml_template(disks_list,record,options,dirname):
>>
>
> Kind of a nit, but create_xml_template isn't really descriptive.
> convert_to_image? vmx_to_image?
>
>
>> + pv_disk_list = []
>> + fv_disk_list = []
>> + storage_disk_list = []
>> +
>> + name = record["displayName"]
>> + memory = record["memsize"]
>> + memory = int(memory) * 1024
>> +
>> + # can't guarantee annotation or vcpus are set in vmware config, requires evaluation
>> + if record.has_key("annotation"):
>> + annotation = record["annotation"]
>> + else:
>> + annotation = ""
>> +
>> + if record.has_key("numvcpus"):
>> + vcpus = record["numvcpus"]
>> + else:
>> + vcpus = "1"
>> +
>> +# create disk filename lists for xml template
>> + for (number, file) in enumerate(disks_list):
>> + file = str(file.replace(".vmdk","")).strip()
>> + pv_disk_list.append("""<drive disk="%s.img" target="xvd%s"/>""" % \
>> + (file, ascii_letters[number % 26]))
>> + fv_disk_list.append("""<drive disk="%s.img" target="hd%s"/>""" % \
>> + (file, ascii_letters[number % 26]))
>> + storage_disk_list.append("""<disk file="%s.img" use="system" format="raw"/>""" % (file))
>> +# determine virtualization type for image.boot section
>> + if options.type == "paravirt":
>> + virt_boot_template = """<boot type="xen">
>> + <guest>
>> + <arch>%(vm_arch)s</arch>
>> + <features>
>> + <pae/>
>> + </features>
>> + </guest>
>> + <os>
>> + <loader>pygrub</loader>
>> + </os>
>> + %(vm_pv_disks)s
>> + </boot>"""
>> + elif options.type == "fullvirt":
>> +
>> + virt_boot_template = """<boot type="hvm">
>> + <guest>
>> + <arch>%(vm_arch)s</arch>
>> + </guest>
>> + <os>
>> + <loader dev="hd"/>
>> + </os>
>> + %(vm_fv_disks)s
>> + </boot>"""
>> + else:
>> + print "Invalid virtualization type specified"
>> + sys.exit(1)
>>
>
> If you use the above --paravirt/--hvm option change, you won't need
> this check.
>
>
>> +
>> +
>> +# xml replacements dictionaries
>> + virt_boot_xml_dict = {
>> + "vm_pv_disks" : "".join(pv_disk_list),
>> + "vm_fv_disks" : "".join(fv_disk_list),
>> + "vm_arch" : options.arch,
>> + }
>>
>
> If you do the above --paravirt/--hvm change, you can then just pass
> options.arch directly to the function rather than the whole options
> list.
>
>
>> + virt_boot_template = virt_boot_template % virt_boot_xml_dict
>> +
>> + virt_image_xml_dict = {
>> + "virt_boot_template" : virt_boot_template,
>> + "vm_displayName": name.replace(" ","_"),
>> + "vm_annotation" : annotation,
>> + "vm_vcpus" : vcpus,
>> + "vm_mem" : memory,
>> + "vm_storage" : "".join(storage_disk_list),
>> + }
>> +
>> + virt_image_xml_template = """<image>
>> + <name>%(vm_displayName)s</name>
>> + <label>%(vm_displayName)s</label>
>> + <description>
>> + %(vm_annotation)s
>> + </description>
>> + <domain>
>> + %(virt_boot_template)s
>> + <devices>
>> + <vcpu>%(vm_vcpus)s</vcpu>
>> + <memory>%(vm_mem)s</memory>
>> + <interface/>
>> + <graphics/>
>> + </devices>
>> + </domain>
>> + <storage>
>> + %(vm_storage)s
>> + </storage>
>> +</image>"""
>> +
>> + virt_image_xml_template = virt_image_xml_template % virt_image_xml_dict
>> + virt_image_xml_config_file = open(dirname + "/" + name + ".virtimage.xml","w")
>> + virt_image_xml_config_file.writelines(virt_image_xml_template)
>> + virt_image_xml_config_file.close()
>>
>
> I'd recommend moving this end piece to the main function. If keeps the
> conversion function doing only the conversion. If we ever wanted to reuse
> this code in another app which won't be writing an output file, this would
> be a necessary change.
>
>
>> + return
>> +
>> +# parse input vmware configuration
>> +def parse_vmware_config(options):
>> + if not os.access(options.file,os.R_OK):
>> + raise ValueError, "Could not read file: %s" % options.file
>> + input = open(options.file,"r")
>> + contents = input.readlines()
>> + input.close()
>> + record = {}
>> + vm_config = []
>> + disks_list = []
>> +
>> +# strip out comment and blank lines for easy splitting of values
>> + for line in contents:
>> + if not line.strip() or line.startswith("#"):
>> + continue
>> + else:
>> + vm_config.append(line)
>> +
>> + for line in vm_config:
>> + beforeEq, afterEq = line.split("=", 1)
>> + key = beforeEq.replace(" ","")
>> + value = afterEq.replace('"',"")
>> + record[key] = value.strip()
>> + logging.debug("Key: %s Value: %s" % (key,value))
>> + if value.endswith("vmdk\n"): # separate disks from config
>> + disks_list.append(value)
>> + return vm_config,record,disks_list,options
>>
>
> Shouldn't need to return vm_config or options here: options hasn't changed,
> vm_config isn't used anywhere else.
>
>
>> +
>> +
>> +def convert_disks(disks_list,name,dirname):
>>
>
> Name isn't used here anymore.
>
>
>> + for disk in disks_list:
>> + file = disk.replace(".vmdk","").strip()
>> + convert_cmd="qemu-img convert %s -O raw %s/%s.img" % (disk.strip(),dirname,file)
>> + logging.debug("Converting %s" % disk.strip())
>> + print "\nConverting %s to %s/%s.img" % (disk.strip(),dirname,file)
>> + os.system(convert_cmd)
>> +
>> +
>> +
>> +def main():
>> + options = parse_args()
>> + cli.setupLogging("virt-unpack", options.debug)
>> + vm_config = parse_vmware_config(options)
>> + config, record, disks_list, options = vm_config
>> + name = record["displayName"].replace(" ","-")
>>
>
> Just thinking, there are a lot of checks here directly into the record. If
> we are passed a vmx file that is missing one of these options, either
> through a bad config or some change in the format, we are going to throw
> an undescriptive error like "Key error: displayName". Maybe create a small
> wrapper function like record_lookup that you pass the record and the key
> to, and will throw a more clear error to the user like "key 'blah' was
> not parsed from the vmx config"
>
>
>> + dirname = options.outputdir
>> + if not dirname:
>> + dirname = name
>> + try:
>> + logging.debug ("Creating directory %s" % dirname)
>> + os.mkdir(dirname)
>> + except OSError,e:
>> + if e.errno == 17:
>> + logging.error("Could not create directory %s or directory already exists", dirname)
>> + sys.exit(1)
>>
>
> You'll want to exit here regardless of the errno. I'd recommend just losing
> the errno check, and use something general like:
> logging.error("Could not create directory '%s': %s" % (dirname, str(e))
> sys.exit(1)
>
> Also, if you move the file creation aspect out of the create_xml_template,
> you can move the dir creation after the create_xml_template call. That
> way we move the dir creation as late as possible so that if there is
> an exception earlier in the process we don't need to clean up/delete
> the unpopulated directory.
>
>> +
>> + create_xml_template(disks_list,record,options,dirname)
>> + convert_disks(disks_list,name,dirname)
>> +
>> + print "\n\nConversion completed and placed in: %s" % dirname
>> +
>> +
>> +if __name__ == "__main__":
>> + try:
>> + main()
>> + except SystemExit, e:
>> + sys.exit(e.code)
>> + except KeyboardInterrupt, e:
>> + print >> sys.stderr, _("Aborted at user request")
>> + except Exception, e:
>> + logging.exception(e)
>> + sys.exit(1)
>> +
>>
>
> Thanks,
> Cole
>
>
More information about the et-mgmt-tools
mailing list