Change in vdsm[master]: Added support for iothreads

tjelinek at redhat.com tjelinek at redhat.com
Mon Apr 13 13:22:31 UTC 2015


Tomas Jelinek has posted comments on this change.

Change subject: Added support for iothreads
......................................................................


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/39796/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 827:         # the numOfIoThreads has to be passed to the specific disks
Line 828:         # so they can set the pinning up
Line 829:         if 'numOfIoThreads' in self.conf:
Line 830:             if drv['type'] == 'disk' and drv['iface'] == 'virtio':
Line 831:                 drv['numOfIoThreads'] = self.conf['numOfIoThreads']
> Indeed the root issue here is the (fragile and convoluted) assignement of i
I don't think this should concern us since typically (by default) you will have only one ioThread anyway :) And after restart it will get distributed properly anyway.

So, what about this: 
- make a method like "setupPinToIoThread" this will expect the index to be already set up for the drv and will set the pin to iothread properly (e.g. will remove this logic from the Drive class)
- now either be careful to call it everywhere the updateDriveIndex is called (2 places) or to create a new method like "updateDrivePositionalParams" which will first call the updateDriveIndex and than the setupPinToIoThread (and of course replace the call to updateDriveIndex by this new method)

what do you think?
Line 832: 
Line 833:     def __legacyDrives(self):
Line 834:         """
Line 835:         Backward compatibility for qa scripts that specify direct paths.


https://gerrit.ovirt.org/#/c/39796/2/vdsm/virt/vmxml.py
File vdsm/virt/vmxml.py:

Line 162: 
Line 163:         self.dom.appendChildWithArgs('name', text=self.conf['vmName'])
Line 164:         self.dom.appendChildWithArgs('uuid', text=self.conf['vmId'])
Line 165:         if 'numOfIoThreads' in self.conf:
Line 166:             ioThreads = self.conf['numOfIoThreads']
> it is fine and also considered good style to break lines inside parenthesis
ok, I will break it up
Line 167:             self.dom.appendChildWithArgs('iothreads', text=str(ioThreads))
Line 168:         memSizeKB = str(int(self.conf.get('memSize', '256')) * 1024)
Line 169:         self.dom.appendChildWithArgs('memory', text=memSizeKB)
Line 170:         self.dom.appendChildWithArgs('currentMemory', text=memSizeKB)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576e5af72869ac03d09bdc2d6e6f16d5b06b5aed
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <tjelinek at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjelinek 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