[pacman-dev] Implement multiplexed download using mCURL

Message ID 20200310002826.242770-1-anatol.pomozov@gmail.com
State Superseded, archived
Headers show
Series
  • [pacman-dev] Implement multiplexed download using mCURL
Related show

Commit Message

Anatol Pomozov March 10, 2020, 12:28 a.m. UTC
curl_multi_download_internal() is the main loop that creates up to
'ParallelDownloads' easy curl handles, adds them to mcurl and then
performs curl execution. This is when the paralled downloads heppen.
Once any of the downloads complete the function checks its result.
In case if the download fails it initiates retry with the next server
from payload->servers list. At the download completion all the payload
resources are cleaned up.

curl_multi_handle_single_done() is essentially refactored version of
curl_download_internal() adopted for multi_curl. Once mcurl porting is
complete curl_download_internal() will be removed.

Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
---
 lib/libalpm/dload.c | 361 +++++++++++++++++++++++++++++++++++++++++++-
 lib/libalpm/dload.h |   2 +
 2 files changed, 359 insertions(+), 4 deletions(-)

Comments

Anatol Pomozov March 10, 2020, 12:30 a.m. UTC | #1
Hello

The the the largest change (in terms of LoC) in this patch series. It
add the multi-curl machinery, the bread-and-butter of the parallel
download functionality.

