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

[John Levon]

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.

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...

+        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?

 
 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?

regards
john