Zhou Zheng Sheng has posted comments on this change.

Change subject: agent: Build and install ovirt guest agent on Ubuntu
......................................................................


Patch Set 1: (5 inline comments)

....................................................
File Makefile.am
Line 58:        $(MKDIR_P) $(DESTDIR)/$(pkgdatadir)
Line 59:        $(LN_S) -f $(bindir)/consolehelper 
$(DESTDIR)/$(pkgdatadir)/ovirt-locksession
Line 60:        $(LN_S) -f $(bindir)/consolehelper 
$(DESTDIR)/$(pkgdatadir)/ovirt-shutdown
Line 61:        $(LN_S) -f $(bindir)/consolehelper 
$(DESTDIR)/$(pkgdatadir)/ovirt-hibernate
Line 62:        $(INSTALL) -d $(DESTDIR)/$(localstatedir)/lock/ovirt-guest-agent
Maybe we can add a --with-lock-path option to configure.ac, then the packager 
can specify the lock path by 

 ./configure --with-lock-path=/var/lock


....................................................
File ovirt-guest-agent/hibernate
Line 1: #!/bin/bash
For lone term solution, we should fix all the bashisms in this script. I use 
checkbashism tool in Ubuntu to check and fix them. Some bashisms are not 
harmful and is supported by dash, so it's OK not fixing those. The "local" 
keyword for variables is among this kind.
Line 2: 
Line 3: usage() {
Line 4:     echo "usage: $0 disk|mem"
Line 5:     exit 1


....................................................
File ovirt-guest-agent/ovirt-guest-agent
Line 12: . /etc/rc.d/init.d/functions
Line 13: 
Line 14: agentd=/usr/share/ovirt-guest-agent/ovirt-guest-agent.py
Line 15: prog=ovirt-guest-agent
Line 16: lockfile=/var/lock/${prog}
I think a configurable @LOCKPATH@ variable is more appropriate here.
Line 17: pidfile=/var/run/${prog}.pid
Line 18: 
Line 19: RETVAL=0
Line 20: 


....................................................
File ubuntuInstall.sh
Line 1: #!/bin/bash
Line 2: 
Line 3: echo "Dependencies: make gcc libtool autoconf libpam-dev pep8 usermode 
python-ethtool python-dev python-dbus"
Line 4: echo "Please resolve the dependencies manually by apt-get install them, 
then run this script to build and install ovirt guest agent."
I believe these dependencies can be listed in a deb package spec file.
Line 5: echo "[Press ENTER to continue]"
Line 6: read
Line 7: 
Line 8: [ -e Makefile ] && make clean


Line 16: 
Line 17: sudo stop ovirt-guest-agent >/dev/null 2>&1
Line 18: 
Line 19: sudo make install
Line 20: 
"make install" does not install all the oVirt guest agent things, it needs to 
run some scripts in the rpm post install section. So I copied them here. I 
think we can either move these actions to  makefile.am and have  them executed 
during "make install", or copy them to post install section of the deb package. 
(Does deb package provide post install action ability?)
Line 21: getent group ovirtagent >/dev/null || sudo groupadd -r -g 175 
ovirtagent
Line 22: getent passwd ovirtagent > /dev/null || \
Line 23:     sudo /usr/sbin/useradd -u 175 -g 175 -o -r ovirtagent \
Line 24:     -c "oVirt Guest Agent" -d /usr/share/ovirt-guest-agent -s 
/sbin/nologin


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7a694bf1f7ff4230bac305dd571899265ef63950
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-guest-agent
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshz...@linux.vnet.ibm.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to