All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] rbd: activate v2 image support
@ 2012-09-07 21:09 Alex Elder
  2012-09-07 21:13 ` [PATCH 1/9] rbd: lay out header probe infrastructure Alex Elder
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: Alex Elder @ 2012-09-07 21:09 UTC (permalink / raw)
  To: ceph-devel

This is the eight and final series of patches, this one finally
populating the code that implements support for rbd version 2
images in the Linux kernel rbd client.

All of the previous patches laid the ground work to make these
last steps pretty straightforward.  The first patch lays out
a mechanism where a request to map an rbd image results in a
"probe" operation to determine whether the named image exists
as either version 1 or version 2 format.  Once its existence
(and version) is determined, the rest of the setup for the
rbd device structure proceeds using code appropriate for the
image type.

The last patch in this series simply activates support for
version 2 images, by returning a success indication rather
than ENOSUPP from the version 2 probe routine.

The 7 patches bracketed by these two simply populate the code
that fetches information about a version 2 image, including
its snapshot context and the name and other information about
each snapshot.

This series is available as branch "wip-rbd-review-8" on the
ceph-client git repository, and is based on the branch
"wip-rbd-review-7".

    https://github.com/ceph/ceph-client/tree/wip-rbd-review-8

					-Alex

[PATCH 1/9]  17d4a86 rbd: lay out header probe infrastructure
[PATCH 2/9]  86dbebe rbd: add code to get the size of a v2 rbd image
[PATCH 3/9]  0e94833 rbd: get the object prefix for a v2 rbd image
[PATCH 4/9]  a0b250d rbd: get image features for a v2 image
[PATCH 5/9]  57b9193 rbd: get the snapshot context for a v2 image
[PATCH 6/9]  bd39e1f rbd: get snapshot name for a v2 image
[PATCH 7/9]  f33e41f rbd: update remaining header fields for v2
[PATCH 8/9]  431a5bd rbd: define rbd_dev_v2_snapc_refresh()
[PATCH 9/9]  c9c6c27 rbd: activate v2 image support

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/9] rbd: lay out header probe infrastructure
  2012-09-07 21:09 [PATCH 0/9] rbd: activate v2 image support Alex Elder
@ 2012-09-07 21:13 ` Alex Elder
  2012-09-19 18:35   ` Josh Durgin
  2012-09-07 21:13 ` [PATCH 2/9] rbd: add code to get the size of a v2 rbd image Alex Elder
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Alex Elder @ 2012-09-07 21:13 UTC (permalink / raw)
  To: ceph-devel

This defines a new function rbd_dev_probe() as a top-level
function for populating detailed information about an rbd device.

It first checks for the existence of a format 2 rbd image id object.
If it exists, the image is assumed to be a format 2 rbd image, and
another function rbd_dev_v2() is called to finish populating
header data for that image.  If it does not exist, it is assumed to
be an old (format 1) rbd image, and calls a similar function
rbd_dev_v1() to populate its header information.

A new field, rbd_dev->format, is defined to record which version
of the rbd image format the device represents.  For a valid mapped
rbd device it will have one of two values, 1 or 2.

So far, the format 2 images are not really supported; this is
laying out the infrastructure for fleshing out that support.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |  126
++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 99 insertions(+), 27 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index e6b8fa6..019e695 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -170,6 +170,7 @@ struct rbd_device {
 	int			major;		/* blkdev assigned major */
 	struct gendisk		*disk;		/* blkdev's gendisk and rq */

+	u32			image_format;	/* Either 1 or 2 */
 	struct rbd_options	rbd_opts;
 	struct rbd_client	*rbd_client;

@@ -507,6 +508,11 @@ static void rbd_coll_release(struct kref *kref)
 	kfree(coll);
 }

+static bool rbd_image_format_valid(u32 image_format)
+{
+	return image_format == 1 || image_format == 2;
+}
+
 static bool rbd_dev_ondisk_valid(struct rbd_image_header_ondisk *ondisk)
 {
 	size_t size;
@@ -2579,6 +2585,96 @@ out:
 	return ret;
 }

+static int rbd_dev_v1_probe(struct rbd_device *rbd_dev)
+{
+	int ret;
+	size_t size;
+
+	/* Version 1 images have no id; empty string is used */
+
+	rbd_dev->image_id = kstrdup("", GFP_KERNEL);
+	if (!rbd_dev->image_id)
+		return -ENOMEM;
+	rbd_dev->image_id_len = 0;
+
+	/* Record the header object name for this rbd image. */
+
+	size = rbd_dev->image_name_len + sizeof (RBD_SUFFIX);
+	rbd_dev->header_name = kmalloc(size, GFP_KERNEL);
+	if (!rbd_dev->header_name) {
+		ret = -ENOMEM;
+		goto out_err;
+	}
+	sprintf(rbd_dev->header_name, "%s%s", rbd_dev->image_name, RBD_SUFFIX);
+
+	/* Populate rbd image metadata */
+
+	ret = rbd_read_header(rbd_dev, &rbd_dev->header);
+	if (ret < 0)
+		goto out_err;
+	rbd_dev->image_format = 1;
+
+	dout("discovered version 1 image, header name is %s\n",
+		rbd_dev->header_name);
+
+	return 0;
+
+out_err:
+	kfree(rbd_dev->header_name);
+	rbd_dev->header_name = NULL;
+	kfree(rbd_dev->image_id);
+	rbd_dev->image_id = NULL;
+
+	return ret;
+}
+
+static int rbd_dev_v2_probe(struct rbd_device *rbd_dev)
+{
+	size_t size;
+
+	/*
+	 * Image id was filled in by the caller.  Record the header
+	 * object name for this rbd image.
+	 */
+	size = sizeof (RBD_HEADER_PREFIX) + rbd_dev->image_id_len;
+	rbd_dev->header_name = kmalloc(size, GFP_KERNEL);
+	if (!rbd_dev->header_name)
+		return -ENOMEM;
+	sprintf(rbd_dev->header_name, "%s%s",
+			RBD_HEADER_PREFIX, rbd_dev->image_id);
+	rbd_dev->image_format = 2;
+
+	dout("discovered version 2 image, header name is %s\n",
+		rbd_dev->header_name);
+
+	return -ENOTSUPP;
+}
+
+/*
+ * Probe for the existence of the header object for the given rbd
+ * device.  For format 2 images this includes determining the image
+ * id.
+ */
+static int rbd_dev_probe(struct rbd_device *rbd_dev)
+{
+	int ret;
+
+	/*
+	 * Get the id from the image id object.  If it's not a
+	 * format 2 image, we'll get ENOENT back, and we'll assume
+	 * it's a format 1 image.
+	 */
+	ret = rbd_dev_image_id(rbd_dev);
+	if (ret == -ENOENT)
+		ret = rbd_dev_v1_probe(rbd_dev);
+	else if (!ret)
+		ret = rbd_dev_v2_probe(rbd_dev);
+	if (ret)
+		dout("probe failed, returning %d\n", ret);
+
+	return ret;
+}
+
 static ssize_t rbd_add(struct bus_type *bus,
 		       const char *buf,
 		       size_t count)
@@ -2628,34 +2724,10 @@ static ssize_t rbd_add(struct bus_type *bus,
 		goto err_out_client;
 	rbd_dev->pool_id = rc;

-	rc = rbd_dev_image_id(rbd_dev);
-	if (rc == -ENOENT) {
-		/* Version 1 images have no id; empty string is used */
-		rbd_dev->image_id = kstrdup("", GFP_KERNEL);
-		if (!rbd_dev->image_id) {
-			rc = -ENOMEM;
-			goto err_out_client;
-		}
-		rbd_dev->image_id_len = 0;
-	} else {
-	        /* Not actually supporting format 2 yet */
-		goto err_out_client;
-	}
-
-	/* Create the name of the header object */
-
-	rbd_dev->header_name = kmalloc(rbd_dev->image_name_len
-						+ sizeof (RBD_SUFFIX),
-					GFP_KERNEL);
-	if (!rbd_dev->header_name)
-		goto err_out_client;
-	sprintf(rbd_dev->header_name, "%s%s", rbd_dev->image_name, RBD_SUFFIX);
-
-	/* Get information about the image being mapped */
-
-	rc = rbd_read_header(rbd_dev, &rbd_dev->header);
-	if (rc)
+	rc = rbd_dev_probe(rbd_dev);
+	if (rc < 0)
 		goto err_out_client;
+	rbd_assert(rbd_image_format_valid(rbd_dev->image_format));

 	rc = rbd_dev_snaps_update(rbd_dev);
 	if (rc)
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/9]  rbd: add code to get the size of a v2 rbd image
  2012-09-07 21:09 [PATCH 0/9] rbd: activate v2 image support Alex Elder
  2012-09-07 21:13 ` [PATCH 1/9] rbd: lay out header probe infrastructure Alex Elder
@ 2012-09-07 21:13 ` Alex Elder
  2012-09-19 18:52   ` Josh Durgin
  2012-09-07 21:13 ` [PATCH 3/9] rbd: get the object prefix for " Alex Elder
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Alex Elder @ 2012-09-07 21:13 UTC (permalink / raw)
  To: ceph-devel

