[PATCH 2/2] discover-server: Download and parse config files specfied by user

[Jeremy Kerr]

Hi Sam,

> Allow arbritrary conf files to be passed to pxe-parser. This also
> enables the pb-event conf at dev option for URLs.

I'd suggest splitting these patches up differently:

 - fixing the conf@ code and hooking it up to the parser (as you've
   done in this patch)
 - add support for the conf messages in the types & protocol code
   (_serialise, _deserialise and _len) functions
 - add support in the server to handle these messages
 - add support in the UI.

Also, can we make the conf@ event name match the protocol action name?
(maybe url@) ?

> 
> Signed-off-by: Samuel Mendoza-Jonas <sam.mj at au1.ibm.com>
> ---
>  discover/device-handler.c  | 150 ++++++++++++++++++++++++++++++++++++++++++---
>  discover/device-handler.h  |   4 +-
>  discover/discover-server.c |   7 +++
>  discover/user-event.c      |  13 +---
>  4 files changed, 154 insertions(+), 20 deletions(-)
> 
> diff --git a/discover/device-handler.c b/discover/device-handler.c
> index 1a8649e..8ea4154 100644
> --- a/discover/device-handler.c
> +++ b/discover/device-handler.c
> @@ -17,6 +17,11 @@
>  #include <process/process.h>
>  #include <url/url.h>
>  
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include <netdb.h>
> +#include <arpa/inet.h>
> +
>  #include "device-handler.h"
>  #include "discover-server.h"
>  #include "user-event.h"
> @@ -723,21 +728,22 @@ int device_handler_dhcp(struct device_handler *handler,
>  
>  /* incoming conf event */
>  int device_handler_conf(struct device_handler *handler,
> -		struct discover_device *dev, struct pb_url *url)
> +		struct discover_device *dev, struct pb_url *url, struct event *event)
>  {
> -        struct discover_context *ctx;
> +	struct discovexr_context *ctx;

Looks like you've got some tab/space weirdness here.

> +	const char *argv[] = {
> +			pb_system_apps.ip,
> +			"route","show","to","match",
> +			ipaddr,
> +			NULL
> +	};

Hey, nice idea.

> +
> +	p = process_create(ctx);
> +
> +	p->path = pb_system_apps.ip;
> +	p->argv = argv;
> +	p->keep_stdout = true;
> +
> +	rc = process_run_sync(p);
> +
> +	if (rc) {
> +		/* ip has complained for some reason; most likely
> +		 * there is no route to the host - bail out */
> +		pb_debug("%s: No route to %s\n",__func__,url->host);
> +		return NULL;
> +	}
> +
> +	buf = p->stdout_buf;
> +	/* If a route is found, ip-route output will be of the form
> +	 * "... dev DEVNAME ... " */
> +	tok = strtok(buf, delim);
> +	while (tok) {
> +		if (!strcmp(tok, "dev")) {
> +			tok = strtok(NULL, delim);
> +			dev = talloc_strdup(ctx, tok);
> +			break;
> +		}
> +		tok = strtok(NULL, delim);
> +	}
> +
> +	process_release(p);
> +	if (dev)
> +		pb_debug("%s: Found interface '%s'\n", __func__,dev);
> +	return dev;
> +}
> +
> +
> +void device_handler_process_url(struct device_handler *handler,
> +		const char *url)
> +{
> +	struct boot_status *status = talloc(handler,struct boot_status);
> +	struct discover_device *dev;
> +	struct pb_url *pb_url;
> +	struct event *event;
> +	struct param *param;
> +
> +	status->type = BOOT_STATUS_ERROR;
> +	status->progress = 0;
> +	status->detail = talloc_asprintf(handler,"Received config URL %s",url);
> +
> +	/* Catch a case where pb-event can send an empty string,
> +	 * which would otherwise trigger an assert in pb_url_parse
> +	 */

Should we just catch this in pb_url_parse instead?

> +	if (!strlen(url)) {
> +		status->message = talloc_asprintf(handler,
> +					"Missing URL!");
> +		goto msg;
> +	}
> +
> +	event = talloc(handler, struct event);
> +	event->type = EVENT_TYPE_USER;
> +	event->action = EVENT_ACTION_CONF;
> +
> +	event->params = talloc_array(event, struct param, 1);
> +	param = &event->params[0];
> +	param->name = talloc_strdup(event, "pxeconffile");
> +	param->value = talloc_strdup(event, url);
> +	event->n_params = 1;

I'm not clear on why we need the URL in the event as well as the
parameter to device_handler_conf. Is there something that needs to be
cleaned-up there?

Cheers,


Jeremy