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@xxxxxxxxxx
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)
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