The size of an rbd format 2 image is fetched from the server using a
"get_size" method.  The same method is used for getting the size of
a snapshot, so structure this addition with a generic helper routine
that we can get this information for either.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   53
+++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 019e695..24d12e3 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2123,6 +2123,47 @@ static char *rbd_dev_v1_snap_info(struct
rbd_device *rbd_dev, u32 which,
 }

 /*
+ * Get the size and object order for an image snapshot, or if
+ * snap_id is CEPH_NOSNAP, gets this information for the base
+ * image.
+ */
+static int _rbd_dev_v2_snap_size(struct rbd_device *rbd_dev, u64 snap_id,
+				u8 *order, u64 *snap_size)
+{
+	__le64 snapid = cpu_to_le64(snap_id);
+	int ret;
+	struct {
+		u8 order;
+		__le64 size;
+	} __attribute__ ((packed)) size_buf = { 0 };
+
+	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
+				"rbd", "get_size",
+				(char *) &snapid, sizeof (snapid),
+				(char *) &size_buf, sizeof (size_buf),
+				CEPH_OSD_FLAG_READ, NULL);
+	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
+	if (ret < 0)
+		return ret;
+
+	*order = size_buf.order;
+	*snap_size = le64_to_cpu(size_buf.size);
+
+	dout("  snap_id 0x%016llx order = %u, snap_size = %llu\n",
+		(unsigned long long) snap_id, (unsigned int) *order,
+		(unsigned long long) *snap_size);
+
+	return 0;
+}
+
+static int rbd_dev_v2_image_size(struct rbd_device *rbd_dev)
+{
+	return _rbd_dev_v2_snap_size(rbd_dev, CEPH_NOSNAP,
+					&rbd_dev->header.obj_order,
+					&rbd_dev->header.image_size);
+}
+
+/*
  * Scan the rbd device's current snapshot list and compare it to the
  * newly-received snapshot context.  Remove any existing snapshots
  * not present in the new snapshot context.  Add a new snapshot for
@@ -2631,6 +2672,7 @@ out_err:
 static int rbd_dev_v2_probe(struct rbd_device *rbd_dev)
 {
 	size_t size;
+	int ret;

 	/*
 	 * Image id was filled in by the caller.  Record the header
@@ -2642,12 +2684,23 @@ static int rbd_dev_v2_probe(struct rbd_device
*rbd_dev)
 		return -ENOMEM;
 	sprintf(rbd_dev->header_name, "%s%s",
 			RBD_HEADER_PREFIX, rbd_dev->image_id);
+
+	/* Get the size and object order for the image */
+
+	ret = rbd_dev_v2_image_size(rbd_dev);
+	if (ret < 0)
+		goto out_err;
 	rbd_dev->image_format = 2;

 	dout("discovered version 2 image, header name is %s\n",
 		rbd_dev->header_name);

 	return -ENOTSUPP;
+out_err:
+	kfree(rbd_dev->header_name);
+	rbd_dev->header_name = NULL;
+
+	return ret;
 }

 /*
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/9] rbd: get the object prefix for a v2 rbd image
  2012-09-07 21:09 [PATCH 0/9] rbd: activate v2 image support Alex Elder
  2012-09-07 21:13 ` [PATCH 1/9] rbd: lay out header probe infrastructure Alex Elder
  2012-09-07 21:13 ` [PATCH 2/9] rbd: add code to get the size of a v2 rbd image Alex Elder
@ 2012-09-07 21:13 ` Alex Elder
  2012-09-19 18:57   ` Josh Durgin
  2012-09-07 21:13 ` [PATCH 4/9] rbd: get image features for a v2 image Alex Elder
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Alex Elder @ 2012-09-07 21:13 UTC (permalink / raw)
  To: ceph-devel

The object prefix of an rbd format 2 image is fetched from the
server using a "get_object_prefix" method.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 24d12e3..36e848a 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -66,7 +66,8 @@

 #define RBD_SNAP_HEAD_NAME	"-"

-#define	RBD_IMAGE_ID_LEN_MAX	64
+#define RBD_IMAGE_ID_LEN_MAX	64
+#define RBD_OBJ_PREFIX_LEN_MAX	64

 /*
  * An RBD device name will be "rbd#", where the "rbd" comes from
@@ -2163,6 +2164,42 @@ static int rbd_dev_v2_image_size(struct
rbd_device *rbd_dev)
 					&rbd_dev->header.image_size);
 }

+static int rbd_dev_v2_object_prefix(struct rbd_device *rbd_dev)
+{
+	void *reply_buf;
+	int ret;
+	void *p;
+
+	reply_buf = kzalloc(RBD_OBJ_PREFIX_LEN_MAX, GFP_KERNEL);
+	if (!reply_buf)
+		return -ENOMEM;
+
+	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
+				"rbd", "get_object_prefix",
+				NULL, 0,
+				reply_buf, RBD_OBJ_PREFIX_LEN_MAX,
+				CEPH_OSD_FLAG_READ, NULL);
+	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
+	if (ret < 0)
+		goto out;
+
+	p = reply_buf;
+	rbd_dev->header.object_prefix = ceph_extract_encoded_string(&p,
+						p + RBD_OBJ_PREFIX_LEN_MAX,
+						NULL, GFP_NOIO);
+
+	if (IS_ERR(rbd_dev->header.object_prefix)) {
+		ret = PTR_ERR(rbd_dev->header.object_prefix);
+		rbd_dev->header.object_prefix = NULL;
+	} else
+		dout("  object_prefix = %s\n", rbd_dev->header.object_prefix);
+
+out:
+	kfree(reply_buf);
+
+	return ret;
+}
+
 /*
  * Scan the rbd device's current snapshot list and compare it to the
  * newly-received snapshot context.  Remove any existing snapshots
@@ -2690,6 +2727,12 @@ static int rbd_dev_v2_probe(struct rbd_device
*rbd_dev)
 	ret = rbd_dev_v2_image_size(rbd_dev);
 	if (ret < 0)
 		goto out_err;
+
+	/* Get the object prefix (a.k.a. block_name) for the image */
+
+	ret = rbd_dev_v2_object_prefix(rbd_dev);
+	if (ret < 0)
+		goto out_err;
 	rbd_dev->image_format = 2;

 	dout("discovered version 2 image, header name is %s\n",
@@ -2699,6 +2742,8 @@ static int rbd_dev_v2_probe(struct rbd_device
*rbd_dev)
 out_err:
 	kfree(rbd_dev->header_name);
 	rbd_dev->header_name = NULL;
+	kfree(rbd_dev->header.object_prefix);
+	rbd_dev->header.object_prefix = NULL;

 	return ret;
 }
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/9] rbd: get image features for a v2 image
  2012-09-07 21:09 [PATCH 0/9] rbd: activate v2 image support Alex Elder
                   ` (2 preceding siblings ...)
  2012-09-07 21:13 ` [PATCH 3/9] rbd: get the object prefix for " Alex Elder
@ 2012-09-07 21:13 ` Alex Elder
  2012-09-19 19:02   ` Josh Durgin
  2012-09-07 21:13 ` [PATCH 5/9] rbd: get the snapshot context " Alex Elder
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Alex Elder @ 2012-09-07 21:13 UTC (permalink / raw)
  To: ceph-devel

The features values for an rbd format 2 image are fetched from the
server using a "get_features" method.  The same method is used for
getting the features for a snapshot, so structure this addition with
a generic helper routine that can get this information for either.

The server will provide two 64-bit feature masks, one representing
the features used for this image (or snapshot) and one representing
features which cannot be used by the client when working with the
image (or snapshot).

For the time being, neither of these is really used so we keep
things simple and just record the first feature vector.  Once we
start using these feature masks, what we record and what we expose
to the user will most likely change.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 36e848a..d48f025 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2200,6 +2200,40 @@ out:
 	return ret;
 }

+static int _rbd_dev_v2_snap_features(struct rbd_device *rbd_dev, u64
snap_id,
+		u64 *snap_features)
+{
+	__le64 snapid = cpu_to_le64(snap_id);
+	struct {
+		__le64 features;
+		__le64 incompat;
+	} features_buf = { 0 };
+	int ret;
+
+	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
+				"rbd", "get_features",
+				(char *) &snapid, sizeof (snapid),
+				(char *) &features_buf, sizeof (features_buf),
+				CEPH_OSD_FLAG_READ, NULL);
+	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
+	if (ret < 0)
+		return ret;
+	*snap_features = le64_to_cpu(features_buf.features);
+
+	dout("  snap_id 0x%016llx features = 0x%016llx incompat = 0x%016llx\n",
+		(unsigned long long) snap_id,
+		(unsigned long long) *snap_features,
+		(unsigned long long) le64_to_cpu(features_buf.incompat));
+
+	return 0;
+}
+
+static int rbd_dev_v2_features(struct rbd_device *rbd_dev)
+{
+	return _rbd_dev_v2_snap_features(rbd_dev, CEPH_NOSNAP,
+						&rbd_dev->header.features);
+}
+
 /*
  * Scan the rbd device's current snapshot list and compare it to the
  * newly-received snapshot context.  Remove any existing snapshots
@@ -2733,6 +2767,12 @@ static int rbd_dev_v2_probe(struct rbd_device
*rbd_dev)
 	ret = rbd_dev_v2_object_prefix(rbd_dev);
 	if (ret < 0)
 		goto out_err;
+
+	/* Get the features for the image */
+
+	ret = rbd_dev_v2_features(rbd_dev);
+	if (ret < 0)
+		goto out_err;
 	rbd_dev->image_format = 2;

 	dout("discovered version 2 image, header name is %s\n",
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/9] rbd: get the snapshot context for a v2 image
  2012-09-07 21:09 [PATCH 0/9] rbd: activate v2 image support Alex Elder
                   ` (3 preceding siblings ...)
  2012-09-07 21:13 ` [PATCH 4/9] rbd: get image features for a v2 image Alex Elder
@ 2012-09-07 21:13 ` Alex Elder
  2012-09-19 19:17   ` Josh Durgin
  2012-09-07 21:14 ` [PATCH 6/9] rbd: get snapshot name " Alex Elder
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Alex Elder @ 2012-09-07 21:13 UTC (permalink / raw)
  To: ceph-devel

