Amador Pahim has posted comments on this change. Change subject: hook: diskunmap: To include UNMAP support for disk and lun devices ......................................................................
Patch Set 4: -Verified (3 comments) Thank you for the review. A new patch set is on the way. http://gerrit.ovirt.org/#/c/29770/4/vdsm.spec.in File vdsm.spec.in: Line 368: Line 369: %package hook-diskunmap Line 370: Summary: Activate UNMAP for disk/lun devices Line 371: BuildArch: noarch Line 372: Requires: qemu-kvm >= 1.5 > so no intention for EL 6 support? discard support was introduced by qemu 1.5 and it was not backported to el6 so far. As soon as I see discard in downstream, I send a new patch to require the proper version also in downstream. Line 373: Line 374: %description hook-diskunmap Line 375: VDSM hooks which allow to activate disk UNMAP. Line 376: http://gerrit.ovirt.org/#/c/29770/4/vdsm_hooks/diskunmap/README File vdsm_hooks/diskunmap/README: Line 34: ... Line 35: <driver cache="none" discard="unmap" ... /> Line 36: </disk> Line 37: Line 38: This option will be tranlated to qemu as bellow: > *translated Done Line 39: http://gerrit.ovirt.org/#/c/29770/4/vdsm_hooks/diskunmap/before_vm_start.py File vdsm_hooks/diskunmap/before_vm_start.py: Line 84: Line 85: if __name__ == '__main__': Line 86: try: Line 87: if '--test' in sys.argv: Line 88: test() > I like the idea of test…but maybe less changes if you do it in main()? Hmm... I like the current way, keeping the test apart. test() is not supposed to have 'diskunmap' in os.environ, so the scope is out of main() primary intention. Line 89: else: Line 90: main() Line 91: except: Line 92: hooking.exit_hook(' diskunmap hook: [unexpected error]: %s\n' % -- To view, visit http://gerrit.ovirt.org/29770 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I36385f1af24043755b3d4b6594bbe598b0d9518d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amador Pahim <[email protected]> Gerrit-Reviewer: Amador Pahim <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[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
