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

feat: reboot(2) for OpenBSD/NetBSD #2251

Merged
merged 1 commit into from
Dec 9, 2023

Conversation

SteveLauC
Copy link
Member

What does this PR do

Implements reboot(2) for NetBSD/OpenBSD, closes #2174.

Manual:

The RebootMode type, different from the one defined on Linux, is a bitflag rather than an enum, so I defined a new type. On NetBSD, there is a RebootMode RB_RDONLY:

RB_RDONLY 0x0080

Initially mount the root file system read-only. This is currently the default, and this option has been deprecated.

which is deprecated, so I didn't add it.

Checklist:

  • I have read CONTRIBUTING.md
  • I have written necessary tests and rustdoc comments
  • A change log has been added if this PR modifies nix's API

/// For more information, see [`reboot(2)`](https://man.netbsd.org/reboot.2).
#[cfg(target_os = "netbsd")]
pub fn reboot<S: AsRef<OsStr>>(how: RebootMode, bootstr: S) -> Result<Infallible> {
let bootstr = bootstr.as_ref().as_bytes().as_ptr().cast_mut().cast();
Copy link
Member Author

@SteveLauC SteveLauC Dec 4, 2023

Choose a reason for hiding this comment

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

int reboot(int howto, char *bootstr);
fn reboot(mode: ::c_int, bootstr: *mut ::c_char) -> ::c_int;

If the underlying shutdown procedure will modify bootstr, then we will get a UB here. I am not sure if reboot(2) would change it or not, the manual does not mention this:

bootstr is a string passed to the firmware on the machine, if possible, if this option is set. Currently this is only implemented on the sparc and the sun3 ports.

But practically, it has little point in writing to that piece of memory given that everything will be gone after shutdown.

Copy link
Member

Choose a reason for hiding this comment

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

The man page seems to suggest that bootstr is entirely ignored unless RB_STRING is specified, and that can only happen on sparc64. So I suggest that we simply remove this argument and the RB_STRING flag until such a time as there is a least one known user of Nix on sparc64-unknown-netbsd.

Copy link
Member Author

Choose a reason for hiding this comment

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

argument removed

}

/// Reboots or shuts down the system.
#[cfg(target_os = "linux")]
Copy link
Member

Choose a reason for hiding this comment

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

This line is redundant with line 11

Suggested change
#[cfg(target_os = "linux")]

Copy link
Member Author

Choose a reason for hiding this comment

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

I am sorry about this, removed

/// Enable or disable the reboot keystroke (Ctrl-Alt-Delete).
///
/// Corresponds to calling `reboot(RB_ENABLE_CAD)` or `reboot(RB_DISABLE_CAD)` in C.
#[cfg(target_os = "linux")]
Copy link
Member

Choose a reason for hiding this comment

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

This line too.

Suggested change
#[cfg(target_os = "linux")]

/// `init(8)`) other than `/sbin/init` to be run when the system
/// reboots.
///
/// This switch is not currently available.
Copy link
Member

Choose a reason for hiding this comment

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

What does it mean that it isn't currently available? Does that mean that NetBSD defines it but does not implement it? If so, I think that Nix shouldn't expose it.

Copy link
Member Author

Choose a reason for hiding this comment

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

From this man page, seems that it is obsolete and replaced with RB_ASKNAME, removed

RB_INITNAME 0x00000010 This flag is obsolete. It was previously used to cause the kernel to prompt for the name of the init(8) program, but that function is now controlled by the RB_ASKNAME flag.

/// on the machine, if possible, if this option is set.
///
/// Currently this is only implemented on the sparc and the sun3 ports.
#[cfg(target_os = "netbsd")]
Copy link
Member

Choose a reason for hiding this comment

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

Let's define it for sparc64 only. It looks like NetBSD doesn't have a Rust package for sun3, so we don't have to worry about that. https://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/lang/rust/index.html

Suggested change
#[cfg(target_os = "netbsd")]
#[cfg(all(target_os = "netbsd", target_arch = "sparc64"))]

Copy link
Member Author

@SteveLauC SteveLauC Dec 9, 2023

Choose a reason for hiding this comment

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

This flag and that bootstr argument are now removed

/// For more information, see [`reboot(2)`](https://man.netbsd.org/reboot.2).
#[cfg(target_os = "netbsd")]
pub fn reboot<S: AsRef<OsStr>>(how: RebootMode, bootstr: S) -> Result<Infallible> {
let bootstr = bootstr.as_ref().as_bytes().as_ptr().cast_mut().cast();
Copy link
Member

Choose a reason for hiding this comment

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

The man page seems to suggest that bootstr is entirely ignored unless RB_STRING is specified, and that can only happen on sparc64. So I suggest that we simply remove this argument and the RB_STRING flag until such a time as there is a least one known user of Nix on sparc64-unknown-netbsd.

@asomers asomers added this pull request to the merge queue Dec 9, 2023
Merged via the queue into nix-rust:master with commit d518abd Dec 9, 2023
35 checks passed
@SteveLauC SteveLauC deleted the reboot_openbsd branch December 10, 2023 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

reboot api on openbsd
2 participants