Fetch the snapshot context for an rbd format 2 image by calling
the "get_snapcontext" method on its header object.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   85
+++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index d48f025..8ff84fd 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -62,6 +62,7 @@
 #define RBD_MINORS_PER_MAJOR	256		/* max minors per blkdev */

 #define RBD_MAX_SNAP_NAME_LEN	32
+#define RBD_MAX_SNAP_COUNT	510	/* allows max snapc to fit in 4KB */
 #define RBD_MAX_OPT_LEN		1024

 #define RBD_SNAP_HEAD_NAME	"-"
@@ -2234,6 +2235,84 @@ static int rbd_dev_v2_features(struct rbd_device
*rbd_dev)
 						&rbd_dev->header.features);
 }

+static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev)
+{
+	size_t size;
+	int ret;
+	void *reply_buf;
+	void *p;
+	void *end;
+	u64 seq;
+	u32 snap_count;
+	struct ceph_snap_context *snapc;
+	u32 i;
+
+	/*
+	 * We'll need room for the seq value (maximum snapshot id),
+	 * snapshot count, and array of that many snapshot ids.
+	 * For now we have a fixed upper limit on the number we're
+	 * prepared to receive.
+	 */
+	size = sizeof (__le64) + sizeof (__le32) +
+			RBD_MAX_SNAP_COUNT * sizeof (__le64);
+	reply_buf = kzalloc(size, GFP_KERNEL);
+	if (!reply_buf)
+		return -ENOMEM;
+
+	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
+				"rbd", "get_snapcontext",
+				NULL, 0,
+				reply_buf, size,
+				CEPH_OSD_FLAG_READ, NULL);
+	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
+	if (ret < 0)
+		goto out;
+
+	ret = -ERANGE;
+	p = reply_buf;
+	end = (char *) reply_buf + size;
+	ceph_decode_64_safe(&p, end, seq, out);
+	ceph_decode_32_safe(&p, end, snap_count, out);
+
+	/*
+	 * Make sure the reported number of snapshot ids wouldn't go
+	 * beyond the end of our buffer.  But before checking that,
+	 * make sure the computed size of the snapshot context we
+	 * allocate is representable in a size_t.
+	 */
+	if (snap_count > (SIZE_MAX - sizeof (struct ceph_snap_context))
+				 / sizeof (u64)) {
+		ret = -EINVAL;
+		goto out;
+	}
+	if (!ceph_has_room(&p, end, snap_count * sizeof (__le64)))
+		goto out;
+
+	size = sizeof (struct ceph_snap_context) +
+				snap_count * sizeof (snapc->snaps[0]);
+	snapc = kmalloc(size, GFP_KERNEL);
+	if (!snapc) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	atomic_set(&snapc->nref, 1);
+	snapc->seq = seq;
+	snapc->num_snaps = snap_count;
+	for (i = 0; i < snap_count; i++)
+		snapc->snaps[i] = ceph_decode_64(&p);
+
+	rbd_dev->header.snapc = snapc;
+
+	dout("  snap context seq = %llu, snap_count = %u\n",
+		(unsigned long long) seq, (unsigned int) snap_count);
+
+out:
+	kfree(reply_buf);
+
+	return 0;
+}
+
 /*
  * Scan the rbd device's current snapshot list and compare it to the
  * newly-received snapshot context.  Remove any existing snapshots
@@ -2773,6 +2852,12 @@ static int rbd_dev_v2_probe(struct rbd_device
*rbd_dev)
 	ret = rbd_dev_v2_features(rbd_dev);
 	if (ret < 0)
 		goto out_err;
+
+	/* Get the snapshot context */
+
+	ret = rbd_dev_v2_snap_context(rbd_dev);
+	if (ret)
+		goto out_err;
 	rbd_dev->image_format = 2;

 	dout("discovered version 2 image, header name is %s\n",
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6/9] rbd: get snapshot name for a v2 image
  2012-09-07 21:09 [PATCH 0/9] rbd: activate v2 image support Alex Elder
                   ` (4 preceding siblings ...)
  2012-09-07 21:13 ` [PATCH 5/9] rbd: get the snapshot context " Alex Elder
@ 2012-09-07 21:14 ` Alex Elder
  2012-09-19 19:31   ` Josh Durgin
  2012-09-07 21:15 ` [PATCH 7/9] rbd: update remaining header fields for v2 Alex Elder
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Alex Elder @ 2012-09-07 21:14 UTC (permalink / raw)
  To: ceph-devel

Define rbd_dev_v2_snap_name() to fetch the name for a particular
snapshot in a format 2 rbd image.

Define rbd_dev_v2_snap_info() to to be a wrapper for getting the
name, size, and features for a particular snapshot, using an
interface that matches the equivalent function for version 1 images.

Define rbd_dev_snap_info() wrapper function and use it to call the
appropriate function for getting the snapshot name, size, and
features, dependent on the rbd image format.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   78
++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 77 insertions(+), 1 deletion(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 8ff84fd..c6922a1 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2313,6 +2313,82 @@ out:
 	return 0;
 }

+static char *rbd_dev_v2_snap_name(struct rbd_device *rbd_dev, u32 which)
+{
+	size_t size;
+	void *reply_buf;
+	__le64 snap_id;
+	int ret;
+	void *p;
+	void *end;
+	size_t snap_name_len;
+	char *snap_name;
+
+	size = sizeof (__le32) + RBD_MAX_SNAP_NAME_LEN;
+	reply_buf = kmalloc(size, GFP_KERNEL);
+	if (!reply_buf)
+		return ERR_PTR(-ENOMEM);
+
+	snap_id = cpu_to_le64(rbd_dev->header.snapc->snaps[which]);
+	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
+				"rbd", "get_snapshot_name",
+				(char *) &snap_id, sizeof (snap_id),
+				reply_buf, size,
+				CEPH_OSD_FLAG_READ, NULL);
+	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
+	if (ret < 0)
+		goto out;
+
+	p = reply_buf;
+	end = (char *) reply_buf + size;
+	snap_name_len = 0;
+	snap_name = ceph_extract_encoded_string(&p, end, &snap_name_len,
+				GFP_KERNEL);
+	if (IS_ERR(snap_name)) {
+		ret = PTR_ERR(snap_name);
+		goto out;
+	} else
+		dout("  snap_id 0x%016llx snap_name = %s\n",
+			(unsigned long long) le64_to_cpu(snap_id), snap_name);
+	kfree(reply_buf);
+
+	return snap_name;
+out:
+	kfree(reply_buf);
+
+	return ERR_PTR(ret);
+}
+
+static char *rbd_dev_v2_snap_info(struct rbd_device *rbd_dev, u32 which,
+		u64 *snap_size, u64 *snap_features)
+{
+	__le64 snap_id;
+	u8 order;
+	int ret;
+
+	snap_id = rbd_dev->header.snapc->snaps[which];
+	ret = _rbd_dev_v2_snap_size(rbd_dev, snap_id, &order, snap_size);
+	if (ret)
+		return ERR_PTR(ret);
+	ret = _rbd_dev_v2_snap_features(rbd_dev, snap_id, snap_features);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return rbd_dev_v2_snap_name(rbd_dev, which);
+}
+
+static char *rbd_dev_snap_info(struct rbd_device *rbd_dev, u32 which,
+		u64 *snap_size, u64 *snap_features)
+{
+	if (rbd_dev->image_format == 1)
+		return rbd_dev_v1_snap_info(rbd_dev, which,
+					snap_size, snap_features);
+	if (rbd_dev->image_format == 2)
+		return rbd_dev_v2_snap_info(rbd_dev, which,
+					snap_size, snap_features);
+	return ERR_PTR(-EINVAL);
+}
+
 /*
  * Scan the rbd device's current snapshot list and compare it to the
  * newly-received snapshot context.  Remove any existing snapshots
@@ -2366,7 +2442,7 @@ static int rbd_dev_snaps_update(struct rbd_device
*rbd_dev)
 			continue;
 		}

-		snap_name = rbd_dev_v1_snap_info(rbd_dev, index,
+		snap_name = rbd_dev_snap_info(rbd_dev, index,
 						&snap_size, &snap_features);
 		if (IS_ERR(snap_name))
 			return PTR_ERR(snap_name);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 7/9] rbd: update remaining header fields for v2
  2012-09-07 21:09 [PATCH 0/9] rbd: activate v2 image support Alex Elder
                   ` (5 preceding siblings ...)
  2012-09-07 21:14 ` [PATCH 6/9] rbd: get snapshot name " Alex Elder
@ 2012-09-07 21:15 ` Alex Elder
  2012-09-19 19:38   ` Josh Durgin
  2012-09-07 21:15 ` [PATCH 8/9] rbd: define rbd_dev_v2_snapc_refresh() Alex Elder
  2012-09-07 21:15 ` [PATCH 9/9] rbd: activate v2 image support Alex Elder
  8 siblings, 1 reply; 20+ messages in thread
From: Alex Elder @ 2012-09-07 21:15 UTC (permalink / raw)
  To: ceph-devel

There are three fields that are not yet updated for format 2 rbd
image headers:  the version of the header object; the encryption
type; and the compression type.  There is no interface defined for
fetching the latter two, so just initialize them explicitly to 0 for
now.

Change rbd_dev_v2_snap_context() so the caller can be supplied the
version for the header object.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index c6922a1..ad27be2 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2235,7 +2235,7 @@ static int rbd_dev_v2_features(struct rbd_device
*rbd_dev)
 						&rbd_dev->header.features);
 }

-static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev)
+static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev, u64 *ver)
 {
 	size_t size;
 	int ret;
@@ -2263,7 +2263,7 @@ static int rbd_dev_v2_snap_context(struct
rbd_device *rbd_dev)
 				"rbd", "get_snapcontext",
 				NULL, 0,
 				reply_buf, size,
-				CEPH_OSD_FLAG_READ, NULL);
+				CEPH_OSD_FLAG_READ, ver);
 	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
 	if (ret < 0)
 		goto out;
@@ -2899,6 +2899,7 @@ static int rbd_dev_v2_probe(struct rbd_device
*rbd_dev)
 {
 	size_t size;
 	int ret;
+	u64 ver = 0;

 	/*
 	 * Image id was filled in by the caller.  Record the header
@@ -2929,11 +2930,18 @@ static int rbd_dev_v2_probe(struct rbd_device
*rbd_dev)
 	if (ret < 0)
 		goto out_err;

-	/* Get the snapshot context */
+	/* crypto and compression type aren't (yet) supported for v2 images */
+
+	rbd_dev->header.crypt_type = 0;
+	rbd_dev->header.comp_type = 0;

-	ret = rbd_dev_v2_snap_context(rbd_dev);
+	/* Get the snapshot context, plus the header version */
+
+	ret = rbd_dev_v2_snap_context(rbd_dev, &ver);
 	if (ret)
 		goto out_err;
+	rbd_dev->header.obj_version = ver;
+
 	rbd_dev->image_format = 2;

 	dout("discovered version 2 image, header name is %s\n",
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 8/9] rbd: define rbd_dev_v2_snapc_refresh()
  2012-09-07 21:09 [PATCH 0/9] rbd: activate v2 image support Alex Elder
                   ` (6 preceding siblings ...)
  2012-09-07 21:15 ` [PATCH 7/9] rbd: update remaining header fields for v2 Alex Elder
@ 2012-09-07 21:15 ` Alex Elder
  2012-09-19 21:00   ` Josh Durgin
  2012-09-07 21:15 ` [PATCH 9/9] rbd: activate v2 image support Alex Elder
  8 siblings, 1 reply; 20+ messages in thread
