Francesco Romani has posted comments on this change.
Change subject: hook: spiceoptions: To provide spice option attributes to vm ......................................................................
Patch Set 5:
(4 comments)
.................................................... Commit Message Line 17: <graphics type='spice' port='-1' tlsPort='-1' autoport='yes'> Line 18: ****** Line 19: <image compression='auto_glz'/> Line 20: <streaming mode='filter'/> Line 21: <clipboard copypaste='no'/> copypaste is already being addressed by http://gerrit.ovirt.org/#/c/22646/ Line 22: <mouse mode='client'/> Line 23: </graphics> Line 24: Line 25: Change-Id: I7761aab98cdc3a0d37bed7984ae540278e6bbf70
.................................................... File vdsm_hooks/spiceoptions/before_vm_start.py Line 49: 'jpeg': {'compression': JPEG_COMP_LIST}, Line 50: 'streaming': {'mode': STR_MODE_LIST}, Line 51: 'mouse': {'mode': MOUSE_MODE_LIST}, Line 52: 'clipboard': {'copypaste': COPY_PASTE_LIST}} Line 53: As matter as personal taste I'd not use a name for a variable which contains the expected type (like _LIST and _DICT) above, I think it's non-pythonic. Line 54: Line 55: def createOptionElement(domxml, attrname, attroption, attrvalue): Line 56: if attrname == "jpeg": Line 57: jpegEle = domxml.createElement(attrname)
Line 54: Line 55: def createOptionElement(domxml, attrname, attroption, attrvalue): Line 56: if attrname == "jpeg": Line 57: jpegEle = domxml.createElement(attrname) Line 58: if attrvalue in JPEG_COMP_LIST: If the list is used just for this maybe a frozenset() is better. Line 59: jpegEle.setAttribute(attroption, attrvalue) Line 60: return jpegEle Line 61: else: Line 62: sys.stderr.write("\n InValid option:%s \t Available options are %s"
Line 104: % (attrvalue, COPY_PASTE_LIST)) Line 105: else: Line 106: sys.stderr.write('\n No Valid Element created for graphic device :%s\n' Line 107: % (attrname)) Line 108: Seems there is a pattern here, can't be this factored in some way? Line 109: Line 110: def main(): Line 111: if 'spiceoptions' in os.environ: Line 112: try: