Skip to content
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.

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

Patches 2 upstream systemd #32430

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rajmohanrgithub
Copy link

@rajmohanrgithub rajmohanrgithub commented Apr 23, 2024

Message from commits is given below:

systemd: Add more logs for boot procedure analysis
systemd-analyze: Add svg scaling options
systemd: set suid_dumpable to "suidssafe"
mount: Remove files in place of mount points

@github-actions github-actions bot added the udev label Apr 23, 2024
@rajmohanrgithub rajmohanrgithub force-pushed the patches-2-upstream-systemd branch 2 times, most recently from 6f51013 to d866b87 Compare April 23, 2024 10:05
* unit.c: log unit names and timestamp of unit state change
* manager.c: log time stamp when boot is completed
* main.c: log time stamp with systemd internal states

Signed-off-by: rajmohan r <rajmohan.r@kpit.com>
+ Scale the x-axis of the resulting plot by a factor (default 1.0)
+ Add activation timestamps to each bar

Signed-off-by: rajmohan r <rajmohan.r@kpit.com>
We want core dumps from regularly skipped files (changing credentials
or lacking read permission)

Signed-off-by: rajmohan r <rajmohan.r@kpit.com>
If a mount point is not a directory, systemd will remove() it and then
mkdir_p() the full path.

Signed-off-by: rajmohan r <rajmohan.r@kpit.com>
@rajmohanrgithub rajmohanrgithub marked this pull request as ready for review April 23, 2024 10:25
@keszybz
Copy link
Member

keszybz commented Apr 23, 2024

Please set the title of the pull request.

@@ -1699,6 +1699,17 @@ static void cmdline_take_random_seed(void) {
"This functionality should not be used outside of testing environments.");
}

static void set_suid_dumpable(void) {
const char *sd_path = "/proc/sys/fs/suid_dumpable";
Copy link
Member

Choose a reason for hiding this comment

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

Uh, we already do this?

fs.suid_dumpable=2

@@ -2032,6 +2032,14 @@ static int invoke_main_loop(
assert(ret_switch_root_init);
assert(ret_error_message);

log_warning("sd: i=userspace, inactive_exit="USEC_FMT, m->timestamps[MANAGER_TIMESTAMP_USERSPACE].monotonic);
Copy link
Member

Choose a reason for hiding this comment

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

If anything, these definitely shouldn't be logged at the warning level (debug seems to be appropriate).

Copy link
Member

Choose a reason for hiding this comment

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

and they can all be queried at runtime, and some are already logged later, so doesn't seem worth adding any of these

@@ -1168,6 +1169,16 @@ static void mount_enter_mounting(Mount *m) {
m->control_command_id = MOUNT_EXEC_MOUNT;
m->control_command = m->exec_command + MOUNT_EXEC_MOUNT;

dp = opendir(m->where);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to nuke any files from the filesystem which are not under our direct control.

@mrc0mmand
Copy link
Member

@@ -274,6 +276,8 @@ static int help(int argc, char *argv[], void *userdata) {
" security review of the unit(s)\n"
" --unit=UNIT Evaluate conditions and asserts of unit\n"
" --table Output plot's raw time data as a table\n"
" -s --scale=FACTOR Stretch the x-axis of the plot by FACTOR (default: 1.0)\n"
Copy link
Member

Choose a reason for hiding this comment

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

shorthands are prime real estate, and sd-analyze has tons of options and will get more. This is not something common enough to warrant taking over one of the 52 available single-letter options, so please just use the long one

@bluca bluca added the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Apr 23, 2024
@bluca
Copy link
Member

bluca commented Apr 23, 2024

Please split the sd-analyze commit into a separate PR, as that looks reasonable and could be merged quickly once fixed up

@YHNdnzj
Copy link
Member

YHNdnzj commented Apr 23, 2024

Sorry, but as mentioned, all commits except for the sd-analyze one make no sense at all. The timestamps can be queried through dbus, and we do not do the intense logging about them. The mount one is even more problematic. What's the rationale???

@YHNdnzj YHNdnzj added pid1 needs-reporter-feedback ❓ There's an unanswered question, the reporter needs to answer labels Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-reporter-feedback ❓ There's an unanswered question, the reporter needs to answer pid1 reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks udev
Development

Successfully merging this pull request may close these issues.

None yet

5 participants