Skip to content

RDKEMW-10363: Update logrotate Conf from Middleware Layer on RDK devices #332

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
madhubabutt wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

RDKEMW-10363: Update logrotate Conf from Middleware Layer on RDK devices #332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
95 changes: 0 additions & 95 deletions recipes-extended/logrotate/logrotate/logrotate-update-service-files.patch
View file
Open in desktop

This file was deleted.

12 changes: 12 additions & 0 deletions recipes-extended/logrotate/logrotate/logrotate_nohdd.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/opt/logs/*.txt
/opt/logs/*.log
/opt/logs/*.txt.0{
Comment on lines +1 to +3
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

The logrotate configuration includes overlapping file patterns: '/opt/logs/.txt', '/opt/logs/.log', and '/opt/logs/.txt.0'. The third pattern '/opt/logs/.txt.0' appears to target already-rotated files, which would match files like 'example.txt.0'. This could cause logrotate to attempt rotating already rotated files, leading to unexpected behavior. Verify if this is intentional or if it should be adjusted to only target active log files.

Suggested change
/opt/logs/*.txt
/opt/logs/*.log
/opt/logs/*.txt.0{
/opt/logs/*.txt /opt/logs/*.log {

Copilot uses AI. Check for mistakes.
size 2097152
rotate 1
create
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The new logrotate_nohdd.conf configuration file uses 'create' directive on line 6, which differs from the previous configuration in the deleted patch file that used 'copytruncate'. This is a significant behavioral change:

  • 'copytruncate': Copies the log file and truncates the original file in place, keeping the same inode and file descriptor. Applications can continue writing to the same file descriptor without needing to reopen.
  • 'create': Renames the original log file and creates a new empty file, changing the inode. Applications need to reopen the file or receive a signal to write to the new file.

The postrotate script on line 10 uses 'syslog-ng-ctl reopen' which should handle the file descriptor reopen. However, this change may affect other applications writing to these log files. Verify that all applications writing to /opt/logs/.txt and /opt/logs/.log files properly handle log rotation with the 'create' directive, or consider whether 'copytruncate' should be retained for backward compatibility.

Copilot uses AI. Check for mistakes.
missingok
ignoreduplicates
postrotate
syslog-ng-ctl reopen --control=/tmp/syslog-ng/syslog-ng.ctl
endscript
Comment on lines +6 to +11
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

The new logrotate_nohdd.conf file uses different directives compared to the version in the deleted patch. Specifically, it replaces 'copytruncate' with 'create' and adds a 'postrotate' script to reopen syslog-ng. This is a significant behavioral change: 'copytruncate' copies and then truncates the file in place, while 'create' moves the file and creates a new one. The postrotate script added is necessary with 'create' mode to signal syslog-ng to reopen log files, but this change should be verified to ensure it matches the intended behavior for RDK devices.

Copilot uses AI. Check for mistakes.
}
4 changes: 1 addition & 3 deletions recipes-extended/logrotate/logrotate_3.21.0.bb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ do_install(){
mkdir -p ${D}${sysconfdir}/logrotate.d
mkdir -p ${D}${localstatedir}/lib
install -p -m 644 ${S}/examples/logrotate.conf ${D}${sysconfdir}/logrotate.conf
install -p -m 644 ${S}/examples/btmp ${D}${sysconfdir}/logrotate.d/btmp
install -p -m 644 ${S}/examples/wtmp ${D}${sysconfdir}/logrotate.d/wtmp
touch ${D}${localstatedir}/lib/logrotate.status
Comment on lines 70 to 73
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The CONFFILES variable still references btmp and wtmp configuration files that are no longer being installed. Since the installation of these files was removed (lines 73-74 in the diff), they should also be removed from the CONFFILES list to prevent build or runtime warnings about missing configuration files.

Copilot uses AI. Check for mistakes.
Comment on lines 72 to 73
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The CONFFILES variable in this recipe references btmp and wtmp configuration files that are no longer being installed. Lines 26-27 declare btmp and wtmp as configuration files, but the installation commands for these files were removed from the do_install function. This inconsistency will cause packaging issues as the system will expect these files to exist but they won't be installed. Consider either:

  1. Removing the btmp and wtmp references from CONFFILES, or
  2. Keeping the installation of these files if they are actually needed

Copilot uses AI. Check for mistakes.


Expand All @@ -84,5 +82,5 @@ do_install(){


do_install:append:client(){
install -m 0644 ${S}/logrotate_nohdd.conf ${D}${sysconfdir}/logrotatemax.conf
install -m 0644 ${WORKDIR}/logrotate_nohdd.conf ${D}${sysconfdir}/logrotatemax.conf
}
12 changes: 10 additions & 2 deletions recipes-extended/logrotate/logrotate_3.21.0.bbappend
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
FILESEXTRAPATHS:prepend := "${THISDIR}/${BPN}:"

SRC_URI:append = " file://logrotate-update-service-files.patch \
file://logrotate_memory_issues.patch \
SRC_URI:append = " file://logrotate_memory_issues.patch \
file://logrotate-update-log-files.patch \
file://fix_fd_leak.patch \
file://logrotate_nohdd.conf \
"

PACKAGES =+ "${PN}-conf"

FILES:${PN}-conf += "${sysconfdir}/logrotatemax.conf"
FILES:${PN}-conf += "${sysconfdir}/logrotate.conf"

FILES:${PN}:remove = "${sysconfdir}/logrotatemax.conf"
Comment on lines +11 to +14
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

The logrotatemax.conf file is assigned to the new logrotate-conf package but only gets installed on client machines (via do_install:append:client). This means the logrotate-conf package will be incomplete on non-client builds, potentially containing only logrotate.conf but not logrotatemax.conf. Consider whether the package splitting logic should account for machine-specific configurations or if the FILES assignment should be conditional.

Suggested change
FILES:${PN}-conf += "${sysconfdir}/logrotatemax.conf"
FILES:${PN}-conf += "${sysconfdir}/logrotate.conf"
FILES:${PN}:remove = "${sysconfdir}/logrotatemax.conf"
FILES:${PN}-conf:client += "${sysconfdir}/logrotatemax.conf"
FILES:${PN}-conf += "${sysconfdir}/logrotate.conf"
FILES:${PN}:client:remove = "${sysconfdir}/logrotatemax.conf"

Copilot uses AI. Check for mistakes.
FILES:${PN}:remove = "${sysconfdir}/logrotate.conf"