Change in vdsm[master]: Support for QEMU cmdline hook

danken at redhat.com danken at redhat.com
Tue Aug 7 11:26:19 UTC 2012


Dan Kenigsberg has posted comments on this change.

Change subject: Support for QEMU cmdline hook
......................................................................


Patch Set 1: I would prefer that you didn't submit this

(3 inline comments)

Thanks.
(this is only a partial review)

....................................................
File vdsm_hooks/qemucmdline/before_vm_start.py
Line 46: def addQemuNsSupport(domXML):
"addQemuNs" is a better name in my opinion.

Line 76:         qemu_cmdline = eval(os.environ['qemu_cmdline'])
I'm afraid of eval. Could you use json.loads instead?

Line 79:         qcmdline_tag = startInject(domxml)
it would be clearer if you create the new elelement and add it to the domain in one shot - no need to have separate start/stop functions.

--
To view, visit http://gerrit.ovirt.org/6969
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I48477926d271bb9f6ae74f79de5d8c4d37b87caa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Deepak C Shetty <deepakcs at linux.vnet.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Deepak C Shetty <deepakcs at linux.vnet.ibm.com>


More information about the vdsm-patches mailing list