[et-mgmt-tools] [PATCH] virtinst - virt-convert vmware output

Joey Boggs jboggs at redhat.com
Wed Sep 24 20:08:30 UTC 2008


Got that patch file mixed with another codebase I was playing with, I'll 
work on getting those changes out, as for the extra whitespace, that was 
copy/paste from Unware.py I'll clean those up as well and work on the 
other changes


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
>   




More information about the et-mgmt-tools mailing list