discover/pxe-parser: Retrieve configs asynchronously
authorSamuel Mendoza-Jonas <sam@mendozajonas.com>
Mon, 16 May 2016 06:06:30 +0000 (16:06 +1000)
committerSamuel Mendoza-Jonas <sam@mendozajonas.com>
Tue, 28 Jun 2016 06:17:21 +0000 (16:17 +1000)
Depending on the configuration of the DHCP server and the network, tftp
requests made by the pxe parser can timeout. The pxe parser makes these
requests synchronously so several timeouts can block the server
completely for several minutes, leaving the server unresponsive to UI
requests.

Rework the pxe parser such that it handles the result of each tftp
request in a callback, which can complete after iterate_parsers() has
returned. Each callback is allocated its own conf_context which takes a
talloc reference on the discover_context so that each callback can
commit new boot options after the initial iterate loop has completed.
This also means talloc_unlink must be used instead by the original
parent of the discover_context.

Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
discover/device-handler.c
discover/pxe-parser.c
lib/talloc/talloc.c
lib/talloc/talloc.h
lib/url/url.c
lib/url/url.h

index 9fd9fab5cf5f60db1b8c372b7401cc07b511b176..cd4e3acba1bd0814f378771232a337309d391a2d 100644 (file)
@@ -861,7 +861,7 @@ out:
        device_handler_boot_status(handler, status);
 
        talloc_free(status);
-       talloc_free(ctx);
+       talloc_unlink(handler, ctx);
 
        return 0;
 }
@@ -900,7 +900,7 @@ int device_handler_dhcp(struct device_handler *handler,
        device_handler_boot_status(handler, status);
 
        talloc_free(status);
-       talloc_free(ctx);
+       talloc_unlink(handler, ctx);
 
        return 0;
 }
@@ -930,7 +930,7 @@ int device_handler_conf(struct device_handler *handler,
        device_handler_boot_status(handler, status);
 
        talloc_free(status);
-       talloc_free(ctx);
+       talloc_unlink(handler, ctx);
 
        return 0;
 }