From: Alex Elder @ 2012-09-07 21:15 UTC (permalink / raw)
  To: ceph-devel

Define a new function rbd_dev_v2_snapc_refresh() to update/refresh
the snapshot context for a format version 2 rbd image.

Update rbd_refresh_header() so it selects which function to use
based on the image format.

Rename __rbd_refresh_header() to be rbd_dev_v1_snapc_refresh()
to be consistent with the naming of its version 2 counterpart.
Similarly rename rbd_refresh_header to be rbd_dev_snapc_refresh().

Unrelated--we use rbd_image_format_valid() here.  Delete the other
use of it, which was primarily put in place to ensure that function
was referenced at the time it was defined.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index ad27be2..b466393 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -268,7 +268,8 @@ static void rbd_put_dev(struct rbd_device *rbd_dev)
 	put_device(&rbd_dev->dev);
 }

-static int rbd_refresh_header(struct rbd_device *rbd_dev, u64 *hver);
+static int rbd_dev_snapc_refresh(struct rbd_device *rbd_dev, u64 *hver);
+static int rbd_dev_v2_snapc_refresh(struct rbd_device *rbd_dev, u64 *hver);

 static int rbd_open(struct block_device *bdev, fmode_t mode)
 {
@@ -1303,7 +1304,7 @@ static void rbd_watch_cb(u64 ver, u64 notify_id,
u8 opcode, void *data)
 	dout("rbd_watch_cb %s notify_id=%llu opcode=%u\n",
 		rbd_dev->header_name, (unsigned long long) notify_id,
 		(unsigned int) opcode);
-	rc = rbd_refresh_header(rbd_dev, &hver);
+	rc = rbd_dev_snapc_refresh(rbd_dev, &hver);
 	if (rc)
 		pr_warning(RBD_DRV_NAME "%d got notification but failed to "
 			   " update snaps: %d\n", rbd_dev->major, rc);
@@ -1718,7 +1719,7 @@ static void __rbd_remove_all_snaps(struct
rbd_device *rbd_dev)
 /*
  * only read the first part of the ondisk header, without the snaps info
  */
-static int __rbd_refresh_header(struct rbd_device *rbd_dev, u64 *hver)
+static int rbd_dev_v1_snapc_refresh(struct rbd_device *rbd_dev, u64 *hver)
 {
 	int ret;
 	struct rbd_image_header h;
@@ -1767,12 +1768,16 @@ static int __rbd_refresh_header(struct
rbd_device *rbd_dev, u64 *hver)
 	return ret;
 }

-static int rbd_refresh_header(struct rbd_device *rbd_dev, u64 *hver)
+static int rbd_dev_snapc_refresh(struct rbd_device *rbd_dev, u64 *hver)
 {
 	int ret;

+	rbd_assert(rbd_image_format_valid(rbd_dev->image_format));
 	mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
-	ret = __rbd_refresh_header(rbd_dev, hver);
+	if (rbd_dev->image_format == 1)
+		ret = rbd_dev_v1_snapc_refresh(rbd_dev, hver);
+	else
+		ret = rbd_dev_v2_snapc_refresh(rbd_dev, hver);
 	mutex_unlock(&ctl_mutex);

 	return ret;
@@ -1932,7 +1937,7 @@ static ssize_t rbd_image_refresh(struct device *dev,
 	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
 	int ret;

-	ret = rbd_refresh_header(rbd_dev, NULL);
+	ret = rbd_dev_snapc_refresh(rbd_dev, NULL);

 	return ret < 0 ? ret : size;
 }
@@ -2389,6 +2394,27 @@ static char *rbd_dev_snap_info(struct rbd_device
*rbd_dev, u32 which,
 	return ERR_PTR(-EINVAL);
 }

+static int rbd_dev_v2_snapc_refresh(struct rbd_device *rbd_dev, u64 *hver)
+{
+	int ret;
+
+	down_write(&rbd_dev->header_rwsem);
+	ret = rbd_dev_v2_snap_context(rbd_dev, hver);
+	dout("rbd_dev_v2_snap_context returned %d\n", ret);
+	if (ret)
+		goto out;
+	ret = rbd_dev_snaps_update(rbd_dev);
+	dout("rbd_dev_snaps_update returned %d\n", ret);
+	if (ret)
+		goto out;
+	ret = rbd_dev_snaps_register(rbd_dev);
+	dout("rbd_dev_snaps_register returned %d\n", ret);
+out:
+	up_write(&rbd_dev->header_rwsem);
+
+	return 0;
+}
+
 /*
  * Scan the rbd device's current snapshot list and compare it to the
  * newly-received snapshot context.  Remove any existing snapshots
@@ -2551,7 +2577,7 @@ static int rbd_init_watch_dev(struct rbd_device
*rbd_dev)
 	do {
 		ret = rbd_req_sync_watch(rbd_dev);
 		if (ret == -ERANGE) {
-			rc = rbd_refresh_header(rbd_dev, NULL);
+			rc = rbd_dev_snapc_refresh(rbd_dev, NULL);
 			if (rc < 0)
 				return rc;
 		}
@@ -3034,7 +3060,6 @@ static ssize_t rbd_add(struct bus_type *bus,
 	rc = rbd_dev_probe(rbd_dev);
 	if (rc < 0)
 		goto err_out_client;
-	rbd_assert(rbd_image_format_valid(rbd_dev->image_format));

 	rc = rbd_dev_snaps_update(rbd_dev);
 	if (rc)
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 9/9] rbd: activate v2 image support
  2012-09-07 21:09 [PATCH 0/9] rbd: activate v2 image support Alex Elder
                   ` (7 preceding siblings ...)
  2012-09-07 21:15 ` [PATCH 8/9] rbd: define rbd_dev_v2_snapc_refresh() Alex Elder
@ 2012-09-07 21:15 ` Alex Elder
  2012-09-19 19:56   ` Josh Durgin
  8 siblings, 1 reply; 20+ messages in thread
From: Alex Elder @ 2012-09-07 21:15 UTC (permalink / raw)
  To: ceph-devel

Now that v2 images support is fully implemented, have
rbd_dev_v2_probe() return 0 to indicate a successful probe.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index b466393..2f83f71 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2973,7 +2973,7 @@ static int rbd_dev_v2_probe(struct rbd_device
*rbd_dev)
 	dout("discovered version 2 image, header name is %s\n",
 		rbd_dev->header_name);

-	return -ENOTSUPP;
+	return 0;
 out_err:
 	kfree(rbd_dev->header_name);
 	rbd_dev->header_name = NULL;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/9] rbd: lay out header probe infrastructure
  2012-09-07 21:13 ` [PATCH 1/9] rbd: lay out header probe infrastructure Alex Elder
@ 2012-09-19 18:35   ` Josh Durgin
  0 siblings, 0 replies; 20+ messages in thread
From: Josh Durgin @ 2012-09-19 18:35 UTC (permalink / raw)
  To: Alex Elder; +Cc: ceph-devel

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 09/07/2012 02:13 PM, Alex Elder wrote:
> This defines a new function rbd_dev_probe() as a top-level
> function for populating detailed information about an rbd device.
>
> It first checks for the existence of a format 2 rbd image id object.
> If it exists, the image is assumed to be a format 2 rbd image, and
> another function rbd_dev_v2() is called to finish populating
> header data for that image.  If it does not exist, it is assumed to
> be an old (format 1) rbd image, and calls a similar function
> rbd_dev_v1() to populate its header information.
>
> A new field, rbd_dev->format, is defined to record which version
> of the rbd image format the device represents.  For a valid mapped
> rbd device it will have one of two values, 1 or 2.
>
> So far, the format 2 images are not really supported; this is
> laying out the infrastructure for fleshing out that support.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |  126
> ++++++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 99 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index e6b8fa6..019e695 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -170,6 +170,7 @@ struct rbd_device {
>   	int			major;		/* blkdev assigned major */
>   	struct gendisk		*disk;		/* blkdev's gendisk and rq */
>
> +	u32			image_format;	/* Either 1 or 2 */
>   	struct rbd_options	rbd_opts;
>   	struct rbd_client	*rbd_client;
>
> @@ -507,6 +508,11 @@ static void rbd_coll_release(struct kref *kref)
>   	kfree(coll);
>   }
>
> +static bool rbd_image_format_valid(u32 image_format)
> +{
> +	return image_format == 1 || image_format == 2;
> +}
> +
>   static bool rbd_dev_ondisk_valid(struct rbd_image_header_ondisk *ondisk)
>   {
>   	size_t size;
> @@ -2579,6 +2585,96 @@ out:
>   	return ret;
>   }
>
> +static int rbd_dev_v1_probe(struct rbd_device *rbd_dev)
> +{
> +	int ret;
> +	size_t size;
> +
> +	/* Version 1 images have no id; empty string is used */
> +
> +	rbd_dev->image_id = kstrdup("", GFP_KERNEL);
> +	if (!rbd_dev->image_id)
> +		return -ENOMEM;
> +	rbd_dev->image_id_len = 0;
> +
> +	/* Record the header object name for this rbd image. */
> +
> +	size = rbd_dev->image_name_len + sizeof (RBD_SUFFIX);
> +	rbd_dev->header_name = kmalloc(size, GFP_KERNEL);
> +	if (!rbd_dev->header_name) {
> +		ret = -ENOMEM;
> +		goto out_err;
> +	}
> +	sprintf(rbd_dev->header_name, "%s%s", rbd_dev->image_name, RBD_SUFFIX);
> +
> +	/* Populate rbd image metadata */
> +
> +	ret = rbd_read_header(rbd_dev, &rbd_dev->header);
> +	if (ret < 0)
> +		goto out_err;
> +	rbd_dev->image_format = 1;
> +
> +	dout("discovered version 1 image, header name is %s\n",
> +		rbd_dev->header_name);
> +
> +	return 0;
> +
> +out_err:
> +	kfree(rbd_dev->header_name);
> +	rbd_dev->header_name = NULL;
> +	kfree(rbd_dev->image_id);
> +	rbd_dev->image_id = NULL;
> +
> +	return ret;
> +}
> +
> +static int rbd_dev_v2_probe(struct rbd_device *rbd_dev)
> +{
> +	size_t size;
> +
> +	/*
> +	 * Image id was filled in by the caller.  Record the header
> +	 * object name for this rbd image.
> +	 */
> +	size = sizeof (RBD_HEADER_PREFIX) + rbd_dev->image_id_len;
> +	rbd_dev->header_name = kmalloc(size, GFP_KERNEL);
> +	if (!rbd_dev->header_name)
> +		return -ENOMEM;
> +	sprintf(rbd_dev->header_name, "%s%s",
> +			RBD_HEADER_PREFIX, rbd_dev->image_id);
> +	rbd_dev->image_format = 2;
> +
> +	dout("discovered version 2 image, header name is %s\n",
> +		rbd_dev->header_name);
> +
> +	return -ENOTSUPP;
> +}
> +
> +/*
> + * Probe for the existence of the header object for the given rbd
> + * device.  For format 2 images this includes determining the image
> + * id.
> + */
> +static int rbd_dev_probe(struct rbd_device *rbd_dev)
> +{
> +	int ret;
> +
> +	/*
> +	 * Get the id from the image id object.  If it's not a
> +	 * format 2 image, we'll get ENOENT back, and we'll assume
> +	 * it's a format 1 image.
> +	 */
> +	ret = rbd_dev_image_id(rbd_dev);
> +	if (ret == -ENOENT)
> +		ret = rbd_dev_v1_probe(rbd_dev);
> +	else if (!ret)
> +		ret = rbd_dev_v2_probe(rbd_dev);
> +	if (ret)
> +		dout("probe failed, returning %d\n", ret);
> +
> +	return ret;
> +}
> +
>   static ssize_t rbd_add(struct bus_type *bus,
>   		       const char *buf,
>   		       size_t count)
> @@ -2628,34 +2724,10 @@ static ssize_t rbd_add(struct bus_type *bus,
>   		goto err_out_client;
>   	rbd_dev->pool_id = rc;
>
> -	rc = rbd_dev_image_id(rbd_dev);
> -	if (rc == -ENOENT) {
> -		/* Version 1 images have no id; empty string is used */
> -		rbd_dev->image_id = kstrdup("", GFP_KERNEL);
> -		if (!rbd_dev->image_id) {
> -			rc = -ENOMEM;
> -			goto err_out_client;
> -		}
> -		rbd_dev->image_id_len = 0;
> -	} else {
> -	        /* Not actually supporting format 2 yet */
> -		goto err_out_client;
> -	}
> -
> -	/* Create the name of the header object */
> -
> -	rbd_dev->header_name = kmalloc(rbd_dev->image_name_len
> -						+ sizeof (RBD_SUFFIX),
> -					GFP_KERNEL);
> -	if (!rbd_dev->header_name)
> -		goto err_out_client;
> -	sprintf(rbd_dev->header_name, "%s%s", rbd_dev->image_name, RBD_SUFFIX);
> -
> -	/* Get information about the image being mapped */
> -
> -	rc = rbd_read_header(rbd_dev, &rbd_dev->header);
> -	if (rc)
> +	rc = rbd_dev_probe(rbd_dev);
> +	if (rc < 0)
>   		goto err_out_client;
> +	rbd_assert(rbd_image_format_valid(rbd_dev->image_format));
>
>   	rc = rbd_dev_snaps_update(rbd_dev);
>   	if (rc)
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/9]  rbd: add code to get the size of a v2 rbd image
  2012-09-07 21:13 ` [PATCH 2/9] rbd: add code to get the size of a v2 rbd image Alex Elder
@ 2012-09-19 18:52   ` Josh Durgin
  0 siblings, 0 replies; 20+ messages in thread
From: Josh Durgin @ 2012-09-19 18:52 UTC (permalink / raw)
  To: Alex Elder; +Cc: ceph-devel

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 09/07/2012 02:13 PM, Alex Elder wrote:
> The size of an rbd format 2 image is fetched from the server using a
> "get_size" method.  The same method is used for getting the size of
> a snapshot, so structure this addition with a generic helper routine
> that we can get this information for either.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   53
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 53 insertions(+)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 019e695..24d12e3 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2123,6 +2123,47 @@ static char *rbd_dev_v1_snap_info(struct
> rbd_device *rbd_dev, u32 which,
>   }
>
>   /*
> + * Get the size and object order for an image snapshot, or if
> + * snap_id is CEPH_NOSNAP, gets this information for the base
> + * image.
> + */
> +static int _rbd_dev_v2_snap_size(struct rbd_device *rbd_dev, u64 snap_id,
> +				u8 *order, u64 *snap_size)
> +{
> +	__le64 snapid = cpu_to_le64(snap_id);
> +	int ret;
> +	struct {
> +		u8 order;
> +		__le64 size;
> +	} __attribute__ ((packed)) size_buf = { 0 };
> +
> +	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
> +				"rbd", "get_size",
> +				(char *) &snapid, sizeof (snapid),
> +				(char *) &size_buf, sizeof (size_buf),
> +				CEPH_OSD_FLAG_READ, NULL);
> +	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
> +	if (ret < 0)
> +		return ret;
> +
> +	*order = size_buf.order;
> +	*snap_size = le64_to_cpu(size_buf.size);
> +
> +	dout("  snap_id 0x%016llx order = %u, snap_size = %llu\n",
> +		(unsigned long long) snap_id, (unsigned int) *order,
> +		(unsigned long long) *snap_size);
> +
> +	return 0;
> +}
> +
> +static int rbd_dev_v2_image_size(struct rbd_device *rbd_dev)
> +{
> +	return _rbd_dev_v2_snap_size(rbd_dev, CEPH_NOSNAP,
> +					&rbd_dev->header.obj_order,
> +					&rbd_dev->header.image_size);
> +}
> +
> +/*
>    * Scan the rbd device's current snapshot list and compare it to the
>    * newly-received snapshot context.  Remove any existing snapshots
>    * not present in the new snapshot context.  Add a new snapshot for
> @@ -2631,6 +2672,7 @@ out_err:
>   static int rbd_dev_v2_probe(struct rbd_device *rbd_dev)
>   {
>   	size_t size;
> +	int ret;
>
>   	/*
>   	 * Image id was filled in by the caller.  Record the header
> @@ -2642,12 +2684,23 @@ static int rbd_dev_v2_probe(struct rbd_device
> *rbd_dev)
>   		return -ENOMEM;
>   	sprintf(rbd_dev->header_name, "%s%s",
>   			RBD_HEADER_PREFIX, rbd_dev->image_id);
> +
> +	/* Get the size and object order for the image */
> +
> +	ret = rbd_dev_v2_image_size(rbd_dev);
> +	if (ret < 0)
> +		goto out_err;
>   	rbd_dev->image_format = 2;
>
>   	dout("discovered version 2 image, header name is %s\n",
>   		rbd_dev->header_name);
>
>   	return -ENOTSUPP;
> +out_err:
> +	kfree(rbd_dev->header_name);
> +	rbd_dev->header_name = NULL;
> +
> +	return ret;
>   }
>
>   /*
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/9] rbd: get the object prefix for a v2 rbd image
  2012-09-07 21:13 ` [PATCH 3/9] rbd: get the object prefix for " Alex Elder
@ 2012-09-19 18:57   ` Josh Durgin
  0 siblings, 0 replies; 20+ messages in thread
From: Josh Durgin @ 2012-09-19 18:57 UTC (permalink / raw)
  To: Alex Elder; +Cc: ceph-devel

One minor nit below. Otherwise looks good to me.

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 09/07/2012 02:13 PM, Alex Elder wrote:
> The object prefix of an rbd format 2 image is fetched from the
> server using a "get_object_prefix" method.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   47 ++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 24d12e3..36e848a 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -66,7 +66,8 @@
>
>   #define RBD_SNAP_HEAD_NAME	"-"
>
> -#define	RBD_IMAGE_ID_LEN_MAX	64
> +#define RBD_IMAGE_ID_LEN_MAX	64
> +#define RBD_OBJ_PREFIX_LEN_MAX	64
>
>   /*
>    * An RBD device name will be "rbd#", where the "rbd" comes from
> @@ -2163,6 +2164,42 @@ static int rbd_dev_v2_image_size(struct
> rbd_device *rbd_dev)
>   					&rbd_dev->header.image_size);
>   }
>
> +static int rbd_dev_v2_object_prefix(struct rbd_device *rbd_dev)
> +{
> +	void *reply_buf;
> +	int ret;
> +	void *p;
> +
> +	reply_buf = kzalloc(RBD_OBJ_PREFIX_LEN_MAX, GFP_KERNEL);
> +	if (!reply_buf)
> +		return -ENOMEM;
> +
> +	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
> +				"rbd", "get_object_prefix",
> +				NULL, 0,
> +				reply_buf, RBD_OBJ_PREFIX_LEN_MAX,
> +				CEPH_OSD_FLAG_READ, NULL);
> +	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
> +	if (ret < 0)
> +		goto out;
> +
> +	p = reply_buf;
> +	rbd_dev->header.object_prefix = ceph_extract_encoded_string(&p,
> +						p + RBD_OBJ_PREFIX_LEN_MAX,
> +						NULL, GFP_NOIO);
> +
> +	if (IS_ERR(rbd_dev->header.object_prefix)) {
> +		ret = PTR_ERR(rbd_dev->header.object_prefix);
> +		rbd_dev->header.object_prefix = NULL;
> +	} else
> +		dout("  object_prefix = %s\n", rbd_dev->header.object_prefix);

The else block should use braces as well.

> +
> +out:
> +	kfree(reply_buf);
> +
> +	return ret;
> +}
> +
>   /*
>    * Scan the rbd device's current snapshot list and compare it to the
>    * newly-received snapshot context.  Remove any existing snapshots
> @@ -2690,6 +2727,12 @@ static int rbd_dev_v2_probe(struct rbd_device
> *rbd_dev)
>   	ret = rbd_dev_v2_image_size(rbd_dev);
>   	if (ret < 0)
>   		goto out_err;
> +
> +	/* Get the object prefix (a.k.a. block_name) for the image */
> +
> +	ret = rbd_dev_v2_object_prefix(rbd_dev);
> +	if (ret < 0)
> +		goto out_err;
>   	rbd_dev->image_format = 2;
>
>   	dout("discovered version 2 image, header name is %s\n",
> @@ -2699,6 +2742,8 @@ static int rbd_dev_v2_probe(struct rbd_device
> *rbd_dev)
>   out_err:
>   	kfree(rbd_dev->header_name);
>   	rbd_dev->header_name = NULL;
> +	kfree(rbd_dev->header.object_prefix);
> +	rbd_dev->header.object_prefix = NULL;
>
>   	return ret;
>   }
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/9] rbd: get image features for a v2 image
  2012-09-07 21:13 ` [PATCH 4/9] rbd: get image features for a v2 image Alex Elder
@ 2012-09-19 19:02   ` Josh Durgin
  0 siblings, 0 replies; 20+ messages in thread
