From patchwork Thu Sep 29 04:56:33 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sam Mendoza-Jonas X-Patchwork-Id: 676509 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3sl2Nd1dfdz9sXR for ; Thu, 29 Sep 2016 14:56:49 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=mendozajonas.com header.i=@mendozajonas.com header.b=NrWuxVWE; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3sl2Nd0dQdzDsht for ; Thu, 29 Sep 2016 14:56:49 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=mendozajonas.com header.i=@mendozajonas.com header.b=NrWuxVWE; dkim-atps=neutral X-Original-To: petitboot@lists.ozlabs.org Delivered-To: petitboot@lists.ozlabs.org Received: from mendozajonas.com (mendozajonas.com [188.166.185.233]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3sl2NW6Fb2zDsff for ; Thu, 29 Sep 2016 14:56:43 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=mendozajonas.com header.i=@mendozajonas.com header.b=NrWuxVWE; dkim-atps=neutral Received: from skellige.ozlabs.ibm.com (unknown [122.99.82.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: sam@mendozajonas.com) by mendozajonas.com (Postfix) with ESMTPSA id 60061143F66; Thu, 29 Sep 2016 12:56:40 +0800 (SGT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mendozajonas.com; s=mail; t=1475125000; bh=FiyFcJZSsBv7qMx4euZgbDWY2YXkEB8CLZVQ01nK1nc=; h=From:To:Cc:Subject:Date:From; b=NrWuxVWEF4R9DhDEJf7JyRcACHaLyvYvvZCybJe9wckuPrWNodTZVX4MipdhxJJAd 53+ufeCFG5wZAQaULvFgoDwtOz0kHex+AK5kMy5pszPbamkDr77Hp7fTNiKZPeD7cA YRHhMw7JYXv8bFC+xTSouf0PdPP86PqfYb4jho8s= From: Samuel Mendoza-Jonas To: petitboot@lists.ozlabs.org Subject: [PATCH] discover:pxe-parser: Parse only the first config Date: Thu, 29 Sep 2016 14:56:33 +1000 Message-Id: <20160929045633.27662-1-sam@mendozajonas.com> X-Mailer: git-send-email 2.10.0 X-BeenThere: petitboot@lists.ozlabs.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Petitboot bootloader development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Samuel Mendoza-Jonas MIME-Version: 1.0 Errors-To: petitboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Petitboot" Commit 2163af5 "discover/pxe-parser: Retrieve configs asynchronously" added asynchronous loading of remote pxe filenames, but made an unintended change in behaviour to the PXE parser. Previously the parser would try a list of possible filenames, and parse the first one it found. However the above commit spawns an asynchronous job for every filename, and parses any that can be retrieved. It is a common configuration to have a machine-specific config and a 'fallback' default config, and the change means we could erroneously retrieve and parse both configs. Update the PXE parser so that asynchronous jobs are spawned sequentially. That is, spawn a job for the first filename and if not successful spawn another job for the next filename, and so on. Once a remote config is successfully retrieved, parse it and stop. Signed-off-by: Samuel Mendoza-Jonas --- discover/pxe-parser.c | 104 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 70 insertions(+), 34 deletions(-) diff --git a/discover/pxe-parser.c b/discover/pxe-parser.c index 4aae8b1..9cdd89c 100644 --- a/discover/pxe-parser.c +++ b/discover/pxe-parser.c @@ -22,9 +22,14 @@ static const char *pxelinux_prefix = "pxelinux.cfg/"; +static void pxe_conf_parse_cb(struct load_url_result *result, void *data); + struct pxe_parser_info { - struct discover_boot_option *opt; - const char *default_name; + struct discover_boot_option *opt; + const char *default_name; + char **pxe_conf_files; + struct pb_url *pxe_base_url; + int current; }; static void pxe_finish(struct conf_context *conf) @@ -206,6 +211,38 @@ static void pxe_process_pair(struct conf_context *ctx, } +static void pxe_load_next_filename(struct conf_context *conf) +{ + struct pxe_parser_info *info = conf->parser_info; + struct load_url_result *result; + struct pb_url *url; + + if (!info->pxe_conf_files) + return; + + for (; info->pxe_conf_files[info->current]; info->current++) { + url = pb_url_join(conf->dc, info->pxe_base_url, + info->pxe_conf_files[info->current]); + if (!url) + continue; + + result = load_url_async(conf, url, pxe_conf_parse_cb, conf); +#ifdef PETITBOOT_TEST + /* + * load_url_async() is synchronous in tests, which means + * pxe_conf_parse_cb() will have been recursively called until + * either a success or all filenames have been exhausted. + */ + break; +#endif + if (result && (result->status == LOAD_OK || + result->status == LOAD_ASYNC)) + break; + } + + return; +} + /* * Callback for asynchronous loads from pxe_parse() * @param result Result of load_url_async() @@ -216,22 +253,35 @@ static void pxe_conf_parse_cb(struct load_url_result *result, void *data) struct conf_context *conf = data; struct device_handler *handler; struct boot_status status = {0}; + struct pxe_parser_info *info; char *buf = NULL; - int len, rc; + int len, rc = 0; if (!data) return; - if (!result || result->status != LOAD_OK) { - talloc_free(conf); + if (result && result->status == LOAD_OK) + rc = read_file(conf, result->local, &buf, &len); + if (!result || result->status != LOAD_OK || rc) { + /* This load failed so try the next available filename */ + info = conf->parser_info; + if (!info->pxe_conf_files) + return; + + info->current++; + pxe_load_next_filename(conf); + if (info->pxe_conf_files[info->current] == NULL) { + /* Nothing left to try */ + goto out_clean; + } return; } - rc = read_file(conf, result->local, &buf, &len); - if (rc) { - pb_log("Read failed during pxe callback for %s\n", result->local); - goto out_clean; - } + /* + * Parse the first successfully downloaded file. We only want to parse + * the first because otherwise we could parse options from both a + * machine-specific config and a 'fallback' default config + */ conf_parse_buf(conf, buf, len); @@ -295,11 +345,12 @@ static struct conf_context *copy_context(void *ctx, struct discover_context *dc) static int pxe_parse(struct discover_context *dc) { - struct pb_url *pxe_base_url, *url; - char **pxe_conf_files, **filename; + struct pb_url *pxe_base_url; struct conf_context *conf = NULL; struct load_url_result *result; void *ctx = talloc_parent(dc); + struct pxe_parser_info *info; + char **pxe_conf_files; bool complete_url; /* Expects dhcp event parameters to support network boot */ @@ -314,9 +365,9 @@ static int pxe_parse(struct discover_context *dc) * Retrieving PXE configs over the network can take some time depending * on factors such as slow network, malformed paths, bad DNS, and * overzealous firewalls. Instead of blocking the discover server while - * we wait for these, spawn an asynchronous job for each URL we can - * parse and process the resulting files in a callback. A separate - * conf_context is created for each job. + * we wait for these, spawn an asynchronous job that will attempt to + * retrieve each possible URL until it successfully finds one, and + * parse and process the resulting file in a callback. */ conf = copy_context(ctx, dc); if (!conf) @@ -340,26 +391,11 @@ static int pxe_parse(struct discover_context *dc) if (!pxe_base_url) goto out_pxe_conf; - for (filename = pxe_conf_files; *filename; filename++) { - if (!conf) { - conf = copy_context(ctx, dc); - } - url = pb_url_join(conf->dc, pxe_base_url, *filename); - if (!url) - continue; - result = load_url_async(conf, url, pxe_conf_parse_cb, - conf); - if (!result) { - pb_log("load_url_async fails for %s\n", - conf->dc->conf_url->path); - talloc_free(conf); - } - /* conf now needed by callback, don't reuse */ - conf = NULL; - } + info = conf->parser_info; + info->pxe_conf_files = pxe_conf_files; + info->pxe_base_url = pxe_base_url; - talloc_free(pxe_base_url); - talloc_free(pxe_conf_files); + pxe_load_next_filename(conf); } return 0;