dbus: duplicate Job.Cancel() as CancelJob() and Snapshot.Remove() as RemoveSnapshot...

For all other object mehtods there are already counterparts on the
manager object, as they help us reduce round-trips. So let's complete
this, and reduce complexity on the client side a bit.

As a side effect this also makes "systemctl snapshot" without arguments
work again.

diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c
index fdc1dce1777e16517c0f34810f9419fd6dd4fd8d..20c2a623380cf2aba17e3ef20cda10ee4cfc2a05 100644 (file)
--- a/src/core/dbus-job.c
+++ b/src/core/dbus-job.c
@@ -101,12 +101,11 @@ static DBusHandlerResult bus_job_message_dispatch(Job *j, DBusConnection *connec
         if (dbus_message_is_method_call(message, "org.freedesktop.systemd1.Job", "Cancel")) {
 
                 SELINUX_UNIT_ACCESS_CHECK(j->unit, connection, message, "stop");
         if (dbus_message_is_method_call(message, "org.freedesktop.systemd1.Job", "Cancel")) {
 
                 SELINUX_UNIT_ACCESS_CHECK(j->unit, connection, message, "stop");

+                job_finish_and_invalidate(j, JOB_CANCELED, true);

 
                 reply = dbus_message_new_method_return(message);
                 if (!reply)
                         return DBUS_HANDLER_RESULT_NEED_MEMORY;
 
                 reply = dbus_message_new_method_return(message);
                 if (!reply)
                         return DBUS_HANDLER_RESULT_NEED_MEMORY;

-
-                job_finish_and_invalidate(j, JOB_CANCELED, true);

         } else {
                 const BusBoundProperties bps[] = {
                         { "org.freedesktop.systemd1.Job", bus_job_properties, j },
         } else {
                 const BusBoundProperties bps[] = {
                         { "org.freedesktop.systemd1.Job", bus_job_properties, j },


@@ -114,7 +113,6 @@ static DBusHandlerResult bus_job_message_dispatch(Job *j, DBusConnection *connec
                 };
 
                 SELINUX_UNIT_ACCESS_CHECK(j->unit, connection, message, "status");
                 };
 
                 SELINUX_UNIT_ACCESS_CHECK(j->unit, connection, message, "status");

-

                 return bus_default_message_handler(connection, message, INTROSPECTION, INTERFACES_LIST, bps);
         }
 
                 return bus_default_message_handler(connection, message, INTROSPECTION, INTERFACES_LIST, bps);
         }
 

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 859fa1a90699bc348881eb2c25dd567eff0307df..2d9cea676f6cb9c9a071f280d6085a9e97f85bb3 100644 (file)
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -106,6 +106,9 @@
         "   <arg name=\"id\" type=\"u\" direction=\"in\"/>\n"           \
         "   <arg name=\"job\" type=\"o\" direction=\"out\"/>\n"         \
         "  </method>\n"                                                 \
         "   <arg name=\"id\" type=\"u\" direction=\"in\"/>\n"           \
         "   <arg name=\"job\" type=\"o\" direction=\"out\"/>\n"         \
         "  </method>\n"                                                 \

+        "  <method name=\"CancelJob\">\n"                               \
+        "   <arg name=\"id\" type=\"u\" direction=\"in\"/>\n"           \
+        "  </method>\n"                                                 \

         "  <method name=\"ClearJobs\"/>\n"                              \
         "  <method name=\"ResetFailed\"/>\n"                            \
         "  <method name=\"ListUnits\">\n"                               \
         "  <method name=\"ClearJobs\"/>\n"                              \
         "  <method name=\"ResetFailed\"/>\n"                            \
         "  <method name=\"ListUnits\">\n"                               \


@@ -124,6 +127,9 @@
         "   <arg name=\"cleanup\" type=\"b\" direction=\"in\"/>\n"      \
         "   <arg name=\"unit\" type=\"o\" direction=\"out\"/>\n"        \
         "  </method>\n"                                                 \
         "   <arg name=\"cleanup\" type=\"b\" direction=\"in\"/>\n"      \
         "   <arg name=\"unit\" type=\"o\" direction=\"out\"/>\n"        \
         "  </method>\n"                                                 \

+        "  <method name=\"RemoveSnapshot\">\n"                          \
+        "   <arg name=\"name\" type=\"s\" direction=\"in\"/>\n"         \
+        "  </method>\n"                                                 \

         "  <method name=\"Reload\"/>\n"                                 \
         "  <method name=\"Reexecute\"/>\n"                              \
         "  <method name=\"Exit\"/>\n"                                   \
         "  <method name=\"Reload\"/>\n"                                 \
         "  <method name=\"Reexecute\"/>\n"                              \
         "  <method name=\"Exit\"/>\n"                                   \


@@ -774,10 +780,33 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection,
                                     DBUS_TYPE_INVALID))
                         goto oom;
 
                                     DBUS_TYPE_INVALID))
                         goto oom;
 

