diff mbox series

rpcbind: Fix boot time start failure

Message ID 20240726112431.698884-1-zboszor@gmail.com
State Accepted, archived
Commit 53fb871f84c99a66485979da2588c1d11d8749e7
Headers show
Series rpcbind: Fix boot time start failure | expand

Commit Message

Böszörményi Zoltán July 26, 2024, 11:24 a.m. UTC
With commits 90bc1810 ("bitbake.conf: Add runtimedir") and
561e853e ("rpcbind: Specify state directory under /run") rpcbind
still can fail during startup.

It has two problems:
* The lockfile is still hardcoded as "/var/run/rpcbind.lock".
  It needs to use the same internal define for RPCBIND_STATEDIR
  as the paths for rpcbind.xdr and portmap.xdr.
* Using --with-statedir=/run/rpcbind doesn't guarantee that this
  directory exists when rpcbind.service starts. Add this guarantee
  by running rpcbind.service with After=systemd-tmpfiles-setup.service
  and add the tmpfiles.d entry for /run/rpcbind.

Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
---
 ...ke-the-lockfile-follow-with-statedir.patch | 31 ++++++++++++++++++
 ...ice-after-etc-tmpfiles.d-is-processe.patch | 32 +++++++++++++++++++
 .../rpcbind/rpcbind/rpcbind.tmpfiles          |  1 +
 .../recipes-extended/rpcbind/rpcbind_1.2.6.bb |  6 ++++
 4 files changed, 70 insertions(+)
 create mode 100644 meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch
 create mode 100644 meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch
 create mode 100644 meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles

Comments

patchtest@automation.yoctoproject.org July 26, 2024, 11:33 a.m. UTC | #1
Thank you for your submission. Patchtest identified one
or more issues with the patch. Please see the log below for
more information:

---
Testing patch /home/patchtest/share/mboxes/rpcbind-Fix-boot-time-start-failure.patch

FAIL: test Upstream-Status presence: Upstream-Status is in incorrect format (test_patch.TestPatch.test_upstream_status_presence_format)

PASS: pretest src uri left files (test_metadata.TestMetadata.pretest_src_uri_left_files)
PASS: test CVE check ignore (test_metadata.TestMetadata.test_cve_check_ignore)
PASS: test CVE tag format (test_patch.TestPatch.test_cve_tag_format)
PASS: test Signed-off-by presence (test_mbox.TestMbox.test_signed_off_by_presence)
PASS: test Signed-off-by presence (test_patch.TestPatch.test_signed_off_by_presence)
PASS: test author valid (test_mbox.TestMbox.test_author_valid)
PASS: test commit message presence (test_mbox.TestMbox.test_commit_message_presence)
PASS: test lic files chksum modified not mentioned (test_metadata.TestMetadata.test_lic_files_chksum_modified_not_mentioned)
PASS: test max line length (test_metadata.TestMetadata.test_max_line_length)
PASS: test mbox format (test_mbox.TestMbox.test_mbox_format)
PASS: test non-AUH upgrade (test_mbox.TestMbox.test_non_auh_upgrade)
PASS: test shortlog format (test_mbox.TestMbox.test_shortlog_format)
PASS: test shortlog length (test_mbox.TestMbox.test_shortlog_length)
PASS: test src uri left files (test_metadata.TestMetadata.test_src_uri_left_files)

SKIP: pretest pylint: No python related patches, skipping test (test_python_pylint.PyLint.pretest_pylint)
SKIP: test bugzilla entry format: No bug ID found (test_mbox.TestMbox.test_bugzilla_entry_format)
SKIP: test lic files chksum presence: No added recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_presence)
SKIP: test license presence: No added recipes, skipping test (test_metadata.TestMetadata.test_license_presence)
SKIP: test pylint: No python related patches, skipping test (test_python_pylint.PyLint.test_pylint)
SKIP: test series merge on head: Merge test is disabled for now (test_mbox.TestMbox.test_series_merge_on_head)
SKIP: test summary presence: No added recipes, skipping test (test_metadata.TestMetadata.test_summary_presence)
SKIP: test target mailing list: Series merged, no reason to check other mailing lists (test_mbox.TestMbox.test_target_mailing_list)

---

Please address the issues identified and
submit a new revision of the patch, or alternatively, reply to this
email with an explanation of why the patch should be accepted. If you
believe these results are due to an error in patchtest, please submit a
bug at https://bugzilla.yoctoproject.org/ (use the 'Patchtest' category
under 'Yocto Project Subprojects'). For more information on specific
failures, see: https://wiki.yoctoproject.org/wiki/Patchtest. Thank
you!
Böszörményi Zoltán July 26, 2024, 11:40 a.m. UTC | #2
2024. 07. 26. 13:33 keltezéssel, patchtest@automation.yoctoproject.org írta:
> Thank you for your submission. Patchtest identified one
> or more issues with the patch. Please see the log below for
> more information:
>
> ---
> Testing patch /home/patchtest/share/mboxes/rpcbind-Fix-boot-time-start-failure.patch
>
> FAIL: test Upstream-Status presence: Upstream-Status is in incorrect format (test_patch.TestPatch.test_upstream_status_presence_format)

