Joey Boggs wrote:
> These patches were originally written by David Lutterkort and I've
> tweaked the conversion process a little bit to work out some staging issues
>
> virt-pack will tar all the files for the image into a tarball. The name
> and toplevel directory are derived from the name and version of the
> image descriptor. Scratch disks are omitted, since they can be rebuilt
> when the image is deployed.
>
Hi Joey,
I committed all the patches except the actual virt-pack command, since they
were separate and straight forward.
...
> +import os, sys, string
> +from optparse import OptionParser, OptionValueError
> +import subprocess
> +import logging
> +import libxml2
> +import urlgrabber.progress as progress
> +
> +import virtinst
> +import virtinst.ImageParser
> +import virtinst.CapabilitiesParser
> +import virtinst.cli as cli
> +import virtinst.util as util
> +import virtinst.UnWare
> +
> +import tempfile
> +
> +import gettext
> +import locale
> +
> +locale.setlocale(locale.LC_ALL, '')
> +gettext.bindtextdomain(virtinst.gettext_app, virtinst.gettext_dir)
> +gettext.install(virtinst.gettext_app, virtinst.gettext_dir)
> +
> +class PackageException(Exception):
> + def __init__(self, msg):
> + Exception.__init__(self, msg)
> +
> +class Package:
> +
> + def __init__(self, image):
> + self.image = image
> + if image.name is None or image.version is None:
> + raise PackageException(
> + _("The image name and version must be present"))
> + self.vername = "%s-%s" % (image.name, image.version)
> + self.tmpdir = tempfile.mkdtemp(dir="/var/tmp", prefix="virt-pack")
> + self.stagedir = os.path.join(self.tmpdir, self.vername)
> + self.files = []
> +
> + def add_image_files(self):
> + cwd = os.getcwd()
> + img = self.image
> + self.files.append(os.path.basename(img.filename))
> + try:
> + os.chdir(img.base)
> + for d in img.storage.keys():
> + disk = img.storage[d]
> + if disk.use == disk.USE_SCRATCH:
> + if disk.size is None:
> + raise PackageException(_("Scratch disk %s does not have a size attribute") % disk.id)
> + else:
> + if not os.path.exists(disk.file):
> + raise PackageException(_("Disk file %s could not be found") % disk.id)
> + self.files.append(disk.file)
> + finally:
> + os.chdir(cwd)
Why change directories here? Seems like it could all be avoided if the
above check is changed to: os.path.exists(os.path.join(img.base, disk.file))
> +
> + def make_vmx_files(self):
> + img = virtinst.UnWare.Image(self.image)
> + files = img.make(self.image.base)
> + self.files.extend(files)
> +
> + def pack(self, outdir):
> + outfile = os.path.join(outdir, self.vername + ".tgz")
> + for f in set(self.files):
> + dir = os.path.join(self.stagedir, os.path.dirname(f))
> + if not os.path.exists(dir):
> + os.makedirs(dir)
> + os.symlink(os.path.join(self.image.base, f),
> + os.path.join(self.stagedir, f))
> + print "Writing %s" % outfile
> + cmd = "tar chzv -C %s -f %s %s" % (self.tmpdir,
> + outfile,
> + os.path.basename(self.vername))
> + util.system(cmd)
> + return outfile
> +
> +### Option parsing
> +def parse_args():
> + parser = OptionParser()
> + parser.set_usage("%prog [options] image.xml")
> +
> + parser.add_option("-o", "--output", type="string", dest="output",
> + action="callback", callback=cli.check_before_store,
> + help=_("Directory in which packaged file will be put"))
> + parser.add_option("-d", "--debug", action="store_true", dest="debug",
> + help=_("Print debugging information"))
> +
> + (options,args) = parser.parse_args()
> + if len(args) < 1:
> + parser.error(_("You need to provide an image XML descriptor"))
> + options.image = args[0]
> +
> + return options
> +
> +def main():
> + options = parse_args()
> +
> +
> + cli.setupLogging("virt-pack", options.debug)
> +
> + image = virtinst.ImageParser.parse_file(os.path.abspath(options.image))
> +
> + if image.name is None or image.version is None:
> + print >> sys.stderr, _("The image descriptor must contain name and version")
> + valid = False
> +
> + if options.output is None:
> + options.output = os.path.join(image.base, "..")
> +
> + pkg = Package(image)
> + try:
> + pkg.add_image_files()
> + except PackageException, e:
> + print >> sys.stderr, "Validation failed: %s", e
> + return 1
> +
> + try:
> + pkg.make_vmx_files()
> + pkg.pack(options.output)
> + except PackageException, e:
> + print >> sys.stderr, "Packaging failed: %s" % e
> + return 1
> +
Might want to wrap the above two messages in _("...") to mark them as
translatable.
> +if __name__ == "__main__":
> + main()
> +
> diff -r 2f7a50a57c1c virtinst/UnWare.py
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/virtinst/UnWare.py Wed Dec 12 17:53:53 2007 -0800
> @@ -0,0 +1,291 @@
...
> +
> +class Disk:
> + """A disk for a VMWare(tm) virtual machine"""
> +
> + MONOLITHIC_FLAT = "monolithicFlat"
> + TWO_GB_MAX_EXTENT_SPARSE = "twoGbMaxExtentSparse"
> + # This seems only to be usable if the vmdk header is embedded in the
> + # data file, not when the descriptor is in a separate text file. Use
> + # TWO_GB_MAX_EXTENT_SPARSE instead.
> + # VMWare's(tm) documentation of VMDK seriously sucks. A lot.
> + MONOLITHIC_SPARSE = "monolithicSparse"
> +
> + IDE_HEADS = 16
> + IDE_SECTORS = 63
> +
> + def __init__(self, descriptor, extent, size, dev, format):
> + """Create a new disk. DESCRIPTOR is the name of the VMDK descriptor
> + file. EXTENT is the name of the file holding the actual data. SIZE
> + is the filesize in bytes. DEV identifies the device, for IDE (the
> + only one supported right now) it should be $bus:$master. FORMAT is
> + the format of the underlying extent, one of the formats defined in
> + virtinst.ImageParser.Disk"""
> + self.cid = 0xffffffff
> + self.createType = Disk.MONOLITHIC_FLAT
> + self.descriptor = descriptor
> + self.extent = extent
> + self.size = size
> + self.dev = dev
> + self.format = format
> +
> + def make_extent(self, base):
> + """Write the descriptor file, and create the extent as a monolithic
> + sparse extent if it does not exist yet"""
> + f = os.path.join(base, self.extent)
> + print "Checking %s" % f
We shouldn't just straight 'print' from a library, even if only one app is using it.
I suggest using logging.debug.
> + if not os.path.exists(f):
> + util.system("qemu-img create -f vmdk %s %d" % (f, self.size/1024))
> + self.createType = Disk.TWO_GB_MAX_EXTENT_SPARSE
> + else:
> + qemu = os.popen("qemu-img info %s" % f, "r")
> + for l in qemu:
> + (tag, val) = l.split(":")
> + if tag == "file format" and val.strip() == "vmdk":
> + self.createType = Disk.TWO_GB_MAX_EXTENT_SPARSE
> + qemu.close()
> + return self.extent
> +
> + def to_vmdk(self):
> + """Return the VMDK descriptor for this disk"""
> +
> + vmdk = """# Disk DescriptorFile
> +# Generated from libvirt
> +version=1
> +"""
Generated from libvirt? If putting anything, I guess the best thing would be 'virtinst'
> + vmdk += "CID=%08x\nparentCID=ffffffff\n" % self.cid
> + vmdk += "createType=\"%s\"\n\n" % self.createType
> + vmdk += "# Extent description\n"
> + blocks = self.size/512
> + if self.createType == Disk.MONOLITHIC_FLAT:
> + vmdk += "RW %d FLAT \"%s\" 0\n" % (blocks, os.path.basename(self.extent))
> + else: # Disk.MONOLITHIC_SPARSE
> + vmdk += "RW %d SPARSE \"%s\"\n" % (blocks, os.path.basename(self.extent))
> +
> + vmdk += """
> +# Disk Data Base
> +ddb.virtualHWVersion = "4"
> +ddb.adapterType = \"ide\"
> +ddb.geometry.sectors = \"%d\"
> +ddb.geometry.heads = \"%d\"
> +ddb.geometry.cylinders = \"%d\"
> +""" % (Disk.IDE_SECTORS, Disk.IDE_HEADS, blocks/(Disk.IDE_SECTORS*Disk.IDE_HEADS))
> + return vmdk
This piece above is kind of a mess. My recommendation is to wrap the entire thing
in triple quotes, and just do all the variable calculation prior to the assignment.
You won't need to escape anything or manually insert newlines. Or use the same
format as used later in the file, just define a _VMDK_TEMPLATE.
The rest looks fine to me.
- Cole
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