+        } else if (dbus_message_is_method_call(message, "org.freedesktop.systemd1.Manager", "CancelJob")) {
+                uint32_t id;
+                Job *j;
+
+                if (!dbus_message_get_args(
+                                    message,
+                                    &error,
+                                    DBUS_TYPE_UINT32, &id,
+                                    DBUS_TYPE_INVALID))
+                        return bus_send_error_reply(connection, message, &error, -EINVAL);
+
+                j = manager_get_job(m, id);
+                if (!j) {
+                        dbus_set_error(&error, BUS_ERROR_NO_SUCH_JOB, "Job %u does not exist.", (unsigned) id);
+                        return bus_send_error_reply(connection, message, &error, -ENOENT);
+                }
+
+                SELINUX_UNIT_ACCESS_CHECK(j->unit, connection, message, "stop");
+                job_finish_and_invalidate(j, JOB_CANCELED, true);
+
+                reply = dbus_message_new_method_return(message);
+                if (!reply)
+                        goto oom;
+

         } else if (dbus_message_is_method_call(message, "org.freedesktop.systemd1.Manager", "ClearJobs")) {
 
                 SELINUX_ACCESS_CHECK(connection, message, "reboot");
         } else if (dbus_message_is_method_call(message, "org.freedesktop.systemd1.Manager", "ClearJobs")) {
 
                 SELINUX_ACCESS_CHECK(connection, message, "reboot");

-

                 manager_clear_jobs(m);
 
                 reply = dbus_message_new_method_return(message);
                 manager_clear_jobs(m);
 
                 reply = dbus_message_new_method_return(message);


@@ -1080,6 +1109,35 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection,
                                     DBUS_TYPE_INVALID))
                         goto oom;
 
                                     DBUS_TYPE_INVALID))
                         goto oom;
 

