[et-mgmt-tools] [PATCH] virtinst - virt-convert vmware output
Joey Boggs
jboggs at redhat.com
Mon Sep 29 11:19:26 UTC 2008
Made all suggested changes and cleaned up the extra whitespace
Cole Robinson wrote:
> Joseph Boggs wrote:
>
>
>> virt-pack was creating a wrapper file for each disk rather than
>> converting it. If we were to convert an image once and then try to
>> reconvert that image to another format it will fail based on that
>> wrapper file not being the actual disk. We can write an exception to
>> handle this. It works perfectly in vmware to redirect it to the raw file
>> with the appropriate properties but conflicts with qemu-img for now. It
>> was more of a way to package multiple configurations using the same disk
>> image file.
>>
>> Rediffed against current changeset 600 and conflicts resolved with the
>> vmware to vmx changes reverted.
>>
>>
>
> Thanks.
>
>
>> diff -r 58a909b4f71c virt-convert
>> --- a/virt-convert Mon Sep 22 11:32:11 2008 -0400
>> +++ b/virt-convert Tue Sep 23 11:53:19 2008 -0400
>> @@ -48,10 +48,9 @@
>> opts.add_option("-d", "--debug", action="store_true", dest="debug",
>> help=("Print debugging information"))
>> opts.add_option("-i", "--input-format", action="store",
>> - dest="input_format", help=("Input format, e.g. 'vmx'"))
>> + dest="input_format", help=("Input format, e.g. 'vmx / virt-image'"))
>> opts.add_option("-o", "--output-format", action="store",
>> - dest="output_format", default="virt-image",
>> - help=("Output format, e.g. 'virt-image'"))
>> + dest="output_format", help=("Output format, e.g. 'virt-image / vmx'"))
>> opts.add_option("-D", "--disk-format", action="store",
>> dest="disk_format", help=("Output disk format"))
>> opts.add_option("-v", "--hvm", action="store_true", dest="fullvirt",
>> @@ -211,11 +210,13 @@
>> try:
>> for d in vmdef.disks.values():
>> format = options.disk_format
>> -
>> # VMDK disks on Solaris converted to vdisk by default
>> if (d.format == diskcfg.DISK_FORMAT_VMDK and
>> not format and vmcfg.host() == "SunOS"):
>> format = "vdisk"
>> +
>> + if options.output_format == "vmx":
>> + format = "vmdk"
>>
>>
>
> Hmm, I'm not sure if this is correct wrt the above
> code piece. Make this piece an elif with the block
> above it.
>
> Also, does vmx _only_ read vmdk files? If that's not
> the case, we shouldn't overwrite whatever the specified
> format may have been, just default to it if none is
> specified.
>
>
>
>> if not format:
>> format = "raw"
>> diff -r 58a909b4f71c virtconv/diskcfg.py
>> --- a/virtconv/diskcfg.py Mon Sep 22 11:32:11 2008 -0400
>> +++ b/virtconv/diskcfg.py Tue Sep 23 11:53:19 2008 -0400
>> @@ -181,7 +181,7 @@
>> absout = os.path.join(outdir, relout)
>>
>> if not (out_format == DISK_FORMAT_VDISK or
>> - out_format == DISK_FORMAT_RAW):
>> + out_format == DISK_FORMAT_RAW or out_format == DISK_FORMAT_VMDK):
>> raise NotImplementedError("Cannot convert to disk format %s" %
>> output_format)
>>
>> diff -r 58a909b4f71c virtconv/formats.py
>> --- a/virtconv/formats.py Mon Sep 22 11:32:11 2008 -0400
>> +++ b/virtconv/formats.py Tue Sep 23 11:53:19 2008 -0400
>> @@ -44,6 +44,7 @@
>> Import a configuration file. Raises if the file couldn't be
>> opened, or parsing otherwise failed.
>> """
>> + print "importing file"
>> raise NotImplementedError
>>
>>
>
> Accidentally left in? :)
>
>
>> @staticmethod
>> diff -r 58a909b4f71c virtconv/parsers/virtimage.py
>> --- a/virtconv/parsers/virtimage.py Mon Sep 22 11:32:11 2008 -0400
>> +++ b/virtconv/parsers/virtimage.py Tue Sep 23 11:53:19 2008 -0400
>> @@ -21,10 +21,13 @@
>> import virtconv.formats as formats
>> import virtconv.vmcfg as vmcfg
>> import virtconv.diskcfg as diskcfg
>> +import virtconv.netdevcfg as netdevcfg
>> import virtinst.FullVirtGuest as fv
>> -
>> +import virtinst.ImageParser as ImageParser
>> +from virtinst.cli import fail
>> from xml.sax.saxutils import escape
>> from string import ascii_letters
>> +import os
>> import re
>>
>> pv_boot_template = """
>> @@ -75,6 +78,10 @@
>> </storage>
>> </image>
>> """
>> +
>> +def parse_disk_entry(vm, fullkey, value):
>> + return
>> +
>>
>>
>
> Hmm, if this isn't doing anything, it doesn't need to be
> present. It isn't an explicit part of the API.
>
>
>> def export_os_params(vm):
>> """
>> @@ -183,16 +190,23 @@
>> """
>> name = "virt-image"
>> suffix = ".virt-image.xml"
>> - can_import = False
>> + can_import = True
>> can_export = True
>> - can_identify = False
>> + can_identify = True
>>
>> @staticmethod
>> def identify_file(input_file):
>> """
>> Return True if the given file is of this format.
>> + Not able to guarantee <image> is on the top line rather than a blankline
>> """
>> - raise NotImplementedError
>> + infile = open(input_file, "r")
>> + line = infile
>> + for line in line.readlines():
>> + if re.match(r'^<image>', line):
>> + return True
>> + infile.close()
>> +
>>
>>
>
> No thoughts on my previous comment? :
>
> 'Hmm, might it be better just to do ImageParser.parse_file here,
> and if it throws an exception, log it and return False?'
>
> I think that's the way to go.
>
>
>> @staticmethod
>> def import_file(input_file):
>> @@ -200,7 +214,48 @@
>> Import a configuration file. Raises if the file couldn't be
>> opened, or parsing otherwise failed.
>> """
>> - raise NotImplementedError
>> + vm = vmcfg.vm()
>> + try:
>> + config = ImageParser.parse_file(input_file)
>> + except Exception, e:
>> + fail("Couldn't import file \"%s\": %s" % (input_file, e))
>> +
>>
>
> I mentioned in a response to your other patch, but skip
> the quote escaping, just use single quotes.
>
>
>> + domain = config.domain
>> + boot = domain.boots[0]
>> +
>> + if not config.name:
>> + raise ValueError("No Name defined in \"%s\"" % input_file)
>> + vm.name = config.name
>> + vm.memory = config.domain.memory
>> + if config.descr:
>> + vm.description = config.descr
>> + vm.nr_vcpus = config.domain.vcpu
>> +
>> + bus="scsi"
>> + diskinst = 0
>> + devid = (bus, diskinst)
>> +
>> + for d in boot.drives:
>> + disk = d.disk
>> + if disk.format == "raw":
>> + disk.format = diskcfg.DISK_FORMAT_RAW
>> + else:
>> + raise ValueError("%s is not a supported disk format" % disk.format)
>> +
>>
>
> The 'Disk' class also supports vmdk, which is supported
> by diskcfg, so you probably want to allow that as well.
> Also, compare against the Disk class constants, not
> their values (eg. use FORMAT_RAW and FORMAT_VMDK)
>
>
>> + vm.disks[devid] = diskcfg.disk(bus = bus,
>> + type = diskcfg.DISK_TYPE_DISK)
>> + vm.disks[devid].format = disk.format
>> + vm.disks[devid].path = disk.file
>> + diskinst = diskinst + 1
>> +
>> + nics = domain.interface
>> + nicinst = 0
>> + while nicinst < nics:
>> + vm.netdevs[nicinst] = netdevcfg.netdev(type = netdevcfg.NETDEV_TYPE_UNKNOWN)
>> + nicinst = nicinst + 1
>> + """ Eventually need to add support for mac addresses if given"""
>> + vm.validate()
>> + return vm
>>
>> @staticmethod
>> def export_file(vm, output_file):
>> diff -r 58a909b4f71c virtconv/parsers/vmx.py
>> --- a/virtconv/parsers/vmx.py Mon Sep 22 11:32:11 2008 -0400
>> +++ b/virtconv/parsers/vmx.py Tue Sep 23 11:53:19 2008 -0400
>> @@ -22,9 +22,105 @@
>> import virtconv.vmcfg as vmcfg
>> import virtconv.diskcfg as diskcfg
>> import virtconv.netdevcfg as netdevcfg
>> -
>> +import time
>> +import sys
>> import re
>> import os
>> +
>> +_VMX_MAIN_TEMPLATE = """
>> +#!/usr/bin/vmplayer
>> +
>> +# Generated %(now)s by %(progname)s
>> +# http://virt-manager.et.redhat.com/
>> +
>> +# This is a Workstation 5 or 5.5 config file
>> +# It can be used with Player
>> +config.version = "8"
>> +virtualHW.version = "4"
>> +
>> +# Selected operating system for your virtual machine
>> +guestOS = "other"
>> +
>> +# displayName is your own name for the virtual machine
>> +displayName = "%(/image/name)s"
>> +
>> +# These fields are free text description fields
>> +annotation = "%(/image/description)s"
>> +guestinfo.vmware.product.long = "%(/image/label)s"
>> +guestinfo.vmware.product.url = "http://virt-manager.et.redhat.com/"
>> +guestinfo.vmware.product.class = "virtual machine"
>> +
>> +# Number of virtual CPUs. Your virtual machine will not
>> +# work if this number is higher than the number of your physical CPUs
>> +numvcpus = "%(/image/devices/vcpu)s"
>> +
>> +# Memory size and other memory settings
>> +memsize = "%(/image/devices/memory)d"
>> +MemAllowAutoScaleDown = "FALSE"
>> +MemTrimRate = "-1"
>> +
>> +# Unique ID for the virtual machine will be created
>> +uuid.action = "create"
>> +
>> +## For appliances where tools are installed already, this should really
>> +## be false, but we don't have that ionfo in the metadata
>> +# Remind to install VMware Tools
>> +# This setting has no effect in VMware Player
>> +tools.remindInstall = "TRUE"
>> +
>> +# Startup hints interfers with automatic startup of a virtual machine
>> +# This setting has no effect in VMware Player
>> +hints.hideAll = "TRUE"
>> +
>> +# Enable time synchronization between computer
>> +# and virtual machine
>> +tools.syncTime = "TRUE"
>> +
>> +# First serial port, physical COM1 is not available
>> +serial0.present = "FALSE"
>> +
>> +# Optional second serial port, physical COM2 is not available
>> +serial1.present = "FALSE"
>> +
>> +# First parallell port, physical LPT1 is not available
>> +parallel0.present = "FALSE"
>> +
>> +# Logging
>> +# This config activates logging, and keeps last log
>> +logging = "TRUE"
>> +log.fileName = "%(/image/name)s.log"
>> +log.append = "TRUE"
>> +log.keepOld = "3"
>> +
>> +# These settings decides interaction between your
>> +# computer and the virtual machine
>> +isolation.tools.hgfs.disable = "FALSE"
>> +isolation.tools.dnd.disable = "FALSE"
>> +isolation.tools.copy.enable = "TRUE"
>> +isolation.tools.paste.enabled = "TRUE"
>> +
>> +# Settings for physical floppy drive
>> +floppy0.present = "FALSE"
>> +"""
>> +
>> +
>> +
>>
>
> Trim a couple of these lines.
>
>
>> +_VMX_ETHERNET_TEMPLATE = """
>> +ethernet%(dev)s.present = "TRUE"
>> +ethernet%(dev)s.connectionType = "nat"
>> +ethernet%(dev)s.addressType = "generated"
>> +ethernet%(dev)s.generatedAddressOffset = "0"
>> +ethernet%(dev)s.autoDetect = "TRUE"
>> +"""
>> +
>> +_VMX_IDE_TEMPLATE = """
>> +# IDE disk
>> +ide%(dev)s.present = "TRUE"
>> +ide%(dev)s.fileName = "%(disk_filename)s"
>> +ide%(dev)s.mode = "persistent"
>> +ide%(dev)s.startConnected = "TRUE"
>> +ide%(dev)s.writeThrough = "TRUE"
>> +"""
>>
>> def parse_netdev_entry(vm, fullkey, value):
>> """
>> @@ -70,14 +166,13 @@
>>
>> # Does anyone else think it's scary that we're still doing things
>> # like this?
>> +
>> if bus == "ide":
>> inst = int(inst) + int(bus_nr) * 2
>> -
>> devid = (bus, inst)
>> if not vm.disks.get(devid):
>> vm.disks[devid] = diskcfg.disk(bus = bus,
>> type = diskcfg.DISK_TYPE_DISK)
>> -
>> if key == "devicetype":
>> if lvalue == "atapi-cdrom" or lvalue == "cdrom-raw":
>> vm.disks[devid].type = diskcfg.DISK_TYPE_CDROM
>> @@ -100,7 +195,7 @@
>> name = "vmx"
>> suffix = ".vmx"
>> can_import = True
>> - can_export = False
>> + can_export = True
>> can_identify = True
>>
>> @staticmethod
>> @@ -160,7 +255,7 @@
>> for devid, disk in vm.disks.iteritems():
>> if disk.type == diskcfg.DISK_TYPE_DISK:
>> continue
>> -
>> +
>> # vmx files often have dross left in path for CD entries
>> if (disk.path is None
>> or disk.path.lower() == "auto detect" or
>> @@ -180,6 +275,8 @@
>>
>> @staticmethod
>> def export_file(vm, output_file):
>> +
>> +
>> """
>> Export a configuration file.
>> @vm vm configuration instance
>>
>
> Hmm, the 3/4 of the previous changes are all whitespace.
> Please try to minimize these in the future, they just
> balloon patch size and generally add nothing of value.
>
>
>
>> @@ -189,6 +286,56 @@
>> exception on failure to write the output file.
>> """
>>
>> - raise NotImplementedError
>> + """ Cleanup spaces and multiple lines"""
>> + vm.description = vm.description.strip()
>> + vm.description = vm.description.replace("\n","|")
>> + vmx_out_template = []
>> + vmx_dict = {
>> + "now": time.strftime("%Y-%m-%dT%H:%M:%S %Z", time.localtime()),
>> + "progname": os.path.basename(sys.argv[0]),
>> + "/image/name": vm.name,
>> + "/image/description": vm.description or "None",
>> + "/image/label": vm.name,
>> + "/image/devices/vcpu" : vm.nr_vcpus,
>> + "/image/devices/memory": long(vm.memory)/1024
>> + }
>> + vmx_out = _VMX_MAIN_TEMPLATE % vmx_dict
>> + vmx_out_template.append(vmx_out)
>> +
>> + disk_out_template = []
>> + diskcount = 0
>> + for disk in vm.disks:
>> + ide_count = 0
>> + dev = "%d:%d" % (ide_count / 2, ide_count % 2)
>> + disk_dict = {
>> + "dev": dev,
>> + "disk_filename" : vm.disks[disk].path
>> + }
>> + disk_out = _VMX_IDE_TEMPLATE % disk_dict
>> + disk_out_template.append(disk_out)
>> + diskcount = diskcount + 1
>> +
>> + eth_out_template = []
>> + if len(vm.netdevs):
>> + for devnum in vm.netdevs:
>> + eth_dict = {
>> + "dev" : devnum
>> + }
>> + eth_out = _VMX_ETHERNET_TEMPLATE % eth_dict
>> + eth_out_template.append(eth_out)
>> +
>> +
>> +
>> +
>> +
>> +
>> +### write out generated templates, will sequence these better so we can write inline ###
>> + outfile = open(output_file, "w")
>> + outfile.writelines(vmx_out_template)
>> + outfile.writelines(disk_out_template)
>> + outfile.writelines(eth_out_template)
>> + outfile.close()
>> +
>> + #raise NotImplementedError
>>
>
> Hmm, why all the extra newlines and strangely indented comment?
>
> Thanks,
> Cole
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: virt-convert-vmware-conversion-module-9-29-1115.patch
Type: text/x-patch
Size: 10030 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/et-mgmt-tools/attachments/20080929/39290b1b/attachment.bin>
More information about the et-mgmt-tools
mailing list