Francesco Romani has posted comments on this change.
Change subject: hook: spiceoptions: To provide spice option attributes to vm ......................................................................
Patch Set 7:
(5 comments)
http://gerrit.ovirt.org/#/c/22178/7/vdsm_hooks/spiceoptions/before_vm_start.... File vdsm_hooks/spiceoptions/before_vm_start.py:
Line 14: ex IMO redundant
Line 15: Line 16: Syntax: Line 17: spiceoptions={'element': {'attribute': 'value'},.. Line 18: Line 19: For ex: No need of shortening here:
Example: Line 20: spiceoptions={'image': {'compression': 'auto_glz'}, Line 21: 'jpeg': {'compression': 'never'}, Line 22: 'streaming':{'mode':'filter'}} Line 23:
Line 52: Line 53: def main(): Line 54: if 'spiceoptions' in os.environ: Line 55: try: Line 56: spiceConfig = ast.literal_eval(os.environ['spiceoptions']) I see this is already used, but I still wonder if that's the best solution. Not big problem however. Line 57: spiceConfig = dict((k.lower(), v) Line 58: for k, v in spiceConfig.iteritems()) Line 59: Line 60: domxml = hooking.read_domxml()
Line 64: keys please use
if elmt not in spiceOpts:
Line 65: sys.stderr.write(" Invalid ELEMENT" Line 66: " [%s] " % elmt) Line 67: else: Line 68: for attr, attrValue in value.items(): Line 69: if attr not in spiceOpts[elmt].keys(): same here:
if attr not in spiceOpts[elmt]: Line 70: sys.stderr.write(" Invalid ATTRIBUTE" Line 71: " [%s]" % attr) Line 72: elif attrValue not in spiceOpts[elmt][attr]: Line 73: sys.stderr.write(" Invalid VALUE"