On Mon, Mar 9, 2020 at 5:28 PM Anatol Pomozov <anatol.pomozov@gmail.com> wrote:
>
> curl_multi_download_internal() is the main loop that creates up to
> 'ParallelDownloads' easy curl handles, adds them to mcurl and then
> performs curl execution. This is when the paralled downloads heppen.
> Once any of the downloads complete the function checks its result.
> In case if the download fails it initiates retry with the next server
> from payload->servers list. At the download completion all the payload
> resources are cleaned up.
>
> curl_multi_handle_single_done() is essentially refactored version of
> curl_download_internal() adopted for multi_curl. Once mcurl porting is
> complete curl_download_internal() will be removed.
>
> Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
> ---
>  lib/libalpm/dload.c | 361 +++++++++++++++++++++++++++++++++++++++++++-
>  lib/libalpm/dload.h |   2 +
>  2 files changed, 359 insertions(+), 4 deletions(-)
>
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 3570e234..06bce330 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -18,6 +18,7 @@
>   *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>
> +#include <assert.h>
>  #include <stdlib.h>
>  #include <stdio.h>
>  #include <errno.h>
> @@ -271,6 +272,8 @@ static void curl_set_handle_opts(struct dload_payload *payload,
>         curl_easy_setopt(curl, CURLOPT_TCP_KEEPINTVL, 60L);
>         curl_easy_setopt(curl, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
>
> +       curl_easy_setopt(curl, CURLOPT_PRIVATE, (void *)payload);
> +
>         _alpm_log(handle, ALPM_LOG_DEBUG, "url: %s\n", payload->fileurl);
>
>         if(payload->max_size) {
> @@ -601,15 +604,365 @@ cleanup:
>         return ret;
>  }
>
> +/* Return 0 if retry was sucessfull, -1 otherwise */
> +static int curl_multi_retry_next_server(CURLM *curlm, CURL *curl, struct dload_payload *payload)
> +{
> +       const char *server;
> +       size_t len;
> +       alpm_handle_t *handle = payload->handle;
> +
> +       payload->servers = payload->servers->next;
> +       if(!payload->servers) {
> +               return -1;
> +       }
> +       server = payload->servers->data;
> +
> +       /* regenerate a new fileurl */
> +       free(payload->fileurl);
> +       len = strlen(server) + strlen(payload->filepath) + 2;
> +       MALLOC(payload->fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
> +       snprintf(payload->fileurl, len, "%s/%s", server, payload->filepath);
> +
> +       if(payload->unlink_on_fail) {
> +               /* we keep the file for a new retry but remove its data if any */
> +               fflush(payload->localf);
> +               ftruncate(fileno(payload->localf), 0);
> +               fseek(payload->localf, 0, SEEK_SET);
> +       }
> +
> +       /* Set curl with the new URL */
> +       curl_easy_setopt(curl, CURLOPT_URL, payload->fileurl);
> +
> +       curl_multi_remove_handle(curlm, curl);
> +       curl_multi_add_handle(curlm, curl);
> +
> +       return 0;
> +}
> +
> +/* Returns 2 if download retry happened
> + * Returns 1 if the file is up-to-date
> + * Returns 0 if current payload is completed sucessfully
> + * Returns -1 if an error happened
> + */
> +static int curl_multi_handle_single_done(CURLM *curlm, CURLMsg *msg, const char *localpath)
> +{
> +       alpm_handle_t *handle = NULL;
> +       struct dload_payload *payload = NULL;
> +       CURL *curl = msg->easy_handle;
> +       CURLcode curlerr;
> +       char *effective_url;
> +       long timecond;
> +       double remote_size, bytes_dl = 0;
> +       long remote_time = -1;
> +       struct stat st;
> +       char hostname[HOSTNAME_SIZE];
> +       int ret = -1;
> +
> +       curlerr = curl_easy_getinfo(curl, CURLINFO_PRIVATE, &payload);
> +       assert(curlerr == CURLE_OK);
> +       handle = payload->handle;
> +
> +       curl_gethost(payload->fileurl, hostname, sizeof(hostname));
> +       curlerr = msg->data.result;
> +       _alpm_log(handle, ALPM_LOG_DEBUG, "curl returned result %d from transfer\n",
> +                       curlerr);
> +
> +       /* was it a success? */
> +       switch(curlerr) {
> +               case CURLE_OK:
> +                       /* get http/ftp response code */
> +                       _alpm_log(handle, ALPM_LOG_DEBUG, "response code: %ld\n", payload->respcode);
> +                       if(payload->respcode >= 400) {
> +                               payload->unlink_on_fail = 1;
> +                               if(!payload->errors_ok) {
> +                                       handle->pm_errno = ALPM_ERR_RETRIEVE;
> +                                       /* non-translated message is same as libcurl */
> +                                       snprintf(payload->error_buffer, sizeof(payload->error_buffer),
> +                                                       "The requested URL returned error: %ld", payload->respcode);
> +                                       _alpm_log(handle, ALPM_LOG_ERROR,
> +                                                       _("failed retrieving file '%s' from %s : %s\n"),
> +                                                       payload->remote_name, hostname, payload->error_buffer);
> +                               }
> +                               if(curl_multi_retry_next_server(curlm, curl, payload) == 0) {
> +                                       return 2;
> +                               } else {
> +                                       goto cleanup;
> +                               }
> +                       }
> +                       break;
> +               case CURLE_ABORTED_BY_CALLBACK:
> +                       /* handle the interrupt accordingly */
> +                       if(dload_interrupted == ABORT_OVER_MAXFILESIZE) {
> +                               curlerr = CURLE_FILESIZE_EXCEEDED;
> +                               payload->unlink_on_fail = 1;
> +                               handle->pm_errno = ALPM_ERR_LIBCURL;
> +                               _alpm_log(handle, ALPM_LOG_ERROR,
> +                                               _("failed retrieving file '%s' from %s : expected download size exceeded\n"),
> +                                               payload->remote_name, hostname);
> +                       }
> +                       goto cleanup;
> +               case CURLE_COULDNT_RESOLVE_HOST:
> +                       payload->unlink_on_fail = 1;
> +                       handle->pm_errno = ALPM_ERR_SERVER_BAD_URL;
> +                       _alpm_log(handle, ALPM_LOG_ERROR,
> +                                       _("failed retrieving file '%s' from %s : %s\n"),
> +                                       payload->remote_name, hostname, payload->error_buffer);
> +                       if(curl_multi_retry_next_server(curlm, curl, payload) == 0) {
> +                               return 2;
> +                       } else {
> +                               goto cleanup;
> +                       }
> +               default:
> +                       /* delete zero length downloads */
> +                       if(fstat(fileno(payload->localf), &st) == 0 && st.st_size == 0) {
> +                               payload->unlink_on_fail = 1;
> +                       }
> +                       if(!payload->errors_ok) {
> +                               handle->pm_errno = ALPM_ERR_LIBCURL;
> +                               _alpm_log(handle, ALPM_LOG_ERROR,
> +                                               _("failed retrieving file '%s' from %s : %s\n"),
> +                                               payload->remote_name, hostname, payload->error_buffer);
> +                       } else {
> +                               _alpm_log(handle, ALPM_LOG_DEBUG,
> +                                               "failed retrieving file '%s' from %s : %s\n",
> +                                               payload->remote_name, hostname, payload->error_buffer);
> +                       }
> +                       if(curl_multi_retry_next_server(curlm, curl, payload) == 0) {
> +                               return 2;
> +                       } else {
> +                               goto cleanup;
> +                       }
> +       }
> +
> +       /* retrieve info about the state of the transfer */
> +       curl_easy_getinfo(curl, CURLINFO_FILETIME, &remote_time);
> +       curl_easy_getinfo(curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &remote_size);
> +       curl_easy_getinfo(curl, CURLINFO_SIZE_DOWNLOAD, &bytes_dl);
> +       curl_easy_getinfo(curl, CURLINFO_CONDITION_UNMET, &timecond);
> +       curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &effective_url);
> +
> +       /* time condition was met and we didn't download anything. we need to
> +        * clean up the 0 byte .part file that's left behind. */
> +       if(timecond == 1 && DOUBLE_EQ(bytes_dl, 0)) {
> +               _alpm_log(handle, ALPM_LOG_DEBUG, "file met time condition\n");
> +               ret = 1;
> +               unlink(payload->tempfile_name);
> +               goto cleanup;
> +       }
> +
> +       /* remote_size isn't necessarily the full size of the file, just what the
> +        * server reported as remaining to download. compare it to what curl reported
> +        * as actually being transferred during curl_easy_perform() */
> +       if(!DOUBLE_EQ(remote_size, -1) && !DOUBLE_EQ(bytes_dl, -1) &&
> +                       !DOUBLE_EQ(bytes_dl, remote_size)) {
> +               _alpm_log(handle, ALPM_LOG_ERROR, _("%s appears to be truncated: %jd/%jd bytes\n"),
> +                               payload->remote_name, (intmax_t)bytes_dl, (intmax_t)remote_size);
> +               GOTO_ERR(handle, ALPM_ERR_RETRIEVE, cleanup);
> +       }
> +
> +       if(payload->trust_remote_name) {
> +               if(payload->content_disp_name) {
> +                       /* content-disposition header has a better name for our file */
> +                       free(payload->destfile_name);
> +                       payload->destfile_name = get_fullpath(localpath,
> +                               get_filename(payload->content_disp_name), "");
> +               } else {
> +                       const char *effective_filename = strrchr(effective_url, '/');
> +                       if(effective_filename && strlen(effective_filename) > 2) {
> +                               effective_filename++;
> +
> +                               /* if destfile was never set, we wrote to a tempfile. even if destfile is
> +                                * set, we may have followed some redirects and the effective url may
> +                                * have a better suggestion as to what to name our file. in either case,
> +                                * refactor destfile to this newly derived name. */
> +                               if(!payload->destfile_name || strcmp(effective_filename,
> +                                                       strrchr(payload->destfile_name, '/') + 1) != 0) {
> +                                       free(payload->destfile_name);
> +                                       payload->destfile_name = get_fullpath(localpath, effective_filename, "");
> +                               }
> +                       }
> +               }
> +       }
> +
> +       ret = 0;
> +
> +cleanup:
> +       /* disconnect relationships from the curl handle for things that might go out
> +        * of scope, but could still be touched on connection teardown. This really
> +        * only applies to FTP transfers. */
> +       curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 1L);
> +       curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, (char *)NULL);
> +
> +       if(payload->localf != NULL) {
> +               fclose(payload->localf);
> +               utimes_long(payload->tempfile_name, remote_time);
> +       }
> +
> +       if(ret == 0) {
> +               if(payload->destfile_name) {
> +                       if(rename(payload->tempfile_name, payload->destfile_name)) {
> +                               _alpm_log(handle, ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"),
> +                                               payload->tempfile_name, payload->destfile_name, strerror(errno));
> +                               ret = -1;
> +                       }
> +               }
> +       }
> +
> +       if((ret == -1 || dload_interrupted) && payload->unlink_on_fail &&
> +                       payload->tempfile_name) {
> +               unlink(payload->tempfile_name);
> +       }
> +
> +       // TODO: report that the download has been completed
> +
> +       curl_multi_remove_handle(curlm, curl);
> +       curl_easy_cleanup(curl);
> +       payload->curl = NULL;
> +
> +       FREE(payload->fileurl);
> +       return ret;
> +}
> +
> +/* Returns 0 in case if a new download transaction has been successfully started
> + * Returns -1 if am error happened while starting a new download
> + */
> +static int curl_multi_add_payload(alpm_handle_t *handle, CURLM *curlm,
> +               struct dload_payload *payload, const char *localpath)
> +{
> +       size_t len;
> +       const char *server;
> +       CURL *curl = NULL;
> +       char hostname[HOSTNAME_SIZE];
> +
> +       ASSERT(payload->servers, RET_ERR(handle, ALPM_ERR_SERVER_NONE, -1));
> +       server = payload->servers->data;
> +
> +       curl = curl_easy_init();
> +       payload->curl = curl;
> +
> +       len = strlen(server) + strlen(payload->filepath) + 2;
> +       MALLOC(payload->fileurl, len, GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));
> +       snprintf(payload->fileurl, len, "%s/%s", server, payload->filepath);
> +
> +       payload->tempfile_openmode = "wb";
> +       if(!payload->remote_name) {
> +               STRDUP(payload->remote_name, get_filename(payload->fileurl),
> +                       GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));
> +       }
> +       if(curl_gethost(payload->fileurl, hostname, sizeof(hostname)) != 0) {
> +               _alpm_log(handle, ALPM_LOG_ERROR, _("url '%s' is invalid\n"), payload->fileurl);
> +               GOTO_ERR(handle, ALPM_ERR_SERVER_BAD_URL, cleanup);
> +       }
> +
> +       if(payload->remote_name && strlen(payload->remote_name) > 0) {
> +               payload->destfile_name = get_fullpath(localpath, payload->remote_name, "");
> +               payload->tempfile_name = get_fullpath(localpath, payload->remote_name, ".part");
> +               if(!payload->destfile_name || !payload->tempfile_name) {
> +                       goto cleanup;
> +               }
> +       } else {
> +               /* URL doesn't contain a filename, so make a tempfile. We can't support
> +                * resuming this kind of download; partial transfers will be destroyed */
> +               payload->unlink_on_fail = 1;
> +
> +               payload->localf = create_tempfile(payload, localpath);
> +               if(payload->localf == NULL) {
> +                       goto cleanup;
> +               }
> +       }
> +
> +       curl_set_handle_opts(payload, curl, payload->error_buffer);
> +
> +       if(payload->max_size == payload->initial_size) {
> +               /* .part file is complete */
> +               goto cleanup;
> +       }
> +
> +       if(payload->localf == NULL) {
> +               payload->localf = fopen(payload->tempfile_name, payload->tempfile_openmode);
> +               if(payload->localf == NULL) {
> +                       _alpm_log(handle, ALPM_LOG_ERROR,
> +                                       _("could not open file %s: %s\n"),
> +                                       payload->tempfile_name, strerror(errno));
> +                       GOTO_ERR(handle, ALPM_ERR_RETRIEVE, cleanup);
> +               }
> +       }
> +
> +       _alpm_log(handle, ALPM_LOG_DEBUG,
> +                       "opened tempfile for download: %s (%s)\n", payload->tempfile_name,
> +                       payload->tempfile_openmode);
> +
> +       curl_easy_setopt(curl, CURLOPT_WRITEDATA, payload->localf);
> +       curl_multi_add_handle(curlm, curl);
> +       return 0;
> +
> +cleanup:
> +       FREE(payload->fileurl);
> +       FREE(payload->tempfile_name);
> +       FREE(payload->destfile_name);
> +       FREE(payload->content_disp_name);
> +       curl_easy_cleanup(curl);
> +       return -1;
> +}
> +
>  static int curl_multi_download_internal(alpm_handle_t *handle,
>                 alpm_list_t *payloads /* struct dload_payload */,
>                 const char *localpath)
>  {
> -       (void)handle;
> -       (void)payloads;
> -       (void)localpath;
> -       return 0;
> +       int still_running = 0;
> +       int err = 0;
> +       int parallel_downloads = handle->parallel_downloads;
> +
> +       CURLM *curlm = handle->curlm;
> +       CURLMsg *msg;
> +
> +       while(still_running || payloads) {
> +               int msgs_left = -1;
> +
> +               for(; still_running < parallel_downloads && payloads; still_running++) {
> +                       struct dload_payload *payload = payloads->data;
> +
> +                       if(curl_multi_add_payload(handle, curlm, payloads->data, localpath) == 0) {
> +                               // TODO: report that download has started
> +                               payloads = payloads->next;
> +                       } else {
> +                               // the payload failed to start, do not start any new downloads just wait until
> +                               // active one complete.
> +                               _alpm_log(handle, ALPM_LOG_ERROR, "failed to setup a download payload for %s\n", payload->remote_name);
> +                               payloads = NULL;
> +                               err = -1;
> +                       }
> +               }
> +
> +               CURLMcode mc = curl_multi_perform(curlm, &still_running);
> +
> +               if(mc != CURLM_OK) {
> +                       _alpm_log(handle, ALPM_LOG_ERROR, "curl returned error %d from transfer\n", mc);
> +                       payloads = NULL;
> +                       err = -1;
> +               }
> +
> +               while((msg = curl_multi_info_read(curlm, &msgs_left))) {
> +                       if(msg->msg == CURLMSG_DONE) {
> +                               int done_code = curl_multi_handle_single_done(curlm, msg, localpath);
> +                               if(done_code == 2) {
> +                                       /* in case of a retry increase the counter of active requests
> +                                        * to avoid exiting the loop early
> +                                        */
> +                                       still_running++;
> +                               }
> +                       } else {
> +                               _alpm_log(handle, ALPM_LOG_ERROR, "curl curl_multi_info_read error %d\n", msg->msg);
> +                       }
> +               }
> +               if(still_running) {
> +                       curl_multi_wait(curlm, NULL, 0, 1000, NULL);
> +               }
> +       }
> +
> +       return err;
>  }
> +
>  #endif
>
>  /** Download a file given by a URL to a local directory.
> diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h
> index e87b6a93..a40b51b7 100644
> --- a/lib/libalpm/dload.h
> +++ b/lib/libalpm/dload.h
> @@ -45,6 +45,8 @@ struct dload_payload {
>         int cb_initialized;
>  #ifdef HAVE_LIBCURL
>         CURL *curl;
> +       char error_buffer[CURL_ERROR_SIZE];
> +       FILE *localf; /* temp download file */
>  #endif
>  };
>
> --
> 2.25.1
>
Allan McRae March 25, 2020, 11:37 a.m. UTC | #2
On 10/3/20 10:28 am, Anatol Pomozov wrote:
> curl_multi_download_internal() is the main loop that creates up to
> 'ParallelDownloads' easy curl handles, adds them to mcurl and then
> performs curl execution. This is when the paralled downloads heppen.
> Once any of the downloads complete the function checks its result.
> In case if the download fails it initiates retry with the next server
> from payload->servers list. At the download completion all the payload
> resources are cleaned up.
> 
> curl_multi_handle_single_done() is essentially refactored version of
> curl_download_internal() adopted for multi_curl. Once mcurl porting is
> complete curl_download_internal() will be removed.
> 
> Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
> ---
>  lib/libalpm/dload.c | 361 +++++++++++++++++++++++++++++++++++++++++++-
>  lib/libalpm/dload.h |   2 +
>  2 files changed, 359 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 3570e234..06bce330 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -18,6 +18,7 @@
>   *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <assert.h>

