From 227620704107a19c03824ae146249fff4a939839 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Tue, 24 Sep 2013 11:03:11 +0800 Subject: [PATCH] discover: Add struct load_url_result and load_task Currently, load_url and load_url_async return a filename parameter and a tempfile flag (indicating whether the file needs to be cleaned after use). Instead, encapsulate this data in a struct load_url_result, which the caller (and async callbacks) can read the status, filename and clean parameters. For internal use in load_url and helpers, we add a struct load_task to hold a pointer to the load_url_result and async data. Signed-off-by: Jeremy Kerr --- discover/boot.c | 121 ++++++------- discover/parser.c | 13 +- discover/paths.c | 434 +++++++++++++++++++++------------------------- discover/paths.h | 26 ++- 4 files changed, 287 insertions(+), 307 deletions(-) diff --git a/discover/boot.c b/discover/boot.c index d0b57b5..2f381a7 100644 --- a/discover/boot.c +++ b/discover/boot.c @@ -38,14 +38,11 @@ enum boot_process_state { struct boot_task { - char *local_image; - char *local_initrd; - char *local_dtb; - char *args; - unsigned int clean_image; - unsigned int clean_initrd; - unsigned int clean_dtb; - struct pb_url *image, *initrd, *dtb; + struct load_url_result *image; + struct load_url_result *initrd; + struct load_url_result *dtb; + const char *args; + struct pb_url *image_url, *initrd_url, *dtb_url; boot_status_fn status_fn; void *status_arg; enum boot_process_state state; @@ -68,16 +65,16 @@ static int kexec_load(struct boot_task *boot_task) *p++ = pb_system_apps.kexec; /* 1 */ *p++ = "-l"; /* 2 */ - if (boot_task->local_initrd) { + if (boot_task->initrd) { s_initrd = talloc_asprintf(boot_task, "--initrd=%s", - boot_task->local_initrd); + boot_task->initrd->local); assert(s_initrd); *p++ = s_initrd; /* 3 */ } - if (boot_task->local_dtb) { + if (boot_task->dtb) { s_dtb = talloc_asprintf(boot_task, "--dtb=%s", - boot_task->local_dtb); + boot_task->dtb->local); assert(s_dtb); *p++ = s_dtb; /* 4 */ } @@ -89,7 +86,7 @@ static int kexec_load(struct boot_task *boot_task) *p++ = s_args; /* 5 */ } - *p++ = boot_task->local_image; /* 6 */ + *p++ = boot_task->image->local; /* 6 */ *p++ = NULL; /* 7 */ result = process_run_simple_argv(boot_task, argv); @@ -160,11 +157,11 @@ static void boot_hook_update_param(void *ctx, struct boot_task *task, { struct p { const char *name; - char **p; + const char **p; } *param, params[] = { - { "boot_image", &task->local_image }, - { "boot_initrd", &task->local_initrd }, - { "boot_dtb", &task->local_dtb }, + { "boot_image", &task->image->local }, + { "boot_initrd", &task->initrd->local }, + { "boot_dtb", &task->dtb->local }, { "boot_args", &task->args }, { NULL, NULL }, }; @@ -212,11 +209,11 @@ static void boot_hook_setenv(struct boot_task *task) unsetenv("boot_dtb"); unsetenv("boot_args"); - setenv("boot_image", task->local_image, 1); - if (task->local_initrd) - setenv("boot_initrd", task->local_initrd, 1); - if (task->local_dtb) - setenv("boot_dtb", task->local_dtb, 1); + setenv("boot_image", task->image->local, 1); + if (task->initrd) + setenv("boot_initrd", task->initrd->local, 1); + if (task->dtb) + setenv("boot_dtb", task->dtb->local, 1); if (task->args) setenv("boot_args", task->args, 1); } @@ -291,17 +288,28 @@ static void run_boot_hooks(struct boot_task *task) free(hooks); } -static void boot_process(void *ctx, int status) +static void cleanup_load(struct load_url_result *result) { - struct boot_task *task = ctx; - int result = -1; + if (!result) + return; + if (result->status != LOAD_OK) + return; + if (!result->cleanup_local) + return; + unlink(result->local); +} + +static void boot_process(struct load_url_result *result, void *data) +{ + struct boot_task *task = data; + int rc = -1; if (task->state == BOOT_STATE_INITIAL) { update_status(task->status_fn, task->status_arg, BOOT_STATUS_INFO, "loading kernel"); - task->local_image = load_url_async(task, task->image, - &task->clean_image, boot_process); - if (!task->local_image) { + task->image = load_url_async(task, task->image_url, + boot_process, task); + if (!task->image) { update_status(task->status_fn, task->status_arg, BOOT_STATUS_ERROR, "Couldn't load kernel image"); @@ -312,7 +320,7 @@ static void boot_process(void *ctx, int status) } if (task->state == BOOT_STATE_IMAGE_LOADING) { - if (status) { + if (result->status == LOAD_ERROR) { update_status(task->status_fn, task->status_arg, BOOT_STATUS_ERROR, "Error loading kernel image"); @@ -320,12 +328,12 @@ static void boot_process(void *ctx, int status) } task->state = BOOT_STATE_INITRD_LOADING; - if (task->initrd) { + if (task->initrd_url) { update_status(task->status_fn, task->status_arg, BOOT_STATUS_INFO, "loading initrd"); - task->local_initrd = load_url_async(task, task->initrd, - &task->clean_initrd, boot_process); - if (!task->local_initrd) { + task->initrd = load_url_async(task, task->initrd_url, + boot_process, task); + if (!task->initrd) { update_status(task->status_fn, task->status_arg, BOOT_STATUS_ERROR, "Couldn't load initrd image"); @@ -336,7 +344,7 @@ static void boot_process(void *ctx, int status) } if (task->state == BOOT_STATE_INITRD_LOADING) { - if (status) { + if (result->status) { update_status(task->status_fn, task->status_arg, BOOT_STATUS_ERROR, "Error loading initrd"); @@ -344,13 +352,13 @@ static void boot_process(void *ctx, int status) } task->state = BOOT_STATE_DTB_LOADING; - if (task->dtb) { + if (task->dtb_url) { update_status(task->status_fn, task->status_arg, BOOT_STATUS_INFO, "loading device tree"); - task->local_dtb = load_url_async(task, task->dtb, - &task->clean_dtb, boot_process); - if (!task->local_dtb) { + task->dtb = load_url_async(task, task->dtb_url, + boot_process, task); + if (!task->dtb) { update_status(task->status_fn, task->status_arg, BOOT_STATUS_ERROR, "Couldn't load device tree"); @@ -361,7 +369,7 @@ static void boot_process(void *ctx, int status) } if (task->state == BOOT_STATE_DTB_LOADING) { - if (status) { + if (result->status) { update_status(task->status_fn, task->status_arg, BOOT_STATUS_ERROR, "Error loading dtb"); @@ -381,29 +389,24 @@ static void boot_process(void *ctx, int status) update_status(task->status_fn, task->status_arg, BOOT_STATUS_INFO, "performing kexec_load"); - result = kexec_load(task); - - if (result) { + rc = kexec_load(task); + if (rc) { update_status(task->status_fn, task->status_arg, BOOT_STATUS_ERROR, "kexec load failed"); } no_load: - if (task->clean_image) - unlink(task->local_image); - if (task->clean_initrd) - unlink(task->local_initrd); - if (task->clean_dtb) - unlink(task->local_dtb); - - if (!result) { + cleanup_load(task->image); + cleanup_load(task->initrd); + cleanup_load(task->dtb); + + if (!rc) { update_status(task->status_fn, task->status_arg, BOOT_STATUS_INFO, "performing kexec reboot"); - result = kexec_reboot(task); - - if (result) { + rc = kexec_reboot(task); + if (rc) { update_status(task->status_fn, task->status_arg, BOOT_STATUS_ERROR, "kexec reboot failed"); @@ -442,22 +445,22 @@ int boot(void *ctx, struct discover_boot_option *opt, struct boot_command *cmd, } boot_task = talloc_zero(ctx, struct boot_task); - boot_task->image = image; + boot_task->image_url = image; boot_task->dry_run = dry_run; boot_task->status_fn = status_fn; boot_task->status_arg = status_arg; boot_task->state = BOOT_STATE_INITIAL; if (cmd && cmd->initrd_file) { - boot_task->initrd = pb_url_parse(opt, cmd->initrd_file); + boot_task->initrd_url = pb_url_parse(opt, cmd->initrd_file); } else if (opt && opt->initrd) { - boot_task->initrd = opt->initrd->url; + boot_task->initrd_url = opt->initrd->url; } if (cmd && cmd->dtb_file) { - boot_task->dtb = pb_url_parse(opt, cmd->dtb_file); + boot_task->dtb_url = pb_url_parse(opt, cmd->dtb_file); } else if (opt && opt->dtb) { - boot_task->dtb = opt->dtb->url; + boot_task->dtb_url = opt->dtb->url; } if (cmd && cmd->boot_args) { @@ -469,7 +472,7 @@ int boot(void *ctx, struct discover_boot_option *opt, struct boot_command *cmd, boot_task->args = NULL; } - boot_process(boot_task, 0); + boot_process(NULL, boot_task); return 0; } diff --git a/discover/parser.c b/discover/parser.c index 7d9cd0d..a304f0c 100644 --- a/discover/parser.c +++ b/discover/parser.c @@ -29,23 +29,22 @@ static char *local_path(struct discover_context *ctx, static int download_config(struct discover_context *ctx, char **buf, int *len) { - unsigned tempfile; - const char *file; + struct load_url_result *result; int rc; - file = load_url(ctx, ctx->conf_url, &tempfile); - if (!file) + result = load_url(ctx, ctx->conf_url); + if (!result) return -1; - rc = read_file(ctx, file, buf, len); + rc = read_file(ctx, result->local, buf, len); if (rc) goto out_clean; return 0; out_clean: - if (tempfile) - unlink(file); + if (result->cleanup_local) + unlink(result->local); return -1; } diff --git a/discover/paths.c b/discover/paths.c index 82a82b1..b163e45 100644 --- a/discover/paths.c +++ b/discover/paths.c @@ -14,9 +14,13 @@ #define DEVICE_MOUNT_BASE (LOCAL_STATE_DIR "/petitboot/mnt") -struct load_url_async_data { - load_url_callback url_cb; - void *ctx; +struct load_task { + struct pb_url *url; + struct process *process; + struct load_url_result *result; + bool async; + load_url_complete async_cb; + void *async_data; }; const char *mount_base(void) @@ -54,136 +58,147 @@ static char *local_name(void *ctx) return ret; } -static void load_url_exit_cb(struct process *process) +static void load_url_result_cleanup_local(struct load_url_result *result) { - struct load_url_async_data *url_data = process->data; + if (result->cleanup_local) + unlink(result->local); +} - pb_log("The download client '%s' [pid %d] exited, rc %d\n", - process->path, process->pid, process->exit_status); +static void load_url_process_exit(struct process *process) +{ + struct load_task *task = process->data; + struct load_url_result *result; + load_url_complete cb; + void *data; + + pb_debug("The download client '%s' [pid %d, url %s] exited, rc %d\n", + process->path, process->pid, task->url->full, + process->exit_status); + + result = task->result; + data = task->async_data; + cb = task->async_cb; + + result->status = process->exit_status == 0 ? LOAD_OK : LOAD_ERROR; + if (result->status == LOAD_ERROR) + load_url_result_cleanup_local(result); + + /* The load callback may well free the ctx, which was the + * talloc parent of the task. Therefore, we want to do our cleanup + * before invoking it + */ + process_release(process); + talloc_free(task); - url_data->url_cb(url_data->ctx, process->exit_status); + cb(result, data); +} - process_release(process); +static void load_process_to_local_file(struct load_task *task, + const char **argv, int argv_local_idx) +{ + int rc; + + task->result->local = local_name(task->result); + if (!task->result->local) { + task->result->status = LOAD_ERROR; + return; + } + task->result->cleanup_local = true; + + if (argv_local_idx) + argv[argv_local_idx] = task->result->local; + + task->process->argv = argv; + task->process->path = argv[0]; + + if (task->async) { + rc = process_run_async(task->process); + if (rc) { + process_release(task->process); + task->process = NULL; + } + task->result->status = rc ? LOAD_ERROR : LOAD_ASYNC; + } else { + rc = process_run_sync(task->process); + task->result->status = rc ? LOAD_ERROR : LOAD_OK; + process_release(task->process); + task->process = NULL; + } } /** - * pb_load_nfs - Mounts the NFS export and returns the local file path. - * - * Returns the local file path in a talloc'ed character string on success, - * or NULL on error. + * pb_load_nfs - Create a mountpoint, set the local file within that + * mountpoint, and run the appropriate mount command */ -static char *load_nfs(void *ctx, struct pb_url *url, - struct load_url_async_data *url_data) + +static void load_nfs(struct load_task *task) { - char *local, *opts; - int result; - struct process *process; + char *mountpoint, *opts; + int rc; const char *argv[] = { pb_system_apps.mount, "-t", "nfs", - NULL, - url->host, - url->dir, - NULL, + NULL, /* 3: opts */ + task->url->host, + task->url->dir, + NULL, /* 6: mountpoint */ NULL, }; - local = local_name(ctx); - if (!local) - return NULL; - argv[6] = local; + task->result->status = LOAD_ERROR; + mountpoint = local_name(task->result); + if (!mountpoint) + return; + task->result->cleanup_local = true; + argv[6] = mountpoint; - result = pb_mkdir_recursive(local); - if (result) - goto fail; + rc = pb_mkdir_recursive(mountpoint); + if (rc) + return; opts = talloc_strdup(NULL, "ro,nolock,nodiratime"); argv[3] = opts; - if (url->port) - opts = talloc_asprintf_append(opts, ",port=%s", url->port); + if (task->url->port) + opts = talloc_asprintf_append(opts, ",port=%s", + task->url->port); - if (url_data) { - process = process_create(ctx); + task->result->local = talloc_asprintf(task->result, "%s/%s", + mountpoint, + task->url->path); - process->path = pb_system_apps.mount; - process->argv = argv; - process->exit_cb = load_url_exit_cb; - process->data = url_data; + task->process->path = pb_system_apps.mount; + task->process->argv = argv; - result = process_run_async(process); - if (result) - process_release(process); + if (task->async) { + rc = process_run_async(task->process); + if (rc) { + process_release(task->process); + task->process = NULL; + } + task->result->status = rc ? LOAD_ERROR : LOAD_ASYNC; } else { - result = process_run_simple_argv(ctx, argv); + rc = process_run_sync(task->process); + task->result->status = rc ? LOAD_ERROR : LOAD_OK; + process_release(task->process); + task->process = NULL; } talloc_free(opts); - - if (result) - goto fail; - - local = talloc_asprintf_append(local, "/%s", url->path); - pb_log("%s: local '%s'\n", __func__, local); - - return local; - -fail: - pb_rmdir_recursive("/", local); - talloc_free(local); - return NULL; } -/** - * pb_load_sftp - Loads a remote file via sftp and returns the local file path. - * - * Returns the local file path in a talloc'ed character string on success, - * or NULL on error. - */ -static char *load_sftp(void *ctx, struct pb_url *url, - struct load_url_async_data *url_data) +static void load_sftp(struct load_task *task) { - char *host_path, *local; - int result; - struct process *process; const char *argv[] = { pb_system_apps.sftp, - NULL, - NULL, + NULL, /* 1: host:path */ + NULL, /* 2: local file */ NULL, }; - local = local_name(ctx); - if (!local) - return NULL; - argv[2] = local; - - host_path = talloc_asprintf(local, "%s:%s", url->host, url->path); - argv[1] = host_path; - - if (url_data) { - process = process_create(ctx); - - process->path = pb_system_apps.sftp; - process->argv = argv; - process->exit_cb = load_url_exit_cb; - process->data = url_data; - - result = process_run_async(process); - if (result) - process_release(process); - } else { - result = process_run_simple_argv(ctx, argv); - } - - if (result) - goto fail; - - return local; - -fail: - talloc_free(local); - return NULL; + argv[1] = talloc_asprintf(task, "%s:%s", + task->url->host, task->url->path); + load_process_to_local_file(task, argv, 2); } static enum tftp_type check_tftp_type(void *ctx) @@ -221,79 +236,45 @@ static enum tftp_type check_tftp_type(void *ctx) return type; } -/** - * pb_load_tftp - Loads a remote file via tftp and returns the local file path. - * - * Returns the local file path in a talloc'ed character string on success, - * or NULL on error. - */ - -static char *load_tftp(void *ctx, struct pb_url *url, - struct load_url_async_data *url_data) +static void load_tftp(struct load_task *task) { - int result; - const char *argv[10]; - const char **p; - char *local; - struct process *process; - - if (tftp_type == TFTP_TYPE_UNKNOWN) - tftp_type = check_tftp_type(ctx); + const char *port = "69"; + const char *argv[10] = { + pb_system_apps.tftp, + }; - if (tftp_type == TFTP_TYPE_BROKEN) - return NULL; + if (task->url->port) + port = task->url->port; - local = local_name(ctx); - if (!local) - return NULL; + if (tftp_type == TFTP_TYPE_UNKNOWN) + tftp_type = check_tftp_type(task); if (tftp_type == TFTP_TYPE_BUSYBOX) { - /* first try busybox tftp args */ - - p = argv; - *p++ = pb_system_apps.tftp; /* 1 */ - *p++ = "-g"; /* 2 */ - *p++ = "-l"; /* 3 */ - *p++ = local; /* 4 */ - *p++ = "-r"; /* 5 */ - *p++ = url->path; /* 6 */ - *p++ = url->host; /* 7 */ - if (url->port) - *p++ = url->port; /* 8 */ - *p++ = NULL; /* 9 */ - } else { - p = argv; - *p++ = pb_system_apps.tftp; /* 1 */ - *p++ = "-m"; /* 2 */ - *p++ = "binary"; /* 3 */ - *p++ = url->host; /* 4 */ - if (url->port) - *p++ = url->port; /* 5 */ - *p++ = "-c"; /* 6 */ - *p++ = "get"; /* 7 */ - *p++ = url->path; /* 8 */ - *p++ = local; /* 9 */ - *p++ = NULL; /* 10 */ - } - - if (url_data) { - process = process_create(ctx); - - process->path = pb_system_apps.tftp; - process->argv = argv; - process->exit_cb = load_url_exit_cb; - process->data = url_data; - - result = process_run_async(process); - } else { - result = process_run_simple_argv(ctx, argv); - } - - if (!result) - return local; - - talloc_free(local); - return NULL; + argv[1] = "-g"; + argv[2] = "-l"; + argv[3] = NULL; /* 3: local file */ + argv[4] = "-r"; + argv[5] = task->url->path; + argv[6] = task->url->host; + argv[7] = port; + argv[8] = NULL; + + load_process_to_local_file(task, argv, 3); + + } else if (tftp_type == TFTP_TYPE_HPA) { + argv[1] = "-m"; + argv[2] = "binary"; + argv[3] = task->url->host; + argv[4] = port; + argv[5] = "-c"; + argv[6] = "get"; + argv[7] = task->url->path; + argv[8] = NULL; /* 8: local file */ + argv[9] = NULL; + load_process_to_local_file(task, argv, 8); + + } else + task->result->status = LOAD_ERROR; } enum wget_flags { @@ -308,55 +289,28 @@ enum wget_flags { * or NULL on error. */ -static char *load_wget(void *ctx, struct pb_url *url, enum wget_flags flags, - struct load_url_async_data *url_data) +static void load_wget(struct load_task *task, int flags) { - int result; - const char *argv[7]; - const char **p; - char *local; - struct process *process; - - local = local_name(ctx); - - if (!local) - return NULL; + const char *argv[] = { + pb_system_apps.wget, + "-O", + NULL, /* 2: local file */ + NULL, + NULL, + NULL, + }; + int i; - p = argv; - *p++ = pb_system_apps.wget; /* 1 */ + i = 3; #if !defined(DEBUG) - *p++ = "--quiet"; /* 2 */ + argv[i++] = "--quiet"; #endif - *p++ = "-O"; /* 3 */ - *p++ = local; /* 4 */ - *p++ = url->full; /* 5 */ if (flags & wget_no_check_certificate) - *p++ = "--no-check-certificate"; /* 6 */ - *p++ = NULL; /* 7 */ + argv[i++] = "--no-check-certificate"; - if (url_data) { - process = process_create(ctx); + argv[i] = task->url->full; - process->path = pb_system_apps.wget; - process->argv = argv; - process->exit_cb = load_url_exit_cb; - process->data = url_data; - - result = process_run_async(process); - if (result) - process_release(process); - } else { - result = process_run_simple_argv(ctx, argv); - } - - if (result) - goto fail; - - return local; - -fail: - talloc_free(local); - return NULL; + load_process_to_local_file(task, argv, 2); } /** @@ -373,60 +327,66 @@ fail: * or NULL on error. */ -char *load_url_async(void *ctx, struct pb_url *url, unsigned int *tempfile, - load_url_callback url_cb) +struct load_url_result *load_url_async(void *ctx, struct pb_url *url, + load_url_complete async_cb, void *async_data) { - char *local; - int tmp = 0; - struct load_url_async_data *url_data; + struct load_url_result *result; + struct load_task *task; if (!url) return NULL; - url_data = NULL; - - if (url_cb) { - url_data = talloc_zero(ctx, struct load_url_async_data); - url_data->url_cb = url_cb; - url_data->ctx = ctx; + task = talloc_zero(ctx, struct load_task); + task->url = url; + task->async = async_cb != NULL; + task->result = talloc_zero(ctx, struct load_url_result); + task->process = process_create(task); + if (task->async) { + task->async_cb = async_cb; + task->async_data = async_data; + task->process->exit_cb = load_url_process_exit; + task->process->data = task; } switch (url->scheme) { case pb_url_ftp: case pb_url_http: - local = load_wget(ctx, url, 0, url_data); - tmp = !!local; + load_wget(task, 0); break; case pb_url_https: - local = load_wget(ctx, url, wget_no_check_certificate, - url_data); - tmp = !!local; + load_wget(task, wget_no_check_certificate); break; case pb_url_nfs: - local = load_nfs(ctx, url, url_data); - tmp = !!local; + load_nfs(task); break; case pb_url_sftp: - local = load_sftp(ctx, url, url_data); - tmp = !!local; + load_sftp(task); break; case pb_url_tftp: - local = load_tftp(ctx, url, url_data); - tmp = !!local; + load_tftp(task); break; default: - local = talloc_strdup(ctx, url->full); - tmp = 0; + task->result->local = talloc_strdup(task->result, + url->path); + task->result->status = LOAD_OK; break; } - if (tempfile) - *tempfile = tmp; + result = task->result; + if (result->status == LOAD_ERROR) { + load_url_result_cleanup_local(task->result); + talloc_free(result); + talloc_free(task); + return NULL; + } + + if (!task->async) + talloc_free(task); - return local; + return result; } -char *load_url(void *ctx, struct pb_url *url, unsigned int *tempfile) +struct load_url_result *load_url(void *ctx, struct pb_url *url) { - return load_url_async(ctx, url, tempfile, NULL); + return load_url_async(ctx, url, NULL, NULL); } diff --git a/discover/paths.h b/discover/paths.h index 2f52e82..e905094 100644 --- a/discover/paths.h +++ b/discover/paths.h @@ -16,12 +16,30 @@ char *join_paths(void *alloc_ctx, const char *a, const char *b); */ const char *mount_base(void); -typedef void (*load_url_callback)(void *data, int status); + +struct load_url_result { + enum { + LOAD_OK, /* load complete. other members should only be + accessed if status == LOAD_OK */ + + LOAD_ERROR, /* only signalled to async loaders + * (sync will see a NULL result) */ + + LOAD_ASYNC, /* async load still in progress */ + } status; + const char *local; + bool cleanup_local; +}; + +/* callback type for asynchronous loads. The callback implementation is + * responsible for freeing result. + */ +typedef void (*load_url_complete)(struct load_url_result *result, void *data); /* Load a (potentially remote) file, and return a guaranteed-local name */ -char *load_url_async(void *ctx, struct pb_url *url, unsigned int *tempfile, - load_url_callback url_cb); +struct load_url_result *load_url_async(void *ctx, struct pb_url *url, + load_url_complete complete, void *data); -char *load_url(void *ctx, struct pb_url *url, unsigned int *tempfile); +struct load_url_result *load_url(void *ctx, struct pb_url *url); #endif /* PATHS_H */ -- 2.39.2