+        } else if (dbus_message_is_method_call(message, "org.freedesktop.systemd1.Manager", "RemoveSnapshot")) {
+                const char *name;
+                Unit *u;
+
+                if (!dbus_message_get_args(
+                                    message,
+                                    &error,
+                                    DBUS_TYPE_STRING, &name,
+                                    DBUS_TYPE_INVALID))
+                        return bus_send_error_reply(connection, message, &error, -EINVAL);
+
+                u = manager_get_unit(m, name);
+                if (!u) {
+                        dbus_set_error(&error, BUS_ERROR_NO_SUCH_UNIT, "Unit %s does not exist.", name);
+                        return bus_send_error_reply(connection, message, &error, -ENOENT);
+                }
+
+                if (u->type != UNIT_SNAPSHOT) {
+                        dbus_set_error(&error, BUS_ERROR_NO_SUCH_UNIT, "Unit %s is not a snapshot.", name);
+                        return bus_send_error_reply(connection, message, &error, -ENOENT);
+                }
+
+                SELINUX_UNIT_ACCESS_CHECK(u, connection, message, "stop");
+                snapshot_remove(SNAPSHOT(u));
+
+                reply = dbus_message_new_method_return(message);
+                if (!reply)
+                        goto oom;
+

         } else if (dbus_message_is_method_call(message, "org.freedesktop.DBus.Introspectable", "Introspect")) {
                 char *introspection = NULL;
                 FILE *f;
         } else if (dbus_message_is_method_call(message, "org.freedesktop.DBus.Introspectable", "Introspect")) {
                 char *introspection = NULL;
                 FILE *f;

diff --git a/src/core/dbus-snapshot.c b/src/core/dbus-snapshot.c
index 435c6df39ce11b9f57e39cb1e00bde38e4ec041c..02cfcb1829a1de854890f2089f329110aef3acf0 100644 (file)
--- a/src/core/dbus-snapshot.c
+++ b/src/core/dbus-snapshot.c
@@ -54,19 +54,16 @@ static const BusProperty bus_snapshot_properties[] = {
 DBusHandlerResult bus_snapshot_message_handler(Unit *u, DBusConnection *c, DBusMessage *message) {
         Snapshot *s = SNAPSHOT(u);
         _cleanup_dbus_message_unref_ DBusMessage *reply = NULL;
 DBusHandlerResult bus_snapshot_message_handler(Unit *u, DBusConnection *c, DBusMessage *message) {
         Snapshot *s = SNAPSHOT(u);
         _cleanup_dbus_message_unref_ DBusMessage *reply = NULL;

-        DBusError error;
-
-        dbus_error_init(&error);

 
         if (dbus_message_is_method_call(message, "org.freedesktop.systemd1.Snapshot", "Remove")) {
 
                 SELINUX_UNIT_ACCESS_CHECK(u, c, message, "stop");
 
 
         if (dbus_message_is_method_call(message, "org.freedesktop.systemd1.Snapshot", "Remove")) {
 
                 SELINUX_UNIT_ACCESS_CHECK(u, c, message, "stop");
 

-                snapshot_remove(SNAPSHOT(u));
-

                 reply = dbus_message_new_method_return(message);
                 if (!reply)
                 reply = dbus_message_new_method_return(message);
                 if (!reply)

-                        goto oom;
+                        return DBUS_HANDLER_RESULT_NEED_MEMORY;
+
+                snapshot_remove(SNAPSHOT(u));

 
         } else {
                 const BusBoundProperties bps[] = {
 
         } else {
                 const BusBoundProperties bps[] = {


@@ -80,15 +77,8 @@ DBusHandlerResult bus_snapshot_message_handler(Unit *u, DBusConnection *c, DBusM
                 return bus_default_message_handler(c, message, INTROSPECTION, INTERFACES_LIST, bps);
         }
 
                 return bus_default_message_handler(c, message, INTROSPECTION, INTERFACES_LIST, bps);
         }
 

-        if (reply) {
-                if (!dbus_connection_send(c, reply, NULL))
-                        goto oom;
-        }
+        if (!dbus_connection_send(c, reply, NULL))
+                return DBUS_HANDLER_RESULT_NEED_MEMORY;

 
         return DBUS_HANDLER_RESULT_HANDLED;
 
         return DBUS_HANDLER_RESULT_HANDLED;

-
-oom:
-        dbus_error_free(&error);
-
-        return DBUS_HANDLER_RESULT_NEED_MEMORY;

 }
 }

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 2ebfff8daf94e3ef71491401eb3aeb3cdc585df0..b4c4ea36dcde1957325d7d761fdcff1f6feb4da2 100644 (file)
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -1032,12 +1032,14 @@ finish:
 }
 
 static int load_unit(DBusConnection *bus, char **args) {
 }
 
 static int load_unit(DBusConnection *bus, char **args) {

-        int r = 0;
-        char **name, *n;
+        char **name;

 
         assert(args);
 
         STRV_FOREACH(name, args+1) {
 
         assert(args);
 
         STRV_FOREACH(name, args+1) {

+                _cleanup_free_ char *n = NULL;
+                int r;
+

                 n = unit_name_mangle(*name);
                 r = bus_method_call_with_reply (
                                 bus,
                 n = unit_name_mangle(*name);
                 r = bus_method_call_with_reply (
                                 bus,


@@ -1049,18 +1051,14 @@ static int load_unit(DBusConnection *bus, char **args) {
                                 NULL,
                                 DBUS_TYPE_STRING, n ? &n : name,
                                 DBUS_TYPE_INVALID);
                                 NULL,
                                 DBUS_TYPE_STRING, n ? &n : name,
                                 DBUS_TYPE_INVALID);

-                free(n);
-                if (r)
-                        goto finish;
+                if (r < 0)
+                        return r;

         }
 
         }
 

-finish:
-        return r;
+        return 0;

 }
 
 static int cancel_job(DBusConnection *bus, char **args) {
 }
 
 static int cancel_job(DBusConnection *bus, char **args) {

-        DBusMessage *reply = NULL;
-        int r = 0;

         char **name;
 
         assert(args);
         char **name;
 
         assert(args);


@@ -1069,54 +1067,30 @@ static int cancel_job(DBusConnection *bus, char **args) {
                 return daemon_reload(bus, args);
 
         STRV_FOREACH(name, args+1) {
                 return daemon_reload(bus, args);
 
         STRV_FOREACH(name, args+1) {

-                unsigned id;
-                const char *path;
+                uint32_t id;
+                int r;

 
 

-                r = safe_atou(*name, &id);
+                r = safe_atou32(*name, &id);

                 if (r < 0) {
                         log_error("Failed to parse job id: %s", strerror(-r));
                 if (r < 0) {
                         log_error("Failed to parse job id: %s", strerror(-r));

-                        goto finish;
+                        return r;

                 }
                 }

-                assert_cc(sizeof(uint32_t) == sizeof(id));

 
 

-                r = bus_method_call_with_reply (
+                r = bus_method_call_with_reply(

                                 bus,
                                 "org.freedesktop.systemd1",
                                 "/org/freedesktop/systemd1",
                                 "org.freedesktop.systemd1.Manager",
                                 bus,
                                 "org.freedesktop.systemd1",
                                 "/org/freedesktop/systemd1",
                                 "org.freedesktop.systemd1.Manager",

-                                "GetJob",
-                                &reply,
-                                NULL,
-                                DBUS_TYPE_UINT32, &id,
-                                DBUS_TYPE_INVALID);
-                if (r)
-                        goto finish;
-
-                if (!dbus_message_get_args(reply, NULL,
-                                           DBUS_TYPE_OBJECT_PATH, &path,
-                                           DBUS_TYPE_INVALID)) {
-                        log_error("Failed to parse reply");
-                        dbus_message_unref(reply);
-                        r = -EIO;
-                        goto finish;
-                }
-                dbus_message_unref(reply);
-
-                r = bus_method_call_with_reply (
-                                bus,
-                                "org.freedesktop.systemd1",
-                                path,
-                                "org.freedesktop.systemd1.Job",
-                                "Cancel",
+                                "CancelJob",

                                 NULL,
                                 NULL,
                                 NULL,
                                 NULL,

+                                DBUS_TYPE_UINT32, &id,

                                 DBUS_TYPE_INVALID);
                                 DBUS_TYPE_INVALID);

-                if (r)
-                        goto finish;
+                if (r < 0)
+                        return r;

         }
 
         }
 

-finish:
-        return r;
+        return 0;

 }
 
 static bool need_daemon_reload(DBusConnection *bus, const char *unit) {
 }
 
 static bool need_daemon_reload(DBusConnection *bus, const char *unit) {


@@ -3062,7 +3036,7 @@ finish:
 }
 
 static int snapshot(DBusConnection *bus, char **args) {
 }
 
 static int snapshot(DBusConnection *bus, char **args) {

-        DBusMessage *reply = NULL;
+        _cleanup_dbus_message_unref_ DBusMessage *reply = NULL;

         DBusError error;
         int r;
         dbus_bool_t cleanup = FALSE;
         DBusError error;
         int r;
         dbus_bool_t cleanup = FALSE;


@@ -3071,14 +3045,15 @@ static int snapshot(DBusConnection *bus, char **args) {
                 *name = "", *path, *id,
                 *interface = "org.freedesktop.systemd1.Unit",
                 *property = "Id";
                 *name = "", *path, *id,
                 *interface = "org.freedesktop.systemd1.Unit",
                 *property = "Id";

-        char *n;
+        _cleanup_free_ char *n = NULL;

 
         dbus_error_init(&error);
 
 
         dbus_error_init(&error);
 

-        if (strv_length(args) > 1)
+        if (strv_length(args) > 1) {

                 name = args[1];
                 name = args[1];

+                n = unit_name_mangle(name);
+        }

 
 

-        n = unit_name_mangle(name);

         r = bus_method_call_with_reply (
                         bus,
                         "org.freedesktop.systemd1",
         r = bus_method_call_with_reply (
                         bus,
                         "org.freedesktop.systemd1",


@@ -3090,8 +3065,7 @@ static int snapshot(DBusConnection *bus, char **args) {
                         DBUS_TYPE_STRING, n ? (const char**) &n : &name,
                         DBUS_TYPE_BOOLEAN, &cleanup,
                         DBUS_TYPE_INVALID);
                         DBUS_TYPE_STRING, n ? (const char**) &n : &name,
                         DBUS_TYPE_BOOLEAN, &cleanup,
                         DBUS_TYPE_INVALID);

-        free(n);
-        if (r)
+        if (r < 0)

                 goto finish;
 
         if (!dbus_message_get_args(reply, &error,
                 goto finish;
 
         if (!dbus_message_get_args(reply, &error,


@@ -3103,6 +3077,8 @@ static int snapshot(DBusConnection *bus, char **args) {
         }
 
         dbus_message_unref(reply);
         }
 
         dbus_message_unref(reply);

+        reply = NULL;
+

         r = bus_method_call_with_reply (
                         bus,
                         "org.freedesktop.systemd1",
         r = bus_method_call_with_reply (
                         bus,
                         "org.freedesktop.systemd1",


@@ -3114,7 +3090,7 @@ static int snapshot(DBusConnection *bus, char **args) {
                         DBUS_TYPE_STRING, &interface,
                         DBUS_TYPE_STRING, &property,
                         DBUS_TYPE_INVALID);
                         DBUS_TYPE_STRING, &interface,
                         DBUS_TYPE_STRING, &property,
                         DBUS_TYPE_INVALID);

-        if (r)
+        if (r < 0)

                 goto finish;
 
         if (!dbus_message_iter_init(reply, &iter) ||
                 goto finish;
 
         if (!dbus_message_iter_init(reply, &iter) ||


@@ -3138,69 +3114,36 @@ static int snapshot(DBusConnection *bus, char **args) {
                 puts(id);
 
 finish:
                 puts(id);
 
 finish:

-        if (reply)
-                dbus_message_unref(reply);
-

         dbus_error_free(&error);
 
         return r;
 }
 
 static int delete_snapshot(DBusConnection *bus, char **args) {
         dbus_error_free(&error);
 
         return r;
 }
 
 static int delete_snapshot(DBusConnection *bus, char **args) {

-        DBusMessage *reply = NULL;
-        int r = 0;
-        DBusError error;

         char **name;
 
         assert(args);
 
         char **name;
 
         assert(args);
 

-        dbus_error_init(&error);
-

         STRV_FOREACH(name, args+1) {
         STRV_FOREACH(name, args+1) {

-                const char *path = NULL;
-                char *n;
+                _cleanup_free_ char *n = NULL;
+                int r;

 
                 n = unit_name_mangle(*name);
 
                 n = unit_name_mangle(*name);

-                r = bus_method_call_with_reply (
+                r = bus_method_call_with_reply(

                                 bus,
                                 "org.freedesktop.systemd1",
                                 "/org/freedesktop/systemd1",
                                 "org.freedesktop.systemd1.Manager",
                                 bus,
                                 "org.freedesktop.systemd1",
                                 "/org/freedesktop/systemd1",
                                 "org.freedesktop.systemd1.Manager",

-                                "GetUnit",
-                                &reply,
-                                NULL,
-                                DBUS_TYPE_STRING, n ? &n : name,
-                                DBUS_TYPE_INVALID);
-                free(n);
-                if (r)
-                        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;
-                        dbus_message_unref(reply);
-                        dbus_error_free(&error);
-                        goto finish;
-                }
-                dbus_message_unref(reply);
-
-                r = bus_method_call_with_reply (
-                                bus,
-                                "org.freedesktop.systemd1",
-                                path,
-                                "org.freedesktop.systemd1.Snapshot",
-                                "Remove",
+                                "RemoveSnapshot",

                                 NULL,
                                 NULL,
                                 NULL,
                                 NULL,

+                                DBUS_TYPE_STRING, n ? &n : name,

                                 DBUS_TYPE_INVALID);
                                 DBUS_TYPE_INVALID);

-                if (r)
-                        goto finish;
+                if (r < 0)
+                        return r;

         }
 
         }
 

-finish:
-        return r;
+        return 0;

 }
 
 static int daemon_reload(DBusConnection *bus, char **args) {
 }
 
 static int daemon_reload(DBusConnection *bus, char **args) {