@@ -1162,7 +1162,7 @@ void device_handler_process_url(struct device_handler *handler,
 
        device_handler_discover_context_commit(handler, ctx);
 
-       talloc_free(ctx);
+       talloc_unlink(handler, ctx);
 
        status->type = BOOT_STATUS_INFO;
        status->message = talloc_asprintf(status, _("Config file %s parsed"),
index 4812c374d1991b74ba938e9fd935154982a6b3d3..4481e5da9f7c9709d31cea7e00e29709cf203d38 100644 (file)
@@ -8,6 +8,8 @@
 #include <talloc/talloc.h>
 #include <url/url.h>
 #include <log/log.h>
+#include <file/file.h>
+#include <i18n/i18n.h>
 
 #include "parser.h"
 #include "parser-conf.h"
@@ -197,87 +199,168 @@ static void pxe_process_pair(struct conf_context *ctx,
 
 }
 
-static int pxe_parse(struct discover_context *dc)
+/*
+ * Callback for asynchronous loads from pxe_parse()
+ * @param result Result of load_url_async()
+ * @param data   Pointer to associated conf_context
+ */
+static void pxe_conf_parse_cb(struct load_url_result *result, void *data)
 {
-       struct pb_url *pxe_base_url, *url;
-       struct pxe_parser_info *parser_info;
-       char **pxe_conf_files, **filename;
-       struct conf_context *conf;
-       bool complete_url;
+       struct conf_context *conf = data;
+       struct device_handler *handler;
+       struct boot_status status = {0};
+       char *buf = NULL;
        int len, rc;
-       char *buf;
 
-       /* Expects dhcp event parameters to support network boot */
-       if (!dc->event)
-               return -1;
+       if (!data)
+               return;
 
-       conf = talloc_zero(dc, struct conf_context);
+       if (!result || result->status != LOAD_OK) {
+               talloc_free(conf);
+               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;
+       }
+
+       conf_parse_buf(conf, buf, len);
+
+       /* We may be called well after the original caller of iterate_parsers(),
+        * commit any new boot options ourselves */
+       handler = talloc_parent(conf);
+       device_handler_discover_context_commit(handler, conf->dc);
+
+       status.type = BOOT_STATUS_INFO;
+       /*
+        * TRANSLATORS: the format specifier in this string in an IP address,
+        * eg. 192.168.1.1
+        */
+       status.message = talloc_asprintf(conf, _("pxe: parsed config for %s"),
+                                       conf->dc->conf_url->host);
+       device_handler_boot_status(handler, &status);
+
+       talloc_free(buf);
+out_clean:
+       if (result->cleanup_local)
+               unlink(result->local);
+       talloc_free(conf);
+}
+
+/**
+ * Return a new conf_context and increment the talloc reference count on
+ * the discover_context struct.
+ * @param  ctx  Parent talloc context
+ * @param  orig Original discover_context
+ * @return      Pointer to new conf_context
+ */
+static struct conf_context *copy_context(void *ctx, struct discover_context *dc)
+{
+       struct pxe_parser_info *info;
+       struct conf_context *conf;
+
+       conf = talloc_zero(ctx, struct conf_context);
 
        if (!conf)
-               goto out;
+               return NULL;
 
-       conf->dc = dc;
        conf->get_pair = conf_get_pair_space;
        conf->process_pair = pxe_process_pair;
        conf->finish = pxe_finish;
+       info = talloc_zero(conf, struct pxe_parser_info);
+       if (!info) {
+               talloc_free(conf);
+               return NULL;
+       }
+       conf->parser_info = info;
+
+       /*
+        * The discover_context may be freed once pxe_parse() returns, but the
+        * callback will still need it. Take a reference so that that it will
+        * persist until the last callback completes.
+        */
+       conf->dc = talloc_reference(conf, dc);
 
-       parser_info = talloc_zero(conf, struct pxe_parser_info);
-       conf->parser_info = parser_info;
+       return conf;
+}
+
+static int pxe_parse(struct discover_context *dc)
+{
+       struct pb_url *pxe_base_url, *url;
+       char **pxe_conf_files, **filename;
+       struct conf_context *conf = NULL;
+       struct load_url_result *result;
+       void *ctx = talloc_parent(dc);
+       bool complete_url;
+
+       /* Expects dhcp event parameters to support network boot */
+       if (!dc->event)
+               return -1;
 
        dc->conf_url = user_event_parse_conf_url(dc, dc->event, &complete_url);
        if (!dc->conf_url)
-               goto out_conf;
+               return -1;
+
+       /*
+        * 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.
+        */
+       conf = copy_context(ctx, dc);
+       if (!conf)
+               return -1;
 
        if (complete_url) {
                /* we have a complete URL; use this and we're done. */
-               rc = parser_request_url(dc, dc->conf_url, &buf, &len);
-               if (rc)
+               result = load_url_async(conf->dc, conf->dc->conf_url,
+                                       pxe_conf_parse_cb, conf);
+               if (!result) {
+                       pb_log("load_url_async fails for %s\n",
+                                       dc->conf_url->path);
                        goto out_conf;
+               }
        } else {
                pxe_conf_files = user_event_parse_conf_filenames(dc, dc->event);
                if (!pxe_conf_files)
                        goto out_conf;
 
-               rc = -1;
-
                pxe_base_url = pb_url_join(dc, dc->conf_url, pxelinux_prefix);
                if (!pxe_base_url)
                        goto out_pxe_conf;
 
                for (filename = pxe_conf_files; *filename; filename++) {
-                       url = pb_url_join(dc, pxe_base_url, *filename);
+                       if (!conf) {
+                               conf = copy_context(ctx, dc);
+                       }
+                       url = pb_url_join(conf->dc, pxe_base_url, *filename);
                        if (!url)
                                continue;
-
-                       rc = parser_request_url(dc, url, &buf, &len);
-                       if (!rc) /* found one, just break */
-                               break;
-
-                       talloc_free(url);
+                       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;
                }
 
                talloc_free(pxe_base_url);
-
-               /* No configuration file found on the boot server */
-               if (rc)
-                       goto out_pxe_conf;
-
                talloc_free(pxe_conf_files);
        }
 
-       /* Call the config file parser with the data read from the file */
-       conf_parse_buf(conf, buf, len);
-
-       talloc_free(buf);
-       talloc_free(conf);
-
        return 0;
 
 out_pxe_conf:
        talloc_free(pxe_conf_files);
 out_conf:
        talloc_free(conf);
-out:
        return -1;
 }
 
index f2335410eac65a812c50103b957daa1a522cdc05..d3e2065d36f3b5e34008fa5d075f587245c58c1a 100644 (file)
@@ -760,7 +760,7 @@ off_t talloc_total_blocks(const void *ptr)
 /*
   return the number of external references to a pointer
 */
-static int talloc_reference_count(const void *ptr)
+int talloc_reference_count(const void *ptr)
 {
        struct talloc_chunk *tc = talloc_chunk_from_ptr(ptr);
        struct talloc_reference_handle *h;
index b5fca44aa227d412d28ab1ff63b9ad6a98454d94..b9afc22669f5517df1a01b9dc6106fc792925d3d 100644 (file)
@@ -133,6 +133,7 @@ void *_talloc_realloc_array(const void *ctx, void *ptr, size_t el_size, unsigned
 void *talloc_realloc_fn(const void *context, void *ptr, size_t size);
 void *talloc_autofree_context(void);
 size_t talloc_get_size(const void *ctx);
+int talloc_reference_count(const void *ptr);
 
 #endif
 
index 7202f4965414be300cd9e1c5cb0ee8ea68c3c29b..6eeced32538350cb0aea0fa3edbe1a9c73aab411 100644 (file)
@@ -246,7 +246,7 @@ static void pb_url_update_full(struct pb_url *url)
        url->full = pb_url_to_string(url);
 }
 
-static struct pb_url *pb_url_copy(void *ctx, const struct pb_url *url)
+struct pb_url *pb_url_copy(void *ctx, const struct pb_url *url)
 {
        struct pb_url *new_url;
 
index 25e1ad8175b9681935c5c53ee5a932638d1531d2..9043615590c1a7e95e5d938ccc2d0565af994456 100644 (file)
@@ -62,6 +62,7 @@ struct pb_url {
 
 bool is_url(const char *str);
 struct pb_url *pb_url_parse(void *ctx, const char *url_str);
+struct pb_url *pb_url_copy(void *ctx, const struct pb_url *url);
 struct pb_url *pb_url_join(void *ctx, const struct pb_url *url, const char *s);
 char *pb_url_to_string(struct pb_url *url);