7:55 p.m.
Lei Li has uploaded a new change for review.
Change subject: Add passwd command to vdsm-tool
......................................................................
Add passwd command to vdsm-tool
Move and encapsulate the update-saslpasswd2 from
post section to vdsm-tool function.
Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Signed-off-by: Lei Li <lilei(a)linux.vnet.ibm.com>
---
M vdsm-tool/Makefile.am
A vdsm-tool/passwd.py
M vdsm.spec.in
3 files changed, 48 insertions(+), 7 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/4528/1
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
1:19 p.m.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Add passwd command to vdsm-tool
......................................................................
Add passwd command to vdsm-tool
Change since v3
- Automate this process by calling vdsm-tool
accordingly.
Change since v2
- Processes problem fixs and report the error
for missing file suggested by Saggi.
Change since v1
- Small fix suggested by Mark.
Move and encapsulate set-saslpasswd from post
section into vdsm-tool function.
Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Signed-off-by: Lei Li <lilei(a)linux.vnet.ibm.com>
---
M vdsm-tool/Makefile.am
A vdsm-tool/passwd.py
M vdsm.spec.in
3 files changed, 52 insertions(+), 6 deletions(-)
Approvals:
Dan Kenigsberg: Verified; Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:19 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 14: Verified; Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:13 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 13: Verified
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:13 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 13: Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
12:22 a.m.
ShaoHe Feng has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 13: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
8:59 p.m.
Xu He Jie has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 13: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
7:44 a.m.
Adam Litke has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 13: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
4:33 a.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 12: (1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 46: raise Exception("Set password failed:", err)
Done
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
12:56 a.m.
New subject: Change in vdsm[master]: Add passwd command to vdsm tool
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm tool
......................................................................
Patch Set 12: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 46: raise Exception("Set password failed:", err)
It should probably be Exception("Set password failed: %s" % (err, ))
to set e.message properly.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
2:52 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 13: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
2:34 p.m.
New subject: Change in vdsm[master]: Add passwd command to vdsm tool
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm tool
......................................................................
Patch Set 11: (1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 47: print >> sys.stderr, "Set password failed:", err
How about raise like this?
raise Exception("Set password failed:", err)
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
2:12 p.m.
New subject: Change in vdsm[master]: Add passwd command to vdsm tool
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm tool
......................................................................
Patch Set 11: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 47: print >> sys.stderr, "Set password failed:", err
printing should not replace raising the exception. with this code, vdsm-tool would have a
0 returncode, even though "Set password failed".
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:23 p.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 10: (1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 36: except IOError:
Ah, I see. Thanks! : ) Update soon.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:17 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 10: (1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 36: except IOError:
yes, I was misunderstood. you should have something like
except IOErrorr as e:
if e.errno == errno.ENOENT:
return
raise
that's the meaning of "`return` on errno.ENOENT".
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:07 p.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 10: (1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 36: except IOError:
I modify it based on your comment..
" I should have noticed this earlier, but you should `return` on errno.ENOENT."
To deal with the missing of this file P_VDSM_LIBVIRT_PASSWD.
Please see the comment in the former set, did I misunderstand?
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
12:58 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 10: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 36: except IOError:
?
we said we should *ignore the case of ENOENT, what has happened to that?
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
3:01 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 9: (1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 45: raise Exception("Set password failed: %s" % p.returncode)
I'm pretty sure that saslpasswd2 has multiple interesting modes of failure. printing
the error may help debugging, but I'm not fervently advocating this.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:58 a.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 9: (2 inline comments)
....................................................
File vdsm-tool/passwd.py
Line 35: f = open(constants.P_VDSM_LIBVIRT_PASSWD, 'r')
Dan' comment about pythonic is 'it is more pythonic and less raceful to try to
open, and then catch the IOError'.
I guess you mean should use 'with open' to be more pythonic.
I think 'with open' is necessary to avoid close issue, but it is not an
'must' thing, sometime it depends the context.
Line 42: stderr=subprocess.PIPE, close_fds=True)
Thanks for the link, and what's your point?
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:16 a.m.
ShaoHe Feng has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 9: Fails; I would prefer that you didn't submit this
(2 inline comments)
test, after subprocess.Popen with close_fds=True, father can close file descriptors.
....................................................
File vdsm-tool/passwd.py
Line 35: f = open(constants.P_VDSM_LIBVIRT_PASSWD, 'r')
I think it is better to use ptyonic code.
Line 42: stderr=subprocess.PIPE, close_fds=True)
http://docs.python.org/library/subprocess.html
If close_fds is true, all file descriptors except 0, 1 and 2 will be closed before the
child process is executed.
father process does not close file descriptors. it means it can not read the file
descriptors in subprocess, but father process can.
so you can close the f in father process.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
9:51 p.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 9: (2 inline comments)
....................................................
File vdsm-tool/passwd.py
Line 38: raise
Actually, I have tested with these cases, and it would pass and call the subprocess with
'f' referenced before assignment if this file miss. But you suggested to change
like this, so I thought you might consider this file would be certain exist, we should
pass this case...
Ok, I will change it without this error ENOENT pass.
Line 45: raise Exception("Set password failed: %s" % p.returncode)
Yes, I have the same thought too, and in fact I was trying to code like that. But seems it
would have hang problem if code like WenYi Gao's comment for some reason. And as
explained in the former version, since the input command was already wrapped, the return
error would be 'generic failure' which is a general error from sasl, like the
user doesn't have the right accession. So I choose to raise like this.
If you think we should print stderr, fine, I will modify it...
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:20 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 9: I would prefer that you didn't submit this
(2 inline comments)
....................................................
File vdsm-tool/passwd.py
Line 38: raise
I should have noticed this earlier, but you should `return` on errno.ENOENT.
Please try your code with missing/existing P_VDSM_LIBVIRT_PASSWD.
Line 45: raise Exception("Set password failed: %s" % p.returncode)
as Wenyi Gao mentioned earlier, it would make sense to print stderr in this case.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
4:27 a.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 7: (2 inline comments)
....................................................
File vdsm-tool/passwd.py
Line 41: raise IOError(e.errno, os.strerror(e.errno))
Okey..
Line 48: raise Exception("Set password failed: %s" % p.returncode)
raise err like this would hang up in this case. And since the input command was already
wrapped, the possible return error would be 'generic failure' , like the user
doesn't have the right accession. So I think it would not be more useful to print the
err.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:42 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 7: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 41: raise IOError(e.errno, os.strerror(e.errno))
a simple
raise
is much better, as it does not generate a new exception with a new traceback.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:28 a.m.
Wenyi Gao has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 7: (1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 48: raise Exception("Set password failed: %s" % p.returncode)
raise Exception(("Set password failed: %s" % err)
Maybe more helpful
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:09 a.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 6: (2 inline comments)
With small fixed.
....................................................
File vdsm-tool/passwd.py
Line 37: except IOError, e:
Done
Line 38: raise IOError("Open password file failed:", e)
Done
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
11:04 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 6: I would prefer that you didn't submit this
(2 inline comments)
....................................................
File vdsm-tool/passwd.py
Line 37: except IOError, e:
You need to ignore the case of errno=ENOENT.
There is no point in plain re-raising of exceptions.
Line 38: raise IOError("Open password file failed:", e)
btw, IOError(errno, str) is the only valid construction.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
10:43 a.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 5: (4 inline comments)
....................................................
File vdsm-tool/passwd.py
Line 29: @vdsm_tool_expose("update-saslpasswd2")
Done
Line 34: script = ['/usr/sbin/saslpasswd2', '-p', '-a',
'libvirt', 'vdsm@rhevh']
Done, I have replaced it by a constant in another patch.
Line 42: raise OSError("Execution failed:", e)
Done
Line 48: if err:
Done
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
10:42 a.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 6:
All the comments from Dan fixed.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
4:05 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 5: I would prefer that you didn't submit this
(6 inline comments)
....................................................
File vdsm-tool/passwd.py
Line 29: @vdsm_tool_expose("update-saslpasswd2")
set-saslpasswd would be a more precise name. yeah, spec comment has had the same problem
since ever.
Line 34: script = ['/usr/sbin/saslpasswd2', '-p', '-a',
'libvirt', 'vdsm@rhevh']
vdsm@rhevh should be replaced by a constant, shared by libvirtconnection. (can be done in
a separate patch)
Line 37: print "Password file %s doesn't exist" % PASSWORD_FILE
it is more pythonic and less raceful to try to open, and then catch the IOError.
Line 40: fd = open(PASSWORD_FILE, 'r')
open returns a file, not a file descriptor
Line 42: raise OSError("Execution failed:", e)
I do not see the point in catching one exception, only to raise a less informative, and
more general one.
Line 48: if err:
you are still not checking p.returncode
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
11:10 p.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 4: (2 inline comments)
....................................................
File vdsm-tool/Makefile.am
Line 25: passwd.py
Oh, okey.
BTW, how do you know...? :)
....................................................
File vdsm-tool/passwd.py
Line 42: print >> sys.stderr, "Execution failed:", e
Yeah... That would be better, thanks for pointing that out to make this perfect...
Will do, also for the 'open' issue.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
9:52 p.m.
Xu He Jie has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(2 inline comments)
....................................................
File vdsm-tool/Makefile.am
Line 25: passwd.py
there should use tab, not space
....................................................
File vdsm-tool/passwd.py
Line 42: print >> sys.stderr, "Execution failed:", e
When got exception, it should exit?
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1:28 a.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 1: (2 inline comments)
Seems the directory vdsm-tool already in PEP8 check list.
....................................................
File vdsm-tool/passwd.py
Line 25: PASSWORD_FILE = '/etc/pki/vdsm/keys/libvirt_password'
Thanks for pointing this out! Will do.
Line 30: Update the vdsm password for libvirt
Yes, it's more clear. But It will divided into two lines showed as vdsm-tool help
usage if the comment take too long, so I short it like this...
Maybe we can modify the format of help usage in the vdsm-tool.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
9:43 p.m.
Mark Wu has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 1:
New introduced file should be put into PEP8 WHITELIST. Is it right?
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
9:42 p.m.
Mark Wu has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File vdsm-tool/passwd.py
Line 1: # Copyright 2012 Red Hat, Inc.
Should be IBM?
Line 25: PASSWORD_FILE = '/etc/pki/vdsm/keys/libvirt_password'
It could be better to use constants.P_VDSM_KEYS + 'libvirt_password'
Line 30: Update the vdsm password for libvirt
I feel it's a little bit confusing. "Update vdsm's authenticate credentials
for libvirt connection" Better?
Line 44: if err:
Is it better to check p.returncode ?
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
2:48 a.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 3: (1 inline comment)
....................................................
File vdsm.spec.in
Line 364
Hi Adam,
You are right. So the point is whether the vdsm-tool command should be called here.
If yes, will add line like "%{_bindir/vdsm-tool update-saslpasswd2}", but the
question is does it make sense to call an vdsm-tool command here? Then what's the
meaning of moving these code into vdsm-tool module?
If no, have to set the password by run the vdsm-tool command manually, of course the
previous package installation process would be changed.
Dan, Federico, Ryan, any ideas?
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
7:47 a.m.
Adam Litke has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 3: No score
(1 inline comment)
Removing -1 to encourage further review from others.
....................................................
File vdsm.spec.in
Line 364
Yes, I like that you moved the code into vdsm-tool. But it seems you have also changed
the package installation process. Before your patch, the libvirt sasl password was set
here during installation. After your patch, it is never set.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
3:15 a.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 3: No score
Hmm, seems I failed to refresh the score '-1' there...give +1 didn't work...
:(
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
3:11 a.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 3: Looks good to me, but someone else must approve
(1 inline comment)
First, I give +1 by myself just to attract your attention since I notice that it would be
very likely to miss your review if the patch got -1 there... :)
Adam, I didn't forgot configure the password using the new method, for the details I
have posted in patch page as reply to you comment, could you please let me know your
opinion?
Thank you!
....................................................
File vdsm.spec.in
Line 364
Hi Adam,
Yes, the same question from Wen Yi and I before, we considered if it should be called
here, or just move this to vdsm-tool and let it be called there independently.
Actually In my first version as link below, I move these code to its own script and let it
be called here as well as vdsm-unregister part.
http://gerrit.ovirt.org/#change,4135
http://gerrit.ovirt.org/#change,4134
The current code is based on the latter way, since Dan asked to move and encapsulate each
logical function to vdsm-tool, I think the post section is optional, and I am not sure
whether it make sense to call vdsm-tool command here...
So what's your opinion?
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
4:38 p.m.
Adam Litke has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(1 inline comment)
I am worried that you forgot to configure the password using the new method.
....................................................
File vdsm.spec.in
Line 364
I notice you have removed the old code here but I do not see where you call into vdsm-tool
to do it the new way. Have I missed it somewhere else?
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
7:56 a.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 2: (2 inline comments)
....................................................
File vdsm-tool/passwd.py
Line 36: if os.path.exists(PASSWORD_FILE):
This passwd file is installed at the step vdsm installed, which means it would be there
when this command be called from vdsm-tool, so I didn't consider the situation it
doesn't exist... But your suggestion sound good, ok, I will modify it later.
Line 38: sys.stdin = open(PASSWORD_FILE, 'r')
Sorry, I don't understand your point. Do you mean sys.stdin is not good here? Can you
let me know why?
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
10:56 a.m.
Saggi Mizrahi has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm-tool/passwd.py
Line 34: script = '/usr/sbin/saslpasswd2 -p -a libvirt vdsm@rhevh'
shell=true is evil incarnate
Line 36: if os.path.exists(PASSWORD_FILE):
If the file doesn't exist, you silently fail.
This doesn't look right
Line 38: sys.stdin = open(PASSWORD_FILE, 'r')
Why replace the processes stdin?
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
4:29 a.m.
Lei Li has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 2: (1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 30: def update_saslpasswd2():
It's from post script section which is optional. Will be called by vdsm-tool command
to add flexibility since vdsm spec file has very long %post section based on Ryan and
Dan's suggestions.
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
2:40 a.m.
Wenyi Gao has posted comments on this change.
Change subject: Add passwd command to vdsm-tool
......................................................................
Patch Set 2: (1 inline comment)
....................................................
File vdsm-tool/passwd.py
Line 30: def update_saslpasswd2():
Where is the update_saslpasswd2 called?
--
To view, visit http://gerrit.ovirt.org/4528
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76b5aee4c2f11a7680c675c171ff35a463a70402
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Lei Li <lilei(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
4374
days inactive
4399
days old
vdsm-patches@lists.fedorahosted.org
43 comments
8 participants
participants (8)
-
agl＠us.ibm.com -
Dan Kenigsberg -
lilei＠linux.vnet.ibm.com -
shaohef＠linux.vnet.ibm.com -
smizrahi＠redhat.com -
wenyi＠linux.vnet.ibm.com -
wudxw＠linux.vnet.ibm.com -
Xu He Jie