Use our ASSERT define instead.   Allows for cleanup.

>  #include <stdlib.h>
>  #include <stdio.h>
>  #include <errno.h>
> @@ -271,6 +272,8 @@ static void curl_set_handle_opts(struct dload_payload *payload,
>  	curl_easy_setopt(curl, CURLOPT_TCP_KEEPINTVL, 60L);
>  	curl_easy_setopt(curl, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
>  

Extra newline not needed.

> +	curl_easy_setopt(curl, CURLOPT_PRIVATE, (void *)payload);
> +
>  	_alpm_log(handle, ALPM_LOG_DEBUG, "url: %s\n", payload->fileurl);
>  
>  	if(payload->max_size) {
> @@ -601,15 +604,365 @@ cleanup:
>  	return ret;
>  }
>  
> +/* Return 0 if retry was sucessfull, -1 otherwise */
> +static int curl_multi_retry_next_server(CURLM *curlm, CURL *curl, struct dload_payload *payload)
> +{
> +	const char *server;
> +	size_t len;
> +	alpm_handle_t *handle = payload->handle;
> +
> +	payload->servers = payload->servers->next;
> +	if(!payload->servers) {
> +		return -1;
> +	}
> +	server = payload->servers->data;

OK.

> +
> +	/* regenerate a new fileurl */
> +	free(payload->fileurl);
> +	len = strlen(server) + strlen(payload->filepath) + 2;
> +	MALLOC(payload->fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
> +	snprintf(payload->fileurl, len, "%s/%s", server, payload->filepath);
> +

OK.

> +	if(payload->unlink_on_fail) {
> +		/* we keep the file for a new retry but remove its data if any */
> +		fflush(payload->localf);
> +		ftruncate(fileno(payload->localf), 0);
> +		fseek(payload->localf, 0, SEEK_SET);
> +	}
> +
> +	/* Set curl with the new URL */
> +	curl_easy_setopt(curl, CURLOPT_URL, payload->fileurl);
> +
> +	curl_multi_remove_handle(curlm, curl);
> +	curl_multi_add_handle(curlm, curl);

Is this right.  I don't know the curl API, but wouldn't you do the
remove at the start of the function?

> +
> +	return 0;
> +}
> +
> +/* Returns 2 if download retry happened
> + * Returns 1 if the file is up-to-date
> + * Returns 0 if current payload is completed sucessfully

Typo

> + * Returns -1 if an error happened
> + */
> +static int curl_multi_handle_single_done(CURLM *curlm, CURLMsg *msg, const char *localpath)

I have no idea what this function does from its name.

curl_multi_check_download_finished() ?

> +{
> +	alpm_handle_t *handle = NULL;
> +	struct dload_payload *payload = NULL;
> +	CURL *curl = msg->easy_handle;
> +	CURLcode curlerr;
> +	char *effective_url;
> +	long timecond;
> +	double remote_size, bytes_dl = 0;
> +	long remote_time = -1;
> +	struct stat st;
> +	char hostname[HOSTNAME_SIZE];
> +	int ret = -1;
> +
> +	curlerr = curl_easy_getinfo(curl, CURLINFO_PRIVATE, &payload);
> +	assert(curlerr == CURLE_OK);

ASSERT(curlerr == CURLE_OK, return ret);

> +	handle = payload->handle;
> +
> +	curl_gethost(payload->fileurl, hostname, sizeof(hostname));
> +	curlerr = msg->data.result;
> +	_alpm_log(handle, ALPM_LOG_DEBUG, "curl returned result %d from transfer\n",
> +			curlerr);
> +
> +	/* was it a success? */
> +	switch(curlerr) {
> +		case CURLE_OK:
> +			/* get http/ftp response code */
> +			_alpm_log(handle, ALPM_LOG_DEBUG, "response code: %ld\n", payload->respcode);
> +			if(payload->respcode >= 400) {
> +				payload->unlink_on_fail = 1;
> +				if(!payload->errors_ok) {
> +					handle->pm_errno = ALPM_ERR_RETRIEVE;
> +					/* non-translated message is same as libcurl */
> +					snprintf(payload->error_buffer, sizeof(payload->error_buffer),
> +							"The requested URL returned error: %ld", payload->respcode);
> +					_alpm_log(handle, ALPM_LOG_ERROR,
> +							_("failed retrieving file '%s' from %s : %s\n"),
> +							payload->remote_name, hostname, payload->error_buffer);
> +				}
> +				if(curl_multi_retry_next_server(curlm, curl, payload) == 0) {
> +					return 2;
> +				} else {
> +					goto cleanup;
> +				}
> +			}
> +			break;

OK.

> +		case CURLE_ABORTED_BY_CALLBACK:
> +			/* handle the interrupt accordingly */
> +			if(dload_interrupted == ABORT_OVER_MAXFILESIZE) {
> +				curlerr = CURLE_FILESIZE_EXCEEDED;
> +				payload->unlink_on_fail = 1;
> +				handle->pm_errno = ALPM_ERR_LIBCURL;
> +				_alpm_log(handle, ALPM_LOG_ERROR,
> +						_("failed retrieving file '%s' from %s : expected download size exceeded\n"),
> +						payload->remote_name, hostname);
> +			}
> +			goto cleanup;

OK.

> +		case CURLE_COULDNT_RESOLVE_HOST:
> +			payload->unlink_on_fail = 1;
> +			handle->pm_errno = ALPM_ERR_SERVER_BAD_URL;
> +			_alpm_log(handle, ALPM_LOG_ERROR,
> +					_("failed retrieving file '%s' from %s : %s\n"),
> +					payload->remote_name, hostname, payload->error_buffer);
> +			if(curl_multi_retry_next_server(curlm, curl, payload) == 0) {
> +				return 2;
> +			} else {
> +				goto cleanup;
> +			}

OK.

> +		default:
> +			/* delete zero length downloads */
> +			if(fstat(fileno(payload->localf), &st) == 0 && st.st_size == 0) {
> +				payload->unlink_on_fail = 1;
> +			}
> +			if(!payload->errors_ok) {
> +				handle->pm_errno = ALPM_ERR_LIBCURL;
> +				_alpm_log(handle, ALPM_LOG_ERROR,
> +						_("failed retrieving file '%s' from %s : %s\n"),
> +						payload->remote_name, hostname, payload->error_buffer);
> +			} else {
> +				_alpm_log(handle, ALPM_LOG_DEBUG,
> +						"failed retrieving file '%s' from %s : %s\n",
> +						payload->remote_name, hostname, payload->error_buffer);
> +			}
> +			if(curl_multi_retry_next_server(curlm, curl, payload) == 0) {
> +				return 2;
> +			} else {
> +				goto cleanup;
> +			}

OK.    That if() is repeated a lot, but seems necessary.

> +	}
> +
> +	/* retrieve info about the state of the transfer */
> +	curl_easy_getinfo(curl, CURLINFO_FILETIME, &remote_time);
> +	curl_easy_getinfo(curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &remote_size);
> +	curl_easy_getinfo(curl, CURLINFO_SIZE_DOWNLOAD, &bytes_dl);
> +	curl_easy_getinfo(curl, CURLINFO_CONDITION_UNMET, &timecond);
> +	curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &effective_url);
> +

In curl_download_internal there is a "if(final_url != NULL) {" bit here.
 Why is that not transferred here?


> +	/* time condition was met and we didn't download anything. we need to
> +	 * clean up the 0 byte .part file that's left behind. */
> +	if(timecond == 1 && DOUBLE_EQ(bytes_dl, 0)) {
> +		_alpm_log(handle, ALPM_LOG_DEBUG, "file met time condition\n");
> +		ret = 1;
> +		unlink(payload->tempfile_name);
> +		goto cleanup;
> +	}

OK

> +
> +	/* remote_size isn't necessarily the full size of the file, just what the
> +	 * server reported as remaining to download. compare it to what curl reported
> +	 * as actually being transferred during curl_easy_perform() */
> +	if(!DOUBLE_EQ(remote_size, -1) && !DOUBLE_EQ(bytes_dl, -1) &&
> +			!DOUBLE_EQ(bytes_dl, remote_size)) {
> +		_alpm_log(handle, ALPM_LOG_ERROR, _("%s appears to be truncated: %jd/%jd bytes\n"),
> +				payload->remote_name, (intmax_t)bytes_dl, (intmax_t)remote_size);
> +		GOTO_ERR(handle, ALPM_ERR_RETRIEVE, cleanup);
> +	}

OK.

> +
> +	if(payload->trust_remote_name) {
> +		if(payload->content_disp_name) {
> +			/* content-disposition header has a better name for our file */
> +			free(payload->destfile_name);
> +			payload->destfile_name = get_fullpath(localpath,
> +				get_filename(payload->content_disp_name), "");
> +		} else {
> +			const char *effective_filename = strrchr(effective_url, '/');
> +			if(effective_filename && strlen(effective_filename) > 2) {
> +				effective_filename++;
> +
> +				/* if destfile was never set, we wrote to a tempfile. even if destfile is
> +				 * set, we may have followed some redirects and the effective url may
> +				 * have a better suggestion as to what to name our file. in either case,
> +				 * refactor destfile to this newly derived name. */
> +				if(!payload->destfile_name || strcmp(effective_filename,
> +							strrchr(payload->destfile_name, '/') + 1) != 0) {
> +					free(payload->destfile_name);
> +					payload->destfile_name = get_fullpath(localpath, effective_filename, "");
> +				}
> +			}
> +		}
> +	}

OK.

> +
> +	ret = 0;
> +
> +cleanup:
> +	/* disconnect relationships from the curl handle for things that might go out
> +	 * of scope, but could still be touched on connection teardown. This really
> +	 * only applies to FTP transfers. */
> +	curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 1L);
> +	curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, (char *)NULL);
> +

OK.

> +	if(payload->localf != NULL) {
> +		fclose(payload->localf);
> +		utimes_long(payload->tempfile_name, remote_time);
> +	}
> +

OK.

> +	if(ret == 0) {
> +		if(payload->destfile_name) {
> +			if(rename(payload->tempfile_name, payload->destfile_name)) {
> +				_alpm_log(handle, ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"),
> +						payload->tempfile_name, payload->destfile_name, strerror(errno));
> +				ret = -1;
> +			}
> +		}
> +	}

OK.

> +
> +	if((ret == -1 || dload_interrupted) && payload->unlink_on_fail &&
> +			payload->tempfile_name) {
> +		unlink(payload->tempfile_name);
> +	}

OK.

> +
> +	// TODO: report that the download has been completed
> +
> +	curl_multi_remove_handle(curlm, curl);
> +	curl_easy_cleanup(curl);
> +	payload->curl = NULL;
> +
> +	FREE(payload->fileurl);
> +	return ret;
> +}