From: Josh Durgin @ 2012-09-19 19:02 UTC (permalink / raw)
  To: Alex Elder; +Cc: ceph-devel

On 09/07/2012 02:13 PM, Alex Elder wrote:
> The features values for an rbd format 2 image are fetched from the
> server using a "get_features" method.  The same method is used for
> getting the features for a snapshot, so structure this addition with
> a generic helper routine that can get this information for either.
>
> The server will provide two 64-bit feature masks, one representing
> the features used for this image (or snapshot) and one representing
> features which cannot be used by the client when working with the
> image (or snapshot).

This second mask is features that are required for the client to
access the image correctly. They are called incompatible because
the are the subset of features used for this image that are not
backwards compatible with clients that do not support them.

With the commit message fixed:

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

> For the time being, neither of these is really used so we keep
> things simple and just record the first feature vector.  Once we
> start using these feature masks, what we record and what we expose
> to the user will most likely change.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   40 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 40 insertions(+)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 36e848a..d48f025 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2200,6 +2200,40 @@ out:
>   	return ret;
>   }
>
> +static int _rbd_dev_v2_snap_features(struct rbd_device *rbd_dev, u64
> snap_id,
> +		u64 *snap_features)
> +{
> +	__le64 snapid = cpu_to_le64(snap_id);
> +	struct {
> +		__le64 features;
> +		__le64 incompat;
> +	} features_buf = { 0 };
> +	int ret;
> +
> +	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
> +				"rbd", "get_features",
> +				(char *) &snapid, sizeof (snapid),
> +				(char *) &features_buf, sizeof (features_buf),
> +				CEPH_OSD_FLAG_READ, NULL);
> +	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
> +	if (ret < 0)
> +		return ret;
> +	*snap_features = le64_to_cpu(features_buf.features);
> +
> +	dout("  snap_id 0x%016llx features = 0x%016llx incompat = 0x%016llx\n",
> +		(unsigned long long) snap_id,
> +		(unsigned long long) *snap_features,
> +		(unsigned long long) le64_to_cpu(features_buf.incompat));
> +
> +	return 0;
> +}
> +
> +static int rbd_dev_v2_features(struct rbd_device *rbd_dev)
> +{
> +	return _rbd_dev_v2_snap_features(rbd_dev, CEPH_NOSNAP,
> +						&rbd_dev->header.features);
> +}
> +
>   /*
>    * Scan the rbd device's current snapshot list and compare it to the
>    * newly-received snapshot context.  Remove any existing snapshots
> @@ -2733,6 +2767,12 @@ static int rbd_dev_v2_probe(struct rbd_device
> *rbd_dev)
>   	ret = rbd_dev_v2_object_prefix(rbd_dev);
>   	if (ret < 0)
>   		goto out_err;
> +
> +	/* Get the features for the image */
> +
> +	ret = rbd_dev_v2_features(rbd_dev);
> +	if (ret < 0)
> +		goto out_err;
>   	rbd_dev->image_format = 2;
>
>   	dout("discovered version 2 image, header name is %s\n",
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/9] rbd: get the snapshot context for a v2 image
  2012-09-07 21:13 ` [PATCH 5/9] rbd: get the snapshot context " Alex Elder
@ 2012-09-19 19:17   ` Josh Durgin
  0 siblings, 0 replies; 20+ messages in thread
