Change in vdsm[master]: v2v: ova should support zip and extracted directory formats

fromani at redhat.com fromani at redhat.com
Fri Nov 6 09:31:57 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: v2v: ova should support zip and extracted directory formats
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

nice, a couple more of possible improvements.

https://gerrit.ovirt.org/#/c/48129/2/vdsm/v2v.py
File vdsm/v2v.py:

Line 751: 
Line 752: 
Line 753: def _read_ovf_from_ova_dir(ova_path):
Line 754:     files = os.listdir(ova_path)
Line 755:     for ova_file in files:
please see also comment at line 766 which may make this irrelevant
minor: here you use ova_file; below you use ova_file. The difference is just one char. Maybe using

  for entry in files:
    if '.ovf' == os.path.splitext(entry)[1].lower():
            with open(os.path.join(ova_path, entry), 'r') as ovf_file:
                return ovf_file.read()

leads to better code.
Your call, as you feels is most fit.
Line 756:         if '.ovf' == os.path.splitext(ova_file)[1].lower():
Line 757:             with open(os.path.join(ova_path, ova_file), 'r') as ovf_file:
Line 758:                 return ovf_file.read()
Line 759:     raise ClientError('OVA directory %s does not contain ovf file' % ova_path)


Line 762: def _read_ovf_from_zip_ova(ova_path):
Line 763:     with open(ova_path, 'rb') as fh:
Line 764:         zf = zipfile.ZipFile(fh)
Line 765:         for name in zf.namelist():
Line 766:             if '.ovf' == os.path.splitext(name)[1].lower():
we can extract one helper here:

  def _find_ovf(entries):
    for entry in entries:
      if '.ovf' == os.path.splitext(entry)[1].lower():
        return entry
    return None

use it like:

  def _read_ovf_from_zip_ova(ova_path):
      with open(ova_path, 'rb') as fh:
        zf = zipfile.ZipFile(fh)
        name = _find_ovf(zf.namelist())
        if name is not None:
          return zf.read(name)
      raise ClientError('OVA does not contains file with .ovf suffix')

and, above:

  def _read_ovf_from_ova_dir(ova_path):
    name = _find_ovf(ova_path)
    if name is not None:
      with open(os.path.join(ova_path, name), 'r') as ovf_file:
        return ovf_file.read()
    raise ClientError('OVA directory %s does not contain ovf file' % ova_path)
Line 767:                 return zf.read(name)
Line 768:     raise ClientError('OVA does not contains file with .ovf suffix')
Line 769: 
Line 770: 


-- 
To view, visit https://gerrit.ovirt.org/48129
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I494b88709c4c23fd39690b589eff1134e74f81ba
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list