Francesco Romani has posted comments on this change.

Change subject: ceph: Quick and dirty prototype
......................................................................


Patch Set 2:

(3 comments)

initial comments just to share thoughts

http://gerrit.ovirt.org/#/c/36872/2/vdsm/virt/vmdevices/storage.py
File vdsm/virt/vmdevices/storage.py:

Line 52:     VOLWM_CHUNK_REPLICATE_MULT = 2  # Chunk multiplier during 
replication
Line 53: 
Line 54:     def __new__(cls, conf, log, **kwargs):
Line 55:         """
Line 56:         Create the required drive subclass using "diskType" attribute.
nice!
Line 57: 
Line 58:         For example: "network" -> NetworkDrive
Line 59:         """
Line 60:         diskType = kwargs.get("diskType", "legacy")


Line 390:         diskelem.appendChild(source)
Line 391: 
Line 392:         # TODO: This was copied from Drive.getXML. We should probbaly 
get a
Line 393:         # basic drive element from Drive.getXML and add the specific 
class info
Line 394:         # here.
I'm OK to have a generic method in Drive which creates the basic XML element 
layout, to be used in derived classes.
Probably the cleanest solution, not entirely sure about the end result just 
because (in general and in libvirt) storage drives are among the messiest and 
more complex devices around. To properly model them is not easy.
Line 395: 
Line 396:         targetAttrs = {'dev': self.name}
Line 397:         if self.iface:
Line 398:             targetAttrs['bus'] = self.iface


Line 429:         else:
Line 430:             driverAttrs['error_policy'] = 'stop'
Line 431:         diskelem.appendChildWithArgs('driver', **driverAttrs)
Line 432: 
Line 433:         # IOTune
this should be safe and (hopefully) easy to move in the generic Drive class
Line 434:         if hasattr(self, 'specParams') and 'ioTune' in 
self.specParams:
Line 435:             self._validateIoTuneParams(self.specParams['ioTune'])
Line 436:             iotune = vmxml.Element('iotune')
Line 437:             for key, value in self.specParams['ioTune'].iteritems():


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1aa096ef01bca66db0e6796a54eb573ddeb43865
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to