Message ID | 20231204085408.2125531-1-mingli.yu@windriver.com |
---|---|
State | New |
Headers | show |
Series | openssh: Add SSHD_SERVICE_TYPE variable | expand |
On Mon, 2023-12-04 at 16:54 +0800, Yu, Mingli wrote: > From: Mingli Yu <mingli.yu@windriver.com> > > There are two types of sshd server now, one is based on socket > activation(sshd.socket) and another is service activation(sshd.service). > And the default sshd service type is based on socket by default as below. Firstly, I'm a little frustrated about how these changes are being proposed in pieces. The original patches made no mention that this was eventually going to be controlled by a new variable. I did wonder if something like this was going to happen but the didn't push back. Now we're "pinned" into a corner with the direction this code takes now we've already merged the on target configuration change. > But it's more convenient to determine the service type at build time > if there are so many devices. Why did we merge a patch which made this an on target decision then? > So add SSHD_SERVICE_TYPE variable to enable sshd.socket or sshd.service > at build time and we still enable sshd.socket by default now. No, we're not adding randomly named variables which take magic options. I cannot look at SSHD_SERVICE_TYPE and know that "1" means socket activation and not setting it means service activation (or whatever, the fact I can't check what I've written easily proves my point). What happens if I set it to "true", or "socket", or "apples"? I'd suggest thinking about a PACKAGECONFIG option such as systemd-sshd- socket-mode and systemd-sshd-service-mode which at least uses naming users are mode used to and says what it does. Cheers, Richard
Hi Richard, On 12/4/23 19:07, Richard Purdie wrote: > CAUTION: This email comes from a non Wind River email account! > Do not click links or open attachments unless you recognize the sender and know the content is safe. > > On Mon, 2023-12-04 at 16:54 +0800, Yu, Mingli wrote: >> From: Mingli Yu <mingli.yu@windriver.com> >> >> There are two types of sshd server now, one is based on socket >> activation(sshd.socket) and another is service activation(sshd.service). >> And the default sshd service type is based on socket by default as below. > > Firstly, I'm a little frustrated about how these changes are being > proposed in pieces. The original patches made no mention that this was > eventually going to be controlled by a new variable. I did wonder if > something like this was going to happen but the didn't push back. Now > we're "pinned" into a corner with the direction this code takes now > we've already merged the on target configuration change. > >> But it's more convenient to determine the service type at build time >> if there are so many devices. > > Why did we merge a patch which made this an on target decision then? Maybe usage requirement change? sorry for noise! > >> So add SSHD_SERVICE_TYPE variable to enable sshd.socket or sshd.service >> at build time and we still enable sshd.socket by default now. > > No, we're not adding randomly named variables which take magic options. > > I cannot look at SSHD_SERVICE_TYPE and know that "1" means socket > activation and not setting it means service activation (or whatever, > the fact I can't check what I've written easily proves my point). What > happens if I set it to "true", or "socket", or "apples"? > > I'd suggest thinking about a PACKAGECONFIG option such as systemd-sshd- > socket-mode and systemd-sshd-service-mode which at least uses naming > users are mode used to and says what it does. Will sent v2 to use the common PACKAGECONFIG option to control the sshd type. Thanks, > > Cheers, > > Richard
diff --git a/meta/recipes-connectivity/openssh/openssh_9.5p1.bb b/meta/recipes-connectivity/openssh/openssh_9.5p1.bb index bbb8fb091a..6a603cd12d 100644 --- a/meta/recipes-connectivity/openssh/openssh_9.5p1.bb +++ b/meta/recipes-connectivity/openssh/openssh_9.5p1.bb @@ -50,7 +50,7 @@ INITSCRIPT_NAME:${PN}-sshd = "sshd" INITSCRIPT_PARAMS:${PN}-sshd = "defaults 9" SYSTEMD_PACKAGES = "${PN}-sshd" -SYSTEMD_SERVICE:${PN}-sshd = "sshd.socket sshd.service" +SYSTEMD_SERVICE:${PN}-sshd = "${@bb.utils.contains('SSHD_SERVICE_TYPE', '1', 'sshd.service', 'sshd.socket', d)}" inherit autotools-brokensep ptest pkgconfig DEPENDS += "${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'systemd', '', d)}" @@ -125,15 +125,23 @@ do_install:append () { echo "HostKey /var/run/ssh/ssh_host_ed25519_key" >> ${D}${sysconfdir}/ssh/sshd_config_readonly install -d ${D}${systemd_system_unitdir} - install -c -m 0644 ${WORKDIR}/sshd.socket ${D}${systemd_system_unitdir} - install -c -m 0644 ${WORKDIR}/sshd.service ${D}${systemd_system_unitdir} - install -c -m 0644 ${WORKDIR}/sshd@.service ${D}${systemd_system_unitdir} + if ${@bb.utils.contains('SSHD_SERVICE_TYPE','1','true','false',d)}; then + install -c -m 0644 ${WORKDIR}/sshd.service ${D}${systemd_system_unitdir} + else + install -c -m 0644 ${WORKDIR}/sshd.socket ${D}${systemd_system_unitdir} + install -c -m 0644 ${WORKDIR}/sshd@.service ${D}${systemd_system_unitdir} + sed -i -e 's,@BASE_BINDIR@,${base_bindir},g' \ + -e 's,@SBINDIR@,${sbindir},g' \ + -e 's,@BINDIR@,${bindir},g' \ + -e 's,@LIBEXECDIR@,${libexecdir}/${BPN},g' \ + ${D}${systemd_system_unitdir}/sshd.socket + fi install -c -m 0644 ${WORKDIR}/sshdgenkeys.service ${D}${systemd_system_unitdir} sed -i -e 's,@BASE_BINDIR@,${base_bindir},g' \ -e 's,@SBINDIR@,${sbindir},g' \ -e 's,@BINDIR@,${bindir},g' \ -e 's,@LIBEXECDIR@,${libexecdir}/${BPN},g' \ - ${D}${systemd_system_unitdir}/sshd.socket ${D}${systemd_system_unitdir}/*.service + ${D}${systemd_system_unitdir}/*.service sed -i -e 's,@LIBEXECDIR@,${libexecdir}/${BPN},g' \ ${D}${sysconfdir}/init.d/sshd