Change in vdsm[master]: Add new api to get the IO tune policies for all vms

Francesco Romani has posted comments on this change.

Change subject: Add new api to get the IO tune policies for all vms
......................................................................


Patch Set 2: Code-Review-1

(5 comments)

-1 for visibility

https://gerrit.ovirt.org/#/c/63748/2/vdsm/clientIF.py
File vdsm/clientIF.py:

Line 443: 
Line 444:     def getAllVmIoTunePolicies(self):
Line 445:         vm_io_tune_policies = {}
Line 446:         for v in self.vmContainer.values():
Line 447:                 vm_io_tune_policies[v.id] =
{'policy':v.getIoTunePolicy(), 'current_values': v.getIoTune()}
this saves us the roundtrips. It is one improvement, but not sure how significant it is.
First, we need a benchmark to compare and to assess our progress.
Then, we could compare the gains.
If this change is not enough, we can evolve from there, still guided by the benchmark.
Line 448:         return vm_io_tune_policies
Line 449: 
Line 450:     def createStompClient(self, client_socket):
Line 451:         if 'jsonrpc' in self.bindings:


https://gerrit.ovirt.org/#/c/63748/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:

PS2, Line 2579: getIoTunePolicyResponse
what's the need for this?


PS2, Line 2601: getIoTuneResponse
ditto


Line 2624: 
Line 2625:             except libvirt.libvirtError as e:
Line 2626:                 self.log.exception("getVmIoTune failed")
Line 2627:                 if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
Line 2628:                     self.logger.error('noVM')
not sure you wanna keep this.
Line 2629:                     return response.error('noVM')
Line 2630:                 else:
Line 2631:                     self.logger.error('updateIoTuneErr', e.message)
Line 2632:                     return response.error('updateIoTuneErr',
e.message)


Line 2627:                 if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
Line 2628:                     self.logger.error('noVM')
Line 2629:                     return response.error('noVM')
Line 2630:                 else:
Line 2631:                     self.logger.error('updateIoTuneErr', e.message)
ditto
Line 2632:                     return response.error('updateIoTuneErr',
e.message)
Line 2633: 
Line 2634:         return response.success(ioTuneList=resultList)
Line 2635: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I16ead268367901ae85e47fb71104e23705f0e0e1
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Jenny Tokar <jtokar(a)redhat.com&gt;
Gerrit-Reviewer: Andrej Krejcir <akrejcir(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenny Tokar <jtokar(a)redhat.com&gt;
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com&gt;
Gerrit-Reviewer: Phillip Bailey <phbailey(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Roman Mohr <rmohr(a)redhat.com&gt;
Gerrit-Reviewer: Roy Golan <rgolan(a)redhat.com&gt;
Gerrit-Reviewer: Yanir Quinn <yquinn(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes