[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Joey Boggs wrote:
> Adds disk signatures into virt-convert for virt-image format virtual 
> machines
> 
>

Comments inline.

> 
> diff -r 58a909b4f71c virtconv/parsers/virtimage.py
> --- a/virtconv/parsers/virtimage.py	Mon Sep 22 11:32:11 2008 -0400
> +++ b/virtconv/parsers/virtimage.py	Fri Sep 26 15:58:29 2008 -0400
> @@ -22,7 +22,7 @@
>  import virtconv.vmcfg as vmcfg
>  import virtconv.diskcfg as diskcfg
>  import virtinst.FullVirtGuest as fv
> -
> +import sha

Please keep the preceeding newline. Generally imports are 
grouped in some attempt at a logical manner for the sake
of readability.

>  from xml.sax.saxutils import escape
>  from string import ascii_letters
>  import re
> @@ -171,9 +171,11 @@
>          type = "raw"
>          if disk.type == diskcfg.DISK_TYPE_ISO:
>              type = "iso"
> +        diskfile=open(path,'r').read()
> +        checksum=sha.new(diskfile).hexdigest()

Please try to be consistent with surrounding code
format. 

The code above is using: var = val
You've added:            var=val

It helps code readability if spacing is reasonably
consistent throughout.

Hmm, so I just tried the above. What that is essentially
trying to do is read the entire disk into memory, then
pass it off as a giant string to the sha function. Clearly
this is not a workable solution. This also applies to
the virt-image hash changes as well. Haven't looked into
the correct way to do it though.

This is also a rather large performance drain. So at the
very least it should not be the default. That said, I
don't know the optimal way to expose this option, or
even if we should.

Is there a way that vmx files can store checksums?

>          storage.append(
> -            """<disk file="%s" use="system" format="%s"/>\n""" %
> -                (path, type))
> +            """<disk file="%s" use="system" format="%s">\n"""
> +            """   <checksum type="sha1">%s</checksum>\n  </disk>\n""" % (path, type,checksum))
>  
>      return storage, diskout


Thanks,
Cole



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]