From: Josh Durgin @ 2012-09-19 19:17 UTC (permalink / raw)
  To: Alex Elder; +Cc: ceph-devel

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 09/07/2012 02:13 PM, Alex Elder wrote:
> Fetch the snapshot context for an rbd format 2 image by calling
> the "get_snapcontext" method on its header object.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   85
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 85 insertions(+)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index d48f025..8ff84fd 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -62,6 +62,7 @@
>   #define RBD_MINORS_PER_MAJOR	256		/* max minors per blkdev */
>
>   #define RBD_MAX_SNAP_NAME_LEN	32
> +#define RBD_MAX_SNAP_COUNT	510	/* allows max snapc to fit in 4KB */
>   #define RBD_MAX_OPT_LEN		1024
>
>   #define RBD_SNAP_HEAD_NAME	"-"
> @@ -2234,6 +2235,84 @@ static int rbd_dev_v2_features(struct rbd_device
> *rbd_dev)
>   						&rbd_dev->header.features);
>   }
>
> +static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev)
> +{
> +	size_t size;
> +	int ret;
> +	void *reply_buf;
> +	void *p;
> +	void *end;
> +	u64 seq;
> +	u32 snap_count;
> +	struct ceph_snap_context *snapc;
> +	u32 i;
> +
> +	/*
> +	 * We'll need room for the seq value (maximum snapshot id),
> +	 * snapshot count, and array of that many snapshot ids.
> +	 * For now we have a fixed upper limit on the number we're
> +	 * prepared to receive.
> +	 */
> +	size = sizeof (__le64) + sizeof (__le32) +
> +			RBD_MAX_SNAP_COUNT * sizeof (__le64);
> +	reply_buf = kzalloc(size, GFP_KERNEL);
> +	if (!reply_buf)
> +		return -ENOMEM;
> +
> +	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
> +				"rbd", "get_snapcontext",
> +				NULL, 0,
> +				reply_buf, size,
> +				CEPH_OSD_FLAG_READ, NULL);
> +	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = -ERANGE;
> +	p = reply_buf;
> +	end = (char *) reply_buf + size;
> +	ceph_decode_64_safe(&p, end, seq, out);
> +	ceph_decode_32_safe(&p, end, snap_count, out);
> +
> +	/*
> +	 * Make sure the reported number of snapshot ids wouldn't go
> +	 * beyond the end of our buffer.  But before checking that,
> +	 * make sure the computed size of the snapshot context we
> +	 * allocate is representable in a size_t.
> +	 */
> +	if (snap_count > (SIZE_MAX - sizeof (struct ceph_snap_context))
> +				 / sizeof (u64)) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +	if (!ceph_has_room(&p, end, snap_count * sizeof (__le64)))
> +		goto out;
> +
> +	size = sizeof (struct ceph_snap_context) +
> +				snap_count * sizeof (snapc->snaps[0]);
> +	snapc = kmalloc(size, GFP_KERNEL);
> +	if (!snapc) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	atomic_set(&snapc->nref, 1);
> +	snapc->seq = seq;
> +	snapc->num_snaps = snap_count;
> +	for (i = 0; i < snap_count; i++)
> +		snapc->snaps[i] = ceph_decode_64(&p);
> +
> +	rbd_dev->header.snapc = snapc;
> +
> +	dout("  snap context seq = %llu, snap_count = %u\n",
> +		(unsigned long long) seq, (unsigned int) snap_count);
> +
> +out:
> +	kfree(reply_buf);
> +
> +	return 0;
> +}
> +
>   /*
>    * Scan the rbd device's current snapshot list and compare it to the
>    * newly-received snapshot context.  Remove any existing snapshots
> @@ -2773,6 +2852,12 @@ static int rbd_dev_v2_probe(struct rbd_device
> *rbd_dev)
>   	ret = rbd_dev_v2_features(rbd_dev);
>   	if (ret < 0)
>   		goto out_err;
> +
> +	/* Get the snapshot context */
> +
> +	ret = rbd_dev_v2_snap_context(rbd_dev);
> +	if (ret)
> +		goto out_err;
>   	rbd_dev->image_format = 2;
>
>   	dout("discovered version 2 image, header name is %s\n",
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6/9] rbd: get snapshot name for a v2 image
  2012-09-07 21:14 ` [PATCH 6/9] rbd: get snapshot name " Alex Elder
@ 2012-09-19 19:31   ` Josh Durgin
  0 siblings, 0 replies; 20+ messages in thread
