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

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

 



Cole Robinson wrote:
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.

I think this will become important as we look to distribute the appliances either via RHN or on public sites. Also.. this makes us a bit more consistent with OVF.

I am fine with making it off by default, and then causing the command line to turn it on.

-- bk

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

[Index of Archives]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux