Skip to content
This repository was archived by the owner on Jan 31, 2019. It is now read-only.

Add a couple of small features to dhcpcd recipe #485

Open
kimrhh wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add a couple of small features to dhcpcd recipe #485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ae79276
dhcpcd: make restart work even if start has not been run
Jan 11, 2017
11007a3
dhcpcd: provide dhcp-client
Jan 12, 2017
6888ed6
dhcpcd: add more configuration options
Jan 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions recipes/dhcpcd/dhcpcd.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ DHCPCD_UDEV:USE_dhcpcd_udev = ""
DEPENDS:>dhcpcd_udev = " libudev libdl"
DEPENDS_${PN}:>dhcpcd_udev = " libudev libdl"
RDEPENDS_${PN}:>dhcpcd_udev = " libudev libdl"
PROVIDES_${PN} += "dhcp-client"

EXTRA_OECONF += "${DHCPCD_UDEV}"

Expand Down Expand Up @@ -67,3 +68,14 @@ do_install_initscript() {
sysvinit_install_script ${SRCDIR}/init dhcpcd
}
do_install[prefuncs] += "do_install_initscript"

RECIPE_FLAGS += "dhcpcd_args"
DEFAULT_USE_dhcpcd_args = " "
do_install_config() {
install -d ${D}/${sysconfdir}/default
echo 'ARGS="${USE_dhcpcd_args}"' > ${D}/${sysconfdir}/default/dhcpcd
}
do_install[prefuncs] += "do_install_config"
PACKAGES =+ "${PN}-conf"
FILES_${PN}-conf = "${sysconfdir}/default/dhcpcd ${sysconfdir}/dhcpcd.conf"
RDEPENDS_${PN} += "${PN}-conf"
7 changes: 6 additions & 1 deletion recipes/dhcpcd/files/init
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ do_start() {
}

do_stop() {
start-stop-daemon -K --pidfile $PIDFILE --exec $DAEMON
# only try to stop the daemon if there is actually something
# running. Otherwise a restart before a start will fail
start-stop-daemon -K -o --pidfile $PIDFILE --exec $DAEMON
if [ "$?" != "0" ] ; then
start-stop-daemon -K --pidfile $PIDFILE --exec $DAEMON
fi
}
Copy link
Contributor

@Villemoes Villemoes Jan 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work? You still have set -e, so if the first start-stop-daemon fails, you should never get around to testing $?. At least that how a quick test in bash behaves. If you make the do_stop part of a conditional chain, the set -e behaviour is suppressed, so it seems (again, tested in bash) that one can do "do_stop || true" in the restart) case. But then you don't need to change do_stop itself.

Copy link
Contributor Author

@kimrhh kimrhh Jan 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does work, since the -o just "checking if work is to be done" according to the docs, which means we only really stop it if there is something to stop. It is surely not the most obvious way to solve this, but dropping set -e means we need to change it a lot more to handle errors

Copy link
Contributor

@Villemoes Villemoes Jan 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs say "-o,--oknodo Exit with status 0 if nothing is done", which I read as saying that the first start-stop-daemon does attempt to stop $DAEMON - it just also returns 0 if it doesn't succeed in doing so. And it seems that this means "/etc/init.d/dhcpcd stop" will always return 0, which I don't like (but I'm fine with making restart mean "restart or start"). But regardless of the semantics of -o, the last three lines are dead code, because if the first start-stop-daemon exits with 0, we do nothing, and if it exits non-zero, the script exits right there.

Copy link
Contributor Author

@kimrhh kimrhh Jan 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right the three lines are dead code, but I don't think you are right that stop will always return 0 when we add the -o , there is surely a difference between "no work done" and "there was an error when stopping", but will verify tomorrow to be sure.

I don't think it is a great idea to throw away an error when stopping with || true

Copy link
Contributor

@Villemoes Villemoes Jan 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would be absolutely ideal to have the action "stop" exit non-zero if an error occured or no daemon was running, while "restart" would ignore the latter case and proceed to the start part. However, the source doesn't suggest that's how it works:

	if (opt & CTX_STOP) {
		int i = do_stop();
		return (opt & OPT_OKNODO) ? 0 : (i <= 0);
	}


do_status() {
Expand Down