[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



comments inline, all issues addressed other than vmdk/scsi




John Levon wrote:
On Mon, Sep 29, 2008 at 07:19:26AM -0400, Joey Boggs wrote:

Made all suggested changes and cleaned up the extra whitespace

Cool beans. Some comments:

diff -r 58a909b4f71c virt-convert
--- a/virt-convert	Mon Sep 22 11:32:11 2008 -0400
+++ b/virt-convert	Mon Sep 29 07:17:09 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'"))

These are just examples, so why list all the formats? The existing ones
should be fine.

reverting changes
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	Mon Sep 29 07:17:09 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

Surely this isn't right - this code is "library" code and shouldn't be
using fail() ? It should be re-raising the exception...

Not sure about this, all the other apps in virtinst use fail() now in exceptions even virt-convert, or am I misunderstanding something?
+        bus="scsi"

Inconsistent spacing.

+        diskinst = 0

Isn't nr_disks a little more understandable?

+        devid = (bus, diskinst)

This line...

+
+        for d in boot.drives:
+            disk = d.disk
+            format = None
+            if disk.format == ImageParser.Disk.FORMAT_RAW:
+                format = diskcfg.DISK_FORMAT_RAW
+            elif format == ImageParser.DISK_FORMAT_VMDK:
+                format == diskcfg.DISK_FORMAT_VMDK
+ + if format is None:
+                raise ValueError("Unable to determine disk format")
+
... should surely be here?

+            vm.disks[devid] = diskcfg.disk(bus = bus,
+                type = diskcfg.DISK_TYPE_DISK)

No CD-ROM handling?

+        nics = domain.interface
+        nicinst = 0

nr_nics? :)

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	Mon Sep 29 07:17:09 2008 -0400
+_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"
+"""

Hmm, above we're importing virt-image as SCSI disks, but exporting as
IDE - can you clarify this?
We can't export as scsi without qemu-img vmdk scsi support. It's in the wild and in opensuse but not upstream nor Fedora. I'll file a bugzilla on our side for it. When we add more formats the vmx ide exception will impact them if we change how they are imported on the virt-image side. Can't we import them and export them as we have available? Once we get both ide/scsi vmdk support we can identify and export in either needed format.

def parse_netdev_entry(vm, fullkey, value):
     """
@@ -70,14 +123,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

I don't see these whitespace changes as improvements...

@@ -180,6 +232,8 @@
@staticmethod
     def export_file(vm, output_file):
+
+

ditto.

@@ -188,7 +242,47 @@
         Raises ValueError if configuration is not suitable, or another
         exception on failure to write the output file.
         """
+        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

It's not clear why you used such strange names for the dict's keywords?
that was copy/past from virt-pack, all updated to new values
regards
john

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

diff -r 58a909b4f71c virt-convert
--- a/virt-convert	Mon Sep 22 11:32:11 2008 -0400
+++ b/virt-convert	Mon Sep 29 15:26:22 2008 -0400
@@ -217,6 +217,8 @@
                 not format and vmcfg.host() == "SunOS"):
                 format = "vdisk"
 
+            elif options.output_format == "vmx":
+                format = "vmdk"
             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	Mon Sep 29 15:26:22 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/parsers/virtimage.py
--- a/virtconv/parsers/virtimage.py	Mon Sep 22 11:32:11 2008 -0400
+++ b/virtconv/parsers/virtimage.py	Mon Sep 29 15:26:22 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 = """
@@ -183,16 +186,20 @@
     """
     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.
         """
-        raise NotImplementedError
+        try:
+            image = ImageParser.parse_file(input_file)
+        except ImageParser.ParserException, msg:
+            return False
+        return True
 
     @staticmethod
     def import_file(input_file):
@@ -200,7 +207,52 @@
         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))
+
+        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"
+        nr_disks = 0
+        devid = (bus, nr_disks)
+
+        for d in boot.drives:
+            disk = d.disk
+            format = None
+            if disk.format == ImageParser.Disk.FORMAT_RAW:
+                format = diskcfg.DISK_FORMAT_RAW
+            elif format == ImageParser.DISK_FORMAT_VMDK:
+                format == diskcfg.DISK_FORMAT_VMDK
+
+            if format is None:
+                raise ValueError("Unable to determine disk format")
+            devid = (bus, nr_disks)
+            vm.disks[devid] = diskcfg.disk(bus = bus,
+                type = diskcfg.DISK_TYPE_DISK)
+            vm.disks[devid].format = format
+            vm.disks[devid].path = disk.file
+            nr_disks = nr_disks + 1
+           
+        nics = domain.interface
+        nr_nics = 0
+        while nr_nics < nics:
+            vm.netdevs[nr_nics] = netdevcfg.netdev(type = netdevcfg.NETDEV_TYPE_UNKNOWN)
+            nr_nics = nr_nics + 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	Mon Sep 29 15:26:22 2008 -0400
@@ -22,9 +22,62 @@
 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 and can be used with Player
+config.version = "8"
+virtualHW.version = "4"
+guestOS = "other"
+displayName = "%(vm_name)s"
+annotation = "%(vm_description)s"
+guestinfo.vmware.product.long = "%(vm_name)s"
+guestinfo.vmware.product.url = "http://virt-manager.et.redhat.com/";
+guestinfo.vmware.product.class = "virtual machine"
+numvcpus = "%(vm_nr_vcpus)s"
+memsize = "%(vm_memory)d"
+MemAllowAutoScaleDown = "FALSE"
+MemTrimRate = "-1"
+uuid.action = "create"
+tools.remindInstall = "TRUE"
+hints.hideAll = "TRUE"
+tools.syncTime = "TRUE"
+serial0.present = "FALSE"
+serial1.present = "FALSE"
+parallel0.present = "FALSE"
+logging = "TRUE"
+log.fileName = "%(vm_name)s.log"
+log.append = "TRUE"
+log.keepOld = "3"
+isolation.tools.hgfs.disable = "FALSE"
+isolation.tools.dnd.disable = "FALSE"
+isolation.tools.copy.enable = "TRUE"
+isolation.tools.paste.enabled = "TRUE"
+floppy0.present = "FALSE"
+"""
+_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):
     """
@@ -72,7 +125,7 @@
     # 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,
@@ -100,7 +153,7 @@
     name = "vmx"
     suffix = ".vmx"
     can_import = True
-    can_export = False
+    can_export = True
     can_identify = True
 
     @staticmethod
@@ -160,7 +213,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
@@ -188,7 +241,46 @@
         Raises ValueError if configuration is not suitable, or another
         exception on failure to write the output file.
         """
+        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]),
+            "vm_name": vm.name,
+            "vm_description": vm.description or "None",
+            "vm_nr_vcpus" : vm.nr_vcpus,
+            "vm_memory": long(vm.memory)/1024
+        }
+        vmx_out = _VMX_MAIN_TEMPLATE % vmx_dict
+        vmx_out_template.append(vmx_out)
 
-        raise NotImplementedError
+        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)
+
+        outfile = open(output_file, "w")
+        outfile.writelines(vmx_out_template)
+        outfile.writelines(disk_out_template)
+        outfile.writelines(eth_out_template)
+        outfile.close()
 
 formats.register_parser(vmx_parser)

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]