sysupdated: Split Update() into Acquire() and Install() D-Bus methods (#40628)

These mirror the `sysupdate acquire` and `sysupdate install` verbs,
which are themselves a split of `sysupdate update` into downloading and
installing stages.

I have dropped the existing `Update()` method in the final commit, as
per the suggestion on
https://gitlab.gnome.org/GNOME/gnome-software/-/merge_requests/2004#note_2145880.
If the D-Bus API for `sysupdated` is considered stable now, though, I
can drop that commit.

CC @AdrianVovk

Fixes https://github.com/systemd/systemd/issues/34814

src/sysupdate/org.freedesktop.sysupdate1.conf

@@ -64,7 +64,11 @@
 
                 <allow send_destination="org.freedesktop.sysupdate1"
                        send_interface="org.freedesktop.sysupdate1.Target"
-                       send_member="Update"/>
+                       send_member="Acquire"/>
+
+                <allow send_destination="org.freedesktop.sysupdate1"
+                       send_interface="org.freedesktop.sysupdate1.Target"
+                       send_member="Install"/>
 
                 <allow send_destination="org.freedesktop.sysupdate1"
                        send_interface="org.freedesktop.sysupdate1.Target"

src/sysupdate/sysupdated.c

@@ -100,7 +100,8 @@ typedef enum JobType {
         JOB_LIST,
         JOB_DESCRIBE,
         JOB_CHECK_NEW,
-        JOB_UPDATE,
+        JOB_ACQUIRE,
+        JOB_INSTALL,
         JOB_VACUUM,
         JOB_DESCRIBE_FEATURE,
         _JOB_TYPE_MAX,
@@ -121,7 +122,7 @@ struct Job {
 
         JobType type;
         bool offline;
-        char *version; /* Passed into sysupdate for JOB_DESCRIBE and JOB_UPDATE */
+        char *version; /* Passed into sysupdate for JOB_DESCRIBE, JOB_ACQUIRE and JOB_INSTALL */
         char *feature; /* Passed into sysupdate for JOB_DESCRIBE_FEATURE */
 
         unsigned progress_percent;
@@ -153,7 +154,8 @@ static const char* const job_type_table[_JOB_TYPE_MAX] = {
         [JOB_LIST]             = "list",
         [JOB_DESCRIBE]         = "describe",
         [JOB_CHECK_NEW]        = "check-new",
-        [JOB_UPDATE]           = "update",
+        [JOB_ACQUIRE]          = "acquire",
+        [JOB_INSTALL]          = "install",
         [JOB_VACUUM]           = "vacuum",
         [JOB_DESCRIBE_FEATURE] = "describe-feature",
 };
@@ -220,6 +222,11 @@ static int job_new(JobType type, Target *t, sd_bus_message *msg, JobComplete com
         return 0;
 }
 
+/* Is Job in the set of jobs which require Target.busy to be set so they run exclusively? */
+static bool job_requires_busy(Job *j) {
+        return IN_SET(j->type, JOB_ACQUIRE, JOB_INSTALL, JOB_VACUUM);
+}
+
 static int job_parse_child_output(int _fd, sd_json_variant **ret) {
         _cleanup_close_ int fd = ASSERT_FD(_fd); /* Take ownership of the passed fd */
         _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL;
@@ -332,7 +339,7 @@ static int job_on_exit(sd_event_source *s, const siginfo_t *si, void *userdata)
         assert(s);
         assert(si);
 
-        if (IN_SET(j->type, JOB_UPDATE, JOB_VACUUM)) {
+        if (job_requires_busy(j)) {
                 assert(j->target->busy);
                 j->target->busy = false;
         }
@@ -430,7 +437,7 @@ static int job_start(Job *j) {
 
         assert(j);
 
-        if (IN_SET(j->type, JOB_UPDATE, JOB_VACUUM) && j->target->busy)
+        if (job_requires_busy(j) && j->target->busy)
                 return log_notice_errno(SYNTHETIC_ERRNO(EBUSY), "Target %s busy, ignoring job.", j->target->name);
 
         stdout_fd = memfd_new("sysupdate-stdout");
@@ -453,8 +460,8 @@ static int job_start(Job *j) {
                         NULL, /* maybe --verify=no */
                         NULL, /* maybe --component=, --root=, or --image= */
                         NULL, /* maybe --offline */
-                        NULL, /* list, check-new, update, vacuum, features */
-                        NULL, /* maybe version (for list, update), maybe feature (features) */
+                        NULL, /* list, check-new, acquire, update, vacuum, features */
+                        NULL, /* maybe version (for list, acquire, update), maybe feature (features) */
                         NULL
                 };
                 size_t k = 2;
@@ -479,7 +486,7 @@ static int job_start(Job *j) {
                 if (target_arg)
                         cmd[k++] = target_arg;
 
-                if (j->offline)
+                if (j->offline || j->type == JOB_INSTALL)  /* install is implemented as `update --offline` */
                         cmd[k++] = "--offline";
 
                 switch (j->type) {
@@ -497,8 +504,13 @@ static int job_start(Job *j) {
                         cmd[k++] = "check-new";
                         break;
 
-                case JOB_UPDATE:
-                        cmd[k++] = "update";
+                case JOB_ACQUIRE:
+                        cmd[k++] = "acquire";
+                        cmd[k++] = empty_to_null(j->version);
+                        break;
+
+                case JOB_INSTALL:
+                        cmd[k++] = "update";  /* install is implemented as `update --offline` */
                         cmd[k++] = empty_to_null(j->version);
                         break;
 
@@ -547,7 +559,7 @@ static int job_start(Job *j) {
 
         j->stdout_fd = TAKE_FD(stdout_fd);
 
-        if (IN_SET(j->type, JOB_UPDATE, JOB_VACUUM))
+        if (job_requires_busy(j))
                 j->target->busy = true;
 
         return 0;
@@ -581,7 +593,8 @@ static int job_method_cancel(sd_bus_message *msg, void *userdata, sd_bus_error *
                 action = "org.freedesktop.sysupdate1.check";
                 break;
 
-        case JOB_UPDATE:
+        case JOB_ACQUIRE:
+        case JOB_INSTALL:
                 if (j->version)
                         action = "org.freedesktop.sysupdate1.update-to-version";
                 else
@@ -1065,7 +1078,7 @@ static int target_method_check_new(sd_bus_message *msg, void *userdata, sd_bus_e
         return 1;
 }
 
-static int target_method_update_finished_early(
+static int target_method_acquire_finished_early(
                 sd_bus_message *msg,
                 const Job *j,
                 sd_json_variant *json,
@@ -1073,12 +1086,12 @@ static int target_method_update_finished_early(
 
         /* Called when job finishes w/ a successful exit code, but before any work begins.
          * This happens when there is no candidate (i.e. we're already up-to-date), or
-         * specified update is already installed. */
+         * specified update is already acquired. */
         return sd_bus_error_setf(error, BUS_ERROR_NO_UPDATE_CANDIDATE,
-                                 "Job exited successfully with no work to do, assume already updated");
+                                 "Job exited successfully with no work to do, assume already acquired");
 }
 
-static int target_method_update_detach(sd_bus_message *msg, const Job *j) {
+static int target_method_acquire_detach(sd_bus_message *msg, const Job *j) {
         int r;
 
         assert(msg);
@@ -1091,7 +1104,7 @@ static int target_method_update_detach(sd_bus_message *msg, const Job *j) {
         return 0;
 }
 
-static int target_method_update(sd_bus_message *msg, void *userdata, sd_bus_error *error) {
+static int target_method_acquire(sd_bus_message *msg, void *userdata, sd_bus_error *error) {
         Target *t = ASSERT_PTR(userdata);
         _cleanup_(job_freep) Job *j = NULL;
         const char *version, *action;
@@ -1107,6 +1120,8 @@ static int target_method_update(sd_bus_message *msg, void *userdata, sd_bus_erro
         if (flags != 0)
                 return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Flags must be 0");
 
+        /* We don’t have a separate polkit action for acquire/install as they are both effectively (part of)
+         * an update anyway. */
         if (isempty(version))
                 action = "org.freedesktop.sysupdate1.update";
         else
@@ -1130,10 +1145,94 @@ static int target_method_update(sd_bus_message *msg, void *userdata, sd_bus_erro
         if (r == 0)
                 return 1; /* Will call us back */
 
-        r = job_new(JOB_UPDATE, t, msg, target_method_update_finished_early, &j);
+        r = job_new(JOB_ACQUIRE, t, msg, target_method_acquire_finished_early, &j);
         if (r < 0)
                 return r;
-        j->detach_cb = target_method_update_detach;
+        j->detach_cb = target_method_acquire_detach;
+
+        j->version = strdup(version);
+        if (!j->version)
+                return -ENOMEM;
+
+        r = job_start(j);
+        if (r < 0)
+                return sd_bus_error_set_errnof(error, r, "Failed to start job: %m");
+        TAKE_PTR(j);
+
+        return 1;
+}
+
+static int target_method_install_finished_early(
+                sd_bus_message *msg,
+                const Job *j,
+                sd_json_variant *json,
+                sd_bus_error *error) {
+
+        /* Called when job finishes w/ a successful exit code, but before any work begins.
+         * This happens when there is no candidate (i.e. we're already up-to-date), or
+         * specified update is already installed. */
+        return sd_bus_error_setf(error, BUS_ERROR_NO_UPDATE_CANDIDATE,
+                                 "Job exited successfully with no work to do, assume already installed");
+}
+
+static int target_method_install_detach(sd_bus_message *msg, const Job *j) {
+        int r;
+
+        assert(msg);
+        assert(j);
+
+        r = sd_bus_reply_method_return(msg, "sto", j->version, j->id, j->object_path);
+        if (r < 0)
+                return bus_log_parse_error(r);
+
+        return 0;
+}
+
+static int target_method_install(sd_bus_message *msg, void *userdata, sd_bus_error *error) {
+        Target *t = ASSERT_PTR(userdata);
+        _cleanup_(job_freep) Job *j = NULL;
+        const char *version, *action;
+        uint64_t flags;
+        int r;
+
+        assert(msg);
+
+        r = sd_bus_message_read(msg, "st", &version, &flags);
+        if (r < 0)
+                return r;
+
+        if (flags != 0)
+                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Flags must be 0");
+
+        /* We don’t have a separate polkit action for acquire/install as they are both effectively (part of)
+         * an update anyway. */
+        if (isempty(version))
+                action = "org.freedesktop.sysupdate1.update";
+        else
+                action = "org.freedesktop.sysupdate1.update-to-version";
+
+        const char *details[] = {
+                "class", target_class_to_string(t->class),
+                "name", t->name,
+                "version", version,
+                NULL
+        };
+
+        r = bus_verify_polkit_async(
+                        msg,
+                        action,
+                        details,
+                        &t->manager->polkit_registry,
+                        error);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return 1; /* Will call us back */
+
+        r = job_new(JOB_INSTALL, t, msg, target_method_install_finished_early, &j);
+        if (r < 0)
+                return r;
+        j->detach_cb = target_method_install_detach;
 
         j->version = strdup(version);
         if (!j->version)
@@ -1580,10 +1679,16 @@ static const sd_bus_vtable target_vtable[] = {
                                 target_method_check_new,
                                 SD_BUS_VTABLE_UNPRIVILEGED),
 
-        SD_BUS_METHOD_WITH_ARGS("Update",
+        SD_BUS_METHOD_WITH_ARGS("Acquire",
                                 SD_BUS_ARGS("s", new_version, "t", flags),
                                 SD_BUS_RESULT("s", new_version, "t", job_id, "o", job_path),
-                                target_method_update,
+                                target_method_acquire,
+                                SD_BUS_VTABLE_UNPRIVILEGED),
+
+        SD_BUS_METHOD_WITH_ARGS("Install",
+                                SD_BUS_ARGS("s", new_version, "t", flags),
+                                SD_BUS_RESULT("s", new_version, "t", job_id, "o", job_path),
+                                target_method_install,
                                 SD_BUS_VTABLE_UNPRIVILEGED),
 
         SD_BUS_METHOD_WITH_ARGS("Vacuum",

src/sysupdate/updatectl.c

@@ -72,6 +72,9 @@ typedef struct Operation {
         sd_event_source *job_interrupt_source;
         sd_bus_slot *job_properties_slot;
         sd_bus_slot *job_finished_slot;
+
+        /* Only used for Acquire()/Install() operations: */
+        char *acquired_version;
 } Operation;
 
 static Operation* operation_free(Operation *p) {
@@ -86,6 +89,7 @@ static Operation* operation_free(Operation *p) {
                 assert_se(sd_event_exit(p->event, 0) >= 0);
 
         free(p->job_path);
+        free(p->acquired_version);
 
         sd_event_source_disable_unref(p->job_interrupt_source);
         sd_bus_slot_unref(p->job_properties_slot);
@@ -334,6 +338,8 @@ typedef struct DescribeParams {
         bool newest;
         bool available;
         bool installed;
+        bool partial;
+        bool pending;
         bool obsolete;
         bool protected;
         bool incomplete;
@@ -369,6 +375,8 @@ static int parse_describe(sd_bus_message *reply, Version *ret) {
                 { "newest",        SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(DescribeParams, newest),        0 },
                 { "available",     SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(DescribeParams, available),     0 },
                 { "installed",     SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(DescribeParams, installed),     0 },
+                { "partial",       SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(DescribeParams, partial),       0 },
+                { "pending",       SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(DescribeParams, pending),       0 },
                 { "obsolete",      SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(DescribeParams, obsolete),      0 },
                 { "protected",     SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(DescribeParams, protected),     0 },
                 { "incomplete",    SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(DescribeParams, incomplete),    0 },
@@ -386,6 +394,8 @@ static int parse_describe(sd_bus_message *reply, Version *ret) {
         SET_FLAG(p.v.flags, UPDATE_NEWEST, p.newest);
         SET_FLAG(p.v.flags, UPDATE_AVAILABLE, p.available);
         SET_FLAG(p.v.flags, UPDATE_INSTALLED, p.installed);
+        SET_FLAG(p.v.flags, UPDATE_PARTIAL, p.partial);
+        SET_FLAG(p.v.flags, UPDATE_PENDING, p.pending);
         SET_FLAG(p.v.flags, UPDATE_OBSOLETE, p.obsolete);
         SET_FLAG(p.v.flags, UPDATE_PROTECTED, p.protected);
         SET_FLAG(p.v.flags, UPDATE_INCOMPLETE, p.incomplete);
@@ -804,6 +814,7 @@ static int verb_check(int argc, char **argv, void *userdata) {
 }
 
 #define UPDATE_PROGRESS_FAILED INT_MIN
+#define UPDATE_PROGRESS_ACQUIRED (INT_MAX - 1)
 #define UPDATE_PROGRESS_DONE INT_MAX
 /* Make sure it doesn't overlap w/ errno values */
 assert_cc(UPDATE_PROGRESS_FAILED < -ERRNO_MAX);
@@ -853,6 +864,10 @@ static int update_render_progress(sd_event_source *source, void *userdata) {
                         clear_progress_bar_unbuffered(target);
                         fprintf(stderr, "%s: %s %s\n", target, RED_CROSS_MARK(), STRERROR(progress));
                         total += 100;
+                } else if (progress == UPDATE_PROGRESS_ACQUIRED) {
+                        clear_progress_bar_unbuffered(target);
+                        fprintf(stderr, "%s: %s Installing\n", target, glyph(GLYPH_DOWNLOAD));
+                        total += 100;
                 } else if (progress == UPDATE_PROGRESS_DONE) {
                         clear_progress_bar_unbuffered(target);
                         fprintf(stderr, "%s: %s Done\n", target, GREEN_CHECK_MARK());
@@ -920,7 +935,60 @@ static int update_properties_changed(sd_bus_message *m, void *userdata, sd_bus_e
         return 0;
 }
 
-static int update_finished(sd_bus_message *m, void *userdata, sd_bus_error *error) {
+static int update_install_started(sd_bus_message *reply, void *userdata, sd_bus_error *ret_error) {
+        _cleanup_(operation_freep) Operation *op = ASSERT_PTR(userdata);
+        OrderedHashmap *map = ASSERT_PTR(op->userdata);
+        const sd_bus_error *e;
+        const char *job_path;
+        int r;
+
+        assert(reply);
+
+        e = sd_bus_message_get_error(reply);
+        if (e) {
+                r = -sd_bus_error_get_errno(e);
+
+                r = ordered_hashmap_replace(map, op->target_id, INT_TO_PTR(r));
+                if (r < 0)
+                        log_debug_errno(r, "Failed to update hashmap: %m");
+
+                return 0;
+        }
+
+        r = sd_bus_message_read(reply, "sto", NULL, &op->job_id, &job_path);
+        if (r < 0)
+                return bus_log_parse_error(r);
+        r = free_and_strdup_warn(&op->job_path, job_path);
+        if (r < 0)
+                return r;
+
+        /* Update this job in the hashmap. */
+        r = ordered_hashmap_replace(map, op->target_id, INT_TO_PTR(UPDATE_PROGRESS_ACQUIRED));
+        if (r < 0)
+                log_debug_errno(r, "Failed to update hashmap: %m");
+
+        /* Register for progress notifications for this Install() D-Bus call; previously
+         * op->job_properties_slot was registered for progress notifications for the Acquire() D-Bus call. */
+        sd_bus_slot_unref(TAKE_PTR(op->job_properties_slot));
+        r = sd_bus_match_signal_async(
+                        op->bus,
+                        &op->job_properties_slot,
+                        bus_sysupdate_mgr->destination,
+                        job_path,
+                        "org.freedesktop.DBus.Properties",
+                        "PropertiesChanged",
+                        update_properties_changed,
+                        NULL,
+                        op);
+        if (r < 0)
+                return log_bus_error(r, NULL, op->target_id, "listen for PropertiesChanged");
+
+        TAKE_PTR(op); /* update_install_finished/update_interrupted take ownership of the data */
+
+        return 0;
+}
+
+static int update_install_finished(sd_bus_message *m, void *userdata, sd_bus_error *error) {
         _cleanup_(operation_freep) Operation *op = ASSERT_PTR(userdata);
         OrderedHashmap *map = ASSERT_PTR(op->userdata);
         uint64_t id;
@@ -951,6 +1019,64 @@ static int update_finished(sd_bus_message *m, void *userdata, sd_bus_error *erro
         return 0;
 }
 
+static int update_acquire_finished(sd_bus_message *m, void *userdata, sd_bus_error *error) {
+        _cleanup_(operation_freep) Operation *op = ASSERT_PTR(userdata);
+        OrderedHashmap *map = ASSERT_PTR(op->userdata);
+        uint64_t id;
+        int r, status;
+
+        assert(m);
+
+        r = sd_bus_message_read(m, "toi", &id, NULL, &status);
+        if (r < 0) {
+                bus_log_parse_error_debug(r);
+                return 0;
+        }
+
+        if (id != op->job_id) {
+                TAKE_PTR(op);
+                return 0;
+        }
+
+        if (status == 0) /* success */
+                status = UPDATE_PROGRESS_ACQUIRED;
+        else if (status > 0) /* exit status without errno */
+                status = UPDATE_PROGRESS_FAILED; /* i.e. EXIT_FAILURE */
+        /* else errno */
+
+        r = ordered_hashmap_replace(map, op->target_id, INT_TO_PTR(status));
+        if (r < 0)
+                log_debug_errno(r, "Failed to update hashmap: %m");
+
+        /* Renew the JobRemoved notification for the Install() call instead. */
+        sd_bus_slot_unref(op->job_finished_slot);
+        r = bus_match_signal_async(
+                        op->bus, &op->job_finished_slot, bus_sysupdate_mgr, "JobRemoved", update_install_finished, NULL, op);
+        if (r < 0)
+                return log_bus_error(r, NULL, op->target_id, "listen for JobRemoved");
+
+        /* With the Acquire() call finished, immediately call Install() to deploy the downloaded update.
+         * This reuses the same Operation struct so the progress reporting continues to be done in the same
+         * slot in the terminal. */
+        r = sd_bus_call_method_async(
+                        op->bus,
+                        NULL,
+                        bus_sysupdate_mgr->destination,
+                        op->target_path,
+                        SYSUPDATE_TARGET_INTERFACE,
+                        "Install",
+                        update_install_started,
+                        op,
+                        "st",
+                        op->acquired_version,
+                        0LU);
+        if (r < 0)
+                return log_bus_error(r, NULL, op->target_id, "call Install");
+        TAKE_PTR(op);
+
+        return 0;
+}
+
 static int update_interrupted(sd_event_source *source, void *userdata) {
         /* Since the event loop is exiting, we will never receive the JobRemoved
          * signal. So, we must free the userdata here. */
@@ -959,6 +1085,8 @@ static int update_interrupted(sd_event_source *source, void *userdata) {
         OrderedHashmap *map = ASSERT_PTR(op->userdata);
         int r;
 
+        /* This call should work regardless of whether we’re cancelling the Acquire() call or the Install()
+         * call. */
         r = sd_bus_call_method(op->bus,
                                bus_sysupdate_mgr->destination,
                                op->job_path,
@@ -977,7 +1105,7 @@ static int update_interrupted(sd_event_source *source, void *userdata) {
         return 0;
 }
 
-static int update_started(sd_bus_message *reply, void *userdata, sd_bus_error *ret_error) {
+static int update_acquire_started(sd_bus_message *reply, void *userdata, sd_bus_error *ret_error) {
         _cleanup_(operation_freep) Operation *op = ASSERT_PTR(userdata);
         OrderedHashmap *map = ASSERT_PTR(op->userdata);
         const sd_bus_error *e;
@@ -1008,6 +1136,12 @@ static int update_started(sd_bus_message *reply, void *userdata, sd_bus_error *r
         op->job_path = strdup(job_path);
         if (!op->job_path)
                 return log_oom();
+
+        /* Store the version for the subsequent Install() call */
+        op->acquired_version = strdup(new_version);
+        if (!op->acquired_version)
+                return log_oom();
+
         if (isempty(new_version))
                 new_version = "latest";
 
@@ -1047,7 +1181,7 @@ static int update_started(sd_bus_message *reply, void *userdata, sd_bus_error *r
         if (r < 0)
                 return log_bus_error(r, NULL, op->target_id, "listen for PropertiesChanged");
 
-        TAKE_PTR(op); /* update_finished/update_interrupted take ownership of the data */
+        TAKE_PTR(op); /* update_acquire_finished/update_interrupted take ownership of the data */
 
         return 0;
 }
@@ -1094,7 +1228,7 @@ static int do_update(sd_bus *bus, char **targets) {
 
                 /* Sign up for notification when the associated job finishes */
                 r = bus_match_signal_async(
-                                op->bus, &op->job_finished_slot, bus_sysupdate_mgr, "JobRemoved", update_finished, NULL, op);
+                                op->bus, &op->job_finished_slot, bus_sysupdate_mgr, "JobRemoved", update_acquire_finished, NULL, op);
                 if (r < 0)
                         return log_bus_error(r, NULL, op->target_id, "listen for JobRemoved");
 
@@ -1104,14 +1238,14 @@ static int do_update(sd_bus *bus, char **targets) {
                                 bus_sysupdate_mgr->destination,
                                 target_paths[i],
                                 SYSUPDATE_TARGET_INTERFACE,
-                                "Update",
-                                update_started,
+                                "Acquire",
+                                update_acquire_started,
                                 op,
                                 "st",
                                 versions[i],
                                 0LU);
                 if (r < 0)
-                        return log_bus_error(r, NULL, targets[i], "call Update");
+                        return log_bus_error(r, NULL, targets[i], "call Acquire");
                 TAKE_PTR(op);
 
                 remaining++;