Change in vdsm[master]: Apply storage QoS on running VM

fromani at redhat.com fromani at redhat.com
Mon Feb 15 16:09:13 UTC 2016


Francesco Romani has posted comments on this change.

Change subject: Apply storage QoS on running VM
......................................................................


Patch Set 8: Code-Review-1

(7 comments)

few questions inside. -1 for visibility

https://gerrit.ovirt.org/#/c/52746/8/lib/api/vdsmapi-schema.json
File lib/api/vdsmapi-schema.json:

Line 8867: #
Line 8868: # Returns:
Line 8869: # list of VmDiskDeviceTuneParams objects
Line 8870: #
Line 8871: # Since: 4.15.0
since 4.18.0 most likely. Or do we need to backport this?
Line 8872: ##
Line 8873: {'command': {'class': 'VM', 'name': 'getIoTune'},
Line 8874:  'data': {'vmID': 'UUID'},
Line 8875:  'returns': ['VmDiskDeviceTuneParams']}


https://gerrit.ovirt.org/#/c/52746/8/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2386: self._devices[hwclass.DISK][:]:
Works. Better alternative:

self.getDiskDevices():


Line 2392: asDomainID and hasPoolID and hasImageID and hasVolumeID
hasAnyId = isVdsmImage(dev)

isVdsmImage is in vdsm/virt/utils.py (already imported and used in this module)


Line 2394:                     (not hasDomainID or dev.domainID == domainId) and
         :                     (not hasPoolID or dev.poolID == poolId) and
         :                     (not hasImageID or dev.imageID == imageId) and
         :                     (not hasVolumeID or dev.volumeID == volumeId)):
so it is possible that up to three of those IDs may be False-ish? How so?


Line 2466:                 domainId = ioTune.get("domainId")
         :                 poolId = ioTune.get("poolId")
         :                 imageId = ioTune.get("imageId")
         :                 volumeId = ioTune.get("volumeId")
same comment as per line 2394-2397: Doesn't feel right if one or more of those are missing. Could you please provide an example case on which this is actaully OK?


Line 2574: res = {'name': device.name, 'path': device.path, 'ioTune': io_tune}
We don't need this temporary


Line 2580:                     return response.error('noVM')
Line 2581:                 else:
Line 2582:                     return response.error('updateIoTuneErr', e.message)
Line 2583: 
Line 2584:         return {'status': doneCode, 'ioTune': resultList}
return response.success(ioTune=resultList)
Line 2585: 
Line 2586:     def setIoTune(self, tunables):
Line 2587:         for io_tune_change in tunables:
Line 2588:             device_name = io_tune_change.get('name', None)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f5fcb8705c974f1d91c90cdb3158d2e8dd314f8
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Andrej Krejcir <akrejcir at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák <msivak at redhat.com>
Gerrit-Reviewer: Roman Mohr <rmohr at redhat.com>
Gerrit-Reviewer: Roy Golan <rgolan at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list