[virt-tools-list] [virt-convert] ec2 export module

Cole Robinson crobinso at redhat.com
Mon Oct 12 16:56:57 UTC 2009


On 10/08/2009 02:51 PM, Joey Boggs wrote:

> Round 2
> 
> - ran setup.py check and cleaned up the code alot
> - Added exceptions and logging.error in place of fail() sections
> - moved helper files under virtconv/parsers/ec2helpers
> 
> Description of how this module works:
> 
> The EC2 module expects a single disk file from any other input 
> format(vmx/virt-image), more than 1 with throw an error. Adding support 
> for 2 disks should work fine out of the box but I've never tested it. 
> Once the base module is in I have a few more items to add, multiple 
> disks is one of them.
> - the input image is attached to a loop device and kpartx is run to 
> create device maps for each partition.
> - e2label is ran to grab the file system labels on the input disk
> - the list is sorted and begins mounting those file systems in the 
> correct order to a temp directory
> - du -sh is ran on the temp directory and we add 30% free space to the 
> minimum amount of space required

Why 30%? Why not some static amount like 100MB? Isn't the extra space
only needed for the stuff we add (ec2 tools and kernel RPMS?)

> - new file system is created using above calculation
> - rsync copies the fully mounted filesystems to the new single loopback 
> filesystem.

Just to clarify, we copy the contents from the original disk image to a
new image which is 30% bigger, and has a single partition for /.

> - EC2 standardizations added to the configuration
> - eth0

Here we are actually overwriting any existing eth0 configuration?

> - rc.local (to download public key of user on boot, and ami tools, to 
> allow rebundling)
> - kernel modules (pulls in the latest matching Fedora kernel that EC2 
> support, for net/block device modules)
> - api tools ( to allow rebundling and upload to S3)
> - fstab (rootfs/swap/S3)
> 
> Once the configuration is done, we unmount the new filesystem and all of 
> the input disk image partitions.
> 
> Normal operation of virt-convert continues
> - We skip the disk format convert since we already accomplished this above
> - New disk image is moved into the correct location for the user to take 
> and bundle with the EC2's ami-tools or others.
> 
> 
> -----------------
> Sample output
> 
> The output is mostly quiet since it looks like logging.info isn't sent out.
> -----------------
> 
> [jboggs at localhost jeos]$ sudo virt-convert jeos.xml -o ec2
> Generating output in 'ec2' format to jeos/
> 469+0 records in
> 469+0 records out
> 491782144 bytes (492 MB) copied, 8.93075 s, 55.1 MB/s
> mke2fs 1.41.4 (27-Jan-2009)
> Filesystem label=
> OS type: Linux
> Block size=1024 (log=0)
> Fragment size=1024 (log=0)
> 120360 inodes, 480256 blocks
> 24012 blocks (5.00%) reserved for the super user
> First data block=1
> Maximum filesystem blocks=67633152
> 59 block groups
> 8192 blocks per group, 8192 fragments per group
> 2040 inodes per group
> Superblock backups stored on blocks:
> 8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409
> 
> Writing inode tables: done
> Creating journal (8192 blocks): done
> Writing superblocks and filesystem accounting information: done
> 
> This filesystem will be automatically checked every 30 mounts or
> 180 days, whichever comes first. Use tune2fs -c or -i to override.
> % Total % Received % Xferd Average Speed Time Time Time Current
> Dload Upload Total Spent Left Speed
> 100 8027k 100 8027k 0 0 339k 0 0:00:23 0:00:23 --:--:-- 115k
> % Total % Received % Xferd Average Speed Time Time Time Current
> Dload Upload Total Spent Left Speed
> 100 16.5M 100 16.5M 0 0 1200k 0 0:00:14 0:00:14 --:--:-- 1220k
> Done.

A few more general comments:

- There are still lot's of 'cleanup' issues, that an unexpected
exception can leave loopback devices or temporary files hanging around.
Please audit this carefully. I've pointed out some examples inline.

