[et-mgmt-tools] [patch] virt-convert add disk signature into virt-image format export

[Joey Boggs]

>   
>> diff -r 58a909b4f71c virt-convert
>> --- a/virt-convert	Mon Sep 22 11:32:11 2008 -0400
>> +++ b/virt-convert	Wed Oct 01 17:12:45 2008 -0400
>> @@ -64,6 +64,8 @@
>>      opts.add_option("", "--os-variant", type="string", dest="os_variant",
>>                        action="callback", callback=cli.check_before_store,
>>                        help=("The OS variant for fully virtualized guests, e.g. 'fedora6', 'rhel5', 'solaris10', 'win2k', 'vista'"))
>> +    opts.add_option("", "--checksum", action="store_true", dest="checksum",
>> +                    help=("Generate a checksum for a virt-image guest"))
>>      opts.add_option("", "--noapic", action="store_true", dest="noapic",
>>          help=("Disables APIC for fully virtualized guest (overrides value in os-type/os-variant db)"), default=False)
>>      opts.add_option("", "--noacpi", action="store_true", dest="noacpi",
>> @@ -184,6 +186,9 @@
>>  
>>      unixname = vmdef.name.replace(" ", "-")
>>  
>> +    if options.checksum:
>> +        vmdef.checksum = "yes"
>> +
>>     
>
> Rather than use a string yes/no, why not just call
> the variable 'use_checksum' and have it as a bool
> value?
>
> We probably also want to add an option like 
> checksum_type, since it really isn't a simple
> yes/no option. If no type is specified, we can
> just whatever we deem is a sensible default.
> This can be worked out later though.
>
>   
I'll make that change for use_checksum. Would that mean we're only 
generating 1 checksum by default?
>
>> +
>> +        if vm.checksum == "yes":
>> +            md5checksum = None
>> +            shachecksum = None
>> +
>> +            storage.append("""<disk file="%s" use="system" format="%s">\n""" % (path, type))
>> +            
>> +            try:
>> +                import hashlib
>> +                m1 = hashlib.md5(path)
>> +                m2 = hashlib.sha256(path)
>> +            except:
>> +                import md5
>> +                m1 = md5.new(path)
>> +                m2 = None
>> +
>> +            f = open(path,"r")
>> +            while 1:
>> +                chunk = f.read(65536)
>> +                if not chunk:
>> +                    break
>> +                m1.update(chunk)
>> +
>> +                if m2:
>> +                   m2.update(chunk)
>> +
>>     
>
> I tested the above, and it generates different checksums than
> the cli utils (md5sum, sha256sum). Problem seems to be that
> you initialize the hash with the file's pathname. Is that
> intentional?
>
> We should probably match the output of the cli utils, so
> let's make sure the hashes match for both small and large
> files to be sure we aren't missing anything.
>
>   
I just discovered that this morning when working on the 
appliance-creator portions of this and just removed the filename out of 
the hash.