9:26 a.m.
Ala Hino has uploaded a new change for review.
Change subject: gluster:refactor: User specified backup servers option
......................................................................
gluster:refactor: User specified backup servers option
Move handling user specified backup servers option from
_get_backup_servers_option method to options property.
Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
Signed-off-by: Ala Hino <ahino(a)redhat.com>
---
M vdsm/storage/storageServer.py
1 file changed, 5 insertions(+), 5 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/03/43003/1
diff --git a/vdsm/storage/storageServer.py b/vdsm/storage/storageServer.py
index 3d62694..e71b88c 100644
--- a/vdsm/storage/storageServer.py
+++ b/vdsm/storage/storageServer.py
@@ -297,7 +297,11 @@
@property
def options(self):
- backup_servers_option = self._get_backup_servers_option()
+ if "backup-volfile-servers" in self._options:
+ self.log.warn("Using user specified backup-volfile-servers
option")
+ backup_servers_option = ""
+ else:
+ backup_servers_option = self._get_backup_servers_option()
return ",".join(
p for p in (self._options, backup_servers_option) if p)
@@ -313,10 +317,6 @@
raise se.UnsupportedGlusterVolumeReplicaCountError(replicaCount)
def _get_backup_servers_option(self):
- if "backup-volfile-servers" in self._options:
- self.log.warn("Using user specified backup-volfile-servers
option")
- return ""
-
servers = [brick.split(":")[0] for brick in
self.volinfo['bricks']]
servers.remove(self._volfileserver)
if not servers:
--
To view, visit https://gerrit.ovirt.org/43003
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com>
9:26 a.m.
automation(a)ovirt.org has posted comments on this change.
Change subject: gluster:refactor: User specified backup servers option
......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])
--
To view, visit https://gerrit.ovirt.org/43003
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
9:26 a.m.
Ala Hino has posted comments on this change.
Change subject: gluster:refactor: User specified backup servers option
......................................................................
Patch Set 1: Verified+1
--
To view, visit https://gerrit.ovirt.org/43003
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
10:36 a.m.
Nir Soffer has posted comments on this change.
Change subject: gluster:refactor: User specified backup servers option
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://gerrit.ovirt.org/43003
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
5:47 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: gluster:refactor: User specified backup servers option
......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/43003/1//COMMIT_MSG
Commit Message:
Line 6:
Line 7: gluster:refactor: User specified backup servers option
Line 8:
Line 9: Move handling user specified backup servers option from
Line 10: _get_backup_servers_option method to options property.
can you explain why the suggested code is better than the original one?
Line 11:
Line 12: Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
--
To view, visit https://gerrit.ovirt.org/43003
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
3:26 a.m.
Ala Hino has posted comments on this change.
Change subject: gluster:refactor: User specified backup servers option
......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/43003/1//COMMIT_MSG
Commit Message:
Line 6:
Line 7: gluster:refactor: User specified backup servers option
Line 8:
Line 9: Move handling user specified backup servers option from
Line 10: _get_backup_servers_option method to options property.
can you explain why the suggested code is better than the original
one?
To address Nir's comment:
"
This check should be in options() - you should not access self._options in many places,
better access it only in options() and let other code access self.options.
"
Provided here:
https://gerrit.ovirt.org/#/c/41931/40/vdsm/storage/storageServer.py
Line 11:
Line 12: Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
--
To view, visit https://gerrit.ovirt.org/43003
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
5:31 a.m.
Nir Soffer has posted comments on this change.
Change subject: gluster:refactor: User specified backup servers option
......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/43003/1//COMMIT_MSG
Commit Message:
Line 6:
Line 7: gluster:refactor: User specified backup servers option
Line 8:
Line 9: Move handling user specified backup servers option from
Line 10: _get_backup_servers_option method to options property.
To address Nir's comment:
Additionally,
get_backup_servers_option() should do one thing. If we don't need to get this option,
we should not call it. Having the logic in a higher level make the code easier to follow.
Line 11:
Line 12: Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
--
To view, visit https://gerrit.ovirt.org/43003
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
5:39 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: gluster:refactor: User specified backup servers option
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.ovirt.org/43003
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
5:39 a.m.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: gluster:refactor: User specified backup servers option
......................................................................
gluster:refactor: User specified backup servers option
Move handling user specified backup servers option from
_get_backup_servers_option method to options property.
Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
Signed-off-by: Ala Hino <ahino(a)redhat.com>
Reviewed-on: https://gerrit.ovirt.org/43003
Continuous-Integration: Jenkins CI
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M vdsm/storage/storageServer.py
1 file changed, 5 insertions(+), 5 deletions(-)
Approvals:
Nir Soffer: Looks good to me, but someone else must approve
Jenkins CI: Passed CI tests
Dan Kenigsberg: Looks good to me, approved
Ala Hino: Verified
--
To view, visit https://gerrit.ovirt.org/43003
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
5:39 a.m.
automation(a)ovirt.org has posted comments on this change.
Change subject: gluster:refactor: User specified backup servers option
......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.
--
To view, visit https://gerrit.ovirt.org/43003
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4238328cfe230852182a1d997bd51b00f686f4a8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
3219
days inactive
3221
days old
vdsm-patches@lists.fedorahosted.org
9 comments
4 participants
participants (4)
-
ahino＠redhat.com -
automation＠ovirt.org -
Dan Kenigsberg -
Nir Soffer