rbd: separate reading header from decoding it

Right now rbd_read_header() both reads the header object for an rbd
image and decodes its contents.  It does this repeatedly if needed,
in order to ensure a complete and intact header is obtained.

Separate this process into two steps--reading of the raw header
data (in new function, rbd_dev_v1_header_read()) and separately
decoding its contents (in rbd_header_from_disk()).  As a result,
the latter function no longer requires its allocated_snaps argument.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

1 files changed, 79 insertions, 57 deletions

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 5bcd4ebb22e7..8e6e29eacb1a 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -513,15 +513,11 @@ static bool rbd_dev_ondisk_valid(struct rbd_image_header_ondisk *ondisk)
  * header.
  */
 static int rbd_header_from_disk(struct rbd_image_header *header,
-				 struct rbd_image_header_ondisk *ondisk,
-				 u32 allocated_snaps)
+				 struct rbd_image_header_ondisk *ondisk)
 {
 	u32 snap_count;
 	size_t size;
 
-	if (!rbd_dev_ondisk_valid(ondisk))
-		return -ENXIO;
-
 	memset(header, 0, sizeof (*header));
 
 	snap_count = le32_to_cpu(ondisk->snap_count);
@@ -558,15 +554,6 @@ static int rbd_header_from_disk(struct rbd_image_header *header,
 	header->comp_type = ondisk->options.comp_type;
 	header->total_snaps = snap_count;
 
-	/*
-	 * If the number of snapshot ids provided by the caller
-	 * doesn't match the number in the entire context there's
-	 * no point in going further.  Caller will try again after
-	 * getting an updated snapshot context from the server.
-	 */
-	if (allocated_snaps != snap_count)
-		return 0;
-
 	size = sizeof (struct ceph_snap_context);
 	size += snap_count * sizeof (header->snapc->snaps[0]);
 	header->snapc = kzalloc(size, GFP_KERNEL);
@@ -1629,61 +1616,96 @@ static void rbd_free_disk(struct rbd_device *rbd_dev)
 }
 
 /*
- * reload the ondisk the header 
+ * Read the complete header for the given rbd device.
+ *
+ * Returns a pointer to a dynamically-allocated buffer containing
+ * the complete and validated header.  Caller can pass the address
+ * of a variable that will be filled in with the version of the
+ * header object at the time it was read.
+ *
+ * Returns a pointer-coded errno if a failure occurs.
  */
-static int rbd_read_header(struct rbd_device *rbd_dev,
-			   struct rbd_image_header *header)
+static struct rbd_image_header_ondisk *
+rbd_dev_v1_header_read(struct rbd_device *rbd_dev, u64 *version)
 {
-	ssize_t rc;
-	struct rbd_image_header_ondisk *dh;
+	struct rbd_image_header_ondisk *ondisk = NULL;
 	u32 snap_count = 0;
-	u64 ver;
-	size_t len;
+	u64 names_size = 0;
+	u32 want_count;
+	int ret;
 
 	/*
-	 * First reads the fixed-size header to determine the number
-	 * of snapshots, then re-reads it, along with all snapshot
-	 * records as well as their stored names.
+	 * The complete header will include an array of its 64-bit
+	 * snapshot ids, followed by the names of those snapshots as
+	 * a contiguous block of NUL-terminated strings.  Note that
+	 * the number of snapshots could change by the time we read
+	 * it in, in which case we re-read it.
 	 */
-	len = sizeof (*dh);
-	while (1) {
-		dh = kmalloc(len, GFP_KERNEL);
-		if (!dh)
-			return -ENOMEM;
-
-		rc = rbd_req_sync_read(rbd_dev,
-				       CEPH_NOSNAP,
+	do {
+		size_t size;
+
+		kfree(ondisk);
+
+		size = sizeof (*ondisk);
+		size += snap_count * sizeof (struct rbd_image_snap_ondisk);
+		size += names_size;
+		ondisk = kmalloc(size, GFP_KERNEL);
+		if (!ondisk)
+			return ERR_PTR(-ENOMEM);
+
+		ret = rbd_req_sync_read(rbd_dev, CEPH_NOSNAP,
 				       rbd_dev->header_name,
-				       0, len,
-				       (char *)dh, &ver);
-		if (rc < 0)
-			goto out_dh;
-
-		rc = rbd_header_from_disk(header, dh, snap_count);
-		if (rc < 0) {
-			if (rc == -ENXIO)
-				pr_warning("unrecognized header format"
-					   " for image %s\n",
-					   rbd_dev->image_name);
-			goto out_dh;
+				       0, size,
+				       (char *) ondisk, version);
+
+		if (ret < 0)
+			goto out_err;
+		if (WARN_ON((size_t) ret < size)) {
+			ret = -ENXIO;
+			pr_warning("short header read for image %s"
+					" (want %zd got %d)\n",
+				rbd_dev->image_name, size, ret);
+			goto out_err;
+		}
+		if (!rbd_dev_ondisk_valid(ondisk)) {
+			ret = -ENXIO;
+			pr_warning("invalid header for image %s\n",
+				rbd_dev->image_name);
+			goto out_err;
 		}
 
-		if (snap_count == header->total_snaps)
-			break;
+		names_size = le64_to_cpu(ondisk->snap_names_len);
+		want_count = snap_count;
+		snap_count = le32_to_cpu(ondisk->snap_count);
+	} while (snap_count != want_count);
 
-		snap_count = header->total_snaps;
-		len = sizeof (*dh) +
-			snap_count * sizeof(struct rbd_image_snap_ondisk) +
-			header->snap_names_len;
+	return ondisk;
 
-		rbd_header_free(header);
-		kfree(dh);
-	}
-	header->obj_version = ver;
+out_err:
+	kfree(ondisk);
 
-out_dh:
-	kfree(dh);
-	return rc;
+	return ERR_PTR(ret);
+}
+
+/*
+ * reload the ondisk the header
+ */
+static int rbd_read_header(struct rbd_device *rbd_dev,
+			   struct rbd_image_header *header)
+{
+	struct rbd_image_header_ondisk *ondisk;
+	u64 ver = 0;
+	int ret;
+
+	ondisk = rbd_dev_v1_header_read(rbd_dev, &ver);
+	if (IS_ERR(ondisk))
+		return PTR_ERR(ondisk);
+	ret = rbd_header_from_disk(header, ondisk);
+	if (ret >= 0)
+		header->obj_version = ver;
+	kfree(ondisk);
+
+	return ret;
 }
 
 /*