[systemd-devel] [systemd-commits] 4 commits - src/core test/TEST-03-JOBS
Zbigniew Jędrzejewski-Szmek
zbyszek at in.waw.pl
Wed Nov 26 18:42:16 PST 2014
On Wed, Nov 26, 2014 at 07:35:30AM -0800, Michal Schmidt wrote:
> src/core/job.c | 7 +++++--
> src/core/job.h | 14 ++++++++++----
> src/core/transaction.c | 2 +-
> test/TEST-03-JOBS/test-jobs.sh | 9 +++++++++
> 4 files changed, 25 insertions(+), 7 deletions(-)
Hi,
test-engine does not pass anymore. I didn't check exactly, but one
of those commits is the most likely culprit.
Requested transaction contradicts existing jobs: Resource deadlock avoided
Assertion 'manager_add_job(m, JOB_RESTART, g, JOB_FAIL, false, NULL, &j) == 0' failed at src/test/test-engine.c:95, function main(). Aborting.
[1] 17355 abort (core dumped) ./test-engine
Zbyszek
>
> New commits:
> commit e0312f4db08c7100bd00299614e87bedc759b366
> Author: Michal Schmidt <mschmidt at redhat.com>
> Date: Wed Nov 26 16:33:46 2014 +0100
>
> core: fix check for transaction destructiveness
>
> When checking if the transaction is destructive, we need to check if the
> previously installed job is a superset of the new job (and hence the new
> job will fold into the installed one without changing it), not the other
> way around.
>
> diff --git a/src/core/transaction.c b/src/core/transaction.c
> index bbaa6da..b992edd 100644
> --- a/src/core/transaction.c
> +++ b/src/core/transaction.c
> @@ -511,7 +511,7 @@ static int transaction_is_destructive(Transaction *tr, JobMode mode, sd_bus_erro
> assert(!j->transaction_next);
>
> if (j->unit->job && (mode == JOB_FAIL || j->unit->job->irreversible) &&
> - !job_type_is_superset(j->type, j->unit->job->type))
> + !job_type_is_superset(j->unit->job->type, j->type))
> return sd_bus_error_setf(e, BUS_ERROR_TRANSACTION_IS_DESTRUCTIVE,
> "Transaction is destructive.");
> }
>
> commit 61da906a744594002c2c967ecf6ec7899c7a9397
> Author: Michal Schmidt <mschmidt at redhat.com>
> Date: Wed Nov 26 16:33:45 2014 +0100
>
> core: drop now-redundant special-casing of JOB_NOP
>
> job_type_is_conflicting(X, JOB_NOP) correctly gives: false.
>
> job_type_allows_late_merge(JOB_NOP) && job_type_is_superset(X, JOB_NOP)
> correctly gives: true.
>
> diff --git a/src/core/job.c b/src/core/job.c
> index 1411603..9adc3fd 100644
> --- a/src/core/job.c
> +++ b/src/core/job.c
> @@ -160,12 +160,12 @@ Job* job_install(Job *j) {
> uj = *pj;
>
> if (uj) {
> - if (j->type != JOB_NOP && job_type_is_conflicting(uj->type, j->type))
> + if (job_type_is_conflicting(uj->type, j->type))
> job_finish_and_invalidate(uj, JOB_CANCELED, false);
> else {
> /* not conflicting, i.e. mergeable */
>
> - if (j->type == JOB_NOP || uj->state == JOB_WAITING ||
> + if (uj->state == JOB_WAITING ||
> (job_type_allows_late_merge(j->type) && job_type_is_superset(uj->type, j->type))) {
> job_merge_into_installed(uj, j);
> log_debug_unit(uj->unit->id,
>
> commit 7e803f5ecf689216d6fcd8a1d19a442f234bf28b
> Author: Michal Schmidt <mschmidt at redhat.com>
> Date: Wed Nov 26 16:33:43 2014 +0100
>
> core: fix assertion failure in checking a transaction with a JOB_NOP
>
> Several functions called from transaction_activate() need to correctly
> handle the case where a JOB_NOP job is being checked against a unit's
> pending job. The assumption that JOB_NOP never merges with other job
> types was correct, but since the job_type_is_*() functions are
> implemented using the merge lookup, they need to special-case JOB_NOP
> to avoid hitting assertion failures.
>
> diff --git a/src/core/job.c b/src/core/job.c
> index 51d1581..1411603 100644
> --- a/src/core/job.c
> +++ b/src/core/job.c
> @@ -352,6 +352,9 @@ bool job_type_is_redundant(JobType a, UnitActiveState b) {
> return
> b == UNIT_ACTIVATING;
>
> + case JOB_NOP:
> + return true;
> +
> default:
> assert_not_reached("Invalid job type");
> }
> diff --git a/src/core/job.h b/src/core/job.h
> index b7ebd8d..223ff9c 100644
> --- a/src/core/job.h
> +++ b/src/core/job.h
> @@ -49,9 +49,11 @@ enum JobType {
> _JOB_TYPE_MAX_MERGING,
>
> /* JOB_NOP can enter into a transaction, but as it won't pull in
> - * any dependencies, it won't have to merge with anything.
> - * job_install() avoids the problem of merging JOB_NOP too (it's
> - * special-cased, only merges with other JOB_NOPs). */
> + * any dependencies and it uses the special 'nop_job' slot in Unit,
> + * it won't have to merge with anything (except possibly into another
> + * JOB_NOP, previously installed). JOB_NOP is special-cased in
> + * job_type_is_*() functions so that the transaction can be
> + * activated. */
> JOB_NOP = _JOB_TYPE_MAX_MERGING, /* do nothing */
>
> _JOB_TYPE_MAX_IN_TRANSACTION,
> @@ -191,11 +193,15 @@ _pure_ static inline bool job_type_is_mergeable(JobType a, JobType b) {
> }
>
> _pure_ static inline bool job_type_is_conflicting(JobType a, JobType b) {
> - return !job_type_is_mergeable(a, b);
> + return a != JOB_NOP && b != JOB_NOP && !job_type_is_mergeable(a, b);
> }
>
> _pure_ static inline bool job_type_is_superset(JobType a, JobType b) {
> /* Checks whether operation a is a "superset" of b in its actions */
> + if (b == JOB_NOP)
> + return true;
> + if (a == JOB_NOP)
> + return false;
> return a == job_type_lookup_merge(a, b);
> }
>
>
> commit 06c1c4f98c0d0b4c93e58d75ed5a08d722ec4de3
> Author: Michal Schmidt <mschmidt at redhat.com>
> Date: Wed Nov 26 16:33:40 2014 +0100
>
> test: add test for crash when adding a JOB_NOP
>
> diff --git a/test/TEST-03-JOBS/test-jobs.sh b/test/TEST-03-JOBS/test-jobs.sh
> index 12b38af..28368b7 100755
> --- a/test/TEST-03-JOBS/test-jobs.sh
> +++ b/test/TEST-03-JOBS/test-jobs.sh
> @@ -21,6 +21,15 @@ ELAPSED=$(($END_SEC-$START_SEC))
> systemctl list-jobs > /root/list-jobs.txt
> grep 'sleep\.service.*running' /root/list-jobs.txt || exit 1
> grep 'hello\.service' /root/list-jobs.txt && exit 1
> +systemctl stop sleep.service hello-after-sleep.target || exit 1
> +
> +# Test for a crash when enqueueing a JOB_NOP when other job already exists
> +systemctl start --no-block hello-after-sleep.target || exit 1
> +# hello.service should still be waiting, so these try-restarts will collapse
> +# into NOPs.
> +systemctl try-restart --fail hello.service || exit 1
> +systemctl try-restart hello.service || exit 1
> +systemctl stop hello.service sleep.service hello-after-sleep.target || exit 1
>
> # TODO: add more job queueing/merging tests here.
>
>
> _______________________________________________
> systemd-commits mailing list
> systemd-commits at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-commits
More information about the systemd-devel
mailing list