[systemd-commits] 2 commits - src/core src/systemctl
Michal Schmidt
michich at kemper.freedesktop.org
Mon May 21 04:04:14 PDT 2012
src/core/dbus-unit.c | 23 ++---
src/core/manager.c | 12 +-
src/core/manager.h | 2
src/systemctl/systemctl.c | 210 +++++++++++++++++-----------------------------
4 files changed, 98 insertions(+), 149 deletions(-)
New commits:
commit a223b325b409b325f4c2c2e11268596edd842631
Author: Michal Schmidt <mschmidt at redhat.com>
Date: Mon May 21 12:54:43 2012 +0200
systemctl: drop useless DBus calls from 'systemctl show foo.service'
systemctl called LoadUnit, GetUnit, GetAll in this order to get the properties.
It is useless to load units explicitly, because it won't ensure anything. The
unit may be freed immediately by the garbage collector.
It is unnecessary to call GetUnit, because systemctl can easily translate the
unit name to DBus path by itself.
GetAll will load the unit if necessary.
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 0034c55..92c79d0 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -2955,18 +2955,70 @@ finish:
return r;
}
-static int show(DBusConnection *bus, char **args) {
+static int show_one_by_pid(const char *verb, DBusConnection *bus, uint32_t pid, bool *new_line) {
DBusMessage *m = NULL, *reply = NULL;
- int r, ret = 0;
+ const char *path = NULL;
DBusError error;
+ int r;
+
+ dbus_error_init(&error);
+
+ m = dbus_message_new_method_call(
+ "org.freedesktop.systemd1",
+ "/org/freedesktop/systemd1",
+ "org.freedesktop.systemd1.Manager",
+ "GetUnitByPID");
+ if (!m) {
+ log_error("Could not allocate message.");
+ r = -ENOMEM;
+ goto finish;
+ }
+
+ if (!dbus_message_append_args(m,
+ DBUS_TYPE_UINT32, &pid,
+ DBUS_TYPE_INVALID)) {
+ log_error("Could not append arguments to message.");
+ r = -ENOMEM;
+ goto finish;
+ }
+
+ reply = dbus_connection_send_with_reply_and_block(bus, m, -1, &error);
+ if (!reply) {
+ log_error("Failed to issue method call: %s", bus_error_message(&error));
+ r = -EIO;
+ goto finish;
+ }
+
+ if (!dbus_message_get_args(reply, &error,
+ DBUS_TYPE_OBJECT_PATH, &path,
+ DBUS_TYPE_INVALID)) {
+ log_error("Failed to parse reply: %s", bus_error_message(&error));
+ r = -EIO;
+ goto finish;
+ }
+
+ r = show_one(verb, bus, path, false, new_line);
+
+finish:
+ if (m)
+ dbus_message_unref(m);
+
+ if (reply)
+ dbus_message_unref(reply);
+
+ dbus_error_free(&error);
+
+ return r;
+}
+
+static int show(DBusConnection *bus, char **args) {
+ int r, ret = 0;
bool show_properties, new_line = false;
char **name;
assert(bus);
assert(args);
- dbus_error_init(&error);
-
show_properties = !streq(args[0], "status");
if (show_properties)
@@ -2976,157 +3028,55 @@ static int show(DBusConnection *bus, char **args) {
/* If not argument is specified inspect the manager
* itself */
- ret = show_one(args[0], bus, "/org/freedesktop/systemd1", show_properties, &new_line);
- goto finish;
+ return show_one(args[0], bus, "/org/freedesktop/systemd1", show_properties, &new_line);
}
STRV_FOREACH(name, args+1) {
- const char *path = NULL;
uint32_t id;
if (safe_atou32(*name, &id) < 0) {
/* Interpret as unit name */
- if (!(m = dbus_message_new_method_call(
- "org.freedesktop.systemd1",
- "/org/freedesktop/systemd1",
- "org.freedesktop.systemd1.Manager",
- "LoadUnit"))) {
- log_error("Could not allocate message.");
- ret = -ENOMEM;
- goto finish;
- }
-
- if (!dbus_message_append_args(m,
- DBUS_TYPE_STRING, name,
- DBUS_TYPE_INVALID)) {
- log_error("Could not append arguments to message.");
- ret = -ENOMEM;
- goto finish;
- }
-
- if (!(reply = dbus_connection_send_with_reply_and_block(bus, m, -1, &error))) {
-
- if (!dbus_error_has_name(&error, DBUS_ERROR_ACCESS_DENIED)) {
- log_error("Failed to issue method call: %s", bus_error_message(&error));
- ret = -EIO;
- goto finish;
- }
-
- dbus_error_free(&error);
-
- dbus_message_unref(m);
- if (!(m = dbus_message_new_method_call(
- "org.freedesktop.systemd1",
- "/org/freedesktop/systemd1",
- "org.freedesktop.systemd1.Manager",
- "GetUnit"))) {
- log_error("Could not allocate message.");
- ret = -ENOMEM;
- goto finish;
- }
+ char *e, *p;
+ e = bus_path_escape(*name);
+ if (!e)
+ return -ENOMEM;
+ p = strappend("/org/freedesktop/systemd1/unit/", e);
+ free(e);
+ if (!p)
+ return -ENOMEM;
- if (!dbus_message_append_args(m,
- DBUS_TYPE_STRING, name,
- DBUS_TYPE_INVALID)) {
- log_error("Could not append arguments to message.");
- ret = -ENOMEM;
- goto finish;
- }
+ r = show_one(args[0], bus, p, show_properties, &new_line);
+ free(p);
- if (!(reply = dbus_connection_send_with_reply_and_block(bus, m, -1, &error))) {
- log_error("Failed to issue method call: %s", bus_error_message(&error));
-
- if (dbus_error_has_name(&error, BUS_ERROR_NO_SUCH_UNIT))
- ret = 4; /* According to LSB: "program or service status is unknown" */
- else
- ret = -EIO;
- goto finish;
- }
- }
+ if (r != 0)
+ ret = r;
} else if (show_properties) {
/* Interpret as job id */
- if (!(m = dbus_message_new_method_call(
- "org.freedesktop.systemd1",
- "/org/freedesktop/systemd1",
- "org.freedesktop.systemd1.Manager",
- "GetJob"))) {
- log_error("Could not allocate message.");
- ret = -ENOMEM;
- goto finish;
- }
+ char *p;
+ if (asprintf(&p, "/org/freedesktop/systemd1/job/%u", id) < 0)
+ return -ENOMEM;
- if (!dbus_message_append_args(m,
- DBUS_TYPE_UINT32, &id,
- DBUS_TYPE_INVALID)) {
- log_error("Could not append arguments to message.");
- ret = -ENOMEM;
- goto finish;
- }
+ r = show_one(args[0], bus, p, show_properties, &new_line);
+ free(p);
+
+ if (r != 0)
+ ret = r;
- if (!(reply = dbus_connection_send_with_reply_and_block(bus, m, -1, &error))) {
- log_error("Failed to issue method call: %s", bus_error_message(&error));
- ret = -EIO;
- goto finish;
- }
} else {
/* Interpret as PID */
- if (!(m = dbus_message_new_method_call(
- "org.freedesktop.systemd1",
- "/org/freedesktop/systemd1",
- "org.freedesktop.systemd1.Manager",
- "GetUnitByPID"))) {
- log_error("Could not allocate message.");
- ret = -ENOMEM;
- goto finish;
- }
-
- if (!dbus_message_append_args(m,
- DBUS_TYPE_UINT32, &id,
- DBUS_TYPE_INVALID)) {
- log_error("Could not append arguments to message.");
- ret = -ENOMEM;
- goto finish;
- }
-
- if (!(reply = dbus_connection_send_with_reply_and_block(bus, m, -1, &error))) {
- log_error("Failed to issue method call: %s", bus_error_message(&error));
- ret = -EIO;
- goto finish;
- }
+ r = show_one_by_pid(args[0], bus, id, &new_line);
+ if (r != 0)
+ ret = r;
}
-
- if (!dbus_message_get_args(reply, &error,
- DBUS_TYPE_OBJECT_PATH, &path,
- DBUS_TYPE_INVALID)) {
- log_error("Failed to parse reply: %s", bus_error_message(&error));
- ret = -EIO;
- goto finish;
- }
-
- if ((r = show_one(args[0], bus, path, show_properties, &new_line)) != 0)
- ret = r;
-
- dbus_message_unref(m);
- dbus_message_unref(reply);
- m = reply = NULL;
}
-finish:
- if (m)
- dbus_message_unref(m);
-
- if (reply)
- dbus_message_unref(reply);
-
- dbus_error_free(&error);
-
return ret;
}
commit 80fbf05e75b75b7dd342ec844275efae90c479ec
Author: Michal Schmidt <mschmidt at redhat.com>
Date: Mon May 21 12:54:34 2012 +0200
dbus-unit: always load the unit before handling a message for it
We need to be able to show the properties even of inactive units.
systemctl loads the unit before getting its properties, but this is racy
as the garbage collector may kick in right after the loading.
Fix it by always loading the unit before handling a message for it.
https://bugzilla.redhat.com/show_bug.cgi?id=814966#c6
diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index b8d6616..834fbd7 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -558,12 +558,15 @@ static DBusHandlerResult bus_unit_message_handler(DBusConnection *connection, DB
Manager *m = data;
Unit *u;
int r;
- DBusMessage *reply;
+ DBusMessage *reply = NULL;
+ DBusError error;
assert(connection);
assert(message);
assert(m);
+ dbus_error_init(&error);
+
if (streq(dbus_message_get_path(message), "/org/freedesktop/systemd1/unit")) {
/* Be nice to gdbus and return introspection data for our mid-level paths */
@@ -638,20 +641,12 @@ static DBusHandlerResult bus_unit_message_handler(DBusConnection *connection, DB
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
}
- if ((r = manager_get_unit_from_dbus_path(m, dbus_message_get_path(message), &u)) < 0) {
-
+ r = manager_load_unit_from_dbus_path(m, dbus_message_get_path(message), &error, &u);
+ if (r < 0) {
if (r == -ENOMEM)
- return DBUS_HANDLER_RESULT_NEED_MEMORY;
-
- if (r == -ENOENT) {
- DBusError e;
-
- dbus_error_init(&e);
- dbus_set_error_const(&e, DBUS_ERROR_UNKNOWN_OBJECT, "Unknown unit");
- return bus_send_error_reply(connection, message, &e, r);
- }
+ goto oom;
- return bus_send_error_reply(connection, message, NULL, r);
+ return bus_send_error_reply(connection, message, &error, r);
}
return bus_unit_message_dispatch(u, connection, message);
@@ -660,6 +655,8 @@ oom:
if (reply)
dbus_message_unref(reply);
+ dbus_error_free(&error);
+
return DBUS_HANDLER_RESULT_NEED_MEMORY;
}
diff --git a/src/core/manager.c b/src/core/manager.c
index bd86f89..f8fb8a2 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1490,9 +1490,10 @@ int manager_loop(Manager *m) {
return m->exit_code;
}
-int manager_get_unit_from_dbus_path(Manager *m, const char *s, Unit **_u) {
+int manager_load_unit_from_dbus_path(Manager *m, const char *s, DBusError *e, Unit **_u) {
char *n;
Unit *u;
+ int r;
assert(m);
assert(s);
@@ -1501,14 +1502,15 @@ int manager_get_unit_from_dbus_path(Manager *m, const char *s, Unit **_u) {
if (!startswith(s, "/org/freedesktop/systemd1/unit/"))
return -EINVAL;
- if (!(n = bus_path_unescape(s+31)))
+ n = bus_path_unescape(s+31);
+ if (!n)
return -ENOMEM;
- u = manager_get_unit(m, n);
+ r = manager_load_unit(m, n, NULL, e, &u);
free(n);
- if (!u)
- return -ENOENT;
+ if (r < 0)
+ return r;
*_u = u;
diff --git a/src/core/manager.h b/src/core/manager.h
index 92dc75d..046540d 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -249,11 +249,11 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds);
Job *manager_get_job(Manager *m, uint32_t id);
Unit *manager_get_unit(Manager *m, const char *name);
-int manager_get_unit_from_dbus_path(Manager *m, const char *s, Unit **_u);
int manager_get_job_from_dbus_path(Manager *m, const char *s, Job **_j);
int manager_load_unit_prepare(Manager *m, const char *name, const char *path, DBusError *e, Unit **_ret);
int manager_load_unit(Manager *m, const char *name, const char *path, DBusError *e, Unit **_ret);
+int manager_load_unit_from_dbus_path(Manager *m, const char *s, DBusError *e, Unit **_u);
int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool force, DBusError *e, Job **_ret);
int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode mode, bool force, DBusError *e, Job **_ret);
More information about the systemd-commits
mailing list