Shahar Havivi has posted comments on this change.

Change subject: v2v: add volume size to disk info
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.ovirt.org/#/c/36263/4/vdsm/v2v.py
File vdsm/v2v.py:

Line 49
Line 50
Line 51
Line 52
Line 53
> Idea for another patch - the code getting info from the vm object can be ex
you mean all the input ie the status and the name?


Line 19: import xml.etree.ElementTree as ET
Line 20: from contextlib import closing
Line 21: 
Line 22: import logging
Line 23: from vdsm.define import errCode
> This should be bellow near the other "from vdsm ..." import.
Done
Line 24: import caps
Line 25: import libvirt
Line 26: from vdsm import libvirtconnection
Line 27: 


Line 107:     if e is not None:
Line 108:         params['arch'] = e.get('arch')
Line 109: 
Line 110: 
Line 111: def _add_disks(root, params, conn):
> It is little ugly that this function mix extracting info from xml and perfo
We do need the size in the verb (vdsm call)
I can separate to two calls but I will need the same logic to calculate the 
size of given file..., maybe I can call a new function for _add_disks() and 
pass the source file name?
Line 112:     params['disks'] = []
Line 113:     disks = root.findall('.//disk[@type="file"]')
Line 114:     for disk in disks:
Line 115:         d = {}


Line 123:                 vol = conn.storageVolLookupByPath(source.get('file'))
Line 124:                 d['truesize'] = str(vol.info()[1])
Line 125:             except libvirt.libvirtError as e:
Line 126:                 logging.error('error parsing storage size, msg: %s  
xml: %s',
Line 127:                               e.message)
> Also there is no need to access e.message, just format e as string:
Done
Line 128:         params['disks'].append(d)
Line 129: 
Line 130: 
Line 131: def _add_networks(root, params):


Line 123:                 vol = conn.storageVolLookupByPath(source.get('file'))
Line 124:                 d['truesize'] = str(vol.info()[1])
Line 125:             except libvirt.libvirtError as e:
Line 126:                 logging.error('error parsing storage size, msg: %s  
xml: %s',
Line 127:                               e.message)
> I see two '%s' in the format string, but just one more argument.
Done
Line 128:         params['disks'].append(d)
Line 129: 
Line 130: 
Line 131: def _add_networks(root, params):


-- 
To view, visit http://gerrit.ovirt.org/36263
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7e9ba73514292cc2bb4a025d286e2c732e88a75
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Shahar Havivi <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to