OK.

> +
> +/* Returns 0 in case if a new download transaction has been successfully started
> + * Returns -1 if am error happened while starting a new download
> + */
> +static int curl_multi_add_payload(alpm_handle_t *handle, CURLM *curlm,
> +		struct dload_payload *payload, const char *localpath)
> +{
> +	size_t len;
> +	const char *server;
> +	CURL *curl = NULL;
> +	char hostname[HOSTNAME_SIZE];
> +
> +	ASSERT(payload->servers, RET_ERR(handle, ALPM_ERR_SERVER_NONE, -1));
> +	server = payload->servers->data;

OK.

> +
> +	curl = curl_easy_init();
> +	payload->curl = curl;
> +
> +	len = strlen(server) + strlen(payload->filepath) + 2;
> +	MALLOC(payload->fileurl, len, GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));
> +	snprintf(payload->fileurl, len, "%s/%s", server, payload->filepath);

OK.

> +
> +	payload->tempfile_openmode = "wb";
> +	if(!payload->remote_name) {
> +		STRDUP(payload->remote_name, get_filename(payload->fileurl),
> +			GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));
> +	}
> +	if(curl_gethost(payload->fileurl, hostname, sizeof(hostname)) != 0) {
> +		_alpm_log(handle, ALPM_LOG_ERROR, _("url '%s' is invalid\n"), payload->fileurl);
> +		GOTO_ERR(handle, ALPM_ERR_SERVER_BAD_URL, cleanup);
> +	}

OK.

> +
> +	if(payload->remote_name && strlen(payload->remote_name) > 0) {
> +		payload->destfile_name = get_fullpath(localpath, payload->remote_name, "");
> +		payload->tempfile_name = get_fullpath(localpath, payload->remote_name, ".part");
> +		if(!payload->destfile_name || !payload->tempfile_name) {
> +			goto cleanup;
> +		}
> +	} else {
> +		/* URL doesn't contain a filename, so make a tempfile. We can't support
> +		 * resuming this kind of download; partial transfers will be destroyed */
> +		payload->unlink_on_fail = 1;
> +
> +		payload->localf = create_tempfile(payload, localpath);
> +		if(payload->localf == NULL) {
> +			goto cleanup;
> +		}
> +	}

OK.

> +
> +	curl_set_handle_opts(payload, curl, payload->error_buffer);
> +
> +	if(payload->max_size == payload->initial_size) {
> +		/* .part file is complete */
> +		goto cleanup;
> +	}
> +
> +	if(payload->localf == NULL) {
> +		payload->localf = fopen(payload->tempfile_name, payload->tempfile_openmode);
> +		if(payload->localf == NULL) {
> +			_alpm_log(handle, ALPM_LOG_ERROR,
> +					_("could not open file %s: %s\n"),
> +					payload->tempfile_name, strerror(errno));
> +			GOTO_ERR(handle, ALPM_ERR_RETRIEVE, cleanup);
> +		}
> +	}
> +

OK.

> +	_alpm_log(handle, ALPM_LOG_DEBUG,
> +			"opened tempfile for download: %s (%s)\n", payload->tempfile_name,
> +			payload->tempfile_openmode);
> +
> +	curl_easy_setopt(curl, CURLOPT_WRITEDATA, payload->localf);
> +	curl_multi_add_handle(curlm, curl);
> +	return 0;
> +
> +cleanup:
> +	FREE(payload->fileurl);
> +	FREE(payload->tempfile_name);
> +	FREE(payload->destfile_name);
> +	FREE(payload->content_disp_name);
> +	curl_easy_cleanup(curl);
> +	return -1;
> +}
> +
OK.

>  static int curl_multi_download_internal(alpm_handle_t *handle,
>  		alpm_list_t *payloads /* struct dload_payload */,
>  		const char *localpath)
>  {
> -	(void)handle;
> -	(void)payloads;
> -	(void)localpath;
> -	return 0;
> +	int still_running = 0;
> +	int err = 0;
> +	int parallel_downloads = handle->parallel_downloads;
> +
> +	CURLM *curlm = handle->curlm;
> +	CURLMsg *msg;
> +
> +	while(still_running || payloads) {
> +		int msgs_left = -1;
> +
> +		for(; still_running < parallel_downloads && payloads; still_running++) {
> +			struct dload_payload *payload = payloads->data;
> +
> +			if(curl_multi_add_payload(handle, curlm, payloads->data, localpath) == 0) {
> +				// TODO: report that download has started
> +				payloads = payloads->next;
> +			} else {
> +				// the payload failed to start, do not start any new downloads just wait until
> +				// active one complete.
> +				_alpm_log(handle, ALPM_LOG_ERROR, "failed to setup a download payload for %s\n", payload->remote_name);
> +				payloads = NULL;
> +				err = -1;

OK.

> +			}
> +		}
> +
> +		CURLMcode mc = curl_multi_perform(curlm, &still_running);
> +
> +		if(mc != CURLM_OK) {
> +			_alpm_log(handle, ALPM_LOG_ERROR, "curl returned error %d from transfer\n", mc);
> +			payloads = NULL;
> +			err = -1;
> +		}

OK

> +
> +		while((msg = curl_multi_info_read(curlm, &msgs_left))) {
> +			if(msg->msg == CURLMSG_DONE) {
> +				int done_code = curl_multi_handle_single_done(curlm, msg, localpath);
> +				if(done_code == 2) {
> +					/* in case of a retry increase the counter of active requests
> +					 * to avoid exiting the loop early
> +					 */
> +					still_running++;
> +				}

OK.

> +			} else {
> +				_alpm_log(handle, ALPM_LOG_ERROR, "curl curl_multi_info_read error %d\n", msg->msg);
> +			}
> +		}
> +		if(still_running) {
> +			curl_multi_wait(curlm, NULL, 0, 1000, NULL);
> +		}
> +	}
> +
> +	return err;
>  }
> +
>  #endif
>  
>  /** Download a file given by a URL to a local directory.
> diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h
> index e87b6a93..a40b51b7 100644
> --- a/lib/libalpm/dload.h
> +++ b/lib/libalpm/dload.h
> @@ -45,6 +45,8 @@ struct dload_payload {
>  	int cb_initialized;
>  #ifdef HAVE_LIBCURL
>  	CURL *curl;
> +	char error_buffer[CURL_ERROR_SIZE];
> +	FILE *localf; /* temp download file */
>  #endif
>  };
>  
>
Anatol Pomozov March 26, 2020, 8:59 p.m. UTC | #3
Hi

On Wed, Mar 25, 2020 at 4:37 AM Allan McRae <allan@archlinux.org> wrote:
>
> On 10/3/20 10:28 am, Anatol Pomozov wrote:
> > curl_multi_download_internal() is the main loop that creates up to
> > 'ParallelDownloads' easy curl handles, adds them to mcurl and then
> > performs curl execution. This is when the paralled downloads heppen.
> > Once any of the downloads complete the function checks its result.
> > In case if the download fails it initiates retry with the next server
> > from payload->servers list. At the download completion all the payload
> > resources are cleaned up.
> >
> > curl_multi_handle_single_done() is essentially refactored version of
> > curl_download_internal() adopted for multi_curl. Once mcurl porting is
> > complete curl_download_internal() will be removed.
> >
> > Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
> > ---
> >  lib/libalpm/dload.c | 361 +++++++++++++++++++++++++++++++++++++++++++-
> >  lib/libalpm/dload.h |   2 +
> >  2 files changed, 359 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> > index 3570e234..06bce330 100644
> > --- a/lib/libalpm/dload.c
> > +++ b/lib/libalpm/dload.c
> > @@ -18,6 +18,7 @@
> >   *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >   */
> >
> > +#include <assert.h>
>
> Use our ASSERT define instead.   Allows for cleanup.

