Piotr Kliczewski has posted comments on this change.

Change subject: [WIP]Use jsonrpc during migration of vms
......................................................................


Patch Set 4:

(3 comments)

https://gerrit.ovirt.org/#/c/36701/4/vdsm/virt/migration.py
File vdsm/virt/migration.py:

Line 50: 
Line 51: METHOD_ONLINE = 'online'
Line 52: 
Line 53: 
Line 54: class _MigrationDestServer(object):
> +1
+1
Line 55: 
Line 56:     def __init__(self, client):
Line 57:         self._client = client
Line 58: 


Line 55: 
Line 56:     def __init__(self, client):
Line 57:         self._client = client
Line 58: 
Line 59:     def _callMethod(self, methodName, *args):
I would move that method to JsonRpcClient. There is similar method which waits 
for response but we can't provide timeout.
Line 60:         req = JsonRpcRequest(methodName, args, reqId=str(uuid4()))
Line 61:         call = self._client.call_async(req)
Line 62:         call.wait(CALL_TIMEOUT)
Line 63: 


Line 63: 
Line 64:         resp = call.responses[0]
Line 65: 
Line 66:         if resp.error is not None:
Line 67:             raise JsonRpcError(resp.error['code'], 
resp.error['message'])
Why JsonRpcError? It is processes by JsonRpcServer in order to marshal error 
part of the message. Is there any value of using it here?
Line 68: 
Line 69:         return resp.result
Line 70: 
Line 71:     def destroy(self, vmId):


-- 
To view, visit https://gerrit.ovirt.org/36701
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie38334e6cdcc4d7899bd5e836b3196567fc0bfd8
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykap...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuzn...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to