Change in vdsm[master]: introducing virtio-scsi support

fsimonce at redhat.com fsimonce at redhat.com
Mon Jun 10 14:14:05 UTC 2013


Federico Simoncelli has posted comments on this change.

Change subject: introducing virtio-scsi support
......................................................................


Patch Set 5: (1 inline comment)

....................................................
File vdsm/vm.py
Line 1488:             driverAttrs['cache'] = self.cache
Line 1489: 
Line 1490:             self._setErrorPolicy(driverAttrs)
Line 1491:             diskelem.appendChildWithArgs('driver', **driverAttrs)
Line 1492:         elif self.device == 'lun':
Actually I'm a fan of the previous patch set. I don't think that 'lun' deserves a specific elif. Up until now the flow went through the "disk" code also for luns and there wasn't anything wrong with that. Moreover I don't think we should forge static values.
For example here you're tying the luns to the native aio, if we discover that there's a better aio for block devices we'll have to remember to change this too.
You're also silently forcing the "raw" format, if the "qcow2" format is not supported on luns (even though it's legitimate) then we should raise an error (but this is new, in the old flow we weren't doing it).

I can't see any advantage in this new patch set, it's just duplicated logic/values.
Line 1493:             driverAttrs = {'name': 'qemu'}
Line 1494:             driverAttrs['io'] = 'native'
Line 1495:             driverAttrs['type'] = 'raw'
Line 1496:             driverAttrs['cache'] = self.cache


-- 
To view, visit http://gerrit.ovirt.org/14681
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If624064269328253dbc3cb32a72e20f01cdb0958
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list