- Please capitalize all comments (the existing code isn't consistent
with this, but it doesn't hurt for new code).

- Please replace examples of function spacing like func(arg1,arg2) with
func(arg1, arg2)

- Running pylint still showed a few more issues, mostly unused
variables. If you need to have an unused variable, you can add 'ignore'
somewhere in the name (ex. disks_ignore) and pylint will skip it .

> diff -r dfb9841a8a85 MANIFEST.in
> --- a/MANIFEST.in	Tue Oct 06 11:37:02 2009 -0400
> +++ b/MANIFEST.in	Thu Oct 08 14:09:37 2009 -0400
> @@ -3,6 +3,7 @@
>  include setup.py
>  recursive-include virtinst *.py
>  recursive-include virtconv *.py
> +recursive-include virtconv/parsers *.py
>  recursive-include tests *.py *.xml *.vmx *.virt-image *.sh
>  recursive-include tests/cli-test-xml *
>  include virt-install virt-clone virt-image virt-convert
> diff -r dfb9841a8a85 setup.py
> --- a/setup.py	Tue Oct 06 11:37:02 2009 -0400
> +++ b/setup.py	Thu Oct 08 14:09:37 2009 -0400
> @@ -9,7 +9,7 @@
>  from os.path import splitext, basename, join as pjoin
>  import os, sys
>  
> -pkgs = ['virtinst', 'virtconv', 'virtconv.parsers' ]
> +pkgs = ['virtinst', 'virtconv', 'virtconv.parsers', 'virtconv.parsers.ec2helpers' ]
>  
>  datafiles = [('share/man/man1', ['man/en/virt-install.1',
>                                   'man/en/virt-clone.1',
> diff -r dfb9841a8a85 virt-convert
> --- a/virt-convert	Tue Oct 06 11:37:02 2009 -0400
> +++ b/virt-convert	Thu Oct 08 14:09:37 2009 -0400
> @@ -247,12 +247,12 @@
>              if not dformat:
>                  dformat = "raw"
>  
> -            if d.path and dformat != "none":
> -                verbose(options, _("Converting disk '%(path)s' to type "
> -                                   "%(format)s...") % {"path": d.path,
> -                                                       "format": dformat})
> -
> -            d.convert(options.input_dir, options.output_dir, dformat)
> +            if not options.output_format == "ec2":
> +                if d.path and format != "none":
> +                    verbose(options, _("Converting disk '%(path)s' to type "
> +                                       "%(format)s...") % {"path": d.path,
> +                                                       "format": format})
> +                d.convert(options.input_dir, options.output_dir, format)
>  
>      except OSError, e:
>          cleanup(_("Couldn't convert disks: %s") % e.strerror,
> diff -r dfb9841a8a85 virtconv/__init__.py
> --- a/virtconv/__init__.py	Tue Oct 06 11:37:02 2009 -0400
> +++ b/virtconv/__init__.py	Thu Oct 08 14:09:37 2009 -0400
> @@ -34,7 +34,7 @@
>  parsers_path = [os.path.join(__path__[0], "parsers/")]
>  
>  # iter_modules is only in Python 2.5, sadly
> -parser_names = [ "vmx", "virtimage" ]
> +parser_names = [ "vmx", "virtimage", "ec2" ]
>  
>  if hasattr(pkgutil, "iter_modules"):
>      parser_names = []
> diff -r dfb9841a8a85 virtconv/parsers/__init__.py
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/virtconv/parsers/__init__.py	Thu Oct 08 14:09:37 2009 -0400
> @@ -0,0 +1,20 @@
> +#!/usr/bin/python
> +#
> +# Copyright 2009  Red Hat, Inc.
> +# Joey Boggs <jboggs at redhat.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA 02110-1301 USA.
> +
> diff -r dfb9841a8a85 virtconv/parsers/ec2.py
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/virtconv/parsers/ec2.py	Thu Oct 08 14:09:37 2009 -0400
> @@ -0,0 +1,106 @@
> +#!/usr/bin/python
> +#
> +# Copyright 2009  Red Hat, Inc.
> +# Joey Boggs <jboggs at redhat.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA 02110-1301 USA.
> +
> +from virtconv import _gettext as _
> +from tempfile import mkdtemp
> +import virtconv.formats as formats
> +import virtconv.parsers.ec2helpers.ec2config as ec2config
> +import virtconv.parsers.ec2helpers.ec2fs as ec2fs
> +import os
> +import logging
> +import shutil
> +
> +class ec2_parser(formats.parser):
> +    name = "ec2"
> +    suffix = ".ec2"
> +    can_import = False
> +    can_export = True
> +    can_identify = True
> +
> +    @staticmethod
> +    def identify_file(input_file):
> +        """
> +        Return True if the given file is of this format.
> +        """
> +        return False
> +
> +    @staticmethod
> +    def import_file(input_file):
> +        """
> +        Import a configuration file.  Raises if the file couldn't be
> +        opened, or parsing otherwise failed.
> +        """

Should 'raise NotImplementedError'

> +
> +    @staticmethod
> +    def export(vm,output_file):

I know it's a nitpick, but this arg1,arg2 format makes parameter lists
difficult to read. Please add a space between arguments (arg1, arg2)

> +        """
> +        Export a configuration file as a string.
> +        @vm vm configuration instance
> +
> +        Raises ValueError if configuration is not suitable.
> +        """
> +        tmpdir = mkdtemp()
> +        tmpimage = tmpdir + "-tmpimage"
> +        os.mkdir(tmpimage)

You should just use mkdtemp("-tmpimage"), otherwise your generated dir
name could collide with an existing directory (though highly unlikely).

> +        newimage = tmpimage + "/ec2-diskimage.img"
> +
> +        fsutil = ec2fs.LoopBackDiskImage()
> +
> +        if len(vm.disks) > 1:
> +            logging.error(_("EC2 conversion only supports 1 disk currently"))
> +        for devid, disk in sorted(vm.disks.items()):
> +            fsutil.setup_fs(disk.path,tmpdir)

If setup_fs throws an exception, the tmp directory isn't removed.

> +
> +        config = ec2config.EC2Config()
> +        try:
> +            # make needed device nodes
> +            config.makedev(tmpdir)
> +            # setup fstab correctly
> +            config.fstab(tmpdir)
> +            # download ami-tools on boot of instance
> +            config.rclocal_config(tmpdir)
> +            # setup eth0
> +            config.eth0_config(tmpdir)
> +            # install api tools
> +            config.api_tools(tmpdir)
> +            # configure kernel
> +            config.kernel_modules(tmpdir)

Spacing between each block would help for readability:

# Comment 1
func()

# Comment 2
func2()

> +        finally:
> +            # unmount all temporary directories
> +            fsutil.unmount(tmpdir)
> +            # move image to output directory
> +            logging.info(_("Moving image to output directory: %s" % output_file))
> +            shutil.move(newimage,output_file)

This 'move' should probably be added to the try: block, since it isn't
really cleanup.

> +            # cleanup temp directories
> +            fsutil.cleanup(tmpdir)
> +
> +    @staticmethod
> +    def export_file(vm, output_file):
> +        """
> +        Export a configuration file.
> +        @vm vm configuration instance
> +        @output_file Output file
> +
> +        Raises ValueError if configuration is not suitable, or another
> +        exception on failure to write the output file.
> +        """
> +        output = ec2_parser.export(vm,output_file)
> +
> +formats.register_parser(ec2_parser)
> diff -r dfb9841a8a85 virtconv/parsers/ec2helpers/__init__.py
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/virtconv/parsers/ec2helpers/__init__.py	Thu Oct 08 14:09:37 2009 -0400
> @@ -0,0 +1,20 @@
> +#!/usr/bin/python
> +#
> +# Copyright 2009  Red Hat, Inc.
> +# Joey Boggs <jboggs at redhat.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA 02110-1301 USA.
> +
> diff -r dfb9841a8a85 virtconv/parsers/ec2helpers/ec2config.py
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/virtconv/parsers/ec2helpers/ec2config.py	Thu Oct 08 14:09:37 2009 -0400
> @@ -0,0 +1,158 @@
> +#!/usr/bin/python
> +#
> +# ec2config.py: Convert a virtual appliance image in an EC2 AMI
> +#
> +# Copyright 2008, Red Hat  Inc.
> +# Joey Boggs <jboggs at redhat.com>
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU Library General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> +
> +import os
> +import logging
> +from virtconv import _gettext as _
> +import subprocess    
> +
> +class EC2Config():
> + 
> +    # make devices nodes on the new file system
> +    def makedev(self,tmpdir):
> +        devs=["console","null","zero"]
> +        devnull = file('/dev/null', 'w')
> +        for dev in devs:  
> +            makedev_cmd="/sbin/MAKEDEV -d %s/dev -x %s" % (tmpdir,dev)
> +            makedev_ret = subprocess.call(makedev_cmd, shell=True,stdout=devnull, stderr=devnull)
> +
> +    # insert fstab template
> +    def fstab(self,tmpdir):
> +        logging.info("* - Updating /etc/fstab")
> +        fstab_path = tmpdir + "/etc/fstab"
> +        os.system("touch " + fstab_path)

This 'touch' is redundant, open(..., "w") will create the file if
neccessary.

> +        fstab = open(fstab_path, "w")
> +        ec2_fstab =  "/dev/sda1  /         ext3    defaults        1 1\n"
> +        ec2_fstab += "/dev/sda2  /mnt      ext3    defaults        1 2\n"
> +        ec2_fstab += "/dev/sda3  swap      swap    defaults        0 0\n"
> +        ec2_fstab += "none       /dev/pts  devpts  gid=5,mode=620  0 0\n"
> +        ec2_fstab += "none       /dev/shm  tmpfs   defaults        0 0\n"
> +        ec2_fstab += "none       /proc     proc    defaults        0 0\n"
> +        ec2_fstab += "none       /sys      sysfs   defaults        0 0\n"

Also, I'd prefer if these large blocks of text could be broken out into
a single string using """ ... """, removed from the function and made
global like is done in the vmx parser.

> +        try:
> +            fstab.writelines(ec2_fstab)
> +            fstab.close()
> +        except Exception, e:
> +            raise RuntimeError(_("Couldn't create %s: %s" % (fstab_path,e)))
> +
> +    # fetch public key for login and most current ami-tools rpm
> +    def rclocal_config(self,tmpdir):
> +        rclocal_path = tmpdir + "/etc/rc.local"
> +        rclocal = open(rclocal_path, "w")
> +        logging.info("* - Creating rc.local configuration\n")
> +        ec2_rclocal = "if [ ! -d /root/.ssh ] ; then\n"
> +        ec2_rclocal += "mkdir -p /root/.ssh\n"
> +        ec2_rclocal += "chmod 700 /root/.ssh\n"
> +        ec2_rclocal += "fi\n\n"
> +        ec2_rclocal += " # Fetch public key using HTTP\n"
> +        ec2_rclocal += "curl -f http://169.254.169.254/latest/meta-data/public-keys/0/openssh-key > /tmp/my-key\n"
> +        ec2_rclocal += "if [ $? -eq 0 ] ; then\n"
> +        ec2_rclocal += "cat /tmp/my-key >> /root/.ssh/authorized_keys\n"
> +        ec2_rclocal += "chmod 600 /root/.ssh/authorized_keys\n"
> +        ec2_rclocal += "rm /tmp/my-key\n"
> +        ec2_rclocal += "fi\n\n"
> +        ec2_rclocal += "# or fetch public key using the file in the ephemeral store:\n"
> +        ec2_rclocal += "if [ -e /mnt/openssh_id.pub ] ; then\n"
> +        ec2_rclocal += "cat /mnt/openssh_id.pub >> /root/.ssh/authorized_keys\n"
> +        ec2_rclocal += "chmod 600 /root/.ssh/authorized_keys\n"
> +        ec2_rclocal += "fi\n\n"
> +        ec2_rclocal += "# Update the EC2 AMI creation tools\n"
> +        ec2_rclocal += "echo Updating ec2-ami-tools\n"
> +        ec2_rclocal += "curl -o /tmp/ec2-ami-tools.noarch.rpm http://s3.amazonaws.com/ec2-downloads/ec2-ami-tools.noarch.rpm && \n"
> +        ec2_rclocal += "rpm -Uvh /tmp/ec2-ami-tools.noarch.rpm && \n"
> +        ec2_rclocal += "echo \" + Updated ec2-ami-tools\"\n"
> +        try:
> +            rclocal.writelines(ec2_rclocal)
> +            rclocal.close()
> +        except Exception, e:
> +            raise RuntimeError(_("Couldn't create %s: %s" % (rclocal_path,e)))
> +        return
> +
> +    # enable ssh
> +    def ssh_config(self,tmpdir):
> +        try: 
> +            sshdconfig_path = tmpdir + "/etc/ssh/sshd_config"
> +            sshdconfig = open(sshdconfig_path,"w")
> +        except IOError, (errno, strerror):
> +            logging.error(_( "%s, %s %s" % (strerror,errno,sshdconfig_path)))
> +            logging.error(_("The openssh_server package must be installed to convert and function properly on EC2"))
> +        else:
> +            logging.info("* - Creating ssh configuration")
> +            ec2_sshdconfig = "UseDNS  no\n"
> +            ec2_sshdconfig +="PermitRootLogin without-password\n"
> +            try:
> +                sshdconfig.writelines(ec2_sshdconfig)
> +                sshdconfig.close()
> +            except Exception, e:
> +                raise RuntimeError(_("Couldn't create %s: %s" % (sshdconfig_path,e)))
> +        return
> +
> +    # eth0 default setup
> +    def eth0_config(self,tmpdir):
> +        try: 
> +            logging.info("* - Creating eth0 configuration")
> +            eth0_path = tmpdir + "/etc/sysconfig/network-scripts/ifcfg-eth0"
> +            os.system("touch %s" % eth0_path)
> +            eth0 = open(eth0_path, "w")
> +        except IOError, (errno, strerror):
> +            logging.error(_("%s, %s %s" % (strerror,errno,eth0_path)))
> +        else:
> +            ec2_eth0  = "ONBOOT=yes\n"
> +            ec2_eth0 += "DEVICE=eth0\n"
> +            ec2_eth0 += "BOOTPROTO=dhcp\n"
> +            try:
> +                eth0.writelines(ec2_eth0)
> +                eth0.close()
> +                os.system("chroot %s /sbin/chkconfig network on" % tmpdir)
> +            except Exception, e:
> +                raise RuntimeError(_("Couldn't create %s: %s" % (eth0_path,e)))
> +        
> +        logging.info("* - Prevent nosegneg errors")
> +        os.system("echo \"hwcap 0 nosegneg\" > %s/etc/ld.so.conf.d/nosegneg.conf" % tmpdir)    
> +    
> +    # install api tools
> +    # there is no ec2-api-tools-LATEST-VERSION, hardcoded this for now
> +    def api_tools(self,tmpdir):
> +        logging.info("Adding EC2 Tools")
> +        if not os.path.isdir(tmpdir + "/home/ec2"): 
> +            os.mkdir(tmpdir + "/home/ec2")
> +        ec2td = os.system("curl -o /tmp/ec2-api-tools-1.2-9739.zip http://s3.amazonaws.com/ec2-downloads/ec2-api-tools-1.2-9739.zip")

This /tmp file is never removed.

We already use 'urlgrabber' for virtinst, might as well avoid calling
out to the command line for fetching this file.

Also, please break the amazon link out to a global variable, so it is
easily updated if needed in the future.

> +        if ec2td == 0:
> +            try:
> +                os.system("unzip -qo /tmp/ec2-api-tools-1.2-9739.zip -d /home/ec2")

Is this trying to unzip to /home/ec2 on the host?

> +            except Exception, e:
> +                raise RuntimeError(_("Couldn't extract api tools zip file: %s" % (e)))
> +        else:
> +            logging.error(_("EC2 api tools download error"))
> +            raise RuntimeError(_("EC2 api tools download error"))

No need to log the error and raise an exception, just stick with the
exception.

> +
> +    # install a matching EC2 kernel for necessary block/network kernel modules
> +    def kernel_modules(self,tmpdir):    
> +        devnull = file('/dev/null', 'w')
> +        logging.info("Configure image for accepting the EC2 kernel")
> +        # note this is the most current Fedora kernel available in EC2 as of 10/2009
> +        kd = os.system("curl -o /tmp/kernel-xen-2.6.21.7-2.fc8.i686.rpm http://kojipkgs.fedoraproject.org/packages/kernel-xen-2.6/2.6.21.7/2.fc8/i686/kernel-xen-2.6.21.7-2.fc8.i686.rpm")

This tmp file isn't removed, and please break out the koji link to a
global variable.

We should really find a way to programmatically determine the
best/latest links to use, but that can come later.

> +        if kd == 0:
> +            kernel_install_cmd=("rpm -ivh --nodeps /tmp/kernel-xen-2.6.21.7-2.fc8.i686.rpm --root=%s" % tmpdir)
> +            kernel_install_ret = subprocess.call(kernel_install_cmd, shell=True,stdout=devnull)
> +            if kernel_install_ret !="0":
> +                logging.error(_("kernel install failed"))
> +                raise RuntimeError(_("Couldn't install kernel %s" % kernel_install_cmd))
> +        else:
> +            logging.error(_("Kernel download error!"))
> diff -r dfb9841a8a85 virtconv/parsers/ec2helpers/ec2fs.py
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/virtconv/parsers/ec2helpers/ec2fs.py	Thu Oct 08 14:09:37 2009 -0400
> @@ -0,0 +1,108 @@
> +#!/usr/bin/python
> +#
> +# ec2fs.py: Convert a virtual appliance image in an EC2 AMI
> +#
> +# Copyright 2008, Red Hat  Inc.
> +# Joey Boggs <jboggs at redhat.com>
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU Library General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> +
> +import os
> +import logging
> + 
> +class LoopBackDiskImage(): 
> +
> +    def setup_fs(self,imagefile,tmpdir):

Seems like rather than pass 'tmpdir' to every function, just __init__
the class and make tmpdir and class member.

> +        loop_partition_dict = {}
> +        tmproot = tmpdir + "-tmproot"
> +        tmpimage = tmpdir + "-tmpimage"
> +        logging.debug("TMPDIR: " + tmpdir)
> +        # get a loop device we can use for mounting the input disk image
> +        free_loop_dev = os.popen("/sbin/losetup -f")
> +        loop_device = free_loop_dev.read().strip()
> +
> +        if not loop_device:
> +            raise RuntimeError(_("Unable to find a free loop device"))
> +     
> +        # associate disk image to the loop device
> +        try:
> +            os.system("/sbin/losetup %s %s" % (loop_device,imagefile))
> +            os.system("/sbin/kpartx -a %s" % loop_device)
> +            loop_partitions = os.popen("/sbin/kpartx -lv %s|awk {'print $1'}" % loop_device)
> +        except Exception, e:
> +            raise RuntimeError(_("Couldn't create device maps:" % e))

If anything past this point throws an exception, we will not properly
clean up the loop devices and kpartx device maps.

> +        # loop through partitions to read file system labels and determine correct mounting order
> +        for dev in loop_partitions:
> +            dev = dev.strip()
> +            label = os.popen("e2label /dev/mapper/%s 2>&1" % dev)
> +            label = label.read().strip()
> +            if label.startswith("e2label"):
> +                logging.info(_("Unable to detect partition label on %s, continuing anyways, if", 
> +                                     "%s is a swap partition, no action is needed" % (dev,dev)))
> +            else: 
> +                loop_partition_dict[dev] = label  
> +                logging.debug( dev + " : " + label)
> +
> +        dev = loop_partition_dict.values()
> +        dev.sort()
> +        os.mkdir(tmproot) 
> +
> +        # mount partitions discovered through kpartx in correct order
> +        for value in dev:
> +            for key in loop_partition_dict.keys():
> +                if (value == loop_partition_dict[key]):
> +                    if not loop_device:
> +                        logging.error(_("Please review your loopback device settings and remove unneeded ones"))
> +                    os.system("mount -o loop /dev/mapper/%s %s%s" % (key,tmproot,value))
> +            
> +        # determine minimum amount of disk space for the new image
> +        tmp_disk_space = os.popen("du -s %s|awk {'print $1'}" % tmproot)
> +        tmp_disk_space= int(tmp_disk_space.read()) / 1024
> +        # add 30% free space to new image 
> +        new_disk_space = int(tmp_disk_space + ((tmp_disk_space * 0.30)))
> +
> +        logging.info(_("\nCreating a new disk image: %sM total" % str(new_disk_space)))
> +        # create new disk and file system
> +        create_disk = os.system("dd if=/dev/zero of=%s/ec2-diskimage.img bs=1M count=%s" % (tmpimage,new_disk_space))
> +        os.system("mke2fs -Fj %s/ec2-diskimage.img" % tmpimage)
> +        # if we run out of loop devices, error out
> +        if not loop_device:
> +                raise RuntimeError(_("Couldn't find a free loop device: %s" % e))

Indentation issue: python setup.py check should have picked this up.

> +        os.system("mount -o loop %s/ec2-diskimage.img %s" % (tmpimage,tmpdir))
> +            
> +        # rsync old root to new root
> +        logging.info(_("Performing rsync on all partitions to new root"))
> +        try:
> +            os.system("rsync -u -r -a  %s/* %s" % (tmproot,tmpdir))
> +        except Exception, e:
> +            raise RuntimeError(_("Couldn't rsync file system to new root:" % e))        
> +
> +        # umount devices in correct reverse order
> +        dev.sort(reverse=True)
> +        for value in dev:
> +            logging.info("Unmounting %s%s" % (tmpdir,value))
> +            os.system("umount %s%s" % (tmproot,value))
> +
> +        logging.info("Freeing loopdevices")
> +        os.system("kpartx -d %s" % loop_device)
> +        os.system("losetup -d %s" % loop_device)
> +        return
> +        
> +    def unmount(self,tmpdir):
> +        logging.debug("Unmounting directory %s" % tmpdir)
> +        os.system("/bin/umount %s" % tmpdir)
> +        return
> +    
> +    def cleanup(self,tmpdir):
> +        os.system("rm -rf %s/*" % tmpdir)
> +        return

Thanks,
Cole




More information about the virt-tools-list mailing list