Change in vdsm[master]: vm: avoid to reply with half-baked statistics

fromani at redhat.com fromani at redhat.com
Fri Mar 28 08:44:04 UTC 2014


Francesco Romani has posted comments on this change.

Change subject: vm: avoid to reply with half-baked statistics
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.ovirt.org/#/c/25803/3/lib/vdsm/define.py
File lib/vdsm/define.py:

Line 139:         'code': 61,
Line 140:         'message': 'Failed to set Hosted Engine HA policy'}},
Line 141:     'statsErr': {'status': {
Line 142:         'code': 62,
Line 143:         'message': 'Failed to gather statistics'}},
> please doublecheck and probably handle the new return code in VdcUpdateRunt
Backward compatibility will be the top priority. Will find a way to work nicely with existing engines.
Line 144:     'recovery': {'status': {
Line 145:         'code': 99,
Line 146:         'message': 'Recovering from crash or Initializing'}},
Line 147: }


http://gerrit.ovirt.org/#/c/25803/3/vdsm/API.py
File vdsm/API.py:

Line 361:         v = self._cif.vmContainer.get(self._UUID)
Line 362:         if not v:
Line 363:             return errCode['noVM']
Line 364:         stats = v.getStats().copy()
Line 365:         if not stats:
> hm, what about partial stats? if the exception occurs later in the stats ga
Yep, this was actually a typo of mine.

The idea was to discard the partial data, but if turns out it is better to return partial data and error code, I've no objections against this.
Line 366:             return errCode['statsErr']
Line 367:         stats['vmId'] = self._UUID
Line 368:         return {'status': doneCode, 'statsList': [stats]}
Line 369: 


http://gerrit.ovirt.org/#/c/25803/3/vdsm/vm.py
File vdsm/vm.py:

Line 2820:             self.log.error("Error fetching vm stats", exc_info=True)
Line 2821:         return stats
Line 2822: 
Line 2823:     def _getStatsInternal(self):
Line 2824:         # used by API.Vm.getStats
> the comment doesn't seem to be true actually, the only one calling this is 
Ack, will add to my TODO.
Line 2825: 
Line 2826:         def _getGuestStatus():
Line 2827:             GUEST_WAIT_TIMEOUT = 60
Line 2828:             now = time.time()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65197459cd183af5e7a1634a5ffb01719364a070
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofrenkel at redhat.com>
Gerrit-Reviewer: Roy Golan <rgolan at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list