[image] Simplify image management commands and internal API

Remove the name, cmdline, and action parameters from imgdownload() and
imgdownload_string().  These functions now simply download and return
an image.

Add the function imgacquire(), which will interpret a "name or URI
string" parameter and return either an existing image or a newly
downloaded image.

Use imgacquire() to merge similar image-management commands that
currently differ only by whether they take the name of an existing
image or the URI of a new image to download.  For example, "chain" and
"imgexec" can now be merged.

Extend imgstat and imgfree commands to take an optional list of
images.

Remove the arbitrary restriction on the length of image names.

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

2 files changed, 42 insertions, 46 deletions

diff --git a/src/usr/autoboot.c b/src/usr/autoboot.c
index 5bfadec0..da82f5e8 100644
--- a/src/usr/autoboot.c
+++ b/src/usr/autoboot.c
@@ -131,6 +131,7 @@ struct setting skip_san_boot_setting __setting ( SETTING_SANBOOT_EXTRA ) = {
  */
 int uriboot ( struct uri *filename, struct uri *root_path, int drive,
 	      unsigned int flags ) {
+	struct image *image;
 	int rc;
 
 	/* Hook SAN device, if applicable */
@@ -157,9 +158,10 @@ int uriboot ( struct uri *filename, struct uri *root_path, int drive,
 
 	/* Attempt filename boot if applicable */
 	if ( filename ) {
-		if ( ( rc = imgdownload ( filename, NULL, NULL,
-					  image_exec ) ) != 0 ) {
-			printf ( "\nCould not chain image: %s\n",
+		if ( ( rc = imgdownload ( filename, &image ) ) != 0 )
+			goto err_download;
+		if ( ( rc = image_exec ( image ) ) != 0 ) {
+			printf ( "Could not boot image: %s\n",
 				 strerror ( rc ) );
 			/* Fall through to (possibly) attempt a SAN boot
 			 * as a fallback.  If no SAN boot is attempted,
@@ -190,6 +192,7 @@ int uriboot ( struct uri *filename, struct uri *root_path, int drive,
 		}
 	}
 
+ err_download:
  err_san_describe:
 	/* Unhook SAN device, if applicable */
 	if ( ( drive >= 0 ) && ! ( flags & URIBOOT_NO_SAN_UNHOOK ) ) {
diff --git a/src/usr/imgmgmt.c b/src/usr/imgmgmt.c
index 59011415..2c74f486 100644
--- a/src/usr/imgmgmt.c
+++ b/src/usr/imgmgmt.c
@@ -36,39 +36,25 @@ FILE_LICENCE ( GPL2_OR_LATER );
  */
 
 /**
- * Download an image
+ * Download a new image
  *
  * @v uri		URI
- * @v name		Image name, or NULL to use default
- * @v cmdline		Command line, or NULL for no command line
- * @v action		Action to take upon a successful download, or NULL
+ * @v image		Image to fill in
  * @ret rc		Return status code
  */
-int imgdownload ( struct uri *uri, const char *name, const char *cmdline,
-		  int ( * action ) ( struct image *image ) ) {
-	struct image *image;
+int imgdownload ( struct uri *uri, struct image **image ) {
 	size_t len = ( unparse_uri ( NULL, 0, uri, URI_ALL ) + 1 );
 	char uri_string_redacted[len];
 	const char *password;
 	int rc;
 
 	/* Allocate image */
-	image = alloc_image();
-	if ( ! image ) {
+	*image = alloc_image ( uri );
+	if ( ! *image ) {
 		rc = -ENOMEM;
 		goto err_alloc_image;
 	}
 
-	/* Set image name */
-	if ( name )
-		image_set_name ( image, name );
-
-	/* Set image URI */
-	image_set_uri ( image, uri );
-
-	/* Set image command line */
-	image_set_cmdline ( image, cmdline );
-
 	/* Redact password portion of URI, if necessary */
 	password = uri->password;
 	if ( password )
@@ -78,8 +64,9 @@ int imgdownload ( struct uri *uri, const char *name, const char *cmdline,
 	uri->password = password;
 
 	/* Create downloader */
-	if ( ( rc = create_downloader ( &monojob, image, LOCATION_URI,
+	if ( ( rc = create_downloader ( &monojob, *image, LOCATION_URI,
 					uri ) ) != 0 ) {
+		printf ( "Could not start download: %s\n", strerror ( rc ) );
 		goto err_create_downloader;
 	}
 
@@ -88,50 +75,65 @@ int imgdownload ( struct uri *uri, const char *name, const char *cmdline,
 		goto err_monojob_wait;
 
 	/* Register image */
-	if ( ( rc = register_image ( image ) ) != 0 )
+	if ( ( rc = register_image ( *image ) ) != 0 ) {
+		printf ( "Could not register image: %s\n", strerror ( rc ) );
 		goto err_register_image;
+	}
 
 	/* Drop local reference to image.  Image is guaranteed to
 	 * remain in scope since it is registered.
 	 */
-	image_put ( image );
+	image_put ( *image );
 
-	/* Carry out specified post-download action, if applicable */
-	return ( action ? action ( image ) : 0 );
+	return 0;
 
  err_register_image:
  err_monojob_wait:
  err_create_downloader:
-	image_put ( image );
+	image_put ( *image );
  err_alloc_image:
 	return rc;
 }
 
 /**
- * Download an image
+ * Download a new image
  *
- * @v uri_string	URI as a string (e.g. "http://www.nowhere.com/vmlinuz")
- * @v name		Image name, or NULL to use default
- * @v cmdline		Command line, or NULL for no command line
- * @v action		Action to take upon a successful download
+ * @v uri_string	URI string
+ * @v image		Image to fill in
  * @ret rc		Return status code
  */
-int imgdownload_string ( const char *uri_string, const char *name,
-			 const char *cmdline,
-			 int ( * action ) ( struct image *image ) ) {
+int imgdownload_string ( const char *uri_string, struct image **image ) {
 	struct uri *uri;
 	int rc;
 
 	if ( ! ( uri = parse_uri ( uri_string ) ) )
 		return -ENOMEM;
 
-	rc = imgdownload ( uri, name, cmdline, action );
+	rc = imgdownload ( uri, image );
 
 	uri_put ( uri );
 	return rc;
 }
 
 /**
+ * Acquire an image
+ *
+ * @v name_uri		Name or URI string
+ * @v image		Image to fill in
+ * @ret rc		Return status code
+ */
+int imgacquire ( const char *name_uri, struct image **image ) {
+
+	/* If we already have an image with the specified name, use it */
+	*image = find_image ( name_uri );
+	if ( *image )
+		return 0;
+
+	/* Otherwise, download a new image */
+	return imgdownload_string ( name_uri, image );
+}
+
+/**
  * Display status of an image
  *
  * @v image		Executable/loadable image
@@ -148,12 +150,3 @@ void imgstat ( struct image *image ) {
 		printf ( " \"%s\"", image->cmdline );
 	printf ( "\n" );
 }
-
-/**
- * Free an image
- *
- * @v image		Executable/loadable image
- */
-void imgfree ( struct image *image ) {
-	unregister_image ( image );
-}