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