aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPatrick Ohly <patrick.ohly@intel.com>2016-04-14 12:08:56 +0200
committerJoshua Lock <joshua.g.lock@intel.com>2016-04-14 14:03:26 +0100
commit252ce007e58748ada766b6153c26e280aac10af9 (patch)
tree3f7b60e6ba19d8c5f636be88e7b5d62c933724d2
parentaa4855b034fa3875b13e4c00a6720b8268b5a60c (diff)
downloadmeta-swupd-252ce007e58748ada766b6153c26e280aac10af9.tar.gz
meta-swupd-252ce007e58748ada766b6153c26e280aac10af9.tar.bz2
meta-swupd-252ce007e58748ada766b6153c26e280aac10af9.zip
swupd-client_2.87.bb: improve file downloading
Errors are now printed including the "staging" filename (and without segfaulting, which was already fixed by the previous patch), and download performance is a lot better on Ostro OS where IMA is active and affects the performance of the close() syscall. The download performance patch was also ported to current swupd master, see https://github.com/clearlinux/swupd-client/pull/42. It does not get included here because there is a chance to get it via an upstream update, whereas that is unlikely for 2.87. Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
-rw-r--r--recipes-core/swupd-client/swupd-client-2.87/0001-log.c-Check-filename-for-NULL-before-logging-it.patch34
-rw-r--r--recipes-core/swupd-client/swupd-client-2.87/0001-log.c-avoid-segfault-and-show-staging-file-name.patch38
-rw-r--r--recipes-core/swupd-client/swupd-client-2.87/0002-downloads-minimize-syscalls-to-improve-performance.patch192
-rw-r--r--recipes-core/swupd-client/swupd-client_2.87.bb3
4 files changed, 232 insertions, 35 deletions
diff --git a/recipes-core/swupd-client/swupd-client-2.87/0001-log.c-Check-filename-for-NULL-before-logging-it.patch b/recipes-core/swupd-client/swupd-client-2.87/0001-log.c-Check-filename-for-NULL-before-logging-it.patch
deleted file mode 100644
index 9487b17..0000000
--- a/recipes-core/swupd-client/swupd-client-2.87/0001-log.c-Check-filename-for-NULL-before-logging-it.patch
+++ /dev/null
@@ -1,34 +0,0 @@
-From c4ac8d370d3a842e32756481ee594bacfe1f5059 Mon Sep 17 00:00:00 2001
-From: Dmitry Rozhkov <dmitry.rozhkov@linux.intel.com>
-Date: Wed, 13 Apr 2016 10:11:51 +0300
-Subject: [PATCH] log.c: Check filename for NULL before logging it
-
-In many error situations a fake file struct is passed to
-the logger. The filename field of the struct needs to be
-checked for NULL before printing it to a log to avoid
-dereference of a NULL-pointer.
-
-Upstream-Status: Inappropriate [the latest upstream version doesn't use
-this code]
-
-Signed-off-by: Dmitry Rozhkov <dmitry.rozhkov@linux.intel.com>
----
- src/log.c | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
-
-diff --git a/src/log.c b/src/log.c
-index 5e5b030..2e3f693 100644
---- a/src/log.c
-+++ b/src/log.c
-@@ -412,7 +412,7 @@ static char *format_log_message(int log_type, enum log_priority priority, struct
- filebuf[PATH_MAXLEN - 1] = 0;
- filebuf2[0] = 0;
- strncpy(filebuf, filename, PATH_MAXLEN - 1);
-- if (file) {
-+ if (file && file->filename) {
- strncpy(filebuf2, file->filename, PATH_MAXLEN - 1);
- filebuf2[PATH_MAXLEN - 1] = 0;
- }
---
-2.5.0
-
diff --git a/recipes-core/swupd-client/swupd-client-2.87/0001-log.c-avoid-segfault-and-show-staging-file-name.patch b/recipes-core/swupd-client/swupd-client-2.87/0001-log.c-avoid-segfault-and-show-staging-file-name.patch
new file mode 100644
index 0000000..424a3bd
--- /dev/null
+++ b/recipes-core/swupd-client/swupd-client-2.87/0001-log.c-avoid-segfault-and-show-staging-file-name.patch
@@ -0,0 +1,38 @@
+From 03ea6060f2992fffe242d0af769b7550a014c28f Mon Sep 17 00:00:00 2001
+From: Patrick Ohly <patrick.ohly@intel.com>
+Date: Thu, 14 Apr 2016 11:01:10 +0200
+Subject: [PATCH 1/2] log.c: avoid segfault and show staging file name
+
+When downloading files, "file->filename" is not set, only
+"file->staging" is. Now the code checks both, starting with
+"file->filename", and proceeds if both are unset, instead of
+segfaulting as before.
+
+Upstream-Status: Submitted [https://github.com/clearlinux/swupd-client/issues/37]
+
+Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
+---
+ src/log.c | 7 +++++--
+ 1 file changed, 5 insertions(+), 2 deletions(-)
+
+diff --git a/src/log.c b/src/log.c
+index 5e5b030..0549f29 100644
+--- a/src/log.c
++++ b/src/log.c
+@@ -413,8 +413,11 @@ static char *format_log_message(int log_type, enum log_priority priority, struct
+ filebuf2[0] = 0;
+ strncpy(filebuf, filename, PATH_MAXLEN - 1);
+ if (file) {
+- strncpy(filebuf2, file->filename, PATH_MAXLEN - 1);
+- filebuf2[PATH_MAXLEN - 1] = 0;
++ const char *filename = file->filename ? file->filename : file->staging;
++ if (filename) {
++ strncpy(filebuf2, filename, PATH_MAXLEN - 1);
++ filebuf2[PATH_MAXLEN - 1] = 0;
++ }
+ }
+ while (strlen(filebuf) < 29)
+ strcat(filebuf, " ");
+--
+2.1.4
+
diff --git a/recipes-core/swupd-client/swupd-client-2.87/0002-downloads-minimize-syscalls-to-improve-performance.patch b/recipes-core/swupd-client/swupd-client-2.87/0002-downloads-minimize-syscalls-to-improve-performance.patch
new file mode 100644
index 0000000..f75496a
--- /dev/null
+++ b/recipes-core/swupd-client/swupd-client-2.87/0002-downloads-minimize-syscalls-to-improve-performance.patch
@@ -0,0 +1,192 @@
+From 96d18d373436fda5c643abd8621d932002b2a007 Mon Sep 17 00:00:00 2001
+From: Patrick Ohly <patrick.ohly@intel.com>
+Date: Thu, 14 Apr 2016 11:03:31 +0200
+Subject: [PATCH 2/2] downloads: minimize syscalls to improve performance
+
+The previous approach was to open/fdopen/fclose the file for each
+chunk that gets passed from curl. This incurrs a huge performance hit
+when close() triggers a hashing of the file content on systems where
+integrity protection via IMA is enabled.
+
+Now the file is opened only once and kept open until the download is
+complete. In addition, the unnecessary usage of C file IO is avoided.
+
+The semantic is changed as little as possible:
+- file gets created only after the first chunk of data arrived
+- file descriptors do not leak to child processes (O_CLOEXEC)
+- data gets appended to existing files (via O_APPEND, used
+ to keep the code simple and avoid an additional lseek)
+- data gets flushed explicitly for each chunk (via fdatasync(),
+ which somewhat approximates the effect that an explicit
+ close() may have had)
+
+As an additional improvement, failures during close() are checked when
+downloading single files. However, perform_curl_io_and_complete()
+still ignores the error.
+
+Upstream-Status: Submitted [https://github.com/clearlinux/swupd-client/issues/37]
+
+Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
+---
+ include/swupd.h | 3 +++
+ src/curl.c | 69 ++++++++++++++++++++++++++++++++++++---------------------
+ src/download.c | 7 ++++++
+ 3 files changed, 54 insertions(+), 25 deletions(-)
+
+diff --git a/include/swupd.h b/include/swupd.h
+index 554e795..fbaa5ae 100644
+--- a/include/swupd.h
++++ b/include/swupd.h
+@@ -82,6 +82,7 @@ struct file {
+ int last_change;
+ struct update_stat stat;
+
++ unsigned int fd_valid : 1;
+ unsigned int is_dir : 1;
+ unsigned int is_file : 1;
+ unsigned int is_link : 1;
+@@ -101,6 +102,7 @@ struct file {
+
+ char *staging; /* output name used during download & staging */
+ CURL *curl; /* curl handle if downloading */
++ int fd; /* file written into during downloading, unset when fd_valid is false */
+ };
+
+ extern bool download_only;
+@@ -183,6 +185,7 @@ extern void swupd_curl_cleanup(void);
+ extern void swupd_curl_set_current_version(int v);
+ extern void swupd_curl_set_requested_version(int v);
+ extern size_t swupd_download_file(void *ptr, size_t size, size_t nmemb, void *userdata);
++extern CURLcode swupd_download_file_complete(CURLcode curl_ret, struct file *file);
+ extern int swupd_curl_get_file(const char *url, char *filename, struct file *file,
+ char *tmp_version, bool pack);
+ #define SWUPD_CURL_LOW_SPEED_LIMIT 1
+diff --git a/src/curl.c b/src/curl.c
+index c989426..c4e1398 100644
+--- a/src/curl.c
++++ b/src/curl.c
+@@ -122,37 +122,54 @@ size_t swupd_download_file(void *ptr, size_t size, size_t nmemb, void *userdata)
+ const char *outfile;
+ int fd;
+ FILE *f;
+- size_t written;
++ size_t written, remaining;
+
+ outfile = file->staging;
++ if (file->fd_valid) {
++ fd = file->fd;
++ } else {
++ fd = open(outfile, O_CREAT | O_RDWR | O_CLOEXEC | O_APPEND, 00600);
++ if (fd < 0) {
++ LOG_ERROR(file, "Cannot open file for write", class_file_io,
++ "\\*outfile=\"%s\",strerror=\"%s\"*\\", outfile, strerror(errno));
++ return -1;
++ }
++ file->fd = fd;
++ file->fd_valid = 1;
++ }
++
++ /* handle short writes with repeated write() calls */
++ for (remaining = size*nmemb; remaining; remaining -= written) {
++ written = write(fd, ptr, size*nmemb);
++ if (written < 0) {
++ LOG_ERROR(file, "write error", class_file_io,
++ "\\*outfile=\"%s\",strerror=\"%s\"*\\", outfile, strerror(errno));
++ return -1;
++ }
++ }
+
+- fd = open(outfile, O_CREAT | O_RDWR , 00600);
+- if (fd < 0) {
+- LOG_ERROR(file, "Cannot open file for write", class_file_io,
+- "\\*outfile=\"%s\",strerror=\"%s\"*\\", outfile, strerror(errno));
++ if (fdatasync(fd)) {
++ LOG_ERROR(file, "fdatasync", class_file_io,
++ "\\*outfile=\"%s\",strerror=\"%s\"*\\", outfile, strerror(errno));
+ return -1;
+- }
++ }
+
+- f = fdopen(fd, "a");
+- if (!f) {
+- LOG_ERROR(file, "Cannot fdopen file for write", class_file_io,
+- "\\*outfile=\"%s\",strerror=\"%s\"*\\", outfile, strerror(errno));
+- close(fd);
+- return -1;
+- }
+-
+- written = fwrite(ptr, size*nmemb, 1, f);
+-
+- fflush(f);
+- fclose(f);
++ return size*nmemb;
++}
+
+- if (written != 1) {
+- LOG_ERROR(file, "short write", class_file_io,
+- "\\*outfile=\"%s\",strerror=\"%s\"*\\", outfile, strerror(errno));
+- return -1;
++CURLcode swupd_download_file_complete(CURLcode curl_ret, struct file *file)
++{
++ if (file->fd_valid) {
++ if (close(file->fd)) {
++ LOG_ERROR(file, "Cannot close file after write", class_file_io,
++ "\\*outfile=\"%s\",strerror=\"%s\"*\\", file->staging, strerror(errno));
++ if (curl_ret == CURLE_OK) {
++ curl_ret = CURLE_WRITE_ERROR;
++ }
++ }
++ file->fd_valid = 0;
+ }
+-
+- return size*nmemb;
++ return curl_ret;
+ }
+
+ /* Download a single file SYNCHRONOUSLY
+@@ -193,7 +210,6 @@ int swupd_curl_get_file(const char *url, char *filename, struct file *file,
+ }
+ }
+ local->staging = filename;
+-
+ if (lstat(filename, &stat) == 0) {
+ if (pack) {
+ curl_ret = curl_easy_setopt(curl, CURLOPT_RESUME_FROM_LARGE, (curl_off_t) stat.st_size);
+@@ -263,6 +279,9 @@ int swupd_curl_get_file(const char *url, char *filename, struct file *file,
+ }
+
+ exit:
++ if (local) {
++ curl_ret = swupd_download_file_complete(curl_ret, local);
++ }
+ if (curl_ret == CURLE_OK) {
+ /* curl command succeeded, download might've failed, let our caller LOG_ */
+ switch (ret) {
+diff --git a/src/download.c b/src/download.c
+index 211ee24..2f88fb1 100644
+--- a/src/download.c
++++ b/src/download.c
+@@ -165,6 +165,7 @@ static void free_curl_list_data(void *data)
+ struct file *file = (struct file*)data;
+ CURL *curl = file->curl;
+
++ swupd_download_file_complete(CURLE_OK, file);
+ curl_multi_remove_handle(mcurl, curl);
+ curl_easy_cleanup(curl);
+ }
+@@ -379,6 +380,12 @@ static int perform_curl_io_and_complete(int *left)
+ continue;
+ }
+
++ curl_ret = swupd_download_file_complete(curl_ret, file);
++ /*
++ * Any error as logged already. Let's continue and
++ * hope for the best... or abort via "return -1"?
++ */
++
+ if (ret == 200) {
+ untar_full_download(file);
+ } else {
+--
+2.1.4
+
diff --git a/recipes-core/swupd-client/swupd-client_2.87.bb b/recipes-core/swupd-client/swupd-client_2.87.bb
index 251a94f..6c394f8 100644
--- a/recipes-core/swupd-client/swupd-client_2.87.bb
+++ b/recipes-core/swupd-client/swupd-client_2.87.bb
@@ -12,7 +12,8 @@ SRC_URI = "\
file://0005-swupd-client-Add-existence-check-to-staging-target.patch \
file://0006-Backport-Use-rename-instead-of-tar-transform.patch \
file://0007-Add-compatibility-with-libarchive-s-bsdtar-command.patch \
- file://0001-log.c-Check-filename-for-NULL-before-logging-it.patch \
+ file://0001-log.c-avoid-segfault-and-show-staging-file-name.patch \
+ file://0002-downloads-minimize-syscalls-to-improve-performance.patch \
"
SRC_URI[md5sum] = "5d272c62edb8a9c576005ac5e1182ea3"