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' %