From 984794baf4af59da7f2496ecab33f2596d6619ff Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 15 Nov 2016 19:18:36 +0100 Subject: shared: split out code for adding multiple names to sd_bus_track object Let's introduce a new call bus_track_add_name_many() that adds a string list to a tracking object. --- src/core/dbus.c | 12 +----------- src/shared/bus-util.c | 19 +++++++++++++++++++ src/shared/bus-util.h | 2 ++ 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/core/dbus.c b/src/core/dbus.c index 070974fe66..ed4697c37e 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -1185,7 +1185,6 @@ void bus_track_serialize(sd_bus_track *t, FILE *f, const char *prefix) { } int bus_track_coldplug(Manager *m, sd_bus_track **t, bool recursive, char **l) { - char **i; int r = 0; assert(m); @@ -1207,16 +1206,7 @@ int bus_track_coldplug(Manager *m, sd_bus_track **t, bool recursive, char **l) { if (r < 0) return r; - r = 0; - STRV_FOREACH(i, l) { - int k; - - k = sd_bus_track_add_name(*t, *i); - if (k < 0) - r = k; - } - - return r; + return bus_track_add_name_many(*t, l); } int bus_verify_manage_units_async(Manager *m, sd_bus_message *call, sd_bus_error *error) { diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index e7b1b1cb20..6aebe18fc0 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -1583,3 +1583,22 @@ int bus_property_get_rlimit( return sd_bus_message_append(reply, "t", u); } + +int bus_track_add_name_many(sd_bus_track *t, char **l) { + int r = 0; + char **i; + + assert(t); + + /* Continues adding after failure, and returns the first failure. */ + + STRV_FOREACH(i, l) { + int k; + + k = sd_bus_track_add_name(t, *i); + if (k < 0 && r >= 0) + r = k; + } + + return r; +} diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index 934e0b5b77..af5f133912 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -159,3 +159,5 @@ int bus_path_encode_unique(sd_bus *b, const char *prefix, const char *sender_id, int bus_path_decode_unique(const char *path, const char *prefix, char **ret_sender, char **ret_external); int bus_property_get_rlimit(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error); + +int bus_track_add_name_many(sd_bus_track *t, char **l); -- cgit v1.2.3-54-g00ecf From 0a23a627296841a299e95118313eddb4d2e07e04 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 15 Nov 2016 19:19:57 +0100 Subject: core: a few small coding style/modernization updates for job.c --- src/core/job.c | 12 +++++------- src/core/job.h | 4 ++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/core/job.c b/src/core/job.c index ac6910a906..7b492564ab 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -267,7 +267,8 @@ JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool * this means the 'anchor' job (i.e. the one the user * explicitly asked for) is the requester. */ - if (!(l = new0(JobDependency, 1))) + l = new0(JobDependency, 1); + if (!l) return NULL; l->subject = subject; @@ -457,9 +458,7 @@ static bool job_is_runnable(Job *j) { if (j->type == JOB_NOP) return true; - if (j->type == JOB_START || - j->type == JOB_VERIFY_ACTIVE || - j->type == JOB_RELOAD) { + if (IN_SET(j->type, JOB_START, JOB_VERIFY_ACTIVE, JOB_RELOAD)) { /* Immediate result is that the job is or might be * started. In this case let's wait for the @@ -476,8 +475,7 @@ static bool job_is_runnable(Job *j) { SET_FOREACH(other, j->unit->dependencies[UNIT_BEFORE], i) if (other->job && - (other->job->type == JOB_STOP || - other->job->type == JOB_RESTART)) + IN_SET(other->job->type, JOB_STOP, JOB_RESTART)) return false; /* This means that for a service a and a service b where b @@ -1205,7 +1203,7 @@ int job_get_timeout(Job *j, usec_t *timeout) { static const char* const job_state_table[_JOB_STATE_MAX] = { [JOB_WAITING] = "waiting", - [JOB_RUNNING] = "running" + [JOB_RUNNING] = "running", }; DEFINE_STRING_TABLE_LOOKUP(job_state, JobState); diff --git a/src/core/job.h b/src/core/job.h index 85368f0d30..02560c41c5 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -122,8 +122,8 @@ struct JobDependency { LIST_FIELDS(JobDependency, subject); LIST_FIELDS(JobDependency, object); - bool matters; - bool conflicts; + bool matters:1; + bool conflicts:1; }; struct Job { -- cgit v1.2.3-54-g00ecf From a2d72e265aaf0cceb0eb16d7f76730054e7ff439 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 15 Nov 2016 19:23:29 +0100 Subject: core: drop n_in_gc_queue field of Manager structure We count the units in the GC queue with this, but actually never make use of it, hence drop it. --- src/core/manager.c | 2 -- src/core/manager.h | 1 - src/core/unit.c | 6 +----- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 52174eac07..dc81af9492 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1015,8 +1015,6 @@ static unsigned manager_dispatch_gc_queue(Manager *m) { } } - m->n_in_gc_queue = 0; - return n; } diff --git a/src/core/manager.h b/src/core/manager.h index 35172fdba9..aa3f95e8e0 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -229,7 +229,6 @@ struct Manager { int pin_cgroupfs_fd; int gc_marker; - unsigned n_in_gc_queue; /* Flags */ ManagerExitCode exit_code:5; diff --git a/src/core/unit.c b/src/core/unit.c index da9bb58a52..df60a5bf04 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -391,8 +391,6 @@ void unit_add_to_gc_queue(Unit *u) { LIST_PREPEND(gc_queue, u->manager->gc_queue, u); u->in_gc_queue = true; - - u->manager->n_in_gc_queue++; } void unit_add_to_dbus_queue(Unit *u) { @@ -570,10 +568,8 @@ void unit_free(Unit *u) { if (u->in_cleanup_queue) LIST_REMOVE(cleanup_queue, u->manager->cleanup_queue, u); - if (u->in_gc_queue) { + if (u->in_gc_queue) LIST_REMOVE(gc_queue, u->manager->gc_queue, u); - u->manager->n_in_gc_queue--; - } if (u->in_cgroup_queue) LIST_REMOVE(cgroup_queue, u->manager->cgroup_queue, u); -- cgit v1.2.3-54-g00ecf From 1a465207ab0a0b6756ab0d9305102d9159955a14 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 15 Nov 2016 19:29:50 +0100 Subject: core: rename "clients" field of Job structure to "bus_track" Let's make semantics of this field more similar to the same functionality in the Unit object, in particular as we add new functionality to it later on. --- src/core/dbus-job.c | 6 +++--- src/core/dbus-unit.c | 6 +++--- src/core/dbus.c | 4 ++-- src/core/job.c | 6 +++--- src/core/job.h | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c index ccf7453d47..e8c69ed3e4 100644 --- a/src/core/dbus-job.c +++ b/src/core/dbus-job.c @@ -65,7 +65,7 @@ int bus_job_method_cancel(sd_bus_message *message, void *userdata, sd_bus_error return r; /* Access is granted to the job owner */ - if (!sd_bus_track_contains(j->clients, sd_bus_message_get_sender(message))) { + if (!sd_bus_track_contains(j->bus_track, sd_bus_message_get_sender(message))) { /* And for everybody else consult PolicyKit */ r = bus_verify_manage_units_async(j->unit->manager, message, error); @@ -143,7 +143,7 @@ void bus_job_send_change_signal(Job *j) { j->in_dbus_queue = false; } - r = bus_foreach_bus(j->manager, j->clients, j->sent_dbus_new_signal ? send_changed_signal : send_new_signal, j); + r = bus_foreach_bus(j->manager, j->bus_track, j->sent_dbus_new_signal ? send_changed_signal : send_new_signal, j); if (r < 0) log_debug_errno(r, "Failed to send job change signal for %u: %m", j->id); @@ -187,7 +187,7 @@ void bus_job_send_removed_signal(Job *j) { if (!j->sent_dbus_new_signal) bus_job_send_change_signal(j); - r = bus_foreach_bus(j->manager, j->clients, send_removed_signal, j); + r = bus_foreach_bus(j->manager, j->bus_track, send_removed_signal, j); if (r < 0) log_debug_errno(r, "Failed to send job remove signal for %u: %m", j->id); } diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index b6cb6e1350..90cf5651ca 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1224,13 +1224,13 @@ int bus_unit_queue_job( return r; if (sd_bus_message_get_bus(message) == u->manager->api_bus) { - if (!j->clients) { - r = sd_bus_track_new(sd_bus_message_get_bus(message), &j->clients, NULL, NULL); + if (!j->bus_track) { + r = sd_bus_track_new(sd_bus_message_get_bus(message), &j->bus_track, NULL, NULL); if (r < 0) return r; } - r = sd_bus_track_add_sender(j->clients, message); + r = sd_bus_track_add_sender(j->bus_track, message); if (r < 0) return r; } diff --git a/src/core/dbus.c b/src/core/dbus.c index ed4697c37e..07ab21f199 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -1054,8 +1054,8 @@ static void destroy_bus(Manager *m, sd_bus **bus) { m->subscribed = sd_bus_track_unref(m->subscribed); HASHMAP_FOREACH(j, m->jobs, i) - if (j->clients && sd_bus_track_get_bus(j->clients) == *bus) - j->clients = sd_bus_track_unref(j->clients); + if (j->bus_track && sd_bus_track_get_bus(j->bus_track) == *bus) + j->bus_track = sd_bus_track_unref(j->bus_track); /* Get rid of queued message on this bus */ if (m->queued_message && sd_bus_message_get_bus(m->queued_message) == *bus) diff --git a/src/core/job.c b/src/core/job.c index 7b492564ab..3a20da6d06 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -92,7 +92,7 @@ void job_free(Job *j) { sd_event_source_unref(j->timer_event_source); - sd_bus_track_unref(j->clients); + sd_bus_track_unref(j->bus_track); strv_free(j->deserialized_clients); free(j); @@ -1010,7 +1010,7 @@ int job_serialize(Job *j, FILE *f) { if (j->begin_usec > 0) fprintf(f, "job-begin="USEC_FMT"\n", j->begin_usec); - bus_track_serialize(j->clients, f, "subscribed"); + bus_track_serialize(j->bus_track, f, "subscribed"); /* End marker */ fputc('\n', f); @@ -1121,7 +1121,7 @@ int job_coldplug(Job *j) { /* After deserialization is complete and the bus connection * set up again, let's start watching our subscribers again */ - (void) bus_track_coldplug(j->manager, &j->clients, false, j->deserialized_clients); + (void) bus_track_coldplug(j->manager, &j->bus_track, false, j->deserialized_clients); j->deserialized_clients = strv_free(j->deserialized_clients); if (j->state == JOB_WAITING) diff --git a/src/core/job.h b/src/core/job.h index 02560c41c5..ccfc7def4d 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -156,7 +156,7 @@ struct Job { * * There can be more than one client, because of job merging. */ - sd_bus_track *clients; + sd_bus_track *bus_track; char **deserialized_clients; JobResult result; -- cgit v1.2.3-54-g00ecf From c5a97ed132b400ad82f7939d55fe1027a2b13f6e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 15 Nov 2016 19:32:50 +0100 Subject: core: GC redundant device jobs from the run queue In contrast to all other unit types device units when queued just track external state, they cannot effect state changes on their own. Hence unless a client or other job waits for them there's no reason to keep them in the job queue. This adds a concept of GC'ing jobs of this type as soon as no client or other job waits for them anymore. To ensure this works correctly we need to track which clients actually reference a job (i.e. which ones enqueued it). Unfortunately that's pretty nasty to do for direct connections, as sd_bus_track doesn't work for them. For now, work around this, by simply remembering in a boolean that a job was requested by a direct connection, and reset it when we notice the direct connection is gone. This means the GC logic works fine, except that jobs are not immediately removed when direct connections disconnect. In the longer term, a rework of the bus logic should fix this properly. For now this should be good enough, as GC works for fine all cases except this one, and thus is a clear improvement over the previous behaviour. Fixes: #1921 --- src/core/dbus-job.c | 68 +++++++++++++++++++++++++++ src/core/dbus-job.h | 3 ++ src/core/dbus-unit.c | 25 ++++------ src/core/device.c | 2 + src/core/job.c | 114 +++++++++++++++++++++++++++++++++++++++++++-- src/core/job.h | 7 +++ src/core/manager.c | 41 +++++++++++++--- src/core/manager.h | 5 +- src/core/unit.c | 4 +- src/core/unit.h | 3 ++ src/shared/bus-unit-util.c | 4 +- 11 files changed, 244 insertions(+), 32 deletions(-) diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c index e8c69ed3e4..7888c163f1 100644 --- a/src/core/dbus-job.c +++ b/src/core/dbus-job.c @@ -191,3 +191,71 @@ void bus_job_send_removed_signal(Job *j) { if (r < 0) log_debug_errno(r, "Failed to send job remove signal for %u: %m", j->id); } + +static int bus_job_track_handler(sd_bus_track *t, void *userdata) { + Job *j = userdata; + + assert(t); + assert(j); + + j->bus_track = sd_bus_track_unref(j->bus_track); /* make sure we aren't called again */ + + /* Last client dropped off the bus, maybe we should GC this now? */ + job_add_to_gc_queue(j); + return 0; +} + +static int bus_job_allocate_bus_track(Job *j) { + int r; + + assert(j); + + if (j->bus_track) + return 0; + + r = sd_bus_track_new(j->unit->manager->api_bus, &j->bus_track, bus_job_track_handler, j); + if (r < 0) + return r; + + return 0; +} + +int bus_job_coldplug_bus_track(Job *j) { + int r = 0; + + assert(j); + + if (strv_isempty(j->deserialized_clients)) + goto finish; + + if (!j->manager->api_bus) + goto finish; + + r = bus_job_allocate_bus_track(j); + if (r < 0) + goto finish; + + r = bus_track_add_name_many(j->bus_track, j->deserialized_clients); + +finish: + j->deserialized_clients = strv_free(j->deserialized_clients); + return r; +} + +int bus_job_track_sender(Job *j, sd_bus_message *m) { + int r; + + assert(j); + assert(m); + + if (sd_bus_message_get_bus(m) != j->unit->manager->api_bus) { + j->ref_by_private_bus = true; + return 0; + } + + r = bus_job_allocate_bus_track(j); + if (r < 0) + return r; + + return sd_bus_track_add_sender(j->bus_track, m); +} diff --git a/src/core/dbus-job.h b/src/core/dbus-job.h index 024d06719e..f9148895be 100644 --- a/src/core/dbus-job.h +++ b/src/core/dbus-job.h @@ -29,3 +29,6 @@ int bus_job_method_cancel(sd_bus_message *message, void *job, sd_bus_error *erro void bus_job_send_change_signal(Job *j); void bus_job_send_removed_signal(Job *j); + +int bus_job_coldplug_bus_track(Job *j); +int bus_job_track_sender(Job *j, sd_bus_message *m); diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 90cf5651ca..2adc1d9288 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -22,6 +22,7 @@ #include "alloc-util.h" #include "bus-common-errors.h" #include "cgroup-util.h" +#include "dbus-job.h" #include "dbus-unit.h" #include "dbus.h" #include "fd-util.h" @@ -1223,17 +1224,9 @@ int bus_unit_queue_job( if (r < 0) return r; - if (sd_bus_message_get_bus(message) == u->manager->api_bus) { - if (!j->bus_track) { - r = sd_bus_track_new(sd_bus_message_get_bus(message), &j->bus_track, NULL, NULL); - if (r < 0) - return r; - } - - r = sd_bus_track_add_sender(j->bus_track, message); - if (r < 0) - return r; - } + r = bus_job_track_sender(j, message); + if (r < 0) + return r; path = job_dbus_path(j); if (!path) @@ -1507,7 +1500,7 @@ int bus_unit_check_load_state(Unit *u, sd_bus_error *error) { return sd_bus_error_set_errnof(error, u->load_error, "Unit %s is not loaded properly: %m.", u->id); } -static int bus_track_handler(sd_bus_track *t, void *userdata) { +static int bus_unit_track_handler(sd_bus_track *t, void *userdata) { Unit *u = userdata; assert(t); @@ -1519,7 +1512,7 @@ static int bus_track_handler(sd_bus_track *t, void *userdata) { return 0; } -static int allocate_bus_track(Unit *u) { +static int bus_unit_allocate_bus_track(Unit *u) { int r; assert(u); @@ -1527,7 +1520,7 @@ static int allocate_bus_track(Unit *u) { if (u->bus_track) return 0; - r = sd_bus_track_new(u->manager->api_bus, &u->bus_track, bus_track_handler, u); + r = sd_bus_track_new(u->manager->api_bus, &u->bus_track, bus_unit_track_handler, u); if (r < 0) return r; @@ -1545,7 +1538,7 @@ int bus_unit_track_add_name(Unit *u, const char *name) { assert(u); - r = allocate_bus_track(u); + r = bus_unit_allocate_bus_track(u); if (r < 0) return r; @@ -1557,7 +1550,7 @@ int bus_unit_track_add_sender(Unit *u, sd_bus_message *m) { assert(u); - r = allocate_bus_track(u); + r = bus_unit_allocate_bus_track(u); if (r < 0) return r; diff --git a/src/core/device.c b/src/core/device.c index c572a6737c..074e93ffe2 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -831,6 +831,8 @@ const UnitVTable device_vtable = { "Device\0" "Install\0", + .gc_jobs = true, + .init = device_init, .done = device_done, .load = unit_load_fragment_and_dropin_optional, diff --git a/src/core/job.c b/src/core/job.c index 3a20da6d06..d6e71d68ef 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -90,6 +90,9 @@ void job_free(Job *j) { if (j->in_dbus_queue) LIST_REMOVE(dbus_queue, j->manager->dbus_job_queue, j); + if (j->in_gc_queue) + LIST_REMOVE(gc_queue, j->manager->gc_job_queue, j); + sd_event_source_unref(j->timer_event_source); sd_bus_track_unref(j->bus_track); @@ -226,6 +229,9 @@ Job* job_install(Job *j) { log_unit_debug(j->unit, "Installed new job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); + + job_add_to_gc_queue(j); + return j; } @@ -639,6 +645,7 @@ _pure_ static const char *job_get_status_message_format(Unit *u, JobType t, JobR [JOB_DEPENDENCY] = "Dependency failed for %s.", [JOB_ASSERT] = "Assertion failed for %s.", [JOB_UNSUPPORTED] = "Starting of %s not supported.", + [JOB_COLLECTED] = "Unecessary job for %s was removed.", }; static const char *const generic_finished_stop_job[_JOB_RESULT_MAX] = { [JOB_DONE] = "Stopped %s.", @@ -698,6 +705,7 @@ static void job_print_status_message(Unit *u, JobType t, JobResult result) { [JOB_SKIPPED] = { ANSI_HIGHLIGHT, " INFO " }, [JOB_ASSERT] = { ANSI_HIGHLIGHT_YELLOW, "ASSERT" }, [JOB_UNSUPPORTED] = { ANSI_HIGHLIGHT_YELLOW, "UNSUPP" }, + [JOB_COLLECTED] = { ANSI_HIGHLIGHT, " INFO " }, }; const char *format; @@ -749,6 +757,7 @@ static void job_log_status_message(Unit *u, JobType t, JobResult result) { [JOB_INVALID] = LOG_INFO, [JOB_ASSERT] = LOG_WARNING, [JOB_UNSUPPORTED] = LOG_WARNING, + [JOB_COLLECTED] = LOG_INFO, }; assert(u); @@ -860,6 +869,7 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive, bool alr job_set_state(j, JOB_WAITING); job_add_to_run_queue(j); + job_add_to_gc_queue(j); goto finish; } @@ -903,11 +913,15 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive, bool alr finish: /* Try to start the next jobs that can be started */ SET_FOREACH(other, u->dependencies[UNIT_AFTER], i) - if (other->job) + if (other->job) { job_add_to_run_queue(other->job); + job_add_to_gc_queue(other->job); + } SET_FOREACH(other, u->dependencies[UNIT_BEFORE], i) - if (other->job) + if (other->job) { job_add_to_run_queue(other->job); + job_add_to_gc_queue(other->job); + } manager_check_finished(u->manager); @@ -1121,12 +1135,14 @@ int job_coldplug(Job *j) { /* After deserialization is complete and the bus connection * set up again, let's start watching our subscribers again */ - (void) bus_track_coldplug(j->manager, &j->bus_track, false, j->deserialized_clients); - j->deserialized_clients = strv_free(j->deserialized_clients); + (void) bus_job_coldplug_bus_track(j); if (j->state == JOB_WAITING) job_add_to_run_queue(j); + /* Maybe due to new dependencies we don't actually need this job anymore? */ + job_add_to_gc_queue(j); + if (j->begin_usec == 0 || j->unit->job_timeout == USEC_INFINITY) return 0; @@ -1201,6 +1217,95 @@ int job_get_timeout(Job *j, usec_t *timeout) { return 1; } +bool job_check_gc(Job *j) { + Unit *other; + Iterator i; + + assert(j); + + /* Checks whether this job should be GC'ed away. We only do this for jobs of units that have no effect on their + * own and just track external state. For now the only unit type that qualifies for this are .device units. */ + + if (!UNIT_VTABLE(j->unit)->gc_jobs) + return true; + + if (sd_bus_track_count(j->bus_track) > 0) + return true; + + /* FIXME: So this is a bit ugly: for now we don't properly track references made via private bus connections + * (because it's nasty, as sd_bus_track doesn't apply to it). We simply remember that the job was once + * referenced by one, and reset this whenever we notice that no private bus connections are around. This means + * the GC is a bit too conservative when it comes to jobs created by private bus connections. */ + if (j->ref_by_private_bus) { + if (set_isempty(j->unit->manager->private_buses)) + j->ref_by_private_bus = false; + else + return true; + } + + if (j->type == JOB_NOP) + return true; + + /* If a job is ordered after ours, and is to be started, then it needs to wait for us, regardless if we stop or + * start, hence let's not GC in that case. */ + SET_FOREACH(other, j->unit->dependencies[UNIT_BEFORE], i) { + if (!other->job) + continue; + + if (other->job->ignore_order) + continue; + + if (IN_SET(other->job->type, JOB_START, JOB_VERIFY_ACTIVE, JOB_RELOAD)) + return true; + } + + /* If we are going down, but something else is orederd After= us, then it needs to wait for us */ + if (IN_SET(j->type, JOB_STOP, JOB_RESTART)) { + + SET_FOREACH(other, j->unit->dependencies[UNIT_AFTER], i) { + if (!other->job) + continue; + + if (other->job->ignore_order) + continue; + + return true; + } + } + + /* The logic above is kinda the inverse of the job_is_runnable() logic. Specifically, if the job "we" is + * ordered before the job "other": + * + * we start + other start → stay + * we start + other stop → gc + * we stop + other start → stay + * we stop + other stop → gc + * + * "we" are ordered after "other": + * + * we start + other start → gc + * we start + other stop → gc + * we stop + other start → stay + * we stop + other stop → stay + * + */ + + return false; +} + +void job_add_to_gc_queue(Job *j) { + assert(j); + + if (j->in_gc_queue) + return; + + if (job_check_gc(j)) + return; + + LIST_PREPEND(gc_queue, j->unit->manager->gc_job_queue, j); + j->in_gc_queue = true; +} + static const char* const job_state_table[_JOB_STATE_MAX] = { [JOB_WAITING] = "waiting", [JOB_RUNNING] = "running", @@ -1244,6 +1349,7 @@ static const char* const job_result_table[_JOB_RESULT_MAX] = { [JOB_INVALID] = "invalid", [JOB_ASSERT] = "assert", [JOB_UNSUPPORTED] = "unsupported", + [JOB_COLLECTED] = "collected", }; DEFINE_STRING_TABLE_LOOKUP(job_result, JobResult); diff --git a/src/core/job.h b/src/core/job.h index ccfc7def4d..6fdec9f226 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -107,6 +107,7 @@ enum JobResult { JOB_INVALID, /* JOB_RELOAD of inactive unit */ JOB_ASSERT, /* Couldn't start a unit, because an assert didn't hold */ JOB_UNSUPPORTED, /* Couldn't start a unit, because the unit type is not supported on the system */ + JOB_COLLECTED, /* Job was garbage collected, since nothing needed it anymore */ _JOB_RESULT_MAX, _JOB_RESULT_INVALID = -1 }; @@ -133,6 +134,7 @@ struct Job { LIST_FIELDS(Job, transaction); LIST_FIELDS(Job, run_queue); LIST_FIELDS(Job, dbus_queue); + LIST_FIELDS(Job, gc_queue); LIST_HEAD(JobDependency, subject_list); LIST_HEAD(JobDependency, object_list); @@ -168,6 +170,8 @@ struct Job { bool sent_dbus_new_signal:1; bool ignore_order:1; bool irreversible:1; + bool in_gc_queue:1; + bool ref_by_private_bus:1; }; Job* job_new(Unit *unit, JobType type); @@ -227,6 +231,9 @@ void job_shutdown_magic(Job *j); int job_get_timeout(Job *j, usec_t *timeout) _pure_; +bool job_check_gc(Job *j); +void job_add_to_gc_queue(Job *j); + const char* job_type_to_string(JobType t) _const_; JobType job_type_from_string(const char *s) _pure_; diff --git a/src/core/manager.c b/src/core/manager.c index dc81af9492..31770eef3a 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -981,10 +981,9 @@ good: unit_gc_mark_good(u, gc_marker); } -static unsigned manager_dispatch_gc_queue(Manager *m) { +static unsigned manager_dispatch_gc_unit_queue(Manager *m) { + unsigned n = 0, gc_marker; Unit *u; - unsigned n = 0; - unsigned gc_marker; assert(m); @@ -996,12 +995,12 @@ static unsigned manager_dispatch_gc_queue(Manager *m) { gc_marker = m->gc_marker; - while ((u = m->gc_queue)) { + while ((u = m->gc_unit_queue)) { assert(u->in_gc_queue); unit_gc_sweep(u, gc_marker); - LIST_REMOVE(gc_queue, m->gc_queue, u); + LIST_REMOVE(gc_queue, m->gc_unit_queue, u); u->in_gc_queue = false; n++; @@ -1018,6 +1017,30 @@ static unsigned manager_dispatch_gc_queue(Manager *m) { return n; } +static unsigned manager_dispatch_gc_job_queue(Manager *m) { + unsigned n = 0; + Job *j; + + assert(m); + + while ((j = m->gc_job_queue)) { + assert(j->in_gc_queue); + + LIST_REMOVE(gc_queue, m->gc_job_queue, j); + j->in_gc_queue = false; + + n++; + + if (job_check_gc(j)) + continue; + + log_unit_debug(j->unit, "Collecting job."); + (void) job_finish_and_invalidate(j, JOB_COLLECTED, false, false); + } + + return n; +} + static void manager_clear_jobs_and_units(Manager *m) { Unit *u; @@ -1033,7 +1056,8 @@ static void manager_clear_jobs_and_units(Manager *m) { assert(!m->dbus_unit_queue); assert(!m->dbus_job_queue); assert(!m->cleanup_queue); - assert(!m->gc_queue); + assert(!m->gc_unit_queue); + assert(!m->gc_job_queue); assert(hashmap_isempty(m->jobs)); assert(hashmap_isempty(m->units)); @@ -2226,7 +2250,10 @@ int manager_loop(Manager *m) { if (manager_dispatch_load_queue(m) > 0) continue; - if (manager_dispatch_gc_queue(m) > 0) + if (manager_dispatch_gc_job_queue(m) > 0) + continue; + + if (manager_dispatch_gc_unit_queue(m) > 0) continue; if (manager_dispatch_cleanup_queue(m) > 0) diff --git a/src/core/manager.h b/src/core/manager.h index aa3f95e8e0..d54ca54107 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -104,8 +104,9 @@ struct Manager { /* Units to remove */ LIST_HEAD(Unit, cleanup_queue); - /* Units to check when doing GC */ - LIST_HEAD(Unit, gc_queue); + /* Units and jobs to check when doing GC */ + LIST_HEAD(Unit, gc_unit_queue); + LIST_HEAD(Job, gc_job_queue); /* Units that should be realized */ LIST_HEAD(Unit, cgroup_queue); diff --git a/src/core/unit.c b/src/core/unit.c index df60a5bf04..fbb21e4985 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -389,7 +389,7 @@ void unit_add_to_gc_queue(Unit *u) { if (unit_check_gc(u)) return; - LIST_PREPEND(gc_queue, u->manager->gc_queue, u); + LIST_PREPEND(gc_queue, u->manager->gc_unit_queue, u); u->in_gc_queue = true; } @@ -569,7 +569,7 @@ void unit_free(Unit *u) { LIST_REMOVE(cleanup_queue, u->manager->cleanup_queue, u); if (u->in_gc_queue) - LIST_REMOVE(gc_queue, u->manager->gc_queue, u); + LIST_REMOVE(gc_queue, u->manager->gc_unit_queue, u); if (u->in_cgroup_queue) LIST_REMOVE(cgroup_queue, u->manager->cgroup_queue, u); diff --git a/src/core/unit.h b/src/core/unit.h index 991543664b..6d6885b487 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -441,6 +441,9 @@ struct UnitVTable { /* True if transient units of this type are OK */ bool can_transient:1; + + /* True if queued jobs of this type should be GC'ed if no other job needs them anymore */ + bool gc_jobs:1; }; extern const UnitVTable * const unit_vtable[_UNIT_TYPE_MAX]; diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 4f66497f3a..a7702602eb 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -838,6 +838,8 @@ static int check_wait_response(BusWaitForJobs *d, bool quiet, const char* const* log_error("Assertion failed on job for %s.", strna(d->name)); else if (streq(d->result, "unsupported")) log_error("Operation on or unit type of %s not supported on this system.", strna(d->name)); + else if (streq(d->result, "collected")) + log_error("Queued job for %s was garbage collected.", strna(d->name)); else if (!streq(d->result, "done") && !streq(d->result, "skipped")) { if (d->name) { int q; @@ -853,7 +855,7 @@ static int check_wait_response(BusWaitForJobs *d, bool quiet, const char* const* } } - if (streq(d->result, "canceled")) + if (STR_IN_SET(d->result, "canceled", "collected")) r = -ECANCELED; else if (streq(d->result, "timeout")) r = -ETIME; -- cgit v1.2.3-54-g00ecf From d4015567adf77d4b992dc6fae84445e4e09c236e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 16 Nov 2016 14:31:03 +0100 Subject: systemctl: add env var to force connection to system manager via the bus Sometimes it is useful for debugging purposes to force systemctl to connect to PID 1 via the bus instead of direct connection, even if the direct connection is possible. --- src/systemctl/systemctl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 49a97ff268..de7e19725c 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -204,6 +204,9 @@ static int acquire_bus(BusFocus focus, sd_bus **ret) { if (arg_transport != BUS_TRANSPORT_LOCAL) focus = BUS_FULL; + if (getenv_bool("SYSTEMCTL_FORCE_BUS") > 0) + focus = BUS_FULL; + if (!busses[focus]) { bool user; -- cgit v1.2.3-54-g00ecf From 289188ca7dcbb2a1193d06349b73e5d7f5473755 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 16 Nov 2016 14:44:04 +0100 Subject: system-run: add support for configuring unit dependencies with --property= Support on the server side has already been in place for quite some time, let's also add support on the client side for this. --- src/shared/bus-unit-util.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index a7702602eb..388b391342 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -62,6 +62,7 @@ int bus_parse_unit_info(sd_bus_message *message, UnitInfo *u) { int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignment) { const char *eq, *field; + UnitDependency dep; int r, rl; assert(m); @@ -572,7 +573,9 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen flags = (~flags) & NAMESPACE_FLAGS_ALL; r = sd_bus_message_append(m, "v", "t", flags); - } else { + } else if ((dep = unit_dependency_from_string(field)) >= 0) + r = sd_bus_message_append(m, "v", "as", 1, eq); + else { log_error("Unknown assignment %s.", assignment); return -EINVAL; } -- cgit v1.2.3-54-g00ecf From 15ea79f85c3a9b174b6ba14cc9a0f5f482ca1553 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 16 Nov 2016 16:07:32 +0100 Subject: core: add bus calls for determining jobs waiting for other jobs This should make it easier to debug job deadlocks. --- src/core/dbus-job.c | 52 ++++++++++++++ src/core/dbus-job.h | 1 + src/core/dbus-manager.c | 22 ++++++ src/core/job.c | 127 +++++++++++++++++++++++++++++++++ src/core/job.h | 3 + src/core/org.freedesktop.systemd1.conf | 16 +++++ 6 files changed, 221 insertions(+) diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c index 7888c163f1..087a08dc0d 100644 --- a/src/core/dbus-job.c +++ b/src/core/dbus-job.c @@ -80,9 +80,61 @@ int bus_job_method_cancel(sd_bus_message *message, void *userdata, sd_bus_error return sd_bus_reply_method_return(message, NULL); } +int bus_job_method_get_waiting_jobs(sd_bus_message *message, void *userdata, sd_bus_error *error) { + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + _cleanup_free_ Job **list = NULL; + Job *j = userdata; + int r, i, n; + + if (strstr(sd_bus_message_get_member(message), "After")) + n = job_get_after(j, &list); + else + n = job_get_before(j, &list); + if (n < 0) + return n; + + r = sd_bus_message_new_method_return(message, &reply); + if (r < 0) + return r; + + r = sd_bus_message_open_container(reply, 'a', "(usssoo)"); + if (r < 0) + return r; + + for (i = 0; i < n; i ++) { + _cleanup_free_ char *unit_path = NULL, *job_path = NULL; + + job_path = job_dbus_path(list[i]); + if (!job_path) + return -ENOMEM; + + unit_path = unit_dbus_path(list[i]->unit); + if (!unit_path) + return -ENOMEM; + + r = sd_bus_message_append(reply, "(usssoo)", + list[i]->id, + list[i]->unit->id, + job_type_to_string(list[i]->type), + job_state_to_string(list[i]->state), + job_path, + unit_path); + if (r < 0) + return r; + } + + r = sd_bus_message_close_container(reply); + if (r < 0) + return r; + + return sd_bus_send(NULL, reply, NULL); +} + const sd_bus_vtable bus_job_vtable[] = { SD_BUS_VTABLE_START(0), SD_BUS_METHOD("Cancel", NULL, NULL, bus_job_method_cancel, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("GetAfter", NULL, "a(usssoo)", bus_job_method_get_waiting_jobs, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("GetBefore", NULL, "a(usssoo)", bus_job_method_get_waiting_jobs, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_PROPERTY("Id", "u", NULL, offsetof(Job, id), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Unit", "(so)", property_get_unit, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("JobType", "s", property_get_type, offsetof(Job, type), SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/core/dbus-job.h b/src/core/dbus-job.h index f9148895be..a4366a0720 100644 --- a/src/core/dbus-job.h +++ b/src/core/dbus-job.h @@ -26,6 +26,7 @@ extern const sd_bus_vtable bus_job_vtable[]; int bus_job_method_cancel(sd_bus_message *message, void *job, sd_bus_error *error); +int bus_job_method_get_waiting_jobs(sd_bus_message *message, void *userdata, sd_bus_error *error); void bus_job_send_change_signal(Job *j); void bus_job_send_removed_signal(Job *j); diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 5a7922a249..9af49dd1bc 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -2285,6 +2285,26 @@ static int method_get_unit_file_links(sd_bus_message *message, void *userdata, s return sd_bus_send(NULL, reply, NULL); } +static int method_get_job_waiting(sd_bus_message *message, void *userdata, sd_bus_error *error) { + Manager *m = userdata; + uint32_t id; + Job *j; + int r; + + assert(message); + assert(m); + + r = sd_bus_message_read(message, "u", &id); + if (r < 0) + return r; + + j = manager_get_job(m, id); + if (!j) + return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_JOB, "Job %u does not exist.", (unsigned) id); + + return bus_job_method_get_waiting_jobs(message, j, error); +} + const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_VTABLE_START(0), @@ -2390,6 +2410,8 @@ const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_METHOD("StartTransientUnit", "ssa(sv)a(sa(sv))", "o", method_start_transient_unit, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("GetUnitProcesses", "s", "a(sus)", method_get_unit_processes, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("GetJob", "u", "o", method_get_job, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("GetJobAfter", "u", "a(usssoo)", method_get_job_waiting, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("GetJobBefore", "u", "a(usssoo)", method_get_job_waiting, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("CancelJob", "u", NULL, method_cancel_job, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ClearJobs", NULL, NULL, method_clear_jobs, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ResetFailed", NULL, NULL, method_reset_failed, SD_BUS_VTABLE_UNPRIVILEGED), diff --git a/src/core/job.c b/src/core/job.c index d6e71d68ef..2ba4c78096 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -1306,6 +1306,133 @@ void job_add_to_gc_queue(Job *j) { j->in_gc_queue = true; } +static int job_compare(const void *a, const void *b) { + Job *x = *(Job**) a, *y = *(Job**) b; + + if (x->id < y->id) + return -1; + if (x->id > y->id) + return 1; + + return 0; +} + +static size_t sort_job_list(Job **list, size_t n) { + Job *previous = NULL; + size_t a, b; + + /* Order by numeric IDs */ + qsort_safe(list, n, sizeof(Job*), job_compare); + + /* Filter out duplicates */ + for (a = 0, b = 0; a < n; a++) { + + if (previous == list[a]) + continue; + + previous = list[b++] = list[a]; + } + + return b; +} + +int job_get_before(Job *j, Job*** ret) { + _cleanup_free_ Job** list = NULL; + size_t n = 0, n_allocated = 0; + Unit *other = NULL; + Iterator i; + + /* Returns a list of all pending jobs that need to finish before this job may be started. */ + + assert(j); + assert(ret); + + if (j->ignore_order) { + *ret = NULL; + return 0; + } + + if (IN_SET(j->type, JOB_START, JOB_VERIFY_ACTIVE, JOB_RELOAD)) { + + SET_FOREACH(other, j->unit->dependencies[UNIT_AFTER], i) { + if (!other->job) + continue; + + if (!GREEDY_REALLOC(list, n_allocated, n+1)) + return -ENOMEM; + list[n++] = other->job; + } + } + + SET_FOREACH(other, j->unit->dependencies[UNIT_BEFORE], i) { + if (!other->job) + continue; + + if (!IN_SET(other->job->type, JOB_STOP, JOB_RESTART)) + continue; + + if (!GREEDY_REALLOC(list, n_allocated, n+1)) + return -ENOMEM; + list[n++] = other->job; + } + + n = sort_job_list(list, n); + + *ret = list; + list = NULL; + + return (int) n; +} + +int job_get_after(Job *j, Job*** ret) { + _cleanup_free_ Job** list = NULL; + size_t n = 0, n_allocated = 0; + Unit *other = NULL; + Iterator i; + + assert(j); + assert(ret); + + /* Returns a list of all pending jobs that are waiting for this job to finish. */ + + SET_FOREACH(other, j->unit->dependencies[UNIT_BEFORE], i) { + if (!other->job) + continue; + + if (other->job->ignore_order) + continue; + + if (!IN_SET(other->job->type, JOB_START, JOB_VERIFY_ACTIVE, JOB_RELOAD)) + continue; + + if (!GREEDY_REALLOC(list, n_allocated, n+1)) + return -ENOMEM; + list[n++] = other->job; + } + + if (IN_SET(j->type, JOB_STOP, JOB_RESTART)) { + + SET_FOREACH(other, j->unit->dependencies[UNIT_AFTER], i) { + if (!other->job) + continue; + + if (other->job->ignore_order) + continue; + + if (!GREEDY_REALLOC(list, n_allocated, n+1)) + return -ENOMEM; + list[n++] = other->job; + } + } + + n = sort_job_list(list, n); + + *ret = list; + list = NULL; + + return (int) n; +} + static const char* const job_state_table[_JOB_STATE_MAX] = { [JOB_WAITING] = "waiting", [JOB_RUNNING] = "running", diff --git a/src/core/job.h b/src/core/job.h index 6fdec9f226..bea743f462 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -234,6 +234,9 @@ int job_get_timeout(Job *j, usec_t *timeout) _pure_; bool job_check_gc(Job *j); void job_add_to_gc_queue(Job *j); +int job_get_before(Job *j, Job*** ret); +int job_get_after(Job *j, Job*** ret); + const char* job_type_to_string(JobType t) _const_; JobType job_type_from_string(const char *s) _pure_; diff --git a/src/core/org.freedesktop.systemd1.conf b/src/core/org.freedesktop.systemd1.conf index a61677e645..e824a2233c 100644 --- a/src/core/org.freedesktop.systemd1.conf +++ b/src/core/org.freedesktop.systemd1.conf @@ -64,6 +64,14 @@ send_interface="org.freedesktop.systemd1.Manager" send_member="GetJob"/> + + + + @@ -250,6 +258,14 @@ send_interface="org.freedesktop.systemd1.Job" send_member="Cancel"/> + + + + -- cgit v1.2.3-54-g00ecf From 82948f6c8e2fe03aa3859757b92bcadfb66a6a9c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 16 Nov 2016 16:44:05 +0100 Subject: systemctl: show waiting jobs when "systemctl list-jobs --after/--before" is called Let's expose the new bus functions we added in the previous commit in systemctl. --- man/systemctl.xml | 12 ++++++++++ src/systemctl/systemctl.c | 56 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/man/systemctl.xml b/man/systemctl.xml index dfa00e0c03..08c3a268bd 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -209,6 +209,10 @@ RequiresMountsFor=). Both explicitly and implicitly introduced dependencies are shown with list-dependencies. + + When passed to the list-jobs command, for each printed job show which other jobs are + waiting for it. May be combined with to show both the jobs waiting for each job as + well as all jobs each job is waiting for. @@ -220,6 +224,10 @@ units that are ordered after the specified unit. In other words, recursively list units following the Before= dependency. + + When passed to the list-jobs command, for each printed job show which other jobs it + is waiting for. May be combined with to show both the jobs waiting for each job as + well as all jobs each job is waiting for. @@ -1388,6 +1396,10 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service List jobs that are in progress. If one or more PATTERNs are specified, only jobs for units matching one of them are shown. + + When combined with or the list is augmented with + information on which other job each job is waiting for, and which other jobs are waiting for it, see + above. diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index de7e19725c..db836639b5 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -173,6 +173,8 @@ static OutputMode arg_output = OUTPUT_SHORT; static bool arg_plain = false; static bool arg_firmware_setup = false; static bool arg_now = false; +static bool arg_jobs_before = false; +static bool arg_jobs_after = false; static int daemon_reload(int argc, char *argv[], void* userdata); static int trivial_method(int argc, char *argv[], void *userdata); @@ -2195,12 +2197,55 @@ finish: return r; } +static void output_waiting_jobs(sd_bus *bus, uint32_t id, const char *method, const char *prefix) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + const char *name, *type, *state, *job_path, *unit_path; + uint32_t other_id; + int r; + + assert(bus); + + r = sd_bus_call_method( + bus, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + method, + &error, + &reply, + "u", id); + if (r < 0) { + log_debug_errno(r, "Failed to get waiting jobs for job %" PRIu32, id); + return; + } + + r = sd_bus_message_enter_container(reply, 'a', "(usssoo)"); + if (r < 0) { + bus_log_parse_error(r); + return; + } + + while ((r = sd_bus_message_read(reply, "(usssoo)", &other_id, &name, &type, &state, &job_path, &unit_path)) > 0) + printf("%s%u (%s/%s)\n", prefix, other_id, name, type); + if (r < 0) { + bus_log_parse_error(r); + return; + } + + r = sd_bus_message_exit_container(reply); + if (r < 0) { + bus_log_parse_error(r); + return; + } +} + struct job_info { uint32_t id; const char *name, *type, *state; }; -static void output_jobs_list(const struct job_info* jobs, unsigned n, bool skipped) { +static void output_jobs_list(sd_bus *bus, const struct job_info* jobs, unsigned n, bool skipped) { unsigned id_len, unit_len, type_len, state_len; const struct job_info *j; const char *on, *off; @@ -2262,6 +2307,11 @@ static void output_jobs_list(const struct job_info* jobs, unsigned n, bool skipp on, unit_len, e ? e : j->name, off, type_len, j->type, on, state_len, j->state, off); + + if (arg_jobs_after) + output_waiting_jobs(bus, j->id, "GetJobAfter", "\tA job waits for this job: "); + if (arg_jobs_before) + output_waiting_jobs(bus, j->id, "GetJobBefore", "\tThis job waits for a job: "); } if (!arg_no_legend) { @@ -2330,7 +2380,7 @@ static int list_jobs(int argc, char *argv[], void *userdata) { pager_open(arg_no_pager, false); - output_jobs_list(jobs, c, skipped); + output_jobs_list(bus, jobs, c, skipped); return 0; } @@ -7305,10 +7355,12 @@ static int systemctl_parse_argv(int argc, char *argv[]) { case ARG_AFTER: arg_dependency = DEPENDENCY_AFTER; + arg_jobs_after = true; break; case ARG_BEFORE: arg_dependency = DEPENDENCY_BEFORE; + arg_jobs_before = true; break; case ARG_SHOW_TYPES: -- cgit v1.2.3-54-g00ecf From 7d992a6ede8034a36699c25c19f03e95476eedac Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 16 Nov 2016 15:03:08 +0100 Subject: update TODO --- TODO | 2 -- 1 file changed, 2 deletions(-) diff --git a/TODO b/TODO index baaac9407f..887ddcd081 100644 --- a/TODO +++ b/TODO @@ -560,7 +560,6 @@ Features: - man: maybe sort directives in man pages, and take sections from --help and apply them to man too * systemctl: - - systemctl list-jobs - show dependencies - add systemctl switch to dump transaction without executing it - Add a verbose mode to "systemctl start" and friends that explains what is being done or not done - "systemctl disable" on a static unit prints no message and does @@ -735,7 +734,6 @@ Features: - maybe introduce WantsMountsFor=? Usecase: http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html - recreate systemd's D-Bus private socket file on SIGUSR2 - - GC unreferenced jobs (such as .device jobs) - move PAM code into its own binary - when we automatically restart a service, ensure we restart its rdeps, too. - hide PAM options in fragment parser when compile time disabled -- cgit v1.2.3-54-g00ecf