[systemd/f17] Fixes for two bugs from the F17Blocker tracker

Michal Schmidt michich at fedoraproject.org
Wed Apr 25 21:21:37 UTC 2012


commit 4318c5a47ec0536186831f98f1689eecbf314580
Author: Michal Schmidt <mschmidt at redhat.com>
Date:   Wed Apr 25 23:20:47 2012 +0200

    Fixes for two bugs from the F17Blocker tracker
    
    Rescue shell on fsck errors (#798328)
    Add systemd-timedated-ntp.target to avoid harcoded ntpd.service
    in timedated. Allows chrony to hook into it. (#815748)

 ...on-cancel-jobs-non-recursively-on-isolate.patch |  244 ++++++++++++++++++++
 ...ntroduce-systemd-timedated-ntp.target-whi.patch |   97 ++++++++
 systemd.spec                                       |   10 +-
 3 files changed, 350 insertions(+), 1 deletions(-)
---
diff --git a/0004-transaction-cancel-jobs-non-recursively-on-isolate.patch b/0004-transaction-cancel-jobs-non-recursively-on-isolate.patch
new file mode 100644
index 0000000..fe58305
--- /dev/null
+++ b/0004-transaction-cancel-jobs-non-recursively-on-isolate.patch
@@ -0,0 +1,244 @@
+From efa270eec499f02cfb949c0090e2d7409c729e1c Mon Sep 17 00:00:00 2001
+From: Michal Schmidt <mschmidt at redhat.com>
+Date: Tue, 24 Apr 2012 11:21:03 +0200
+Subject: [PATCH] transaction: cancel jobs non-recursively on isolate
+
+Recursive cancellation of jobs would trigger OnFailure actions of
+dependent jobs. This is not desirable when isolating.
+
+Fixes https://bugzilla.redhat.com/show_bug.cgi?id=798328
+(cherry picked from commit 5273510e9f228a300ec6207d4502f1c6253aed5e)
+
+Conflicts:
+
+	src/core/transaction.c
+	src/job.c
+---
+ src/dbus-job.c |    2 +-
+ src/job.c      |   39 +++++++++++++++------------------------
+ src/job.h      |    2 +-
+ src/manager.c  |   13 ++++++-------
+ src/unit.c     |   12 ++++++------
+ 5 files changed, 29 insertions(+), 39 deletions(-)
+
+diff --git a/src/dbus-job.c b/src/dbus-job.c
+index ab6d610..6b5a33d 100644
+--- a/src/dbus-job.c
++++ b/src/dbus-job.c
+@@ -100,7 +100,7 @@ static DBusHandlerResult bus_job_message_dispatch(Job *j, DBusConnection *connec
+                 if (!(reply = dbus_message_new_method_return(message)))
+                         goto oom;
+ 
+-                job_finish_and_invalidate(j, JOB_CANCELED);
++                job_finish_and_invalidate(j, JOB_CANCELED, true);
+ 
+         } else {
+                 const BusBoundProperties bps[] = {
+diff --git a/src/job.c b/src/job.c
+index e57286f..ce4db75 100644
+--- a/src/job.c
++++ b/src/job.c
+@@ -464,13 +464,13 @@ int job_run_and_invalidate(Job *j) {
+ 
+         if ((j = manager_get_job(m, id))) {
+                 if (r == -EALREADY)
+-                        r = job_finish_and_invalidate(j, JOB_DONE);
++                        r = job_finish_and_invalidate(j, JOB_DONE, true);
+                 else if (r == -ENOEXEC)
+-                        r = job_finish_and_invalidate(j, JOB_SKIPPED);
++                        r = job_finish_and_invalidate(j, JOB_SKIPPED, true);
+                 else if (r == -EAGAIN)
+                         j->state = JOB_WAITING;
+                 else if (r < 0)
+-                        r = job_finish_and_invalidate(j, JOB_FAILED);
++                        r = job_finish_and_invalidate(j, JOB_FAILED, true);
+         }
+ 
+         return r;
+@@ -523,12 +523,11 @@ static void job_print_status_message(Unit *u, JobType t, JobResult result) {
+         }
+ }
+ 
+-int job_finish_and_invalidate(Job *j, JobResult result) {
++int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) {
+         Unit *u;
+         Unit *other;
+         JobType t;
+         Iterator i;
+-        bool recursed = false;
+ 
+         assert(j);
+         assert(j->installed);
+@@ -565,7 +564,7 @@ int job_finish_and_invalidate(Job *j, JobResult result) {
+         job_print_status_message(u, t, result);
+ 
+         /* Fail depending jobs on failure */
+-        if (result != JOB_DONE) {
++        if (result != JOB_DONE && recursive) {
+ 
+                 if (t == JOB_START ||
+                     t == JOB_VERIFY_ACTIVE ||
+@@ -575,29 +574,23 @@ int job_finish_and_invalidate(Job *j, JobResult result) {
+                                 if (other->job &&
+                                     (other->job->type == JOB_START ||
+                                      other->job->type == JOB_VERIFY_ACTIVE ||
+-                                     other->job->type == JOB_RELOAD_OR_START)) {
+-                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY);
+-                                        recursed = true;
+-                                }
++                                     other->job->type == JOB_RELOAD_OR_START))
++                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true);
+ 
+                         SET_FOREACH(other, u->dependencies[UNIT_BOUND_BY], i)
+                                 if (other->job &&
+                                     (other->job->type == JOB_START ||
+                                      other->job->type == JOB_VERIFY_ACTIVE ||
+-                                     other->job->type == JOB_RELOAD_OR_START)) {
+-                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY);
+-                                        recursed = true;
+-                                }
++                                     other->job->type == JOB_RELOAD_OR_START))
++                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true);
+ 
+                         SET_FOREACH(other, u->dependencies[UNIT_REQUIRED_BY_OVERRIDABLE], i)
+                                 if (other->job &&
+                                     !other->job->override &&
+                                     (other->job->type == JOB_START ||
+                                      other->job->type == JOB_VERIFY_ACTIVE ||
+-                                     other->job->type == JOB_RELOAD_OR_START)) {
+-                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY);
+-                                        recursed = true;
+-                                }
++                                     other->job->type == JOB_RELOAD_OR_START))
++                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true);
+ 
+                 } else if (t == JOB_STOP) {
+ 
+@@ -605,10 +598,8 @@ int job_finish_and_invalidate(Job *j, JobResult result) {
+                                 if (other->job &&
+                                     (other->job->type == JOB_START ||
+                                      other->job->type == JOB_VERIFY_ACTIVE ||
+-                                     other->job->type == JOB_RELOAD_OR_START)) {
+-                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY);
+-                                        recursed = true;
+-                                }
++                                     other->job->type == JOB_RELOAD_OR_START))
++                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true);
+                 }
+         }
+ 
+@@ -636,7 +627,7 @@ finish:
+ 
+         manager_check_finished(u->manager);
+ 
+-        return recursed;
++        return 0;
+ }
+ 
+ int job_start_timer(Job *j) {
+@@ -728,7 +719,7 @@ void job_timer_event(Job *j, uint64_t n_elapsed, Watch *w) {
+         assert(w == &j->timer_watch);
+ 
+         log_warning("Job %s/%s timed out.", j->unit->id, job_type_to_string(j->type));
+-        job_finish_and_invalidate(j, JOB_TIMEOUT);
++        job_finish_and_invalidate(j, JOB_TIMEOUT, true);
+ }
+ 
+ static const char* const job_state_table[_JOB_STATE_MAX] = {
+diff --git a/src/job.h b/src/job.h
+index 2121426..7c62a57 100644
+--- a/src/job.h
++++ b/src/job.h
+@@ -160,7 +160,7 @@ int job_start_timer(Job *j);
+ void job_timer_event(Job *j, uint64_t n_elapsed, Watch *w);
+ 
+ int job_run_and_invalidate(Job *j);
+-int job_finish_and_invalidate(Job *j, JobResult result);
++int job_finish_and_invalidate(Job *j, JobResult result, bool recursive);
+ 
+ char *job_dbus_path(Job *j);
+ 
+diff --git a/src/manager.c b/src/manager.c
+index 74bd740..16b0487 100644
+--- a/src/manager.c
++++ b/src/manager.c
+@@ -1222,18 +1222,16 @@ static int transaction_apply(Manager *m, JobMode mode) {
+ 
+                 /* When isolating first kill all installed jobs which
+                  * aren't part of the new transaction */
+-        rescan:
+                 HASHMAP_FOREACH(j, m->jobs, i) {
+                         assert(j->installed);
+ 
+                         if (hashmap_get(m->transaction_jobs, j->unit))
+                                 continue;
+ 
+-                        /* 'j' itself is safe to remove, but if other jobs
+-                           are invalidated recursively, our iterator may become
+-                           invalid and we need to start over. */
+-                        if (job_finish_and_invalidate(j, JOB_CANCELED) > 0)
+-                                goto rescan;
++                        /* Not invalidating recursively. Avoids triggering
++                         * OnFailure= actions of dependent jobs. Also avoids
++                         * invalidating our iterator. */
++                        job_finish_and_invalidate(j, JOB_CANCELED, false);
+                 }
+         }
+ 
+@@ -1906,7 +1904,8 @@ void manager_clear_jobs(Manager *m) {
+         transaction_abort(m);
+ 
+         while ((j = hashmap_first(m->jobs)))
+-                job_finish_and_invalidate(j, JOB_CANCELED);
++                /* No need to recurse. We're cancelling all jobs. */
++                job_finish_and_invalidate(j, JOB_CANCELED, false);
+ }
+ 
+ unsigned manager_dispatch_run_queue(Manager *m) {
+diff --git a/src/unit.c b/src/unit.c
+index 9e33701..3f6fd37 100644
+--- a/src/unit.c
++++ b/src/unit.c
+@@ -1223,12 +1223,12 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su
+                 case JOB_VERIFY_ACTIVE:
+ 
+                         if (UNIT_IS_ACTIVE_OR_RELOADING(ns))
+-                                job_finish_and_invalidate(u->job, JOB_DONE);
++                                job_finish_and_invalidate(u->job, JOB_DONE, true);
+                         else if (u->job->state == JOB_RUNNING && ns != UNIT_ACTIVATING) {
+                                 unexpected = true;
+ 
+                                 if (UNIT_IS_INACTIVE_OR_FAILED(ns))
+-                                        job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE);
++                                        job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true);
+                         }
+ 
+                         break;
+@@ -1238,12 +1238,12 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su
+ 
+                         if (u->job->state == JOB_RUNNING) {
+                                 if (ns == UNIT_ACTIVE)
+-                                        job_finish_and_invalidate(u->job, reload_success ? JOB_DONE : JOB_FAILED);
++                                        job_finish_and_invalidate(u->job, reload_success ? JOB_DONE : JOB_FAILED, true);
+                                 else if (ns != UNIT_ACTIVATING && ns != UNIT_RELOADING) {
+                                         unexpected = true;
+ 
+                                         if (UNIT_IS_INACTIVE_OR_FAILED(ns))
+-                                                job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE);
++                                                job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true);
+                                 }
+                         }
+ 
+@@ -1254,10 +1254,10 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su
+                 case JOB_TRY_RESTART:
+ 
+                         if (UNIT_IS_INACTIVE_OR_FAILED(ns))
+-                                job_finish_and_invalidate(u->job, JOB_DONE);
++                                job_finish_and_invalidate(u->job, JOB_DONE, true);
+                         else if (u->job->state == JOB_RUNNING && ns != UNIT_DEACTIVATING) {
+                                 unexpected = true;
+-                                job_finish_and_invalidate(u->job, JOB_FAILED);
++                                job_finish_and_invalidate(u->job, JOB_FAILED, true);
+                         }
+ 
+                         break;
diff --git a/0005-timedated-introduce-systemd-timedated-ntp.target-whi.patch b/0005-timedated-introduce-systemd-timedated-ntp.target-whi.patch
new file mode 100644
index 0000000..86236cf
--- /dev/null
+++ b/0005-timedated-introduce-systemd-timedated-ntp.target-whi.patch
@@ -0,0 +1,97 @@
+From 1e6b6f2348a0a280a29f4c48f0d66a6c53ddf183 Mon Sep 17 00:00:00 2001
+From: Lennart Poettering <lennart at poettering.net>
+Date: Wed, 25 Apr 2012 16:49:02 +0200
+Subject: [PATCH] timedated: introduce systemd-timedated-ntp.target which is
+ controlled by timedated's NTP setting
+
+We shouldn't hardcode the name of the NTP implementation in the
+timedated mechanism, especially since Fedora currently switched from NTP
+to chrony.
+
+This patch introduces a new target that is enabled/disabled instead of
+the actual NTP implementation. The various NTP implementations should
+then add .wants/ symlinks to their services and BindTo back to the
+target, so that their implementations are started/stopped jointly with
+the target.
+
+https://bugzilla.redhat.com/show_bug.cgi?id=815748
+(cherry picked from commit e2875c46936a16efc0f58f9e6e2570cdda8d6d98)
+
+Conflicts:
+
+	Makefile.am
+---
+ Makefile.am                        |    3 ++-
+ src/timedate/timedated.c           |    6 +++---
+ units/systemd-timedated-ntp.target |   17 +++++++++++++++++
+ 3 files changed, 22 insertions(+), 4 deletions(-)
+ create mode 100644 units/systemd-timedated-ntp.target
+
+diff --git a/Makefile.am b/Makefile.am
+index 079c118..f8a909d 100644
+--- a/Makefile.am
++++ b/Makefile.am
+@@ -304,7 +304,8 @@ dist_systemunit_DATA = \
+ 	units/quotaon.service \
+ 	units/systemd-ask-password-wall.path \
+ 	units/systemd-ask-password-console.path \
+-	units/syslog.target
++	units/syslog.target \
++	units/systemd-timedated-ntp.target
+ 
+ if HAVE_SYSV_COMPAT
+ dist_systemunit_DATA += \
+diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
+index 6a7d980..75a8536 100644
+--- a/src/timedate/timedated.c
++++ b/src/timedate/timedated.c
+@@ -304,7 +304,7 @@ static int write_data_local_rtc(void) {
+ 
+ static int read_ntp(DBusConnection *bus) {
+         DBusMessage *m = NULL, *reply = NULL;
+-        const char *name = "ntpd.service", *s;
++        const char *name = "systemd-timedated-ntp.target", *s;
+         DBusError error;
+         int r;
+ 
+@@ -374,7 +374,7 @@ finish:
+ 
+ static int start_ntp(DBusConnection *bus, DBusError *error) {
+         DBusMessage *m = NULL, *reply = NULL;
+-        const char *name = "ntpd.service", *mode = "replace";
++        const char *name = "systemd-timedated-ntp.target", *mode = "replace";
+         int r;
+ 
+         assert(bus);
+@@ -421,7 +421,7 @@ finish:
+ 
+ static int enable_ntp(DBusConnection *bus, DBusError *error) {
+         DBusMessage *m = NULL, *reply = NULL;
+-        const char * const names[] = { "ntpd.service", NULL };
++        const char * const names[] = { "systemd-timedated-ntp.target", NULL };
+         int r;
+         DBusMessageIter iter;
+         dbus_bool_t f = FALSE, t = TRUE;
+diff --git a/units/systemd-timedated-ntp.target b/units/systemd-timedated-ntp.target
+new file mode 100644
+index 0000000..1284248
+--- /dev/null
++++ b/units/systemd-timedated-ntp.target
+@@ -0,0 +1,17 @@
++#  This file is part of systemd.
++#
++#  systemd is free software; you can redistribute it and/or modify it
++#  under the terms of the GNU Lesser General Public License as published by
++#  the Free Software Foundation; either version 2.1 of the License, or
++#  (at your option) any later version.
++
++# This target is enabled/disabled via the timedated mechanism when the
++# user asks for it via the UI. NTP implementations should hook
++# themselves into this target via .wants/ symlinks, and then add
++# BindTo= on this target so that they are stopped when it goes away.
++
++[Unit]
++Description=Network Time Protocol
++
++[Install]
++WantedBy=multi-user.target
diff --git a/systemd.spec b/systemd.spec
index 008ce92..ebcf8ac 100644
--- a/systemd.spec
+++ b/systemd.spec
@@ -3,7 +3,7 @@
 Name:           systemd
 Url:            http://www.freedesktop.org/wiki/Software/systemd
 Version:        44
-Release:        6%{?gitcommit:.git%{gitcommit}}%{?dist}
+Release:        7%{?gitcommit:.git%{gitcommit}}%{?dist}
 License:        GPLv2+
 Group:          System Environment/Base
 Summary:        A System and Service Manager
@@ -61,6 +61,8 @@ Source4:        listen.conf
 Patch0001:      0001-util-never-follow-symlinks-in-rm_rf_children.patch
 Patch0002:      0002-journal-PAGE_SIZE-is-not-known-on-ppc-and-other-arch.patch
 Patch0003:      0003-service-place-control-command-in-subcgroup-control.patch
+Patch0004:      0004-transaction-cancel-jobs-non-recursively-on-isolate.patch
+Patch0005:      0005-timedated-introduce-systemd-timedated-ntp.target-whi.patch
 
 # For sysvinit tools
 Obsoletes:      SysVinit < 2.86-24, sysvinit < 2.86-24
@@ -430,6 +432,12 @@ mv /etc/systemd/system/default.target.save /etc/systemd/system/default.target >/
 %{_bindir}/systemd-analyze
 
 %changelog
+* Wed Apr 25 2012 Michal Schmidt <mschmidt at redhat.com> - 44-7
+- Fixes for two bugs from the F17Blocker tracker:
+  - Rescue shell on fsck errors (#798328)
+  - Add systemd-timedated-ntp.target to avoid harcoded ntpd.service
+    in timedated. Allows chrony to hook into it. (#815748)
+
 * Tue Apr 24 2012 Michal Schmidt <mschmidt at redhat.com> - 44-6
 - Revert most of the patches added in 44-5. F17 has 44-4 right now so let's
   try to minimize the risk of breakage before GA release. Apply only:


More information about the scm-commits mailing list