Yaniv Bronhaim has posted comments on this change.

Change subject: fcp: Deactivate vdsm volume groups during boot
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

using a file under /var/run to sign first boot is enough. also bare in mind 
that user can have vdsmd installed without running it at all, so why forcing 
him this in each restart? it should run only on first boot when vdsmd starts 
up. Upgrading or restarting won't harm when using that logic only on first 
boot. this logic must be under pre-start tasks, using another external script 
just make vdsm more complex to understand imo, and harder to maintain for other 
distributions

....................................................
File init/sysvinit/vdsm-deactivate-vgs.init
Line 22: ### END INIT INFO
Line 23: 
Line 24: . /etc/init.d/functions
Line 25: 
Line 26: run_file="var/run/vdsm/lvm/deactivate-vgs"
why not using constants?
Line 27: prog="vdsm-deactivate-vgs"
Line 28: retval=0
Line 29: 
Line 30: log_failure_msg()


Line 80:     ;;
Line 81:     stop)
Line 82:     ;;
Line 83:     *)
Line 84:     echo "Usage: $0 {start|stop}"
stop does nothing.. so why having it?
Line 85:     retval=2
Line 86: esac
Line 87: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to