fixed

>
> >  #include <stdlib.h>
> >  #include <stdio.h>
> >  #include <errno.h>
> > @@ -271,6 +272,8 @@ static void curl_set_handle_opts(struct dload_payload *payload,
> >       curl_easy_setopt(curl, CURLOPT_TCP_KEEPINTVL, 60L);
> >       curl_easy_setopt(curl, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
> >
>
> Extra newline not needed.

fixed

>
> > +     curl_easy_setopt(curl, CURLOPT_PRIVATE, (void *)payload);
> > +
> >       _alpm_log(handle, ALPM_LOG_DEBUG, "url: %s\n", payload->fileurl);
> >
> >       if(payload->max_size) {
> > @@ -601,15 +604,365 @@ cleanup:
> >       return ret;
> >  }
> >
> > +/* Return 0 if retry was sucessfull, -1 otherwise */
> > +static int curl_multi_retry_next_server(CURLM *curlm, CURL *curl, struct dload_payload *payload)
> > +{
> > +     const char *server;
> > +     size_t len;
> > +     alpm_handle_t *handle = payload->handle;
> > +
> > +     payload->servers = payload->servers->next;
> > +     if(!payload->servers) {
> > +             return -1;
> > +     }
> > +     server = payload->servers->data;
>
> OK.
>
> > +
> > +     /* regenerate a new fileurl */
> > +     free(payload->fileurl);
> > +     len = strlen(server) + strlen(payload->filepath) + 2;
> > +     MALLOC(payload->fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
> > +     snprintf(payload->fileurl, len, "%s/%s", server, payload->filepath);
> > +
>
> OK.
>
> > +     if(payload->unlink_on_fail) {
> > +             /* we keep the file for a new retry but remove its data if any */
> > +             fflush(payload->localf);
> > +             ftruncate(fileno(payload->localf), 0);
> > +             fseek(payload->localf, 0, SEEK_SET);
> > +     }
> > +
> > +     /* Set curl with the new URL */
> > +     curl_easy_setopt(curl, CURLOPT_URL, payload->fileurl);
> > +
> > +     curl_multi_remove_handle(curlm, curl);
> > +     curl_multi_add_handle(curlm, curl);
>
> Is this right.  I don't know the curl API, but wouldn't you do the
> remove at the start of the function?

It does not really matter whether we remove here or at the beginning
of the function. curl completed transfer for this easy handle and does
not use it any more. I just keep the curl related calls close to each
other to make it a bit more readable.

Note that if the retry function failed then we do not remove the easy
handle here, but instead remove it later when we clean the payload
data at the end of curl_multi_handle_single_done().

And one more note - remove/add curl handle is essentially a "restart
curl transfer" functionality.

>
> > +
> > +     return 0;
> > +}
> > +
> > +/* Returns 2 if download retry happened
> > + * Returns 1 if the file is up-to-date
> > + * Returns 0 if current payload is completed sucessfully
>
> Typo

fixed.

>
> > + * Returns -1 if an error happened
> > + */
> > +static int curl_multi_handle_single_done(CURLM *curlm, CURLMsg *msg, const char *localpath)
>
> I have no idea what this function does from its name.

This is a callback for mCURL event CURLMSG_DONE
https://curl.haxx.se/libcurl/c/curl_multi_info_read.html
We receive this event when one "curl easy" (or single) transfer is finished.

> curl_multi_check_download_finished() ?
>
> > +{
> > +     alpm_handle_t *handle = NULL;
> > +     struct dload_payload *payload = NULL;
> > +     CURL *curl = msg->easy_handle;
> > +     CURLcode curlerr;
> > +     char *effective_url;
> > +     long timecond;
> > +     double remote_size, bytes_dl = 0;
> > +     long remote_time = -1;
> > +     struct stat st;
> > +     char hostname[HOSTNAME_SIZE];
> > +     int ret = -1;
> > +
> > +     curlerr = curl_easy_getinfo(curl, CURLINFO_PRIVATE, &payload);
> > +     assert(curlerr == CURLE_OK);
>
> ASSERT(curlerr == CURLE_OK, return ret);

done

>
> > +     handle = payload->handle;
> > +
> > +     curl_gethost(payload->fileurl, hostname, sizeof(hostname));
> > +     curlerr = msg->data.result;
> > +     _alpm_log(handle, ALPM_LOG_DEBUG, "curl returned result %d from transfer\n",
> > +                     curlerr);
> > +
> > +     /* was it a success? */
> > +     switch(curlerr) {
> > +             case CURLE_OK:
> > +                     /* get http/ftp response code */
> > +                     _alpm_log(handle, ALPM_LOG_DEBUG, "response code: %ld\n", payload->respcode);
> > +                     if(payload->respcode >= 400) {
> > +                             payload->unlink_on_fail = 1;
> > +                             if(!payload->errors_ok) {
> > +                                     handle->pm_errno = ALPM_ERR_RETRIEVE;
> > +                                     /* non-translated message is same as libcurl */
> > +                                     snprintf(payload->error_buffer, sizeof(payload->error_buffer),
> > +                                                     "The requested URL returned error: %ld", payload->respcode);
> > +                                     _alpm_log(handle, ALPM_LOG_ERROR,
> > +                                                     _("failed retrieving file '%s' from %s : %s\n"),
> > +                                                     payload->remote_name, hostname, payload->error_buffer);
> > +                             }
> > +                             if(curl_multi_retry_next_server(curlm, curl, payload) == 0) {
> > +                                     return 2;
> > +                             } else {
> > +                                     goto cleanup;
> > +                             }
> > +                     }
> > +                     break;
>
> OK.
>
> > +             case CURLE_ABORTED_BY_CALLBACK:
> > +                     /* handle the interrupt accordingly */
> > +                     if(dload_interrupted == ABORT_OVER_MAXFILESIZE) {
> > +                             curlerr = CURLE_FILESIZE_EXCEEDED;
> > +                             payload->unlink_on_fail = 1;
> > +                             handle->pm_errno = ALPM_ERR_LIBCURL;
> > +                             _alpm_log(handle, ALPM_LOG_ERROR,
> > +                                             _("failed retrieving file '%s' from %s : expected download size exceeded\n"),
> > +                                             payload->remote_name, hostname);
> > +                     }
> > +                     goto cleanup;
>
> OK.
>
> > +             case CURLE_COULDNT_RESOLVE_HOST:
> > +                     payload->unlink_on_fail = 1;
> > +                     handle->pm_errno = ALPM_ERR_SERVER_BAD_URL;
> > +                     _alpm_log(handle, ALPM_LOG_ERROR,
> > +                                     _("failed retrieving file '%s' from %s : %s\n"),
> > +                                     payload->remote_name, hostname, payload->error_buffer);
> > +                     if(curl_multi_retry_next_server(curlm, curl, payload) == 0) {
> > +                             return 2;
> > +                     } else {
> > +                             goto cleanup;
> > +                     }
>
> OK.
>
> > +             default:
> > +                     /* delete zero length downloads */
> > +                     if(fstat(fileno(payload->localf), &st) == 0 && st.st_size == 0) {
> > +                             payload->unlink_on_fail = 1;
> > +                     }
> > +                     if(!payload->errors_ok) {
> > +                             handle->pm_errno = ALPM_ERR_LIBCURL;
> > +                             _alpm_log(handle, ALPM_LOG_ERROR,
> > +                                             _("failed retrieving file '%s' from %s : %s\n"),
> > +                                             payload->remote_name, hostname, payload->error_buffer);
> > +                     } else {
> > +                             _alpm_log(handle, ALPM_LOG_DEBUG,
> > +                                             "failed retrieving file '%s' from %s : %s\n",
> > +                                             payload->remote_name, hostname, payload->error_buffer);
> > +                     }
> > +                     if(curl_multi_retry_next_server(curlm, curl, payload) == 0) {
> > +                             return 2;
> > +                     } else {
> > +                             goto cleanup;
> > +                     }
>
> OK.    That if() is repeated a lot, but seems necessary.

Yeah, this curl_multi_retry_next_server() looks a bit ugly and I have
no ideas how to make it prettier.

> > +     }
> > +
> > +     /* retrieve info about the state of the transfer */
> > +     curl_easy_getinfo(curl, CURLINFO_FILETIME, &remote_time);
> > +     curl_easy_getinfo(curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &remote_size);
> > +     curl_easy_getinfo(curl, CURLINFO_SIZE_DOWNLOAD, &bytes_dl);
> > +     curl_easy_getinfo(curl, CURLINFO_CONDITION_UNMET, &timecond);
> > +     curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &effective_url);
> > +
>
> In curl_download_internal there is a "if(final_url != NULL) {" bit here.
>  Why is that not transferred here?

final_url is not used for parallel download functionality.

For the sequential download code (current functionality) the logic is following:
 - start downloading a file
 - once the file is downloaded get its final_url. It might be
different from the originally requested URL due to redirects.
 - construct *.sig file download url as "final_url + .sig"
 - download signature file

With the parallel download code we start downloading a file and its
signature in parallel. Thus final_url is not needed. It will be
removed in the final cleanup CL.

> > +     /* time condition was met and we didn't download anything. we need to
> > +      * clean up the 0 byte .part file that's left behind. */
> > +     if(timecond == 1 && DOUBLE_EQ(bytes_dl, 0)) {
> > +             _alpm_log(handle, ALPM_LOG_DEBUG, "file met time condition\n");
> > +             ret = 1;
> > +             unlink(payload->tempfile_name);
> > +             goto cleanup;
> > +     }
>
> OK
>
> > +
> > +     /* remote_size isn't necessarily the full size of the file, just what the
> > +      * server reported as remaining to download. compare it to what curl reported
> > +      * as actually being transferred during curl_easy_perform() */
> > +     if(!DOUBLE_EQ(remote_size, -1) && !DOUBLE_EQ(bytes_dl, -1) &&
> > +                     !DOUBLE_EQ(bytes_dl, remote_size)) {
> > +             _alpm_log(handle, ALPM_LOG_ERROR, _("%s appears to be truncated: %jd/%jd bytes\n"),
> > +                             payload->remote_name, (intmax_t)bytes_dl, (intmax_t)remote_size);
> > +             GOTO_ERR(handle, ALPM_ERR_RETRIEVE, cleanup);
> > +     }
>
> OK.
>
> > +
> > +     if(payload->trust_remote_name) {
> > +             if(payload->content_disp_name) {
> > +                     /* content-disposition header has a better name for our file */
> > +                     free(payload->destfile_name);
> > +                     payload->destfile_name = get_fullpath(localpath,
> > +                             get_filename(payload->content_disp_name), "");
> > +             } else {
> > +                     const char *effective_filename = strrchr(effective_url, '/');
> > +                     if(effective_filename && strlen(effective_filename) > 2) {
> > +                             effective_filename++;
> > +
> > +                             /* if destfile was never set, we wrote to a tempfile. even if destfile is
> > +                              * set, we may have followed some redirects and the effective url may
> > +                              * have a better suggestion as to what to name our file. in either case,
> > +                              * refactor destfile to this newly derived name. */
> > +                             if(!payload->destfile_name || strcmp(effective_filename,
> > +                                                     strrchr(payload->destfile_name, '/') + 1) != 0) {
> > +                                     free(payload->destfile_name);
> > +                                     payload->destfile_name = get_fullpath(localpath, effective_filename, "");
> > +                             }
> > +                     }
> > +             }
> > +     }
>
> OK.
>
> > +
> > +     ret = 0;
> > +
> > +cleanup:
> > +     /* disconnect relationships from the curl handle for things that might go out
> > +      * of scope, but could still be touched on connection teardown. This really
> > +      * only applies to FTP transfers. */
> > +     curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 1L);
> > +     curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, (char *)NULL);
> > +
>
> OK.
>
> > +     if(payload->localf != NULL) {
> > +             fclose(payload->localf);
> > +             utimes_long(payload->tempfile_name, remote_time);
> > +     }
> > +
>
> OK.
>
> > +     if(ret == 0) {
> > +             if(payload->destfile_name) {
> > +                     if(rename(payload->tempfile_name, payload->destfile_name)) {
> > +                             _alpm_log(handle, ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"),
> > +                                             payload->tempfile_name, payload->destfile_name, strerror(errno));
> > +                             ret = -1;
> > +                     }
> > +             }
> > +     }
>
> OK.
>
> > +
> > +     if((ret == -1 || dload_interrupted) && payload->unlink_on_fail &&
> > +                     payload->tempfile_name) {
> > +             unlink(payload->tempfile_name);
> > +     }
>
> OK.
>
> > +
> > +     // TODO: report that the download has been completed
> > +
> > +     curl_multi_remove_handle(curlm, curl);
> > +     curl_easy_cleanup(curl);
> > +     payload->curl = NULL;
> > +
> > +     FREE(payload->fileurl);
> > +     return ret;
> > +}
>
> OK.
>
> > +
> > +/* Returns 0 in case if a new download transaction has been successfully started
> > + * Returns -1 if am error happened while starting a new download
> > + */
> > +static int curl_multi_add_payload(alpm_handle_t *handle, CURLM *curlm,
> > +             struct dload_payload *payload, const char *localpath)
> > +{
> > +     size_t len;
> > +     const char *server;
> > +     CURL *curl = NULL;
> > +     char hostname[HOSTNAME_SIZE];
> > +
> > +     ASSERT(payload->servers, RET_ERR(handle, ALPM_ERR_SERVER_NONE, -1));
> > +     server = payload->servers->data;
>
> OK.
>
> > +
> > +     curl = curl_easy_init();
> > +     payload->curl = curl;
> > +
> > +     len = strlen(server) + strlen(payload->filepath) + 2;
> > +     MALLOC(payload->fileurl, len, GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));
> > +     snprintf(payload->fileurl, len, "%s/%s", server, payload->filepath);
>
> OK.
>
> > +
> > +     payload->tempfile_openmode = "wb";
> > +     if(!payload->remote_name) {
> > +             STRDUP(payload->remote_name, get_filename(payload->fileurl),
> > +                     GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));
> > +     }
> > +     if(curl_gethost(payload->fileurl, hostname, sizeof(hostname)) != 0) {
> > +             _alpm_log(handle, ALPM_LOG_ERROR, _("url '%s' is invalid\n"), payload->fileurl);
> > +             GOTO_ERR(handle, ALPM_ERR_SERVER_BAD_URL, cleanup);
> > +     }
>
> OK.
>
> > +
> > +     if(payload->remote_name && strlen(payload->remote_name) > 0) {
> > +             payload->destfile_name = get_fullpath(localpath, payload->remote_name, "");
> > +             payload->tempfile_name = get_fullpath(localpath, payload->remote_name, ".part");
> > +             if(!payload->destfile_name || !payload->tempfile_name) {
> > +                     goto cleanup;
> > +             }
> > +     } else {
> > +             /* URL doesn't contain a filename, so make a tempfile. We can't support
> > +              * resuming this kind of download; partial transfers will be destroyed */
> > +             payload->unlink_on_fail = 1;
> > +
> > +             payload->localf = create_tempfile(payload, localpath);
> > +             if(payload->localf == NULL) {
> > +                     goto cleanup;
> > +             }
> > +     }
>
> OK.
>
> > +
> > +     curl_set_handle_opts(payload, curl, payload->error_buffer);
> > +
> > +     if(payload->max_size == payload->initial_size) {
> > +             /* .part file is complete */
> > +             goto cleanup;
> > +     }
> > +
> > +     if(payload->localf == NULL) {
> > +             payload->localf = fopen(payload->tempfile_name, payload->tempfile_openmode);
> > +             if(payload->localf == NULL) {
> > +                     _alpm_log(handle, ALPM_LOG_ERROR,
> > +                                     _("could not open file %s: %s\n"),
> > +                                     payload->tempfile_name, strerror(errno));
> > +                     GOTO_ERR(handle, ALPM_ERR_RETRIEVE, cleanup);
> > +             }
> > +     }
> > +
>
> OK.
>
> > +     _alpm_log(handle, ALPM_LOG_DEBUG,
> > +                     "opened tempfile for download: %s (%s)\n", payload->tempfile_name,
> > +                     payload->tempfile_openmode);
> > +
> > +     curl_easy_setopt(curl, CURLOPT_WRITEDATA, payload->localf);
> > +     curl_multi_add_handle(curlm, curl);
> > +     return 0;
> > +
> > +cleanup:
> > +     FREE(payload->fileurl);
> > +     FREE(payload->tempfile_name);
> > +     FREE(payload->destfile_name);
> > +     FREE(payload->content_disp_name);
> > +     curl_easy_cleanup(curl);
> > +     return -1;
> > +}
> > +
> OK.
>
> >  static int curl_multi_download_internal(alpm_handle_t *handle,
> >               alpm_list_t *payloads /* struct dload_payload */,
> >               const char *localpath)
> >  {
> > -     (void)handle;
> > -     (void)payloads;
> > -     (void)localpath;
> > -     return 0;
> > +     int still_running = 0;
> > +     int err = 0;
> > +     int parallel_downloads = handle->parallel_downloads;
> > +
> > +     CURLM *curlm = handle->curlm;
> > +     CURLMsg *msg;
> > +
> > +     while(still_running || payloads) {
> > +             int msgs_left = -1;
> > +
> > +             for(; still_running < parallel_downloads && payloads; still_running++) {
> > +                     struct dload_payload *payload = payloads->data;
> > +
> > +                     if(curl_multi_add_payload(handle, curlm, payloads->data, localpath) == 0) {
> > +                             // TODO: report that download has started
> > +                             payloads = payloads->next;
> > +                     } else {
> > +                             // the payload failed to start, do not start any new downloads just wait until
> > +                             // active one complete.
> > +                             _alpm_log(handle, ALPM_LOG_ERROR, "failed to setup a download payload for %s\n", payload->remote_name);
> > +                             payloads = NULL;
> > +                             err = -1;
>
> OK.
>
> > +                     }
> > +             }
> > +
> > +             CURLMcode mc = curl_multi_perform(curlm, &still_running);
> > +
> > +             if(mc != CURLM_OK) {
> > +                     _alpm_log(handle, ALPM_LOG_ERROR, "curl returned error %d from transfer\n", mc);
> > +                     payloads = NULL;
> > +                     err = -1;
> > +             }
>
> OK
>
> > +
> > +             while((msg = curl_multi_info_read(curlm, &msgs_left))) {
> > +                     if(msg->msg == CURLMSG_DONE) {
> > +                             int done_code = curl_multi_handle_single_done(curlm, msg, localpath);
> > +                             if(done_code == 2) {
> > +                                     /* in case of a retry increase the counter of active requests
> > +                                      * to avoid exiting the loop early
> > +                                      */
> > +                                     still_running++;
> > +                             }
>
> OK.
>
> > +                     } else {
> > +                             _alpm_log(handle, ALPM_LOG_ERROR, "curl curl_multi_info_read error %d\n", msg->msg);
> > +                     }
> > +             }
> > +             if(still_running) {
> > +                     curl_multi_wait(curlm, NULL, 0, 1000, NULL);
> > +             }
> > +     }
> > +
> > +     return err;
> >  }
> > +
> >  #endif
> >
> >  /** Download a file given by a URL to a local directory.
> > diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h
> > index e87b6a93..a40b51b7 100644
> > --- a/lib/libalpm/dload.h
> > +++ b/lib/libalpm/dload.h
> > @@ -45,6 +45,8 @@ struct dload_payload {
> >       int cb_initialized;
> >  #ifdef HAVE_LIBCURL
> >       CURL *curl;
> > +     char error_buffer[CURL_ERROR_SIZE];
> > +     FILE *localf; /* temp download file */
> >  #endif
> >  };

