Alon Bar-Lev has posted comments on this change.

Change subject: hooks: Adding hooks for ovirt-node-upgrade
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.ovirt.org/#/c/28251/3/hooks/Makefile.am
File hooks/Makefile.am:

Line 16: # also available at http://www.gnu.org/copyleft/gpl.html.
Line 17: 
Line 18: preupgradedir = $(libexecdir)/ovirt-node/hooks/pre-upgrade
Line 19: 
Line 20: dist_preupgrade_DATA = \
should be _SCRIPTS or _PYTHON
Line 21:        pre-upgrade/01-vdsm.py \
Line 22:        $(NULL)
Line 23: 
Line 24: install-data-local:


Line 22:        $(NULL)
Line 23: 
Line 24: install-data-local:
Line 25:        install -Dm 0755 pre-upgrade/01-vdsm.py \
Line 26:                $(DESTDIR)$(preupgradedir)/01-vdsm.py
the above should be sufficient. why do you need to install it manually?


http://gerrit.ovirt.org/#/c/28251/3/hooks/pre-upgrade/01-vdsm.py
File hooks/pre-upgrade/01-vdsm.py:

Line 21:         ['vdsm-tool', 'service-stop', 'vdsmd'],
Line 22:         sudo=True,
Line 23:         raw=True
Line 24:     )
Line 25:     return rc
what about proper message? at least print an error if fail to stop vdsm so that 
the user will be notified what is wrong?
Line 26: 
Line 27: if __name__ == "__main__":


-- 
To view, visit http://gerrit.ovirt.org/28251
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I986fe6edb8f5619dcbca33cccab080245654fd02
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-node-plugin-vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Fabian Deutsch <[email protected]>
Gerrit-Reviewer: Joey Boggs <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to