Change in vdsm[master]: LiveMerge: Add block job info to VM stats
fromani at redhat.com
fromani at redhat.com
Wed Jun 11 14:37:34 UTC 2014
Francesco Romani has posted comments on this change.
Change subject: LiveMerge: Add block job info to VM stats
......................................................................
Patch Set 5: Code-Review+1
(8 comments)
Federico already addressed my biggest concern.
The second biggest is that I have very bad memories about jobInfo being blocking forever due to a stuck qemu, but I don't know if this may apply to blockJobInfo, and, most importantly, I don't have any alternative to suggest.
The remainder of my comments are minor things which can be addressed by followup patches. Actually, I advice to address them in followup patches and leave this one as is right now.
That said, I'm ok with this patch, then +1
http://gerrit.ovirt.org/#/c/26634/5/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 6105: # @unknown: An unrecognized or unsupported job
Line 6106: #
Line 6107: # @block: A block job
Line 6108: #
Line 6109: # Since: 4.14.4
4.15.0, I think
Line 6110: ##
Line 6111: {'enum': 'JobType', 'data': ['unknown', 'block']}
Line 6112:
Line 6113: ##
Line 6122: # @copy: A blockCopy operation
Line 6123: #
Line 6124: # @commit: A blockCommit operation (backward merge)
Line 6125: #
Line 6126: # Since: 4.14.4
ditto
Line 6127: ##
Line 6128: {'enum': 'BlockJobType', 'data': ['unknown', 'pull', 'copy', 'commit']}
Line 6129:
Line 6130: ##
Line 6145: # @bandwidth: I/O limit in MB/s (zero means no limit)
Line 6146: #
Line 6147: # @imgUUID: The UUID of the image on which this job is operating
Line 6148: #
Line 6149: # Since: 4.14.4
ditto
Line 6150: ##
Line 6151: {'type': 'BlockJobInfo',
Line 6152: 'data': {'id': 'UUID', 'jobType': 'JobType', 'blockJobType': 'BlockJobType',
Line 6153: 'cur': 'int', 'end': 'int', 'bandwidth': 'int', 'imgUUID': 'UUID'}}
Line 6160: # @id: An ID that was associated with the job when it was started
Line 6161: #
Line 6162: # @jobType: The general job type
Line 6163: #
Line 6164: # Since: 4.14.4
ditto
Line 6165: ##
Line 6166: {'type': 'VmJobInfo', 'data': {'id': 'UUID', 'jobType': 'JobType'},
Line 6167: 'union': ['BlockJobInfo']}
Line 6168:
Line 6170: # @VmJobsMap:
Line 6171: #
Line 6172: # A mapping of VM job information indexed by job UUID.
Line 6173: #
Line 6174: # Since: 4.14.4
ditto
Line 6175: ##
Line 6176: {'map': 'VmJobsMap', 'key': 'UUID', 'value': 'VmJobInfo'}
Line 6177:
Line 6178: ##
Line 6252: #
Line 6253: # @displayInfo: Display and graphics device informations.
Line 6254: # (new in version 4.15.0)
Line 6255: #
Line 6256: # @vmJobs: #optional Info about active vm jobs
please add '(new in version 4.15.0)'
Line 6257: #
Line 6258: # Since: 4.10.0
Line 6259: ##
Line 6260: {'type': 'RunningVmStats',
http://gerrit.ovirt.org/#/c/26634/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 439: if info is None:
Line 440: # If we are unable to collect stats we must not return anything at
Line 441: # all since an empty dictionary would be interpreted as vm jobs
Line 442: # finishing.
Line 443: self._log.debug("VmJob stats not available")
IIUC this is a benign case which we know can happen.
If this is the case, please drop this log; otherwise, if this should not happen, please make it a warning.
Line 444: else:
Line 445: stats['vmJobs'] = info
Line 446:
Line 447: def get(self):
Line 5147: def queryBlockJobs(self):
Line 5148: opTypeMap = {libvirt.VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN: 'unknown',
Line 5149: libvirt.VIR_DOMAIN_BLOCK_JOB_TYPE_PULL: 'pull',
Line 5150: libvirt.VIR_DOMAIN_BLOCK_JOB_TYPE_COPY: 'copy',
Line 5151: libvirt.VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT: 'commit'}
Can you extract this mapping and made it a (constant) class attribute?
Line 5152:
Line 5153: # This function is meant to be called from multiple threads (ie.
Line 5154: # VMStatsThread and API calls. The _jobsLock ensures that a cohesive
Line 5155: # data set is returned from each call.
--
To view, visit http://gerrit.ovirt.org/26634
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7278c0f06a2a06c8a8d680cf64746de3f73a389
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi 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