From: Josh Durgin @ 2012-09-19 19:31 UTC (permalink / raw)
  To: Alex Elder; +Cc: ceph-devel

A couple small things, otherwise:

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 09/07/2012 02:14 PM, Alex Elder wrote:
> Define rbd_dev_v2_snap_name() to fetch the name for a particular
> snapshot in a format 2 rbd image.
>
> Define rbd_dev_v2_snap_info() to to be a wrapper for getting the
> name, size, and features for a particular snapshot, using an
> interface that matches the equivalent function for version 1 images.
>
> Define rbd_dev_snap_info() wrapper function and use it to call the
> appropriate function for getting the snapshot name, size, and
> features, dependent on the rbd image format.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   78
> ++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 8ff84fd..c6922a1 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2313,6 +2313,82 @@ out:
>   	return 0;
>   }
>
> +static char *rbd_dev_v2_snap_name(struct rbd_device *rbd_dev, u32 which)
> +{
> +	size_t size;
> +	void *reply_buf;
> +	__le64 snap_id;
> +	int ret;
> +	void *p;
> +	void *end;
> +	size_t snap_name_len;
> +	char *snap_name;
> +
> +	size = sizeof (__le32) + RBD_MAX_SNAP_NAME_LEN;
> +	reply_buf = kmalloc(size, GFP_KERNEL);
> +	if (!reply_buf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	snap_id = cpu_to_le64(rbd_dev->header.snapc->snaps[which]);
> +	ret = rbd_req_sync_exec(rbd_dev, rbd_dev->header_name,
> +				"rbd", "get_snapshot_name",
> +				(char *) &snap_id, sizeof (snap_id),
> +				reply_buf, size,
> +				CEPH_OSD_FLAG_READ, NULL);
> +	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
> +	if (ret < 0)
> +		goto out;
> +
> +	p = reply_buf;
> +	end = (char *) reply_buf + size;
> +	snap_name_len = 0;
> +	snap_name = ceph_extract_encoded_string(&p, end, &snap_name_len,
> +				GFP_KERNEL);
> +	if (IS_ERR(snap_name)) {
> +		ret = PTR_ERR(snap_name);
> +		goto out;
> +	} else
> +		dout("  snap_id 0x%016llx snap_name = %s\n",
> +			(unsigned long long) le64_to_cpu(snap_id), snap_name);

else block should have braces to match the if block.

> +	kfree(reply_buf);
> +
> +	return snap_name;
> +out:
> +	kfree(reply_buf);
> +
> +	return ERR_PTR(ret);
> +}
> +
> +static char *rbd_dev_v2_snap_info(struct rbd_device *rbd_dev, u32 which,
> +		u64 *snap_size, u64 *snap_features)
> +{
> +	__le64 snap_id;
> +	u8 order;
> +	int ret;
> +
> +	snap_id = rbd_dev->header.snapc->snaps[which];
> +	ret = _rbd_dev_v2_snap_size(rbd_dev, snap_id, &order, snap_size);
> +	if (ret)
> +		return ERR_PTR(ret);
> +	ret = _rbd_dev_v2_snap_features(rbd_dev, snap_id, snap_features);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return rbd_dev_v2_snap_name(rbd_dev, which);
> +}
> +
> +static char *rbd_dev_snap_info(struct rbd_device *rbd_dev, u32 which,
> +		u64 *snap_size, u64 *snap_features)
> +{
> +	if (rbd_dev->image_format == 1)
> +		return rbd_dev_v1_snap_info(rbd_dev, which,
> +					snap_size, snap_features);
> +	if (rbd_dev->image_format == 2)
> +		return rbd_dev_v2_snap_info(rbd_dev, which,
> +					snap_size, snap_features);
> +	return ERR_PTR(-EINVAL);
> +}
> +
>   /*
>    * Scan the rbd device's current snapshot list and compare it to the
>    * newly-received snapshot context.  Remove any existing snapshots
> @@ -2366,7 +2442,7 @@ static int rbd_dev_snaps_update(struct rbd_device
> *rbd_dev)
>   			continue;
>   		}
>
> -		snap_name = rbd_dev_v1_snap_info(rbd_dev, index,
> +		snap_name = rbd_dev_snap_info(rbd_dev, index,
>   						&snap_size, &snap_features);

This line's indentation is offset now.

>   		if (IS_ERR(snap_name))
>   			return PTR_ERR(snap_name);
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/9] rbd: update remaining header fields for v2
  2012-09-07 21:15 ` [PATCH 7/9] rbd: update remaining header fields for v2 Alex Elder
@ 2012-09-19 19:38   ` Josh Durgin
  0 siblings, 0 replies; 20+ messages in thread
From: Josh Durgin @ 2012-09-19 19:38 UTC (permalink / raw)
  To: Alex Elder; +Cc: ceph-devel

On 09/07/2012 02:15 PM, Alex Elder wrote:
> There are three fields that are not yet updated for format 2 rbd
> image headers:  the version of the header object; the encryption
> type; and the compression type.  There is no interface defined for
> fetching the latter two, so just initialize them explicitly to 0 for
> now.

Encryption and compression type are never actually used, even for
format 1 images. They were just placeholders.

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

> Change rbd_dev_v2_snap_context() so the caller can be supplied the
> version for the header object.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index c6922a1..ad27be2 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2235,7 +2235,7 @@ static int rbd_dev_v2_features(struct rbd_device
> *rbd_dev)
>   						&rbd_dev->header.features);
>   }
>
> -static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev)
> +static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev, u64 *ver)
>   {
>   	size_t size;
>   	int ret;
> @@ -2263,7 +2263,7 @@ static int rbd_dev_v2_snap_context(struct
> rbd_device *rbd_dev)
>   				"rbd", "get_snapcontext",
>   				NULL, 0,
>   				reply_buf, size,
> -				CEPH_OSD_FLAG_READ, NULL);
> +				CEPH_OSD_FLAG_READ, ver);
>   	dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret);
>   	if (ret < 0)
>   		goto out;
> @@ -2899,6 +2899,7 @@ static int rbd_dev_v2_probe(struct rbd_device
> *rbd_dev)
>   {
>   	size_t size;
>   	int ret;
> +	u64 ver = 0;
>
>   	/*
>   	 * Image id was filled in by the caller.  Record the header
> @@ -2929,11 +2930,18 @@ static int rbd_dev_v2_probe(struct rbd_device
> *rbd_dev)
>   	if (ret < 0)
>   		goto out_err;
>
> -	/* Get the snapshot context */
> +	/* crypto and compression type aren't (yet) supported for v2 images */
> +
> +	rbd_dev->header.crypt_type = 0;
> +	rbd_dev->header.comp_type = 0;
>
> -	ret = rbd_dev_v2_snap_context(rbd_dev);
> +	/* Get the snapshot context, plus the header version */
> +
> +	ret = rbd_dev_v2_snap_context(rbd_dev, &ver);
>   	if (ret)
>   		goto out_err;
> +	rbd_dev->header.obj_version = ver;
> +
>   	rbd_dev->image_format = 2;
>
>   	dout("discovered version 2 image, header name is %s\n",
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 9/9] rbd: activate v2 image support
  2012-09-07 21:15 ` [PATCH 9/9] rbd: activate v2 image support Alex Elder
@ 2012-09-19 19:56   ` Josh Durgin
  2012-09-19 19:57     ` Josh Durgin
  0 siblings, 1 reply; 20+ messages in thread
From: Josh Durgin @ 2012-09-19 19:56 UTC (permalink / raw)
  To: Alex Elder; +Cc: ceph-devel

Until layering is implemented, this should report an
error if features is not 0. Layering is feature bit 1.

You can copy the userspace header from ceph.git in
src/include/librbd/features.h to use the same feature macros.

On 09/07/2012 02:15 PM, Alex Elder wrote:
> Now that v2 images support is fully implemented, have
> rbd_dev_v2_probe() return 0 to indicate a successful probe.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index b466393..2f83f71 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2973,7 +2973,7 @@ static int rbd_dev_v2_probe(struct rbd_device
> *rbd_dev)
>   	dout("discovered version 2 image, header name is %s\n",
>   		rbd_dev->header_name);
>
> -	return -ENOTSUPP;
> +	return 0;
>   out_err:
>   	kfree(rbd_dev->header_name);
>   	rbd_dev->header_name = NULL;
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 9/9] rbd: activate v2 image support
  2012-09-19 19:56   ` Josh Durgin
@ 2012-09-19 19:57     ` Josh Durgin
  0 siblings, 0 replies; 20+ messages in thread
From: Josh Durgin @ 2012-09-19 19:57 UTC (permalink / raw)
  To: Alex Elder; +Cc: ceph-devel

On 09/19/2012 12:56 PM, Josh Durgin wrote:
> Until layering is implemented, this should report an
> error if features is not 0. Layering is feature bit 1.
>
> You can copy the userspace header from ceph.git in
> src/include/librbd/features.h to use the same feature macros.

err, that's src/include/rbd/features.h

> On 09/07/2012 02:15 PM, Alex Elder wrote:
>> Now that v2 images support is fully implemented, have
>> rbd_dev_v2_probe() return 0 to indicate a successful probe.
>>
>> Signed-off-by: Alex Elder <elder@inktank.com>
>> ---
>>   drivers/block/rbd.c |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>> index b466393..2f83f71 100644
>> --- a/drivers/block/rbd.c
>> +++ b/drivers/block/rbd.c
>> @@ -2973,7 +2973,7 @@ static int rbd_dev_v2_probe(struct rbd_device
>> *rbd_dev)
>>       dout("discovered version 2 image, header name is %s\n",
>>           rbd_dev->header_name);
>>
>> -    return -ENOTSUPP;
>> +    return 0;
>>   out_err:
>>       kfree(rbd_dev->header_name);
>>       rbd_dev->header_name = NULL;
>>
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 8/9] rbd: define rbd_dev_v2_snapc_refresh()
  2012-09-07 21:15 ` [PATCH 8/9] rbd: define rbd_dev_v2_snapc_refresh() Alex Elder
@ 2012-09-19 21:00   ` Josh Durgin
  0 siblings, 0 replies; 20+ messages in thread
From: Josh Durgin @ 2012-09-19 21:00 UTC (permalink / raw)
  To: Alex Elder; +Cc: ceph-devel


On 09/07/2012 02:15 PM, Alex Elder wrote:
> Define a new function rbd_dev_v2_snapc_refresh() to update/refresh
> the snapshot context for a format version 2 rbd image.
>
> Update rbd_refresh_header() so it selects which function to use
> based on the image format.
>
> Rename __rbd_refresh_header() to be rbd_dev_v1_snapc_refresh()
> to be consistent with the naming of its version 2 counterpart.
> Similarly rename rbd_refresh_header to be rbd_dev_snapc_refresh().

rbd_refresh_header refreshes all fields that may have changed, not
just the snapshot context. The size of the image may also have changed,
and in the future there will be others. Advisory locks may have changed
(supporting those in the kernel is a separate issue, but it's another
thing stored in the header that may change).

Format 2 should reread the size just like format 1, although the
part of __rbd_refresh_header that checks for resizes could be
moved to a separate function called for both formats.

> Unrelated--we use rbd_image_format_valid() here.  Delete the other
> use of it, which was primarily put in place to ensure that function
> was referenced at the time it was defined.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   41 +++++++++++++++++++++++++++++++++--------
>   1 file changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index ad27be2..b466393 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -268,7 +268,8 @@ static void rbd_put_dev(struct rbd_device *rbd_dev)
>   	put_device(&rbd_dev->dev);
>   }
>
> -static int rbd_refresh_header(struct rbd_device *rbd_dev, u64 *hver);
> +static int rbd_dev_snapc_refresh(struct rbd_device *rbd_dev, u64 *hver);
> +static int rbd_dev_v2_snapc_refresh(struct rbd_device *rbd_dev, u64 *hver);
>
>   static int rbd_open(struct block_device *bdev, fmode_t mode)
>   {
> @@ -1303,7 +1304,7 @@ static void rbd_watch_cb(u64 ver, u64 notify_id,
> u8 opcode, void *data)
>   	dout("rbd_watch_cb %s notify_id=%llu opcode=%u\n",
>   		rbd_dev->header_name, (unsigned long long) notify_id,
>   		(unsigned int) opcode);
> -	rc = rbd_refresh_header(rbd_dev, &hver);
> +	rc = rbd_dev_snapc_refresh(rbd_dev, &hver);
>   	if (rc)
>   		pr_warning(RBD_DRV_NAME "%d got notification but failed to "
>   			   " update snaps: %d\n", rbd_dev->major, rc);
> @@ -1718,7 +1719,7 @@ static void __rbd_remove_all_snaps(struct
> rbd_device *rbd_dev)
>   /*
>    * only read the first part of the ondisk header, without the snaps info
>    */
> -static int __rbd_refresh_header(struct rbd_device *rbd_dev, u64 *hver)
> +static int rbd_dev_v1_snapc_refresh(struct rbd_device *rbd_dev, u64 *hver)
>   {
>   	int ret;
>   	struct rbd_image_header h;
> @@ -1767,12 +1768,16 @@ static int __rbd_refresh_header(struct
> rbd_device *rbd_dev, u64 *hver)
>   	return ret;
>   }
>
> -static int rbd_refresh_header(struct rbd_device *rbd_dev, u64 *hver)
> +static int rbd_dev_snapc_refresh(struct rbd_device *rbd_dev, u64 *hver)
>   {
>   	int ret;
>
> +	rbd_assert(rbd_image_format_valid(rbd_dev->image_format));
>   	mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
> -	ret = __rbd_refresh_header(rbd_dev, hver);
> +	if (rbd_dev->image_format == 1)
> +		ret = rbd_dev_v1_snapc_refresh(rbd_dev, hver);
> +	else
> +		ret = rbd_dev_v2_snapc_refresh(rbd_dev, hver);
>   	mutex_unlock(&ctl_mutex);
>
>   	return ret;
> @@ -1932,7 +1937,7 @@ static ssize_t rbd_image_refresh(struct device *dev,
>   	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
>   	int ret;
>
> -	ret = rbd_refresh_header(rbd_dev, NULL);
> +	ret = rbd_dev_snapc_refresh(rbd_dev, NULL);
>
>   	return ret < 0 ? ret : size;
>   }
> @@ -2389,6 +2394,27 @@ static char *rbd_dev_snap_info(struct rbd_device
> *rbd_dev, u32 which,
>   	return ERR_PTR(-EINVAL);
>   }
>
> +static int rbd_dev_v2_snapc_refresh(struct rbd_device *rbd_dev, u64 *hver)
> +{
> +	int ret;
> +
> +	down_write(&rbd_dev->header_rwsem);
> +	ret = rbd_dev_v2_snap_context(rbd_dev, hver);
> +	dout("rbd_dev_v2_snap_context returned %d\n", ret);
> +	if (ret)
> +		goto out;
> +	ret = rbd_dev_snaps_update(rbd_dev);
> +	dout("rbd_dev_snaps_update returned %d\n", ret);
> +	if (ret)
> +		goto out;
> +	ret = rbd_dev_snaps_register(rbd_dev);
> +	dout("rbd_dev_snaps_register returned %d\n", ret);
> +out:
> +	up_write(&rbd_dev->header_rwsem);
> +
> +	return 0;
> +}
> +
>   /*
>    * Scan the rbd device's current snapshot list and compare it to the
>    * newly-received snapshot context.  Remove any existing snapshots
> @@ -2551,7 +2577,7 @@ static int rbd_init_watch_dev(struct rbd_device
> *rbd_dev)
>   	do {
>   		ret = rbd_req_sync_watch(rbd_dev);
>   		if (ret == -ERANGE) {
> -			rc = rbd_refresh_header(rbd_dev, NULL);
> +			rc = rbd_dev_snapc_refresh(rbd_dev, NULL);
>   			if (rc < 0)
>   				return rc;
>   		}
> @@ -3034,7 +3060,6 @@ static ssize_t rbd_add(struct bus_type *bus,
>   	rc = rbd_dev_probe(rbd_dev);
>   	if (rc < 0)
>   		goto err_out_client;
> -	rbd_assert(rbd_image_format_valid(rbd_dev->image_format));
>
>   	rc = rbd_dev_snaps_update(rbd_dev);
>   	if (rc)
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2012-09-19 21:00 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-07 21:09 [PATCH 0/9] rbd: activate v2 image support Alex Elder
2012-09-07 21:13 ` [PATCH 1/9] rbd: lay out header probe infrastructure Alex Elder
2012-09-19 18:35   ` Josh Durgin
2012-09-07 21:13 ` [PATCH 2/9] rbd: add code to get the size of a v2 rbd image Alex Elder
2012-09-19 18:52   ` Josh Durgin
2012-09-07 21:13 ` [PATCH 3/9] rbd: get the object prefix for " Alex Elder
2012-09-19 18:57   ` Josh Durgin
2012-09-07 21:13 ` [PATCH 4/9] rbd: get image features for a v2 image Alex Elder
2012-09-19 19:02   ` Josh Durgin
2012-09-07 21:13 ` [PATCH 5/9] rbd: get the snapshot context " Alex Elder
2012-09-19 19:17   ` Josh Durgin
2012-09-07 21:14 ` [PATCH 6/9] rbd: get snapshot name " Alex Elder
2012-09-19 19:31   ` Josh Durgin
2012-09-07 21:15 ` [PATCH 7/9] rbd: update remaining header fields for v2 Alex Elder
2012-09-19 19:38   ` Josh Durgin
2012-09-07 21:15 ` [PATCH 8/9] rbd: define rbd_dev_v2_snapc_refresh() Alex Elder
2012-09-19 21:00   ` Josh Durgin
2012-09-07 21:15 ` [PATCH 9/9] rbd: activate v2 image support Alex Elder
2012-09-19 19:56   ` Josh Durgin
2012-09-19 19:57     ` Josh Durgin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.