Doron Fediuck has uploaded a new change for review.
Change subject: bootstrapping: avoid usage of tmp folder ......................................................................
bootstrapping: avoid usage of tmp folder
This patch changes hard-coded tmp path elements, so the scripts may be used from a different location. Once invoked by the engine-core from an ad-hoc folder, the bootstrapping scripts should support working on that folder. This is mainly referring to imports, certificate files and log files.
Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 --- M vds_bootstrap/vds_bootstrap.py M vds_bootstrap/vds_bootstrap_complete.py M vdsm_reg/deployUtil.py.in 3 files changed, 35 insertions(+), 32 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/77/2277/1 -- To view, visit http://gerrit.ovirt.org/2277 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Doron Fediuck dfediuck@redhat.com
Ewoud Kohl van Wijngaarden has posted comments on this change.
Change subject: bootstrapping: avoid usage of tmp folder ......................................................................
Patch Set 1: (4 inline comments)
I like the initiative, but it feels a bit inconsistent. Maybe you determine workdir once in main() and pass it it all functions?
.................................................... File vds_bootstrap/vds_bootstrap.py Line 60: log_filename = SCRIPT_DIR + 'vds_bootstrap.' + rnum + '.log' Why use SCRIPT_DIR here and in the other file /tmp if isOvirt() else SCRIPT_DIR?
.................................................... File vdsm_reg/deployUtil.py.in Line 154: workdir = '/tmp/' Why not tempfile.gettempdir()
Line 1120: workdir = '/tmp/' Why not:
workdir = tempfile.gettempdir() if isOvirt() else SCRIPT_DIR
Line 1127: gGroup = grp.getgrnam('VDSM_GROUP') Why should this be quoted?
-- To view, visit http://gerrit.ovirt.org/2277 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ewoud@kohlvanwijngaarden.nl
Douglas Schilling Landgraf has posted comments on this change.
Change subject: bootstrapping: avoid usage of tmp folder ......................................................................
Patch Set 2: Verified
-- To view, visit http://gerrit.ovirt.org/2277 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ewoud@kohlvanwijngaarden.nl
Dan Kenigsberg has posted comments on this change.
Change subject: bootstrapping: avoid usage of tmp folder ......................................................................
Patch Set 2: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/2277 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ewoud@kohlvanwijngaarden.nl
Shu Ming has posted comments on this change.
Change subject: bootstrapping: avoid usage of tmp folder ......................................................................
Patch Set 2: I would prefer that you didn't submit this
(3 inline comments)
.................................................... File vdsm_reg/deployUtil.py.in Line 203: os.unlink(vds_bstrap + 'c') Is that not vds_bstrap_pyc?
Line 204: os.unlink(vds_bstrap_cmp + 'c') Is that not vds_bstrap_cmp_pyc?
Line 206: os.unlink(vds_bstrap_deployutil + 'c') Is that not *_pyc?
-- To view, visit http://gerrit.ovirt.org/2277 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ewoud@kohlvanwijngaarden.nl Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Douglas Schilling Landgraf has posted comments on this change.
Change subject: bootstrapping: avoid usage of tmp folder ......................................................................
Patch Set 2: (3 inline comments)
.................................................... File vdsm_reg/deployUtil.py.in Line 203: os.unlink(vds_bstrap + 'c') concatenate to vds_bstrap the 'c' char to remove the *.pyc file.
Line 204: os.unlink(vds_bstrap_cmp + 'c') the same idea as above.
Line 206: os.unlink(vds_bstrap_deployutil + 'c') the same idea as above.
-- To view, visit http://gerrit.ovirt.org/2277 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ewoud@kohlvanwijngaarden.nl Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Shu Ming has posted comments on this change.
Change subject: bootstrapping: avoid usage of tmp folder ......................................................................
Patch Set 2: Looks good to me, but someone else must approve
(1 inline comment)
.................................................... File vdsm_reg/deployUtil.py.in Line 203: os.unlink(vds_bstrap + 'c') Thanks for for clarification. I am happy with that.
-- To view, visit http://gerrit.ovirt.org/2277 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ewoud@kohlvanwijngaarden.nl Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Shu Ming has posted comments on this change.
Change subject: bootstrapping: avoid usage of tmp folder ......................................................................
Patch Set 2: No score
(1 inline comment)
.................................................... File vdsm_reg/deployUtil.py.in Line 109: if isOvirt(): I would prefer you move these lines to main().
-- To view, visit http://gerrit.ovirt.org/2277 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ewoud@kohlvanwijngaarden.nl Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Douglas Schilling Landgraf has posted comments on this change.
Change subject: bootstrapping: avoid usage of tmp folder ......................................................................
Patch Set 2: (1 inline comment)
.................................................... File vdsm_reg/deployUtil.py.in Line 109: if isOvirt(): Shu, I have tested the patch and it worked. I would prefer we change that in a next patch. Thanks for your review!
-- To view, visit http://gerrit.ovirt.org/2277 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ewoud@kohlvanwijngaarden.nl Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Dan Kenigsberg has submitted this change and it was merged.
Change subject: bootstrapping: avoid usage of tmp folder ......................................................................
bootstrapping: avoid usage of tmp folder
This patch changes hard-coded tmp path elements, so the scripts may be used from a different location. Once invoked by the engine-core from an ad-hoc folder, the bootstrapping scripts should support working on that folder. This is mainly referring to imports, certificate files and log files.
Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 Signed-off-by: Douglas Schilling Landgraf dougsland@redhat.com --- M vds_bootstrap/vds_bootstrap.py M vds_bootstrap/vds_bootstrap_complete.py M vdsm_reg/deployUtil.py.in 3 files changed, 43 insertions(+), 41 deletions(-)
Approvals: Douglas Schilling Landgraf: Verified Dan Kenigsberg: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/2277 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ewoud@kohlvanwijngaarden.nl Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: bootstrapping: avoid usage of tmp folder ......................................................................
Patch Set 2: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/2277 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I1b5df6dd0b5ff0bdf648cd74d160384b86e5bbf7 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ewoud@kohlvanwijngaarden.nl Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
vdsm-patches@lists.fedorahosted.org