[systemd] A couple of fixes from upstream

Michal Schmidt michich at fedoraproject.org
Wed Oct 19 10:32:51 UTC 2011


commit bbee01f832d9830ba274277f23a9b772aaaa180f
Author: Michal Schmidt <mschmidt at redhat.com>
Date:   Wed Oct 19 12:20:43 2011 +0200

    A couple of fixes from upstream
    
    Fix a regression in bash-completion reported in Bodhi.
    Fix a crash in isolating.
    Resolves: #717325

 0002-manager-fix-a-crash-in-isolating.patch        |  117 ++++++++++++++++++++
 ...l-completion-always-invoke-with-no-legend.patch |   76 +++++++++++++
 systemd.spec                                       |   14 ++-
 3 files changed, 206 insertions(+), 1 deletions(-)
---
diff --git a/0002-manager-fix-a-crash-in-isolating.patch b/0002-manager-fix-a-crash-in-isolating.patch
new file mode 100644
index 0000000..b9697c7
--- /dev/null
+++ b/0002-manager-fix-a-crash-in-isolating.patch
@@ -0,0 +1,117 @@
+From 563ba9ea6e60774086555998b957edf923e24b46 Mon Sep 17 00:00:00 2001
+From: Michal Schmidt <mschmidt at redhat.com>
+Date: Mon, 17 Oct 2011 11:12:12 +0200
+Subject: [PATCH 2/5] manager: fix a crash in isolating
+
+HASHMAP_FOREACH is safe against the removal of the current entry, but
+not against the removal of other entries. job_finish_and_invalidate()
+can recursively remove other entries.
+
+It triggered an assertion failure:
+  Assertion 'j->installed' failed at src/manager.c:1218, function
+  transaction_apply(). Aborting.
+
+Fix the crash by iterating from the beginning when there is a
+possibility that the iterator could be invalid.
+
+It is O(n^2) in the worst case, but that's better than a crash.
+
+https://bugzilla.redhat.com/show_bug.cgi?id=717325
+---
+ src/job.c     |   19 ++++++++++++++-----
+ src/manager.c |    7 ++++++-
+ 2 files changed, 20 insertions(+), 6 deletions(-)
+
+diff --git a/src/job.c b/src/job.c
+index 5c0913b..20971da 100644
+--- a/src/job.c
++++ b/src/job.c
+@@ -527,6 +527,7 @@ int job_finish_and_invalidate(Job *j, JobResult result) {
+         Unit *other;
+         JobType t;
+         Iterator i;
++        bool recursed = false;
+ 
+         assert(j);
+         assert(j->installed);
+@@ -573,23 +574,29 @@ int job_finish_and_invalidate(Job *j, JobResult result) {
+                                 if (other->meta.job &&
+                                     (other->meta.job->type == JOB_START ||
+                                      other->meta.job->type == JOB_VERIFY_ACTIVE ||
+-                                     other->meta.job->type == JOB_RELOAD_OR_START))
++                                     other->meta.job->type == JOB_RELOAD_OR_START)) {
+                                         job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY);
++                                        recursed = true;
++                                }
+ 
+                         SET_FOREACH(other, u->meta.dependencies[UNIT_BOUND_BY], i)
+                                 if (other->meta.job &&
+                                     (other->meta.job->type == JOB_START ||
+                                      other->meta.job->type == JOB_VERIFY_ACTIVE ||
+-                                     other->meta.job->type == JOB_RELOAD_OR_START))
++                                     other->meta.job->type == JOB_RELOAD_OR_START)) {
+                                         job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY);
++                                        recursed = true;
++                                }
+ 
+                         SET_FOREACH(other, u->meta.dependencies[UNIT_REQUIRED_BY_OVERRIDABLE], i)
+                                 if (other->meta.job &&
+                                     !other->meta.job->override &&
+                                     (other->meta.job->type == JOB_START ||
+                                      other->meta.job->type == JOB_VERIFY_ACTIVE ||
+-                                     other->meta.job->type == JOB_RELOAD_OR_START))
++                                     other->meta.job->type == JOB_RELOAD_OR_START)) {
+                                         job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY);
++                                        recursed = true;
++                                }
+ 
+                 } else if (t == JOB_STOP) {
+ 
+@@ -597,8 +604,10 @@ int job_finish_and_invalidate(Job *j, JobResult result) {
+                                 if (other->meta.job &&
+                                     (other->meta.job->type == JOB_START ||
+                                      other->meta.job->type == JOB_VERIFY_ACTIVE ||
+-                                     other->meta.job->type == JOB_RELOAD_OR_START))
++                                     other->meta.job->type == JOB_RELOAD_OR_START)) {
+                                         job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY);
++                                        recursed = true;
++                                }
+                 }
+         }
+ 
+@@ -626,7 +635,7 @@ finish:
+ 
+         manager_check_finished(u->meta.manager);
+ 
+-        return 0;
++        return recursed;
+ }
+ 
+ int job_start_timer(Job *j) {
+diff --git a/src/manager.c b/src/manager.c
+index e626347..6d20258 100644
+--- a/src/manager.c
++++ b/src/manager.c
+@@ -1214,13 +1214,18 @@ 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;
+ 
+-                        job_finish_and_invalidate(j, JOB_CANCELED);
++                        /* '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;
+                 }
+         }
+ 
+-- 
+1.7.4.4
+
diff --git a/0005-systemctl-completion-always-invoke-with-no-legend.patch b/0005-systemctl-completion-always-invoke-with-no-legend.patch
new file mode 100644
index 0000000..a085836
--- /dev/null
+++ b/0005-systemctl-completion-always-invoke-with-no-legend.patch
@@ -0,0 +1,76 @@
+From 74eeab044e506a39786f484b160d9f64d48ad243 Mon Sep 17 00:00:00 2001
+From: Dave Reisner <d at falconindy.com>
+Date: Tue, 11 Oct 2011 20:56:53 -0400
+Subject: [PATCH 5/5] systemctl-completion: always invoke with --no-legend
+
+In the case of completion for the 'restart' verb, passing the invalid
+unit name (the colums header) causes completion to cease functioning
+entirely, with the error:
+
+  Failed to issue method call: Unit name UNIT is not valid.
+
+This adds a small wrapper function for systemctl which can have common
+options added to it.
+---
+ src/systemctl-bash-completion.sh |   20 ++++++++++++--------
+ 1 files changed, 12 insertions(+), 8 deletions(-)
+
+diff --git a/src/systemctl-bash-completion.sh b/src/systemctl-bash-completion.sh
+index 6369a6c..6ebb792 100644
+--- a/src/systemctl-bash-completion.sh
++++ b/src/systemctl-bash-completion.sh
+@@ -15,6 +15,10 @@
+ # You should have received a copy of the GNU General Public License
+ # along with systemd; If not, see <http://www.gnu.org/licenses/>.
+ 
++__systemctl() {
++        systemctl --no-legend "$@"
++}
++
+ __contains_word () {
+         local word=$1; shift
+         for w in $*; do [[ $w = $word ]] && return 0; done
+@@ -24,7 +28,7 @@ __contains_word () {
+ __filter_units_by_property () {
+         local property=$1 value=$2 ; shift ; shift
+         local -a units=( $* )
+-        local -a props=( $(systemctl show --property "$property" -- ${units[*]} | grep -v ^$) )
++        local -a props=( $(__systemctl show --property "$property" -- ${units[*]} | grep -v ^$) )
+         for ((i=0; $i < ${#units[*]}; i++)); do
+                 if [[ "${props[i]}" = "$property=$value" ]]; then
+                         echo "${units[i]}"
+@@ -32,10 +36,10 @@ __filter_units_by_property () {
+         done
+ }
+ 
+-__get_all_units      () { systemctl list-units --full --all | awk '                 {print $1}' ; }
+-__get_active_units   () { systemctl list-units --full       | awk '                 {print $1}' ; }
+-__get_inactive_units () { systemctl list-units --full --all | awk '$3 == "inactive" {print $1}' ; }
+-__get_failed_units   () { systemctl list-units --full       | awk '$3 == "failed"   {print $1}' ; }
++__get_all_units      () { __systemctl list-units --full --all | awk '                 {print $1}' ; }
++__get_active_units   () { __systemctl list-units --full       | awk '                 {print $1}' ; }
++__get_inactive_units () { __systemctl list-units --full --all | awk '$3 == "inactive" {print $1}' ; }
++__get_failed_units   () { __systemctl list-units --full       | awk '$3 == "failed"   {print $1}' ; }
+ 
+ _systemctl () {
+         local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]}
+@@ -134,13 +138,13 @@ _systemctl () {
+                 comps=''
+ 
+         elif __contains_word "$verb" ${VERBS[JOBS]}; then
+-                comps=$( systemctl list-jobs | awk '{print $1}' )
++                comps=$( __systemctl list-jobs | awk '{print $1}' )
+ 
+         elif __contains_word "$verb" ${VERBS[SNAPSHOTS]}; then
+-                comps=$( systemctl list-units --type snapshot --full --all | awk '{print $1}' )
++                comps=$( __systemctl list-units --type snapshot --full --all | awk '{print $1}' )
+ 
+         elif __contains_word "$verb" ${VERBS[ENVS]}; then
+-                comps=$( systemctl show-environment | sed 's_\([^=]\+=\).*_\1_' )
++                comps=$( __systemctl show-environment | sed 's_\([^=]\+=\).*_\1_' )
+                 compopt -o nospace
+         fi
+ 
+-- 
+1.7.4.4
+
diff --git a/systemd.spec b/systemd.spec
index 46e0c03..046ec0a 100644
--- a/systemd.spec
+++ b/systemd.spec
@@ -2,7 +2,7 @@ Name:           systemd
 Url:            http://www.freedesktop.org/wiki/Software/systemd
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 Version:        37
-Release:        1%{?dist}
+Release:        2%{?dist}
 License:        GPLv2+
 Group:          System Environment/Base
 Summary:        A System and Service Manager
@@ -45,7 +45,11 @@ Source2:        systemd-sysv-convert
 # Stop-gap, just to ensure things work out-of-the-box for this driver.
 Source3:        udlfb.conf
 # We revert this one for https://bugzilla.redhat.com/show_bug.cgi?id=741078
+# Must keep until https://bugzilla.redhat.com/show_bug.cgi?id=741115 is fixed.
 Patch0:         0001-unit-fix-complementing-of-requirement-deps-with-Afte.patch
+# some post-v37 patches from upstream:
+Patch1:         0002-manager-fix-a-crash-in-isolating.patch
+Patch2:         0005-systemctl-completion-always-invoke-with-no-legend.patch
 
 # For sysvinit tools
 Obsoletes:      SysVinit < 2.86-24, sysvinit < 2.86-24
@@ -109,6 +113,8 @@ SysV compatibility tools for systemd
 %prep
 %setup -q
 %patch0 -p1 -R
+%patch1 -p1
+%patch2 -p1
 
 %build
 %configure --with-rootdir= --with-distro=fedora --with-rootlibdir=/%{_lib}
@@ -357,6 +363,12 @@ fi
 %{_bindir}/systemd-sysv-convert
 
 %changelog
+* Wed Oct 19 2011 Michal Schmidt <mschmidt at redhat.com> - 37-2
+- A couple of fixes from upstream:
+- Fix a regression in bash-completion reported in Bodhi.
+- Fix a crash in isolating.
+- Resolves: #717325
+
 * Tue Oct 11 2011 Lennart Poettering <lpoetter at redhat.com> - 37-1
 - New upstream release
 - Resolves: #744726, #718464, #713567, #713707, #736756


More information about the scm-commits mailing list