[et-mgmt-tools] [PATCH] virt-inst Package an image for VMware distribution

[Joey Boggs]

Cole,

I'll work on those changes this weekend/next week, thanks for the comments.

Joey

Cole Robinson wrote:
> 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
>