[Thincrust-devel] [patch] act md5/sha256 disk signature support for appliance-creator

[Bryan Kearney]

Couple of nits below. Other then that, ACK.

Joey Boggs wrote:
> Updated patch, mostly everything is the same, added in the progress bar 
> just like virt-image uses during verification. Generates sha1/sha256 if 
> available and is off by default.
> 
> appliance-creator  --generate-checksum
> diff --git a/appcreate/appliance.py b/appcreate/appliance.py
> index 054c69a..4345a9c 100644
> --- a/appcreate/appliance.py
> +++ b/appcreate/appliance.py
> @@ -29,6 +29,7 @@ from imgcreate.errors import *
>  from imgcreate.fs import *
>  from imgcreate.creator import *
>  from appcreate.partitionedfs import *
> +import urlgrabber.progress as progress
>  
>  class ApplianceImageCreator(ImageCreator):
>      """Installs a system into a file containing a partitioned disk image.
> @@ -40,7 +41,7 @@ class ApplianceImageCreator(ImageCreator):
>  
>      """
>  
> -    def __init__(self, ks, name, format, package, vmem, vcpu):
> +    def __init__(self, ks, name, format, package, vmem, vcpu, checksum):
>          """Initialize a ApplianceImageCreator instance.
>  
>          This method takes the same arguments as ImageCreator.__init__()
> @@ -55,6 +56,7 @@ class ApplianceImageCreator(ImageCreator):
>          self.__vmem = vmem
>          self.__vcpu = vcpu
>          self.__package = package
> +        self.__checksum = checksum
>          
>  
>      def _get_fstab(self):
> @@ -305,8 +307,46 @@ class ApplianceImageCreator(ImageCreator):
>          xml += "    </devices>\n"
>          xml += "  </domain>\n"
>          xml += "  <storage>\n"
> -        for name in self.__disks.keys():
> -            xml += "    <disk file='%s-%s.%s' use='system' format='%s'/>\n" % (self.name,name, self.__format, self.__format)
> +
> +        if self.__checksum is True:
> +            for name in self.__disks.keys():
> +                diskpath = "%s/%s-%s.%s" % (self.__imgdir,self.name,name, self.__format)
> +                disk_size = os.path.getsize(diskpath)
> +                meter_ct = 0 
> +                meter = progress.TextMeter()
> +                meter.start(size=disk_size, text="Generating disk signature for %s-%s.%s" % (self.name,name,self.__format))
> +                xml += "<disk file='%s-%s.%s' use='system' format='%s'>\n" % (self.name,name, self.__format, self.__format)
In this case, the indentation is off. I think the disk and checksums 
should each go in one more indenteation level.
> +            
> +                try:
> +                    import hashlib
> +                    m1 = hashlib.sha1()
> +                    m2 = hashlib.sha256()
> +                except:
> +                    import sha
> +                    m1 = sha.new()
> +                    m2 = None
> +                f = open(diskpath,"r")
> +                while 1:
> +                    chunk = f.read(65536)
> +                    if not chunk:
> +                        break
> +                    m1.update(chunk)
> +                    if m2:
> +                       m2.update(chunk)
> +                    meter.update(meter_ct)
> +                    meter_ct = meter_ct + 65536
> +
> +                sha1checksum = m1.hexdigest()
> +                xml +=  """   <checksum type="sha1">%s</checksum>\n""" % sha1checksum
The other tags use a single '. Can you stay consistent. As with above, 
need to indent one more level.
> +
> +                if m2:
> +                    sha256checksum = m2.hexdigest()
> +                    xml += """   <checksum type="sha256">%s</checksum>\n""" % sha256checksum
> +                xml += "   </disk>\n"
Same
> +        else:
> +            for name in self.__disks.keys():
> +                xml += "    <disk file='%s-%s.%s' use='system' format='%s'/>\n" % (self.name,name, self.__format, self.__format)
Indent is good here.
> +
>          xml += "  </storage>\n"
>          xml += "</image>\n"
>  
> diff --git a/tools/appliance-creator b/tools/appliance-creator
> index 6d990de..1442f89 100755
> --- a/tools/appliance-creator
> +++ b/tools/appliance-creator
> @@ -47,6 +47,8 @@ def parse_options(args):
>                        help="amount of virtual memory for appliance in MB (default: 512)")
>      appopt.add_option("", "--vcpu", type="int", dest="vcpu", default=1,
>                        help="number of virtual cpus for appliance (default: 1)")
> +    appopt.add_option("", "--generate-checksum", action="store_true", dest="checksum",
> +                     help=("Generate a checksum for the created appliance"))
>      parser.add_option_group(appopt)
>      
>      # options related to the config of your system
> @@ -101,7 +103,7 @@ def main():
>      if options.name:
>          name = options.name
>   
> -    creator = appcreate.ApplianceImageCreator(ks, name, options.format, options.package, options.vmem, options.vcpu)
> +    creator = appcreate.ApplianceImageCreator(ks, name, options.format, options.package, options.vmem, options.vcpu, options.checksum)
>      creator.tmpdir = options.tmpdir
>      
>      try:
> 
>