headers
Daniel P. Berrange | 1 Oct 16:43
Picon
Favicon

Re: [patch] virt-convert add disk signature into virt-image format export

On Wed, Oct 01, 2008 at 10:37:17AM -0400, Joey Boggs wrote:
> I'm done creating a sha256 hash setup should I offer more than just 
> sha256for now? and checksum generation is off by default
> 
> Here's a preview.  Not sure how to catch the module import failure for 
> hashlib though

If we go for doing a compulsory md5 checksum, and optional
sha256 checksum with new enough python, then something
like....

    try:
       import hashlib
       m1 = hashlib.md5()
       m2 = hashlib.sha256()
    except:
       import md5
       m1 = md5.new()
       m2 = None

 
Daniel
--

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Permalink | Reply |
headers
Joey Boggs | 1 Oct 18:27
Picon
Favicon

Re: [patch] virt-convert add disk signature into virt-image format export

Here's an update, will this work best or any other suggestions?

            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)
                md5checksum = m1.hexdigest()

                if m2:
                   m2.update(chunk)
                   shachecksum = m2.hexdigest()
            storage.append("""   <checksum type="md5">%s</checksum>\n""" 
% md5checksum)
            if shachecksum:
                storage.append("""   <checksum 
type="sha256">%s</checksum>\n""" % shachecksum)
            storage.append("""   </disk>\n""")

Daniel P. Berrange wrote:
(Continue reading)

Permalink | Reply |
headers
Daniel P. Berrange | 1 Oct 18:41
Picon
Favicon

Re: [patch] virt-convert add disk signature into virt-image format export

On Wed, Oct 01, 2008 at 12:27:46PM -0400, Joey Boggs wrote:
> Here's an update, will this work best or any other suggestions?
> 
>            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)
>                md5checksum = m1.hexdigest()
> 
>                if m2:
>                   m2.update(chunk)
>                   shachecksum = m2.hexdigest()

hexdigest() should only be called at end, outside
the loop. As it stands you're computing a checksum
for each chunk & resetting it

Daniel
--

--
(Continue reading)

Permalink | Reply |
headers
Joey Boggs | 1 Oct 18:46
Picon
Favicon

Re: [patch] virt-convert add disk signature into virt-image format export

That's one thing I forgot to move out of the loop before I sent it. I 
was still working on the handling of m2 missing prior to that and put it 
inside the loop for testing. Once moved should that be good to go?

Daniel P. Berrange wrote:
> On Wed, Oct 01, 2008 at 12:27:46PM -0400, Joey Boggs wrote:
>   
>> Here's an update, will this work best or any other suggestions?
>>
>>            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)
>>                md5checksum = m1.hexdigest()
>>
>>                if m2:
>>                   m2.update(chunk)
>>                   shachecksum = m2.hexdigest()
>>
(Continue reading)

Permalink | Reply |
headers
Joey Boggs | 1 Oct 23:15
Picon
Favicon

Re: [patch] virt-convert add disk signature into virt-image format export

Here's what I've got, moved the hexdigest() out of the loop, cleanup and 
more testing to verify each scenario outputs the right xml configuration 
data.

Hope this is the final revision :)

Joey Boggs wrote:
> That's one thing I forgot to move out of the loop before I sent it. I 
> was still working on the handling of m2 missing prior to that and put 
> it inside the loop for testing. Once moved should that be good to go?
>
> Daniel P. Berrange wrote:
>> On Wed, Oct 01, 2008 at 12:27:46PM -0400, Joey Boggs wrote:
>>  
>>> Here's an update, will this work best or any other suggestions?
>>>
>>>            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
(Continue reading)

Permalink | Reply |
headers
Cole Robinson | 3 Oct 17:15
Picon
Favicon

Re: [patch] virt-convert add disk signature into virt-image format export

Joey Boggs wrote:
> Here's what I've got, moved the hexdigest() out of the loop, cleanup and 
> more testing to verify each scenario outputs the right xml configuration 
> data.
> 
> Hope this is the final revision :)
> 
> 
> 
> 

Some comments below.

> 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:
(Continue reading)

Permalink | Reply |
headers
Joey Boggs | 3 Oct 17:32
Picon
Favicon

Re: [patch] virt-convert add disk signature into virt-image format export


>   
>> 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
(Continue reading)

Permalink | Reply |
headers
Cole Robinson | 3 Oct 17:39
Picon
Favicon

Re: [patch] virt-convert add disk signature into virt-image format export

Joey Boggs wrote:
>>   
>>> 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.
(Continue reading)

Permalink | Reply |

Gmane