From 0a87f6fb034c4702fca46a0fcc139252e60313f0 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Fri, 16 Jun 2017 16:43:11 +0100 Subject: [PATCH] Store the metadata files rather than merging to one store Now we have multiple remotes that can be enabled or changed at runtime we need to do several things better: * Only load components from remotes that are enabled * Only load a component if a higher priority remote has not already added it Rather than just appending all recieved metadata into one big XML file, save the original metadata .xml.gz files in /var/lib/fwupd/remotes.d and only load them in the correct priority order if the remote is known and enabled. Remove the old /var/cache/app-info/xmls/fwupd.xml file, also noting it wasn't really a cache file at all but actually something quite important. --- data/installed-tests/fwupdmgr.sh | 2 +- libfwupd/fwupd-client.c | 5 +- libfwupd/fwupd-client.h | 1 + src/fu-config.c | 7 + src/fu-config.h | 1 + src/fu-main.c | 292 +++++++++++++++---------------- src/fu-util.c | 15 +- 7 files changed, 162 insertions(+), 161 deletions(-) diff --git a/data/installed-tests/fwupdmgr.sh b/data/installed-tests/fwupdmgr.sh index e821b95d5..d5649ea03 100755 --- a/data/installed-tests/fwupdmgr.sh +++ b/data/installed-tests/fwupdmgr.sh @@ -9,7 +9,7 @@ rc=$?; if [[ $rc != 0 ]]; then exit $rc; fi # --- echo "Refreshing with dummy metadata..." -fwupdmgr refresh ${dirname}/firmware-example.xml.gz ${dirname}/firmware-example.xml.gz.asc +fwupdmgr refresh ${dirname}/firmware-example.xml.gz ${dirname}/firmware-example.xml.gz.asc lvfs rc=$?; if [[ $rc != 0 ]]; then exit $rc; fi # --- diff --git a/libfwupd/fwupd-client.c b/libfwupd/fwupd-client.c index 0c28458b1..fb6d260c5 100644 --- a/libfwupd/fwupd-client.c +++ b/libfwupd/fwupd-client.c @@ -1162,7 +1162,7 @@ fwupd_client_update_metadata (FwupdClient *client, GError **error) { return fwupd_client_update_metadata_with_id (client, - NULL, /* remote_id */ + "lvfs", /* remote_id */ metadata_fn, signature_fn, cancellable, @@ -1206,6 +1206,7 @@ fwupd_client_update_metadata_with_id (FwupdClient *client, g_autoptr(GUnixFDList) fd_list = NULL; g_return_val_if_fail (FWUPD_IS_CLIENT (client), FALSE); + g_return_val_if_fail (remote_id != NULL, FALSE); g_return_val_if_fail (metadata_fn != NULL, FALSE); g_return_val_if_fail (signature_fn != NULL, FALSE); g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE); @@ -1251,7 +1252,7 @@ fwupd_client_update_metadata_with_id (FwupdClient *client, close (fd_sig); /* call into daemon */ - body = g_variant_new ("(shh)", remote_id != NULL ? remote_id : "", fd, fd_sig); + body = g_variant_new ("(shh)", remote_id, fd, fd_sig); g_dbus_message_set_body (request, body); helper = fwupd_client_helper_new (); g_dbus_connection_send_message_with_reply (priv->conn, diff --git a/libfwupd/fwupd-client.h b/libfwupd/fwupd-client.h index 8e5492d58..2d21ac018 100644 --- a/libfwupd/fwupd-client.h +++ b/libfwupd/fwupd-client.h @@ -110,6 +110,7 @@ gboolean fwupd_client_install (FwupdClient *client, FwupdInstallFlags install_flags, GCancellable *cancellable, GError **error); +G_DEPRECATED_FOR(fwupd_client_update_metadata_with_id) gboolean fwupd_client_update_metadata (FwupdClient *client, const gchar *metadata_fn, const gchar *signature_fn, diff --git a/src/fu-config.c b/src/fu-config.c index 7c1ce4a09..977097e9c 100644 --- a/src/fu-config.c +++ b/src/fu-config.c @@ -356,6 +356,13 @@ fu_config_get_enable_option_rom (FuConfig *self) return self->enable_option_rom; } +const gchar * +fu_config_get_cached_metadata_location (FuConfig *self) +{ + g_return_val_if_fail (FU_IS_CONFIG (self), NULL); + return "/var/lib/fwupd/remotes.d"; +} + static void fu_config_class_init (FuConfigClass *klass) { diff --git a/src/fu-config.h b/src/fu-config.h index 16bdea9bc..642df10d5 100644 --- a/src/fu-config.h +++ b/src/fu-config.h @@ -38,6 +38,7 @@ gboolean fu_config_load (FuConfig *self, GPtrArray *fu_config_get_blacklist_devices (FuConfig *self); GPtrArray *fu_config_get_blacklist_plugins (FuConfig *self); gboolean fu_config_get_enable_option_rom (FuConfig *self); +const gchar *fu_config_get_cached_metadata_location (FuConfig *self); GPtrArray *fu_config_get_remotes (FuConfig *self); FwupdRemote *fu_config_get_remote_by_id (FuConfig *self, const gchar *remote_id); diff --git a/src/fu-main.c b/src/fu-main.c index 82e580a22..c868122fd 100644 --- a/src/fu-main.c +++ b/src/fu-main.c @@ -70,7 +70,6 @@ typedef struct { FuPending *pending; AsProfile *profile; AsStore *store; - guint store_changed_id; guint owner_id; gboolean coldplug_running; guint coldplug_id; @@ -1352,129 +1351,74 @@ fu_main_get_action_id_for_device (FuMainAuthHelper *helper) } static gboolean -fu_main_daemon_update_metadata (FuMainPrivate *priv, const gchar *remote_id, - gint fd, gint fd_sig, GError **error) +fu_main_load_metadata_from_file (FuMainPrivate *priv, + const gchar *path, + const gchar *remote_id, + GError **error) { - const guint8 *data; - gsize size; GPtrArray *apps; - g_autofree gchar *xml = NULL; g_autoptr(AsStore) store = NULL; - g_autoptr(GBytes) bytes = NULL; - g_autoptr(GBytes) bytes_raw = NULL; - g_autoptr(GBytes) bytes_sig = NULL; - g_autoptr(FuKeyring) kr = NULL; - g_autoptr(GConverter) converter = NULL; g_autoptr(GFile) file = NULL; - g_autoptr(GFile) file_parent = NULL; - g_autoptr(GInputStream) stream_fd = NULL; - g_autoptr(GInputStream) stream = NULL; - g_autoptr(GInputStream) stream_sig = NULL; - - /* read the entire file into memory */ - stream_fd = g_unix_input_stream_new (fd, TRUE); - bytes_raw = g_input_stream_read_bytes (stream_fd, 0x100000, NULL, error); - if (bytes_raw == NULL) - return FALSE; - - /* read signature */ - stream_sig = g_unix_input_stream_new (fd_sig, TRUE); - bytes_sig = g_input_stream_read_bytes (stream_sig, 0x800, NULL, error); - if (bytes_sig == NULL) - return FALSE; - - /* verify file */ - kr = fu_keyring_new (); - if (!fu_keyring_add_public_keys (kr, "/etc/pki/fwupd-metadata", error)) - return FALSE; - if (!fu_keyring_verify_data (kr, bytes_raw, bytes_sig, error)) - return FALSE; - - /* peek the file type and get data */ - data = g_bytes_get_data (bytes_raw, &size); - if (size < 2) { - g_set_error_literal (error, - FWUPD_ERROR, - FWUPD_ERROR_INVALID_FILE, - "file is too small"); - return FALSE; - } - if (data[0] == 0x1f && data[1] == 0x8b) { - g_autoptr(GInputStream) stream_buf = NULL; - g_debug ("using GZip decompressor for data"); - converter = G_CONVERTER (g_zlib_decompressor_new (G_ZLIB_COMPRESSOR_FORMAT_GZIP)); - stream_buf = g_memory_input_stream_new (); - g_memory_input_stream_add_bytes (G_MEMORY_INPUT_STREAM (stream_buf), bytes_raw); - stream = g_converter_input_stream_new (stream_buf, converter); - bytes = g_input_stream_read_bytes (stream, 0x100000, NULL, error); - if (bytes == NULL) - return FALSE; - } else if (data[0] == '<' && data[1] == '?') { - g_debug ("using no decompressor for data"); - bytes = g_bytes_ref (bytes_raw); - } else { - g_set_error (error, - FWUPD_ERROR, - FWUPD_ERROR_INVALID_FILE, - "file type '0x%02x,0x%02x' not supported", - data[0], data[1]); - return FALSE; - } /* load the store locally until we know it is valid */ store = as_store_new (); - data = g_bytes_get_data (bytes, &size); - xml = g_strndup ((const gchar *) data, size); - if (!as_store_from_xml (store, xml, NULL, error)) + file = g_file_new_for_path (path); + if (!as_store_from_file (store, file, NULL, NULL, error)) return FALSE; - /* remove all the existing devices from the store with this RemoteID */ - apps = as_store_get_apps (priv->store); - for (guint i = 0; i < apps->len; i++) { - AsApp *app = g_ptr_array_index (apps, i); - if (g_strcmp0 (as_app_get_metadata_item (app, "fwupd::RemoteID"), - remote_id) == 0) { - g_debug ("removing %s", as_app_get_unique_id (app)); - as_store_remove_app (priv->store, app); - } - } - /* add the new application from the store */ apps = as_store_get_apps (store); for (guint i = 0; i < apps->len; i++) { AsApp *app = g_ptr_array_index (apps, i); + + /* does this app already exist */ + if (as_store_get_app_by_id (priv->store, as_app_get_id (app)) != NULL) { + g_debug ("%s exists in remote %s, skipping", + as_app_get_unique_id (app), + as_app_get_metadata_item (app, "fwupd::RemoteID")); + continue; + } if (remote_id != NULL && remote_id[0] != '\0') as_app_add_metadata (app, "fwupd::RemoteID", remote_id); as_store_add_app (priv->store, app); } - - /* ensure directory exists */ - file = g_file_new_for_path ("/var/cache/app-info/xmls/fwupd.xml"); - file_parent = g_file_get_parent (file); - if (!g_file_query_exists (file_parent, NULL)) { - if (!g_file_make_directory_with_parents (file_parent, NULL, error)) - return FALSE; - } - - /* save the new file */ - as_store_set_api_version (priv->store, 0.9); - as_store_set_origin (priv->store, NULL); - if (!as_store_to_file (priv->store, file, - AS_NODE_TO_XML_FLAG_ADD_HEADER | - AS_NODE_TO_XML_FLAG_FORMAT_MULTILINE | - AS_NODE_TO_XML_FLAG_FORMAT_INDENT, - NULL, error)) { - return FALSE; - } - return TRUE; } static gboolean -fu_main_store_delay_cb (gpointer user_data) +fu_main_load_metadata_store (FuMainPrivate *priv, + const gchar *location, + GError **error) { - FuMainPrivate *priv = (FuMainPrivate *) user_data; GPtrArray *apps; + GPtrArray *remotes; + + /* clear existing store */ + as_store_remove_all (priv->store); + + /* load each enabled metadata file */ + remotes = fu_config_get_remotes (priv->config); + for (guint i = 0; i < remotes->len; i++) { + g_autofree gchar *path = NULL; + FwupdRemote *remote = g_ptr_array_index (remotes, i); + if (!fwupd_remote_get_enabled (remote)) { + g_debug ("remote %s not enabled, so skipping", + fwupd_remote_get_id (remote)); + continue; + } + path = g_build_filename (location, + fwupd_remote_get_id (remote), + "metadata.xml.gz", + NULL); + if (!g_file_test (path, G_FILE_TEST_EXISTS)) { + g_debug ("no %s, so skipping", path); + continue; + } + if (!fu_main_load_metadata_from_file (priv, path, + fwupd_remote_get_id (remote), + error)) + return FALSE; + } /* print what we've got */ apps = as_store_get_apps (priv->store); @@ -1497,16 +1441,83 @@ fu_main_store_delay_cb (gpointer user_data) fu_main_emit_device_changed (priv, item->device); } - priv->store_changed_id = 0; - return G_SOURCE_REMOVE; + return TRUE; } -static void -fu_main_store_changed_cb (AsStore *store, FuMainPrivate *priv) +static gboolean +fu_main_set_contents (const gchar *filename, GBytes *blob, GError **error) { - if (priv->store_changed_id != 0) - return; - priv->store_changed_id = g_timeout_add (200, fu_main_store_delay_cb, priv); + const gchar *data; + gsize size; + g_autoptr(GFile) file = NULL; + g_autoptr(GFile) file_parent = NULL; + + file = g_file_new_for_path (filename); + file_parent = g_file_get_parent (file); + if (!g_file_query_exists (file_parent, NULL)) { + if (!g_file_make_directory_with_parents (file_parent, NULL, error)) + return FALSE; + } + data = g_bytes_get_data (blob, &size); + return g_file_set_contents (filename, data, size, error); +} + +static gboolean +fu_main_daemon_update_metadata (FuMainPrivate *priv, const gchar *remote_id, + gint fd, gint fd_sig, GError **error) +{ + FwupdRemote *remote; + const gchar *location; + g_autoptr(GBytes) bytes_raw = NULL; + g_autoptr(GBytes) bytes_sig = NULL; + g_autoptr(FuKeyring) kr = NULL; + g_autoptr(GInputStream) stream_fd = NULL; + g_autoptr(GInputStream) stream = NULL; + g_autoptr(GInputStream) stream_sig = NULL; + g_autofree gchar *path = NULL; + + /* check remote is valid */ + remote = fu_config_get_remote_by_id (priv->config, remote_id); + if (remote == NULL) { + g_set_error (error, + FWUPD_ERROR, + FWUPD_ERROR_NOT_FOUND, + "remote %s not found", remote_id); + return FALSE; + } + if (!fwupd_remote_get_enabled (remote)) { + g_set_error (error, + FWUPD_ERROR, + FWUPD_ERROR_NOT_SUPPORTED, + "remote %s not enabled", remote_id); + return FALSE; + } + + /* read the entire file into memory */ + stream_fd = g_unix_input_stream_new (fd, TRUE); + bytes_raw = g_input_stream_read_bytes (stream_fd, 0x100000, NULL, error); + if (bytes_raw == NULL) + return FALSE; + + /* read signature */ + stream_sig = g_unix_input_stream_new (fd_sig, TRUE); + bytes_sig = g_input_stream_read_bytes (stream_sig, 0x800, NULL, error); + if (bytes_sig == NULL) + return FALSE; + + /* verify file */ + kr = fu_keyring_new (); + if (!fu_keyring_add_public_keys (kr, "/etc/pki/fwupd-metadata", error)) + return FALSE; + if (!fu_keyring_verify_data (kr, bytes_raw, bytes_sig, error)) + return FALSE; + + /* save XML to remotes.d */ + location = fu_config_get_cached_metadata_location (priv->config); + path = g_strdup_printf ("%s/%s/metadata.xml.gz", location, remote_id); + if (!fu_main_set_contents (path, bytes_raw, error)) + return FALSE; + return fu_main_load_metadata_store (priv, location, error); } static gboolean @@ -2135,7 +2146,7 @@ fu_main_daemon_method_call (GDBusConnection *connection, const gchar *sender, fu_main_invocation_return_error (priv, invocation, error); return; } - if (!fu_main_daemon_update_metadata (priv, NULL, fd_data, fd_sig, &error)) { + if (!fu_main_daemon_update_metadata (priv, "lvfs", fd_data, fd_sig, &error)) { g_prefix_error (&error, "failed to update metadata: "); fu_main_invocation_return_error (priv, invocation, error); return; @@ -2148,7 +2159,6 @@ fu_main_daemon_method_call (GDBusConnection *connection, const gchar *sender, if (g_strcmp0 (method_name, "UpdateMetadataWithId") == 0) { GDBusMessage *message; GUnixFDList *fd_list; - FwupdRemote *remote; const gchar *id = NULL; gint fd_data; gint fd_sig; @@ -2156,25 +2166,6 @@ fu_main_daemon_method_call (GDBusConnection *connection, const gchar *sender, g_variant_get (parameters, "(&shh)", &id, &fd_data, &fd_sig); g_debug ("Called %s(%s,%i,%i)", method_name, id, fd_data, fd_sig); - /* check remote is valid */ - remote = fu_config_get_remote_by_id (priv->config, id); - if (remote == NULL) { - g_set_error (&error, - FWUPD_ERROR, - FWUPD_ERROR_NOT_FOUND, - "remote %s not found", id); - fu_main_invocation_return_error (priv, invocation, error); - return; - } - if (!fwupd_remote_get_enabled (remote)) { - g_set_error (&error, - FWUPD_ERROR, - FWUPD_ERROR_NOT_SUPPORTED, - "remote %s not enabled", id); - fu_main_invocation_return_error (priv, invocation, error); - return; - } - /* update the metadata store */ message = g_dbus_method_invocation_get_message (invocation); fd_list = g_dbus_message_get_unix_fd_list (message); @@ -3117,8 +3108,6 @@ fu_main_private_free (FuMainPrivate *priv) g_object_unref (priv->store); if (priv->introspection_daemon != NULL) g_dbus_node_info_unref (priv->introspection_daemon); - if (priv->store_changed_id != 0) - g_source_remove (priv->store_changed_id); if (priv->pending != NULL) g_object_unref (priv->pending); if (priv->coldplug_id != 0) @@ -3138,17 +3127,24 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(FuMainPrivate, fu_main_private_free) static gboolean fu_main_cleanup_state (GError **error) { - g_autoptr(GFile) f1 = NULL; - f1 = g_file_new_for_path ("/var/cache/app-info/xmls/fwupd-verify.xml"); - if (g_file_query_exists (f1, NULL)) { - if (!g_file_delete (f1, NULL, error)) - return FALSE; + const gchar *filenames[] = { + "/var/cache/app-info/xmls/fwupd-verify.xml", + "/var/cache/app-info/xmls/fwupd.xml", + NULL }; + for (guint i = 0; filenames[i] != NULL; i++) { + g_autoptr(GFile) file = g_file_new_for_path (filenames[i]); + if (g_file_query_exists (file, NULL)) { + if (!g_file_delete (file, NULL, error)) + return FALSE; + } } return TRUE; } + int main (int argc, char *argv[]) { + const gchar *location; gboolean immediate_exit = FALSE; gboolean timed_exit = FALSE; const GOptionEntry options[] = { @@ -3190,22 +3186,7 @@ main (int argc, char *argv[]) priv->devices = g_ptr_array_new_with_free_func ((GDestroyNotify) fu_main_item_free); priv->loop = g_main_loop_new (NULL, FALSE); priv->pending = fu_pending_new (); - priv->store = as_store_new (); priv->profile = as_profile_new (); - g_signal_connect (priv->store, "changed", - G_CALLBACK (fu_main_store_changed_cb), priv); - as_store_set_watch_flags (priv->store, AS_STORE_WATCH_FLAG_ADDED | - AS_STORE_WATCH_FLAG_REMOVED); - - /* load AppStream */ - as_store_add_filter (priv->store, AS_APP_KIND_FIRMWARE); - if (!as_store_load (priv->store, - AS_STORE_LOAD_FLAG_IGNORE_INVALID | - AS_STORE_LOAD_FLAG_APP_INFO_SYSTEM, - NULL, &error)){ - g_printerr ("Failed to load AppStream data: %s\n", error->message); - return EXIT_FAILURE; - } /* read config file */ if (!fu_config_load (priv->config, &error)) { @@ -3213,6 +3194,15 @@ main (int argc, char *argv[]) return EXIT_FAILURE; } + /* load AppStream metadata */ + priv->store = as_store_new (); + as_store_add_filter (priv->store, AS_APP_KIND_FIRMWARE); + location = fu_config_get_cached_metadata_location (priv->config); + if (!fu_main_load_metadata_store (priv, location, &error)) { + g_printerr ("Failed to load AppStream data: %s\n", error->message); + return EXIT_FAILURE; + } + /* set shared USB context */ priv->usb_ctx = g_usb_context_new (&error); if (priv->usb_ctx == NULL) { diff --git a/src/fu-util.c b/src/fu-util.c index 3b665ddc9..4933ab396 100644 --- a/src/fu-util.c +++ b/src/fu-util.c @@ -918,20 +918,21 @@ fu_util_refresh (FuUtilPrivate *priv, gchar **values, GError **error) { if (g_strv_length (values) == 0) return fu_util_download_metadata (priv, error); - if (g_strv_length (values) != 2) { + if (g_strv_length (values) != 3) { g_set_error_literal (error, FWUPD_ERROR, FWUPD_ERROR_INVALID_ARGS, - "Invalid arguments: expected 'filename.xml' 'filename.xml.asc'"); + "Invalid arguments: expected 'filename.xml' 'filename.xml.asc' 'remote-id'"); return FALSE; } /* open file */ - return fwupd_client_update_metadata (priv->client, - values[0], - values[1], - NULL, - error); + return fwupd_client_update_metadata_with_id (priv->client, + values[2], + values[0], + values[1], + NULL, + error); } static gboolean