How is it in incorrect format? I used the documented format from
https://docs.yoctoproject.org/contributor-guide/recipe-style-guide.html#patch-upstream-statusThis 
is supposed to be a correct format according to the documentation, right? |

Inactive-Upstream [lastrelease: YYYY-MM-DD]
|
> PASS: pretest src uri left files (test_metadata.TestMetadata.pretest_src_uri_left_files)
> PASS: test CVE check ignore (test_metadata.TestMetadata.test_cve_check_ignore)
> PASS: test CVE tag format (test_patch.TestPatch.test_cve_tag_format)
> PASS: test Signed-off-by presence (test_mbox.TestMbox.test_signed_off_by_presence)
> PASS: test Signed-off-by presence (test_patch.TestPatch.test_signed_off_by_presence)
> PASS: test author valid (test_mbox.TestMbox.test_author_valid)
> PASS: test commit message presence (test_mbox.TestMbox.test_commit_message_presence)
> PASS: test lic files chksum modified not mentioned (test_metadata.TestMetadata.test_lic_files_chksum_modified_not_mentioned)
> PASS: test max line length (test_metadata.TestMetadata.test_max_line_length)
> PASS: test mbox format (test_mbox.TestMbox.test_mbox_format)
> PASS: test non-AUH upgrade (test_mbox.TestMbox.test_non_auh_upgrade)
> PASS: test shortlog format (test_mbox.TestMbox.test_shortlog_format)
> PASS: test shortlog length (test_mbox.TestMbox.test_shortlog_length)
> PASS: test src uri left files (test_metadata.TestMetadata.test_src_uri_left_files)
>
> SKIP: pretest pylint: No python related patches, skipping test (test_python_pylint.PyLint.pretest_pylint)
> SKIP: test bugzilla entry format: No bug ID found (test_mbox.TestMbox.test_bugzilla_entry_format)
> SKIP: test lic files chksum presence: No added recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_presence)
> SKIP: test license presence: No added recipes, skipping test (test_metadata.TestMetadata.test_license_presence)
> SKIP: test pylint: No python related patches, skipping test (test_python_pylint.PyLint.test_pylint)
> SKIP: test series merge on head: Merge test is disabled for now (test_mbox.TestMbox.test_series_merge_on_head)
> SKIP: test summary presence: No added recipes, skipping test (test_metadata.TestMetadata.test_summary_presence)
> SKIP: test target mailing list: Series merged, no reason to check other mailing lists (test_mbox.TestMbox.test_target_mailing_list)
>
> ---
>
> Please address the issues identified and
> submit a new revision of the patch, or alternatively, reply to this
> email with an explanation of why the patch should be accepted. If you
> believe these results are due to an error in patchtest, please submit a
> bug at https://bugzilla.yoctoproject.org/ (use the 'Patchtest' category
> under 'Yocto Project Subprojects'). For more information on specific
> failures, see: https://wiki.yoctoproject.org/wiki/Patchtest. Thank
> you!
Joshua Watt July 26, 2024, 3:21 p.m. UTC | #3
On Fri, Jul 26, 2024 at 5:24 AM Zoltán Böszörményi <zboszor@gmail.com> wrote:
>
> With commits 90bc1810 ("bitbake.conf: Add runtimedir") and
> 561e853e ("rpcbind: Specify state directory under /run") rpcbind
> still can fail during startup.
>
> It has two problems:
> * The lockfile is still hardcoded as "/var/run/rpcbind.lock".
>   It needs to use the same internal define for RPCBIND_STATEDIR
>   as the paths for rpcbind.xdr and portmap.xdr.
> * Using --with-statedir=/run/rpcbind doesn't guarantee that this
>   directory exists when rpcbind.service starts. Add this guarantee
>   by running rpcbind.service with After=systemd-tmpfiles-setup.service
>   and add the tmpfiles.d entry for /run/rpcbind.

The original patches have been working for me on nanbield (and
kirkstone), so I'm a little confused as to why this is not working for
you; I'd like to avoid patching the source code if at all possible as
carrying patches like this is a burden

>
> Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
> ---
>  ...ke-the-lockfile-follow-with-statedir.patch | 31 ++++++++++++++++++
>  ...ice-after-etc-tmpfiles.d-is-processe.patch | 32 +++++++++++++++++++
>  .../rpcbind/rpcbind/rpcbind.tmpfiles          |  1 +
>  .../recipes-extended/rpcbind/rpcbind_1.2.6.bb |  6 ++++
>  4 files changed, 70 insertions(+)
>  create mode 100644 meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch
>  create mode 100644 meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch
>  create mode 100644 meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles
>
> diff --git a/meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch b/meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch
> new file mode 100644
> index 0000000000..d487312d22
> --- /dev/null
> +++ b/meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch
> @@ -0,0 +1,31 @@
> +From f10db88174e73c78c028561715f16ed38148ebde Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Zolt=C3=A1n=20B=C3=B6sz=C3=B6rm=C3=A9nyi?=
> + <zboszor@gmail.com>
> +Date: Fri, 26 Jul 2024 12:36:06 +0200
> +Subject: [PATCH 1/2] Make the lockfile follow --with-statedir=
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +Upstream-Status: Inactive-Upstream [lastrelease: 2021-05-10]
> +Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
> +---
> + src/rpcbind.c | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/src/rpcbind.c b/src/rpcbind.c
> +index 36a95b9..948a284 100644
> +--- a/src/rpcbind.c
> ++++ b/src/rpcbind.c
> +@@ -105,7 +105,7 @@ char *nss_modules = "files";
> + /* who to suid to if -s is given */
> + #define RUN_AS  "daemon"
> +
> +-#define RPCBINDDLOCK "/var/run/rpcbind.lock"
> ++#define RPCBINDDLOCK RPCBIND_STATEDIR "/rpcbind.lock"

