On Thu, 01.01.15 19:15, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
> On Tue, Dec 30, 2014 at 08:22:27PM +0100, Jouke Witteveen wrote: > > --- > > > > This fixes #87251 > > This is actually important information that should be included in the > commit message (i.e. above not below "---"). We usually include the > full url, since we also use distribution bug trackers and having the full > url makes things easier to click. In this case: > > https://bugs.freedesktop.org/show_bug.cgi?id=87251 > > > static void service_enter_reload_by_notify(Service *s) { > > + _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL; > > + int r; > > + > > assert(s); > > > > if (s->timeout_start_usec > 0) > > service_arm_timer(s, s->timeout_start_usec); > > > > + r = manager_propagate_reload(UNIT(s)->manager, UNIT(s), > > JOB_REPLACE, false, &error); > > + if(r < 0) > > + log_unit_warning(UNIT(s)->id, "%s failed to schedule > > propagation of reload: %s", UNIT(s)->id, bus_error_message(&error, -r)); > > + > > Let's say that a.service has PropagateReloadsTo=b.service, and a.service > provides > the RELOADING=1 notification during a reload. > What happens if a reload is requested with 'systemctl reload a', and systemd > schedules a reload of a and b. Is it possible for b to be reloaded a second > time > as a result of notification of a? This should not happen, have you verified > that > this will not happen? THis is a real issue I fear. If service units use an asynchronous command in ExecReload= (such as one that just sends a SIGHUP signal to the daemon and terminates), then the daemon might send its RELOADING=1 later than the ExecReload= finishes. This would then trigger a second reload in b.service... This problem goes away if the people use synchronous commands in ExecReload= however, that wait for the reload to complete. In that case the RELOADING=1 from the service would be queued in s->notify_state, and be unset again when the daemon reports READY=1 again. And only after that the ExecReload= process would exit, and hence no second reload be propagated. (this is race-free because we always process notification messages before SIGCHLD). Now I wonder what to do with this. I think the change would be welcome, but I wonder if we can simply require people who use PropagateReloadsTo= to define synchronous ExecReload= processes or accept double reloads to be forwarded... I leaning towards merging the patch, but this would require an explanation in the man page of sd_notify() and in systemd.unit near the description of PropagateReloadsTo=. Jouke, could you resend this patch with such an addition? Thanks, Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
