Deepak C Shetty has uploaded a new change for review.
Change subject: Support for QEMU cmdline hook ......................................................................
Support for QEMU cmdline hook
This patch adds support for injecting QEMU cmdline via VDSM hook. It exploits libvirt's qemu:commandline facility available in the qemu xml namespace.
Change-Id: I48477926d271bb9f6ae74f79de5d8c4d37b87caa Signed-off-by: Deepak C Shetty deepakcs@linux.vnet.ibm.com --- M configure.ac M vdsm.spec.in M vdsm_hooks/Makefile.am A vdsm_hooks/qemucmdline/Makefile.am A vdsm_hooks/qemucmdline/README A vdsm_hooks/qemucmdline/before_vm_start.py 6 files changed, 153 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/69/6969/1 -- To view, visit http://gerrit.ovirt.org/6969 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I48477926d271bb9f6ae74f79de5d8c4d37b87caa Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com
Deepak C Shetty has posted comments on this change.
Change subject: Support for QEMU cmdline hook ......................................................................
Patch Set 1: Verified
-- 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@linux.vnet.ibm.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com
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@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com
Deepak C Shetty has posted comments on this change.
Change subject: Support for QEMU cmdline hook ......................................................................
Patch Set 1: No score
(3 inline comments)
Ran into issues using json.loads. appreciate suggestions.
.................................................... File vdsm_hooks/qemucmdline/before_vm_start.py Line 46: def addQemuNsSupport(domXML): Done
Line 76: qemu_cmdline = eval(os.environ['qemu_cmdline']) Using json.loads gives me this exception... It does like the str(dict) format.
Thread-49::DEBUG::2012-08-07 19:41:15,953::__init__::1249::Storage.Misc.excCmd::(_log) FAILED: <err> = 'qemu_cmdline: [unexpected error]: Traceback (most recent call last):\n File "/usr/libexec/vdsm/hooks/before_vm_start/50_qemucmdline", line 74, in <module>\n qemu_cmdline = json.loads(os.environ['qemu_cmdline'])\n File "/usr/lib64/python2.7/json/__init__.py", line 326, in loads\n return _default_decoder.decode(s)\n File "/usr/lib64/python2.7/json/decoder.py", line 366, in decode\n obj, end = self.raw_decode(s, idx=_w(s, 0).end())\n File "/usr/lib64/python2.7/json/decoder.py", line 382, in raw_decode\n obj, end = self.scan_once(s, idx)\nValueError: Expecting property name: line 1 column 1 (char 1)\n\n'; <rc> = 2 Thread-49::INFO::2012-08-07 19:41:15,954::hooks::76::root::(_runHooksDir) qemu_cmdline: [unexpected error]: Traceback (most recent call last): File "/usr/libexec/vdsm/hooks/before_vm_start/50_qemucmdline", line 74, in <module> qemu_cmdline = json.loads(os.environ['qemu_cmdline']) File "/usr/lib64/python2.7/json/__init__.py", line 326, in loads return _default_decoder.decode(s) File "/usr/lib64/python2.7/json/decoder.py", line 366, in decode obj, end = self.raw_decode(s, idx=_w(s, 0).end()) File "/usr/lib64/python2.7/json/decoder.py", line 382, in raw_decode obj, end = self.scan_once(s, idx) ValueError: Expecting property name: line 1 column 1 (char 1)
Line 79: qcmdline_tag = startInject(domxml) Done
-- 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@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com
Deepak C Shetty has posted comments on this change.
Change subject: Support for QEMU cmdline hook ......................................................................
Patch Set 1: (1 inline comment)
moved to json.loads
.................................................... File vdsm_hooks/qemucmdline/before_vm_start.py Line 76: qemu_cmdline = eval(os.environ['qemu_cmdline']) Done.
Modified to use json.loads. This will put addnl constraint on the developer using the hook, as he/she has to pass the custom arg in json dict.
-- 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@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: Support for QEMU cmdline hook ......................................................................
Patch Set 2: I would prefer that you didn't submit this
(2 inline comments)
.................................................... File vdsm_hooks/qemucmdline/before_vm_start.py Line 43: For eg: qemu_cmdline='{"-cdrom":"<path/to/iso>", ...}' what about cmdline args that does not have a value (think "-S")? what about the order? In some cases it is significant! I think that a simple list is better suited to the task.
.................................................... File vdsm_hooks/qemucmdline/README Line 9: The os environ 'qemu_cmdline' points to a json dict having one trailing whitespace, here and elsewhere.
-- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com
Deepak C Shetty has posted comments on this change.
Change subject: Support for QEMU cmdline hook ......................................................................
Patch Set 2: (2 inline comments)
.................................................... File vdsm_hooks/qemucmdline/before_vm_start.py Line 43: For eg: qemu_cmdline='{"-cdrom":"<path/to/iso>", ...}' Done
.................................................... File vdsm_hooks/qemucmdline/README Line 9: The os environ 'qemu_cmdline' points to a json dict having one Done
-- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for QEMU cmdline hook ......................................................................
Patch Set 3: Verified
-- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Support for QEMU cmdline hook ......................................................................
Patch Set 4: Looks good to me, approved
annoying whitespace removed.
-- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for QEMU cmdline hook ......................................................................
Patch Set 3: No score
Build Started http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/34/
-- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for QEMU cmdline hook ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/34/ : SUCCESS
-- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for QEMU cmdline hook ......................................................................
Patch Set 4:
Build Started http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/261/
-- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for QEMU cmdline hook ......................................................................
Patch Set 4:
Build Successful
http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/261/ : SUCCESS
-- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for QEMU cmdline hook ......................................................................
Patch Set 4: Verified
-- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Support for QEMU cmdline hook ......................................................................
Support for QEMU cmdline hook
This patch adds support for injecting QEMU cmdline via VDSM hook. It exploits libvirt's qemu:commandline facility available in the qemu xml namespace.
v2: code refactoring as per dan's comments moved from eval to json.loads v3: moved from json dict to json list
Change-Id: I48477926d271bb9f6ae74f79de5d8c4d37b87caa Signed-off-by: Deepak C Shetty deepakcs@linux.vnet.ibm.com --- M configure.ac M vdsm.spec.in M vdsm_hooks/Makefile.am A vdsm_hooks/qemucmdline/Makefile.am A vdsm_hooks/qemucmdline/README A vdsm_hooks/qemucmdline/before_vm_start.py 6 files changed, 143 insertions(+), 1 deletion(-)
Approvals: Dan Kenigsberg: Looks good to me, approved Deepak C Shetty: Verified
-- To view, visit http://gerrit.ovirt.org/6969 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: I48477926d271bb9f6ae74f79de5d8c4d37b87caa Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
vdsm-patches@lists.fedorahosted.org