Nir Soffer has posted comments on this change.

Change subject: gluster: Verify volume is Replica 3
......................................................................


Patch Set 37:

(1 comment)

https://gerrit.ovirt.org/#/c/41931/37/vdsm/storage/storageServer.py
File vdsm/storage/storageServer.py:

Line 298:     @property
Line 299:     def options(self):
Line 300:         backup_servers_option = self._get_backup_servers_option()
Line 301:         return ",".join(p for p in (self._options, 
backup_servers_option) if p)
Line 302: 
> Am I missing something but this doesn't seem related at all to the stated p
This change require access to volume info, which was accessed before by 
_get_backup_servers_option.

Since now we need the volume also in validate(), we the volinfo property was 
introduced.

We can separate the addition of the volinfo property, but until you use it, it 
does not make sense to add it.

Anyway confusing you prove that this patch is doing too much and should be 
broken to smaller parts.
Line 303:     @property
Line 304:     def volinfo(self):
Line 305:         if self._volinfo is None:
Line 306:             self._volinfo = self._get_gluster_volinfo()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fee5d023706e2a0613201a9afef7d285b08442f
Gerrit-PatchSet: 37
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Ala Hino <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Darshan N <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Sahina Bose <[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