Revert "switch-to-configuration-ng: wait for NameOwnerChanged after systemd Reexecute"
This reverts commit 67b8817f26.
That commit introduces a new error when:
- switch-to-configuration-ng spawns the "per-user"
switch-to-configuration-ng (the one that does do_user_switch).
- That "per-user" switch-to-configuration-ng instance connects to the
D-Bus session bus, but the bus is not up yet.
- The D-Bus session bus is configured to use systemd socket activation
and is currently being started by the systemd user manager.
- The D-Bus session bus finishes starting. At this point, only
switch-to-configuration-ng is connected to it. Systemd is not
connected yet because it waits for a ready notification from the D-Bus
session bus.
- switch-to-configuration-ng starts listening for a NameOwnerChanged
signal with the org.freedesktop.systemd1 bus name.
- Systemd is notified that the D-Bus session bus is ready, so it
connects to it and acquires the name org.freedesktop.systemd1, which
triggers the NameOwnerChanged signal that we listen to.
- switch-to-configuration-ng marks systemd as reexecuted, but it's _not
started reexecuting yet_.
- Then switch-to-configuration-ng tells systemd to reexecute.
- The loop waiting for systemd to reexecute [1] is never entered.
- switch-to-configuration-ng asks systemd to restart a unit but that
fails because systemd is reexecuting and the method call is sent to
the old deamon.
The idea with #442756 was to:
1. Remove the timeout for the Reexecute method call because it would
fail in a non-obvious way when systemd took too much time to execute,
and that could happen when there are many .mount units.
2. While we're at it, use the NameOwnerChanged signal to determine when
systemd is done reexecuting before we proceed with the other systemd
calls, instead of waiting for the Reexecute method call to error.
But in fact, it seems like 2. is not a good idea. Waiting for the
NameOwnerChanged signal has the disadvantage of not letting us know if
it is due to our Reexecute method call or someone else's, or even just
systemd that's just connected to the D-Bus in between as is the case
here.
It might still be a good idea to increase the timeout for the Reexecute
method call to fix the original error, but at least let's revert the PR.
[1]: https://github.com/NixOS/nixpkgs/blob/ec481667d737b1b366ac55d72ec09d101d842533/pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs#L989
This commit is contained in:
@@ -18,10 +18,6 @@ fn main() {
|
||||
|
||||
let out_path = std::path::PathBuf::from(std::env::var("OUT_DIR").unwrap());
|
||||
|
||||
let fdo_dbus_code = code_for_dbus_xml("org.freedesktop.DBus.xml");
|
||||
let mut file = std::fs::File::create(out_path.join("fdo_dbus.rs")).unwrap();
|
||||
file.write_all(fdo_dbus_code.as_bytes()).unwrap();
|
||||
|
||||
let systemd_manager_code =
|
||||
code_for_dbus_xml(systemd_dbus_interface_dir.join("org.freedesktop.systemd1.Manager.xml"));
|
||||
let mut file = std::fs::File::create(out_path.join("systemd_manager.rs")).unwrap();
|
||||
|
||||
@@ -1,12 +0,0 @@
|
||||
<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN"
|
||||
"https://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
|
||||
<node>
|
||||
<!-- From https://dbus.freedesktop.org/doc/dbus-specification.html#message-bus-messages. -->
|
||||
<interface name="org.freedesktop.DBus">
|
||||
<signal name="NameOwnerChanged">
|
||||
<arg type="s" direction="out" name="name"/>
|
||||
<arg type="s" direction="out" name="old_owner"/>
|
||||
<arg type="s" direction="out" name="new_owner"/>
|
||||
</signal>
|
||||
</interface>
|
||||
</node>
|
||||
@@ -17,8 +17,6 @@ use std::{
|
||||
use anyhow::{anyhow, bail, Context, Result};
|
||||
use dbus::{
|
||||
blocking::{stdintf::org_freedesktop_dbus::Properties, LocalConnection, Proxy},
|
||||
channel::Sender,
|
||||
strings::{BusName, Interface, Member},
|
||||
Message,
|
||||
};
|
||||
use glob::glob;
|
||||
@@ -34,15 +32,6 @@ use nix::{
|
||||
use regex::Regex;
|
||||
use syslog::Facility;
|
||||
|
||||
mod fdo_dbus {
|
||||
#![allow(non_upper_case_globals)]
|
||||
#![allow(non_camel_case_types)]
|
||||
#![allow(non_snake_case)]
|
||||
#![allow(unused)]
|
||||
#![allow(clippy::all)]
|
||||
include!(concat!(env!("OUT_DIR"), "/fdo_dbus.rs"));
|
||||
}
|
||||
|
||||
mod systemd_manager {
|
||||
#![allow(non_upper_case_globals)]
|
||||
#![allow(non_camel_case_types)]
|
||||
@@ -61,9 +50,7 @@ mod logind_manager {
|
||||
include!(concat!(env!("OUT_DIR"), "/logind_manager.rs"));
|
||||
}
|
||||
|
||||
use crate::{
|
||||
fdo_dbus::OrgFreedesktopDBusNameOwnerChanged, systemd_manager::OrgFreedesktopSystemd1Manager,
|
||||
};
|
||||
use crate::systemd_manager::OrgFreedesktopSystemd1Manager;
|
||||
use crate::{
|
||||
logind_manager::OrgFreedesktopLogin1Manager,
|
||||
systemd_manager::{
|
||||
@@ -907,14 +894,6 @@ impl std::fmt::Display for Job {
|
||||
}
|
||||
}
|
||||
|
||||
fn fdo_dbus_proxy(conn: &LocalConnection) -> Proxy<'_, &LocalConnection> {
|
||||
conn.with_proxy(
|
||||
"org.freedesktop.DBus",
|
||||
"/org/freedesktop/DBus",
|
||||
Duration::from_millis(500),
|
||||
)
|
||||
}
|
||||
|
||||
fn systemd1_proxy(conn: &LocalConnection) -> Proxy<'_, &LocalConnection> {
|
||||
conn.with_proxy(
|
||||
"org.freedesktop.systemd1",
|
||||
@@ -951,54 +930,6 @@ fn remove_file_if_exists(p: impl AsRef<Path>) -> std::io::Result<()> {
|
||||
}
|
||||
}
|
||||
|
||||
fn reexecute_systemd_manager(
|
||||
dbus_conn: &LocalConnection,
|
||||
fdo_dbus: &Proxy<'_, &LocalConnection>,
|
||||
) -> anyhow::Result<()> {
|
||||
let reexecute_done = Rc::new(RefCell::new(false));
|
||||
let _reexecute_done = reexecute_done.clone();
|
||||
let owner_changed_token = fdo_dbus
|
||||
.match_signal(
|
||||
move |signal: OrgFreedesktopDBusNameOwnerChanged, _: &LocalConnection, _: &Message| {
|
||||
if signal.name.as_str() == "org.freedesktop.systemd1" {
|
||||
*_reexecute_done.borrow_mut() = true;
|
||||
}
|
||||
|
||||
true
|
||||
},
|
||||
)
|
||||
.context("Failed to add signal match for DBus name owner changes")?;
|
||||
|
||||
let bus_name = BusName::from("org.freedesktop.systemd1");
|
||||
let object_path = dbus::Path::from("/org/freedesktop/systemd1");
|
||||
let interface = Interface::new("org.freedesktop.systemd1.Manager")
|
||||
.expect("the org.freedesktop.systemd1.Manager interface name should be valid");
|
||||
let method_name = Member::new("Reexecute").expect("the Reexecute method name should be valid");
|
||||
|
||||
// Systemd does not reply to the Reexecute method.
|
||||
let _serial = dbus_conn
|
||||
.send(Message::method_call(
|
||||
&bus_name,
|
||||
&object_path,
|
||||
&interface,
|
||||
&method_name,
|
||||
))
|
||||
.map_err(|_err| anyhow!("Failed to send org.freedesktop.systemd1.Manager.Reexecute"))?;
|
||||
|
||||
log::debug!("waiting for systemd to finish reexecuting");
|
||||
while !*reexecute_done.borrow() {
|
||||
_ = dbus_conn
|
||||
.process(Duration::from_secs(500))
|
||||
.context("Failed to process dbus messages")?;
|
||||
}
|
||||
|
||||
dbus_conn
|
||||
.remove_match(owner_changed_token)
|
||||
.context("Failed to remove jobs token")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Performs switch-to-configuration functionality for a single non-root user
|
||||
fn do_user_switch(parent_exe: String) -> anyhow::Result<()> {
|
||||
if Path::new(&parent_exe)
|
||||
@@ -1014,11 +945,8 @@ fn do_user_switch(parent_exe: String) -> anyhow::Result<()> {
|
||||
}
|
||||
|
||||
let dbus_conn = LocalConnection::new_session().context("Failed to open dbus connection")?;
|
||||
let fdo_dbus = fdo_dbus_proxy(&dbus_conn);
|
||||
let systemd = systemd1_proxy(&dbus_conn);
|
||||
|
||||
reexecute_systemd_manager(&dbus_conn, &fdo_dbus)?;
|
||||
|
||||
let nixos_activation_done = Rc::new(RefCell::new(false));
|
||||
let _nixos_activation_done = nixos_activation_done.clone();
|
||||
let jobs_token = systemd
|
||||
@@ -1035,6 +963,10 @@ fn do_user_switch(parent_exe: String) -> anyhow::Result<()> {
|
||||
)
|
||||
.context("Failed to add signal match for systemd removed jobs")?;
|
||||
|
||||
// The systemd user session seems to not send a Reloaded signal, so we don't have anything to
|
||||
// wait on here.
|
||||
_ = systemd.reexecute();
|
||||
|
||||
systemd
|
||||
.restart_unit("nixos-activation.service", "replace")
|
||||
.context("Failed to restart nixos-activation.service")?;
|
||||
@@ -1207,7 +1139,6 @@ won't take effect until you reboot the system.
|
||||
let mut units_to_reload = map_from_list_file(RELOAD_LIST_FILE);
|
||||
|
||||
let dbus_conn = LocalConnection::new_system().context("Failed to open dbus connection")?;
|
||||
let fdo_dbus = fdo_dbus_proxy(&dbus_conn);
|
||||
let systemd = systemd1_proxy(&dbus_conn);
|
||||
let logind = login1_proxy(&dbus_conn);
|
||||
|
||||
@@ -1733,7 +1664,7 @@ won't take effect until you reboot the system.
|
||||
// just in case the new one has trouble communicating with the running pid 1.
|
||||
if restart_systemd {
|
||||
eprintln!("restarting systemd...");
|
||||
reexecute_systemd_manager(&dbus_conn, &fdo_dbus)?;
|
||||
_ = systemd.reexecute(); // we don't get a dbus reply here
|
||||
|
||||
log::debug!("waiting for systemd restart to finish");
|
||||
while !*systemd_reload_status.borrow() {
|
||||
|
||||
Reference in New Issue
Block a user