Will send the updated CL soon. PTAL.
Allan McRae April 14, 2020, 2:41 a.m. UTC | #4
Changes look good.  My only remaining comment is this:

On 27/3/20 6:59 am, Anatol Pomozov wrote:
>>> + * Returns -1 if an error happened
>>> + */
>>> +static int curl_multi_handle_single_done(CURLM *curlm, CURLMsg *msg, const char *localpath)
>> I have no idea what this function does from its name.
> This is a callback for mCURL event CURLMSG_DONE
> https://curl.haxx.se/libcurl/c/curl_multi_info_read.html
> We receive this event when one "curl easy" (or single) transfer is finished.
> 

To be clear, I can see what the function does from the function, but the
name is very vague.  If I saw "handle single done", I would not be any
the wiser what is going on.

How about:

curl_multi_check_finished_download()
curl_multi_download_completion_check()

or even:

curl_multi_handle_curlmsg_done()

Allan
Anatol Pomozov April 14, 2020, 4:47 a.m. UTC | #5
Hi

On Mon, Apr 13, 2020 at 7:42 PM Allan McRae <allan@archlinux.org> wrote:
>
> Changes look good.  My only remaining comment is this:
>
> On 27/3/20 6:59 am, Anatol Pomozov wrote:
> >>> + * Returns -1 if an error happened
> >>> + */
> >>> +static int curl_multi_handle_single_done(CURLM *curlm, CURLMsg *msg, const char *localpath)
> >> I have no idea what this function does from its name.
> > This is a callback for mCURL event CURLMSG_DONE
> > https://curl.haxx.se/libcurl/c/curl_multi_info_read.html
> > We receive this event when one "curl easy" (or single) transfer is finished.
> >
>
> To be clear, I can see what the function does from the function, but the
> name is very vague.  If I saw "handle single done", I would not be any
> the wiser what is going on.
>
> How about:
>
> curl_multi_check_finished_download()
> curl_multi_download_completion_check()

