[efi] Report local file errors during download, rather than on opening

iPXE is designed around fully asynchronous I/O, including asynchronous
connection opening.  Almost all errors are therefore necessarily
reported as occurring during an in-progress download, rather than
occurring at the time that the URI is opened.

Local file access is currently an exception to this: errors such as
nonexistent files will be encountered while opening the URI.  This
results in mildly unexpected error messages of the form "Could not
start download", rather than the usual pattern of showing the URI, the
initial progress dots, and then the error message.

Fix this inconsistency by deferring the local filesystem access until
the local file download process is running.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

1 files changed, 130 insertions, 124 deletions

diff --git a/src/interface/efi/efi_local.c b/src/interface/efi/efi_local.c
index f54f5dae..00cc1147 100644
--- a/src/interface/efi/efi_local.c
+++ b/src/interface/efi/efi_local.c
@@ -60,6 +60,13 @@ struct efi_local {
 	/** Download process */
 	struct process process;
 
+	/** Download URI */
+	struct uri *uri;
+	/** Volume name, or NULL to use loaded image's device */
+	const char *volume;
+	/** File path */
+	const char *path;
+
 	/** EFI root directory */
 	EFI_FILE_PROTOCOL *root;
 	/** EFI file */
@@ -69,6 +76,19 @@ struct efi_local {
 };
 
 /**
+ * Free local file
+ *
+ * @v refcnt		Reference count
+ */
+static void efi_local_free ( struct refcnt *refcnt ) {
+	struct efi_local *local =
+		container_of ( refcnt, struct efi_local, refcnt );
+
+	uri_put ( local->uri );
+	free ( local );
+}
+
+/**
  * Close local file
  *
  * @v local		Local file
@@ -96,91 +116,6 @@ static void efi_local_close ( struct efi_local *local, int rc ) {
 }
 
 /**
- * Local file process
- *
- * @v local		Local file
- */
-static void efi_local_step ( struct efi_local *local ) {
-	EFI_FILE_PROTOCOL *file = local->file;
-	struct io_buffer *iobuf = NULL;
-	size_t remaining;
-	size_t frag_len;
-	UINTN size;
-	EFI_STATUS efirc;
-	int rc;
-
-	/* Wait until data transfer interface is ready */
-	if ( ! xfer_window ( &local->xfer ) )
-		return;
-
-	/* Presize receive buffer */
-	remaining = local->len;
-	xfer_seek ( &local->xfer, remaining );
-	xfer_seek ( &local->xfer, 0 );
-
-	/* Get file contents */
-	while ( remaining ) {
-
-		/* Calculate length for this fragment */
-		frag_len = remaining;
-		if ( frag_len > EFI_LOCAL_BLKSIZE )
-			frag_len = EFI_LOCAL_BLKSIZE;
-
-		/* Allocate I/O buffer */
-		iobuf = xfer_alloc_iob ( &local->xfer, frag_len );
-		if ( ! iobuf ) {
-			rc = -ENOMEM;
-			goto err;
-		}
-
-		/* Read block */
-		size = frag_len;
-		if ( ( efirc = file->Read ( file, &size, iobuf->data ) ) != 0 ){
-			rc = -EEFI ( efirc );
-			DBGC ( local, "LOCAL %p could not read from file: %s\n",
-			       local, strerror ( rc ) );
-			goto err;
-		}
-		assert ( size <= frag_len );
-		iob_put ( iobuf, size );
-
-		/* Deliver data */
-		if ( ( rc = xfer_deliver_iob ( &local->xfer,
-					       iob_disown ( iobuf ) ) ) != 0 ) {
-			DBGC ( local, "LOCAL %p could not deliver data: %s\n",
-			       local, strerror ( rc ) );
-			goto err;
-		}
-
-		/* Move to next block */
-		remaining -= frag_len;
-	}
-
-	/* Close download */
-	efi_local_close ( local, 0 );
-
-	return;
-
- err:
-	free_iob ( iobuf );
-	efi_local_close ( local, rc );
-}
-
-/** Data transfer interface operations */
-static struct interface_operation efi_local_operations[] = {
-	INTF_OP ( xfer_window_changed, struct efi_local *, efi_local_step ),
-	INTF_OP ( intf_close, struct efi_local *, efi_local_close ),
-};
-
-/** Data transfer interface descriptor */
-static struct interface_descriptor efi_local_xfer_desc =
-	INTF_DESC ( struct efi_local, xfer, efi_local_operations );
-
-/** Process descriptor */
-static struct process_descriptor efi_local_process_desc =
-	PROC_DESC_ONCE ( struct efi_local, process, efi_local_step );
-
-/**
  * Check for matching volume name
  *
  * @v local		Local file
@@ -298,15 +233,14 @@ static int efi_local_open_root ( struct efi_local *local, EFI_HANDLE device,
  * Open root filesystem of specified volume
  *
  * @v local		Local file
- * @v volume		Volume name, or NULL to use loaded image's device
  * @ret rc		Return status code
  */
-static int efi_local_open_volume ( struct efi_local *local,
-				   const char *volume ) {
+static int efi_local_open_volume ( struct efi_local *local ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 	EFI_GUID *protocol = &efi_simple_file_system_protocol_guid;
 	int ( * check ) ( struct efi_local *local, EFI_HANDLE device,
 			  EFI_FILE_PROTOCOL *root, const char *volume );
+	const char *volume = local->volume;
 	EFI_DEVICE_PATH_PROTOCOL *path;
 	EFI_FILE_PROTOCOL *root;
 	EFI_HANDLE *handles;
@@ -419,11 +353,9 @@ static int efi_local_open_resolved ( struct efi_local *local,
  * Open specified path
  *
  * @v local		Local file
- * @v filename		Path to file relative to our own image
  * @ret rc		Return status code
  */
-static int efi_local_open_path ( struct efi_local *local,
-				 const char *filename ) {
+static int efi_local_open_path ( struct efi_local *local ) {
 	EFI_DEVICE_PATH_PROTOCOL *path = efi_loaded_image->FilePath;
 	EFI_DEVICE_PATH_PROTOCOL *next;
 	FILEPATH_DEVICE_PATH *fp;
@@ -454,7 +386,7 @@ static int efi_local_open_path ( struct efi_local *local,
 	}
 
 	/* Resolve path */
-	resolved = resolve_path ( base, filename );
+	resolved = resolve_path ( base, local->path );
 	if ( ! resolved ) {
 		rc = -ENOMEM;
 		goto err_resolve;
@@ -524,6 +456,106 @@ static int efi_local_len ( struct efi_local *local ) {
 }
 
 /**
+ * Local file process
+ *
+ * @v local		Local file
+ */
+static void efi_local_step ( struct efi_local *local ) {
+	struct io_buffer *iobuf = NULL;
+	size_t remaining;
+	size_t frag_len;
+	UINTN size;
+	EFI_STATUS efirc;
+	int rc;
+
+	/* Wait until data transfer interface is ready */
+	if ( ! xfer_window ( &local->xfer ) )
+		return;
+
+	/* Open specified volume root directory, if not yet open */
+	if ( ( ! local->root ) &&
+	     ( ( rc = efi_local_open_volume ( local ) ) != 0 ) )
+		goto err;
+
+	/* Open specified file, if not yet open */
+	if ( ( ! local->file ) &&
+	     ( ( rc = efi_local_open_path ( local ) ) != 0 ) )
+		goto err;
+
+	/* Get file length, if not yet known */
+	if ( ( ! local->len ) &&
+	     ( ( rc = efi_local_len ( local ) ) != 0 ) )
+		goto err;
+
+	/* Presize receive buffer */
+	remaining = local->len;
+	xfer_seek ( &local->xfer, remaining );
+	xfer_seek ( &local->xfer, 0 );
+
+	/* Get file contents */
+	while ( remaining ) {
+
+		/* Calculate length for this fragment */
+		frag_len = remaining;
+		if ( frag_len > EFI_LOCAL_BLKSIZE )
+			frag_len = EFI_LOCAL_BLKSIZE;
+
+		/* Allocate I/O buffer */
+		iobuf = xfer_alloc_iob ( &local->xfer, frag_len );
+		if ( ! iobuf ) {
+			rc = -ENOMEM;
+			goto err;
+		}
+
+		/* Read block */
+		size = frag_len;
+		if ( ( efirc = local->file->Read ( local->file, &size,
+						   iobuf->data ) ) != 0 ) {
+			rc = -EEFI ( efirc );
+			DBGC ( local, "LOCAL %p could not read from file: %s\n",
+			       local, strerror ( rc ) );
+			goto err;
+		}
+		assert ( size <= frag_len );
+		iob_put ( iobuf, size );
+
+		/* Deliver data */
+		if ( ( rc = xfer_deliver_iob ( &local->xfer,
+					       iob_disown ( iobuf ) ) ) != 0 ) {
+			DBGC ( local, "LOCAL %p could not deliver data: %s\n",
+			       local, strerror ( rc ) );
+			goto err;
+		}
+
+		/* Move to next block */
+		remaining -= frag_len;
+	}
+
+	/* Close download */
+	efi_local_close ( local, 0 );
+
+	return;
+
+ err:
+	free_iob ( iobuf );
+	efi_local_close ( local, rc );
+}
+
+/** Data transfer interface operations */
+static struct interface_operation efi_local_operations[] = {
+	INTF_OP ( xfer_window_changed, struct efi_local *, efi_local_step ),
+	INTF_OP ( intf_close, struct efi_local *, efi_local_close ),
+};
+
+/** Data transfer interface descriptor */
+static struct interface_descriptor efi_local_xfer_desc =
+	INTF_DESC ( struct efi_local, xfer, efi_local_operations );
+
+/** Process descriptor */
+static struct process_descriptor efi_local_process_desc =
+	PROC_DESC_ONCE ( struct efi_local, process, efi_local_step );
+
+/**
  * Open local file
  *
  * @v xfer		Data transfer interface
@@ -532,36 +564,18 @@ static int efi_local_len ( struct efi_local *local ) {
  */
 static int efi_local_open ( struct interface *xfer, struct uri *uri ) {
 	struct efi_local *local;
-	const char *volume;
-	const char *path;
-	int rc;
-
-	/* Parse URI */
-	volume = ( ( uri->host && uri->host[0] ) ? uri->host : NULL );
-	path = ( uri->opaque ? uri->opaque : uri->path );
 
 	/* Allocate and initialise structure */
 	local = zalloc ( sizeof ( *local ) );
-	if ( ! local ) {
-		rc = -ENOMEM;
-		goto err_alloc;
-	}
-	ref_init ( &local->refcnt, NULL );
+	if ( ! local )
+		return -ENOMEM;
+	ref_init ( &local->refcnt, efi_local_free );
 	intf_init ( &local->xfer, &efi_local_xfer_desc, &local->refcnt );
 	process_init_stopped ( &local->process, &efi_local_process_desc,
 			       &local->refcnt );
-
-	/* Open specified volume */
-	if ( ( rc = efi_local_open_volume ( local, volume ) ) != 0 )
-		goto err_open_root;
-
-	/* Open specified path */
-	if ( ( rc = efi_local_open_path ( local, path ) ) != 0 )
-		goto err_open_file;
-
-	/* Get length of file */
-	if ( ( rc = efi_local_len ( local ) ) != 0 )
-		goto err_len;
+	local->uri = uri_get ( uri );
+	local->volume = ( ( uri->host && uri->host[0] ) ? uri->host : NULL );
+	local->path = ( uri->opaque ? uri->opaque : uri->path );
 
 	/* Start download process */
 	process_add ( &local->process );
@@ -570,14 +584,6 @@ static int efi_local_open ( struct interface *xfer, struct uri *uri ) {
 	intf_plug_plug ( &local->xfer, xfer );
 	ref_put ( &local->refcnt );
 	return 0;
-
- err_len:
- err_open_file:
- err_open_root:
-	efi_local_close ( local, 0 );
-	ref_put ( &local->refcnt );
- err_alloc:
-	return rc;
 }
 
 /** EFI local file URI opener */