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

msivak at redhat.com msivak at redhat.com
Thu Feb 4 13:27:15 UTC 2016


Martin Sivák has posted comments on this change.

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


Patch Set 4:

(5 comments)

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

Line 2593:           'guaranteed': 'VmDiskDeviceIoTuneParams',
Line 2594:           'maximum': 'VmDiskDeviceIoTuneParams'}}
Line 2595: 
Line 2596: ##
Line 2597: # @VmIoTuneLimits:
Wouldn't this be better joined to the VmDiskDeviceTuneLimits structure? We could use inheritance there.. sadly we do not support it.
Line 2598: #
Line 2599: # Extra parameters for VM disk devices.
Line 2600: #
Line 2601: # @domainID:    #optional DomainID


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

Line 2454:             metadata_modified = True
Line 2455:             self._vcpuLimit = params.pop('vcpuLimit')
Line 2456: 
Line 2457:         if 'ioTune' in params:
Line 2458:             # convert [VmIoTuneLimits] to [VmDiskDeviceTuneLimits] structure
You do not need the separate structure definition for metadata. The jsonapi file describes the external api only.
Line 2459:             ioTuneParams = params["ioTune"]
Line 2460: 
Line 2461:             for ioTune in ioTuneParams:
Line 2462:                 self.log.debug("IoTuneParams: %s", str(ioTuneParams))


Line 2458:             # convert [VmIoTuneLimits] to [VmDiskDeviceTuneLimits] structure
Line 2459:             ioTuneParams = params["ioTune"]
Line 2460: 
Line 2461:             for ioTune in ioTuneParams:
Line 2462:                 self.log.debug("IoTuneParams: %s", str(ioTuneParams))
You do not have to compute the path again when it is already provided (or the name).
Line 2463:                 domainId = ioTune["domainId"]
Line 2464:                 poolId = ioTune["poolId"]
Line 2465:                 imageId = ioTune["imageId"]
Line 2466:                 volumeId = ioTune["volumeId"]


Line 2469:                     domainId, poolId, imageId, volumeId)
Line 2470: 
Line 2471:                 self.log.debug("Returned path: %s", path)
Line 2472: 
Line 2473:                 del ioTune["domainId"]
It is not necessary to remove those from the metadata. Maybe they will come handy in future.
Line 2474:                 del ioTune["poolId"]
Line 2475:                 del ioTune["imageId"]
Line 2476:                 del ioTune["volumeId"]
Line 2477: 


Line 2565:     def getIoTune(self):
Line 2566:         resultList = []
Line 2567: 
Line 2568:         for device in self._devices[hwclass.DISK]:
Line 2569:             dom = device.getXML()
I think this will give you the currently configured value in vdsm, not the value libvirt sees. But vdsm considers itself to be the owner of libvirt so it might not matter.
Line 2570:             io_dom_list = dom.getElementsByTagName("iotune")
Line 2571: 
Line 2572:             if not io_dom_list:
Line 2573:                 continue


-- 
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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Andrej Krejcir <akrejcir 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