Change in vdsm[ovirt-3.5]: json-rpc: fix the Host.getVMList return value

Hello Francesco Romani,

I'd like you to do a code review.  Please visit

https://gerrit.ovirt.org/38322

to review the following change.

Change subject: json-rpc: fix the Host.getVMList return value
......................................................................

json-rpc: fix the Host.getVMList return value

Host.getVMList() should return either VmDefinition or
VmShortStatus depending on the parameter fullStatus.

The return value in case fullStatus=False was wrong,
as it was just a list of UUIDs, while VmShortStatus
is supposed to be a dictionary holding (at least)
vmId and vmStatus.

This change made life harder to Engine monitoring code.
Missing the status, Engine's monitoring had no choice
but to call Vm.getStats() after each getVmList(), and that
has a not-negligible performance cost.

This patch makes the JSON-RPC output compliant to the
schema, to what Engine's monitoring expects and also
to XML-RPC output.

Change-Id: I2b8b2ed205385f4bdd341cdc576b44312c3fc117
Bug-Url: https://bugzilla.redhat.com/1196327
Signed-off-by: Francesco Romani 
---
M vdsm/rpc/Bridge.py
1 file changed, 1 insertion(+), 8 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/38322/1

diff --git a/vdsm/rpc/Bridge.py b/vdsm/rpc/Bridge.py
index e4b12aa..e1781a2 100644
--- a/vdsm/rpc/Bridge.py
+++ b/vdsm/rpc/Bridge.py
@@ -333,13 +333,6 @@
     return API.Global().getVMList(True, vmList)

-def Host_getVMList_Ret(ret):
-    """
-    Just return a list of VM UUIDs
-    """
-    return [v['vmId'] for v in ret['vmList']]
-
-
 def StoragePool_getInfo_Ret(ret):
     """
     The result contains two data structures which must be merged
@@ -409,7 +402,7 @@
     'Host_getStorageRepoStats': {'ret': Host_getStorageRepoStats_Ret},
     'Host_startMonitoringDomain': {},
     'Host_stopMonitoringDomain': {},
-    'Host_getVMList': {'call': Host_getVMList_Call, 'ret': Host_getVMList_Ret},
+    'Host_getVMList': {'call': Host_getVMList_Call, 'ret': 'vmList'},
     'Host_getVMFullList': {'call': Host_getVMFullList_Call, 'ret': 'vmList'},
     'Host_getAllVmStats': {'ret': 'statsList'},
     'Host_setupNetworks': {'ret': 'status'},

[e-mail] has posted comments on this change.

Change subject: json-rpc: fix the Host.getVMList return value
......................................................................

Patch Set 1: Verified-1

* Update tracker::#1196327::OK
* Check Bug-Url::OK
* Check Public Bug::#1196327::OK, public bug
* Check Product::#1196327::OK, Correct product oVirt
* Check TR::#1196327::OK, correct target release 3.5.2
* Check merged to previous::WARN, Still open on branches master

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: json-rpc: fix the Host.getVMList return value
......................................................................

Patch Set 1: Code-Review+2

[Piotr Kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: json-rpc: fix the Host.getVMList return value
......................................................................

Patch Set 1: Code-Review+1

[Piotr Kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: json-rpc: fix the Host.getVMList return value
......................................................................

Patch Set 1: Verified+1

Verified by Omer on master branch.

[Oved Ourfali]

Oved Ourfali has posted comments on this change.

Change subject: json-rpc: fix the Host.getVMList return value
......................................................................

Patch Set 1:

Rerun-Hooks: all

[e-mail] has posted comments on this change.

Change subject: json-rpc: fix the Host.getVMList return value
......................................................................

Patch Set 1: -Verified

* Update tracker::#1196327::OK
* Check Bug-Url::OK
* Check Public Bug::#1196327::OK, public bug
* Check Product::#1196327::OK, Correct product oVirt
* Check TR::#1196327::OK, correct target release 3.5.2
* Check merged to previous::OK, change not open on any previous branch

Yaniv Bronhaim has submitted this change and it was merged.

Change subject: json-rpc: fix the Host.getVMList return value
......................................................................

json-rpc: fix the Host.getVMList return value

Host.getVMList() should return either VmDefinition or
VmShortStatus depending on the parameter fullStatus.

The return value in case fullStatus=False was wrong,
as it was just a list of UUIDs, while VmShortStatus
is supposed to be a dictionary holding (at least)
vmId and vmStatus.

This change made life harder to Engine monitoring code.
Missing the status, Engine's monitoring had no choice
but to call Vm.getStats() after each getVmList(), and that
has a not-negligible performance cost.

This patch makes the JSON-RPC output compliant to the
schema, to what Engine's monitoring expects and also
to XML-RPC output.

Change-Id: I2b8b2ed205385f4bdd341cdc576b44312c3fc117
Bug-Url: https://bugzilla.redhat.com/1196327
Signed-off-by: Francesco Romani 
Reviewed-on: https://gerrit.ovirt.org/38322
Reviewed-by: Dan Kenigsberg 
Reviewed-by: Piotr Kliczewski 
Tested-by: Piotr Kliczewski 
---
M vdsm/rpc/Bridge.py
1 file changed, 1 insertion(+), 8 deletions(-)

Approvals:
  Piotr Kliczewski: Verified; Looks good to me, but someone else must approve
  Dan Kenigsberg: Looks good to me, approved

[e-mail] has posted comments on this change.

Change subject: json-rpc: fix the Host.getVMList return value
......................................................................

Patch Set 2:

* Update tracker::#1196327::OK
* Check TR::#1196327::OK
* Set MODIFIED::bug 1196327::::#1196327::::OK

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: json-rpc: fix the Host.getVMList return value
......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.org/job/vdsm_3.5_create-rpms-fc20-x86_64_merged/168/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_3.5_create-rpms-el6-x86_64_merged/171/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_3.5_create-rpms-el7-x86_64_merged/171/ : SUCCESS