/var/run should be a symlink to /run, so I really don't think this
should be necessary (at least with the default value for $runtimedir)

> +
> + int runasdaemon = 0;
> + int insecure = 0;
> +--
> +2.45.2
> +
> diff --git a/meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch b/meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch
> new file mode 100644
> index 0000000000..f454fdbb3b
> --- /dev/null
> +++ b/meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch
> @@ -0,0 +1,32 @@
> +From 408b18c77c254baaa9111b3e7031ebf12149db38 Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Zolt=C3=A1n=20B=C3=B6sz=C3=B6rm=C3=A9nyi?=
> + <zboszor@gmail.com>
> +Date: Fri, 26 Jul 2024 12:54:38 +0200
> +Subject: [PATCH 2/2] Run rpcbind.service after /etc/tmpfiles.d is processed
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +Upstream-Status: Inactive-Upstream [lastrelease: 2021-05-10]
> +Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
> +---
> + systemd/rpcbind.service.in | 3 +++
> + 1 file changed, 3 insertions(+)
> +
> +diff --git a/systemd/rpcbind.service.in b/systemd/rpcbind.service.in
> +index f45ee1e..e9d94ce 100644
> +--- a/systemd/rpcbind.service.in
> ++++ b/systemd/rpcbind.service.in
> +@@ -9,6 +9,9 @@ RequiresMountsFor=@statedir@
> + Requires=rpcbind.socket
> + Wants=rpcbind.target
> +
> ++# Make sure the runtime directory exists
> ++After=systemd-tmpfiles-setup.service

I'd rather have the recipe provide a drop file for this instead of
patching the source

> ++
> + [Service]
> + Type=notify
> + EnvironmentFile=-@_sysconfdir@/rpcbind.conf
> +--
> +2.45.2
> +
> diff --git a/meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles b/meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles
> new file mode 100644
> index 0000000000..fecee72c09
> --- /dev/null
> +++ b/meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles
> @@ -0,0 +1 @@
> +d /run/rpcbind 0755 root root -
> diff --git a/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb b/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb
> index e751eb631c..477092b37d 100644
> --- a/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb
> +++ b/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb
> @@ -13,8 +13,11 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=b46486e4c4a416602693a711bb5bfa39 \
>  SRC_URI = "${SOURCEFORGE_MIRROR}/rpcbind/rpcbind-${PV}.tar.bz2 \
>             file://init.d \
>             file://rpcbind.conf \
> +           file://rpcbind.tmpfiles \
>             file://rpcbind_add_option_to_fix_port_number.patch \
>             file://0001-systemd-use-EnvironmentFile.patch \
> +           file://0001-Make-the-lockfile-follow-with-statedir.patch \
> +           file://0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch \
>            "
>  SRC_URI[sha256sum] = "5613746489cae5ae23a443bb85c05a11741a5f12c8f55d2bb5e83b9defeee8de"
>
> @@ -49,6 +52,9 @@ do_install:append () {
>                 ${UNPACKDIR}/init.d > ${D}${sysconfdir}/init.d/rpcbind
>         chmod 0755 ${D}${sysconfdir}/init.d/rpcbind
>         install -m 0644 ${UNPACKDIR}/rpcbind.conf ${D}${sysconfdir}/rpcbind.conf
> +
> +       install -d ${D}${sysconfdir}/tmpfiles.d
> +       install -m 0644 ${UNPACKDIR}/rpcbind.tmpfiles ${D}${sysconfdir}/tmpfiles.d/rpcbind.conf
>  }
>
>  ALTERNATIVE:${PN} = "rpcinfo"
> --
> 2.45.2
>
Böszörményi Zoltán July 26, 2024, 3:55 p.m. UTC | #4
2024. 07. 26. 17:21 keltezéssel, Joshua Watt írta:
> On Fri, Jul 26, 2024 at 5:24 AM Zoltán Böszörményi <zboszor@gmail.com> wrote:
>> With commits 90bc1810 ("bitbake.conf: Add runtimedir") and
>> 561e853e ("rpcbind: Specify state directory under /run") rpcbind
>> still can fail during startup.
>>
>> It has two problems:
>> * The lockfile is still hardcoded as "/var/run/rpcbind.lock".
>>    It needs to use the same internal define for RPCBIND_STATEDIR
>>    as the paths for rpcbind.xdr and portmap.xdr.
>> * Using --with-statedir=/run/rpcbind doesn't guarantee that this
>>    directory exists when rpcbind.service starts. Add this guarantee
>>    by running rpcbind.service with After=systemd-tmpfiles-setup.service
>>    and add the tmpfiles.d entry for /run/rpcbind.
> The original patches have been working for me on nanbield (and
> kirkstone), so I'm a little confused as to why this is not working for
> you; I'd like to avoid patching the source code if at all possible as
> carrying patches like this is a burden