The names sound good to me. Let's move forward with
curl_multi_check_finished_download(). I will send updated patch in a
moment.

>
> or even:
>
> curl_multi_handle_curlmsg_done()
>
> Allan

Patch

diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 3570e234..06bce330 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -18,6 +18,7 @@ 
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <assert.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <errno.h>
@@ -271,6 +272,8 @@  static void curl_set_handle_opts(struct dload_payload *payload,
 	curl_easy_setopt(curl, CURLOPT_TCP_KEEPINTVL, 60L);
 	curl_easy_setopt(curl, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 
+	curl_easy_setopt(curl, CURLOPT_PRIVATE, (void *)payload);
+
 	_alpm_log(handle, ALPM_LOG_DEBUG, "url: %s\n", payload->fileurl);
 
 	if(payload->max_size) {
@@ -601,15 +604,365 @@  cleanup:
 	return ret;
 }
 
+/* Return 0 if retry was sucessfull, -1 otherwise */
+static int curl_multi_retry_next_server(CURLM *curlm, CURL *curl, struct dload_payload *payload)
+{
+	const char *server;
+	size_t len;
+	alpm_handle_t *handle = payload->handle;
+
+	payload->servers = payload->servers->next;
+	if(!payload->servers) {
+		return -1;
+	}
+	server = payload->servers->data;
+
+	/* regenerate a new fileurl */
+	free(payload->fileurl);
+	len = strlen(server) + strlen(payload->filepath) + 2;
+	MALLOC(payload->fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
+	snprintf(payload->fileurl, len, "%s/%s", server, payload->filepath);
+
+	if(payload->unlink_on_fail) {
+		/* we keep the file for a new retry but remove its data if any */
+		fflush(payload->localf);
+		ftruncate(fileno(payload->localf), 0);
+		fseek(payload->localf, 0, SEEK_SET);
+	}
+
+	/* Set curl with the new URL */
+	curl_easy_setopt(curl, CURLOPT_URL, payload->fileurl);
+
+	curl_multi_remove_handle(curlm, curl);
+	curl_multi_add_handle(curlm, curl);
+
+	return 0;
+}
+
+/* Returns 2 if download retry happened
+ * Returns 1 if the file is up-to-date
+ * Returns 0 if current payload is completed sucessfully
+ * Returns -1 if an error happened
+ */
+static int curl_multi_handle_single_done(CURLM *curlm, CURLMsg *msg, const char *localpath)
+{
+	alpm_handle_t *handle = NULL;
+	struct dload_payload *payload = NULL;
+	CURL *curl = msg->easy_handle;
+	CURLcode curlerr;
+	char *effective_url;
+	long timecond;
+	double remote_size, bytes_dl = 0;
+	long remote_time = -1;
+	struct stat st;
+	char hostname[HOSTNAME_SIZE];
+	int ret = -1;
+
+	curlerr = curl_easy_getinfo(curl, CURLINFO_PRIVATE, &payload);
+	assert(curlerr == CURLE_OK);
+	handle = payload->handle;
+
+	curl_gethost(payload->fileurl, hostname, sizeof(hostname));
+	curlerr = msg->data.result;
+	_alpm_log(handle, ALPM_LOG_DEBUG, "curl returned result %d from transfer\n",
+			curlerr);
+
+	/* was it a success? */
+	switch(curlerr) {
+		case CURLE_OK:
+			/* get http/ftp response code */
+			_alpm_log(handle, ALPM_LOG_DEBUG, "response code: %ld\n", payload->respcode);
+			if(payload->respcode >= 400) {
+				payload->unlink_on_fail = 1;
+				if(!payload->errors_ok) {
+					handle->pm_errno = ALPM_ERR_RETRIEVE;
+					/* non-translated message is same as libcurl */
+					snprintf(payload->error_buffer, sizeof(payload->error_buffer),
+							"The requested URL returned error: %ld", payload->respcode);
+					_alpm_log(handle, ALPM_LOG_ERROR,
+							_("failed retrieving file '%s' from %s : %s\n"),
+							payload->remote_name, hostname, payload->error_buffer);
+				}
+				if(curl_multi_retry_next_server(curlm, curl, payload) == 0) {
+					return 2;
+				} else {
+					goto cleanup;
+				}
+			}
+			break;
+		case CURLE_ABORTED_BY_CALLBACK:
+			/* handle the interrupt accordingly */
+			if(dload_interrupted == ABORT_OVER_MAXFILESIZE) {
+				curlerr = CURLE_FILESIZE_EXCEEDED;
+				payload->unlink_on_fail = 1;
+				handle->pm_errno = ALPM_ERR_LIBCURL;
+				_alpm_log(handle, ALPM_LOG_ERROR,
+						_("failed retrieving file '%s' from %s : expected download size exceeded\n"),
+						payload->remote_name, hostname);
+			}
+			goto cleanup;
+		case CURLE_COULDNT_RESOLVE_HOST:
+			payload->unlink_on_fail = 1;
+			handle->pm_errno = ALPM_ERR_SERVER_BAD_URL;
+			_alpm_log(handle, ALPM_LOG_ERROR,
+					_("failed retrieving file '%s' from %s : %s\n"),
+					payload->remote_name, hostname, payload->error_buffer);
+			if(curl_multi_retry_next_server(curlm, curl, payload) == 0) {
+				return 2;
+			} else {
+				goto cleanup;
+			}
+		default:
+			/* delete zero length downloads */
+			if(fstat(fileno(payload->localf), &st) == 0 && st.st_size == 0) {
+				payload->unlink_on_fail = 1;
+			}
+			if(!payload->errors_ok) {
+				handle->pm_errno = ALPM_ERR_LIBCURL;
+				_alpm_log(handle, ALPM_LOG_ERROR,
+						_("failed retrieving file '%s' from %s : %s\n"),
+						payload->remote_name, hostname, payload->error_buffer);
+			} else {
+				_alpm_log(handle, ALPM_LOG_DEBUG,
+						"failed retrieving file '%s' from %s : %s\n",
+						payload->remote_name, hostname, payload->error_buffer);
+			}
+			if(curl_multi_retry_next_server(curlm, curl, payload) == 0) {
+				return 2;
+			} else {
+				goto cleanup;
+			}
+	}
+
+	/* retrieve info about the state of the transfer */
+	curl_easy_getinfo(curl, CURLINFO_FILETIME, &remote_time);
+	curl_easy_getinfo(curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &remote_size);
+	curl_easy_getinfo(curl, CURLINFO_SIZE_DOWNLOAD, &bytes_dl);
+	curl_easy_getinfo(curl, CURLINFO_CONDITION_UNMET, &timecond);
+	curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &effective_url);
+
+	/* time condition was met and we didn't download anything. we need to
+	 * clean up the 0 byte .part file that's left behind. */
+	if(timecond == 1 && DOUBLE_EQ(bytes_dl, 0)) {
+		_alpm_log(handle, ALPM_LOG_DEBUG, "file met time condition\n");
+		ret = 1;
+		unlink(payload->tempfile_name);
+		goto cleanup;
+	}
+
+	/* remote_size isn't necessarily the full size of the file, just what the
+	 * server reported as remaining to download. compare it to what curl reported
+	 * as actually being transferred during curl_easy_perform() */
+	if(!DOUBLE_EQ(remote_size, -1) && !DOUBLE_EQ(bytes_dl, -1) &&
+			!DOUBLE_EQ(bytes_dl, remote_size)) {
+		_alpm_log(handle, ALPM_LOG_ERROR, _("%s appears to be truncated: %jd/%jd bytes\n"),
+				payload->remote_name, (intmax_t)bytes_dl, (intmax_t)remote_size);
+		GOTO_ERR(handle, ALPM_ERR_RETRIEVE, cleanup);
+	}
+
+	if(payload->trust_remote_name) {
+		if(payload->content_disp_name) {
+			/* content-disposition header has a better name for our file */
+			free(payload->destfile_name);
+			payload->destfile_name = get_fullpath(localpath,
+				get_filename(payload->content_disp_name), "");
+		} else {
+			const char *effective_filename = strrchr(effective_url, '/');
+			if(effective_filename && strlen(effective_filename) > 2) {
+				effective_filename++;
+
+				/* if destfile was never set, we wrote to a tempfile. even if destfile is
+				 * set, we may have followed some redirects and the effective url may
+				 * have a better suggestion as to what to name our file. in either case,
+				 * refactor destfile to this newly derived name. */
+				if(!payload->destfile_name || strcmp(effective_filename,
+							strrchr(payload->destfile_name, '/') + 1) != 0) {
+					free(payload->destfile_name);
+					payload->destfile_name = get_fullpath(localpath, effective_filename, "");
+				}
+			}
+		}
+	}
+
+	ret = 0;
+
+cleanup:
+	/* disconnect relationships from the curl handle for things that might go out
+	 * of scope, but could still be touched on connection teardown. This really
+	 * only applies to FTP transfers. */
+	curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 1L);
+	curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, (char *)NULL);
+
+	if(payload->localf != NULL) {
+		fclose(payload->localf);
+		utimes_long(payload->tempfile_name, remote_time);
+	}
+
+	if(ret == 0) {
+		if(payload->destfile_name) {
+			if(rename(payload->tempfile_name, payload->destfile_name)) {
+				_alpm_log(handle, ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"),
+						payload->tempfile_name, payload->destfile_name, strerror(errno));
+				ret = -1;
+			}
+		}
+	}
+
+	if((ret == -1 || dload_interrupted) && payload->unlink_on_fail &&
+			payload->tempfile_name) {
+		unlink(payload->tempfile_name);
+	}
+
+	// TODO: report that the download has been completed
+
+	curl_multi_remove_handle(curlm, curl);
+	curl_easy_cleanup(curl);
+	payload->curl = NULL;
+
+	FREE(payload->fileurl);
+	return ret;
+}
+
+/* Returns 0 in case if a new download transaction has been successfully started
+ * Returns -1 if am error happened while starting a new download
+ */
+static int curl_multi_add_payload(alpm_handle_t *handle, CURLM *curlm,
+		struct dload_payload *payload, const char *localpath)
+{
+	size_t len;
+	const char *server;
+	CURL *curl = NULL;
+	char hostname[HOSTNAME_SIZE];
+
+	ASSERT(payload->servers, RET_ERR(handle, ALPM_ERR_SERVER_NONE, -1));
+	server = payload->servers->data;
+
+	curl = curl_easy_init();
+	payload->curl = curl;
+
+	len = strlen(server) + strlen(payload->filepath) + 2;
+	MALLOC(payload->fileurl, len, GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));
+	snprintf(payload->fileurl, len, "%s/%s", server, payload->filepath);
+
+	payload->tempfile_openmode = "wb";
+	if(!payload->remote_name) {
+		STRDUP(payload->remote_name, get_filename(payload->fileurl),
+			GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));
+	}
+	if(curl_gethost(payload->fileurl, hostname, sizeof(hostname)) != 0) {
+		_alpm_log(handle, ALPM_LOG_ERROR, _("url '%s' is invalid\n"), payload->fileurl);
+		GOTO_ERR(handle, ALPM_ERR_SERVER_BAD_URL, cleanup);
+	}
+
+	if(payload->remote_name && strlen(payload->remote_name) > 0) {
+		payload->destfile_name = get_fullpath(localpath, payload->remote_name, "");
+		payload->tempfile_name = get_fullpath(localpath, payload->remote_name, ".part");
+		if(!payload->destfile_name || !payload->tempfile_name) {
+			goto cleanup;
+		}
+	} else {
+		/* URL doesn't contain a filename, so make a tempfile. We can't support
+		 * resuming this kind of download; partial transfers will be destroyed */
+		payload->unlink_on_fail = 1;
+
+		payload->localf = create_tempfile(payload, localpath);
+		if(payload->localf == NULL) {
+			goto cleanup;
+		}
+	}
+
+	curl_set_handle_opts(payload, curl, payload->error_buffer);
+
+	if(payload->max_size == payload->initial_size) {
+		/* .part file is complete */
+		goto cleanup;
+	}
+
+	if(payload->localf == NULL) {
+		payload->localf = fopen(payload->tempfile_name, payload->tempfile_openmode);
+		if(payload->localf == NULL) {
+			_alpm_log(handle, ALPM_LOG_ERROR,
+					_("could not open file %s: %s\n"),
+					payload->tempfile_name, strerror(errno));
+			GOTO_ERR(handle, ALPM_ERR_RETRIEVE, cleanup);
+		}
+	}
+
+	_alpm_log(handle, ALPM_LOG_DEBUG,
+			"opened tempfile for download: %s (%s)\n", payload->tempfile_name,
+			payload->tempfile_openmode);
+
+	curl_easy_setopt(curl, CURLOPT_WRITEDATA, payload->localf);
+	curl_multi_add_handle(curlm, curl);
+	return 0;
+
+cleanup:
+	FREE(payload->fileurl);
+	FREE(payload->tempfile_name);
+	FREE(payload->destfile_name);
+	FREE(payload->content_disp_name);
+	curl_easy_cleanup(curl);
+	return -1;
+}
+
 static int curl_multi_download_internal(alpm_handle_t *handle,
 		alpm_list_t *payloads /* struct dload_payload */,
 		const char *localpath)
 {
-	(void)handle;
-	(void)payloads;
-	(void)localpath;
-	return 0;
+	int still_running = 0;
+	int err = 0;
+	int parallel_downloads = handle->parallel_downloads;
+
+	CURLM *curlm = handle->curlm;
+	CURLMsg *msg;
+
+	while(still_running || payloads) {
+		int msgs_left = -1;
+
+		for(; still_running < parallel_downloads && payloads; still_running++) {
+			struct dload_payload *payload = payloads->data;
+
+			if(curl_multi_add_payload(handle, curlm, payloads->data, localpath) == 0) {
+				// TODO: report that download has started
+				payloads = payloads->next;
+			} else {
+				// the payload failed to start, do not start any new downloads just wait until
+				// active one complete.
+				_alpm_log(handle, ALPM_LOG_ERROR, "failed to setup a download payload for %s\n", payload->remote_name);
+				payloads = NULL;
+				err = -1;
+			}
+		}
+
+		CURLMcode mc = curl_multi_perform(curlm, &still_running);
+
+		if(mc != CURLM_OK) {
+			_alpm_log(handle, ALPM_LOG_ERROR, "curl returned error %d from transfer\n", mc);
+			payloads = NULL;
+			err = -1;
+		}
+
+		while((msg = curl_multi_info_read(curlm, &msgs_left))) {
+			if(msg->msg == CURLMSG_DONE) {
+				int done_code = curl_multi_handle_single_done(curlm, msg, localpath);
+				if(done_code == 2) {
+					/* in case of a retry increase the counter of active requests
+					 * to avoid exiting the loop early
+					 */
+					still_running++;
+				}
+			} else {
+				_alpm_log(handle, ALPM_LOG_ERROR, "curl curl_multi_info_read error %d\n", msg->msg);
+			}
+		}
+		if(still_running) {
+			curl_multi_wait(curlm, NULL, 0, 1000, NULL);
+		}
+	}
+
+	return err;
 }
+
 #endif
 
 /** Download a file given by a URL to a local directory.
diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h
index e87b6a93..a40b51b7 100644
--- a/lib/libalpm/dload.h
+++ b/lib/libalpm/dload.h
@@ -45,6 +45,8 @@  struct dload_payload {
 	int cb_initialized;
 #ifdef HAVE_LIBCURL
 	CURL *curl;
+	char error_buffer[CURL_ERROR_SIZE];
+	FILE *localf; /* temp download file */
 #endif
 };