Francesco Romani has posted comments on this change. Change subject: kvm2ovirt: tool for copying images from libvirt ......................................................................
Patch Set 5: Code-Review-1 (3 comments) minor things, looks good. -1 for visibility only. https://gerrit.ovirt.org/#/c/55797/5/configure.ac File configure.ac: Line 376: AC_OUTPUT([ Line 377: Makefile Line 378: client/Makefile Line 379: contrib/Makefile Line 380: helpers/Makefile missing tab Line 381: init/Makefile Line 382: init/systemd/Makefile Line 383: lib/Makefile Line 384: lib/vdsm/Makefile https://gerrit.ovirt.org/#/c/55797/5/helpers/kvm2ovirt File helpers/kvm2ovirt: PS5, Line 129: options = parse_arguments() : : con = libvirtconnection.open_connection(options.uri, : options.username, : get_password()) : : diskno = 1 : disksitems = len(options.source) : bufsize = int(options.bufsize) : write_output('preparing for copy') : for item in itertools.izip(options.source, options.dest): : vol = con.storageVolLookupByPath(item[0]) : download_volume(vol, item[1], diskno, disksitems, bufsize, options.verbose) : diskno = diskno + 1 : write_output('Finishing off') please wrap this in a main() function, like I commented few versions ago. Line 140: vol = con.storageVolLookupByPath(item[0]) Line 141: download_volume(vol, item[1], diskno, disksitems, bufsize, options.verbose) Line 142: diskno = diskno + 1 Line 143: write_output('Finishing off') Line 144 call the new main() function using this idiom if __name__ == "__main__": main() those should be the last lines of the file. -- To view, visit https://gerrit.ovirt.org/55797 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9d95c3bf4b2605e71f899171259d4721204eb8e2 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: Yaniv Kaul <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