The error I got is:

rpcbind[455]: rpcbind: /var/run/rpcbind.lock: Read-only file system

So, while /var/run is a symlink to /run, the latter is not writable yet.
Indeed, rpcbind.service was starting very early. These changes
delay it enough to avoid the problem.

The inconsistency between using /run via --with-statedir=/run
directly for some the xdr files but not for the lockfile is not nice.
That must have been an oversight in the source code.

These changes fix the issue for me in nanbield. I can reproduce
the above problem in scarthgap without these changes.

FWIW, different machines can give very different testing results,
due to the differences in their number of CPU cores and speed.
Recently, I had to add a modified rngd.service from dracut
to rng-tools as an in-house change, because one particular
machine (slow by today's standards, based on AMD Kabini)
ended up with this service failing, due to the CPU not supporting
RDRAND and the JITTER random source driver starting particularly
slowly on it, so rngd couldn't pick it up when started only from
initramfs.

>> Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
>> ---
>>   ...ke-the-lockfile-follow-with-statedir.patch | 31 ++++++++++++++++++
>>   ...ice-after-etc-tmpfiles.d-is-processe.patch | 32 +++++++++++++++++++
>>   .../rpcbind/rpcbind/rpcbind.tmpfiles          |  1 +
>>   .../recipes-extended/rpcbind/rpcbind_1.2.6.bb |  6 ++++
>>   4 files changed, 70 insertions(+)
>>   create mode 100644 meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch
>>   create mode 100644 meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch
>>   create mode 100644 meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles
>>
>> diff --git a/meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch b/meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch
>> new file mode 100644
>> index 0000000000..d487312d22
>> --- /dev/null
>> +++ b/meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch
>> @@ -0,0 +1,31 @@
>> +From f10db88174e73c78c028561715f16ed38148ebde Mon Sep 17 00:00:00 2001
>> +From: =?UTF-8?q?Zolt=C3=A1n=20B=C3=B6sz=C3=B6rm=C3=A9nyi?=
>> + <zboszor@gmail.com>
>> +Date: Fri, 26 Jul 2024 12:36:06 +0200
>> +Subject: [PATCH 1/2] Make the lockfile follow --with-statedir=
>> +MIME-Version: 1.0
>> +Content-Type: text/plain; charset=UTF-8
>> +Content-Transfer-Encoding: 8bit
>> +
>> +Upstream-Status: Inactive-Upstream [lastrelease: 2021-05-10]
>> +Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
>> +---
>> + src/rpcbind.c | 2 +-
>> + 1 file changed, 1 insertion(+), 1 deletion(-)
>> +
>> +diff --git a/src/rpcbind.c b/src/rpcbind.c
>> +index 36a95b9..948a284 100644
>> +--- a/src/rpcbind.c
>> ++++ b/src/rpcbind.c
>> +@@ -105,7 +105,7 @@ char *nss_modules = "files";
>> + /* who to suid to if -s is given */
>> + #define RUN_AS  "daemon"
>> +
>> +-#define RPCBINDDLOCK "/var/run/rpcbind.lock"
>> ++#define RPCBINDDLOCK RPCBIND_STATEDIR "/rpcbind.lock"
> /var/run should be a symlink to /run, so I really don't think this
> should be necessary (at least with the default value for $runtimedir)
>
>> +
>> + int runasdaemon = 0;
>> + int insecure = 0;
>> +--
>> +2.45.2
>> +
>> diff --git a/meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch b/meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch
>> new file mode 100644
>> index 0000000000..f454fdbb3b
>> --- /dev/null
>> +++ b/meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch
>> @@ -0,0 +1,32 @@
>> +From 408b18c77c254baaa9111b3e7031ebf12149db38 Mon Sep 17 00:00:00 2001
>> +From: =?UTF-8?q?Zolt=C3=A1n=20B=C3=B6sz=C3=B6rm=C3=A9nyi?=
>> + <zboszor@gmail.com>
>> +Date: Fri, 26 Jul 2024 12:54:38 +0200
>> +Subject: [PATCH 2/2] Run rpcbind.service after /etc/tmpfiles.d is processed
>> +MIME-Version: 1.0
>> +Content-Type: text/plain; charset=UTF-8
>> +Content-Transfer-Encoding: 8bit
>> +
>> +Upstream-Status: Inactive-Upstream [lastrelease: 2021-05-10]
>> +Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
>> +---
>> + systemd/rpcbind.service.in | 3 +++
>> + 1 file changed, 3 insertions(+)
>> +
>> +diff --git a/systemd/rpcbind.service.in b/systemd/rpcbind.service.in
>> +index f45ee1e..e9d94ce 100644
>> +--- a/systemd/rpcbind.service.in
>> ++++ b/systemd/rpcbind.service.in
>> +@@ -9,6 +9,9 @@ RequiresMountsFor=@statedir@
>> + Requires=rpcbind.socket
>> + Wants=rpcbind.target
>> +
>> ++# Make sure the runtime directory exists
>> ++After=systemd-tmpfiles-setup.service
> I'd rather have the recipe provide a drop file for this instead of
> patching the source
>
>> ++
>> + [Service]
>> + Type=notify
>> + EnvironmentFile=-@_sysconfdir@/rpcbind.conf
>> +--
>> +2.45.2
>> +
>> diff --git a/meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles b/meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles
>> new file mode 100644
>> index 0000000000..fecee72c09
>> --- /dev/null
>> +++ b/meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles
>> @@ -0,0 +1 @@
>> +d /run/rpcbind 0755 root root -
>> diff --git a/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb b/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb
>> index e751eb631c..477092b37d 100644
>> --- a/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb
>> +++ b/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb
>> @@ -13,8 +13,11 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=b46486e4c4a416602693a711bb5bfa39 \
>>   SRC_URI = "${SOURCEFORGE_MIRROR}/rpcbind/rpcbind-${PV}.tar.bz2 \
>>              file://init.d \
>>              file://rpcbind.conf \
>> +           file://rpcbind.tmpfiles \
>>              file://rpcbind_add_option_to_fix_port_number.patch \
>>              file://0001-systemd-use-EnvironmentFile.patch \
>> +           file://0001-Make-the-lockfile-follow-with-statedir.patch \
>> +           file://0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch \
>>             "
>>   SRC_URI[sha256sum] = "5613746489cae5ae23a443bb85c05a11741a5f12c8f55d2bb5e83b9defeee8de"
>>
>> @@ -49,6 +52,9 @@ do_install:append () {
>>                  ${UNPACKDIR}/init.d > ${D}${sysconfdir}/init.d/rpcbind
>>          chmod 0755 ${D}${sysconfdir}/init.d/rpcbind
>>          install -m 0644 ${UNPACKDIR}/rpcbind.conf ${D}${sysconfdir}/rpcbind.conf
>> +
>> +       install -d ${D}${sysconfdir}/tmpfiles.d
>> +       install -m 0644 ${UNPACKDIR}/rpcbind.tmpfiles ${D}${sysconfdir}/tmpfiles.d/rpcbind.conf
>>   }
>>
>>   ALTERNATIVE:${PN} = "rpcinfo"
>> --
>> 2.45.2
>>
Trevor Gamblin July 26, 2024, 4:22 p.m. UTC | #5
On 2024-07-26 7:40 a.m., Zoltan Boszormenyi via lists.openembedded.org 
wrote:
> 2024. 07. 26. 13:33 keltezéssel, patchtest@automation.yoctoproject.org 
> írta:
>> Thank you for your submission. Patchtest identified one
>> or more issues with the patch. Please see the log below for
>> more information:
>>
>> ---
>> Testing patch 
>> /home/patchtest/share/mboxes/rpcbind-Fix-boot-time-start-failure.patch
>>
>> FAIL: test Upstream-Status presence: Upstream-Status is in incorrect 
>> format (test_patch.TestPatch.test_upstream_status_presence_format)
>
> How is it in incorrect format? I used the documented format from
> https://docs.yoctoproject.org/contributor-guide/recipe-style-guide.html#patch-upstream-statusThis 
> is supposed to be a correct format according to the documentation, 
> right? |
>
> Inactive-Upstream [lastrelease: YYYY-MM-DD]
Looks like a bug. I'll get an issue logged.
> |
>> PASS: pretest src uri left files 
>> (test_metadata.TestMetadata.pretest_src_uri_left_files)
>> PASS: test CVE check ignore 
>> (test_metadata.TestMetadata.test_cve_check_ignore)
>> PASS: test CVE tag format (test_patch.TestPatch.test_cve_tag_format)
>> PASS: test Signed-off-by presence 
>> (test_mbox.TestMbox.test_signed_off_by_presence)
>> PASS: test Signed-off-by presence 
>> (test_patch.TestPatch.test_signed_off_by_presence)
>> PASS: test author valid (test_mbox.TestMbox.test_author_valid)
>> PASS: test commit message presence 
>> (test_mbox.TestMbox.test_commit_message_presence)
>> PASS: test lic files chksum modified not mentioned 
>> (test_metadata.TestMetadata.test_lic_files_chksum_modified_not_mentioned)
>> PASS: test max line length 
>> (test_metadata.TestMetadata.test_max_line_length)
>> PASS: test mbox format (test_mbox.TestMbox.test_mbox_format)
>> PASS: test non-AUH upgrade (test_mbox.TestMbox.test_non_auh_upgrade)
>> PASS: test shortlog format (test_mbox.TestMbox.test_shortlog_format)
>> PASS: test shortlog length (test_mbox.TestMbox.test_shortlog_length)
>> PASS: test src uri left files 
>> (test_metadata.TestMetadata.test_src_uri_left_files)
>>
>> SKIP: pretest pylint: No python related patches, skipping test 
>> (test_python_pylint.PyLint.pretest_pylint)
>> SKIP: test bugzilla entry format: No bug ID found 
>> (test_mbox.TestMbox.test_bugzilla_entry_format)
>> SKIP: test lic files chksum presence: No added recipes, skipping test 
>> (test_metadata.TestMetadata.test_lic_files_chksum_presence)
>> SKIP: test license presence: No added recipes, skipping test 
>> (test_metadata.TestMetadata.test_license_presence)
>> SKIP: test pylint: No python related patches, skipping test 
>> (test_python_pylint.PyLint.test_pylint)
>> SKIP: test series merge on head: Merge test is disabled for now 
>> (test_mbox.TestMbox.test_series_merge_on_head)
>> SKIP: test summary presence: No added recipes, skipping test 
>> (test_metadata.TestMetadata.test_summary_presence)
>> SKIP: test target mailing list: Series merged, no reason to check 
>> other mailing lists (test_mbox.TestMbox.test_target_mailing_list)
>>
>> ---
>>
>> Please address the issues identified and
>> submit a new revision of the patch, or alternatively, reply to this
>> email with an explanation of why the patch should be accepted. If you
>> believe these results are due to an error in patchtest, please submit a
>> bug at https://bugzilla.yoctoproject.org/ (use the 'Patchtest' category
>> under 'Yocto Project Subprojects'). For more information on specific
>> failures, see: https://wiki.yoctoproject.org/wiki/Patchtest. Thank
>> you!
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#202527): https://lists.openembedded.org/g/openembedded-core/message/202527
> Mute This Topic: https://lists.openembedded.org/mt/107559599/7611679
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [tgamblin@baylibre.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Joshua Watt July 26, 2024, 5:03 p.m. UTC | #6
On Fri, Jul 26, 2024 at 9:55 AM Böszörményi Zoltán <zboszor@gmail.com> wrote:
>
> 2024. 07. 26. 17:21 keltezéssel, Joshua Watt írta:
> > On Fri, Jul 26, 2024 at 5:24 AM Zoltán Böszörményi <zboszor@gmail.com> wrote:
> >> With commits 90bc1810 ("bitbake.conf: Add runtimedir") and
> >> 561e853e ("rpcbind: Specify state directory under /run") rpcbind
> >> still can fail during startup.
> >>
> >> It has two problems:
> >> * The lockfile is still hardcoded as "/var/run/rpcbind.lock".
> >>    It needs to use the same internal define for RPCBIND_STATEDIR
> >>    as the paths for rpcbind.xdr and portmap.xdr.
> >> * Using --with-statedir=/run/rpcbind doesn't guarantee that this
> >>    directory exists when rpcbind.service starts. Add this guarantee
> >>    by running rpcbind.service with After=systemd-tmpfiles-setup.service
> >>    and add the tmpfiles.d entry for /run/rpcbind.
> > The original patches have been working for me on nanbield (and
> > kirkstone), so I'm a little confused as to why this is not working for
> > you; I'd like to avoid patching the source code if at all possible as
> > carrying patches like this is a burden
>
> The error I got is:
>
> rpcbind[455]: rpcbind: /var/run/rpcbind.lock: Read-only file system
>
> So, while /var/run is a symlink to /run, the latter is not writable yet.
> Indeed, rpcbind.service was starting very early. These changes
> delay it enough to avoid the problem.
>
> The inconsistency between using /run via --with-statedir=/run
> directly for some the xdr files but not for the lockfile is not nice.
> That must have been an oversight in the source code.

Yes it is, however I suspect this patch isn't _strictly_ necessary to
fix the problem as long as the service file change is done (using a
drop file). That said, I'd rather not carry a patch unnecessarily and
we should attempt to fix it upstream instead of carry the patch.

The systemd service change should also be upstreamed, but thankfully
we can fix it with a drop file instead of a patch so I'm fine with
doing that.

>
> These changes fix the issue for me in nanbield. I can reproduce
> the above problem in scarthgap without these changes.
>
> FWIW, different machines can give very different testing results,
> due to the differences in their number of CPU cores and speed.
> Recently, I had to add a modified rngd.service from dracut
> to rng-tools as an in-house change, because one particular
> machine (slow by today's standards, based on AMD Kabini)
> ended up with this service failing, due to the CPU not supporting
> RDRAND and the JITTER random source driver starting particularly
> slowly on it, so rngd couldn't pick it up when started only from
> initramfs.
>
> >> Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
> >> ---
> >>   ...ke-the-lockfile-follow-with-statedir.patch | 31 ++++++++++++++++++
> >>   ...ice-after-etc-tmpfiles.d-is-processe.patch | 32 +++++++++++++++++++
> >>   .../rpcbind/rpcbind/rpcbind.tmpfiles          |  1 +
> >>   .../recipes-extended/rpcbind/rpcbind_1.2.6.bb |  6 ++++
> >>   4 files changed, 70 insertions(+)
> >>   create mode 100644 meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch
> >>   create mode 100644 meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch
> >>   create mode 100644 meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles
> >>
> >> diff --git a/meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch b/meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch
> >> new file mode 100644
> >> index 0000000000..d487312d22
> >> --- /dev/null
> >> +++ b/meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch
> >> @@ -0,0 +1,31 @@
> >> +From f10db88174e73c78c028561715f16ed38148ebde Mon Sep 17 00:00:00 2001
> >> +From: =?UTF-8?q?Zolt=C3=A1n=20B=C3=B6sz=C3=B6rm=C3=A9nyi?=
> >> + <zboszor@gmail.com>
> >> +Date: Fri, 26 Jul 2024 12:36:06 +0200
> >> +Subject: [PATCH 1/2] Make the lockfile follow --with-statedir=
> >> +MIME-Version: 1.0
> >> +Content-Type: text/plain; charset=UTF-8
> >> +Content-Transfer-Encoding: 8bit
> >> +
> >> +Upstream-Status: Inactive-Upstream [lastrelease: 2021-05-10]
> >> +Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
> >> +---
> >> + src/rpcbind.c | 2 +-
> >> + 1 file changed, 1 insertion(+), 1 deletion(-)
> >> +
> >> +diff --git a/src/rpcbind.c b/src/rpcbind.c
> >> +index 36a95b9..948a284 100644
> >> +--- a/src/rpcbind.c
> >> ++++ b/src/rpcbind.c
> >> +@@ -105,7 +105,7 @@ char *nss_modules = "files";
> >> + /* who to suid to if -s is given */
> >> + #define RUN_AS  "daemon"
> >> +
> >> +-#define RPCBINDDLOCK "/var/run/rpcbind.lock"
> >> ++#define RPCBINDDLOCK RPCBIND_STATEDIR "/rpcbind.lock"
> > /var/run should be a symlink to /run, so I really don't think this
> > should be necessary (at least with the default value for $runtimedir)
> >
> >> +
> >> + int runasdaemon = 0;
> >> + int insecure = 0;
> >> +--
> >> +2.45.2
> >> +
> >> diff --git a/meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch b/meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch
> >> new file mode 100644
> >> index 0000000000..f454fdbb3b
> >> --- /dev/null
> >> +++ b/meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch
> >> @@ -0,0 +1,32 @@
> >> +From 408b18c77c254baaa9111b3e7031ebf12149db38 Mon Sep 17 00:00:00 2001
> >> +From: =?UTF-8?q?Zolt=C3=A1n=20B=C3=B6sz=C3=B6rm=C3=A9nyi?=
> >> + <zboszor@gmail.com>
> >> +Date: Fri, 26 Jul 2024 12:54:38 +0200
> >> +Subject: [PATCH 2/2] Run rpcbind.service after /etc/tmpfiles.d is processed
> >> +MIME-Version: 1.0
> >> +Content-Type: text/plain; charset=UTF-8
> >> +Content-Transfer-Encoding: 8bit
> >> +
> >> +Upstream-Status: Inactive-Upstream [lastrelease: 2021-05-10]
> >> +Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
> >> +---
> >> + systemd/rpcbind.service.in | 3 +++
> >> + 1 file changed, 3 insertions(+)
> >> +
> >> +diff --git a/systemd/rpcbind.service.in b/systemd/rpcbind.service.in
> >> +index f45ee1e..e9d94ce 100644
> >> +--- a/systemd/rpcbind.service.in
> >> ++++ b/systemd/rpcbind.service.in
> >> +@@ -9,6 +9,9 @@ RequiresMountsFor=@statedir@
> >> + Requires=rpcbind.socket
> >> + Wants=rpcbind.target
> >> +
> >> ++# Make sure the runtime directory exists
> >> ++After=systemd-tmpfiles-setup.service
> > I'd rather have the recipe provide a drop file for this instead of
> > patching the source
> >
> >> ++
> >> + [Service]
> >> + Type=notify
> >> + EnvironmentFile=-@_sysconfdir@/rpcbind.conf
> >> +--
> >> +2.45.2
> >> +
> >> diff --git a/meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles b/meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles
> >> new file mode 100644
> >> index 0000000000..fecee72c09
> >> --- /dev/null
> >> +++ b/meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles
> >> @@ -0,0 +1 @@
> >> +d /run/rpcbind 0755 root root -
> >> diff --git a/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb b/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb
> >> index e751eb631c..477092b37d 100644
> >> --- a/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb
> >> +++ b/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb
> >> @@ -13,8 +13,11 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=b46486e4c4a416602693a711bb5bfa39 \
> >>   SRC_URI = "${SOURCEFORGE_MIRROR}/rpcbind/rpcbind-${PV}.tar.bz2 \
> >>              file://init.d \
> >>              file://rpcbind.conf \
> >> +           file://rpcbind.tmpfiles \
> >>              file://rpcbind_add_option_to_fix_port_number.patch \
> >>              file://0001-systemd-use-EnvironmentFile.patch \
> >> +           file://0001-Make-the-lockfile-follow-with-statedir.patch \
> >> +           file://0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch \
> >>             "
> >>   SRC_URI[sha256sum] = "5613746489cae5ae23a443bb85c05a11741a5f12c8f55d2bb5e83b9defeee8de"
> >>
> >> @@ -49,6 +52,9 @@ do_install:append () {
> >>                  ${UNPACKDIR}/init.d > ${D}${sysconfdir}/init.d/rpcbind
> >>          chmod 0755 ${D}${sysconfdir}/init.d/rpcbind
> >>          install -m 0644 ${UNPACKDIR}/rpcbind.conf ${D}${sysconfdir}/rpcbind.conf
> >> +
> >> +       install -d ${D}${sysconfdir}/tmpfiles.d
> >> +       install -m 0644 ${UNPACKDIR}/rpcbind.tmpfiles ${D}${sysconfdir}/tmpfiles.d/rpcbind.conf
> >>   }
> >>
> >>   ALTERNATIVE:${PN} = "rpcinfo"
> >> --
> >> 2.45.2
> >>
>
diff mbox series

Patch

diff --git a/meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch b/meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch
new file mode 100644
index 0000000000..d487312d22
--- /dev/null
+++ b/meta/recipes-extended/rpcbind/rpcbind/0001-Make-the-lockfile-follow-with-statedir.patch
@@ -0,0 +1,31 @@ 
+From f10db88174e73c78c028561715f16ed38148ebde Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Zolt=C3=A1n=20B=C3=B6sz=C3=B6rm=C3=A9nyi?=
+ <zboszor@gmail.com>
+Date: Fri, 26 Jul 2024 12:36:06 +0200
+Subject: [PATCH 1/2] Make the lockfile follow --with-statedir=
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Upstream-Status: Inactive-Upstream [lastrelease: 2021-05-10]
+Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
+---
+ src/rpcbind.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/rpcbind.c b/src/rpcbind.c
+index 36a95b9..948a284 100644
+--- a/src/rpcbind.c
++++ b/src/rpcbind.c
+@@ -105,7 +105,7 @@ char *nss_modules = "files";
+ /* who to suid to if -s is given */
+ #define RUN_AS  "daemon"
+ 
+-#define RPCBINDDLOCK "/var/run/rpcbind.lock"
++#define RPCBINDDLOCK RPCBIND_STATEDIR "/rpcbind.lock"
+ 
+ int runasdaemon = 0;
+ int insecure = 0;
+-- 
+2.45.2
+
diff --git a/meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch b/meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch
new file mode 100644
index 0000000000..f454fdbb3b
--- /dev/null
+++ b/meta/recipes-extended/rpcbind/rpcbind/0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch
@@ -0,0 +1,32 @@ 
+From 408b18c77c254baaa9111b3e7031ebf12149db38 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Zolt=C3=A1n=20B=C3=B6sz=C3=B6rm=C3=A9nyi?=
+ <zboszor@gmail.com>
+Date: Fri, 26 Jul 2024 12:54:38 +0200
+Subject: [PATCH 2/2] Run rpcbind.service after /etc/tmpfiles.d is processed
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Upstream-Status: Inactive-Upstream [lastrelease: 2021-05-10]
+Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
+---
+ systemd/rpcbind.service.in | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/systemd/rpcbind.service.in b/systemd/rpcbind.service.in
+index f45ee1e..e9d94ce 100644
+--- a/systemd/rpcbind.service.in
++++ b/systemd/rpcbind.service.in
+@@ -9,6 +9,9 @@ RequiresMountsFor=@statedir@
+ Requires=rpcbind.socket
+ Wants=rpcbind.target
+ 
++# Make sure the runtime directory exists
++After=systemd-tmpfiles-setup.service
++
+ [Service]
+ Type=notify
+ EnvironmentFile=-@_sysconfdir@/rpcbind.conf
+-- 
+2.45.2
+
diff --git a/meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles b/meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles
new file mode 100644
index 0000000000..fecee72c09
--- /dev/null
+++ b/meta/recipes-extended/rpcbind/rpcbind/rpcbind.tmpfiles
@@ -0,0 +1 @@ 
+d /run/rpcbind 0755 root root -
diff --git a/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb b/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb
index e751eb631c..477092b37d 100644
--- a/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb
+++ b/meta/recipes-extended/rpcbind/rpcbind_1.2.6.bb
@@ -13,8 +13,11 @@  LIC_FILES_CHKSUM = "file://COPYING;md5=b46486e4c4a416602693a711bb5bfa39 \
 SRC_URI = "${SOURCEFORGE_MIRROR}/rpcbind/rpcbind-${PV}.tar.bz2 \
            file://init.d \
            file://rpcbind.conf \
+           file://rpcbind.tmpfiles \
            file://rpcbind_add_option_to_fix_port_number.patch \
            file://0001-systemd-use-EnvironmentFile.patch \
+           file://0001-Make-the-lockfile-follow-with-statedir.patch \
+           file://0002-Run-rpcbind.service-after-etc-tmpfiles.d-is-processe.patch \
           "
 SRC_URI[sha256sum] = "5613746489cae5ae23a443bb85c05a11741a5f12c8f55d2bb5e83b9defeee8de"
 
@@ -49,6 +52,9 @@  do_install:append () {
 		${UNPACKDIR}/init.d > ${D}${sysconfdir}/init.d/rpcbind
 	chmod 0755 ${D}${sysconfdir}/init.d/rpcbind
 	install -m 0644 ${UNPACKDIR}/rpcbind.conf ${D}${sysconfdir}/rpcbind.conf
+
+	install -d ${D}${sysconfdir}/tmpfiles.d
+	install -m 0644 ${UNPACKDIR}/rpcbind.tmpfiles ${D}${sysconfdir}/tmpfiles.d/rpcbind.conf
 }
 
 ALTERNATIVE:${PN} = "rpcinfo"