Adam Litke has posted comments on this change.

Change subject: VolumeMetadata: Introduce class structure
......................................................................


Patch Set 20:

(10 comments)

https://gerrit.ovirt.org/#/c/41844/20/tests/sdm_indirection_tests.py
File tests/sdm_indirection_tests.py:

Line 321:         pass
Line 322: 
Line 323: 
Line 324: class FakeFileVolume(fileVolume.FileVolume):
Line 325:     MetadataClass = FakeFileVolumeMetadata
> metadataClass
Done
Line 326: 
Line 327:     def __init__(self):
Line 328:         self.md = self.MetadataClass()
Line 329: 


Line 328:         self.md = self.MetadataClass()
Line 329: 
Line 330: 
Line 331: class FakeBlockVolume(blockVolume.BlockVolume):
Line 332:     MetadataClass = FakeVolumeMetadata
> Same.
Done
Line 333: 
Line 334:     def __init__(self):
Line 335:         self.md = self.MetadataClass()
Line 336: 


Line 331: class FakeBlockVolume(blockVolume.BlockVolume):
Line 332:     MetadataClass = FakeVolumeMetadata
Line 333: 
Line 334:     def __init__(self):
Line 335:         self.md = self.MetadataClass()
> self.metadataClass()
Done
Line 336: 
Line 337: 
Line 338: class RedirectionChecker(object):
Line 339:     """


https://gerrit.ovirt.org/#/c/41844/20/vdsm/storage/blockVolume.py
File vdsm/storage/blockVolume.py:

Line 74
Line 75
Line 76
Line 77
Line 78
> Create md here
ok


Line 68: log = logging.getLogger('Storage.Volume')
Line 69: rmanager = rm.ResourceManager.getInstance()
Line 70: 
Line 71: 
Line 72: class BlockVolumeMetadata(volume.VolumeMetadata):
> Please separate class line and first method with blank line
Done
Line 73:     def __init__(self, repoPath, sdUUID, imgUUID, volUUID):
Line 74:         volume.VolumeMetadata.__init__(self, repoPath, sdUUID, imgUUID,
Line 75:                                        volUUID)
Line 76: 


Line 77: 
Line 78: class BlockVolume(volume.Volume):
Line 79:     """ Actually represents a single volume (i.e. part of virtual 
disk).
Line 80:     """
Line 81:     MetadataClass = BlockVolumeMetadata
> metadataClass
Done
Line 82: 
Line 83:     def __init__(self, repoPath, sdUUID, imgUUID, volUUID):
Line 84:         md = self.MetadataClass(repoPath, sdUUID, imgUUID, volUUID)
Line 85:         self.metaoff = None


Line 80:     """
Line 81:     MetadataClass = BlockVolumeMetadata
Line 82: 
Line 83:     def __init__(self, repoPath, sdUUID, imgUUID, volUUID):
Line 84:         md = self.MetadataClass(repoPath, sdUUID, imgUUID, volUUID)
> Why create this before the point it is used?
Done
Line 85:         self.metaoff = None
Line 86:         volume.Volume.__init__(self, md)
Line 87:         self.lvmActivationNamespace = sd.getNamespace(self.sdUUID,
Line 88:                                                       
LVM_ACTIVATION_NAMESPACE)


https://gerrit.ovirt.org/#/c/41844/20/vdsm/storage/fileVolume.py
File vdsm/storage/fileVolume.py:

Line 50:     target, sdUUID = os.path.split(sdPath)
Line 51:     return sdUUID
Line 52: 
Line 53: 
Line 54: class FileVolumeMetadata(volume.VolumeMetadata):
> Add blank line
Done
Line 55:     def __init__(self, repoPath, sdUUID, imgUUID, volUUID):
Line 56:         volume.VolumeMetadata.__init__(self, repoPath, sdUUID, imgUUID,
Line 57:                                        volUUID)
Line 58: 


Line 59: 
Line 60: class FileVolume(volume.Volume):
Line 61:     """ Actually represents a single volume (i.e. part of virtual 
disk).
Line 62:     """
Line 63:     MetadataClass = FileVolumeMetadata
> metadataClass
Done
Line 64: 
Line 65:     def __init__(self, repoPath, sdUUID, imgUUID, volUUID):
Line 66:         md = self.MetadataClass(repoPath, sdUUID, imgUUID, volUUID)
Line 67:         volume.Volume.__init__(self, md)


https://gerrit.ovirt.org/#/c/41844/20/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 143: class Volume(object):
Line 144:     log = logging.getLogger('Storage.Volume')
Line 145: 
Line 146:     def __init__(self, md):
Line 147:         self.md = md
> Why public?
No reason.  Making private.
Line 148:         self.volumePath = None
Line 149:         self.imagePath = None
Line 150:         self.voltype = None
Line 151:         self.validate()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0552bea23b04b9e58e5d2cc7e016103d03194d1a
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Ala Hino <[email protected]>
Gerrit-Reviewer: Freddy Rolland <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to